ARM: cache-l2x0: fix wrong print of AUX_CTRL register

PATCH New srinidhi kasagar 3 participants 6 messages Apr 27, 2011 Arm (32-bit)

Share & tools

srinidhi kasagar

author

2011-04-27 12:26 UTC #0 lore raw

PATCH 1/1 ARM: cache-l2x0: fix wrong print of AUX_CTRL register

On platforms where l2x0 is already enabled and configured by the secure code, printing the masked value of the AUX_CTRL register seems to be wrong and differ from the programmed value.

Fix it by reading the AUX_CTRL register again.

Signed-off-by srinidhi kasagar <[email protected]>

Signed-off-by Vijaya Kumar Kilari <[email protected]>

---
 arch/arm/mm/cache-l2x0.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

arch/arm/mm/cache-l2x0.c +2 -0

index ef59099..d07f8cb 100644
@@ -340,6 +340,8 @@ void __init l2x0_init(void __iomem *base, __u32 aux_val, __u32 aux_mask)
 	outer_cache.disable = l2x0_disable;
 	outer_cache.set_debug = l2x0_set_debug;
 
+	aux = readl_relaxed(l2x0_base + L2X0_AUX_CTRL);
+
 	printk(KERN_INFO "%s cache controller enabled\n", type);
 	printk(KERN_INFO "l2x0: %d ways, CACHE_ID 0x%08x, AUX_CTRL 0x%08x, Cache size: %d B\n",
 			ways, cache_id, aux, l2x0_size);

Signature · 1 line

1.7.0.4

Santosh Shilimkar

2011-04-27 12:42 UTC #1 lore raw

PATCH 1/1 ARM: cache-l2x0: fix wrong print of AUX_CTRL register

Quoted 18 lines · srinidhi kasagar

On platforms where l2x0 is already enabled and configured by the secure code, printing the masked value of the AUX_CTRL register seems to be wrong and differ from the programmed value.
Fix it by reading the AUX_CTRL register again.
Signed-off-by srinidhi kasagar<[email protected]>
Signed-off-by Vijaya Kumar Kilari<[email protected]>
---
  arch/arm/mm/cache-l2x0.c |    2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)
arch/arm/mm/cache-l2x0.c +0 -0
index ef59099..d07f8cb 100644

arch/arm/mm/cache-l2x0.c void __init l2x0_init(void __iomem *base, __u32 aux_val, __u32 aux_mask) · line 344

Quoted 5 lines

arch/arm/mm/cache-l2x0.c +2 -0

@@ -340,6 +340,8 @@ void __init l2x0_init(void __iomem *base, __u32 aux_val, __u32 aux_mask)
  	outer_cache.disable = l2x0_disable;
  	outer_cache.set_debug = l2x0_set_debug;
 
+	aux = readl_relaxed(l2x0_base + L2X0_AUX_CTRL);
+

Can you not read this register in your machine code and pass the right 'aux_val' as part of l2x0_init() ?

Regards
Santosh

Catalin Marinas

2011-04-27 12:52 UTC #2 lore raw

PATCH 1/1 ARM: cache-l2x0: fix wrong print of AUX_CTRL register

Quoted 24 lines · Santosh Shilimkar

Quoted 23 lines · srinidhi kasagar
On platforms where l2x0 is already enabled and configured by the secure code, printing the masked value of the AUX_CTRL register seems to be wrong and differ from the programmed value.
Fix it by reading the AUX_CTRL register again.
Signed-off-by srinidhi kasagar<[email protected]>
Signed-off-by Vijaya Kumar Kilari<[email protected]>
---
?arch/arm/mm/cache-l2x0.c | ? ?2 ++
?1 files changed, 2 insertions(+), 0 deletions(-)
arch/arm/mm/cache-l2x0.c +0 -0
index ef59099..d07f8cb 100644
@@ -340,6 +340,8 @@ void __init l2x0_init(void __iomem *base, __u32
 aux_val, __u32 aux_mask)
 ? ? ? ?outer_cache.disable = l2x0_disable;
 ? ? ? ?outer_cache.set_debug = l2x0_set_debug;
 

Quoted 5 lines

arch/arm/mm/cache-l2x0.c void __init l2x0_init(void __iomem *base, __u32 · line 345
Quoted 2 lines
arch/arm/mm/cache-l2x0.c +2 -0
+ ? ? ? aux = readl_relaxed(l2x0_base + L2X0_AUX_CTRL);
+
Can you not read this register in your machine code and pass the right 'aux_val' as part of l2x0_init() ?

We already read this register in l2x0_init().

In theory, the Linux kernel shouldn't even touch this register but leave it to the boot loader or other firmware (or just some sane reset values that the hardware people should put). But no-one seems to be doing this.

Signature · 1 line

Catalin

Santosh Shilimkar

2011-04-27 13:01 UTC #3 lore raw

PATCH 1/1 ARM: cache-l2x0: fix wrong print of AUX_CTRL register

Quoted 25 lines · Catalin Marinas

Quoted 24 lines · Santosh Shilimkar
Quoted 23 lines · srinidhi kasagar
On platforms where l2x0 is already enabled and configured by the secure code, printing the masked value of the AUX_CTRL register seems to be wrong and differ from the programmed value.
Fix it by reading the AUX_CTRL register again.
Signed-off-by srinidhi kasagar<[email protected]>
Signed-off-by Vijaya Kumar Kilari<[email protected]>
---
  arch/arm/mm/cache-l2x0.c |    2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)
