Re: [PATCH 2/4] cpuidle: psci: Allow skipping the suspend mode switch

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] cpuidle: psci: skip-suspend-mode-switch ignored in psci_1_0_init()
--

commit ee7d5d7e060784375912b91f91b344ad1eecb0a8
Author: Alejandro Tafalla <atafalla@dnyon.com>

cpuidle: psci: Allow skipping the suspend mode switch

This commit adds support for the skip-suspend-mode-switch property in
psci_cpuidle_domain_probe() to skip calling psci_set_osi_mode() on
platforms whose firmware reports OSI mode support but rejects
PSCI_SET_SUSPEND_MODE calls.

> diff --git a/drivers/cpuidle/cpuidle-psci-domain.c b/drivers/cpuidle/cpuidle-psci-domain.c
> index 5722365b943ea..cc9f14769ba20 100644
> --- a/drivers/cpuidle/cpuidle-psci-domain.c
> +++ b/drivers/cpuidle/cpuidle-psci-domain.c
> @@ -159,13 +159,17 @@ static int psci_cpuidle_domain_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto remove_pd;
>  
> -	/* let's try to enable OSI. */
> -	ret = psci_set_osi_mode(use_osi);
> -	if (ret)
> -		goto remove_pd;
> +	if (of_property_read_bool(np, "skip-suspend-mode-switch")) {
> +		pr_info("Initialized CPU PM domain topology, keeping firmware-set mode\n");
> +	} else {
> +		ret = psci_set_osi_mode(use_osi);

[Severity: Medium]
Should psci_1_0_init() in drivers/firmware/psci/psci.c also check the
skip-suspend-mode-switch property?

During early PSCI initialization, psci_dt_init() -> psci_1_0_init() runs
on the same /psci device_node and unconditionally calls
psci_set_osi_mode(false) when psci_has_osi_support() is true:

drivers/firmware/psci/psci.c:psci_1_0_init() {
    ...
	if (psci_has_osi_support()) {
		pr_info("OSI mode supported.\n");

		/* Default to PC mode. */
		psci_set_osi_mode(false);
	}
    ...
}

Because that call is not gated on skip-suspend-mode-switch, the kernel
still issues PSCI_1_0_FN_SET_SUSPEND_MODE at boot and logs a FW_BUG
message in psci_set_osi_mode() when the firmware rejects the switch to PC
mode.

Furthermore, if a firmware with skip-suspend-mode-switch boots in OSI mode
and accepts the switch to PC mode in psci_1_0_init(), wouldn't
psci_cpuidle_domain_probe() then initialize the CPU PM domains with
use_osi = psci_has_osi_support() (true) and skip switching back to OSI
mode, leaving the kernel in OSI mode while the firmware was switched to PC
mode?

> +		if (ret)
> +			goto remove_pd;
> +
> +		pr_info("Initialized CPU PM domain topology using %s mode\n",
> +			use_osi ? "OSI" : "PC");
> +	}
>  
> -	pr_info("Initialized CPU PM domain topology using %s mode\n",
> -		use_osi ? "OSI" : "PC");
>  	return 0;
>  
>  remove_pd:

iala

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261017-psci-fix-v1-0-dedae27e4a31@gmail.com?part=2