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

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

[Jon Hunter]

Hi all,

On 31/12/2025 16:21, Manivannan Sadhasivam wrote:
> From: Konrad Dybcio <konradybcio@kernel.org>
> 
> 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@linaro.org>
> Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
> [mani: reworded the description to be more elaborative]
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
> 
> 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@oss.qualcomm.com/
> 
> 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].

So for Tegra ...

Tested-by: Jon Hunter <jonathanh@nvidia.com>
Acked-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

[0] 
https://lore.kernel.org/lkml/kkly3z4durpagtenadvmzdpojlctachgfgi2fdapt6zthdl2gx@n2qhmlud2zb7/

-- 
nvpublic

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

[Jon Hunter]

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@kernel.org>
>>
>> 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@linaro.org>
>> Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
>> [mani: reworded the description to be more elaborative]
>> Signed-off-by: Manivannan Sadhasivam 
>> <manivannan.sadhasivam@oss.qualcomm.com>
>> ---
>>
>> 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@oss.qualcomm.com/
>>
>> 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.

Thanks
Jon

-- 
nvpublic

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@kernel.org>
> > > 
> > > 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@linaro.org>
> > > Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
> > > [mani: reworded the description to be more elaborative]
> > > Signed-off-by: Manivannan Sadhasivam
> > > <manivannan.sadhasivam@oss.qualcomm.com>
> > > ---
> > > 
> > > 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@oss.qualcomm.com/
> > > 
> > > 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

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

[Jon Hunter]

Hi Lorenzo,

On 24/03/2026 08:45, Lorenzo Pieralisi wrote:

...

>>> 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.

No worries. Do you plan to pick this up?

Thanks
Jon

-- 
nvpublic

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

[Jon Hunter]

Hi Mani,

On 06/04/2026 15:29, Manivannan Sadhasivam wrote:

...

> Sure. It makes logical sense to relate this API behavior with the state of CX.
> I'll send v2 with the updated commit message.

Did you ever send a V2? I am not sure if I missed it and so wanted to check.

Thanks
Jon

-- 
nvpublic

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

[Manivannan Sadhasivam]

On Tue, May 05, 2026 at 02:47:08PM +0100, Jon Hunter wrote:
> Hi Mani,
> 
> On 06/04/2026 15:29, Manivannan Sadhasivam wrote:
> 
> ...
> 
> > Sure. It makes logical sense to relate this API behavior with the state of CX.
> > I'll send v2 with the updated commit message.
> 
> Did you ever send a V2? I am not sure if I missed it and so wanted to check.
> 

No, I didn't. I had some offline discussions with Bjorn and concluded that it
might not be appropriate to set pm_set_resume/suspend_via_firmware() for all
firmware flavors that our Qcom SoCs support.

Then I found out a limitation with our Root Complex IPs in exiting L1ss if the
platform is suspended. So sent out a series to fix that and that allowed me to
get past the NVMe issue coincidentally.

But since this patch applies to other ARM64 SoCs, it logically makes sense to
apply it IMO. So, I'll leave it to PSCI maintainers to decide.

- Mani

[1] https://lore.kernel.org/linux-pci/20260414-l1ss-fix-v1-0-adbb4555b5ab@oss.qualcomm.com

-- 
மணிவண்ணன் சதாசிவம்