Patchwork [U-Boot,05/11] MIPS: add sleep handler for slave CPUs in multi-processor systems

login
register
mail settings
Submitter Daniel Schwierzeck
Date Nov. 24, 2011, 1:57 p.m.
Message ID <1322143076-20349-6-git-send-email-daniel.schwierzeck@googlemail.com>
Download mbox | patch
Permalink /patch/127521/
State Superseded
Headers show

Comments

Daniel Schwierzeck - Nov. 24, 2011, 1:57 p.m.
This handler can be activated on multi-processor systems to boot only
the master CPU. All slave CPUs are halted by executing the WAIT
instruction. This is also useful to reduce the power consumption at
boot time.

Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck@googlemail.com>
---
 arch/mips/cpu/mips32/start.S |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)
Marek Vasut - Nov. 25, 2011, 8:44 a.m.
> This handler can be activated on multi-processor systems to boot only
> the master CPU. All slave CPUs are halted by executing the WAIT
> instruction. This is also useful to reduce the power consumption at
> boot time.
> 
> Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck@googlemail.com>
> ---
>  arch/mips/cpu/mips32/start.S |   16 ++++++++++++++++
>  1 files changed, 16 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/mips/cpu/mips32/start.S b/arch/mips/cpu/mips32/start.S
> index 9c1b2f7..b6cb4be 100644
> --- a/arch/mips/cpu/mips32/start.S
> +++ b/arch/mips/cpu/mips32/start.S
> @@ -224,6 +224,14 @@ reset:
> 
>  	setup_c0_status_reset
> 
> +	/* Set all slave CPUs in sleep mode */
> +#ifdef CONFIG_SYS_MPS_SLAVE_CPU_SLEEP
> +	mfc0	k0, CP0_EBASE
> +	and	k0, EBASEF_CPUNUM
> +	bne	k0, zero, slave_cpu_sleep
> +	 nop
> +#endif
> +
>  	/* Init Timer */
>  	mtc0	zero, CP0_COUNT
>  	mtc0	zero, CP0_COMPARE
> @@ -383,3 +391,11 @@ romReserved:
> 
>  romExcHandle:
>  	b	romExcHandle
> +
> +	/* Additional handlers */
> +#ifdef CONFIG_SYS_MPS_SLAVE_CPU_SLEEP
> +slave_cpu_sleep:
> +	wait
> +	b	slave_cpu_sleep
> +	 nop
> +#endif

Can't you stall the CPU instead of letting it run in an empty loop? If not:

Acked-by: Marek Vasut <marek.vasut@gmail.com>
Daniel Schwierzeck - Nov. 25, 2011, 12:19 p.m.
On Fri, Nov 25, 2011 at 9:44 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> This handler can be activated on multi-processor systems to boot only
>> the master CPU. All slave CPUs are halted by executing the WAIT
>> instruction. This is also useful to reduce the power consumption at
>> boot time.
>>
>> Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck@googlemail.com>
>> ---
>>  arch/mips/cpu/mips32/start.S |   16 ++++++++++++++++
>>  1 files changed, 16 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/mips/cpu/mips32/start.S b/arch/mips/cpu/mips32/start.S
>> index 9c1b2f7..b6cb4be 100644
>> --- a/arch/mips/cpu/mips32/start.S
>> +++ b/arch/mips/cpu/mips32/start.S
>> @@ -224,6 +224,14 @@ reset:
>>
>>       setup_c0_status_reset
>>
>> +     /* Set all slave CPUs in sleep mode */
>> +#ifdef CONFIG_SYS_MPS_SLAVE_CPU_SLEEP
>> +     mfc0    k0, CP0_EBASE
>> +     and     k0, EBASEF_CPUNUM
>> +     bne     k0, zero, slave_cpu_sleep
>> +      nop
>> +#endif
>> +
>>       /* Init Timer */
>>       mtc0    zero, CP0_COUNT
>>       mtc0    zero, CP0_COMPARE
>> @@ -383,3 +391,11 @@ romReserved:
>>
>>  romExcHandle:
>>       b       romExcHandle
>> +
>> +     /* Additional handlers */
>> +#ifdef CONFIG_SYS_MPS_SLAVE_CPU_SLEEP
>> +slave_cpu_sleep:
>> +     wait
>> +     b       slave_cpu_sleep
>> +      nop
>> +#endif
>
> Can't you stall the CPU instead of letting it run in an empty loop? If not:
>
> Acked-by: Marek Vasut <marek.vasut@gmail.com>
>

