Re: [PATCH v3] i2c: let I2C masters ignore their children for PM

[Wolfram Sang <wsa@the-dreams.de>]

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

On Tue, Apr 12, 2016 at 09:57:35AM +0200, Linus Walleij wrote:
> When using a certain I2C device with runtime PM enabled on
> a certain I2C bus adaper the following happens:
> 
> struct amba_device *foo
>    \
>    struct i2c_adapter *bar
>       \
>       struct i2c_client *baz
> 
> The AMBA device foo has its device PM struct set to ignore
> children with pm_suspend_ignore_children(&foo->dev, true).
> This makes runtime PM work just fine locally in the driver:
> the fact that devices on the bus are suspended or resumed
> individually does not affect its operation, and the hardware
> does not power up unless transferring messages.
> 
> However this child ignorance property is not inherited into
> the struct i2c_adapter *bar.
> 
> On system suspend things will work fine.
> 
> On system resume the following annoying phenomenon occurs:
> 
> - In the pm_runtime_force_resume() path of
>   struct i2c_client *baz, pm_runtime_set_active(&baz->dev); is
>   eventually called.
> 
> - This becomes __pm_runtime_set_status(&baz->dev, RPM_ACTIVE);
> 
> - __pm_runtime_set_status() detects that RPM state is changed,
>   and checks whether the parent is:
>   not active (RPM_ACTIVE) and not ignoring its children
>   If this happens it concludes something is wrong, because
>   a parent that is not ignoring its children must be active
>   before any children activate.
> 
> - Since the struct i2c_adapter *bar does not ignore
>   its children, the PM core thinks that it must indeed go
>   online before its children, the check bails out with
>   -EBUSY, i.e. the i2c_client *baz thinks it can't work
>   because it's parent is not online, and it respects its
>   parent.
> 
> - In the driver the .resume() callback returns -EBUSY from
>   the runtime_force_resume() call as per above. This leaves
>   the device in a suspended state, leading to bad behaviour
>   later when the device is used. The following debug
>   print is made with an extra printg patch but illustrates
>   the problem:
> 
> [   17.040832] bh1780 2-0029: parent (i2c-2) is not active
>                parent->power.ignore_children = 0
> [   17.040832] bh1780 2-0029: pm_runtime_force_resume:
>                pm_runtime_set_active() failed (-16)
> [   17.040863] dpm_run_callback():
>                pm_runtime_force_resume+0x0/0x88 returns -16
> [   17.040863] PM: Device 2-0029 failed to resume: error -16
> 
> Fix this by letting all struct i2c_adapter:s ignore their
> children: i2c children have no business doing keeping
> their parents awake: they are completely autonomous
> devices that just use their parent to talk, a usecase
> which must be power managed in the host on a per-message
> basis.
> 
> Cc: Wolfram Sang <wsa@the-dreams.de>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> Cc: linux-pm@vger.kernel.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Applied to for-next, thanks!


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