[RFT,v3,0/3] i2c: designware: Runtime PM aware system sleep handling

On Monday, September 4, 2017 1:05:57 AM CEST Rafael J. Wysocki wrote:
> Hi,

An update related to the dependency on intel-lpss.

> The point here is to avoid runtime resuming i2c designware devices during
> system suspend in the driver's ->suspend callback in the case when the
> ACPI PM domain is not present.  That will allow us to deal with the
> ACPI PM domain case going forward, among other things.
> 
> The first patch cleans up the runtime PM handling in the i2c-designware-platdrv
> _probe() routine so as to make it enable runtime PM in all cases, but prevent
> the device from being runtime suspended via pm_runtime_forbid() if
> pm_disabled is set.

The second one pushed the intel-lpss system suspend/resume callbacks to the
late/early stages of suspend/resume, respectively, so that the i2c-designware-platdrv
ones can be pushed too.

> The second one get rids of some ugly code and makes the PM callbacks of the
> driver handle runtime-suspended devices during system suspend/resume.

This is the third one and it does less than it did before now.

> Please test if you can and let me know if anything breaks.

That still applies. :-)

Thanks,
Rafael

On Wed, Sep 06, 2017 at 01:41:37AM +0200, Rafael J. Wysocki wrote:
> On Monday, September 4, 2017 1:05:57 AM CEST Rafael J. Wysocki wrote:
> > Please test if you can and let me know if anything breaks.
> 
> That still applies. :-)

Done, works for me on Asus E200HA.

In case it matters, what I tested was v4.13 + sound/topic/dollar-cove-ti-4.13-v5
(PMIC etc) + sound/topic/soc-cx2072x-4.13 (sound codec driver)
+ "S0ix blocker debug patch v3" from https://bugzilla.kernel.org/show_bug.cgi?id=193891
+ your 3 patches.

I suspend + resume a few times, no problems.  (Of course S0ix
still doesn't work according to /sys/kernel/debug/pmc_atom/sleep_state.)

On Wednesday, September 6, 2017 11:16:46 AM CEST Johannes Stezenbach wrote:
> On Wed, Sep 06, 2017 at 01:41:37AM +0200, Rafael J. Wysocki wrote:
> > On Monday, September 4, 2017 1:05:57 AM CEST Rafael J. Wysocki wrote:
> > > Please test if you can and let me know if anything breaks.
> > 
> > That still applies. :-)
> 
> Done, works for me on Asus E200HA.

Thanks!

> In case it matters, what I tested was v4.13 + sound/topic/dollar-cove-ti-4.13-v5
> (PMIC etc) + sound/topic/soc-cx2072x-4.13 (sound codec driver)
> + "S0ix blocker debug patch v3" from https://bugzilla.kernel.org/show_bug.cgi?id=193891
> + your 3 patches.
> 
> I suspend + resume a few times, no problems.  (Of course S0ix
> still doesn't work according to /sys/kernel/debug/pmc_atom/sleep_state.)
> 

Well, we'll need to address the S0ix thing, but that will require some
investigation.

Can you please open a bug entry for that at bugzilla.kernel.org
(against Power Management -> Hibernation/Suspend) and CC it to me
so that it doesn't fall of the radar?

Thanks,
Rafael

On Wed, Sep 06, 2017 at 01:41:37AM +0200, Rafael J. Wysocki wrote:
> > Please test if you can and let me know if anything breaks.
> 
> That still applies. :-)

Tested on Dell XPS 9550 which is using intel-lpss and everything still
works fine :)

Tested-by: Mika Westerberg <mika.westerberg@linux.intel.com>

On Wed, Sep 6, 2017 at 1:06 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> On Wed, Sep 06, 2017 at 01:41:37AM +0200, Rafael J. Wysocki wrote:
>> > Please test if you can and let me know if anything breaks.
>>
>> That still applies. :-)
>
> Tested on Dell XPS 9550 which is using intel-lpss and everything still
> works fine :)
>
> Tested-by: Mika Westerberg <mika.westerberg@linux.intel.com>

OK, thanks!

On 09/06/2017 02:06 PM, Mika Westerberg wrote:
> On Wed, Sep 06, 2017 at 01:41:37AM +0200, Rafael J. Wysocki wrote:
>>> Please test if you can and let me know if anything breaks.
>>
>> That still applies. :-)
> 
> Tested on Dell XPS 9550 which is using intel-lpss and everything still
> works fine :)
> 
> Tested-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> 
Tested on platforms using ACPI LPSS PM domain and intel-lpss MFD driver.

Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>

