Re: [PATCH v2 2/4] pmdomain: mediatek: Restore a register after powering a domain on

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] pmdomain: mediatek: Incomplete error path in scpsys_power_on() fails to unwind hardware state
--

commit f298ac6e69f935973c035a0a1e72ad788976e0f8
Author: Michał Kopeć <michal@nozomi.space>

pmdomain: mediatek: Restore a register after powering a domain on

Some blocks lose a setting when their power domain is turned off, and it
has to be restored every time the domain is powered on again, before
the devices in the domain use it.

> diff --git a/drivers/pmdomain/mediatek/mtk-pm-domains.c b/drivers/pmdomain/mediatek/mtk-pm-domains.c
> index f0fa79e01f9c3..0585a663f48a6 100644
> --- a/drivers/pmdomain/mediatek/mtk-pm-domains.c
> +++ b/drivers/pmdomain/mediatek/mtk-pm-domains.c
> @@ -755,6 +757,13 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
>  	if (ret < 0)
>  		goto err_disable_sram;
>  
> +	if (pd->init_regmap) {
> +		ret = regmap_write(pd->init_regmap, pd->data->reg_init->ofs,
> +				   pd->data->reg_init->val);
> +		if (ret)
> +			goto err_enable_bus_protect;

[Severity: Medium]
If regmap_write fails here, the error path jumps to err_enable_bus_protect,
which cascades down to err_pwr_ack:

err_enable_bus_protect:
	scpsys_bus_protect_enable(pd, 0);
err_disable_sram:
...
err_pwr_ack:
	clk_bulk_disable_unprepare(pd->num_clks, pd->clks);
err_reg:
	scpsys_regulator_disable(pd->supply);
	return ret;

However, err_pwr_ack does not reverse several earlier steps from
scpsys_power_on(). It fails to call scpsys_bus_protect_enable(pd,
BUS_PROT_IGNORE_SUBCLK), does not re-enable ext_buck_iso, and does not invoke
scpsys_ctl_pwrseq_off() (or the other pwrseq variants).

Does this leave the power domain logically powered off in genpd, but physically
powered on and partially unprotected, risking system instability on subsequent
power cycles?

> +	}
> +
>  	if (MTK_SCPD_CAPS(pd, MTK_SCPD_STRICT_BUS_PROTECTION)) {
>  		ret = clk_bulk_prepare_enable(pd->num_subsys_clks,
>  					      pd->subsys_clks);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261011184755.2058017-1-michal@nozomi.space?part=2