On Fri, 1 Jul 2011, Kevin Hilman wrote:
> >> --- a/drivers/base/dd.c
> >> +++ b/drivers/base/dd.c
> >> @@ -329,13 +329,13 @@ static void __device_release_driver(struct device
> >> *dev)
> >> blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
> >> BUS_NOTIFY_UNBIND_DRIVER,
> >> dev);
> >> -
> >> - pm_runtime_put_sync(dev);
> >> -
> >> if (dev->bus && dev->bus->remove)
> >> dev->bus->remove(dev);
> >> else if (drv->remove)
> >> drv->remove(dev);
> >> +
> >> + pm_runtime_put_sync(dev);
> >> +
> >> devres_release_all(dev);
> >> dev->driver = NULL;
> >> klist_remove(&dev->p->knode_driver);
> >
> > To be safer, the put_sync() call should be moved down here. Or maybe
> > even after the blocking_notifier_call_chain() that occurs here.
>
> I was actually thinking about the other direction: moving the get_sync
> after the first notifier chain. IOW, the get_sync/put_sync only
> protects the ->remove() calls, not the notifiers.
>
> The protection around the notifiers doesn't make sense to me, at least
> in the context of driver runtime PM racing with the subsystem.
> Especially since these notifiers are likely how the
> subsystem/bus/pm_domain code getting notified that there may be a device
> to manage in the first place.
The get_sync part doesn't matter so much. Moving it past the notifier
call would probably be okay -- unless one of the listeners on the
notifier chain expects the device to be active. Changing the get_sync
to get_noresume would probably also be okay -- subject to a similar
reservation.
The problem with the put_sync isn't the notifier. If you leave it
where you've got it now, you'll end up invoking a callback at a time
when the driver thinks it no longer controls the device but the
driver-model core still thinks it does. You certainly want to do the
dev->driver = NULL;
first.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
