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

[Ulf Hansson]

On 8 September 2017 at 05:23, Wangtao (Kevin, Kirin)
<kevin.wangtao at hisilicon.com> wrote:
>
> This patch can fix the issue we found on hikey960 that i2c doesn't skip
> system suspend when it is runtime suspended.

We decided to go with a slightly different version. You may try out
the below and see if that also fixes your problem.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a23318feeff662c8d25d21623daebdd2e55ec221

Kind regards
Uffe

>
>
> 在 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)
>>   };
>>
>