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 for Android: free password hash cracker in your pocket

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

Message-ID: <20111205143717.13cf6015@notabene.brown>
Date:	Mon, 5 Dec 2011 14:37:17 +1100
From:	NeilBrown <neilb@...e.de>
To:	Alan Stern <stern@...land.harvard.edu>
Cc:	"Rafael J. Wysocki" <rjw@...k.pl>, Greg KH <greg@...ah.com>,
	Peter Chen <peter.chen@...escale.com>, <gregkh@...e.de>,
	<linux-kernel@...r.kernel.org>, <linux-pm@...r.kernel.org>,
	<hzpeterchen@...il.com>, Igor Grinberg <grinberg@...pulab.co.il>
Subject: Re: [PATCH 1/1] driver core: disable device's runtime pm during
 shutdown

On Sun, 4 Dec 2011 21:42:02 -0500 (EST) Alan Stern
<stern@...land.harvard.edu> wrote:

> On Mon, 5 Dec 2011, NeilBrown wrote:
> 
> > Hi,
> >  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.
> 
> In other words, to perform the system shutdown you need to send a 
> command over the i2c bus after the bus controller's shutdown routine 
> has been called?

Correct.
Though in this case the i2c bus doesn't have a shutdown routine so it doesn't
cause a problem.  It is only the inability to pm_resume that is a problem.

> 
> > So here is a device that should *not* have pm disabled at shutdown.
> 
> Or maybe it shouldn't be shut down at all.

Maybe not.  Current 'device_shutdown()' shuts down every device.
I guess having a "don't shutdown at power-off" flag could be used to solve
the problem.

> 
> > So I feel this fix is a little too heavy-handed.
> > I don't fully understand the problem scenario described above but it seems to
> > me that if the auto-suspend timer can fire after the hardware has been shut
> > down, then maybe the hardware-shutdown should be disabling that timer.  Maybe?
> 
> That's not robust.  The timer can be restarted, and there are other 
> ways of initiating runtime PM besides the timer.

Still, shouldn't they notice that the hardware has been shutdown and so not
do anything?

> 
> > Suggestions?
> 
> Can the shutdown routine for the i2c controller simply call
> pm_runtime_enable()?

No.
The shutdown routine is in drivers/mfd/twl4030-power.c (not in mainline
currently but there are various patches floating around that add
twl4030_poweroff, and this is the obviously-correct place for the code).
It has a 'struct i2c_adapter' device.
The device that needs to have pm_runtime_enable called on it is a 'struct
platform_device' that is linked through the dev_data of the i2c_adapter.

i.e. only omap-i2c specific code knows how to find the device that needs to
be enabled.  The twl4030 code doesn't.

So even if we had a used_for_shutdown flag in dev_pm_info I don't know how we
would manage to set it for the right device.... unless we had a dev_pm_ops
method to ask a device to set it on all devices it depends on.  Sounds messy.

Thanks,
NeilBrown

Download attachment "signature.asc" of type "application/pgp-signature" (829 bytes)