| From a23318feeff662c8d25d21623daebdd2e55ec221 Mon Sep 17 00:00:00 2001 |
| From: Ulf Hansson <ulf.hansson@linaro.org> |
| Date: Wed, 9 Aug 2017 15:28:22 +0200 |
| Subject: i2c: designware: Fix system suspend |
| |
| From: Ulf Hansson <ulf.hansson@linaro.org> |
| |
| commit a23318feeff662c8d25d21623daebdd2e55ec221 upstream. |
| |
| 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() |
| callbacks. |
| |
| However, the PM core may unset the direct_complete flag for a parent |
| device, in case its child device are being system suspended before. In this |
| scenario, the PM core invokes the system sleep callbacks, no matter if the |
| device is runtime suspended or not. |
| |
| Particularly in cases of an existing i2c slave device, the above path is |
| triggered, which breaks the assumption that the i2c device is always |
| runtime resumed whenever the dw_i2c_plat_suspend() is being called. |
| |
| More precisely, dw_i2c_plat_suspend() calls clk_core_disable() and |
| clk_core_unprepare(), for an already disabled/unprepared clock, leading to |
| a splat in the log about clocks calls being wrongly balanced and breaking |
| system sleep. |
| |
| To still allow the direct_complete path in cases when it's possible, but |
| also to keep the fix simple, let's runtime resume the i2c device in the |
| ->suspend() callback, before continuing to put the device into low power |
| state. |
| |
| Note, in cases when the i2c device is attached to the ACPI PM domain, this |
| problem doesn't occur, because ACPI's ->suspend() callback, assigned to |
| acpi_subsys_suspend(), already calls pm_runtime_resume() for the device. |
| |
| It should also be noted that this change does not fix commit 8503ff166504 |
| ("i2c: designware: Avoid unnecessary resuming during system suspend"). |
| Because for the non-ACPI case, the system sleep support was already broken |
| prior that point. |
| |
| Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> |
| Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> |
| Tested-by: John Stultz <john.stultz@linaro.org> |
| Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com> |
| Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com> |
| Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com> |
| Signed-off-by: Wolfram Sang <wsa@the-dreams.de> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| drivers/i2c/busses/i2c-designware-platdrv.c | 14 ++++++++++++-- |
| 1 file changed, 12 insertions(+), 2 deletions(-) |
| |
| --- a/drivers/i2c/busses/i2c-designware-platdrv.c |
| +++ b/drivers/i2c/busses/i2c-designware-platdrv.c |
| @@ -294,7 +294,7 @@ static void dw_i2c_plat_complete(struct |
| #endif |
| |
| #ifdef CONFIG_PM |
| -static int dw_i2c_plat_suspend(struct device *dev) |
| +static int dw_i2c_plat_runtime_suspend(struct device *dev) |
| { |
| struct platform_device *pdev = to_platform_device(dev); |
| struct dw_i2c_dev *i_dev = platform_get_drvdata(pdev); |
| @@ -318,11 +318,21 @@ static int dw_i2c_plat_resume(struct dev |
| return 0; |
| } |
| |
| +#ifdef CONFIG_PM_SLEEP |
| +static int dw_i2c_plat_suspend(struct device *dev) |
| +{ |
| + pm_runtime_resume(dev); |
| + return dw_i2c_plat_runtime_suspend(dev); |
| +} |
| +#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_suspend, dw_i2c_plat_resume, NULL) |
| + SET_RUNTIME_PM_OPS(dw_i2c_plat_runtime_suspend, |
| + dw_i2c_plat_resume, |
| + NULL) |
| }; |
| |
| #define DW_I2C_DEV_PMOPS (&dw_i2c_dev_pm_ops) |
