[PATCH] Change internal SRAM memory type to "MT_MEMORY_SO"

[PATCH] Change internal SRAM memory type to "MT_MEMORY_SO"

[Wenyou Yang]

Hi Russell,

This patch is to change the internal SRAM memory type to "MT_MEMORY_SO", the right type.

It tested on sama5d3xek boards.

It based on v3.10-rc1.

Best Regards,
Wenyou Yang

Wenyou Yang (1):
  ARM: at91: Fix: Change internal SRAM memory type to "MT_MEMORY_SO"

 arch/arm/mach-at91/setup.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
1.7.9.5

[PATCH] ARM: at91: Fix: Change internal SRAM memory type to "MT_MEMORY_SO"

[Wenyou Yang]

Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
---
 arch/arm/mach-at91/setup.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-at91/setup.c b/arch/arm/mach-at91/setup.c
index e2f4bdd..d68be6c 100644
--- a/arch/arm/mach-at91/setup.c
+++ b/arch/arm/mach-at91/setup.c
@@ -80,7 +80,7 @@ void __init at91_init_sram(int bank, unsigned long base, unsigned int length)
 
 	desc->pfn = __phys_to_pfn(base);
 	desc->length = length;
-	desc->type = MT_DEVICE;
+	desc->type = MT_MEMORY_SO;
 
 	pr_info("AT91: sram at 0x%lx of 0x%x mapped at 0x%lx\n",
 		base, length, desc->virtual);
-- 
1.7.9.5

[PATCH] Change internal SRAM memory type to "MT_MEMORY_SO"

[Nicolas Ferre]

On 20/05/2013 03:05, Wenyou Yang :
> Hi Russell,

Wenyou, In fact this patch is targeted for myself and not Russell. It 
will join mainline using arm-soc git tree which is managed by Arnd and Olof.

> This patch is to change the internal SRAM memory type to "MT_MEMORY_SO", the right type.
>
> It tested on sama5d3xek boards.
>
> It based on v3.10-rc1.
>
> Best Regards,
> Wenyou Yang
>
> Wenyou Yang (1):
>    ARM: at91: Fix: Change internal SRAM memory type to "MT_MEMORY_SO"
>
>   arch/arm/mach-at91/setup.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Bye,
-- 
Nicolas Ferre

[PATCH] ARM: at91: Fix: Change internal SRAM memory type to "MT_MEMORY_SO"

[Nicolas Ferre]

On 20/05/2013 03:06, Wenyou Yang :
> Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

And stacked on at91-3.10-fixes.

Best regards,

> ---
>   arch/arm/mach-at91/setup.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-at91/setup.c b/arch/arm/mach-at91/setup.c
> index e2f4bdd..d68be6c 100644
> --- a/arch/arm/mach-at91/setup.c
> +++ b/arch/arm/mach-at91/setup.c
> @@ -80,7 +80,7 @@ void __init at91_init_sram(int bank, unsigned long base, unsigned int length)
>
>   	desc->pfn = __phys_to_pfn(base);
>   	desc->length = length;
> -	desc->type = MT_DEVICE;
> +	desc->type = MT_MEMORY_SO;
>
>   	pr_info("AT91: sram at 0x%lx of 0x%x mapped at 0x%lx\n",
>   		base, length, desc->virtual);
>


-- 
Nicolas Ferre

[PATCH] ARM: at91: Fix: Change internal SRAM memory type to "MT_MEMORY_SO"

[Russell King - ARM Linux]

On Mon, May 20, 2013 at 09:06:19AM +0800, Wenyou Yang wrote:
> Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>

This needs more of a description.  Also, for a single patch, it's silly
to send two mails, the first being a cover which has a little more
information in it about the patch than the patch itself.

You need to explain _why_ you're making this change.  What I want to see
is that you've thought about the implications of this - particularly that
you know that strongly ordered memory does *not* imply any ordering with
any other memory types.

In other words, I want to know that this change is not a bodge but there's
a real reason behind it.

