Re: [PATCH 0/3] Allow specifying an S2RAM sleep on pre-SYSTEM_SUSPEND PSCI impls

Sudeep Holla
2024-12-20T14:36:07Z

On Fri, Dec 20, 2024 at 03:20:37PM +0100, Konrad Dybcio wrote:
>
> I would happen to think that, yes. Especially since the reference firmware
> implementation does *exactly this*:
>
> https://github.com/ARM-software/arm-trusted-firmware/blob/master/lib/psci/psci_main.c#L179-L221
>
> PSCI_SYSTEM_SUSPEND seems to be simply meant as a wrapper around a specific
> CPU_SUSPEND state (which may or may not be only callable from inside the
> firmware when SYSTEM_SUSPEND specifically is requested, for reasons),
> in a platform-agnostic way, so that the OS can enter suspend without
> providing that magic StateID on all supported platforms.

Exactly, that's how it can be OS and platform agnostic. Yet this platform
considered to optimise by not just providing it as a wrapper(if it was
that simple on your platform too) without running any tests and leaving
it to interested parties like you to mess around to get it working.
That practice needs to be fixed and this change won't help and once we
fix this, more such special treatment fixes are needed on newer platforms.
So lets stop and ensure things are fixed properly.

> But since it already requires more elbow grease on the peripheral IP side,
> I'm not really convinced it's that much useful.
>
> Plus, the optional bit of doing more work behind the scenes doesn't seem
> to be very wildly used across TF-A supported platforms.
>
> So please, stop making the argument that it's any different. The firmware
> I'm dealing with simply didn't expose the same thing twice, in perfect
> accordance with the spec.
>

So that it can continue to do so in the future ?
Thanks but no thanks. NACK with no arguments as requested.

--
Regards,
Sudeep