[PATCH] ARM: imx: do not bring up unavailable cores

[PATCH] ARM: imx: do not bring up unavailable cores

[Shawn Guo]

The i.MX6 Quad can be fused as i.MX6 Dual chip, and similarly i.MX6
DualLite can be fused as i.MX6 Solo.  The actual number of available
cores can be found out from SCU.

Since we do not reflect the fusing thing in device tree, the function
arm_dt_init_cpu_maps() will always call set_cpu_possible(true) for 4
cores on i.MX6 Quad/Dual and 2 cores for i.MX6 DualLite/Solo.  This
causes failures when kernel tries to bring those unavailable cores
online.  For example, the following failure message will be seen when
booting an i.MX6 Solo chip.

  CPU1: failed to come online

Though kernel will still boot fine, the message is somehow annoying.
Let's get rid of it by calling set_cpu_possible(false) on those
unavailable cores.

While at it, the set_cpu_possible(true) for available cores is removed,
since it's already been done in arm_dt_init_cpu_maps().

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 arch/arm/mach-imx/platsmp.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-imx/platsmp.c b/arch/arm/mach-imx/platsmp.c
index 7c0b03f..7061bde 100644
--- a/arch/arm/mach-imx/platsmp.c
+++ b/arch/arm/mach-imx/platsmp.c
@@ -79,8 +79,8 @@ static void __init imx_smp_init_cpus(void)
 
 	ncores = scu_get_core_count(scu_base);
 
-	for (i = 0; i < ncores; i++)
-		set_cpu_possible(i, true);
+	for (i = ncores; i < NR_CPUS; i++)
+		set_cpu_possible(i, false);
 }
 
 void imx_smp_prepare(void)
-- 
1.7.9.5

[PATCH] ARM: imx: do not bring up unavailable cores

[Will Deacon]

On Tue, Apr 02, 2013 at 03:32:26PM +0100, Shawn Guo wrote:
> The i.MX6 Quad can be fused as i.MX6 Dual chip, and similarly i.MX6
> DualLite can be fused as i.MX6 Solo.  The actual number of available
> cores can be found out from SCU.
> 
> Since we do not reflect the fusing thing in device tree, the function
> arm_dt_init_cpu_maps() will always call set_cpu_possible(true) for 4
> cores on i.MX6 Quad/Dual and 2 cores for i.MX6 DualLite/Solo.  This
> causes failures when kernel tries to bring those unavailable cores
> online.  For example, the following failure message will be seen when
> booting an i.MX6 Solo chip.
> 
>   CPU1: failed to come online
> 
> Though kernel will still boot fine, the message is somehow annoying.
> Let's get rid of it by calling set_cpu_possible(false) on those
> unavailable cores.
> 
> While at it, the set_cpu_possible(true) for available cores is removed,
> since it's already been done in arm_dt_init_cpu_maps().
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
>  arch/arm/mach-imx/platsmp.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/platsmp.c b/arch/arm/mach-imx/platsmp.c
> index 7c0b03f..7061bde 100644
> --- a/arch/arm/mach-imx/platsmp.c
> +++ b/arch/arm/mach-imx/platsmp.c
> @@ -79,8 +79,8 @@ static void __init imx_smp_init_cpus(void)
>  
>  	ncores = scu_get_core_count(scu_base);
>  
> -	for (i = 0; i < ncores; i++)
> -		set_cpu_possible(i, true);
> +	for (i = ncores; i < NR_CPUS; i++)
> +		set_cpu_possible(i, false);
>  }

So you're passing the wrong DT to the kernel and then getting annoyed about
some warnings in the dmesg? Assuming the bootloader knows how many CPUs
there are, shouldn't that be sorting out the CPU nodes before passing the
dtb to the kernel?

Will

[PATCH] ARM: imx: do not bring up unavailable cores

[Shawn Guo]

On Tue, Apr 02, 2013 at 04:17:15PM +0100, Will Deacon wrote:
> On Tue, Apr 02, 2013 at 03:32:26PM +0100, Shawn Guo wrote:
> > The i.MX6 Quad can be fused as i.MX6 Dual chip, and similarly i.MX6
> > DualLite can be fused as i.MX6 Solo.  The actual number of available
> > cores can be found out from SCU.
> > 
> > Since we do not reflect the fusing thing in device tree, the function
> > arm_dt_init_cpu_maps() will always call set_cpu_possible(true) for 4
> > cores on i.MX6 Quad/Dual and 2 cores for i.MX6 DualLite/Solo.  This
> > causes failures when kernel tries to bring those unavailable cores
> > online.  For example, the following failure message will be seen when
> > booting an i.MX6 Solo chip.
> > 
> >   CPU1: failed to come online
> > 
> > Though kernel will still boot fine, the message is somehow annoying.
> > Let's get rid of it by calling set_cpu_possible(false) on those
> > unavailable cores.
> > 
> > While at it, the set_cpu_possible(true) for available cores is removed,
> > since it's already been done in arm_dt_init_cpu_maps().
> > 
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > ---
> >  arch/arm/mach-imx/platsmp.c |    4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm/mach-imx/platsmp.c b/arch/arm/mach-imx/platsmp.c
> > index 7c0b03f..7061bde 100644
> > --- a/arch/arm/mach-imx/platsmp.c
> > +++ b/arch/arm/mach-imx/platsmp.c
> > @@ -79,8 +79,8 @@ static void __init imx_smp_init_cpus(void)
> >  
> >  	ncores = scu_get_core_count(scu_base);
> >  
> > -	for (i = 0; i < ncores; i++)
> > -		set_cpu_possible(i, true);
> > +	for (i = ncores; i < NR_CPUS; i++)
> > +		set_cpu_possible(i, false);
> >  }
> 
> So you're passing the wrong DT to the kernel and then getting annoyed about
> some warnings in the dmesg?

