Re: [U-Boot] [PATCH 2/2] arm: socfpga: enable data/inst prefetch and shared override in the L2

2015-10-15 Thread Marek Vasut
On Thursday, October 15, 2015 at 08:18:43 PM, Dinh Nguyen wrote:
> On Thu, Oct 15, 2015 at 1:08 PM, Marek Vasut  wrote:
> > On Thursday, October 15, 2015 at 05:04:38 PM, Dinh Nguyen wrote:
> > 
> > Hi!
> > 
> >> >>> diff --git a/arch/arm/include/asm/pl310.h
> >> >>> b/arch/arm/include/asm/pl310.h index 18b90b7..7a11405 100644
> >> >>> --- a/arch/arm/include/asm/pl310.h
> >> >>> +++ b/arch/arm/include/asm/pl310.h
> >> >>> @@ -17,6 +17,8 @@
> >> >>> 
> >> >>>  #define L2X0_CTRL_EN 1
> >> >>>  
> >> >>>  #define PL310_SHARED_ATT_OVERRIDE_ENABLE (1 << 22)
> >> >>> 
> >> >>> +#define PL310_AUX_CTRL_DATA_PREFETCH_MASK(1 << 28)
> >> >>> +#define PL310_AUX_CTRL_INST_PREFETCH_MASK(1 << 29)
> >> >> 
> >> >> These would be
> >> >> 
> >> >> arch/arm/include/asm/hardware/cache-l2x0.h:#define
> >> >> L310_PREFETCH_CTRL_DATA_PREFETCH BIT(28)
> >> >> arch/arm/include/asm/hardware/cache-l2x0.h:#define
> >> >> L310_PREFETCH_CTRL_INSTR_PREFETCHBIT(29)
> >> >> 
> >> >> ...in kernel. So maybe staying with L310_ prefix makes sense?
> >> >> Otherwise it looks ok.
> >> > 
> >> > Why is it L... in one and PL... in the other one ? What does the "PL"
> >> > prefix stand for anyway ?
> >> 
> >> As Pavel pointed out, it should be L310_x as this is how the Linux
> >> kernel is defining it. It's my mistake in the previous patch to change
> >> the define to PL310.
> >> 
> >> I'll respin this patch with L310_PREFETCH_CTRL_DATA_PREFETCH and
> >> L310_PREFETCH_CTRL_INSTR_PREFETCH.
> > 
> > Well that didn't answer either of my questions ;-)
> 
> It should have been just 'L" and not "PL" at all.

Why ? What does that "PL" stand for and why can we omit the P ?

I don't mean to grind you, don't get me wrong, I'm gonna pick the V2 tomorrow,
I just want to understand why it's enough to omit the P .

Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 2/2] arm: socfpga: enable data/inst prefetch and shared override in the L2

2015-10-15 Thread Dinh Nguyen
On Thu, Oct 15, 2015 at 1:08 PM, Marek Vasut  wrote:
> On Thursday, October 15, 2015 at 05:04:38 PM, Dinh Nguyen wrote:
>
> Hi!
>
>> >>> diff --git a/arch/arm/include/asm/pl310.h
>> >>> b/arch/arm/include/asm/pl310.h index 18b90b7..7a11405 100644
>> >>> --- a/arch/arm/include/asm/pl310.h
>> >>> +++ b/arch/arm/include/asm/pl310.h
>> >>> @@ -17,6 +17,8 @@
>> >>>
>> >>>  #define L2X0_CTRL_EN 1
>> >>>
>> >>>  #define PL310_SHARED_ATT_OVERRIDE_ENABLE (1 << 22)
>> >>>
>> >>> +#define PL310_AUX_CTRL_DATA_PREFETCH_MASK(1 << 28)
>> >>> +#define PL310_AUX_CTRL_INST_PREFETCH_MASK(1 << 29)
>> >>
>> >> These would be
>> >>
>> >> arch/arm/include/asm/hardware/cache-l2x0.h:#define
>> >> L310_PREFETCH_CTRL_DATA_PREFETCH BIT(28)
>> >> arch/arm/include/asm/hardware/cache-l2x0.h:#define
>> >> L310_PREFETCH_CTRL_INSTR_PREFETCHBIT(29)
>> >>
>> >> ...in kernel. So maybe staying with L310_ prefix makes sense?
>> >> Otherwise it looks ok.
>> >
>> > Why is it L... in one and PL... in the other one ? What does the "PL"
>> > prefix stand for anyway ?
>>
>> As Pavel pointed out, it should be L310_x as this is how the Linux
>> kernel is defining it. It's my mistake in the previous patch to change
>> the define to PL310.
>>
>> I'll respin this patch with L310_PREFETCH_CTRL_DATA_PREFETCH and
>> L310_PREFETCH_CTRL_INSTR_PREFETCH.
>
> Well that didn't answer either of my questions ;-)

