diff mbox

[v2] ARM: BCM5301X: set customized AUXCTL

Message ID 1414852899-30947-1-git-send-email-hauke@hauke-m.de (mailing list archive)
State New, archived
Headers show

Commit Message

Hauke Mehrtens Nov. 1, 2014, 2:41 p.m. UTC
This activates some more features in the l310 cache.

This is based on some vendor code.

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 arch/arm/mach-bcm/bcm_5301x.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

changes
v1:
 - fix the commit message.

Comments

Arnd Bergmann Nov. 3, 2014, 10 a.m. UTC | #1
On Saturday 01 November 2014 15:41:38 Hauke Mehrtens wrote:
>  };
>  
>  DT_MACHINE_START(BCM5301X, "BCM5301X")
> -       .l2c_aux_val    = 0,
> +       .l2c_aux_val    = L310_AUX_CTRL_CACHE_REPLACE_RR |
> +                         L310_AUX_CTRL_DATA_PREFETCH |
> +                         L310_AUX_CTRL_INSTR_PREFETCH |
> +                         L310_AUX_CTRL_EARLY_BRESP,

Please put these into the dts file instead if the boot loader fails
to set them correctly. We should not override them from source code
for new platforms any more.

	Arnd
Hauke Mehrtens Nov. 3, 2014, 11:16 p.m. UTC | #2
On 11/03/2014 11:00 AM, Arnd Bergmann wrote:
> On Saturday 01 November 2014 15:41:38 Hauke Mehrtens wrote:
>>  };
>>  
>>  DT_MACHINE_START(BCM5301X, "BCM5301X")
>> -       .l2c_aux_val    = 0,
>> +       .l2c_aux_val    = L310_AUX_CTRL_CACHE_REPLACE_RR |
>> +                         L310_AUX_CTRL_DATA_PREFETCH |
>> +                         L310_AUX_CTRL_INSTR_PREFETCH |
>> +                         L310_AUX_CTRL_EARLY_BRESP,
> 
> Please put these into the dts file instead if the boot loader fails
> to set them correctly. We should not override them from source code
> for new platforms any more.
I checked again and I only need L310_AUX_CTRL_DATA_PREFETCH and
L310_AUX_CTRL_INSTR_PREFETCH, the others get activate by the existing
code. Should I make it possible to give l2c_aux_val through dt, or
should I create some boolean options for  L310_AUX_CTRL_DATA_PREFETCH
and  L310_AUX_CTRL_INSTR_PREFETCH?

Hauke
Florian Fainelli Nov. 3, 2014, 11:39 p.m. UTC | #3
On 11/03/2014 03:16 PM, Hauke Mehrtens wrote:
> On 11/03/2014 11:00 AM, Arnd Bergmann wrote:
>> On Saturday 01 November 2014 15:41:38 Hauke Mehrtens wrote:
>>>  };
>>>  
>>>  DT_MACHINE_START(BCM5301X, "BCM5301X")
>>> -       .l2c_aux_val    = 0,
>>> +       .l2c_aux_val    = L310_AUX_CTRL_CACHE_REPLACE_RR |
>>> +                         L310_AUX_CTRL_DATA_PREFETCH |
>>> +                         L310_AUX_CTRL_INSTR_PREFETCH |
>>> +                         L310_AUX_CTRL_EARLY_BRESP,
>>
>> Please put these into the dts file instead if the boot loader fails
>> to set them correctly. We should not override them from source code
>> for new platforms any more.
> I checked again and I only need L310_AUX_CTRL_DATA_PREFETCH and
> L310_AUX_CTRL_INSTR_PREFETCH, the others get activate by the existing
> code. Should I make it possible to give l2c_aux_val through dt, or
> should I create some boolean options for  L310_AUX_CTRL_DATA_PREFETCH
> and  L310_AUX_CTRL_INSTR_PREFETCH?

Last we talked about these prefetch bits with Russell, I think we kind
of agreed that they could probably be enabled by default

There is already a large number of properties defined in
Documentation/devicetree/bindings/arm/l2cc.txt, I suppose that having
prefetch properties could work too.
--
Florian
Arnd Bergmann Nov. 4, 2014, 10:22 a.m. UTC | #4
On Monday 03 November 2014 15:39:28 Florian Fainelli wrote:
> On 11/03/2014 03:16 PM, Hauke Mehrtens wrote:
> > On 11/03/2014 11:00 AM, Arnd Bergmann wrote:
> >> On Saturday 01 November 2014 15:41:38 Hauke Mehrtens wrote:
> >>>  };
> >>>  
> >>>  DT_MACHINE_START(BCM5301X, "BCM5301X")
> >>> -       .l2c_aux_val    = 0,
> >>> +       .l2c_aux_val    = L310_AUX_CTRL_CACHE_REPLACE_RR |
> >>> +                         L310_AUX_CTRL_DATA_PREFETCH |
> >>> +                         L310_AUX_CTRL_INSTR_PREFETCH |
> >>> +                         L310_AUX_CTRL_EARLY_BRESP,
> >>
> >> Please put these into the dts file instead if the boot loader fails
> >> to set them correctly. We should not override them from source code
> >> for new platforms any more.
> > I checked again and I only need L310_AUX_CTRL_DATA_PREFETCH and
> > L310_AUX_CTRL_INSTR_PREFETCH, the others get activate by the existing
> > code. Should I make it possible to give l2c_aux_val through dt, or
> > should I create some boolean options for  L310_AUX_CTRL_DATA_PREFETCH
> > and  L310_AUX_CTRL_INSTR_PREFETCH?

