Re: [PATCH v2] mmc: sdhci-pci-gli: fix LPM negotiation so x86/S0ix SoCs can suspend

[Stanisław Kardach]

On Wed, Aug 30, 2023 at 4:27 AM Ben Chuang <benchuanggli@xxxxxxxxx> wrote:
>
> Hi,
> On Wed, Aug 30, 2023 at 12:35 AM Sven van Ashbrook <svenva@xxxxxxxxxxxx> wrote:
> >
> > + Rafael for advice on runtime_pm corner cases.
> >
> > On Mon, Aug 28, 2023 at 10:48 PM Ben Chuang <benchuanggli@xxxxxxxxx> wrote:
> > >
> > >
> > > My concern is that when runtime_pm is false, gl9763e is disabled LPM
> > > negotiation, gl9763e can't enter L1.x and s0ix may fail.
> > > It seems that runtime_pm will always exist and that's ok.
> > >
> >
> > Thank you. I believe we can address your concern.
> >
> > - XXX_suspend/XXX_resume (i.e. classic suspend/resume) depends on
> >   CONFIG_PM_SLEEP. This always selects CONFIG_PM. This always includes
> >   the runtime_pm framework. So, if XXX_suspend/XXX_resume gets called,
> >   the runtime_pm framework is always present, but may not be actively
> >   managing the device.
> This is ok.
>
> >
> > - "when runtime_pm is false" AFAIK the only way to disable runtime_pm
> >   when CONFIG_PM is set, is to write "on" to /sys/devices/.../power/control.
> >   See https://www.kernel.org/doc/Documentation/ABI/testing/sysfs-devices-power
> >   In that case, the runtime_pm framework will activate the device, calling
> >   XXX_runtime_resume() if necessary. Are there other ways of disabling it?
> >
> > - if /sys/devices/.../power/control is "on", then:
> >   gl9763e_runtime_resume() always called -> LPM always disabled
> >   gl9763e_suspend() -> LPM enabled -> gl9763e_resume() -> LPM disabled
> >   In between "classic" XXX_suspend and XXX_resume, LPM will be enabled,
> >   so the device can enter L1.x and S0ix.
> In this cas, after gl9763e_resume(), it is LPM disabled.
> Is there no chance for gl9763e to enter L1.x again when the system is idle?
With runtime PM disabled via sysfs, the short answer is not since
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f9e5b33934cec24b8c024add5c5d65d2f93ade05.

The longer answer is:
1. System boots up with LPM flags in PCI config space in default value
(might be LPM enabled).
2.1. If runtime PM is disabled before first runtime suspend -
registers will be left in their default state.
2.2. If runtime PM is disabled after first runtime suspend, the device
will be woken up and the gl9763e runtime resume callback will disable
LPM.
>
> >
> > And the LPM negotiation flags look correct.
> > Does that address your concerns?
>
> Best regards,
> Ben Chuang

--
Best Regards,
Stanisław Kardach