It should have been just 'L" and not "PL" at all.

Dinh
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 2/2] arm: socfpga: enable data/inst prefetch and shared override in the L2

2015-10-15 Thread Marek Vasut
On Thursday, October 15, 2015 at 05:04:38 PM, Dinh Nguyen wrote:

Hi!

> >>> diff --git a/arch/arm/include/asm/pl310.h
> >>> b/arch/arm/include/asm/pl310.h index 18b90b7..7a11405 100644
> >>> --- a/arch/arm/include/asm/pl310.h
> >>> +++ b/arch/arm/include/asm/pl310.h
> >>> @@ -17,6 +17,8 @@
> >>> 
> >>>  #define L2X0_CTRL_EN 1
> >>>  
> >>>  #define PL310_SHARED_ATT_OVERRIDE_ENABLE (1 << 22)
> >>> 
> >>> +#define PL310_AUX_CTRL_DATA_PREFETCH_MASK(1 << 28)
> >>> +#define PL310_AUX_CTRL_INST_PREFETCH_MASK(1 << 29)
> >> 
> >> These would be
> >> 
> >> arch/arm/include/asm/hardware/cache-l2x0.h:#define
> >> L310_PREFETCH_CTRL_DATA_PREFETCH BIT(28)
> >> arch/arm/include/asm/hardware/cache-l2x0.h:#define
> >> L310_PREFETCH_CTRL_INSTR_PREFETCHBIT(29)
> >> 
> >> ...in kernel. So maybe staying with L310_ prefix makes sense?
> >> Otherwise it looks ok.
> > 
> > Why is it L... in one and PL... in the other one ? What does the "PL"
> > prefix stand for anyway ?
> 
> As Pavel pointed out, it should be L310_x as this is how the Linux
> kernel is defining it. It's my mistake in the previous patch to change
> the define to PL310.
> 
> I'll respin this patch with L310_PREFETCH_CTRL_DATA_PREFETCH and
> L310_PREFETCH_CTRL_INSTR_PREFETCH.

Well that didn't answer either of my questions ;-)

Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 2/2] arm: socfpga: enable data/inst prefetch and shared override in the L2

2015-10-15 Thread Dinh Nguyen
On 10/15/2015 09:32 AM, Marek Vasut wrote:
> On Wednesday, October 14, 2015 at 06:32:42 PM, Pavel Machek wrote:
>> On Mon 2015-10-12 09:59:57, dingu...@opensource.altera.com wrote:
>>> From: Dinh Nguyen 
>>>
>>> Update the L2 AUX CTRL settings for the SoCFPGA.
>>>
>>> Enabling D and I prefetch bits helps improve SDRAM performance on the
>>> platform.
>>>
>>> Also, we need to enable bit 22 of the L2. By not having bit 22 set in the
>>> PL310 Auxiliary Control register (shared attribute override enable) has
>>> the side effect of transforming Normal Shared Non-cacheable reads into
>>> Cacheable no-allocate reads.
>>>
>>> Coherent DMA buffers in Linux always have a Cacheable alias via the
>>> kernel linear mapping and the processor can speculatively load cache
>>> lines into the PL310 controller. With bit 22 cleared, Non-cacheable
>>> reads would unexpectedly hit such cache lines leading to buffer
>>> corruption.
>>>
>>> Signed-off-by: Dinh Nguyen 
>>> ---
>>>
>>>  arch/arm/include/asm/pl310.h |  2 ++
>>>  arch/arm/mach-socfpga/misc.c | 12 
>>>  2 files changed, 14 insertions(+)
>>>
>>> diff --git a/arch/arm/include/asm/pl310.h b/arch/arm/include/asm/pl310.h
>>> index 18b90b7..7a11405 100644
>>> --- a/arch/arm/include/asm/pl310.h
>>> +++ b/arch/arm/include/asm/pl310.h
>>> @@ -17,6 +17,8 @@
>>>
>>>  #define L2X0_CTRL_EN   1
>>>  
>>>  #define PL310_SHARED_ATT_OVERRIDE_ENABLE   (1 << 22)
>>>
>>> +#define PL310_AUX_CTRL_DATA_PREFETCH_MASK  (1 << 28)
>>> +#define PL310_AUX_CTRL_INST_PREFETCH_MASK  (1 << 29)
>>
>> These would be
>>
>> arch/arm/include/asm/hardware/cache-l2x0.h:#define
>> L310_PREFETCH_CTRL_DATA_PREFETCH BIT(28)
>> arch/arm/include/asm/hardware/cache-l2x0.h:#define
>> L310_PREFETCH_CTRL_INSTR_PREFETCHBIT(29)
>>
>> ...in kernel. So maybe staying with L310_ prefix makes sense?
>> Otherwise it looks ok.
> 
> Why is it L... in one and PL... in the other one ? What does the "PL"
> prefix stand for anyway ?
> 

