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

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

[Manivannan Sadhasivam]

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)
-- 
2.48.1

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]

[+Arnd, arm-soc - to pick this up please]

On Wed, Dec 31, 2025 at 09:51:26PM +0530, 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(+)

Acked-by: Lorenzo Pieralisi <lpieralisi@kernel.org>

> 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)
> -- 
> 2.48.1
> 

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

[Bjorn Andersson]

On Wed, Dec 31, 2025 at 09:51:26PM +0530, 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.

What is the actual extent of "peripheral devices" in this definition?

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

Our typical interpretation of this state is that IP-blocks might be
non-functional during this time, but typically some state is retained.

Will the acceptance of this patch result in that we in the Qualcomm case
should start accepting/writing patches that implement save/restore of
state that is generally retained throughout the drivers - in the name of
"power might be removed"?

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

With the right interpretation of what this means for us, I think this
patch looks good - but as we've discussed, I'm a bit worried about how
to deal with the alternative interpretations.

Specifically, if we fully adopt "power might be removed", we should to
take actions which will prevent a typical Qualcomm system from waking up
from sleep again.

Regards,
Bjorn

> 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)
> -- 
> 2.48.1
> 

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

[Manivannan Sadhasivam]

On Mon, Mar 30, 2026 at 03:48:05PM -0500, Bjorn Andersson wrote:
> On Wed, Dec 31, 2025 at 09:51:26PM +0530, 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.
> 
> What is the actual extent of "peripheral devices" in this definition?
> 

All devices except the ones backing volatile memory (DDR).

> > 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.
> > 
> 
> Our typical interpretation of this state is that IP-blocks might be
> non-functional during this time, but typically some state is retained.
> 
> Will the acceptance of this patch result in that we in the Qualcomm case
> should start accepting/writing patches that implement save/restore of
> state that is generally retained throughout the drivers - in the name of
> "power might be removed"?
> 

From the PSCI spec perspective, the underlying implementation of the
SYSTEM_SUSPEND is implementation defined. So whether the vendor firmware retains
the state or drops it completely, is out of the scope for PSCI. That's why
*assuming* that the devices will loose power is the best possible approach.

For sure, assuming that the power will be lost always will result in some
overhead with drivers saving and restoring the context unnecessarily if the
power if retained. But I can't think of any other way to make the driver work
across all firmware implementations.

> > 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.
> > 
> 
> With the right interpretation of what this means for us, I think this
> patch looks good - but as we've discussed, I'm a bit worried about how
> to deal with the alternative interpretations.
> 

Just for the sake of clarity to others, 'alternative interpretations' is
referring to Qcom DeepSleep firmware implementation, which cuts power to almost
all components except DDR with no wakeup source other than PMIC.

> Specifically, if we fully adopt "power might be removed", we should to
> take actions which will prevent a typical Qualcomm system from waking up
> from sleep again.
> 

I think for wakeup, the driver should just save the device context and call
enable_irq_wake() if the user has configured the device as a wakeup source and
assume that the device will wakeup the system from suspend/sleep.

The underlying firmware should honor the wakeup and not cut the power to the
devices. But if it does, then wakeup will be broken anyway.

So from Qcom drivers perspective:

1. They should save and restore the context if those drivers are going to be
used in both PSCI SYSTEM_SUSPEND (power retained) and DeepSleep (power lost)
firmware implementations.

2. If the user has configured wakeup, they should enable wakeup using
enable_irq_wake() during suspend. On PSCI SYSTEM_SUSPEND implementations, wakeup
should just work, but on DeepSleep, it may not if the power is cut off
completely. But that's the limitation on those platforms and the OS policy
should ensure that wakeup is not configured for devices.
 
- Mani

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

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

[Maulik Shah (mkshah)]

On 3/31/2026 2:18 AM, Bjorn Andersson wrote:
> On Wed, Dec 31, 2025 at 09:51:26PM +0530, 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.
> 
> What is the actual extent of "peripheral devices" in this definition?
> 
>> 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.
>>
> 
> Our typical interpretation of this state is that IP-blocks might be
> non-functional during this time, but typically some state is retained.
> 
> Will the acceptance of this patch result in that we in the Qualcomm case
> should start accepting/writing patches that implement save/restore of
> state that is generally retained throughout the drivers - in the name of
> "power might be removed"?
> 
>> 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.
>>
> 
> With the right interpretation of what this means for us, I think this
> patch looks good - but as we've discussed, I'm a bit worried about how
> to deal with the alternative interpretations.
> 
> Specifically, if we fully adopt "power might be removed", we should to
> take actions which will prevent a typical Qualcomm system from waking up
> from sleep again.

