Re: [PATCH 2/3] pmdomain: mediatek: Add support for secure modem power domain control

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Secure modem power domain is powered off prematurely in the teardown sequence, violating hardware requirements and potentially causing bus timeouts or lockups (SError).
--

--- Patch [2]: [PATCH 2/3] pmdomain: mediatek: Add support for secure modem power domain control ---
commit 6dae8fe193509453ecfd352eacac6f51667e9e72
Author: Nikolai Burov <nikolai.burov@jolla.com>

pmdomain: mediatek: Add support for secure modem power domain control

This commit adds a flag to enable a secure monitor call (SMC) to turn
the modem power domain on or off on recent MediaTek SoCs. The sequence is
needed because firmware prevents direct modification of the power domain
control register.

> diff --git a/drivers/pmdomain/mediatek/mtk-pm-domains.c b/drivers/pmdomain/mediatek/mtk-pm-domains.c
> --- a/drivers/pmdomain/mediatek/mtk-pm-domains.c
> +++ b/drivers/pmdomain/mediatek/mtk-pm-domains.c

[ ... ]

> @@ -709,6 +745,12 @@ static int scpsys_power_off(struct generic_pm_domain *genpd)
>  	if (ret < 0)
>  		return ret;
>  
> +	if (MTK_SCPD_CAPS(pd, MTK_SCPD_MODEM_SECURE)) {
> +		ret = scpsys_modem_sec_power_on(false);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	if (pd->data->ext_buck_iso_offs && MTK_SCPD_CAPS(pd, MTK_SCPD_EXT_BUCK_ISO))
>  		regmap_set_bits(scpsys->base, pd->data->ext_buck_iso_offs,
>  				pd->data->ext_buck_iso_mask);

[Severity: High]
Does calling scpsys_modem_sec_power_on(false) here power off the domain
prematurely?

The secure modem domain is being shut down before isolating the bus and
disabling the subsystem clocks. Could this cause AXI bus lockups (SError)
and crash the system?

[ ... ]

> @@ -719,10 +761,12 @@ static int scpsys_power_off(struct generic_pm_domain *genpd)
>  	if (ret < 0)
>  		return ret;
>  
> -	if (MTK_SCPD_CAPS(pd, MTK_SCPD_MODEM_PWRSEQ))
> -		scpsys_modem_pwrseq_off(pd);
> -	else
> -		scpsys_ctl_pwrseq_off(pd);
> +	if (!MTK_SCPD_CAPS(pd, MTK_SCPD_MODEM_SECURE)) {
> +		if (MTK_SCPD_CAPS(pd, MTK_SCPD_MODEM_PWRSEQ))
> +			scpsys_modem_pwrseq_off(pd);
> +		else
> +			scpsys_ctl_pwrseq_off(pd);
> +	}
>  
>  	/* wait until PWR_ACK = 0 */

[Severity: High]
Should the scpsys_modem_sec_power_on(false) call be moved down here?

For non-secure domains, the actual power down happens here after the bus
protection and clock disable steps have executed.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714-mt6858-pmdomain-v1-0-4f09bbb822e0@jolla.com?part=2