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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