Certainly not the entire l2c_aux_val, what I meant is to use
boolean options for the bits you need. I was under the impression
that by now we had defined bindings for all the bits that are required
on any platform.

> Last we talked about these prefetch bits with Russell, I think we kind
> of agreed that they could probably be enabled by default
> 
> There is already a large number of properties defined in
> Documentation/devicetree/bindings/arm/l2cc.txt, I suppose that having
> prefetch properties could work too.

I can't really help with this if we don't have a binding. Proving that
it's always correct to enable these seems better than defining a
binding.

	Arnd
Hauke Mehrtens Nov. 8, 2014, 1:58 p.m. UTC | #5
On 11/04/2014 11:22 AM, Arnd Bergmann wrote:
> On Monday 03 November 2014 15:39:28 Florian Fainelli wrote:
>> On 11/03/2014 03:16 PM, Hauke Mehrtens wrote:
>>> On 11/03/2014 11:00 AM, Arnd Bergmann wrote:
>>>> On Saturday 01 November 2014 15:41:38 Hauke Mehrtens wrote:
>>>>>  };
>>>>>  
>>>>>  DT_MACHINE_START(BCM5301X, "BCM5301X")
>>>>> -       .l2c_aux_val    = 0,
>>>>> +       .l2c_aux_val    = L310_AUX_CTRL_CACHE_REPLACE_RR |
>>>>> +                         L310_AUX_CTRL_DATA_PREFETCH |
>>>>> +                         L310_AUX_CTRL_INSTR_PREFETCH |
>>>>> +                         L310_AUX_CTRL_EARLY_BRESP,
>>>>
>>>> Please put these into the dts file instead if the boot loader fails
>>>> to set them correctly. We should not override them from source code
>>>> for new platforms any more.
>>> I checked again and I only need L310_AUX_CTRL_DATA_PREFETCH and
>>> L310_AUX_CTRL_INSTR_PREFETCH, the others get activate by the existing
>>> code. Should I make it possible to give l2c_aux_val through dt, or
>>> should I create some boolean options for  L310_AUX_CTRL_DATA_PREFETCH
>>> and  L310_AUX_CTRL_INSTR_PREFETCH?
> 
> Certainly not the entire l2c_aux_val, what I meant is to use
> boolean options for the bits you need. I was under the impression
> that by now we had defined bindings for all the bits that are required
> on any platform.

Currently there are no bindings for L310_AUX_CTRL_DATA_PREFETCH and
L310_AUX_CTRL_INSTR_PREFETCH, but I can add them.

>> Last we talked about these prefetch bits with Russell, I think we kind
>> of agreed that they could probably be enabled by default

If they were changed to be enabled by default I do not have to add any
device tree stuff at all that would be the best solution for me. ;-)

>> There is already a large number of properties defined in
>> Documentation/devicetree/bindings/arm/l2cc.txt, I suppose that having
>> prefetch properties could work too.
> 
> I can't really help with this if we don't have a binding. Proving that
> it's always correct to enable these seems better than defining a
> binding.

When should they get activated automatically, every time we find a l2x0
compatible cache?

Hauke
diff mbox

Patch

diff --git a/arch/arm/mach-bcm/bcm_5301x.c b/arch/arm/mach-bcm/bcm_5301x.c
index e9bcbdb..8433a7c 100644
--- a/arch/arm/mach-bcm/bcm_5301x.c
+++ b/arch/arm/mach-bcm/bcm_5301x.c
@@ -49,7 +49,10 @@  static const char __initconst *bcm5301x_dt_compat[] = {
 };
 
 DT_MACHINE_START(BCM5301X, "BCM5301X")
-	.l2c_aux_val	= 0,
+	.l2c_aux_val	= L310_AUX_CTRL_CACHE_REPLACE_RR |
+			  L310_AUX_CTRL_DATA_PREFETCH |
+			  L310_AUX_CTRL_INSTR_PREFETCH |
+			  L310_AUX_CTRL_EARLY_BRESP,
 	.l2c_aux_mask	= ~0,
 	.init_early	= bcm5301x_init_early,
 	.dt_compat	= bcm5301x_dt_compat,