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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACVXFVOc45HL_qPHvNcGrkcQ7XWxBoLHikrYd1ZrRMYkiw5=ag@mail.gmail.com>
Date:	Mon, 5 Dec 2011 17:05:18 +0800
From:	Ming Lei <tom.leiming@...il.com>
To:	Chen Peter-B29397 <B29397@...escale.com>
Cc:	NeilBrown <neilb@...e.de>, "Rafael J. Wysocki" <rjw@...k.pl>,
	Greg KH <greg@...ah.com>, "gregkh@...e.de" <gregkh@...e.de>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	"stern@...land.harvard.edu" <stern@...land.harvard.edu>,
	"hzpeterchen@...il.com" <hzpeterchen@...il.com>,
	Igor Grinberg <grinberg@...pulab.co.il>
Subject: Re: [PATCH 1/1] driver core: disable device's runtime pm during shutdown

Hi,

On Mon, Dec 5, 2011 at 5:01 PM, Ming Lei <tom.leiming@...il.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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/