| Message ID | 1507645279-25188-3-git-send-email-philipp.tomsich@theobroma-systems.com |
|---|---|
| State | Accepted |
| Delegated to: | Philipp Tomsich |
| Headers | show |
| Series | rockchip: back-to-bootrom: replace assembly-implementation with C-code | expand |
> From: Kever Yang <kever.yang@rock-chips.com> > > The '_start' is using as vector table base address, and will write > to VBAR register, so it needs to be aligned to 0x20 for armv7. > > Signed-off-by: Kever Yang <kever.yang@rock-chips.com> > [Updated to current code base:] > Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com> > --- > > Changes in v5: None > Changes in v4: None > Changes in v3: None > Changes in v2: None > > arch/arm/include/asm/arch-rockchip/boot0.h | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > Applied to u-boot-rockchip/next, thanks!
Hi Phipipp, Kever: On 2017年10月10日 22:21, Philipp Tomsich wrote: > From: Kever Yang <kever.yang@rock-chips.com> > > The '_start' is using as vector table base address, and will write > to VBAR register, so it needs to be aligned to 0x20 for armv7. > > Signed-off-by: Kever Yang <kever.yang@rock-chips.com> > [Updated to current code base:] > Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com> > > --- > > Changes in v5: None > Changes in v4: None > Changes in v3: None > Changes in v2: None > > arch/arm/include/asm/arch-rockchip/boot0.h | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/include/asm/arch-rockchip/boot0.h b/arch/arm/include/asm/arch-rockchip/boot0.h > index 455d842..f7c6146 100644 > --- a/arch/arm/include/asm/arch-rockchip/boot0.h > +++ b/arch/arm/include/asm/arch-rockchip/boot0.h > @@ -6,12 +6,13 @@ > > /* > * Execution starts on the instruction following this 4-byte header > - * (containing the magic 'RK33'). > + * (containing the magic 'RK30', 'RK31', 'RK32' or 'RK33'). This > + * magic constant will be written into the final image by the rkimage > + * tool, but we need to reserve space for it here. > * > * To make life easier for everyone, we build the SPL binary with > * space for this 4-byte header already included in the binary. > */ > - > #ifdef CONFIG_SPL_BUILD > /* > * We need to add 4 bytes of space for the 'RK33' at the > @@ -26,6 +27,15 @@ > b reset /* may be overwritten --- should be 'nop' or a 'b reset' */ > #endif > b reset Do we really need the "b reset" here? the macro ARM_VECTORS already has a b reset. Besides Joseph found that the irq function will not work with this "b reset" > +#if !defined(CONFIG_ARM64) > + /* > + * For armv7, the addr '_start' will used as vector start address > + * and write to VBAR register, which needs to aligned to 0x20. > + */ > + .align(5) > +_start: > + ARM_VECTORS > +#endif > > #if defined(CONFIG_ROCKCHIP_RK3399) && defined(CONFIG_SPL_BUILD) > .space CONFIG_ROCKCHIP_SPL_RESERVE_IRAM /* space for the ATF data */
Andy, On 9 Nov 2017, at 13:59, Andy Yan <andy.yan@rock-chips.com> wrote: > > Hi Phipipp, Kever: > > > On 2017年10月10日 22:21, Philipp Tomsich wrote: >> From: Kever Yang <kever.yang@rock-chips.com> >> >> The '_start' is using as vector table base address, and will write >> to VBAR register, so it needs to be aligned to 0x20 for armv7. >> >> Signed-off-by: Kever Yang <kever.yang@rock-chips.com> >> [Updated to current code base:] >> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com> >> >> --- >> >> Changes in v5: None >> Changes in v4: None >> Changes in v3: None >> Changes in v2: None >> >> arch/arm/include/asm/arch-rockchip/boot0.h | 14 ++++++++++++-- >> 1 file changed, 12 insertions(+), 2 deletions(-) >> >> diff --git a/arch/arm/include/asm/arch-rockchip/boot0.h b/arch/arm/include/asm/arch-rockchip/boot0.h >> index 455d842..f7c6146 100644 >> --- a/arch/arm/include/asm/arch-rockchip/boot0.h >> +++ b/arch/arm/include/asm/arch-rockchip/boot0.h >> @@ -6,12 +6,13 @@ >> /* >> * Execution starts on the instruction following this 4-byte header >> - * (containing the magic 'RK33'). >> + * (containing the magic 'RK30', 'RK31', 'RK32' or 'RK33'). This >> + * magic constant will be written into the final image by the rkimage >> + * tool, but we need to reserve space for it here. >> * >> * To make life easier for everyone, we build the SPL binary with >> * space for this 4-byte header already included in the binary. >> */ >> - >> #ifdef CONFIG_SPL_BUILD >> /* >> * We need to add 4 bytes of space for the 'RK33' at the >> @@ -26,6 +27,15 @@ >> b reset /* may be overwritten --- should be 'nop' or a 'b reset' */ >> #endif >> b reset > > Do we really need the "b reset" here? the macro ARM_VECTORS already has a b reset. > Besides Joseph found that the irq function will not work with this "b reset” The quoted code was from an older version of the series, but the ‘b reset’ is still there in the newer version. What IRQ function does not work? Shouldn't the IRQ be jumping into the ARM_VECTORS and never see the ‘b reset’? >> +#if !defined(CONFIG_ARM64) >> + /* >> + * For armv7, the addr '_start' will used as vector start address >> + * and write to VBAR register, which needs to aligned to 0x20. >> + */ >> + .align(5) >> +_start: >> + ARM_VECTORS >> +#endif >> #if defined(CONFIG_ROCKCHIP_RK3399) && defined(CONFIG_SPL_BUILD) >> .space CONFIG_ROCKCHIP_SPL_RESERVE_IRAM /* space for the ATF data */ > >
Andy, On 11/09/2017 05:03 AM, Dr. Philipp Tomsich wrote: > Andy, > > On 9 Nov 2017, at 13:59, Andy Yan <andy.yan@rock-chips.com> wrote: >> Hi Phipipp, Kever: >> >> >> On 2017年10月10日 22:21, Philipp Tomsich wrote: >>> From: Kever Yang <kever.yang@rock-chips.com> >>> >>> The '_start' is using as vector table base address, and will write >>> to VBAR register, so it needs to be aligned to 0x20 for armv7. >>> >>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com> >>> [Updated to current code base:] >>> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com> >>> >>> --- >>> >>> Changes in v5: None >>> Changes in v4: None >>> Changes in v3: None >>> Changes in v2: None >>> >>> arch/arm/include/asm/arch-rockchip/boot0.h | 14 ++++++++++++-- >>> 1 file changed, 12 insertions(+), 2 deletions(-) >>> >>> diff --git a/arch/arm/include/asm/arch-rockchip/boot0.h b/arch/arm/include/asm/arch-rockchip/boot0.h >>> index 455d842..f7c6146 100644 >>> --- a/arch/arm/include/asm/arch-rockchip/boot0.h >>> +++ b/arch/arm/include/asm/arch-rockchip/boot0.h >>> @@ -6,12 +6,13 @@ >>> /* >>> * Execution starts on the instruction following this 4-byte header >>> - * (containing the magic 'RK33'). >>> + * (containing the magic 'RK30', 'RK31', 'RK32' or 'RK33'). This >>> + * magic constant will be written into the final image by the rkimage >>> + * tool, but we need to reserve space for it here. >>> * >>> * To make life easier for everyone, we build the SPL binary with >>> * space for this 4-byte header already included in the binary. >>> */ >>> - >>> #ifdef CONFIG_SPL_BUILD >>> /* >>> * We need to add 4 bytes of space for the 'RK33' at the >>> @@ -26,6 +27,15 @@ >>> b reset /* may be overwritten --- should be 'nop' or a 'b reset' */ >>> #endif >>> b reset >> Do we really need the "b reset" here? the macro ARM_VECTORS already has a b reset. >> Besides Joseph found that the irq function will not work with this "b reset” > The quoted code was from an older version of the series, but the ‘b reset’ is still there > in the newer version. > > What IRQ function does not work? > Shouldn't the IRQ be jumping into the ARM_VECTORS and never see the ‘b reset’? We need the 'b reset' here, this is not present at the same time with ARM_VECTORS, I think what we need to handle is that we don't need boot0_hook in U-Boot, let me send a patch to fix this. Thanks, - Kever > >>> +#if !defined(CONFIG_ARM64) >>> + /* >>> + * For armv7, the addr '_start' will used as vector start address >>> + * and write to VBAR register, which needs to aligned to 0x20. >>> + */ >>> + .align(5) >>> +_start: >>> + ARM_VECTORS >>> +#endif >>> #if defined(CONFIG_ROCKCHIP_RK3399) && defined(CONFIG_SPL_BUILD) >>> .space CONFIG_ROCKCHIP_SPL_RESERVE_IRAM /* space for the ATF data */ >> >
diff --git a/arch/arm/include/asm/arch-rockchip/boot0.h b/arch/arm/include/asm/arch-rockchip/boot0.h index 455d842..f7c6146 100644 --- a/arch/arm/include/asm/arch-rockchip/boot0.h +++ b/arch/arm/include/asm/arch-rockchip/boot0.h @@ -6,12 +6,13 @@ /* * Execution starts on the instruction following this 4-byte header - * (containing the magic 'RK33'). + * (containing the magic 'RK30', 'RK31', 'RK32' or 'RK33'). This + * magic constant will be written into the final image by the rkimage + * tool, but we need to reserve space for it here. * * To make life easier for everyone, we build the SPL binary with * space for this 4-byte header already included in the binary. */ - #ifdef CONFIG_SPL_BUILD /* * We need to add 4 bytes of space for the 'RK33' at the @@ -26,6 +27,15 @@ b reset /* may be overwritten --- should be 'nop' or a 'b reset' */ #endif b reset +#if !defined(CONFIG_ARM64) + /* + * For armv7, the addr '_start' will used as vector start address + * and write to VBAR register, which needs to aligned to 0x20. + */ + .align(5) +_start: + ARM_VECTORS +#endif #if defined(CONFIG_ROCKCHIP_RK3399) && defined(CONFIG_SPL_BUILD) .space CONFIG_ROCKCHIP_SPL_RESERVE_IRAM /* space for the ATF data */