Re: [RFC PATCH 2/9] 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] Uninitialized State Overwrite in QoS registers on first power-on
--

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

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

This commit introduces a per-domain settle delay for the RK3576 NPU domains.
The delay is applied between releasing the idle request and writing to the
QoS registers to prevent an SError while the NoC is settling.

> diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
> --- 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;
>  
> +		if (pd->info->delay_us)
> +			udelay(pd->info->delay_us);
> +
>  		rockchip_pmu_restore_qos(pd);

[Severity: High]
This is a pre-existing issue, but does this unconditional restore overwrite
QoS registers with uninitialized state on the very first power-on?

If a power domain is initially OFF at boot, qos_save_regs remains
zero-initialized by devm_kcalloc(). On the first power-on request,
rockchip_pd_power() unconditionally calls rockchip_pmu_restore_qos(),
which writes these zeros directly to the hardware:

drivers/pmdomain/rockchip/pm-domains.c:rockchip_pmu_restore_qos() {
    ...
    regmap_write(pd->qos_regmap[i], QOS_PRIORITY, pd->qos_save_regs[0][i]);
    ...
}

If the hardware reset defaults for QoS bandwidth or priority are non-zero,
writing zeros could throttle or block interconnect traffic, potentially causing
the associated IP block to hang when accessing memory.

Is there a mechanism to ensure the QoS state is actually populated before it
is restored, or should the driver avoid restoring QoS state if a prior state
was never saved?

>  	}
>

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