On Thursday, June 22, 2017 01:49:33 PM Mika Westerberg wrote: > On Thu, Jun 22, 2017 at 01:31:51AM +0200, Rafael J. Wysocki wrote: > > On Wed, Jun 21, 2017 at 9:21 PM, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote: > > > 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. > > > > Which really is expected to happen, so direct_complete should only be > > used along with the ACPI PM domain in this case. > > > > Moreover, in the ACPI PM domain case acpi_subsys_prepare() is supposed > > to do the right thing without dw_i2c_plat_prepare() and the return > > value of the latter will be ignored anyway, so dw_i2c_plat_prepare() > > will only have effect without ACPI PM domain AFAICS. > > > > It looks like commit 8503ff166504 is entirely misguided. > > Indeed it is. At the time I suggested that change I did not really > understand how the direct complete is supposed to be used :-/ So can we go for a full revert, please, and then fix up things properly? Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html