the CPU is stalled with the wait instruction. The loop is actually paranoia ;)

From MIPS32 24KE Processor Core Family Software User's Manual:

The WAIT instruction forces the core into low power mode. The pipeline
is stalled and when all external requests are
completed, the processor’s main clock is stopped. The processor will
restart when reset (SI_Reset) is signaled, or a
non-masked interrupt is taken (SI_NMI, SI_Int, or EJ_DINT). Note that
the core does not use the code field in this
instruction.
Marek Vasut - Nov. 25, 2011, 1:40 p.m.
> On Fri, Nov 25, 2011 at 9:44 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
> >> This handler can be activated on multi-processor systems to boot only
> >> the master CPU. All slave CPUs are halted by executing the WAIT
> >> instruction. This is also useful to reduce the power consumption at
> >> boot time.
> >> 
> >> Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck@googlemail.com>
> >> ---
> >>  arch/mips/cpu/mips32/start.S |   16 ++++++++++++++++
> >>  1 files changed, 16 insertions(+), 0 deletions(-)
> >> 
> >> diff --git a/arch/mips/cpu/mips32/start.S b/arch/mips/cpu/mips32/start.S
> >> index 9c1b2f7..b6cb4be 100644
> >> --- a/arch/mips/cpu/mips32/start.S
> >> +++ b/arch/mips/cpu/mips32/start.S
> >> @@ -224,6 +224,14 @@ reset:
> >> 
> >>       setup_c0_status_reset
> >> 
> >> +     /* Set all slave CPUs in sleep mode */
> >> +#ifdef CONFIG_SYS_MPS_SLAVE_CPU_SLEEP
> >> +     mfc0    k0, CP0_EBASE
> >> +     and     k0, EBASEF_CPUNUM
> >> +     bne     k0, zero, slave_cpu_sleep
> >> +      nop
> >> +#endif
> >> +
> >>       /* Init Timer */
> >>       mtc0    zero, CP0_COUNT
> >>       mtc0    zero, CP0_COMPARE
> >> @@ -383,3 +391,11 @@ romReserved:
> >> 
> >>  romExcHandle:
> >>       b       romExcHandle
> >> +
> >> +     /* Additional handlers */
> >> +#ifdef CONFIG_SYS_MPS_SLAVE_CPU_SLEEP
> >> +slave_cpu_sleep:
> >> +     wait
> >> +     b       slave_cpu_sleep
> >> +      nop
> >> +#endif
> > 
> > Can't you stall the CPU instead of letting it run in an empty loop? If
> > not:
> > 
> > Acked-by: Marek Vasut <marek.vasut@gmail.com>
> 
> the CPU is stalled with the wait instruction. The loop is actually paranoia
> ;)
> 
> From MIPS32 24KE Processor Core Family Software User's Manual:
> 
> The WAIT instruction forces the core into low power mode. The pipeline
> is stalled and when all external requests are
> completed, the processor’s main clock is stopped. The processor will
> restart when reset (SI_Reset) is signaled, or a
> non-masked interrupt is taken (SI_NMI, SI_Int, or EJ_DINT). Note that
> the core does not use the code field in this
> instruction.

Oh yea, this kind of stuff I remember like I hacked on it yesterday :)
Andrew Dyer - Nov. 25, 2011, 3:35 p.m.
On Fri, Nov 25, 2011 at 02:44, Marek Vasut <marek.vasut@gmail.com> wrote:

> > This handler can be activated on multi-processor systems to boot only
> > the master CPU. All slave CPUs are halted by executing the WAIT
> > instruction. This is also useful to reduce the power consumption at
> > boot time.
> >
> > Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck@googlemail.com>
> > ---
> >  arch/mips/cpu/mips32/start.S |   16 ++++++++++++++++
> >  1 files changed, 16 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/mips/cpu/mips32/start.S b/arch/mips/cpu/mips32/start.S
> > index 9c1b2f7..b6cb4be 100644
> > --- a/arch/mips/cpu/mips32/start.S
> > +++ b/arch/mips/cpu/mips32/start.S
> > @@ -224,6 +224,14 @@ reset:
> >
> >       setup_c0_status_reset
> >
> > +     /* Set all slave CPUs in sleep mode */
> > +#ifdef CONFIG_SYS_MPS_SLAVE_CPU_SLEEP
>

