[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