The "power might be removed" not necessarily indicate that devices/ peripherals
should remove the wake up capability of interrupts in order to prevent waking up
from sleep.

The hibernation + suspend-to-ram is one such example, where "power might
be removed" but devices may still leave the wake up interrupts enabled.

Pasting from commit 62c552ccc3ed ("PM / Hibernate: Enable suspend to both for in-kernel hibernation.")

 |   It is often useful to suspend to memory after hibernation image has been
 |   written to disk. If the battery runs out or power is otherwise lost, the
 |   computer will resume from the hibernated image. If not, it will resume
 |   from memory

Which can be interpreted as wake up interrupt should be able to wake from suspend-to-ram state
"to the point until the power is cut off or battery runs out".

And in order to do so, wake up capable interrupts should be left enabled in HW,
for interrupt controllers like GIC (and PDC in Qualcomm SoC cases) to support wake up.

Adding Cc: linux-arm-msm for better visibility.

Thanks,
Maulik

> 
> Regards,
> Bjorn
> 
>> 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)
>> -- 
>> 2.48.1
>>
> 

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

[Bjorn Andersson]

On Tue, Mar 31, 2026 at 12:09:38PM +0530, Manivannan Sadhasivam wrote:
> On Mon, Mar 30, 2026 at 03:48:05PM -0500, Bjorn Andersson wrote:
> > On Wed, Dec 31, 2025 at 09:51:26PM +0530, 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.
> > 
> > What is the actual extent of "peripheral devices" in this definition?
> > 
> 
> All devices except the ones backing volatile memory (DDR).
> 

I do not agree that this should be the interpretation on a Qualcomm
platform.


We do have the corner case of DeepSleep, where this indeed is what will
happen (not sure if other resource votes in the system are ignored
though?). For all typical targets making the PSCI jump might result in
CX being turned off (unless something else votes for it...). 

State is still retained on a case-by-case basis and some parts of the
system are still functional when we enter such state - and that is the
system state we desire.

Making the interpretation that all SoC resources will be disabled will
result in a large amount of unnecessary save/restore work, in addition
to breaking many use cases.

I do however not think that such interpretation is necessary, the
pm_suspend_via_firmware() kernel-doc describes that the firmware might
perform actions to power down things, it isn't specific about the
extent, so I think that's fine - while equating this to DeepSleep (SoC
fully powered down) is too extreme.


What bothers me with this patch in itself is that the behavior in
relation to PCIe does match the description of pm_suspend_via_firmware()
- the firmware does things which causes PCIe to "break", but IMHO this
is merely because PCIe operates without voting for the resources that it
depends on. But you keep telling me that this can't be solved in the PCI
layer...

If we can agree that pm_suspend_via_firmware() relates to the state of
CX - and merely the implicit PCSI-owned vote thereof - then I think we
should merge this patch.


But regardless of this interpretation. If PCI/NVMe relies on the PSCI
implementation's implicit vote for CX and its absence breaks NVMe during
suspend, then we're faced with exactly the same problem if the user
chooses s2idle as their means of suspending the system.

I.e. on a Qualcomm platform, we should flag PM_SUSPEND_FLAG_FW_SUSPEND
in s2idle as well - because from PCI/NVMe's point of view, the relevant
resources will be gone in either configuration.

Regards,
Bjorn

> > > 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.
> > > 
> > 
> > Our typical interpretation of this state is that IP-blocks might be
> > non-functional during this time, but typically some state is retained.
> > 
> > Will the acceptance of this patch result in that we in the Qualcomm case
> > should start accepting/writing patches that implement save/restore of
> > state that is generally retained throughout the drivers - in the name of
> > "power might be removed"?
> > 
> 
> From the PSCI spec perspective, the underlying implementation of the
> SYSTEM_SUSPEND is implementation defined. So whether the vendor firmware retains
> the state or drops it completely, is out of the scope for PSCI. That's why
> *assuming* that the devices will loose power is the best possible approach.
> 
> For sure, assuming that the power will be lost always will result in some
> overhead with drivers saving and restoring the context unnecessarily if the
> power if retained. But I can't think of any other way to make the driver work
> across all firmware implementations.
> 
> > > 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.
> > > 
> > 
> > With the right interpretation of what this means for us, I think this
> > patch looks good - but as we've discussed, I'm a bit worried about how
> > to deal with the alternative interpretations.
> > 
> 
> Just for the sake of clarity to others, 'alternative interpretations' is
> referring to Qcom DeepSleep firmware implementation, which cuts power to almost
> all components except DDR with no wakeup source other than PMIC.
> 
> > Specifically, if we fully adopt "power might be removed", we should to
> > take actions which will prevent a typical Qualcomm system from waking up
> > from sleep again.
> > 
> 
> I think for wakeup, the driver should just save the device context and call
> enable_irq_wake() if the user has configured the device as a wakeup source and
> assume that the device will wakeup the system from suspend/sleep.
> 
> The underlying firmware should honor the wakeup and not cut the power to the
> devices. But if it does, then wakeup will be broken anyway.
> 
> So from Qcom drivers perspective:
> 
> 1. They should save and restore the context if those drivers are going to be
> used in both PSCI SYSTEM_SUSPEND (power retained) and DeepSleep (power lost)
> firmware implementations.
> 
> 2. If the user has configured wakeup, they should enable wakeup using
> enable_irq_wake() during suspend. On PSCI SYSTEM_SUSPEND implementations, wakeup
> should just work, but on DeepSleep, it may not if the power is cut off
> completely. But that's the limitation on those platforms and the OS policy
> should ensure that wakeup is not configured for devices.
>  
> - Mani
> 
> -- 
> மணிவண்ணன் சதாசிவம்

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

