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

[Patrick DELAUNAY <patrick.delaunay@foss.st.com>]

Hi,

On 11/12/26 11:06, Sudeep Holla wrote:
> 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.
>

Ok, it is clear

Thanks for the review

you can drop this patch and I will propose an other patch in PCSI CPU idle

driver to no more request "PC mode" with psci_set_osi_mode()

when use_osi= false


Patrick