On Tue, 2017-06-27 at 15:56 +0200, Hans de Goede wrote: > Hi, > > On 06/27/2017 03:31 PM, Andy Shevchenko wrote: > > On Mon, 2017-06-26 at 23:57 +0200, Hans de Goede wrote: > > > > I didn't get why the first patch exists if you effectively remove > > what > > it brought here. > > The first patch calls lpss_iosf_exit_d3_state() on activate, but > if you're right that the probe functions will cause a runtime_resume > when enabling runtime-pm then that may not be necessary. If that is > right it does make one wonder why we have the activate function at all > though? I guess to de-assert the reset ? But shouldn't that then not > be > all it does ? Sorry for misleading comment. ACPI (and so platform drivers) should take care of power (runtime PM) by themselves. So, I dunno how to make the code less uglifying except put pm_runtime_get_sync() on each ->probe() of LPSS drivers. Perhaps your patch does that in better way and we may get rid of DW DMA uglified piece of code. > > > > > > If lpss_iosf_enter_d3_state() hits the code path where it actually > > > puts the DMA controllers into D3, then, at least on Bay Trail, the > > > LPSS > > > PWM controller will stop working / gets stuck. After this > > > happening > > > the > > > PWM controller's control reg will always reads 0x00010000 and > > > writes > > > seem to be ignored. > > > > > > Note that the chances of this code-path actually being hit are > > > actually > > > very low. On Bay Trail devices with an AXP288 PMIC and on any > > > Cherry > > > Trail > > > device, the I2C controller connected to the PMIC has (runtime) > > > suspend > > > disabled, so the condition of all LPSS and SCC devices being in D3 > > > will > > > never happen. > > > > > > Even on Bay Trail devices with another PMIC testing has shown that > > > lpss_iosf_enter_d3_state() will only put the DMA controllers in D3 > > > during > > > early boot when not all devices have been initialized yet, which > > > is > > > enough > > > to get the PWM controller stuck, while not resulting in any > > > significant > > > power saving as this only happens during boot. > > > > > > So in practice lpss_iosf_enter_d3_state() is almost always a no-op > > > and > > > when it is not, it is causing problems. Therefor this commit > > > simply > > > removes it. > > > > Let's continue discuss in your letter regarding IOSF LPSS stuff. > > Ok. > > Regards, > > Hans -- Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> Intel Finland Oy -- 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