On Wed, Sep 06, 2017 at 11:55:01AM +0200, Rafael J. Wysocki wrote:
> On Wednesday, September 6, 2017 11:16:46 AM CEST Johannes Stezenbach wrote:
> 
> > In case it matters, what I tested was v4.13 + sound/topic/dollar-cove-ti-4.13-v5
> > (PMIC etc) + sound/topic/soc-cx2072x-4.13 (sound codec driver)
> > + "S0ix blocker debug patch v3" from https://bugzilla.kernel.org/show_bug.cgi?id=193891
> > + your 3 patches.
> > 
> > I suspend + resume a few times, no problems.  (Of course S0ix
> > still doesn't work according to /sys/kernel/debug/pmc_atom/sleep_state.)
> > 
> 
> Well, we'll need to address the S0ix thing, but that will require some
> investigation.
> 
> Can you please open a bug entry for that at bugzilla.kernel.org
> (against Power Management -> Hibernation/Suspend) and CC it to me
> so that it doesn't fall of the radar?

I'll do that tomorrow, TIA for your interest in the issue.

Today I spent several hours to bisect the S0ix "regression"
on Asus E200HA, I put regression in quotes because S0ix
only ever worked with gross hacks like poking registers
using busybox devmem.  Anyway, the gross hack worked
on v4.12 and stopped working in v4.13-rc and the
culprit is d31fd43c0f9a4 "clk: x86: Do not gate clocks
enabled by the firmware" (I'll report this properly in a seperate mail).

But I guess that's not the part you're interested in,
the bug entry should cover interaction of dw i2c, acpi pm,
acpi-lpss and ACPI OpRegion, right?


Thanks,
Johannes

On Wednesday, September 6, 2017 9:59:16 PM CEST Johannes Stezenbach wrote:
> On Wed, Sep 06, 2017 at 11:55:01AM +0200, Rafael J. Wysocki wrote:
> > On Wednesday, September 6, 2017 11:16:46 AM CEST Johannes Stezenbach wrote:
> > 
> > > In case it matters, what I tested was v4.13 + sound/topic/dollar-cove-ti-4.13-v5
> > > (PMIC etc) + sound/topic/soc-cx2072x-4.13 (sound codec driver)
> > > + "S0ix blocker debug patch v3" from https://bugzilla.kernel.org/show_bug.cgi?id=193891
> > > + your 3 patches.
> > > 
> > > I suspend + resume a few times, no problems.  (Of course S0ix
> > > still doesn't work according to /sys/kernel/debug/pmc_atom/sleep_state.)
> > > 
> > 
> > Well, we'll need to address the S0ix thing, but that will require some
> > investigation.
> > 
> > Can you please open a bug entry for that at bugzilla.kernel.org
> > (against Power Management -> Hibernation/Suspend) and CC it to me
> > so that it doesn't fall of the radar?
> 
> I'll do that tomorrow, TIA for your interest in the issue.
> 
> Today I spent several hours to bisect the S0ix "regression"
> on Asus E200HA, I put regression in quotes because S0ix
> only ever worked with gross hacks like poking registers
> using busybox devmem.  Anyway, the gross hack worked
> on v4.12 and stopped working in v4.13-rc and the
> culprit is d31fd43c0f9a4 "clk: x86: Do not gate clocks
> enabled by the firmware" (I'll report this properly in a seperate mail).

Thanks!

> But I guess that's not the part you're interested in,
> the bug entry should cover interaction of dw i2c, acpi pm,
> acpi-lpss and ACPI OpRegion, right?

Right.  Whatever information you have already and is likely to be useful in
diagnosing this.

Thanks,
Rafael

On Wed, Sep 06, 2017 at 11:55:01AM +0200, Rafael J. Wysocki wrote:
> Well, we'll need to address the S0ix thing, but that will require some
> investigation.
> 
> Can you please open a bug entry for that at bugzilla.kernel.org
> (against Power Management -> Hibernation/Suspend) and CC it to me
> so that it doesn't fall of the radar?

The Cc of this mail is rather long, just in case someone
is interested to know I filed bug #196861 "S0ix enablement on Intel Atom"
https://bugzilla.kernel.org/show_bug.cgi?id=196861