Re: [RFC PATCH v2 2/8] pmdomain/rockchip: add optional per-domain power-on settle delay

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing state rollbacks in the error paths of rockchip_pd_power() can leave the hardware in an inconsistent or unusable state, leading to a kernel panic.
--

--- Patch [2]: [RFC PATCH v2 2/8] pmdomain/rockchip: add optional per-domain power-on settle delay ---
commit 5d8a3f8a8260643f6533a94cae0393fe9867b0cc
Author: Jiaxing Hu <gahing@gahingwoo.com>

pmdomain/rockchip: add optional per-domain power-on settle delay

This commit adds a per-domain settle delay for the RK3576 NPU domains.
It introduces a delay_us field to rockchip_domain_info and applies the
delay in rockchip_pd_power() before restoring QoS settings.

> diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
> index 490bbb1d1d8e8..1787f62ba3a81 100644
> --- a/drivers/pmdomain/rockchip/pm-domains.c
> +++ b/drivers/pmdomain/rockchip/pm-domains.c

[ ... ]

> @@ -667,6 +668,9 @@ static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
>  		if (ret < 0)
>  			goto out;

[Severity: High]
This is a pre-existing issue, but there appears to be a problem with the
error path in rockchip_pd_power() when a domain transition fails.

When the genpd framework attempts to power off a domain and
rockchip_pd_power() is called with power_on set to false, the driver
successfully idles the bus interconnect:

        if (!power_on) {
                rockchip_pmu_save_qos(pd);

                /* if powering down, idle request to NIU first */
                ret = rockchip_pmu_set_idle_request(pd, true);
                if (ret < 0)
                        goto out;
        }

If the subsequent call to rockchip_do_pmu_set_power_domain(pd, false) fails
due to a hardware timeout, the function jumps to the out label without
reverting the idle request.

The genpd framework seems to gracefully handle the error by keeping the
domain status as GENPD_STATE_ON. However, when a consumer device later
resumes and attempts to access its MMIO registers, the interconnect remains
physically idled.

Can this cause the bus to hang and raise an SError, leading to a kernel
panic? Would it be appropriate to revert the idle request in the error path
before returning?

>  
> +		if (pd->info->delay_us)
> +			udelay(pd->info->delay_us);
> +
>  		rockchip_pmu_restore_qos(pd);
>  	}
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260718031146.3368811-1-gahing@gahingwoo.com?part=2