Re: [PATCH] firmware: psci: handle unsupported SET_SUSPEND_MODE in PC mode

[Sudeep Holla <sudeep.holla@kernel.org>]

On Fri, Oct 30, 2026 at 11:04:51AM +0100, Patrick DELAUNAY wrote:
> Hi,
> 
> On 10/30/26 09:32, Sudeep Holla wrote:
> > On Thu, Oct 29, 2026 at 07:16:25PM +0100, Patrick Delaunay wrote:
> > > The PSCI specification makes PSCI_SET_SUSPEND_MODE optional. If it
> > > returns NOT_SUPPORTED when Linux requests platform-coordinated (PC)
> > > mode, Linux can assume that PC mode is in effect. Treat this response
> > > as success so PSCI power-domain providers can initialize when firmware
> > > does not support OS-initiated (OSI) mode.
> > > 
> > > Continue to report an error when OSI mode is requested but
> > > SET_SUSPEND_MODE is not supported.
> > > 
> > So, your platform advertises the presence/support for OSI mode and yet
> > opted for not implementing SET_SUSPEND_MODE ? If so, it is indeed a
> > firmware bug as reported.
> > 
> 
> I think it is no realy a platform bug but a too strict management in Linux
> kernel, for PSCI power domain.
> The OSI mode is not yet supported by the STM32MP25 platform in Linux at
> upstream but I introduce PSCI domains in device tree (but without idle power
> domain) just to prepare this activation in next steps.
>

Fair enough, that shouldn't be a problem.

> As the PSCI support is not activated in Firmware (TF-A BL31), the firmware
> correctly expose its capacities (PSCI_OS_INIT_MODE =0) => PSCI feature
> indicate that OSI is not supported (FF_SUPPORTS_OS_INIT_MODE = 0) =>
> PSCI_SET_SUSPEND_MODE function is not supported (optional when OSI
> is not supported)
> 
> in Linux side, the power domain driver
> (drivers/cpuidle/cpuidle-psci-domain.c)
> 
> correctly check the support of OSI:
>     bool use_osi = psci_has_osi_support();
> 
> in STM32MP25 platform use_osi = false, so the driver request
> Plafrom Cooordinated mode (PC mode) as expected
> But here  the GENPD driver try to use optional function to set mode:
> 
>     /* let's try to enable OSI. */
>     ret = psci_set_osi_mode(use_osi);
>     if (ret)
>         goto remove_pd;
> 

IMO we should make this psci_set_osi_mode() call only when use_osi == true
above. If it is passing false, it is setting PC mode but as comments states
it may be assuming OSI mode and proceeding further which is completely wrong.
I would bail out of psci_cpuidle_domain_probe() early if use_osi is false.

> And the problem occurs here because the optional function is called but the
> normal use case is not handled (OSI mode not supported / PSCI_SET_SUSPEND_MODE
> not supported)
> 

Agreed, but I would check before calling here. The proposed fix hides
PSCI_RET_NOT_SUPPORTED error for PC-mode setting. So clear NACK for that.

> 
> With the proposed patch, the Linux kernel can have PSCI domain in device
> tree even when PSCI OSI support is deactivated in PSCI firmware (or not yet
> activated): the domain are always ON (GENPD_FLAG_ALWAYS_ON).
> 

Do you see any issue failing in psci_cpuidle_domain_probe() instead ?

> NB: I hesitate between the current patch and a patch in PSCI power domain
> driver:
> 
>     /* let's try to enable OSI. */
>     ret = psci_set_osi_mode(use_osi);
> + if (ret == -EOPNOTSUPP && !use_osi)
> +    ret = 0;
> 
>     if (ret)
>         goto remove_pd;
> 

See above, if the driver is assuming OSI mode is set after the call like the
comment states, it better check if use_osi = true.

> but I patch PSCI firmware driver because I think it is a more generic place
> to handle optional PSCI function.
> 

If we have to handle it in the psci, I would rather not make a call to the
firmware if PSCI_FEATURES(PSCI_1_0_FN_SET_SUSPEND_MODE) returns not supported.
That is the expectation for any ABI the firmware said it doesn't support.
Making unsupported ABI call to the firmware may result in unexpected behaviour
and must be avoided.

-- 
Regards,
Sudeep