| Message ID | 1369011979-21354-1-git-send-email-wenyou.yang@atmel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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); >
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.
> -----Original Message----- > From: Russell King - ARM Linux [mailto:linux@arm.linux.org.uk] > Sent: 2013?5?22? 8:15 > To: Yang, Wenyou > Cc: linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; > plagnioj@jcrosoft.com; Ferre, Nicolas; linux@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
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 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 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: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.
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);
Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com> --- arch/arm/mach-at91/setup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)