| Message ID | 1510299477-11835-1-git-send-email-kever.yang@rock-chips.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Philipp Tomsich |
| Headers | show |
| Series | [U-Boot] rockchip: update boot0 hook | expand |
> armv7 SPL: TAG(overwrite 'b 1f')+'b reset' + ARM_VECTORS > armv7 U-Boot: ARM_VECTORS > armv8 SPL: TAG(overwrite 'b 1f')+'b reset' + Reserved_iram(rk3399) > armv8 U-Boot: 'b reset' > > Signed-off-by: Kever Yang <kever.yang@rock-chips.com> > --- > > arch/arm/include/asm/arch-rockchip/boot0.h | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) > Acked-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
On Fri, 10 Nov 2017, Kever Yang wrote: > armv7 SPL: TAG(overwrite 'b 1f')+'b reset' + ARM_VECTORS > armv7 U-Boot: ARM_VECTORS > armv8 SPL: TAG(overwrite 'b 1f')+'b reset' + Reserved_iram(rk3399) > armv8 U-Boot: 'b reset' Could you make this a bit more explicit/easier to read? It took me a while to understand that the key difference is that ARM_VECTORS is ARMv7 only and ARMv8 should have a 'b reset'. > > Signed-off-by: Kever Yang <kever.yang@rock-chips.com> > Acked-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com> > --- > > arch/arm/include/asm/arch-rockchip/boot0.h | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) > > diff --git a/arch/arm/include/asm/arch-rockchip/boot0.h b/arch/arm/include/asm/arch-rockchip/boot0.h > index af3a733..65b4213 100644 > --- a/arch/arm/include/asm/arch-rockchip/boot0.h > +++ b/arch/arm/include/asm/arch-rockchip/boot0.h > @@ -26,7 +26,6 @@ > */ > b 1f /* if overwritten, entry-address is at the next word */ > 1: > -#endif This should not be necessary, as ROCKCHIP_EARLYRETURN_TO_BROM is defined for SPL_... and TPL_... only. > #if CONFIG_IS_ENABLED(ROCKCHIP_EARLYRETURN_TO_BROM) > adr r3, entry_counter > ldr r0, [r3] > @@ -40,6 +39,15 @@ entry_counter: > .word 0 > #endif > b reset > + > +#if defined(CONFIG_ROCKCHIP_RK3399) > + .space CONFIG_ROCKCHIP_SPL_RESERVE_IRAM /* space for the ATF data */ > +#endif Could we make this "defined(CONFIG_SPL_BUILD) && CONFIG_ROCKCHIP_SPL_RESERVE_IRAM > 0" so there's no warnings raised if "CONFIG_ROCKCHIP_SPL_RESERVE_IRAM == 0" (with our changes to the ATF-loading, we don't override the SRAM on the RK3399-Q7 ... and with this set to 0, I see warning)? > + > +#elif defined(CONFIG_ARM64) /* U-Boot for arm64 */ Can we do this without an #elif? Having just one level of #if #endif to track makes this much easier to read... > + b reset > +#endif > + > #if !defined(CONFIG_ARM64) > /* > * For armv7, the addr '_start' will used as vector start address > @@ -49,7 +57,3 @@ entry_counter: > _start: > ARM_VECTORS > #endif > - > -#if defined(CONFIG_ROCKCHIP_RK3399) && defined(CONFIG_SPL_BUILD) > - .space CONFIG_ROCKCHIP_SPL_RESERVE_IRAM /* space for the ATF data */ > -#endif >
Hi Kever, On 10 November 2017 at 00:37, Kever Yang <kever.yang@rock-chips.com> wrote: > armv7 SPL: TAG(overwrite 'b 1f')+'b reset' + ARM_VECTORS > armv7 U-Boot: ARM_VECTORS > armv8 SPL: TAG(overwrite 'b 1f')+'b reset' + Reserved_iram(rk3399) > armv8 U-Boot: 'b reset' Can you explain this in English as well? The motivation for this patch seems to be missing. > > Signed-off-by: Kever Yang <kever.yang@rock-chips.com> > --- > > arch/arm/include/asm/arch-rockchip/boot0.h | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) Regards, Simon
diff --git a/arch/arm/include/asm/arch-rockchip/boot0.h b/arch/arm/include/asm/arch-rockchip/boot0.h index af3a733..65b4213 100644 --- a/arch/arm/include/asm/arch-rockchip/boot0.h +++ b/arch/arm/include/asm/arch-rockchip/boot0.h @@ -26,7 +26,6 @@ */ b 1f /* if overwritten, entry-address is at the next word */ 1: -#endif #if CONFIG_IS_ENABLED(ROCKCHIP_EARLYRETURN_TO_BROM) adr r3, entry_counter ldr r0, [r3] @@ -40,6 +39,15 @@ entry_counter: .word 0 #endif b reset + +#if defined(CONFIG_ROCKCHIP_RK3399) + .space CONFIG_ROCKCHIP_SPL_RESERVE_IRAM /* space for the ATF data */ +#endif + +#elif defined(CONFIG_ARM64) /* U-Boot for arm64 */ + b reset +#endif + #if !defined(CONFIG_ARM64) /* * For armv7, the addr '_start' will used as vector start address @@ -49,7 +57,3 @@ entry_counter: _start: ARM_VECTORS #endif - -#if defined(CONFIG_ROCKCHIP_RK3399) && defined(CONFIG_SPL_BUILD) - .space CONFIG_ROCKCHIP_SPL_RESERVE_IRAM /* space for the ATF data */ -#endif
armv7 SPL: TAG(overwrite 'b 1f')+'b reset' + ARM_VECTORS armv7 U-Boot: ARM_VECTORS armv8 SPL: TAG(overwrite 'b 1f')+'b reset' + Reserved_iram(rk3399) armv8 U-Boot: 'b reset' Signed-off-by: Kever Yang <kever.yang@rock-chips.com> --- arch/arm/include/asm/arch-rockchip/boot0.h | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)