arch/arm/mm/cache-l2x0.c +0 -0
index ef59099..d07f8cb 100644
@@ -340,6 +340,8 @@ void __init l2x0_init(void __iomem *base, __u32
 aux_val, __u32 aux_mask)
        outer_cache.disable = l2x0_disable;
        outer_cache.set_debug = l2x0_set_debug;
 

Quoted 8 lines

Quoted 5 lines
arch/arm/mm/cache-l2x0.c void __init l2x0_init(void __iomem *base, __u32 aux_val, __u32 aux_mask) · line 344
Quoted 2 lines
arch/arm/mm/cache-l2x0.c +2 -0
+       aux = readl_relaxed(l2x0_base + L2X0_AUX_CTRL);
+
Can you not read this register in your machine code and pass the right 'aux_val' as part of l2x0_init() ?
We already read this register in l2x0_init().

Right..

Quoted 5 lines

In theory, the Linux kernel shouldn't even touch this register but leave it to the boot loader or other firmware (or just some sane reset values that the hardware people should put). But no-one seems to be doing this.

At times the firmware value is not most optimal. Boot-loaders can never be trusted.
On OMAP on later silicon versions, we could enable some of the enhancement but the secure code was same and was not settings those.

Regards
Santosh

Catalin Marinas

2011-04-27 12:49 UTC #4 lore raw

PATCH 1/1 ARM: cache-l2x0: fix wrong print of AUX_CTRL register

In reply to srinidhi kasagar

Quoted 18 lines · srinidhi kasagar

On platforms where l2x0 is already enabled and configured by the secure code, printing the masked value of the AUX_CTRL register seems to be wrong and differ from the programmed value.
Fix it by reading the AUX_CTRL register again.
Signed-off-by srinidhi kasagar <[email protected]>
Signed-off-by Vijaya Kumar Kilari <[email protected]>
---
?arch/arm/mm/cache-l2x0.c | ? ?2 ++
?1 files changed, 2 insertions(+), 0 deletions(-)
arch/arm/mm/cache-l2x0.c +0 -0
index ef59099..d07f8cb 100644

arch/arm/mm/cache-l2x0.c void __init l2x0_init(void __iomem *base, __u32 aux_val, __u32 aux_mask) · line 347

Quoted 8 lines

arch/arm/mm/cache-l2x0.c +2 -0

@@ -340,6 +340,8 @@ void __init l2x0_init(void __iomem *base, __u32 aux_val, __u32 aux_mask)
 ? ? ? ?outer_cache.disable = l2x0_disable;
 ? ? ? ?outer_cache.set_debug = l2x0_set_debug;
 
+ ? ? ? aux = readl_relaxed(l2x0_base + L2X0_AUX_CTRL);
+
 ? ? ? ?printk(KERN_INFO "%s cache controller enabled\n", type);
 ? ? ? ?printk(KERN_INFO "l2x0: %d ways, CACHE_ID 0x%08x, AUX_CTRL 0x%08x, Cache size: %d B\n",
 ? ? ? ? ? ? ? ? ? ? ? ?ways, cache_id, aux, l2x0_size);

Rather than re-reading it, you could actually move the masking to the 'if' block that writes that value. I have a patch in Russell's system that does this together with setting bit 22 but it hasn't been merged (I still think it should be):

http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=6529/1

Signature · 1 line

Catalin

Santosh Shilimkar

2011-04-27 12:54 UTC #5 lore raw

PATCH 1/1 ARM: cache-l2x0: fix wrong print of AUX_CTRL register

In reply to srinidhi kasagar

Quoted 18 lines · srinidhi kasagar

On platforms where l2x0 is already enabled and configured by the secure code, printing the masked value of the AUX_CTRL register seems to be wrong and differ from the programmed value.
Fix it by reading the AUX_CTRL register again.
Signed-off-by srinidhi kasagar<[email protected]>
Signed-off-by Vijaya Kumar Kilari<[email protected]>
---
  arch/arm/mm/cache-l2x0.c |    2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)
arch/arm/mm/cache-l2x0.c +0 -0
index ef59099..d07f8cb 100644

arch/arm/mm/cache-l2x0.c void __init l2x0_init(void __iomem *base, __u32 aux_val, __u32 aux_mask) · line 344

Quoted 5 lines

arch/arm/mm/cache-l2x0.c +2 -0

@@ -340,6 +340,8 @@ void __init l2x0_init(void __iomem *base, __u32 aux_val, __u32 aux_mask)
  	outer_cache.disable = l2x0_disable;
  	outer_cache.set_debug = l2x0_set_debug;
 
+	aux = readl_relaxed(l2x0_base + L2X0_AUX_CTRL);
+

Can you not read this register in your machine code and pass the right 'aux_val' as part of l2x0_init() ?

Regards
Santosh