I wouldn't way I'm passing the wrong DT to the kernel.  Here is the
thing, for i.MX6 DualLite example.

The i.MX6 DualLite is a dual core CA9 SoC.  The SoC is designed with a
One Time Programming (OTP) fuse bit.  When this fuse bit gets
programmed, one of the core will become unavailable and SCU
register will report core number as 1.

We write up DT for dual core case as default and let kernel to find out
if the i.MX6 DualLite it runs on actually has only one core available.

> Assuming the bootloader knows how many CPUs
> there are, shouldn't that be sorting out the CPU nodes before passing the
> dtb to the kernel?

The bootloader doesn't know the core numbers, unless we have code in
bootloader to find it out by reading fuse bit or SCU register that the
i.MX6 DualLite has one or two cores available.

Shawn

[PATCH] ARM: imx: do not bring up unavailable cores

[Fabio Estevam]

On Tue, Apr 2, 2013 at 11:13 PM, Shawn Guo <shawn.guo@linaro.org> wrote:

> The bootloader doesn't know the core numbers, unless we have code in
> bootloader to find it out by reading fuse bit or SCU register that the
> i.MX6 DualLite has one or two cores available.

U-boot does read SCU register and can identify Quad/Dual-Lite/Solo/Solo-lite.

[PATCH] ARM: imx: do not bring up unavailable cores

[Shawn Guo]

On Tue, Apr 02, 2013 at 11:38:26PM -0300, Fabio Estevam wrote:
> On Tue, Apr 2, 2013 at 11:13 PM, Shawn Guo <shawn.guo@linaro.org> wrote:
> 
> > The bootloader doesn't know the core numbers, unless we have code in
> > bootloader to find it out by reading fuse bit or SCU register that the
> > i.MX6 DualLite has one or two cores available.
> 
> U-boot does read SCU register and can identify Quad/Dual-Lite/Solo/Solo-lite.

Ah, great.  I haven't checked the U-Boot code for some time.  But does
U-Boot remove cpu nodes of those unavailable cores from device tree
before passing it to kernel?

Shawn

[PATCH] ARM: imx: do not bring up unavailable cores

[Fabio Estevam]

On Tue, Apr 2, 2013 at 11:47 PM, Shawn Guo <shawn.guo@linaro.org> wrote:

> Ah, great.  I haven't checked the U-Boot code for some time.  But does
> U-Boot remove cpu nodes of those unavailable cores from device tree
> before passing it to kernel?

No, it just uses the information from the SCU to print the name of the
mx6 variant on boot.

[PATCH] ARM: imx: do not bring up unavailable cores

[Rob Herring]

On 04/02/2013 10:18 PM, Fabio Estevam wrote:
> On Tue, Apr 2, 2013 at 11:47 PM, Shawn Guo <shawn.guo@linaro.org> wrote:
> 
>> Ah, great.  I haven't checked the U-Boot code for some time.  But does
>> U-Boot remove cpu nodes of those unavailable cores from device tree
>> before passing it to kernel?
> 
> No, it just uses the information from the SCU to print the name of the
> mx6 variant on boot.

It is quite common and the correct place IMHO to do this dtb fixup in
u-boot. You can just add 'status = "disabled";' properties. I'm not sure
that the kernel is checking the status though.

Reading the core count from the SCU is the only reason the SCU needs to
be statically mapped, so if you remove that you can remove the static
mapping.

Rob

[PATCH] ARM: imx: do not bring up unavailable cores

[Shawn Guo]

On Wed, Apr 03, 2013 at 08:01:02AM -0500, Rob Herring wrote:
> On 04/02/2013 10:18 PM, Fabio Estevam wrote:
> > On Tue, Apr 2, 2013 at 11:47 PM, Shawn Guo <shawn.guo@linaro.org> wrote:
> > 
> >> Ah, great.  I haven't checked the U-Boot code for some time.  But does
> >> U-Boot remove cpu nodes of those unavailable cores from device tree
> >> before passing it to kernel?
> > 
> > No, it just uses the information from the SCU to print the name of the
> > mx6 variant on boot.
> 
> It is quite common and the correct place IMHO to do this dtb fixup in
> u-boot.

Yes, I agree.

> You can just add 'status = "disabled";' properties. I'm not sure
> that the kernel is checking the status though.
> 
No, I do not think that kernel is current checking property 'status'.

> Reading the core count from the SCU is the only reason the SCU needs to
> be statically mapped, so if you remove that you can remove the static
> mapping.

Yea, that's something desirable.  We can do that after both u-boot and
kernel get ready to manipulate 'status' property of cpu node.

Shawn