On 8 September 2017 at 05:23, Wangtao (Kevin, Kirin) <kevin.wangtao@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx >> Signed-off-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx> >> --- >> 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) >> }; >> >
