[PATCH] ARM: rockchip: keep reset control around

[PATCH] ARM: rockchip: keep reset control around

[Heiko Stuebner]

Do not put the reset control, retain exclusive control over it.
After turning on a CPU, the corresponding reset line must stay
deasserted.

This also avoids calling reset_control_put() before workqueues
are operational.

Fixes: 78ebbff6d1a0 ("reset: handle removing supplier before consumers")
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Tested-by: Steven Price <steven.price@arm.com>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 arch/arm/mach-rockchip/platsmp.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-rockchip/platsmp.c b/arch/arm/mach-rockchip/platsmp.c
index f432d22bfed8..f659d894bfae 100644
--- a/arch/arm/mach-rockchip/platsmp.c
+++ b/arch/arm/mach-rockchip/platsmp.c
@@ -34,6 +34,7 @@ static int ncores;
 
 static struct regmap *pmu;
 static int has_pmu = true;
+static struct reset_control *cpu_rstc[4];
 
 static int pmu_power_domain_is_on(int pd)
 {
@@ -64,9 +65,11 @@ static struct reset_control *rockchip_get_core_reset(int cpu)
 static int pmu_set_power_domain(int pd, bool on)
 {
 	u32 val = (on) ? 0 : BIT(pd);
-	struct reset_control *rstc = rockchip_get_core_reset(pd);
+	struct reset_control *rstc;
 	int ret;
 
+	rstc = pd < ARRAY_SIZE(cpu_rstc) ? cpu_rstc[pd] : ERR_PTR(-EINVAL);
+
 	if (IS_ERR(rstc) && read_cpuid_part() != ARM_CPU_PART_CORTEX_A9) {
 		pr_err("%s: could not get reset control for core %d\n",
 		       __func__, pd);
@@ -100,11 +103,8 @@ static int pmu_set_power_domain(int pd, bool on)
 		}
 	}
 
-	if (!IS_ERR(rstc)) {
-		if (on)
-			reset_control_deassert(rstc);
-		reset_control_put(rstc);
-	}
+	if (!IS_ERR(rstc) && on)
+		reset_control_deassert(rstc);
 
 	return 0;
 }
@@ -312,6 +312,10 @@ static void __init rockchip_smp_prepare_cpus(unsigned int max_cpus)
 		ncores = ((l2ctlr >> 24) & 0x3) + 1;
 	}
 
+	/* Collect cpu core reset control for each core */
+	for (i = 0; i < ncores; i++)
+		cpu_rstc[i] = rockchip_get_core_reset(i);
+
 	/* Make sure that all cores except the first are really off */
 	for (i = 1; i < ncores; i++)
 		pmu_set_power_domain(0 + i, false);
-- 
2.47.3

Re: [PATCH] ARM: rockchip: keep reset control around

[Heiko Stuebner]

Am Donnerstag, 21. Mai 2026, 23:09:15 Mitteleuropäische Sommerzeit schrieb Heiko Stuebner:
> Do not put the reset control, retain exclusive control over it.
> After turning on a CPU, the corresponding reset line must stay
> deasserted.
> 
> This also avoids calling reset_control_put() before workqueues
> are operational.
> 
> Fixes: 78ebbff6d1a0 ("reset: handle removing supplier before consumers")
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> Tested-by: Steven Price <steven.price@arm.com>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>

Forgot to add Bartosz to the Cc list, sorry


Heiko

> ---
>  arch/arm/mach-rockchip/platsmp.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/mach-rockchip/platsmp.c b/arch/arm/mach-rockchip/platsmp.c
> index f432d22bfed8..f659d894bfae 100644
> --- a/arch/arm/mach-rockchip/platsmp.c
> +++ b/arch/arm/mach-rockchip/platsmp.c
> @@ -34,6 +34,7 @@ static int ncores;
>  
>  static struct regmap *pmu;
>  static int has_pmu = true;
> +static struct reset_control *cpu_rstc[4];
>  
>  static int pmu_power_domain_is_on(int pd)
>  {
> @@ -64,9 +65,11 @@ static struct reset_control *rockchip_get_core_reset(int cpu)
>  static int pmu_set_power_domain(int pd, bool on)
>  {
>  	u32 val = (on) ? 0 : BIT(pd);
> -	struct reset_control *rstc = rockchip_get_core_reset(pd);
> +	struct reset_control *rstc;
>  	int ret;
>  
> +	rstc = pd < ARRAY_SIZE(cpu_rstc) ? cpu_rstc[pd] : ERR_PTR(-EINVAL);
> +
>  	if (IS_ERR(rstc) && read_cpuid_part() != ARM_CPU_PART_CORTEX_A9) {
>  		pr_err("%s: could not get reset control for core %d\n",
>  		       __func__, pd);
> @@ -100,11 +103,8 @@ static int pmu_set_power_domain(int pd, bool on)
>  		}
>  	}
>  
> -	if (!IS_ERR(rstc)) {
> -		if (on)
> -			reset_control_deassert(rstc);
> -		reset_control_put(rstc);
> -	}
> +	if (!IS_ERR(rstc) && on)
> +		reset_control_deassert(rstc);
>  
>  	return 0;
>  }
> @@ -312,6 +312,10 @@ static void __init rockchip_smp_prepare_cpus(unsigned int max_cpus)
>  		ncores = ((l2ctlr >> 24) & 0x3) + 1;
>  	}
>  
> +	/* Collect cpu core reset control for each core */
> +	for (i = 0; i < ncores; i++)
> +		cpu_rstc[i] = rockchip_get_core_reset(i);
> +
>  	/* Make sure that all cores except the first are really off */
>  	for (i = 1; i < ncores; i++)
>  		pmu_set_power_domain(0 + i, false);
> 