[PATCH] ARM: at91: Fix: Change internal SRAM memory type to "MT_MEMORY_SO"

[Yang, Wenyou]

> -----Original Message-----
> From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
> Sent: 2013?5?22? 8:15
> To: Yang, Wenyou
> Cc: linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org;
> plagnioj at jcrosoft.com; Ferre, Nicolas; linux at maxim.org.za
> Subject: Re: [PATCH] ARM: at91: Fix: Change internal SRAM memory type to
> "MT_MEMORY_SO"
> 
> On Mon, May 20, 2013 at 09:06:19AM +0800, Wenyou Yang wrote:
> > Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
> 
> This needs more of a description.  Also, for a single patch, it's silly
> to send two mails, the first being a cover which has a little more
> information in it about the patch than the patch itself.
> 
> You need to explain _why_ you're making this change.  What I want to see
> is that you've thought about the implications of this - particularly that
> you know that strongly ordered memory does *not* imply any ordering with
> any other memory types.
> 
> In other words, I want to know that this change is not a bodge but there's
> a real reason behind it.
The story is: for sama5d3x with Cortex-A5 core, if not so, when copying code snippet to the internal SRAM, then jump to run this code, but fail to run.
So, refer to other code(such as omap4), do such change. I also test it on at91sam9 with 926ej-s core.
As what you said, I am digging it. Thank you very much.

Best Regards,
Wenyou Yang

[PATCH] ARM: at91: Fix: Change internal SRAM memory type to "MT_MEMORY_SO"

[Russell King - ARM Linux]

On Fri, May 24, 2013 at 07:11:04AM +0000, Yang, Wenyou wrote:
> The story is: for sama5d3x with Cortex-A5 core, if not so, when copying
> code snippet to the internal SRAM, then jump to run this code, but fail
> to run.

And that is where your mistake is - you forgot that you're working with
a CPU with harvard caches which will require some cache maintanence
between copying the code and executing it.

You want to look at flush_icache_range() rather than making this memory
strongly ordered.

[PATCH] ARM: at91: Fix: Change internal SRAM memory type to "MT_MEMORY_SO"

[Jean-Christophe PLAGNIOL-VILLARD]

On 12:20 Fri 24 May     , Russell King - ARM Linux wrote:
> On Fri, May 24, 2013 at 07:11:04AM +0000, Yang, Wenyou wrote:
> > The story is: for sama5d3x with Cortex-A5 core, if not so, when copying
> > code snippet to the internal SRAM, then jump to run this code, but fail
> > to run.
> 
> And that is where your mistake is - you forgot that you're working with
> a CPU with harvard caches which will require some cache maintanence
> between copying the code and executing it.
> 
> You want to look at flush_icache_range() rather than making this memory
> strongly ordered.

I understand your point but today we map a SRAM as MT_DEVICE

I do not think it's the best

Best Regards,
J.

[PATCH] ARM: at91: Fix: Change internal SRAM memory type to "MT_MEMORY_SO"

[Russell King - ARM Linux]

On Fri, May 24, 2013 at 04:03:22PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 12:20 Fri 24 May     , Russell King - ARM Linux wrote:
> > On Fri, May 24, 2013 at 07:11:04AM +0000, Yang, Wenyou wrote:
> > > The story is: for sama5d3x with Cortex-A5 core, if not so, when copying
> > > code snippet to the internal SRAM, then jump to run this code, but fail
> > > to run.
> > 
> > And that is where your mistake is - you forgot that you're working with
> > a CPU with harvard caches which will require some cache maintanence
> > between copying the code and executing it.
> > 
> > You want to look at flush_icache_range() rather than making this memory
> > strongly ordered.
> 
> I understand your point but today we map a SRAM as MT_DEVICE

If you map SRAM as MT_DEVICE then you won't be able to execute code from
it.  It needs to be a normal memory mapping.

