Re: [PATCH 5/5] arm: omap: Proper cleanups for omap_device

[Tony Lindgren]

* Pantelis Antoniou <panto@xxxxxxxxxxxxxxxxxxxxxxx> [130806 02:44]:
> On Aug 6, 2013, at 12:33 PM, Greg Kroah-Hartman wrote:
> > On Tue, Aug 06, 2013 at 10:53:44AM +0300, Pantelis Antoniou wrote:
> >> +
> >> static int _omap_device_notifier_call(struct notifier_block *nb,
> >> 				      unsigned long event, void *dev)
> >> {
> >> @@ -185,9 +211,13 @@ static int _omap_device_notifier_call(struct notifier_block *nb,
> >> 	struct omap_device *od;
> >> 
> >> 	switch (event) {
> >> -	case BUS_NOTIFY_DEL_DEVICE:
> >> +	case BUS_NOTIFY_UNBOUND_DRIVER:
> >> +		/* NOTIFY_DEL_DEVICE is not the right call...
> >> +		 * we use a callback here, to make sure no-one is going to
> >> +		 * try to use the omap_device data after they're deleted
> >> +		 */
> >> 		if (pdev->archdata.od)
> >> -			omap_device_delete(pdev->archdata.od);
> >> +			device_schedule_callback(dev, _omap_device_cleanup);
> > 
> > Really?  This is one sign that you are totally using the driver core
> > incorrectly.  You shouldn't have to rely on notifier callbacks to handle
> > device removals, your bus code should do that for you directly.
> > 
> > I don't like this at all, sorry.
> > 
> 
> Don't shoot the messenger please...

As you're inititalizing capebus with DT, let's figure out what if
anything you actually need from omap_device. I'd much rather remove
dependencies than add more.

If you need omap_device for the clocks, there are patches pending to
make them DT only for omaps. And we already have DT based solution for
pins, regulators and DMA.

So what else remains? The pieces needed for runtime PM?
 
> This is all about fixing a crash without messing too many things.

It seems this fix is only needed for supporting out-of-tree code?
These features with omap_device we may not even want to support in
the mainline tree as is being discussed..

Regards,

Tony

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/