Re: [PATCH] ARM: rockchip: keep reset control around

[Heiko Stuebner]

Am Donnerstag, 21. Mai 2026, 23:09:15 Mitteleuropäische Sommerzeit schrieb Heiko Stuebner:
> Do not put the reset control, retain exclusive control over it.
> After turning on a CPU, the corresponding reset line must stay
> deasserted.
> 
> This also avoids calling reset_control_put() before workqueues
> are operational.
> 
> Fixes: 78ebbff6d1a0 ("reset: handle removing supplier before consumers")
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> Tested-by: Steven Price <steven.price@arm.com>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>  arch/arm/mach-rockchip/platsmp.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/mach-rockchip/platsmp.c b/arch/arm/mach-rockchip/platsmp.c
> index f432d22bfed8..f659d894bfae 100644
> --- a/arch/arm/mach-rockchip/platsmp.c
> +++ b/arch/arm/mach-rockchip/platsmp.c
> @@ -34,6 +34,7 @@ static int ncores;
>  
>  static struct regmap *pmu;
>  static int has_pmu = true;
> +static struct reset_control *cpu_rstc[4];

After sleeping on that, this should be cpu_rstc[5];

Coretx-A9 SoCs need to enable the SCU power-domain which thankfully
sits at index 4 of the power-domain register.

So while we (already) expect no reset control for that, we need at least
make sure, it's not reading into undefined memory and thus need that
empty field in the array.


Heiko

>  
>  static int pmu_power_domain_is_on(int pd)
>  {
> @@ -64,9 +65,11 @@ static struct reset_control *rockchip_get_core_reset(int cpu)
>  static int pmu_set_power_domain(int pd, bool on)
>  {
>  	u32 val = (on) ? 0 : BIT(pd);
> -	struct reset_control *rstc = rockchip_get_core_reset(pd);
> +	struct reset_control *rstc;
>  	int ret;
>  
> +	rstc = pd < ARRAY_SIZE(cpu_rstc) ? cpu_rstc[pd] : ERR_PTR(-EINVAL);
> +
>  	if (IS_ERR(rstc) && read_cpuid_part() != ARM_CPU_PART_CORTEX_A9) {
>  		pr_err("%s: could not get reset control for core %d\n",
>  		       __func__, pd);
> @@ -100,11 +103,8 @@ static int pmu_set_power_domain(int pd, bool on)
>  		}
>  	}
>  
> -	if (!IS_ERR(rstc)) {
> -		if (on)
> -			reset_control_deassert(rstc);
> -		reset_control_put(rstc);
> -	}
> +	if (!IS_ERR(rstc) && on)
> +		reset_control_deassert(rstc);
>  
>  	return 0;
>  }
> @@ -312,6 +312,10 @@ static void __init rockchip_smp_prepare_cpus(unsigned int max_cpus)
>  		ncores = ((l2ctlr >> 24) & 0x3) + 1;
>  	}
>  
> +	/* Collect cpu core reset control for each core */
> +	for (i = 0; i < ncores; i++)
> +		cpu_rstc[i] = rockchip_get_core_reset(i);
> +
>  	/* Make sure that all cores except the first are really off */
>  	for (i = 1; i < ncores; i++)
>  		pmu_set_power_domain(0 + i, false);
> 

Re: [PATCH] ARM: rockchip: keep reset control around

[Philipp Zabel]

