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 <[email protected]> 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/[email protected]?part=2