As Pavel pointed out, it should be L310_x as this is how the Linux
kernel is defining it. It's my mistake in the previous patch to change
the define to PL310.

I'll respin this patch with L310_PREFETCH_CTRL_DATA_PREFETCH and
L310_PREFETCH_CTRL_INSTR_PREFETCH.


Dinh

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 2/2] arm: socfpga: enable data/inst prefetch and shared override in the L2

2015-10-15 Thread Marek Vasut
On Wednesday, October 14, 2015 at 06:32:42 PM, Pavel Machek wrote:
> On Mon 2015-10-12 09:59:57, dingu...@opensource.altera.com wrote:
> > From: Dinh Nguyen 
> > 
> > Update the L2 AUX CTRL settings for the SoCFPGA.
> > 
> > Enabling D and I prefetch bits helps improve SDRAM performance on the
> > platform.
> > 
> > Also, we need to enable bit 22 of the L2. By not having bit 22 set in the
> > PL310 Auxiliary Control register (shared attribute override enable) has
> > the side effect of transforming Normal Shared Non-cacheable reads into
> > Cacheable no-allocate reads.
> > 
> > Coherent DMA buffers in Linux always have a Cacheable alias via the
> > kernel linear mapping and the processor can speculatively load cache
> > lines into the PL310 controller. With bit 22 cleared, Non-cacheable
> > reads would unexpectedly hit such cache lines leading to buffer
> > corruption.
> > 
> > Signed-off-by: Dinh Nguyen 
> > ---
> > 
> >  arch/arm/include/asm/pl310.h |  2 ++
> >  arch/arm/mach-socfpga/misc.c | 12 
> >  2 files changed, 14 insertions(+)
> > 
> > diff --git a/arch/arm/include/asm/pl310.h b/arch/arm/include/asm/pl310.h
> > index 18b90b7..7a11405 100644
> > --- a/arch/arm/include/asm/pl310.h
> > +++ b/arch/arm/include/asm/pl310.h
> > @@ -17,6 +17,8 @@
> > 
> >  #define L2X0_CTRL_EN   1
> >  
> >  #define PL310_SHARED_ATT_OVERRIDE_ENABLE   (1 << 22)
> > 
> > +#define PL310_AUX_CTRL_DATA_PREFETCH_MASK  (1 << 28)
> > +#define PL310_AUX_CTRL_INST_PREFETCH_MASK  (1 << 29)
> 
> These would be
> 
> arch/arm/include/asm/hardware/cache-l2x0.h:#define
> L310_PREFETCH_CTRL_DATA_PREFETCH BIT(28)
> arch/arm/include/asm/hardware/cache-l2x0.h:#define
> L310_PREFETCH_CTRL_INSTR_PREFETCHBIT(29)
> 
> ...in kernel. So maybe staying with L310_ prefix makes sense?
> Otherwise it looks ok.

Why is it L... in one and PL... in the other one ? What does the "PL"
prefix stand for anyway ?

Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 2/2] arm: socfpga: enable data/inst prefetch and shared override in the L2

