public inbox for linux-acpi@vger.kernel.org
 help / color / mirror / Atom feed
From: "Rafael J. Wysocki" <rjw@rjwysocki.net>
To: linux-pm@vger.kernel.org, linux-i2c@vger.kernel.org
Cc: Wolfram Sang <wsa@the-dreams.de>,
	linux-acpi@vger.kernel.org, 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>,
	Johannes Stezenbach <js@sig21.net>,
	Ulf Hansson <ulf.hansson@linaro.org>
Subject: [RFT][PATCH v2 2/2] PM / i2c: designware: Clean up system sleep handling without ACPI
Date: Mon, 04 Sep 2017 12:01:54 +0200	[thread overview]
Message-ID: <1729594.8Da7J7iljp@aspire.rjw.lan> (raw)
In-Reply-To: <6044272.h4jTMoqRfC@aspire.rjw.lan>

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

After commit a23318feeff6 (i2c: designware: Fix system suspend)
the i2c-designware-platdrv driver always resumes the device in its
system sleep ->suspend callback which isn't particularly nice,
even though it is technically correct.

A better approach would be to make the driver track the PM state of
the device so that it doesn't need to resume it in ->suspend and
to drop its ->prepare and ->complete callbacks which are only
marginally useful, so implement that.

First, drop dw_i2c_plat_suspend() added by commit a23318feeff6 and
rename dw_i2c_plat_runtime_suspend() back to dw_i2c_plat_suspend().

Second, point the driver's ->late_suspend and ->early_resume
callbacks, rather than its ->suspend and ->resume callbacks,
to dw_i2c_plat_suspend() and dw_i2c_plat_resume(), respectively,
so that they are not executed in parallel with each other, for
example if runtime resume of the device takes place during system
suspend.

Next, add "suspended" and "skip_resume" flags to struct dw_i2c_dev
and make dw_i2c_plat_suspend() and dw_i2c_plat_resume() use them to
avoid suspending or resuming the device twice in a row and to avoid
resuming a previously runtime-suspended device during system resume.

