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

[NeilBrown <neilb@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 2902 bytes --]

On Sun, 4 Dec 2011 21:42:02 -0500 (EST) Alan Stern
<stern@rowland.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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]