[PATCH 1/2] sh: mach-ecovec24: modify memory area calculation

[PATCH 1/2] sh: mach-ecovec24: modify memory area calculation

[Kuninori Morimoto]

This operation expect to jump to cached area, but "mova" command will
get un-cached area in this case.
This patch subtract 0x20000000 from it to change un-cached area
to cached area

Signed-off-by: Kuninori Morimoto <morimoto.kuninori@renesas.com>
---
I think this patch is for 2.6.33

 arch/sh/include/mach-ecovec24/mach/romimage.h |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/sh/include/mach-ecovec24/mach/romimage.h b/arch/sh/include/mach-ecovec24/mach/romimage.h
index 1c8787e..115495d 100644
--- a/arch/sh/include/mach-ecovec24/mach/romimage.h
+++ b/arch/sh/include/mach-ecovec24/mach/romimage.h
@@ -11,10 +11,13 @@
 	icbi	@r0
 
 	/* jump to cached area */
-	mova	2f, r0
+	mov.l	2f, r1
+	mova	3f, r0
+	sub	r1, r0
 	jmp	@r0
 	nop
 
 	.align 2
 1 :	.long 0xa8000000
-2 :
+2 :	.long 0x20000000 /* 0xA0000000 - 0x80000000 */
+3 :
-- 
1.6.3.3

Re: [PATCH 1/2] sh: mach-ecovec24: modify memory area calculation

[Matt Fleming]

On Thu, Nov 26, 2009 at 07:34:30PM +0900, Kuninori Morimoto wrote:
> diff --git a/arch/sh/include/mach-ecovec24/mach/romimage.h b/arch/sh/include/mach-ecovec24/mach/romimage.h
> index 1c8787e..115495d 100644
> --- a/arch/sh/include/mach-ecovec24/mach/romimage.h
> +++ b/arch/sh/include/mach-ecovec24/mach/romimage.h
> @@ -11,10 +11,13 @@
>  	icbi	@r0
>  
>  	/* jump to cached area */
> -	mova	2f, r0
> +	mov.l	2f, r1
> +	mova	3f, r0
> +	sub	r1, r0
>  	jmp	@r0
>  	nop
>  
>  	.align 2
>  1 :	.long 0xa8000000
> -2 :
> +2 :	.long 0x20000000 /* 0xA0000000 - 0x80000000 */
> +3 :
> -- 
> 1.6.3.3
> 

I think both of these patches should be using the variable
"cached_to_uncached", rather than hard coding the value 0x20000000.

Re: [PATCH 1/2] sh: mach-ecovec24: modify memory area calculation

[Kuninori Morimoto]

Dear Matt

Thank you for checking patch !!

> I think both of these patches should be using the variable
> "cached_to_uncached", rather than hard coding the value 0x20000000.

I didn't know cached_to_uncached.
But I wonder can I use this variable here (romImage) ?
I think we can use macro here, but can not variable.

How about this ?
----------
 1 :	.long 0xa8000000
-2 :
+2 :	.long P2SEG - P1SEG /* cached_to_uncached */
+3 :
----------


Best regards
--
Kuninori Morimoto
 

Re: [PATCH 1/2] sh: mach-ecovec24: modify memory area calculation

[Matt Fleming]

On Thu, Nov 26, 2009 at 08:08:17PM +0900, Kuninori Morimoto wrote:
> 
> Dear Matt
> 
> Thank you for checking patch !!
> 

No problem.

> > I think both of these patches should be using the variable
> > "cached_to_uncached", rather than hard coding the value 0x20000000.
> 
> I didn't know cached_to_uncached.
> But I wonder can I use this variable here (romImage) ?
> I think we can use macro here, but can not variable.
> 

Ah, sorry, I forgot that this is a romImage thing. Yeah, including
system_32.h causes lots of assembler errors to appear so I'm guessing it
was never intended to be included by assembly code.

Re: [PATCH 1/2] sh: mach-ecovec24: modify memory area calculation

[Paul Mundt]

On Thu, Nov 26, 2009 at 11:46:29AM +0000, Matt Fleming wrote:
> On Thu, Nov 26, 2009 at 08:08:17PM +0900, Kuninori Morimoto wrote:
> > 
> > Dear Matt
> > 
> > Thank you for checking patch !!
> > 
> 
> No problem.
> 
> > > I think both of these patches should be using the variable
> > > "cached_to_uncached", rather than hard coding the value 0x20000000.
> > 
> > I didn't know cached_to_uncached.
> > But I wonder can I use this variable here (romImage) ?
> > I think we can use macro here, but can not variable.
> > 
> 
> Ah, sorry, I forgot that this is a romImage thing. Yeah, including
> system_32.h causes lots of assembler errors to appear so I'm guessing it
> was never intended to be included by assembly code.

It was never intended to be used in assembly code, no, but assembly code
was not intended to make cached<->uncached transitions on its own either.
Applying the 0x20000000 shift here works fine for legacy mode, but will
obviously completely blow up on 32-bit mode when legacy mappings don't
exist.

As it stands now this cached<->uncached thing is completely bogus and
doesn't deserve to live, there is no way to make this transition safely
from a fixed macro given the differing boot options available. EcoVec
presumably also supports a 32-bit boot mode that will be broken by this.

Basically any new code referencing P1SEG or P2SEG definitions is broken
by design and is grounds for an immediate NAK. The sooner we can kill
those macros off in their entirety the better.