Patchwork [U-Boot,01/14] arm, omap3: Define save_boot_params in lowlevel_init.S for SPL only

login
register
mail settings
Submitter Pali Rohár
Date Jan. 24, 2012, 2:27 p.m.
Message ID <1327415291-13260-2-git-send-email-pali.rohar@gmail.com>
Download mbox | patch
Permalink /patch/137559/
State Accepted
Delegated to: Tom Rini
Headers show

Comments

Pali Rohár - Jan. 24, 2012, 2:27 p.m.
* Hide function save_boot_params if CONFIG_SPL_BUILD is not defined (function do nothing)

* Same behaviour as in file arch/arm/cpu/armv7/omap4/lowlevel_init.S
* This allow to implement board specified function save_boot_params in board code

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
Changes since original version:
   - Fixed commit message

 arch/arm/cpu/armv7/omap3/lowlevel_init.S |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
Marek Vasut - Jan. 25, 2012, 6 p.m.
> * Hide function save_boot_params if CONFIG_SPL_BUILD is not defined
> (function do nothing)

What do you mean by this statement here ?
> 
> * Same behaviour as in file arch/arm/cpu/armv7/omap4/lowlevel_init.S

Eh?

> * This allow to implement board specified function save_boot_params in
> board code

I see ... do you want to write overridable function? Make it a weak alias?

