[PATCH] ARM: cache-l2x0: fix wrong print of AUX_CTRL register

[PATCH] ARM: cache-l2x0: fix wrong print of AUX_CTRL register

[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 <srinidhi.kasagar@stericsson.com>
Signed-off-by: Vijaya Kumar Kilari <vijay.kilari@stericsson.com>
---
 arch/arm/mm/cache-l2x0.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index ef59099..d07f8cb 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -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);
-- 
1.7.0.4

[PATCH] ARM: cache-l2x0: fix wrong print of AUX_CTRL register

[Santosh Shilimkar]

On 4/27/2011 5:56 PM, srinidhi kasagar wrote:
> 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<srinidhi.kasagar@stericsson.com>
> Signed-off-by: Vijaya Kumar Kilari<vijay.kilari@stericsson.com>
> ---
>   arch/arm/mm/cache-l2x0.c |    2 ++
>   1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
> index ef59099..d07f8cb 100644
> --- a/arch/arm/mm/cache-l2x0.c
> +++ b/arch/arm/mm/cache-l2x0.c
> @@ -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

[PATCH] ARM: cache-l2x0: fix wrong print of AUX_CTRL register

[Catalin Marinas]

On 27 April 2011 13:42, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:
> On 4/27/2011 5:56 PM, srinidhi kasagar wrote:
>>
>> 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<srinidhi.kasagar@stericsson.com>
>> Signed-off-by: Vijaya Kumar Kilari<vijay.kilari@stericsson.com>
>> ---
>> ?arch/arm/mm/cache-l2x0.c | ? ?2 ++
>> ?1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
>> index ef59099..d07f8cb 100644
>> --- a/arch/arm/mm/cache-l2x0.c
>> +++ b/arch/arm/mm/cache-l2x0.c
>> @@ -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() ?

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.

-- 
Catalin

[PATCH] ARM: cache-l2x0: fix wrong print of AUX_CTRL register

[Santosh Shilimkar]

On 4/27/2011 6:22 PM, Catalin Marinas wrote:
> On 27 April 2011 13:42, Santosh Shilimkar<santosh.shilimkar@ti.com>  wrote:
>> On 4/27/2011 5:56 PM, srinidhi kasagar wrote:
>>>
>>> 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<srinidhi.kasagar@stericsson.com>
>>> Signed-off-by: Vijaya Kumar Kilari<vijay.kilari@stericsson.com>
>>> ---
>>>   arch/arm/mm/cache-l2x0.c |    2 ++
>>>   1 files changed, 2 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
>>> index ef59099..d07f8cb 100644
>>> --- a/arch/arm/mm/cache-l2x0.c
>>> +++ b/arch/arm/mm/cache-l2x0.c
>>> @@ -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() ?
>
> We already read this register in l2x0_init().
>
Right..

> 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

[PATCH] ARM: cache-l2x0: fix wrong print of AUX_CTRL register

[Catalin Marinas]

On 27 April 2011 13:26, srinidhi kasagar
<srinidhi.kasagar@stericsson.com> wrote:
> 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 <srinidhi.kasagar@stericsson.com>
> Signed-off-by: Vijaya Kumar Kilari <vijay.kilari@stericsson.com>
> ---
> ?arch/arm/mm/cache-l2x0.c | ? ?2 ++
> ?1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
> index ef59099..d07f8cb 100644
> --- a/arch/arm/mm/cache-l2x0.c
> +++ b/arch/arm/mm/cache-l2x0.c
> @@ -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

-- 
Catalin

[PATCH] ARM: cache-l2x0: fix wrong print of AUX_CTRL register

[Santosh Shilimkar]

On 4/27/2011 5:56 PM, srinidhi kasagar wrote:
> 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<srinidhi.kasagar@stericsson.com>
> Signed-off-by: Vijaya Kumar Kilari<vijay.kilari@stericsson.com>
> ---
>   arch/arm/mm/cache-l2x0.c |    2 ++
>   1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
> index ef59099..d07f8cb 100644
> --- a/arch/arm/mm/cache-l2x0.c
> +++ b/arch/arm/mm/cache-l2x0.c
> @@ -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