[1/9] i2c: designware: Fix system suspend

[Wangtao (Kevin, Kirin)]

This patch can fix the issue we found on hikey960 that i2c doesn't skip system suspend when it is runtime suspended.

在 2017/6/22 3:21, Ulf Hansson 写道:
> 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 at vger.linux.org
> Signed-off-by: Ulf Hansson <ulf.hansson at 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)
>   };
>