Finally, drop the driver's ->prepare and ->complete PM callbacks,
because returning "true" from ->prepare for runtime-suspended
devices is marginally useful (the PM core may still ignore that
return value and invoke the driver's ->suspend callback anyway)
and ->complete is only needed because of what ->prepare does.

Overrides: a23318feeff6 (i2c: designware: Fix system suspend)
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

-> v2: Add the "suspended" flag, which is needed, because
       dw_i2c_plat_suspend() doesn't update the runtime PM
       status of the device, so dw_i2c_plat_resume() can't
       rely on the pm_runtime_status_suspended() check
       during system resume.

---
 drivers/i2c/busses/i2c-designware-core.h    |    2 +
 drivers/i2c/busses/i2c-designware-platdrv.c |   51 ++++++++++------------------
 2 files changed, 22 insertions(+), 31 deletions(-)

Index: linux-pm/drivers/i2c/busses/i2c-designware-core.h
===================================================================
--- linux-pm.orig/drivers/i2c/busses/i2c-designware-core.h
+++ linux-pm/drivers/i2c/busses/i2c-designware-core.h
@@ -280,6 +280,8 @@ struct dw_i2c_dev {
 	int			(*acquire_lock)(struct dw_i2c_dev *dev);
 	void			(*release_lock)(struct dw_i2c_dev *dev);
 	bool			pm_disabled;
+	bool			suspended;
+	bool			skip_resume;
 	void			(*disable)(struct dw_i2c_dev *dev);
 	void			(*disable_int)(struct dw_i2c_dev *dev);
 	int			(*init)(struct dw_i2c_dev *dev);
Index: linux-pm/drivers/i2c/busses/i2c-designware-platdrv.c
===================================================================
--- linux-pm.orig/drivers/i2c/busses/i2c-designware-platdrv.c
+++ linux-pm/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -434,31 +434,22 @@ static const struct of_device_id dw_i2c_
 MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
 #endif
 
-#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);
-}
-#else
-#define dw_i2c_plat_prepare	NULL
-#define dw_i2c_plat_complete	NULL
-#endif
-
 #ifdef CONFIG_PM
-static int dw_i2c_plat_runtime_suspend(struct device *dev)
+static int dw_i2c_plat_suspend(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
 	struct dw_i2c_dev *i_dev = platform_get_drvdata(pdev);
 
+	if (i_dev->suspended) {
+		i_dev->skip_resume = true;
+		return 0;
+	}
+
 	i_dev->disable(i_dev);
 	i2c_dw_plat_prepare_clk(i_dev, false);
 
+	i_dev->suspended = true;
+
 	return 0;
 }
 
@@ -467,27 +458,25 @@ static int dw_i2c_plat_resume(struct dev
 	struct platform_device *pdev = to_platform_device(dev);
 	struct dw_i2c_dev *i_dev = platform_get_drvdata(pdev);
 
+	if (!i_dev->suspended)
+		return 0;
+
+	if (i_dev->skip_resume) {
+		i_dev->skip_resume = false;
+		return 0;
+	}
+
 	i2c_dw_plat_prepare_clk(i_dev, true);
 	i_dev->init(i_dev);
 
-	return 0;
-}
+	i_dev->suspended = false;
 
-#ifdef CONFIG_PM_SLEEP
-static int dw_i2c_plat_suspend(struct device *dev)
-{
-	pm_runtime_resume(dev);
-	return dw_i2c_plat_runtime_suspend(dev);
+	return 0;
 }
-#endif
 
 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_runtime_suspend,
-			   dw_i2c_plat_resume,
-			   NULL)
+	SET_LATE_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)
 };
 
 #define DW_I2C_DEV_PMOPS (&dw_i2c_dev_pm_ops)


  reply	other threads:[~2017-09-04 10:10 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-03 23:05 [RFT][PATCH 0/2] i2c: designware: Runtime PM aware system sleep handling Rafael J. Wysocki
2017-09-03 23:08 ` [RFT][PATCH 1/2] i2c: designware: Clean up PM handling in dw_i2c_plat_probe() Rafael J. Wysocki
2017-09-05 14:40   ` Jarkko Nikula
2017-09-05 14:41     ` Rafael J. Wysocki
2017-09-03 23:14 ` [RFT][PATCH 2/2] PM / i2c: designware: Clean up system sleep handling without ACPI Rafael J. Wysocki
2017-09-04 10:01   ` Rafael J. Wysocki [this message]
2017-09-05 14:45     ` [RFT][PATCH v2 " Mika Westerberg
2017-09-05 14:46       ` Rafael J. Wysocki
2017-09-05 14:58         ` Mika Westerberg
2017-09-05 14:55           ` Rafael J. Wysocki
2017-09-05 15:02             ` Rafael J. Wysocki
2017-09-05 15:07             ` Mika Westerberg
2017-09-05 15:04               ` Rafael J. Wysocki
2017-09-05 15:24                 ` Mika Westerberg
2017-09-05 15:32                   ` Rafael J. Wysocki
2017-09-05 15:41                     ` Mika Westerberg
2017-09-05 21:00                       ` Johannes Stezenbach
2017-09-05 21:22                         ` Rafael J. Wysocki
2017-09-04 10:07   ` [RFT][PATCH " Rafael J. Wysocki
2017-09-05 15:46   ` Andy Shevchenko
2017-09-05 23:41 ` [RFT][PATCH v3 0/3] i2c: designware: Runtime PM aware system sleep handling Rafael J. Wysocki
2017-09-05 23:43   ` [RFT][PATCH v3 1/3] i2c: designware: Clean up PM handling in dw_i2c_plat_probe() Rafael J. Wysocki
2017-09-05 23:46   ` [RFT][PATCH v3 2/3] PM / mfd: intel-lpss: Push system sleep callbacks to late/early stages Rafael J. Wysocki
2017-09-05 23:48   ` [RFT][PATCH v3 3/3] PM / i2c: designware: Clean up system sleep handling Rafael J. Wysocki
2017-09-06  9:16   ` [RFT][PATCH v3 0/3] i2c: designware: Runtime PM aware " Johannes Stezenbach
2017-09-06  9:55     ` Rafael J. Wysocki
2017-09-06 19:59       ` Johannes Stezenbach
2017-09-06 21:37         ` Rafael J. Wysocki
2017-09-08  7:34       ` Johannes Stezenbach
2017-09-06 11:06   ` Mika Westerberg
2017-09-06 11:14     ` Rafael J. Wysocki
2017-09-06 13:46     ` Jarkko Nikula
2017-09-24 23:13   ` [PATCH v4 " Rafael J. Wysocki
2017-09-24 23:17     ` [PATCH v4 1/3] PM: i2c-designware-platdrv: Clean up PM handling in probe Rafael J. Wysocki
2017-09-25 19:12       ` Ulf Hansson
2017-09-25 20:45         ` Rafael J. Wysocki
2017-09-25 21:10       ` [Update][PATCH v5 " Rafael J. Wysocki
2017-10-05 11:00         ` Wolfram Sang
2017-09-24 23:20     ` [PATCH v4 2/3] PM / mfd: intel-lpss: Push system sleep callbacks to late/early stages Rafael J. Wysocki
2017-09-25  7:28       ` Wolfram Sang
2017-09-25 12:50         ` Rafael J. Wysocki
2017-09-25 14:24           ` Wolfram Sang
2017-10-02 23:58         ` Rafael J. Wysocki
2017-09-28 19:19       ` Lee Jones
2017-09-28 20:51         ` Rafael J. Wysocki
2017-10-05 11:00       ` Wolfram Sang
2017-09-24 23:30     ` [PATCH v4 3/3] PM: i2c-designware-platdrv: Suspend/resume at the " Rafael J. Wysocki
2017-10-05 11:00       ` Wolfram Sang

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=1729594.8Da7J7iljp@aspire.rjw.lan \
    --to=rjw@rjwysocki.net \
    --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=js@sig21.net \
    --cc=jszhang@marvell.com \
    --cc=khilman@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mika.westerberg@linux.intel.com \
    --cc=sumit.semwal@linaro.org \
    --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