On Fr, 2026-05-22 at 09:20 +0200, Heiko Stuebner wrote:
> Am Donnerstag, 21. Mai 2026, 23:09:15 Mitteleuropäische Sommerzeit schrieb Heiko Stuebner:
> > Do not put the reset control, retain exclusive control over it.
> > After turning on a CPU, the corresponding reset line must stay
> > deasserted.
> > 
> > This also avoids calling reset_control_put() before workqueues
> > are operational.
> > 
> > Fixes: 78ebbff6d1a0 ("reset: handle removing supplier before consumers")
> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > Tested-by: Steven Price <steven.price@arm.com>
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > ---
> >  arch/arm/mach-rockchip/platsmp.c | 16 ++++++++++------
> >  1 file changed, 10 insertions(+), 6 deletions(-)
> > 
> > diff --git a/arch/arm/mach-rockchip/platsmp.c b/arch/arm/mach-rockchip/platsmp.c
> > index f432d22bfed8..f659d894bfae 100644
> > --- a/arch/arm/mach-rockchip/platsmp.c
> > +++ b/arch/arm/mach-rockchip/platsmp.c
> > @@ -34,6 +34,7 @@ static int ncores;
> >  
> >  static struct regmap *pmu;
> >  static int has_pmu = true;
> > +static struct reset_control *cpu_rstc[4];
> 
> After sleeping on that, this should be cpu_rstc[5];
> 
> Coretx-A9 SoCs need to enable the SCU power-domain which thankfully
> sits at index 4 of the power-domain register.
> 
> So while we (already) expect no reset control for that, we need at least
> make sure, it's not reading into undefined memory

The access in pmu_set_power_domain() is gated by
(pd < ARRAY_SIZE(cpu_rstc)). And ncores in rockchip_smp_prepare_cpus()
can not be larger than 4.

> and thus need that empty field in the array.

We could drop the ARRAY_SIZE check and add a placeholder to the array,
but it would need to be set to ERR_PTR(-EINVAL). Otherwise we'd have to
replace all the IS_ERR(rstc) checks as well. I think that would be a
good follow-up change.

regards
Philipp

Re: [PATCH] ARM: rockchip: keep reset control around

[Heiko Stuebner]

Am Freitag, 22. Mai 2026, 10:20:06 Mitteleuropäische Sommerzeit schrieb Philipp Zabel:
> On Fr, 2026-05-22 at 09:20 +0200, Heiko Stuebner wrote:
> > Am Donnerstag, 21. Mai 2026, 23:09:15 Mitteleuropäische Sommerzeit schrieb Heiko Stuebner:
> > > Do not put the reset control, retain exclusive control over it.
> > > After turning on a CPU, the corresponding reset line must stay
> > > deasserted.
> > > 
> > > This also avoids calling reset_control_put() before workqueues
> > > are operational.
> > > 
> > > Fixes: 78ebbff6d1a0 ("reset: handle removing supplier before consumers")
> > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > > Tested-by: Steven Price <steven.price@arm.com>
> > > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > > ---
> > >  arch/arm/mach-rockchip/platsmp.c | 16 ++++++++++------
> > >  1 file changed, 10 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/arch/arm/mach-rockchip/platsmp.c b/arch/arm/mach-rockchip/platsmp.c
> > > index f432d22bfed8..f659d894bfae 100644
> > > --- a/arch/arm/mach-rockchip/platsmp.c
> > > +++ b/arch/arm/mach-rockchip/platsmp.c
> > > @@ -34,6 +34,7 @@ static int ncores;
> > >  
> > >  static struct regmap *pmu;
> > >  static int has_pmu = true;
> > > +static struct reset_control *cpu_rstc[4];
> > 
> > After sleeping on that, this should be cpu_rstc[5];
> > 
> > Coretx-A9 SoCs need to enable the SCU power-domain which thankfully
> > sits at index 4 of the power-domain register.
> > 
> > So while we (already) expect no reset control for that, we need at least
> > make sure, it's not reading into undefined memory
> 
> The access in pmu_set_power_domain() is gated by
> (pd < ARRAY_SIZE(cpu_rstc)). And ncores in rockchip_smp_prepare_cpus()
> can not be larger than 4.

argh ... you're right of course. So the patch can stay as it is.
Should not think about code before going to sleep ;-)


Heiko

> > and thus need that empty field in the array.
> 
> We could drop the ARRAY_SIZE check and add a placeholder to the array,
> but it would need to be set to ERR_PTR(-EINVAL). Otherwise we'd have to
> replace all the IS_ERR(rstc) checks as well. I think that would be a
> good follow-up change.
> 
> regards
> Philipp
>