Re: [PATCH v3 3/5] pmdomain: mediatek: Respect PD relationships during error cleanup

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unbalanced clock and regulator disables during error cleanup for domains with MTK_SCPD_KEEP_DEFAULT_OFF.
--

--- Patch [3]: [PATCH v3 3/5] pmdomain: mediatek: Respect PD relationships during error cleanup ---
commit 197bd2fa3c14c60d9b5f2423843880328e47fbc9
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Subject: pmdomain: mediatek: Respect PD relationships during error cleanup

In case any probe error occurs (usually, a probe deferral) the
power domains shall be cleaned up while respecting their child
to parent relationship, or the system may freeze.

> diff --git a/drivers/pmdomain/mediatek/mtk-pm-domains.c b/drivers/pmdomain/mediatek/mtk-pm-domains.c
> index f0a6339affd75..8d49d9ee3cd29 100644
> --- a/drivers/pmdomain/mediatek/mtk-pm-domains.c
> +++ b/drivers/pmdomain/mediatek/mtk-pm-domains.c

[ ... ]

> @@ -1017,14 +1020,16 @@ static void scpsys_remove_one_domain(struct scpsys_domain *pd)
>  	clk_bulk_put(pd->num_subsys_clks, pd->subsys_clks);
>  }
>  
> -static void scpsys_domain_cleanup(struct scpsys *scpsys)
> +static void scpsys_domain_cleanup(struct scpsys *scpsys, u8 *domains_idx, u8 num_probed)
>  {
>  	struct generic_pm_domain *genpd;
>  	struct scpsys_domain *pd;
>  	int i;
>  
> -	for (i = scpsys->pd_data.num_domains - 1; i >= 0; i--) {
> -		genpd = scpsys->pd_data.domains[i];
> +	for (i = num_probed - 1; i >= 0; i--) {
> +		u8 pd_idx = domains_idx[i];
> +
> +		genpd = scpsys->pd_data.domains[pd_idx];
>  		if (genpd) {
>  			pd = to_scpsys_domain(genpd);
>  			scpsys_remove_one_domain(pd);

[Severity: High]
This is a pre-existing issue, but does the error cleanup path cause unbalanced
clock and regulator disables for default-off domains?

In scpsys_add_one_domain(), if a domain is configured with
MTK_SCPD_KEEP_DEFAULT_OFF, it intentionally skips turning on the domain,
meaning clk_bulk_prepare_enable() and scpsys_regulator_enable() are never
called by the driver during initialization.

However, if a probe deferral occurs later in scpsys_probe(), this cleanup path
calls scpsys_remove_one_domain(). If the domain was left ON by the bootloader,
scpsys_remove_one_domain() unconditionally invokes the internal power-off
functions:

drivers/pmdomain/mediatek/mtk-pm-domains.c:scpsys_remove_one_domain() {
    ...
    if (scpsys->soc_data->type == SCPSYS_MTCMOS_TYPE_HW_VOTER) {
        if (scpsys_hwv_domain_is_enable_done(pd))
            scpsys_hwv_power_off_internal(pd);
    } else {
        if (scpsys_domain_is_on(pd))
            scpsys_power_off_internal(pd);
    }
    ...
}

Since the driver never successfully enabled these resources, will these
unconditional power off calls decrement their usage counts below zero,
potentially leading to kernel warnings or system crashes from unintentionally
disabling shared clocks or regulators?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709084635.24912-1-angelogioacchino.delregno@collabora.com?part=3