Re: Potential issue with pci_prepare_to_sleep if there's no platform support for D3cold transition

[Heiner Kallweit <hkallweit1@gmail.com>]

On Wed, Apr 23, 2025 at 11:32 AM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> On Tue, Apr 22, 2025 at 1:43 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
> >
> > On Tue, Apr 22, 2025 at 8:28 AM Lukas Wunner <lukas@wunner.de> wrote:
> > >
> > > [cc += Rafael, linux-acpi]
> > >
> > > On Mon, Apr 21, 2025 at 10:05:59PM +0200, Heiner Kallweit wrote:
> > > > If there's no platform support for transition to D3cold, then
> > > > pci_set_power_state(dev, D3cold) still returns 0, even though
> > > > power state is transitioned to D3hot only. We called
> > > > pci_enable_wake(dev, D3cold, wakeup) before, therefore PME for
> > > > D3hot may not be enabled. Is this a bug?
> >
> > On platforms using ACPI, no it isn't.
> >
> > Internally, pci_enable_wake() evaluates _DSW and it doesn't
> > distinguish between D3hot and D3cold as per the spec.
> >
> > > > Background:
> > > > In __pci_set_power_state we have the following:
> > > >
> > > > error = pci_set_low_power_state(dev, PCI_D3hot, locked);
> > > > if (pci_platform_power_transition(dev, PCI_D3cold))
> > > >       return error;
> > > >
> > > > The acpi_pci_set_power_state() stub returns -ENODEV.
> > > > Therefore, if error=0,  __pci_set_power_state() will
> > > > return 0 if pci_platform_power_transition() fails.
> > >
> > > pci_prepare_to_sleep() calls pci_target_state() right at the top.
> > >
> > > If wakeup is supported and enabled, pci_target_state() is supposed
> > > to find the deepest power state supporting wakeup.  If D3cold doesn't
> > > support wakeup, D3hot or a shallower state is returned.
> > >
> > > Hence I don't quite understand how the scenario you're describing
> > > could occur in practice.  Are you seeing actual issues and have tracked
> > > them down to incorrect handling in pci_prepare_to_sleep()?
>
> On non-ACPI systems pci_target_state() just looks at the device
> capabilities. The described scenario can happen if device supports
> wake from D3cold, but platform has no means to switch a device to
> D3cold. I'm thinking of e.g. RTL8125 on ARM. Typically I don't expect
> an issue because boot-up default on these devices is to have PME wake
> from all states enabled. So it's more of a theoretical exercise at the
> moment.

I just see that pci_target_state() won't return D3cold for non-ACPI systems.
Therefore sorry for the noise.