[Manivannan Sadhasivam]

On Mon, Apr 06, 2026 at 08:48:44AM -0500, Bjorn Andersson wrote:
> On Tue, Mar 31, 2026 at 12:09:38PM +0530, Manivannan Sadhasivam wrote:
> > On Mon, Mar 30, 2026 at 03:48:05PM -0500, Bjorn Andersson wrote:
> > > On Wed, Dec 31, 2025 at 09:51:26PM +0530, 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.
> > > 
> > > What is the actual extent of "peripheral devices" in this definition?
> > > 
> > 
> > All devices except the ones backing volatile memory (DDR).
> > 
> 
> I do not agree that this should be the interpretation on a Qualcomm
> platform.
> 
> 
> We do have the corner case of DeepSleep, where this indeed is what will
> happen (not sure if other resource votes in the system are ignored
> though?). For all typical targets making the PSCI jump might result in
> CX being turned off (unless something else votes for it...). 
> 
> State is still retained on a case-by-case basis and some parts of the
> system are still functional when we enter such state - and that is the
> system state we desire.
> 
> Making the interpretation that all SoC resources will be disabled will
> result in a large amount of unnecessary save/restore work, in addition
> to breaking many use cases.
> 

I think adding these kind of per-device power state might be cumbersome as it
may vary from SoC to SoC. But I do agree on the pain point of context
save/restore work which will happen majority of the times since DeepSleep
platforms are limited as of now.

> I do however not think that such interpretation is necessary, the
> pm_suspend_via_firmware() kernel-doc describes that the firmware might
> perform actions to power down things, it isn't specific about the
> extent, so I think that's fine - while equating this to DeepSleep (SoC
> fully powered down) is too extreme.
> 
> 
> What bothers me with this patch in itself is that the behavior in
> relation to PCIe does match the description of pm_suspend_via_firmware()
> - the firmware does things which causes PCIe to "break", but IMHO this
> is merely because PCIe operates without voting for the resources that it
> depends on. But you keep telling me that this can't be solved in the PCI
> layer...
> 

We do not want to vote for any resources during system suspend from the PCI
layer (host controller driver in specific), and that's the problem (only a few
platforms are exceptions).

Let's say if we have connected a NVMe to the PCIe bus. Since this is a passive
device i.e, not supporting wakeup, we can safely put the PCIe bus into low power
mode (turning off resources) to save power during system suspend (let's
ignore the NVMe driver behavior now). Now the suspend (s2ram) behavior will be:

CXPC - Controller context will be retained by switching the power domain to
always ON domain

DeepSleep - Controller context will be lost once the SoC enters CXPC, since CXPC
is the pre-requisite for entering DeepSleep

So only if the NVMe driver had prepared for the power loss, the driver will
resume successfully, if the platform used DeepSleep.

> If we can agree that pm_suspend_via_firmware() relates to the state of
> CX - and merely the implicit PCSI-owned vote thereof - then I think we
> should merge this patch.
> 

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

> 
> But regardless of this interpretation. If PCI/NVMe relies on the PSCI
> implementation's implicit vote for CX and its absence breaks NVMe during
> suspend, then we're faced with exactly the same problem if the user
> chooses s2idle as their means of suspending the system.
> 

From the PCIe controller driver, we are going to keep the votes in s2idle, but
just let go in s2ram.

> I.e. on a Qualcomm platform, we should flag PM_SUSPEND_FLAG_FW_SUSPEND
> in s2idle as well - because from PCI/NVMe's point of view, the relevant
> resources will be gone in either configuration.
> 

No they don't, as we do not plan to drop votes in s2idle. So
PM_SUSPEND_FLAG_FW_SUSPEND only applies to s2ram.

- Mani

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