diff mbox

[RFC,v2,3/5] ARM: kernel: update cpu_suspend code to use cache LoUIS operations

Message ID 20121211163843.GH16759@mudshark.cambridge.arm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Will Deacon Dec. 11, 2012, 4:38 p.m. UTC
On Tue, Dec 11, 2012 at 04:33:13PM +0000, Will Deacon wrote:
> On Tue, Dec 11, 2012 at 04:07:56PM +0000, Guennadi Liakhovetski wrote:
> > Git bisect identified this patch, in the mainline as
> > 
> > commit dbee0c6fb4c1269b2dfc8b0b7a29907ea7fed560
> > Author: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Date:   Fri Sep 7 11:06:57 2012 +0530
> > 
> >     ARM: kernel: update cpu_suspend code to use cache LoUIS operations
> > 
> > as the culprit of the broken wake up from STR on mackerel, based on an 
> > sh7372 A8 SoC. .config attached.
> 
> My guess is that because Cortex-A8 does not implement the MP extensions,
> the LoUIS field of the CLIDR reads as zero, and the cache isn't flushed at
> all (I can see an early exit in v7_flush_dcache_louis).
> 
> Lorenzo -- how is this supposed to work for uniprocessor CPUs?

Bah, forgot to ask you if the following patch helps...

Will

--->8

Comments

Guennadi Liakhovetski Dec. 11, 2012, 5:07 p.m. UTC | #1
Hi Will

On Tue, 11 Dec 2012, Will Deacon wrote:

> On Tue, Dec 11, 2012 at 04:33:13PM +0000, Will Deacon wrote:
> > On Tue, Dec 11, 2012 at 04:07:56PM +0000, Guennadi Liakhovetski wrote:
> > > Git bisect identified this patch, in the mainline as
> > > 
> > > commit dbee0c6fb4c1269b2dfc8b0b7a29907ea7fed560
> > > Author: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > > Date:   Fri Sep 7 11:06:57 2012 +0530
> > > 
> > >     ARM: kernel: update cpu_suspend code to use cache LoUIS operations
> > > 
> > > as the culprit of the broken wake up from STR on mackerel, based on an 
> > > sh7372 A8 SoC. .config attached.
> > 
> > My guess is that because Cortex-A8 does not implement the MP extensions,
> > the LoUIS field of the CLIDR reads as zero, and the cache isn't flushed at
> > all (I can see an early exit in v7_flush_dcache_louis).
> > 
> > Lorenzo -- how is this supposed to work for uniprocessor CPUs?
> 
> Bah, forgot to ask you if the following patch helps...

Yes, it does.

Thanks
Guennadi