[PATCH] ARM: at91: Fix: Change internal SRAM memory type to "MT_MEMORY_SO"

[Jean-Christophe PLAGNIOL-VILLARD]

On 17:40 Fri 24 May     , Russell King - ARM Linux wrote:
> On Fri, May 24, 2013 at 04:03:22PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 12:20 Fri 24 May     , Russell King - ARM Linux wrote:
> > > On Fri, May 24, 2013 at 07:11:04AM +0000, Yang, Wenyou wrote:
> > > > The story is: for sama5d3x with Cortex-A5 core, if not so, when copying
> > > > code snippet to the internal SRAM, then jump to run this code, but fail
> > > > to run.
> > > 
> > > And that is where your mistake is - you forgot that you're working with
> > > a CPU with harvard caches which will require some cache maintanence
> > > between copying the code and executing it.
> > > 
> > > You want to look at flush_icache_range() rather than making this memory
> > > strongly ordered.
> > 
> > I understand your point but today we map a SRAM as MT_DEVICE
> 
> If you map SRAM as MT_DEVICE then you won't be able to execute code from
> it.  It needs to be a normal memory mapping.

Yeah that a bug on AT91, by luck it work on armv3/v4 with MT_DEVICE, I should
have spot it earlier when cleanning the at91 but did not

That's why Yang change the SRAM mapping as MT_MEMORY_SO

I agree the commit message need to be re-done

Best Regards,
J.

[PATCH] ARM: at91: Fix: Change internal SRAM memory type to "MT_MEMORY_SO"

[Russell King - ARM Linux]

On Fri, May 24, 2013 at 06:52:54PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 17:40 Fri 24 May     , Russell King - ARM Linux wrote:
> > On Fri, May 24, 2013 at 04:03:22PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > > On 12:20 Fri 24 May     , Russell King - ARM Linux wrote:
> > > > On Fri, May 24, 2013 at 07:11:04AM +0000, Yang, Wenyou wrote:
> > > > > The story is: for sama5d3x with Cortex-A5 core, if not so, when copying
> > > > > code snippet to the internal SRAM, then jump to run this code, but fail
> > > > > to run.
> > > > 
> > > > And that is where your mistake is - you forgot that you're working with
> > > > a CPU with harvard caches which will require some cache maintanence
> > > > between copying the code and executing it.
> > > > 
> > > > You want to look at flush_icache_range() rather than making this memory
> > > > strongly ordered.
> > > 
> > > I understand your point but today we map a SRAM as MT_DEVICE
> > 
> > If you map SRAM as MT_DEVICE then you won't be able to execute code from
> > it.  It needs to be a normal memory mapping.
> 
> Yeah that a bug on AT91, by luck it work on armv3/v4 with MT_DEVICE, I should
> have spot it earlier when cleanning the at91 but did not
> 
> That's why Yang change the SRAM mapping as MT_MEMORY_SO

I said "normal memory".  Strongly ordered is not "normal memory".

[PATCH] ARM: at91: Fix: Change internal SRAM memory type to "MT_MEMORY_SO"

[Jean-Christophe PLAGNIOL-VILLARD]

On 17:59 Fri 24 May     , Russell King - ARM Linux wrote:
> On Fri, May 24, 2013 at 06:52:54PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 17:40 Fri 24 May     , Russell King - ARM Linux wrote:
> > > On Fri, May 24, 2013 at 04:03:22PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > > > On 12:20 Fri 24 May     , Russell King - ARM Linux wrote:
> > > > > On Fri, May 24, 2013 at 07:11:04AM +0000, Yang, Wenyou wrote:
> > > > > > The story is: for sama5d3x with Cortex-A5 core, if not so, when copying
> > > > > > code snippet to the internal SRAM, then jump to run this code, but fail
> > > > > > to run.
> > > > > 
> > > > > And that is where your mistake is - you forgot that you're working with
> > > > > a CPU with harvard caches which will require some cache maintanence
> > > > > between copying the code and executing it.
> > > > > 
> > > > > You want to look at flush_icache_range() rather than making this memory
> > > > > strongly ordered.
> > > > 
> > > > I understand your point but today we map a SRAM as MT_DEVICE
> > > 
> > > If you map SRAM as MT_DEVICE then you won't be able to execute code from
> > > it.  It needs to be a normal memory mapping.
> > 
> > Yeah that a bug on AT91, by luck it work on armv3/v4 with MT_DEVICE, I should
> > have spot it earlier when cleanning the at91 but did not
> > 
> > That's why Yang change the SRAM mapping as MT_MEMORY_SO
> 
> I said "normal memory".  Strongly ordered is not "normal memory".