shouldn't this be CONFIG_SYS_MIPS_SLAVE_CPU_SLEEP?  (s/MPS/MIPS/)
Shinya Kuribayashi - Nov. 28, 2011, 4:24 p.m.
On 11/24/11 10:57 PM, Daniel Schwierzeck wrote:
> diff --git a/arch/mips/cpu/mips32/start.S b/arch/mips/cpu/mips32/start.S
> index 9c1b2f7..b6cb4be 100644
> --- a/arch/mips/cpu/mips32/start.S
> +++ b/arch/mips/cpu/mips32/start.S
> @@ -224,6 +224,14 @@ reset:
>
>   	setup_c0_status_reset
>
> +	/* Set all slave CPUs in sleep mode */
> +#ifdef CONFIG_SYS_MPS_SLAVE_CPU_SLEEP
> +	mfc0	k0, CP0_EBASE
> +	and	k0, EBASEF_CPUNUM
> +	bne	k0, zero, slave_cpu_sleep
> +	 nop
> +#endif
> +
>   	/* Init Timer */
>   	mtc0	zero, CP0_COUNT
>   	mtc0	zero, CP0_COMPARE

Just wondered, why is this conditionally selected?  To save text size,
or other reason?

The change looks Ok with s/MPS/MIPS/ typo fixed as pointed by Andrew.
Daniel Schwierzeck - Nov. 29, 2011, 3:54 p.m.
On Mon, Nov 28, 2011 at 5:24 PM, Shinya Kuribayashi <skuribay@pobox.com> wrote:
> On 11/24/11 10:57 PM, Daniel Schwierzeck wrote:
>>
>> diff --git a/arch/mips/cpu/mips32/start.S b/arch/mips/cpu/mips32/start.S
>> index 9c1b2f7..b6cb4be 100644
>> --- a/arch/mips/cpu/mips32/start.S
>> +++ b/arch/mips/cpu/mips32/start.S
>> @@ -224,6 +224,14 @@ reset:
>>
>>        setup_c0_status_reset
>>
>> +       /* Set all slave CPUs in sleep mode */
>> +#ifdef CONFIG_SYS_MPS_SLAVE_CPU_SLEEP
>> +       mfc0    k0, CP0_EBASE
>> +       and     k0, EBASEF_CPUNUM
>> +       bne     k0, zero, slave_cpu_sleep
>> +        nop
>> +#endif
>> +
>>        /* Init Timer */
>>        mtc0    zero, CP0_COUNT
>>        mtc0    zero, CP0_COMPARE
>
> Just wondered, why is this conditionally selected?  To save text size,
> or other reason?
>
> The change looks Ok with s/MPS/MIPS/ typo fixed as pointed by Andrew.
>

the patch is relevant only for Lantiq XWAY Danube SoCs which have two 24KC cores
and is ported from Lantiq BSPs.
Actually the internal BootROM already handles the second CPU core.
This code is only
executed, if the board boots from parallel NOR flash without involving
the BootROM. This
is selectable by pin-strapping. But in this mode the second CPU needs
some additonal handling.

So please ignore this patch for the current series. I'll send another
one in the upcoming
Danube SoC support series. Thanks.

Patch

diff --git a/arch/mips/cpu/mips32/start.S b/arch/mips/cpu/mips32/start.S
index 9c1b2f7..b6cb4be 100644
--- a/arch/mips/cpu/mips32/start.S
+++ b/arch/mips/cpu/mips32/start.S
@@ -224,6 +224,14 @@  reset:
 
 	setup_c0_status_reset
 
+	/* Set all slave CPUs in sleep mode */
+#ifdef CONFIG_SYS_MPS_SLAVE_CPU_SLEEP
+	mfc0	k0, CP0_EBASE
+	and	k0, EBASEF_CPUNUM
+	bne	k0, zero, slave_cpu_sleep
+	 nop
+#endif
+
 	/* Init Timer */
 	mtc0	zero, CP0_COUNT
 	mtc0	zero, CP0_COMPARE
@@ -383,3 +391,11 @@  romReserved:
 
 romExcHandle:
 	b	romExcHandle
+
+	/* Additional handlers */
+#ifdef CONFIG_SYS_MPS_SLAVE_CPU_SLEEP
+slave_cpu_sleep:
+	wait
+	b	slave_cpu_sleep
+	 nop
+#endif