> 
> Will
> 
> --->8
> 
> diff --git a/arch/arm/mm/cache-v7.S b/arch/arm/mm/cache-v7.S
> index cd95664..f58248f 100644
> --- a/arch/arm/mm/cache-v7.S
> +++ b/arch/arm/mm/cache-v7.S
> @@ -44,7 +44,8 @@ ENDPROC(v7_flush_icache_all)
>  ENTRY(v7_flush_dcache_louis)
>         dmb                                     @ ensure ordering with previous memory accesses
>         mrc     p15, 1, r0, c0, c0, 1           @ read clidr, r0 = clidr
> -       ands    r3, r0, #0xe00000               @ extract LoUIS from clidr
> +       ALT_SMP(ands    r3, r0, #(7 << 21))     @ extract LoUIS from clidr
> +       ALT_UP(ands     r3, r0, #(7 << 27))     @ extract LoUU from clidr
>         mov     r3, r3, lsr #20                 @ r3 = LoUIS * 2
>         moveq   pc, lr                          @ return if level == 0
>         mov     r10, #0                         @ r10 (starting level) = 0
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
Will Deacon Dec. 11, 2012, 5:47 p.m. UTC | #2
On Tue, Dec 11, 2012 at 05:07:35PM +0000, Guennadi Liakhovetski wrote:
> Hi Will
> 
> On Tue, 11 Dec 2012, Will Deacon wrote:
> 
> > On Tue, Dec 11, 2012 at 04:33:13PM +0000, Will Deacon wrote:
> > > On Tue, Dec 11, 2012 at 04:07:56PM +0000, Guennadi Liakhovetski wrote:
> > > > Git bisect identified this patch, in the mainline as
> > > > 
> > > > commit dbee0c6fb4c1269b2dfc8b0b7a29907ea7fed560
> > > > Author: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > > > Date:   Fri Sep 7 11:06:57 2012 +0530
> > > > 
> > > >     ARM: kernel: update cpu_suspend code to use cache LoUIS operations
> > > > 
> > > > as the culprit of the broken wake up from STR on mackerel, based on an 
> > > > sh7372 A8 SoC. .config attached.
> > > 
> > > My guess is that because Cortex-A8 does not implement the MP extensions,
> > > the LoUIS field of the CLIDR reads as zero, and the cache isn't flushed at
> > > all (I can see an early exit in v7_flush_dcache_louis).
> > > 
> > > Lorenzo -- how is this supposed to work for uniprocessor CPUs?
> > 
> > Bah, forgot to ask you if the following patch helps...
> 
> Yes, it does.

Cracking, can I add you tested-by please?

Will
Guennadi Liakhovetski Dec. 11, 2012, 5:55 p.m. UTC | #3
On Tue, 11 Dec 2012, Will Deacon wrote:

> On Tue, Dec 11, 2012 at 04:33:13PM +0000, Will Deacon wrote:
> > On Tue, Dec 11, 2012 at 04:07:56PM +0000, Guennadi Liakhovetski wrote:
> > > Git bisect identified this patch, in the mainline as
> > > 
> > > commit dbee0c6fb4c1269b2dfc8b0b7a29907ea7fed560
> > > Author: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > > Date:   Fri Sep 7 11:06:57 2012 +0530
> > > 
> > >     ARM: kernel: update cpu_suspend code to use cache LoUIS operations
> > > 
> > > as the culprit of the broken wake up from STR on mackerel, based on an 
> > > sh7372 A8 SoC. .config attached.
> > 
> > My guess is that because Cortex-A8 does not implement the MP extensions,
> > the LoUIS field of the CLIDR reads as zero, and the cache isn't flushed at
> > all (I can see an early exit in v7_flush_dcache_louis).
> > 
> > Lorenzo -- how is this supposed to work for uniprocessor CPUs?
> 
> Bah, forgot to ask you if the following patch helps...
> 
> Will
> 
> --->8
> 
> diff --git a/arch/arm/mm/cache-v7.S b/arch/arm/mm/cache-v7.S
> index cd95664..f58248f 100644
> --- a/arch/arm/mm/cache-v7.S
> +++ b/arch/arm/mm/cache-v7.S
> @@ -44,7 +44,8 @@ ENDPROC(v7_flush_icache_all)
>  ENTRY(v7_flush_dcache_louis)
>         dmb                                     @ ensure ordering with previous memory accesses
>         mrc     p15, 1, r0, c0, c0, 1           @ read clidr, r0 = clidr
> -       ands    r3, r0, #0xe00000               @ extract LoUIS from clidr
> +       ALT_SMP(ands    r3, r0, #(7 << 21))     @ extract LoUIS from clidr
> +       ALT_UP(ands     r3, r0, #(7 << 27))     @ extract LoUU from clidr
>         mov     r3, r3, lsr #20                 @ r3 = LoUIS * 2
>         moveq   pc, lr                          @ return if level == 0
>         mov     r10, #0                         @ r10 (starting level) = 0

[... later]

> > Yes, it does.
>
> Cracking, can I add you tested-by please?
 
Sure:
 
Tested-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
  
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
Stephen Boyd Dec. 11, 2012, 11:27 p.m. UTC | #4
On 12/11/12 08:38, Will Deacon wrote:
> diff --git a/arch/arm/mm/cache-v7.S b/arch/arm/mm/cache-v7.S
> index cd95664..f58248f 100644
> --- a/arch/arm/mm/cache-v7.S
> +++ b/arch/arm/mm/cache-v7.S
> @@ -44,7 +44,8 @@ ENDPROC(v7_flush_icache_all)
>  ENTRY(v7_flush_dcache_louis)
>         dmb                                     @ ensure ordering with previous memory accesses
>         mrc     p15, 1, r0, c0, c0, 1           @ read clidr, r0 = clidr
> -       ands    r3, r0, #0xe00000               @ extract LoUIS from clidr
> +       ALT_SMP(ands    r3, r0, #(7 << 21))     @ extract LoUIS from clidr
> +       ALT_UP(ands     r3, r0, #(7 << 27))     @ extract LoUU from clidr
>         mov     r3, r3, lsr #20                 @ r3 = LoUIS * 2

You need to fix this mov as well, right?
diff mbox

Patch

diff --git a/arch/arm/mm/cache-v7.S b/arch/arm/mm/cache-v7.S
index cd95664..f58248f 100644
--- a/arch/arm/mm/cache-v7.S
+++ b/arch/arm/mm/cache-v7.S
@@ -44,7 +44,8 @@  ENDPROC(v7_flush_icache_all)
 ENTRY(v7_flush_dcache_louis)
        dmb                                     @ ensure ordering with previous memory accesses
        mrc     p15, 1, r0, c0, c0, 1           @ read clidr, r0 = clidr
-       ands    r3, r0, #0xe00000               @ extract LoUIS from clidr
+       ALT_SMP(ands    r3, r0, #(7 << 21))     @ extract LoUIS from clidr
+       ALT_UP(ands     r3, r0, #(7 << 27))     @ extract LoUU from clidr
        mov     r3, r3, lsr #20                 @ r3 = LoUIS * 2
        moveq   pc, lr                          @ return if level == 0
        mov     r10, #0                         @ r10 (starting level) = 0