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

Wangtao (Kevin, Kirin) kevin.wangtao at hisilicon.com
Tue Sep 12 02:44:55 PDT 2017


OK, this patch can also fix the problem.
On 2017/9/8 16:29, Ulf Hansson wrote:
> 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)
>>>    };
>>>
>>
> 
> .
> 




More information about the linux-arm-kernel mailing list