yeah btw why omap map their SRAM as strongly orderered?

Best Regards,
J.

[PATCH] Change the internal SRAM memory type MT_MEMORY_NONCACHED

[Wenyou Yang]

Because MT_DEVICE is not executable in armv7, we change
the internal SRAM memory type to MT_MEMORY_NONCACHED.
As it seems that caching this internal SRAM memory is not necessary,
we chose the this memory type.

Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
---
 arch/arm/mach-at91/setup.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-at91/setup.c b/arch/arm/mach-at91/setup.c
index a33d4b5..98d55ef 100644
--- a/arch/arm/mach-at91/setup.c
+++ b/arch/arm/mach-at91/setup.c
@@ -84,7 +84,7 @@ void __init at91_init_sram(int bank, unsigned long base, unsigned int length)
 
 	desc->pfn = __phys_to_pfn(base);
 	desc->length = length;
-	desc->type = MT_DEVICE;
+	desc->type = MT_MEMORY_NONCACHED;
 
 	pr_info("AT91: sram at 0x%lx of 0x%x mapped at 0x%lx\n",
 		base, length, desc->virtual);
-- 
1.7.9.5

[PATCH] Change the internal SRAM memory type MT_MEMORY_NONCACHED

[Nicolas Ferre]

On 08/06/2013 04:17, Wenyou Yang :
> Because MT_DEVICE is not executable in armv7, we change
> the internal SRAM memory type to MT_MEMORY_NONCACHED.
> As it seems that caching this internal SRAM memory is not necessary,
> we chose the this memory type.
>
> Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

But Russell, can we have your advice on this as you made us re-think 
about that during previous patch discussion?

Bye,

> ---
>   arch/arm/mach-at91/setup.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-at91/setup.c b/arch/arm/mach-at91/setup.c
> index a33d4b5..98d55ef 100644
> --- a/arch/arm/mach-at91/setup.c
> +++ b/arch/arm/mach-at91/setup.c
> @@ -84,7 +84,7 @@ void __init at91_init_sram(int bank, unsigned long base, unsigned int length)
>
>   	desc->pfn = __phys_to_pfn(base);
>   	desc->length = length;
> -	desc->type = MT_DEVICE;
> +	desc->type = MT_MEMORY_NONCACHED;
>
>   	pr_info("AT91: sram at 0x%lx of 0x%x mapped at 0x%lx\n",
>   		base, length, desc->virtual);
>


-- 
Nicolas Ferre

[PATCH] Change the internal SRAM memory type MT_MEMORY_NONCACHED

[Russell King - ARM Linux]

On Thu, Jun 13, 2013 at 03:19:58PM +0200, Nicolas Ferre wrote:
> On 08/06/2013 04:17, Wenyou Yang :
>> Because MT_DEVICE is not executable in armv7, we change
>> the internal SRAM memory type to MT_MEMORY_NONCACHED.
>> As it seems that caching this internal SRAM memory is not necessary,
>> we chose the this memory type.
>>
>> Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
>
> Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>
> But Russell, can we have your advice on this as you made us re-think  
> about that during previous patch discussion?

I think this is fine - using MT_MEMORY_NONCACHED gets you a memory-like
mapping instead of a strongly ordered or device mapping, but without
caching issues, which is what you want here.