Messages in this thread Patch in this message | | | Date | Mon, 5 Dec 2011 17:05:18 +0800 | Subject | Re: [PATCH 1/1] driver core: disable device's runtime pm during shutdown | From | Ming Lei <> |
| |
Hi,
On Mon, Dec 5, 2011 at 5:01 PM, Ming Lei <tom.leiming@gmail.com> wrote:
>>> >>> >>>> > this patches causes a problem for me. >>>> > >>>> > Specifically it makes it impossible to power-down a device which uses >>>> twl4030 >>>> > for power control on an omap3 processor. >>>> > >>>> > To perform the shutdown we need to send a command over the i2c bus. >>>> > The relevant bus is called omap_i2c.1 and this is normally in suspend >>>> mode. >>>> > When a request is sent, omap_i2c_xfer uses pm_runtime_get_sync to wake >>>> it up, >>>> > performs the transfer, then calls pm_runtime_put to let it go back to >>>> sleep. >>>> > >>>> > So it is asleep when the new pm_runtime_disable() call is made, so it >>>> stays >>>> > asleep, omap_i2c_xfer cannot wake it, the transfer doesn't happen and >>>> the >>>> > system doesn't get powered off. >>>> > >>>> > So here is a device that should *not* have pm disabled at shutdown. >>>> > >>>> > So I feel this fix is a little too heavy-handed. >>>> >>>> Maybe the device's runtime PM should not be disabled if >>>> there is no ->shutdown defined in its driver, how about the blew? >>>> >>>> diff --git a/drivers/base/core.c b/drivers/base/core.c >>>> index d8b3d89..ca30659 100644 >>>> --- a/drivers/base/core.c >>>> +++ b/drivers/base/core.c >>>> @@ -1743,14 +1743,16 @@ void device_shutdown(void) >>>> */ >>>> list_del_init(&dev->kobj.entry); >>>> spin_unlock(&devices_kset->list_lock); >>>> - /* Disable all device's runtime power management */ >>>> - pm_runtime_disable(dev); >>>> >>>> + /* Disable the device's runtime power management if >>>> + * it is to be shutdown*/ >>>> if (dev->bus && dev->bus->shutdown) { >>>> dev_dbg(dev, "shutdown\n"); >>>> + pm_runtime_disable(dev); >>>> dev->bus->shutdown(dev); >>>> } else if (dev->driver && dev->driver->shutdown) { >>>> dev_dbg(dev, "shutdown\n"); >>>> + pm_runtime_disable(dev); >>>> dev->driver->shutdown(dev); >>>> } >>>> put_device(dev); >>>> >>>> >>> Yes, it is the solution for Neil's case, but I am not sure for all. >>> Is it possible to resume driver's itself at driver's shutdown? >> >> Looks like this way is still not good. Maybe we should put all >> device into active state first, then set its runtime pm as disabled. > > Below patch should a simple fix on the issue, but it may prolong > shutdown/reboot time. > > > diff --git a/drivers/base/core.c b/drivers/base/core.cindex > d8b3d89..96b266c 100644--- a/drivers/base/core.c+++ > b/drivers/base/core.c@@ -1743,8 +1743,9 @@ void device_shutdown(void) > */ list_del_init(&dev->kobj.entry); > spin_unlock(&devices_kset->list_lock);- /* Disable all device's > runtime power management */- pm_runtime_disable(dev);++ /* put > device into active state and forbid runtime pm > */+ pm_runtime_forbid(dev); if (dev->bus && dev->bus->shutdown) { > dev_dbg(dev, "shutdown\n"); , Sorry, the above is line wrapped badly, see the below:
diff --git a/drivers/base/core.c b/drivers/base/core.c index d8b3d89..96b266c 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -1743,8 +1743,9 @@ void device_shutdown(void) */ list_del_init(&dev->kobj.entry); spin_unlock(&devices_kset->list_lock); - /* Disable all device's runtime power management */ - pm_runtime_disable(dev); + + /* put device into active state and forbit runtime pm */ + pm_runtime_forbid(dev);
if (dev->bus && dev->bus->shutdown) { dev_dbg(dev, "shutdown\n");
thanks, -- Ming Lei -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |