Re: [PATCH 1/1] driver core: disable device's runtime pm during shutdown

[Ming Lei]

Hi,

On Mon, Dec 5, 2011 at 1:12 PM, Ming Lei <tom.leiming@xxxxxxxxx> wrote:
> Hi,
>
> On Mon, Dec 5, 2011 at 12:42 PM, Chen Peter-B29397 <B29397@xxxxxxxxxxxxx> 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");

thanks,
-- 
Ming Lei
--
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/