Re: [PATCH 3/3] firmware/psci: Allow specifying an S2RAM state through CPU_SUSPEND

[Sudeep Holla <sudeep.holla@arm.com>]

On Wed, Nov 13, 2024 at 01:57:23PM +0100, Lorenzo Pieralisi wrote:
> On Mon, Oct 28, 2024 at 03:22:59PM +0100, Konrad Dybcio wrote:
> > From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> > 
> > Certain firmware implementations (such as the ones found on Qualcomm
> > SoCs between roughly 2015 and 2023) expose an S3-like S2RAM state
> > through the CPU_SUSPEND call.
> > 
> > This works exactly like SYSTEM_SUSPEND. The PSCI spec describes that
> > call as optional (and only introduced in PSCIv1.0), so not all
> > platforms expose it.
> > 
> > Marking a DT-described "domain-idle-state" as such isn't currently
> > well accounted for in the PSCI idle topology infrastructure: the
> > cpuidle and genpd framework are deeply intertwined, and trying to
> > separate them would cause more havoc than good.
> 
> I don't understand what you mean here please elaborate.
> 
> The part of the story I understand is that you have a system (well,
> firmware for an extended set of systems) that does not implement
> SYSTEM_SUSPEND but can reach a S2R like system state through the
> CPU_SUSPEND call. Firmware works in OS-initiated mode, idle-states
> should allow you to define idle states that allow the system to
> enter the S2R state through CPUidle.
> 
> Please explain to me what's missing.
> 
> > Instead, allow the specifying of a single CPU_SUSPEND sleep param
> > under the /psci node that shall be treated exactly like SYSTEM_SUSPEND
> > from Linux's POV. As a bonus, this way we also don't have to fight
> > with the genpd idle governor to avoid taking the S3-like state into
> > consideration.
> 
> That's not acceptable. I want to understand what's preventing this
> system to enter that state through suspend2idle and the mainline code.
> 
> > Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> > ---
> >  drivers/firmware/psci/psci.c | 36 +++++++++++++++++++++++++++++++-----
> >  1 file changed, 31 insertions(+), 5 deletions(-)
> 
> NACK
> 

+1, will wait for the response here before adding any more questions that
may lead to more confusion or discussion churn.

-- 
Regards,
Sudeep