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

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