mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] firmware: psci: handle unsupported SET_SUSPEND_MODE in PC mode
@ 2026-10-29 18:16 Patrick Delaunay
  2026-10-30  8:32 ` Sudeep Holla
  0 siblings, 1 reply; 6+ messages in thread
From: Patrick Delaunay @ 2026-10-29 18:16 UTC (permalink / raw)
  To: Mark Rutland, Lorenzo Pieralisi, Sudeep Holla, Ulf Hansson
  Cc: dario.binacchi, Linux STM32, Patrick Delaunay, linux-arm-kernel,
	linux-kernel

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.

Fixes: 49fdcd7577ef ("firmware: psci: Export functions to manage the OSI mode")
Signed-off-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
---
This patch allows to Start Linux kernel with
- device tree using PSCI power domains
- with TF-A BL31 compiled without PSCI OSI support (PSCI_OS_INIT_MODE=0)

For example, since commit
9c8d852dabbd ("arm64: dts: st: add power domain on stm32mp25")

we have errors on STM32MP25 boards with latest TF-A (v2.15.0):

[    0.041800] psci: [Firmware Bug]: failed to set PC mode: -1
[    0.041814] PM: power-domain-retention: unable to remove subdomain power-domain-cluster
[    0.042130] PM: genpd_remove: unable to remove power-domain-cluster
[    0.042313] PM: genpd_remove: unable to remove power-domain-retention
[    0.042455] CPUidle PSCI: failed to create CPU PM domains ret=-95
[    0.042465] psci-cpuidle-domain psci: probe with driver psci-cpuidle-domain failed with error -95

and the associated devices are deferred, for example:

[   10.974374] platform 40130000.i2c: deferred probe pending: platform: wait for supplier /psci/power-domain-cluster

 drivers/firmware/psci/psci.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
index e73bae6cb23a..610756e81363 100644
--- a/drivers/firmware/psci/psci.c
+++ b/drivers/firmware/psci/psci.c
@@ -167,6 +167,11 @@ int psci_set_osi_mode(bool enable)
 			PSCI_1_0_SUSPEND_MODE_PC;
 
 	err = invoke_psci_fn(PSCI_1_0_FN_SET_SUSPEND_MODE, suspend_mode, 0, 0);
+
+	/* when optional PSCI_SET_SUSPEND_MODE is not supported, assume PC mode */
+	if (err ==  PSCI_RET_NOT_SUPPORTED && !enable)
+		err = 0;
+
 	if (err < 0)
 		pr_info(FW_BUG "failed to set %s mode: %d\n",
 				enable ? "OSI" : "PC", err);
---
base-commit: c339f4010dea8b0d754f84ace70cd1b089878ef8
branch: master

-- 
2.43.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] firmware: psci: handle unsupported SET_SUSPEND_MODE in PC mode
  2026-10-29 18:16 [PATCH] firmware: psci: handle unsupported SET_SUSPEND_MODE in PC mode Patrick Delaunay
@ 2026-10-30  8:32 ` Sudeep Holla
  2026-10-30 10:04   ` Patrick DELAUNAY
  0 siblings, 1 reply; 6+ messages in thread
From: Sudeep Holla @ 2026-10-30  8:32 UTC (permalink / raw)
  To: Patrick Delaunay
  Cc: Mark Rutland, Lorenzo Pieralisi, Ulf Hansson, dario.binacchi,
	Linux STM32, linux-arm-kernel, linux-kernel

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.

-- 
Regards,
Sudeep

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] firmware: psci: handle unsupported SET_SUSPEND_MODE in PC mode
  2026-10-30  8:32 ` Sudeep Holla
@ 2026-10-30 10:04   ` Patrick DELAUNAY
  2026-11-12  9:40     ` [Linux-stm32] " Patrick DELAUNAY
  2026-11-12 10:06     ` Sudeep Holla
  0 siblings, 2 replies; 6+ messages in thread
From: Patrick DELAUNAY @ 2026-10-30 10:04 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: Mark Rutland, Lorenzo Pieralisi, Ulf Hansson, dario.binacchi,
	Linux STM32, linux-arm-kernel, linux-kernel

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.


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;


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)


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)


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;


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


Regards


Patrick



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Linux-stm32] [PATCH] firmware: psci: handle unsupported SET_SUSPEND_MODE in PC mode
  2026-10-30 10:04   ` Patrick DELAUNAY
