linux-i2c.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ulf Hansson <ulf.hansson@linaro.org>
To: Wolfram Sang <wsa@the-dreams.de>,
	"Rafael J . Wysocki" <rjw@rjwysocki.net>,
	Len Brown <lenb@kernel.org>,
	linux-acpi@vger.kernel.org, linux-pm@vger.kernel.org
Cc: Kevin Hilman <khilman@kernel.org>,
	Jarkko Nikula <jarkko.nikula@linux.intel.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	Jisheng Zhang <jszhang@marvell.com>,
	John Stultz <john.stultz@linaro.org>,
	Guodong Xu <guodong.xu@linaro.org>,
	Sumit Semwal <sumit.semwal@linaro.org>,
	Haojian Zhuang <haojian.zhuang@linaro.org>,
	linux-arm-kernel@lists.infradead.org, linux-i2c@vger.kernel.org,
	Ulf Hansson <ulf.hansson@linaro.org>,
	stable@vger.linux.org
Subject: [PATCH 1/9] i2c: designware: Fix system suspend
Date: Wed, 21 Jun 2017 21:21:20 +0200	[thread overview]
Message-ID: <1498072888-14782-2-git-send-email-ulf.hansson@linaro.org> (raw)
In-Reply-To: <1498072888-14782-1-git-send-email-ulf.hansson@linaro.org>

