linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Walleij <linus.walleij@linaro.org>
To: "Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Wolfram Sang <wsa@the-dreams.de>, Mark Brown <broonie@kernel.org>,
	Ulf Hansson <ulf.hansson@linaro.org>
Cc: linux-pm@vger.kernel.org, Linus Walleij <linus.walleij@linaro.org>
Subject: [PATCH 2/3] i2c: let I2C masters inherit suspend child ignorance
Date: Thu,  7 Apr 2016 15:20:36 +0200	[thread overview]
Message-ID: <1460035237-12037-3-git-send-email-linus.walleij@linaro.org> (raw)
In-Reply-To: <1460035237-12037-1-git-send-email-linus.walleij@linaro.org>

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
is 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 has not inherited the
  child ignorance and thus flags 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 inherit the
child ignorance setting from their parent when registering
an adapter. This way the child will realize it is ignored
and it can proceed to be resumed.

Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: Mark Brown <broonie@kernel.org>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
Wolfram: if I can convince you and Rafael that this is the
right thing to do, I guess Rafael could eventually merge
this oneliner through his PM tree with your ACK.
---
 drivers/i2c/i2c-core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 0f2f8484e8ec..dd12d4a162ed 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1566,6 +1566,7 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
 
 	pm_runtime_no_callbacks(&adap->dev);
 	pm_runtime_enable(&adap->dev);
+	pm_suspend_inherit_ignore_children(&adap->dev);
 
 #ifdef CONFIG_I2C_COMPAT
 	res = class_compat_create_link(i2c_adapter_compat_class, &adap->dev,
-- 
2.4.3


  parent reply	other threads:[~2016-04-07 13:21 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-07 13:20 [PATCH 0/3] PM / sleep: let slow buses inherit child ignorance Linus Walleij
2016-04-07 13:20 ` [PATCH 1/3] PM / sleep: add a helper function to " Linus Walleij
2016-04-11 11:54   ` Linus Walleij
2016-04-07 13:20 ` Linus Walleij [this message]
2016-04-11  6:53   ` [PATCH 2/3] i2c: let I2C masters inherit suspend " Linus Walleij
2016-04-11  6:58     ` Mark Brown
2016-04-07 13:20 ` [PATCH 3/3] RFC: spi: let SPI " Linus Walleij
2016-04-07 17:06   ` Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1460035237-12037-3-git-send-email-linus.walleij@linaro.org \
    --to=linus.walleij@linaro.org \
    --cc=broonie@kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=rjw@rjwysocki.net \
    --cc=ulf.hansson@linaro.org \
    --cc=wsa@the-dreams.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).