@ 2026-11-12  9:40     ` Patrick DELAUNAY
  2026-11-12 10:13       ` Sudeep Holla
  2026-11-12 10:06     ` Sudeep Holla
  1 sibling, 1 reply; 6+ messages in thread
From: Patrick DELAUNAY @ 2026-11-12  9:40 UTC (permalink / raw)
  To: Sudeep Holla, Ulf Hansson
  Cc: Mark Rutland, Lorenzo Pieralisi, linux-kernel, dario.binacchi,
	Linux STM32, linux-arm-kernel

Hi,

On 10/30/26 11:04, 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.
>
>
> 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;
>
>
> 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)
>
>
> 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)
>
>
> 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;
>
>
> but I patch PSCI firmware driver because I think it is a more generic 
> place to handle optional PSCI function.
>
>
> Regards
>
>
> Patrick


To complete my previous answer,

In  last PSCI specification v1.3, we found this description:

"

5.21 PSCI_SET_SUSPEND_MODE

5.21.3 Caller responsibilities

The caller must be able to cope with the following error return codes:
• NOT_SUPPORTED, in which case the calling OS can assume the platform 
will work in platform-
coordinated mode. In this case, the PSCI_FEATURES feature flag 0 for 
CPU_SUSPEND is also 0.

"


My patch only implements this behavior:

the PSCI stack in Linux assumes PC mode when SET_SUSPEND_MODE is not 
supported.


Regards

Patrick


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] firmware: psci: handle unsupported SET_SUSPEND_MODE in PC mode
  2026-10-30 10:04   ` Patrick DELAUNAY
  2026-11-12  9:40     ` [Linux-stm32] " Patrick DELAUNAY
@ 2026-11-12 10:06     ` Sudeep Holla
  1 sibling, 0 replies; 6+ messages in thread
From: Sudeep Holla @ 2026-11-12 10:06 UTC (permalink / raw)
  To: Patrick DELAUNAY
  Cc: Mark Rutland, Lorenzo Pieralisi, Sudeep Holla, Ulf Hansson,
	dario.binacchi, Linux STM32, linux-arm-kernel, linux-kernel

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Linux-stm32] [PATCH] firmware: psci: handle unsupported SET_SUSPEND_MODE in PC mode
  2026-11-12  9:40     ` [Linux-stm32] " Patrick DELAUNAY
@ 2026-11-12 10:13       ` Sudeep Holla
  0 siblings, 0 replies; 6+ messages in thread
From: Sudeep Holla @ 2026-11-12 10:13 UTC (permalink / raw)
  To: Patrick DELAUNAY
  Cc: Ulf Hansson, Mark Rutland, Sudeep Holla, Lorenzo Pieralisi,
	linux-kernel, dario.binacchi, Linux STM32, linux-arm-kernel

On Thu, Nov 12, 2026 at 10:40:48AM +0100, Patrick DELAUNAY wrote:
> Hi,
> 
> To complete my previous answer,
> In  last PSCI specification v1.3, we found this description:
> "
> 5.21 PSCI_SET_SUSPEND_MODE
> 5.21.3 Caller responsibilities
> The caller must be able to cope with the following error return codes:
> • NOT_SUPPORTED, in which case the calling OS can assume the platform will
> work in platform-coordinated mode. In this case, the PSCI_FEATURES feature
> flag 0 for CPU_SUSPEND is also 0.
> "
> 

Yes, I can't disagree with that, but ...(see below)

> My patch only implements this behavior:
> the PSCI stack in Linux assumes PC mode when SET_SUSPEND_MODE is not
> supported.
> 

To reiterate my earlier point: if PSCI_FEATURES reports that a function is
unsupported, we shouldn't invoke that FID in the first place. Dealing with an
unexpected NOT_SUPPORTED return from an advertised FID is a separate case, and
we must keep that handling unconditional as it currently stands.

-- 
Regards,
Sudeep

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-11-12 10:13 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-29 18:16 [PATCH] firmware: psci: handle unsupported SET_SUSPEND_MODE in PC mode Patrick Delaunay
2026-10-30  8:32 ` Sudeep Holla
2026-10-30 10:04   ` Patrick DELAUNAY
2026-11-12  9:40     ` [Linux-stm32] " Patrick DELAUNAY
2026-11-12 10:13       ` Sudeep Holla
2026-11-12 10:06     ` Sudeep Holla

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®