M
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> ---
> Changes since original version:
>    - Fixed commit message
> 
>  arch/arm/cpu/armv7/omap3/lowlevel_init.S |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/cpu/armv7/omap3/lowlevel_init.S
> b/arch/arm/cpu/armv7/omap3/lowlevel_init.S index 2f6930b..c42c5dd 100644
> --- a/arch/arm/cpu/armv7/omap3/lowlevel_init.S
> +++ b/arch/arm/cpu/armv7/omap3/lowlevel_init.S
> @@ -35,15 +35,15 @@
>  _TEXT_BASE:
>  	.word	CONFIG_SYS_TEXT_BASE	/* sdram load addr from config.mk */
> 
> +#ifdef CONFIG_SPL_BUILD
>  .global save_boot_params
>  save_boot_params:
> -#ifdef CONFIG_SPL_BUILD
>  	ldr	r4, =omap3_boot_device
>  	ldr	r5, [r0, #0x4]
>  	and	r5, r5, #0xff
>  	str	r5, [r4]
> -#endif
>  	bx	lr

Wasn't this jump intentionally left out from the macro ifdef block?

> +#endif
> 
>  .global omap3_gp_romcode_call
>  omap3_gp_romcode_call:
Tom Rini - Jan. 25, 2012, 6:47 p.m.
On Tue, Jan 24, 2012 at 7:27 AM, Pali Rohár <pali.rohar@gmail.com> wrote:
> * Hide function save_boot_params if CONFIG_SPL_BUILD is not defined (function do nothing)

This is fine.

> * Same behaviour as in file arch/arm/cpu/armv7/omap4/lowlevel_init.S

This isn't quite right.  OMAP4/5 and AM33XX can re-use the same code
here to determine boot device/mode but it's not the same as 'OMAP3.'

> * This allow to implement board specified function save_boot_params in board code

This is fine.

> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>

So I will apply this, but modify the commit message slightly.
Tom Rini - Jan. 25, 2012, 6:50 p.m.
On Wed, Jan 25, 2012 at 11:00 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> * Hide function save_boot_params if CONFIG_SPL_BUILD is not defined
>> (function do nothing)
>
> What do you mean by this statement here ?
>>
>> * Same behaviour as in file arch/arm/cpu/armv7/omap4/lowlevel_init.S
>
> Eh?
>
>> * This allow to implement board specified function save_boot_params in
>> board code
>
> I see ... do you want to write overridable function? Make it a weak alias?

There is a weak one today, but we globally replace it for 'omap3'
today.  The N900 needs its own version however.
Pali Rohár - Feb. 28, 2012, 4:25 p.m.
On Tuesday 24 January 2012 15:27:58 Pali Rohár wrote:
> * Hide function save_boot_params if CONFIG_SPL_BUILD is not defined
> (function do nothing)
> 
> * Same behaviour as in file arch/arm/cpu/armv7/omap4/lowlevel_init.S
> * This allow to implement board specified function save_boot_params in board
> code
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> ---
> Changes since original version:
>    - Fixed commit message
> 
>  arch/arm/cpu/armv7/omap3/lowlevel_init.S |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/cpu/armv7/omap3/lowlevel_init.S
> b/arch/arm/cpu/armv7/omap3/lowlevel_init.S index 2f6930b..c42c5dd 100644
> --- a/arch/arm/cpu/armv7/omap3/lowlevel_init.S
> +++ b/arch/arm/cpu/armv7/omap3/lowlevel_init.S
> @@ -35,15 +35,15 @@
>  _TEXT_BASE:
>  	.word	CONFIG_SYS_TEXT_BASE	/* sdram load addr from config.mk */
> 
> +#ifdef CONFIG_SPL_BUILD
>  .global save_boot_params
>  save_boot_params:
> -#ifdef CONFIG_SPL_BUILD
>  	ldr	r4, =omap3_boot_device
>  	ldr	r5, [r0, #0x4]
>  	and	r5, r5, #0xff
>  	str	r5, [r4]
> -#endif
>  	bx	lr
> +#endif
> 
>  .global omap3_gp_romcode_call
>  omap3_gp_romcode_call:

Now I see that somebody commited this patch to u-boot:
http://git.denx.de/?p=u-
boot.git;a=commit;h=204705111ca10f52f62e1a02ba567d8fc33a6d1e

So I will not include it in new version.

Please, can you inform me when somebody commit my patches to git master?
Tom Rini - Feb. 28, 2012, 4:29 p.m.
On Tue, Feb 28, 2012 at 9:25 AM, Pali Rohár <pali.rohar@gmail.com> wrote:
> On Tuesday 24 January 2012 15:27:58 Pali Rohár wrote:
>> * Hide function save_boot_params if CONFIG_SPL_BUILD is not defined
>> (function do nothing)
>>
>> * Same behaviour as in file arch/arm/cpu/armv7/omap4/lowlevel_init.S
>> * This allow to implement board specified function save_boot_params in board
>> code
>>
>> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
>> ---
>> Changes since original version:
>>    - Fixed commit message
>>
>>  arch/arm/cpu/armv7/omap3/lowlevel_init.S |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/cpu/armv7/omap3/lowlevel_init.S
>> b/arch/arm/cpu/armv7/omap3/lowlevel_init.S index 2f6930b..c42c5dd 100644
>> --- a/arch/arm/cpu/armv7/omap3/lowlevel_init.S
>> +++ b/arch/arm/cpu/armv7/omap3/lowlevel_init.S
>> @@ -35,15 +35,15 @@
>>  _TEXT_BASE:
>>       .word   CONFIG_SYS_TEXT_BASE    /* sdram load addr from config.mk */
>>
>> +#ifdef CONFIG_SPL_BUILD
>>  .global save_boot_params
>>  save_boot_params:
>> -#ifdef CONFIG_SPL_BUILD
>>       ldr     r4, =omap3_boot_device
>>       ldr     r5, [r0, #0x4]
>>       and     r5, r5, #0xff
>>       str     r5, [r4]
>> -#endif
>>       bx      lr
>> +#endif
>>
>>  .global omap3_gp_romcode_call
>>  omap3_gp_romcode_call:
>
> Now I see that somebody commited this patch to u-boot:
> http://git.denx.de/?p=u-
> boot.git;a=commit;h=204705111ca10f52f62e1a02ba567d8fc33a6d1e
>
> So I will not include it in new version.
>
> Please, can you inform me when somebody commit my patches to git master?

I did: http://patchwork.ozlabs.org/patch/137559/ :)
Marek Vasut - Feb. 29, 2012, 9:37 p.m.
> On Tue, Feb 28, 2012 at 9:25 AM, Pali Rohár <pali.rohar@gmail.com> wrote:
> > On Tuesday 24 January 2012 15:27:58 Pali Rohár wrote:
> >> * Hide function save_boot_params if CONFIG_SPL_BUILD is not defined
> >> (function do nothing)
> >> 
> >> * Same behaviour as in file arch/arm/cpu/armv7/omap4/lowlevel_init.S
> >> * This allow to implement board specified function save_boot_params in
> >> board code
> >> 
> >> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> >> ---
> >> Changes since original version:
> >>    - Fixed commit message
> >> 
> >>  arch/arm/cpu/armv7/omap3/lowlevel_init.S |    4 ++--
> >>  1 files changed, 2 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/arch/arm/cpu/armv7/omap3/lowlevel_init.S
> >> b/arch/arm/cpu/armv7/omap3/lowlevel_init.S index 2f6930b..c42c5dd 100644
> >> --- a/arch/arm/cpu/armv7/omap3/lowlevel_init.S
> >> +++ b/arch/arm/cpu/armv7/omap3/lowlevel_init.S
> >> @@ -35,15 +35,15 @@
> >>  _TEXT_BASE:
> >>       .word   CONFIG_SYS_TEXT_BASE    /* sdram load addr from config.mk
> >> */
> >> 
> >> +#ifdef CONFIG_SPL_BUILD
> >>  .global save_boot_params
> >>  save_boot_params:
> >> -#ifdef CONFIG_SPL_BUILD
> >>       ldr     r4, =omap3_boot_device
> >>       ldr     r5, [r0, #0x4]
> >>       and     r5, r5, #0xff
> >>       str     r5, [r4]
> >> -#endif
> >>       bx      lr
> >> +#endif
> >> 
> >>  .global omap3_gp_romcode_call
> > 
> >>  omap3_gp_romcode_call:
> > Now I see that somebody commited this patch to u-boot:
> > http://git.denx.de/?p=u-
> > boot.git;a=commit;h=204705111ca10f52f62e1a02ba567d8fc33a6d1e
> > 
> > So I will not include it in new version.
> > 
> > Please, can you inform me when somebody commit my patches to git master?
> 
> I did: http://patchwork.ozlabs.org/patch/137559/ :)

Do we really want this hack-around? Maybe generating such bootargs by uboot in 
the first place (or the ability to actually source bootargs and adjust env 
accordingly) won't be too bad of a solution?

M
Tom Rini - March 5, 2012, 6:24 p.m.
On Wed, Feb 29, 2012 at 10:37:06PM +0100, Marek Vasut wrote:
> > On Tue, Feb 28, 2012 at 9:25 AM, Pali Roh?r <pali.rohar@gmail.com> wrote:
> > > On Tuesday 24 January 2012 15:27:58 Pali Roh?r wrote:
> > >> * Hide function save_boot_params if CONFIG_SPL_BUILD is not defined
> > >> (function do nothing)
> > >> 
> > >> * Same behaviour as in file arch/arm/cpu/armv7/omap4/lowlevel_init.S
> > >> * This allow to implement board specified function save_boot_params in
> > >> board code
> > >> 
> > >> Signed-off-by: Pali Roh?r <pali.rohar@gmail.com>
> > >> ---
> > >> Changes since original version:
> > >>    - Fixed commit message
> > >> 
> > >>  arch/arm/cpu/armv7/omap3/lowlevel_init.S |    4 ++--
> > >>  1 files changed, 2 insertions(+), 2 deletions(-)
> > >> 
> > >> diff --git a/arch/arm/cpu/armv7/omap3/lowlevel_init.S
> > >> b/arch/arm/cpu/armv7/omap3/lowlevel_init.S index 2f6930b..c42c5dd 100644
> > >> --- a/arch/arm/cpu/armv7/omap3/lowlevel_init.S
> > >> +++ b/arch/arm/cpu/armv7/omap3/lowlevel_init.S
> > >> @@ -35,15 +35,15 @@
> > >>  _TEXT_BASE:
> > >>       .word   CONFIG_SYS_TEXT_BASE    /* sdram load addr from config.mk
> > >> */
> > >> 
> > >> +#ifdef CONFIG_SPL_BUILD
> > >>  .global save_boot_params
> > >>  save_boot_params:
> > >> -#ifdef CONFIG_SPL_BUILD
> > >>       ldr     r4, =omap3_boot_device
> > >>       ldr     r5, [r0, #0x4]
> > >>       and     r5, r5, #0xff
> > >>       str     r5, [r4]
> > >> -#endif
> > >>       bx      lr
> > >> +#endif
> > >> 
> > >>  .global omap3_gp_romcode_call
> > > 
> > >>  omap3_gp_romcode_call:
> > > Now I see that somebody commited this patch to u-boot:
> > > http://git.denx.de/?p=u-
> > > boot.git;a=commit;h=204705111ca10f52f62e1a02ba567d8fc33a6d1e
> > > 
> > > So I will not include it in new version.
> > > 
> > > Please, can you inform me when somebody commit my patches to git master?
> > 
> > I did: http://patchwork.ozlabs.org/patch/137559/ :)
> 
> Do we really want this hack-around? Maybe generating such bootargs by uboot in 
> the first place (or the ability to actually source bootargs and adjust env 
> accordingly) won't be too bad of a solution?

IMHO, yes.  There is value in being able to use u-boot as a useful shim
when we don't control 100% of the sequence.
Marek Vasut - March 5, 2012, 6:49 p.m.
Dear Tom Rini,

> On Wed, Feb 29, 2012 at 10:37:06PM +0100, Marek Vasut wrote:
> > > On Tue, Feb 28, 2012 at 9:25 AM, Pali Roh?r <pali.rohar@gmail.com> wrote:
> > > > On Tuesday 24 January 2012 15:27:58 Pali Roh?r wrote:
> > > >> * Hide function save_boot_params if CONFIG_SPL_BUILD is not defined
> > > >> (function do nothing)
> > > >> 
> > > >> * Same behaviour as in file arch/arm/cpu/armv7/omap4/lowlevel_init.S
> > > >> * This allow to implement board specified function save_boot_params
> > > >> in board code
> > > >> 
> > > >> Signed-off-by: Pali Roh?r <pali.rohar@gmail.com>
> > > >> ---
> > > >> 
> > > >> Changes since original version:
> > > >>    - Fixed commit message
> > > >>  
> > > >>  arch/arm/cpu/armv7/omap3/lowlevel_init.S |    4 ++--
> > > >>  1 files changed, 2 insertions(+), 2 deletions(-)
> > > >> 
> > > >> diff --git a/arch/arm/cpu/armv7/omap3/lowlevel_init.S
> > > >> b/arch/arm/cpu/armv7/omap3/lowlevel_init.S index 2f6930b..c42c5dd
> > > >> 100644 --- a/arch/arm/cpu/armv7/omap3/lowlevel_init.S
> > > >> +++ b/arch/arm/cpu/armv7/omap3/lowlevel_init.S
> > > >> @@ -35,15 +35,15 @@
> > > >> 
> > > >>  _TEXT_BASE:
> > > >>       .word   CONFIG_SYS_TEXT_BASE    /* sdram load addr from
> > > >>       config.mk
> > > >> 
> > > >> */
> > > >> 
> > > >> +#ifdef CONFIG_SPL_BUILD
> > > >> 
> > > >>  .global save_boot_params
> > > >> 
> > > >>  save_boot_params:
> > > >> -#ifdef CONFIG_SPL_BUILD
> > > >> 
> > > >>       ldr     r4, =omap3_boot_device
> > > >>       ldr     r5, [r0, #0x4]
> > > >>       and     r5, r5, #0xff
> > > >>       str     r5, [r4]
> > > >> 
> > > >> -#endif
> > > >> 
> > > >>       bx      lr
> > > >> 
> > > >> +#endif
> > > >> 
> > > >>  .global omap3_gp_romcode_call
> > > > 
> > > >>  omap3_gp_romcode_call:
> > > > Now I see that somebody commited this patch to u-boot:
> > > > http://git.denx.de/?p=u-
> > > > boot.git;a=commit;h=204705111ca10f52f62e1a02ba567d8fc33a6d1e
> > > > 
> > > > So I will not include it in new version.
> > > > 
> > > > Please, can you inform me when somebody commit my patches to git
> > > > master?
> > > 
> > > I did: http://patchwork.ozlabs.org/patch/137559/ :)
> > 
> > Do we really want this hack-around? Maybe generating such bootargs by
> > uboot in the first place (or the ability to actually source bootargs and
> > adjust env accordingly) won't be too bad of a solution?
> 
> IMHO, yes.  There is value in being able to use u-boot as a useful shim
> when we don't control 100% of the sequence.

Well ... I won't argue, it's your start.S that's gonne be bloated ;-)

Best regards,
Marek Vasut

Patch

diff --git a/arch/arm/cpu/armv7/omap3/lowlevel_init.S b/arch/arm/cpu/armv7/omap3/lowlevel_init.S
index 2f6930b..c42c5dd 100644
--- a/arch/arm/cpu/armv7/omap3/lowlevel_init.S
+++ b/arch/arm/cpu/armv7/omap3/lowlevel_init.S
@@ -35,15 +35,15 @@ 
 _TEXT_BASE:
 	.word	CONFIG_SYS_TEXT_BASE	/* sdram load addr from config.mk */
 
+#ifdef CONFIG_SPL_BUILD
 .global save_boot_params
 save_boot_params:
-#ifdef CONFIG_SPL_BUILD
 	ldr	r4, =omap3_boot_device
 	ldr	r5, [r0, #0x4]
 	and	r5, r5, #0xff
 	str	r5, [r4]
-#endif
 	bx	lr
+#endif
 
 .global omap3_gp_romcode_call
 omap3_gp_romcode_call: