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

[Jon Hunter <jonathanh@nvidia.com>]

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