The commit 8503ff166504 ("i2c: designware: Avoid unnecessary resuming
during system suspend"), may suggest to the PM core to try out the so
called direct_complete path for system sleep. In this path, the PM core
treats a runtime suspended device as it's already in a proper low power
state for system sleep, which makes it skip calling the system sleep
callbacks for the device, except for the ->prepare() and the ->complete()
callback.

Moreover, under certain circumstances the PM core may unset the
direct_complete flag for a parent device, in case its child device are
being system suspended before. In other words, the PM core doesn't skip
calling the system sleep callbacks, no matter if the device is runtime
suspended or not.

In cases of an i2c slave device, the above situation is triggered.
Unfortunate, this also breaks the assumption that the i2c device is always
runtime resumed, whenever the dw_i2c_plat_suspend() callback is being
invoked, which then leads to a regression.

More precisely, dw_i2c_plat_suspend() then calls clk_core_disable() and
clk_core_unprepare(), for an already disabled/unprepared clock, leading to
complaints about clocks calls being wrongly balanced.

In cases when the i2c device is attached to the ACPI PM domain, the problem
doesn't occur. That's because ACPI's ->suspend() callback, assigned to
acpi_subsys_suspend(), calls pm_runtime_resume() for the i2c device.

To make a simple fix for this regression, let's runtime resume the i2c
device in the ->prepare() callback, assigned to dw_i2c_plat_prepare(). This
prevents the direct_complete path from being executed by the PM core and
guarantees the dw_i2c_plat_suspend() is called with the i2c device always
being runtime resumed.

Of course this change is suboptimal, because to always force a runtime
resume of the i2c device in ->prepare() is a waste, especially in those
cases when it could have remained runtime suspended during the entire
system sleep sequence. However, to accomplish that behaviour a bigger
change is needed, so defer that to future changes not applicable as fixes
or for stable.

Fixes: 8503ff166504 ("i2c: designware: Avoid unnecessary resuming...")
Cc: stable@vger.linux.org
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/i2c/busses/i2c-designware-platdrv.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index d1263b8..2b7fa75 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -375,17 +375,11 @@ MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
 #ifdef CONFIG_PM_SLEEP
 static int dw_i2c_plat_prepare(struct device *dev)
 {
-	return pm_runtime_suspended(dev);
-}
-
-static void dw_i2c_plat_complete(struct device *dev)
-{
-	if (dev->power.direct_complete)
-		pm_request_resume(dev);
+	pm_runtime_resume(dev);
+	return 0;
 }
 #else
 #define dw_i2c_plat_prepare	NULL
-#define dw_i2c_plat_complete	NULL
 #endif
 
 #ifdef CONFIG_PM
@@ -413,7 +407,6 @@ static int dw_i2c_plat_resume(struct device *dev)
 
 static const struct dev_pm_ops dw_i2c_dev_pm_ops = {
 	.prepare = dw_i2c_plat_prepare,
-	.complete = dw_i2c_plat_complete,
 	SET_SYSTEM_SLEEP_PM_OPS(dw_i2c_plat_suspend, dw_i2c_plat_resume)
 	SET_RUNTIME_PM_OPS(dw_i2c_plat_suspend, dw_i2c_plat_resume, NULL)
 };
-- 
2.7.4

  reply	other threads:[~2017-06-21 19:21 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-21 19:21 [PATCH 0/9] PM / ACPI / i2c: Fix system suspend and deploy runtime PM centric path for ACPI Ulf Hansson
2017-06-21 19:21 ` Ulf Hansson [this message]
2017-06-21 23:31   ` [PATCH 1/9] i2c: designware: Fix system suspend Rafael J. Wysocki
2017-06-22 10:49     ` Mika Westerberg
2017-06-22 11:16       ` Jarkko Nikula
2017-06-27  7:55         ` Jarkko Nikula
2017-06-28 14:01           ` Ulf Hansson
2017-06-28 14:51             ` Rafael J. Wysocki
2017-06-28 15:06               ` Ulf Hansson
2017-06-22 14:41       ` Rafael J. Wysocki
2017-06-22 21:37         ` Ulf Hansson
2017-06-22 22:01           ` Rafael J. Wysocki
2017-06-26 16:49             ` Ulf Hansson
2017-06-26 19:39               ` Grygorii Strashko
2017-06-26 21:11                 ` Rafael J. Wysocki
2017-06-27  7:33                   ` Jarkko Nikula
2017-06-27 15:25                   ` Rafael J. Wysocki
2017-06-28 14:31                 ` Ulf Hansson
2017-06-28 16:52                   ` Grygorii Strashko
2017-09-08  3:23   ` [1/9] " Wangtao (Kevin, Kirin)
2017-09-08  8:29     ` Ulf Hansson
2017-09-12  9:44       ` Wangtao (Kevin, Kirin)
2017-06-21 19:21 ` [PATCH 2/9] PM / ACPI: Restore acpi_subsys_complete() Ulf Hansson
2017-06-21 19:21 ` [PATCH 3/9] PM / Sleep: Remove pm_complete_with_resume_check() Ulf Hansson
2017-06-21 19:21 ` [PATCH 4/9] PM / ACPI: Split code validating need for runtime resume in ->prepare() Ulf Hansson
2017-06-21 21:35   ` Rafael J. Wysocki
2017-06-21 19:21 ` [PATCH 5/9] PM / ACPI: Split acpi_lpss_suspend_late|resume_early() Ulf Hansson
2017-06-21 19:21 ` [PATCH 6/9] PM / ACPI: Provide option to disable direct_complete for ACPI devices Ulf Hansson
2017-06-21 21:42   ` Rafael J. Wysocki
2017-06-22  9:35     ` Ulf Hansson
2017-06-22 14:38       ` Rafael J. Wysocki
2017-06-22 21:29         ` Ulf Hansson
2017-06-22 21:33           ` Rafael J. Wysocki
2017-06-21 19:21 ` [PATCH 7/9] PM / ACPI: Enable the runtime PM centric approach for system sleep Ulf Hansson
2017-06-21 21:47   ` Rafael J. Wysocki
2017-06-22  9:42     ` Ulf Hansson
2017-06-22 14:32       ` Rafael J. Wysocki
2017-06-22 21:14         ` Ulf Hansson
2017-06-22 21:28           ` Rafael J. Wysocki
2017-06-21 19:21 ` [PATCH 8/9] PM / ACPI: Avoid runtime resuming device in acpi_subsys_suspend|freeze() Ulf Hansson
2017-06-21 19:21 ` [PATCH 9/9] i2c: designware: Deploy the runtime PM centric approach for system sleep Ulf Hansson
2017-06-21 22:39 ` [PATCH 0/9] PM / ACPI / i2c: Fix system suspend and deploy runtime PM centric path for ACPI John Stultz

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=1498072888-14782-2-git-send-email-ulf.hansson@linaro.org \
    --to=ulf.hansson@linaro.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=guodong.xu@linaro.org \
    --cc=haojian.zhuang@linaro.org \
    --cc=jarkko.nikula@linux.intel.com \
    --cc=john.stultz@linaro.org \
    --cc=jszhang@marvell.com \
    --cc=khilman@kernel.org \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mika.westerberg@linux.intel.com \
    --cc=rjw@rjwysocki.net \
    --cc=stable@vger.linux.org \
    --cc=sumit.semwal@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).