Re: [PATCH] firmware: psci: Set pm_set_resume/suspend_via_firmware() for SYSTEM_SUSPEND

[Lorenzo Pieralisi]

On Mon, Mar 23, 2026 at 10:10:00AM +0000, Jon Hunter wrote:
> Hi Mark, Lorenzo,
> 
> On 12/03/2026 19:00, Jon Hunter wrote:
> > Hi all,
> > 
> > On 31/12/2025 16:21, Manivannan Sadhasivam wrote:
> > > From: Konrad Dybcio <konradybcio@xxxxxxxxxx>
> > > 
> > > PSCI specification defines the SYSTEM_SUSPEND feature which enables the
> > > firmware to implement the suspend to RAM (S2RAM) functionality by
> > > transitioning the system to a deeper low power state. When the system
> > > enters such state, the power to the peripheral devices might be
> > > removed. So
> > > the respective device drivers must prepare for the possible removal
> > > of the
> > > power by performing actions such as shutting down or resetting the device
> > > in their system suspend callbacks.
> > > 
> > > The Linux PM framework allows the platform drivers to convey this info to
> > > device drivers by calling the pm_set_suspend_via_firmware() and
> > > pm_set_resume_via_firmware() APIs.
> > > 
> > > Hence, if the PSCI firmware supports SYSTEM_SUSPEND feature, call
> > > the above
> > > mentioned APIs in the psci_system_suspend_begin() and
> > > psci_system_suspend_enter() callbacks.
> > > 
> > > Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxx>
> > > Reviewed-by: Sudeep Holla <sudeep.holla@xxxxxxx>
> > > [mani: reworded the description to be more elaborative]
> > > Signed-off-by: Manivannan Sadhasivam
> > > <manivannan.sadhasivam@xxxxxxxxxxxxxxxx>
> > > ---
> > > 
> > > This patch was part of an old series that didn't make it to mainline
> > > due to
> > > objections in the binding and exposing CPU_SUSPEND as S2RAM patches:
> > > https://lore.kernel.org/all/20241028-topic-cpu_suspend_s2ram-
> > > v1-0-9fdd9a04b75c@xxxxxxxxxxxxxxxx/
> > > 
> > > But this patch on its own is useful for platforms implementing the S2RAM
> > > feature in PSCI firmware. So I picked it up, tested on Qcom X1E T14s and
> > > resending it.
> > > 
> > >   drivers/firmware/psci/psci.c | 10 ++++++++++
> > >   1 file changed, 10 insertions(+)
> > > 
> > > diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
> > > index 38ca190d4a22..e73bae6cb23a 100644
> > > --- a/drivers/firmware/psci/psci.c
> > > +++ b/drivers/firmware/psci/psci.c
> > > @@ -539,12 +539,22 @@ static int psci_system_suspend(unsigned long
> > > unused)
> > >   static int psci_system_suspend_enter(suspend_state_t state)
> > >   {
> > > +    pm_set_resume_via_firmware();
> > > +
> > >       return cpu_suspend(0, psci_system_suspend);
> > >   }
> > > +static int psci_system_suspend_begin(suspend_state_t state)
> > > +{
> > > +    pm_set_suspend_via_firmware();
> > > +
> > > +    return 0;
> > > +}
> > > +
> > >   static const struct platform_suspend_ops psci_suspend_ops = {
> > >       .valid          = suspend_valid_only_mem,
> > >       .enter          = psci_system_suspend_enter,
> > > +    .begin          = psci_system_suspend_begin,
> > >   };
> > >   static void __init psci_init_system_reset2(void)
> > 
> > 
> > I wanted to ask what the status of this patch is?
> > 
> > It turns out that since commit f3ac2ff14834 ("PCI/ASPM: Enable all
> > ClockPM and ASPM states for devicetree platforms"), this fix is also
> > need for Tegra platforms that have NVMe devices to ensure that they are
> > suspended as needed. There is some more background in this thread [0].
> 
> 
> Any feedback on this? I am not sure if this patch is purposely being
> ignored, but if so, I would like to understand why.

It fell through the cracks, apologies.

Lorenzo