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
Signed-off-by: Wenyou Yang <[email protected]>
---
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
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
On 20/05/2013 03:06, Wenyou Yang :
> Signed-off-by: Wenyou Yang <[email protected]>
Acked-by: Nicolas Ferre <[email protected]>
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
On Mon, May 20, 2013 at 09:06:19AM +0800, Wenyou Yang wrote:
> Signed-off-by: Wenyou Yang <[email protected]>
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.
> -----Original Message-----
> From: Russell King - ARM Linux [mailto:[email protected]]
> Sent: 2013??5??22?? 8:15
> To: Yang, Wenyou
> Cc: [email protected]; [email protected];
> [email protected]; Ferre, Nicolas; [email protected]
> 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 <[email protected]>
>
> 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
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
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.
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.
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.
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".
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.
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.
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 <[email protected]>
---
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
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 <[email protected]>
Acked-by: Nicolas Ferre <[email protected]>
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
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 <[email protected]>
>
> Acked-by: Nicolas Ferre <[email protected]>
>
> 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.