| Message ID | 1496227840-19007-2-git-send-email-kever.yang@rock-chips.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Philipp Tomsich |
| Headers | show |
Hi Andre, Steve, Marek,
Could you help to check how to make it work with this patch on
sunxi, bcm and socfpga platform?
Thanks,
- Kever
On 05/31/2017 06:50 PM, Kever Yang wrote:
> The boot0 hook suppose to add some data before the SPL data,
> let's move it at very begining and before '_start'.
>
> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
> ---
>
> arch/arm/lib/vectors.S | 19 ++++++++++---------
> 1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/arch/arm/lib/vectors.S b/arch/arm/lib/vectors.S
> index f53b1e9..b4cd825 100644
> --- a/arch/arm/lib/vectors.S
> +++ b/arch/arm/lib/vectors.S
> @@ -35,6 +35,16 @@
>
> .section ".vectors", "ax"
>
> +#ifdef CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK
> +/*
> + * Various SoCs need something special and SoC-specific up front in
> + * order to boot, allow them to set that in their boot0.h file and then
> + * use it here.
> + */
> +#include <asm/arch/boot0.h>
> +
> +#endif
> +
> /*
> *************************************************************************
> *
> @@ -60,15 +70,6 @@ _start:
> ldr pc, _irq
> ldr pc, _fiq
>
> -#ifdef CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK
> -/*
> - * Various SoCs need something special and SoC-specific up front in
> - * order to boot, allow them to set that in their boot0.h file and then
> - * use it here.
> - */
> -#include <asm/arch/boot0.h>
> -#endif
> -
> /*
> *************************************************************************
> *
On 06/07/2017 04:28 AM, Kever Yang wrote: > Hi Andre, Steve, Marek, > > Could you help to check how to make it work with this patch on > sunxi, bcm and socfpga platform? The socfpga expects the hook at that exact position (0x40 I think) , so if you moved it somewhere, you broke socfpga. > Thanks, > - Kever > On 05/31/2017 06:50 PM, Kever Yang wrote: >> The boot0 hook suppose to add some data before the SPL data, >> let's move it at very begining and before '_start'. >> >> Signed-off-by: Kever Yang <kever.yang@rock-chips.com> >> --- >> >> arch/arm/lib/vectors.S | 19 ++++++++++--------- >> 1 file changed, 10 insertions(+), 9 deletions(-) >> >> diff --git a/arch/arm/lib/vectors.S b/arch/arm/lib/vectors.S >> index f53b1e9..b4cd825 100644 >> --- a/arch/arm/lib/vectors.S >> +++ b/arch/arm/lib/vectors.S >> @@ -35,6 +35,16 @@ >> .section ".vectors", "ax" >> +#ifdef CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK >> +/* >> + * Various SoCs need something special and SoC-specific up front in >> + * order to boot, allow them to set that in their boot0.h file and then >> + * use it here. >> + */ >> +#include <asm/arch/boot0.h> >> + >> +#endif >> + >> /* >> >> ************************************************************************* >> * >> @@ -60,15 +70,6 @@ _start: >> ldr pc, _irq >> ldr pc, _fiq >> -#ifdef CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK >> -/* >> - * Various SoCs need something special and SoC-specific up front in >> - * order to boot, allow them to set that in their boot0.h file and then >> - * use it here. >> - */ >> -#include <asm/arch/boot0.h> >> -#endif >> - >> /* >> >> ************************************************************************* >> * > >
Hi Marek, On 06/07/2017 02:28 PM, Marek Vasut wrote: > On 06/07/2017 04:28 AM, Kever Yang wrote: >> Hi Andre, Steve, Marek, >> >> Could you help to check how to make it work with this patch on >> sunxi, bcm and socfpga platform? > The socfpga expects the hook at that exact position (0x40 I think) , so > if you moved it somewhere, you broke socfpga. I know this break socfpga, and that's why I cc you for help, could you help the take a look if we can have a solution on socfpga to "make it work with this patch"? Comments from Philipp[0] show that its reasonable for this patch set, I don't know how the boot0-hook works in socfpga and also sunxi and bcm, could you help me to fix the hook content upon this patch? Thanks, - Kever [0]https://www.mail-archive.com/u-boot@lists.denx.de/msg252823.html > >> Thanks, >> - Kever >> On 05/31/2017 06:50 PM, Kever Yang wrote: >>> The boot0 hook suppose to add some data before the SPL data, >>> let's move it at very begining and before '_start'. >>> >>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com> >>> --- >>> >>> arch/arm/lib/vectors.S | 19 ++++++++++--------- >>> 1 file changed, 10 insertions(+), 9 deletions(-) >>> >>> diff --git a/arch/arm/lib/vectors.S b/arch/arm/lib/vectors.S >>> index f53b1e9..b4cd825 100644 >>> --- a/arch/arm/lib/vectors.S >>> +++ b/arch/arm/lib/vectors.S >>> @@ -35,6 +35,16 @@ >>> .section ".vectors", "ax" >>> +#ifdef CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK >>> +/* >>> + * Various SoCs need something special and SoC-specific up front in >>> + * order to boot, allow them to set that in their boot0.h file and then >>> + * use it here. >>> + */ >>> +#include <asm/arch/boot0.h> >>> + >>> +#endif >>> + >>> /* >>> >>> ************************************************************************* >>> * >>> @@ -60,15 +70,6 @@ _start: >>> ldr pc, _irq >>> ldr pc, _fiq >>> -#ifdef CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK >>> -/* >>> - * Various SoCs need something special and SoC-specific up front in >>> - * order to boot, allow them to set that in their boot0.h file and then >>> - * use it here. >>> - */ >>> -#include <asm/arch/boot0.h> >>> -#endif >>> - >>> /* >>> >>> ************************************************************************* >>> * >> >
On 06/13/2017 03:51 AM, Kever Yang wrote: > Hi Marek, Hi, > On 06/07/2017 02:28 PM, Marek Vasut wrote: >> On 06/07/2017 04:28 AM, Kever Yang wrote: >>> Hi Andre, Steve, Marek, >>> >>> Could you help to check how to make it work with this patch on >>> sunxi, bcm and socfpga platform? >> The socfpga expects the hook at that exact position (0x40 I think) , so >> if you moved it somewhere, you broke socfpga. > > I know this break socfpga, and that's why I cc you for help, could you > help the take a look if we can have a solution on socfpga to "make it > work with this patch"? Can you generate the same u-boot binary with this patch ? If so, then it will work on socfpga. SoCFPGA expects that small piece of stuff at offset 0x40 , so if you move this boot0 hook, it will break. HTH > Comments from Philipp[0] show that its reasonable for this patch set, > I don't know how the boot0-hook works in socfpga and also sunxi and bcm, > could you help me to fix the hook content upon this patch? > > Thanks, > - Kever > [0]https://www.mail-archive.com/u-boot@lists.denx.de/msg252823.html >> >>> Thanks, >>> - Kever >>> On 05/31/2017 06:50 PM, Kever Yang wrote: >>>> The boot0 hook suppose to add some data before the SPL data, >>>> let's move it at very begining and before '_start'. >>>> >>>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com> >>>> --- >>>> >>>> arch/arm/lib/vectors.S | 19 ++++++++++--------- >>>> 1 file changed, 10 insertions(+), 9 deletions(-) >>>> >>>> diff --git a/arch/arm/lib/vectors.S b/arch/arm/lib/vectors.S >>>> index f53b1e9..b4cd825 100644 >>>> --- a/arch/arm/lib/vectors.S >>>> +++ b/arch/arm/lib/vectors.S >>>> @@ -35,6 +35,16 @@ >>>> .section ".vectors", "ax" >>>> +#ifdef CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK >>>> +/* >>>> + * Various SoCs need something special and SoC-specific up front in >>>> + * order to boot, allow them to set that in their boot0.h file and >>>> then >>>> + * use it here. >>>> + */ >>>> +#include <asm/arch/boot0.h> >>>> + >>>> +#endif >>>> + >>>> /* >>>> >>>> ************************************************************************* >>>> >>>> * >>>> @@ -60,15 +70,6 @@ _start: >>>> ldr pc, _irq >>>> ldr pc, _fiq >>>> -#ifdef CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK >>>> -/* >>>> - * Various SoCs need something special and SoC-specific up front in >>>> - * order to boot, allow them to set that in their boot0.h file and >>>> then >>>> - * use it here. >>>> - */ >>>> -#include <asm/arch/boot0.h> >>>> -#endif >>>> - >>>> /* >>>> >>>> ************************************************************************* >>>> >>>> * >>> >> > >
Marek, > On 13 Jun 2017, at 10:42, Marek Vasut <marex@denx.de> wrote: > > On 06/13/2017 03:51 AM, Kever Yang wrote: >> Hi Marek, > > Hi, > >> On 06/07/2017 02:28 PM, Marek Vasut wrote: >>> On 06/07/2017 04:28 AM, Kever Yang wrote: >>>> Hi Andre, Steve, Marek, >>>> >>>> Could you help to check how to make it work with this patch on >>>> sunxi, bcm and socfpga platform? >>> The socfpga expects the hook at that exact position (0x40 I think) , so >>> if you moved it somewhere, you broke socfpga. >> >> I know this break socfpga, and that's why I cc you for help, could you >> help the take a look if we can have a solution on socfpga to "make it >> work with this patch"? > > Can you generate the same u-boot binary with this patch ? If so, then it > will work on socfpga. SoCFPGA expects that small piece of stuff at > offset 0x40 , so if you move this boot0 hook, it will break. HTH So the SoCFPGA-implementation relies on the boot0 hook to always start at 0x40? Then this needs fixing anyway… or it will break once someone else touches the armv7 startup code. That said, I read your comment to mean that the following changes would work for SoCFPGA: 1. arch/arm/lib/vectors.S: #if CONFIG_IS_ENABLED(BOOT0_HOOK) … pull in the boot0 hook, which needs to contain the vectors #else b reset … remaining vectors … #endif 2. boot0.h (for socfpga): b reset … remaining vectors … /* now at 0x40 */ … whatever currently lives in the boot0.h for socfpga ... Ok? > >> Comments from Philipp[0] show that its reasonable for this patch set, >> I don't know how the boot0-hook works in socfpga and also sunxi and bcm, >> could you help me to fix the hook content upon this patch? >> >> Thanks, >> - Kever >> [0]https://www.mail-archive.com/u-boot@lists.denx.de/msg252823.html >>> >>>> Thanks, >>>> - Kever >>>> On 05/31/2017 06:50 PM, Kever Yang wrote: >>>>> The boot0 hook suppose to add some data before the SPL data, >>>>> let's move it at very begining and before '_start'. >>>>> >>>>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com> >>>>> --- >>>>> >>>>> arch/arm/lib/vectors.S | 19 ++++++++++--------- >>>>> 1 file changed, 10 insertions(+), 9 deletions(-) >>>>> >>>>> diff --git a/arch/arm/lib/vectors.S b/arch/arm/lib/vectors.S >>>>> index f53b1e9..b4cd825 100644 >>>>> --- a/arch/arm/lib/vectors.S >>>>> +++ b/arch/arm/lib/vectors.S >>>>> @@ -35,6 +35,16 @@ >>>>> .section ".vectors", "ax" >>>>> +#ifdef CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK >>>>> +/* >>>>> + * Various SoCs need something special and SoC-specific up front in >>>>> + * order to boot, allow them to set that in their boot0.h file and >>>>> then >>>>> + * use it here. >>>>> + */ >>>>> +#include <asm/arch/boot0.h> >>>>> + >>>>> +#endif >>>>> + >>>>> /* >>>>> >>>>> ************************************************************************* >>>>> >>>>> * >>>>> @@ -60,15 +70,6 @@ _start: >>>>> ldr pc, _irq >>>>> ldr pc, _fiq >>>>> -#ifdef CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK >>>>> -/* >>>>> - * Various SoCs need something special and SoC-specific up front in >>>>> - * order to boot, allow them to set that in their boot0.h file and >>>>> then >>>>> - * use it here. >>>>> - */ >>>>> -#include <asm/arch/boot0.h> >>>>> -#endif >>>>> - >>>>> /* >>>>> >>>>> ************************************************************************* >>>>> >>>>> * >>>> >>> >> >> > > > -- > Best regards, > Marek Vasut
On 06/13/2017 11:44 AM, Dr. Philipp Tomsich wrote: > Marek, > >> On 13 Jun 2017, at 10:42, Marek Vasut <marex@denx.de >> <mailto:marex@denx.de>> wrote: >> >> On 06/13/2017 03:51 AM, Kever Yang wrote: >>> Hi Marek, >> >> Hi, >> >>> On 06/07/2017 02:28 PM, Marek Vasut wrote: >>>> On 06/07/2017 04:28 AM, Kever Yang wrote: >>>>> Hi Andre, Steve, Marek, >>>>> >>>>> Could you help to check how to make it work with this patch on >>>>> sunxi, bcm and socfpga platform? >>>> The socfpga expects the hook at that exact position (0x40 I think) , so >>>> if you moved it somewhere, you broke socfpga. >>> >>> I know this break socfpga, and that's why I cc you for help, could you >>> help the take a look if we can have a solution on socfpga to "make it >>> work with this patch"? >> >> Can you generate the same u-boot binary with this patch ? If so, then it >> will work on socfpga. SoCFPGA expects that small piece of stuff at >> offset 0x40 , so if you move this boot0 hook, it will break. HTH > > So the SoCFPGA-implementation relies on the boot0 hook to always start > at 0x40? Then this needs fixing anyway… or it will break once someone > else touches the armv7 startup code. I think it's not just the SoCFPGA. > That said, I read your comment to mean that the following changes would > work for SoCFPGA: > 1.arch/arm/lib/vectors.S: > #if CONFIG_IS_ENABLED(BOOT0_HOOK) > … pull in the boot0 hook, which needs to contain the vectors > #else > b reset > … remaining vectors … > #endif > 2.boot0.h (for socfpga): > b reset > … remaining vectors … > /* now at 0x40 */ > … whatever currently lives in the boot0.h for socfpga ... > > Ok? I'm not sure I understand what you're trying to say. IIRC the SoCFPGA bootrom checks the content at 0x40-0x48 in SRAM and then jumps to 0x50 if the content is valid. Otherwise it reloads the content from SD/SPI/NAND... [...] btw can you please disable the HTML email ?
> On 13 Jun 2017, at 11:47, Marek Vasut <marex@denx.de> wrote: > > On 06/13/2017 11:44 AM, Dr. Philipp Tomsich wrote: >> Marek, >> >>> On 13 Jun 2017, at 10:42, Marek Vasut <marex@denx.de >>> <mailto:marex@denx.de>> wrote: >>> >>> On 06/13/2017 03:51 AM, Kever Yang wrote: >>>> Hi Marek, >>> >>> Hi, >>> >>>> On 06/07/2017 02:28 PM, Marek Vasut wrote: >>>>> On 06/07/2017 04:28 AM, Kever Yang wrote: >>>>>> Hi Andre, Steve, Marek, >>>>>> >>>>>> Could you help to check how to make it work with this patch on >>>>>> sunxi, bcm and socfpga platform? >>>>> The socfpga expects the hook at that exact position (0x40 I think) , so >>>>> if you moved it somewhere, you broke socfpga. >>>> >>>> I know this break socfpga, and that's why I cc you for help, could you >>>> help the take a look if we can have a solution on socfpga to "make it >>>> work with this patch"? >>> >>> Can you generate the same u-boot binary with this patch ? If so, then it >>> will work on socfpga. SoCFPGA expects that small piece of stuff at >>> offset 0x40 , so if you move this boot0 hook, it will break. HTH >> >> So the SoCFPGA-implementation relies on the boot0 hook to always start >> at 0x40? Then this needs fixing anyway… or it will break once someone >> else touches the armv7 startup code. > > I think it's not just the SoCFPGA. What I am saying is that the boot0-hook is intended to add a header before the actual U-Boot code (as Andre had originally stated in his commit message and as various comments around the boot0 hook also suggest): arm/arm64: implement a boot header capability Some SPL loaders (like Allwinner's boot0, and Broadcom's boot0) require a header before the actual U-Boot binary to both check its validity and to find other data to load. Sometimes this header may only be a few bytes of information, and sometimes this might simply be space that needs to be reserved for a post-processing tool. However, the actual start of the boot0-hook in the U-Boot today is right after the vector-table (which is 0x40 long), which has a few interesting consequences for various architectures: - sunxi: doesn’t matter - socfpga: boot0 hook relies on something being in front of it, as it uses a ".balign 64, …” to advance to 0x40 - rockchip: comes too late to allow inserting the word-sized field at the very start (and to move the vectors back by a word). Now having looked at the implementation in arch/arm/mach-socfpga/include/mach/boot0.h it is clear why Kever’s change breaks this code: .balign will not forward to 0x40 (after all, 0x0 is also 64-byte aligned). Once the boot0 hook is at the beginning of the binary (as always intended), this will need to become a .space or .fill directive. >> That said, I read your comment to mean that the following changes would >> work for SoCFPGA: >> 1.arch/arm/lib/vectors.S: >> #if CONFIG_IS_ENABLED(BOOT0_HOOK) >> … pull in the boot0 hook, which needs to contain the vectors >> #else >> b reset >> … remaining vectors … >> #endif >> 2.boot0.h (for socfpga): >> b reset >> … remaining vectors … >> /* now at 0x40 */ >> … whatever currently lives in the boot0.h for socfpga ... >> >> Ok? > > I'm not sure I understand what you're trying to say. IIRC the SoCFPGA > bootrom checks the content at 0x40-0x48 in SRAM and then jumps to 0x50 > if the content is valid. Otherwise it reloads the content from > SD/SPI/NAND… Thanks for the background info. With this, it should be possible to create a new version of the patch that also correctly handles socfpga. Regards, Philipp.
Hi, On 13 June 2017 at 04:31, Dr. Philipp Tomsich <philipp.tomsich@theobroma-systems.com> wrote: > >> On 13 Jun 2017, at 11:47, Marek Vasut <marex@denx.de> wrote: >> >> On 06/13/2017 11:44 AM, Dr. Philipp Tomsich wrote: >>> Marek, >>> >>>> On 13 Jun 2017, at 10:42, Marek Vasut <marex@denx.de >>>> <mailto:marex@denx.de>> wrote: >>>> >>>> On 06/13/2017 03:51 AM, Kever Yang wrote: >>>>> Hi Marek, >>>> >>>> Hi, >>>> >>>>> On 06/07/2017 02:28 PM, Marek Vasut wrote: >>>>>> On 06/07/2017 04:28 AM, Kever Yang wrote: >>>>>>> Hi Andre, Steve, Marek, >>>>>>> >>>>>>> Could you help to check how to make it work with this patch on >>>>>>> sunxi, bcm and socfpga platform? >>>>>> The socfpga expects the hook at that exact position (0x40 I think) , so >>>>>> if you moved it somewhere, you broke socfpga. >>>>> >>>>> I know this break socfpga, and that's why I cc you for help, could you >>>>> help the take a look if we can have a solution on socfpga to "make it >>>>> work with this patch"? >>>> >>>> Can you generate the same u-boot binary with this patch ? If so, then it >>>> will work on socfpga. SoCFPGA expects that small piece of stuff at >>>> offset 0x40 , so if you move this boot0 hook, it will break. HTH >>> >>> So the SoCFPGA-implementation relies on the boot0 hook to always start >>> at 0x40? Then this needs fixing anyway… or it will break once someone >>> else touches the armv7 startup code. >> >> I think it's not just the SoCFPGA. > > What I am saying is that the boot0-hook is intended to add a header > before the actual U-Boot code (as Andre had originally stated in his > commit message and as various comments around the boot0 hook > also suggest): > > arm/arm64: implement a boot header capability > > Some SPL loaders (like Allwinner's boot0, and Broadcom's boot0) > require a header before the actual U-Boot binary to both check its > validity and to find other data to load. Sometimes this header may > only be a few bytes of information, and sometimes this might simply > be space that needs to be reserved for a post-processing tool. > > However, the actual start of the boot0-hook in the U-Boot today is right > after the vector-table (which is 0x40 long), which has a few interesting > consequences for various architectures: > - sunxi: doesn’t matter > - socfpga: boot0 hook relies on something being in front of it, as it uses > a ".balign 64, …” to advance to 0x40 > - rockchip: comes too late to allow inserting the word-sized field at the > very start (and to move the vectors back by a word). > > Now having looked at the implementation in > arch/arm/mach-socfpga/include/mach/boot0.h > it is clear why Kever’s change breaks this code: .balign will not forward > to 0x40 (after all, 0x0 is also 64-byte aligned). > > Once the boot0 hook is at the beginning of the binary (as always intended), > this will need to become a .space or .fill directive. > >>> That said, I read your comment to mean that the following changes would >>> work for SoCFPGA: >>> 1.arch/arm/lib/vectors.S: >>> #if CONFIG_IS_ENABLED(BOOT0_HOOK) >>> … pull in the boot0 hook, which needs to contain the vectors >>> #else >>> b reset >>> … remaining vectors … >>> #endif >>> 2.boot0.h (for socfpga): >>> b reset >>> … remaining vectors … >>> /* now at 0x40 */ >>> … whatever currently lives in the boot0.h for socfpga ... >>> >>> Ok? >> >> I'm not sure I understand what you're trying to say. IIRC the SoCFPGA >> bootrom checks the content at 0x40-0x48 in SRAM and then jumps to 0x50 >> if the content is valid. Otherwise it reloads the content from >> SD/SPI/NAND… > > Thanks for the background info. With this, it should be possible to create > a new version of the patch that also correctly handles socfpga. Sounds like this is figured out, great! - Simon
diff --git a/arch/arm/lib/vectors.S b/arch/arm/lib/vectors.S index f53b1e9..b4cd825 100644 --- a/arch/arm/lib/vectors.S +++ b/arch/arm/lib/vectors.S @@ -35,6 +35,16 @@ .section ".vectors", "ax" +#ifdef CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK +/* + * Various SoCs need something special and SoC-specific up front in + * order to boot, allow them to set that in their boot0.h file and then + * use it here. + */ +#include <asm/arch/boot0.h> + +#endif + /* ************************************************************************* * @@ -60,15 +70,6 @@ _start: ldr pc, _irq ldr pc, _fiq -#ifdef CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK -/* - * Various SoCs need something special and SoC-specific up front in - * order to boot, allow them to set that in their boot0.h file and then - * use it here. - */ -#include <asm/arch/boot0.h> -#endif - /* ************************************************************************* *
The boot0 hook suppose to add some data before the SPL data, let's move it at very begining and before '_start'. Signed-off-by: Kever Yang <kever.yang@rock-chips.com> --- arch/arm/lib/vectors.S | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-)