2015-10-14 Thread Pavel Machek
On Mon 2015-10-12 09:59:57, dingu...@opensource.altera.com wrote:
> From: Dinh Nguyen 
> 
> Update the L2 AUX CTRL settings for the SoCFPGA.
> 
> Enabling D and I prefetch bits helps improve SDRAM performance on the
> platform.
> 
> Also, we need to enable bit 22 of the L2. By not having bit 22 set in the
> PL310 Auxiliary Control register (shared attribute override enable) has the
> side effect of transforming Normal Shared Non-cacheable reads into Cacheable
> no-allocate reads.
> 
> Coherent DMA buffers in Linux always have a Cacheable alias via the
> kernel linear mapping and the processor can speculatively load cache
> lines into the PL310 controller. With bit 22 cleared, Non-cacheable
> reads would unexpectedly hit such cache lines leading to buffer
> corruption.
> 
> Signed-off-by: Dinh Nguyen 
> ---
>  arch/arm/include/asm/pl310.h |  2 ++
>  arch/arm/mach-socfpga/misc.c | 12 
>  2 files changed, 14 insertions(+)
> 
> diff --git a/arch/arm/include/asm/pl310.h b/arch/arm/include/asm/pl310.h
> index 18b90b7..7a11405 100644
> --- a/arch/arm/include/asm/pl310.h
> +++ b/arch/arm/include/asm/pl310.h
> @@ -17,6 +17,8 @@
>  #define L2X0_CTRL_EN 1
>  
>  #define PL310_SHARED_ATT_OVERRIDE_ENABLE (1 << 22)
> +#define PL310_AUX_CTRL_DATA_PREFETCH_MASK(1 << 28)
> +#define PL310_AUX_CTRL_INST_PREFETCH_MASK(1 << 29)

These would be

arch/arm/include/asm/hardware/cache-l2x0.h:#define
L310_PREFETCH_CTRL_DATA_PREFETCH BIT(28)
arch/arm/include/asm/hardware/cache-l2x0.h:#define
L310_PREFETCH_CTRL_INSTR_PREFETCHBIT(29)

...in kernel. So maybe staying with L310_ prefix makes sense?
Otherwise it looks ok.

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH 2/2] arm: socfpga: enable data/inst prefetch and shared override in the L2

2015-10-12 Thread dinguyen
From: Dinh Nguyen 

Update the L2 AUX CTRL settings for the SoCFPGA.

Enabling D and I prefetch bits helps improve SDRAM performance on the
platform.

Also, we need to enable bit 22 of the L2. By not having bit 22 set in the
PL310 Auxiliary Control register (shared attribute override enable) has the
side effect of transforming Normal Shared Non-cacheable reads into Cacheable
no-allocate reads.

Coherent DMA buffers in Linux always have a Cacheable alias via the
kernel linear mapping and the processor can speculatively load cache
lines into the PL310 controller. With bit 22 cleared, Non-cacheable
reads would unexpectedly hit such cache lines leading to buffer
corruption.

Signed-off-by: Dinh Nguyen 
---
 arch/arm/include/asm/pl310.h |  2 ++
 arch/arm/mach-socfpga/misc.c | 12 
 2 files changed, 14 insertions(+)

diff --git a/arch/arm/include/asm/pl310.h b/arch/arm/include/asm/pl310.h
index 18b90b7..7a11405 100644
--- a/arch/arm/include/asm/pl310.h
+++ b/arch/arm/include/asm/pl310.h
@@ -17,6 +17,8 @@
 #define L2X0_CTRL_EN   1
 
 #define PL310_SHARED_ATT_OVERRIDE_ENABLE   (1 << 22)
+#define PL310_AUX_CTRL_DATA_PREFETCH_MASK  (1 << 28)
+#define PL310_AUX_CTRL_INST_PREFETCH_MASK  (1 << 29)
 
 struct pl310_regs {
u32 pl310_cache_id;
diff --git a/arch/arm/mach-socfpga/misc.c b/arch/arm/mach-socfpga/misc.c
index 0940cc5..a134760 100644
--- a/arch/arm/mach-socfpga/misc.c
+++ b/arch/arm/mach-socfpga/misc.c
@@ -52,6 +52,18 @@ void enable_caches(void)
 #endif
 }
 
+void v7_outer_cache_enable(void)
+{
+   /* disable the L2 cache */
+   writel(0, &pl310->pl310_ctrl);
+
+   /* enable BRESP, instruction and data prefetch, full line of zeroes */
+   setbits_le32(&pl310->pl310_aux_ctrl,
+   PL310_AUX_CTRL_DATA_PREFETCH_MASK |
+   PL310_AUX_CTRL_INST_PREFETCH_MASK |
+   PL310_SHARED_ATT_OVERRIDE_ENABLE);
+}
+
 /*
  * DesignWare Ethernet initialization
  */
-- 
2.4.5

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot