| Message ID | 1479653838-3574-12-git-send-email-andre.przywara@arm.com |
|---|---|
| State | Accepted |
| Commit | b5402d13d4a3fe49af884ba7d5d32700af911536 |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
Hi Andre, On Sun, Nov 20, 2016 at 02:57:05PM +0000, Andre Przywara wrote: > The ENABLE_ARM_SOC_BOOT0_HOOK option is a generic option shared with > other boards. To allow alternative code to be inserted, we create > another, now function specific config symbol on top of it to simplify > later additions. No functional change at this time. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > board/sunxi/Kconfig | 9 +++++++++ > configs/pine64_plus_defconfig | 2 +- > 2 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig > index e1d4ab1..0cd57a2 100644 > --- a/board/sunxi/Kconfig > +++ b/board/sunxi/Kconfig > @@ -133,6 +133,15 @@ config MACH_SUN8I > bool > default y if MACH_SUN8I_A23 || MACH_SUN8I_A33 || MACH_SUN8I_H3 || MACH_SUN8I_A83T > > +config RESERVE_ALLWINNER_BOOT0_HEADER > + bool "reserve space for Allwinner boot0 header" > + select ENABLE_ARM_SOC_BOOT0_HOOK > + ---help--- > + Prepend a 1536 byte (empty) header to the U-Boot image file, to be > + filled with magic values post build. The Allwinner provided boot0 > + blob relies on this information to load and execute U-Boot. > + Only needed on 64-bit Allwinner boards so far when using boot0. > + Is there a reason you can think of to disable it? If not, you should consider making this enabled by default, so that we don't enable it in all the defconfig for no particular reason. Maxime
Hi Maxime, thanks for having a look! On 21/11/16 07:27, Maxime Ripard wrote: > Hi Andre, > > On Sun, Nov 20, 2016 at 02:57:05PM +0000, Andre Przywara wrote: >> The ENABLE_ARM_SOC_BOOT0_HOOK option is a generic option shared with >> other boards. To allow alternative code to be inserted, we create >> another, now function specific config symbol on top of it to simplify >> later additions. No functional change at this time. >> >> Signed-off-by: Andre Przywara <andre.przywara@arm.com> >> --- >> board/sunxi/Kconfig | 9 +++++++++ >> configs/pine64_plus_defconfig | 2 +- >> 2 files changed, 10 insertions(+), 1 deletion(-) >> >> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig >> index e1d4ab1..0cd57a2 100644 >> --- a/board/sunxi/Kconfig >> +++ b/board/sunxi/Kconfig >> @@ -133,6 +133,15 @@ config MACH_SUN8I >> bool >> default y if MACH_SUN8I_A23 || MACH_SUN8I_A33 || MACH_SUN8I_H3 || MACH_SUN8I_A83T >> >> +config RESERVE_ALLWINNER_BOOT0_HEADER >> + bool "reserve space for Allwinner boot0 header" >> + select ENABLE_ARM_SOC_BOOT0_HOOK >> + ---help--- >> + Prepend a 1536 byte (empty) header to the U-Boot image file, to be >> + filled with magic values post build. The Allwinner provided boot0 >> + blob relies on this information to load and execute U-Boot. >> + Only needed on 64-bit Allwinner boards so far when using boot0. >> + > > Is there a reason you can think of to disable it? We need it only for booting from boot0, so this series actually makes this whole thing obsolete. Since - apart from enlarging the U-Boot (proper) image by 1.5KB - it doesn't hurt, though, my idea was to keep it in as an option for some time until we are confident that boot0 is no longer needed. > If not, you should consider making this enabled by default, so that we > don't enable it in all the defconfig for no particular reason. I can change the logic, make the Kconfig entry "default y if ARM64", and any defconfig could then choose to say "# RESERVE_... is not set". Does that make more sense to you? What was your major concern about this? Having pointless options in various defconfigs? Cheers, Andre.
On Mon, Nov 21, 2016 at 09:29:50AM +0000, Andre Przywara wrote: > Hi Maxime, > > thanks for having a look! > > On 21/11/16 07:27, Maxime Ripard wrote: > > Hi Andre, > > > > On Sun, Nov 20, 2016 at 02:57:05PM +0000, Andre Przywara wrote: > >> The ENABLE_ARM_SOC_BOOT0_HOOK option is a generic option shared with > >> other boards. To allow alternative code to be inserted, we create > >> another, now function specific config symbol on top of it to simplify > >> later additions. No functional change at this time. > >> > >> Signed-off-by: Andre Przywara <andre.przywara@arm.com> > >> --- > >> board/sunxi/Kconfig | 9 +++++++++ > >> configs/pine64_plus_defconfig | 2 +- > >> 2 files changed, 10 insertions(+), 1 deletion(-) > >> > >> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig > >> index e1d4ab1..0cd57a2 100644 > >> --- a/board/sunxi/Kconfig > >> +++ b/board/sunxi/Kconfig > >> @@ -133,6 +133,15 @@ config MACH_SUN8I > >> bool > >> default y if MACH_SUN8I_A23 || MACH_SUN8I_A33 || MACH_SUN8I_H3 || MACH_SUN8I_A83T > >> > >> +config RESERVE_ALLWINNER_BOOT0_HEADER > >> + bool "reserve space for Allwinner boot0 header" > >> + select ENABLE_ARM_SOC_BOOT0_HOOK > >> + ---help--- > >> + Prepend a 1536 byte (empty) header to the U-Boot image file, to be > >> + filled with magic values post build. The Allwinner provided boot0 > >> + blob relies on this information to load and execute U-Boot. > >> + Only needed on 64-bit Allwinner boards so far when using boot0. > >> + > > > > Is there a reason you can think of to disable it? > > We need it only for booting from boot0, so this series actually makes > this whole thing obsolete. Since - apart from enlarging the U-Boot > (proper) image by 1.5KB - it doesn't hurt, though, my idea was to keep > it in as an option for some time until we are confident that boot0 is no > longer needed. Then we don't need to enable it in the defconfig ? > > If not, you should consider making this enabled by default, so that we > > don't enable it in all the defconfig for no particular reason. > > I can change the logic, make the Kconfig entry "default y if ARM64", and > any defconfig could then choose to say "# RESERVE_... is not set". > > Does that make more sense to you? > > What was your major concern about this? Having pointless options in > various defconfigs? Yes, Hans was trying to avoid having too much duplication across defconfig, at least for the common stuff, and I agree with him that we should keep the defconfig as small as possible. Maxime
diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig index e1d4ab1..0cd57a2 100644 --- a/board/sunxi/Kconfig +++ b/board/sunxi/Kconfig @@ -133,6 +133,15 @@ config MACH_SUN8I bool default y if MACH_SUN8I_A23 || MACH_SUN8I_A33 || MACH_SUN8I_H3 || MACH_SUN8I_A83T +config RESERVE_ALLWINNER_BOOT0_HEADER + bool "reserve space for Allwinner boot0 header" + select ENABLE_ARM_SOC_BOOT0_HOOK + ---help--- + Prepend a 1536 byte (empty) header to the U-Boot image file, to be + filled with magic values post build. The Allwinner provided boot0 + blob relies on this information to load and execute U-Boot. + Only needed on 64-bit Allwinner boards so far when using boot0. + config DRAM_TYPE int "sunxi dram type" depends on MACH_SUN8I_A83T diff --git a/configs/pine64_plus_defconfig b/configs/pine64_plus_defconfig index 6d0198f..ea53b96 100644 --- a/configs/pine64_plus_defconfig +++ b/configs/pine64_plus_defconfig @@ -1,5 +1,5 @@ CONFIG_ARM=y -CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK=y +CONFIG_RESERVE_ALLWINNER_BOOT0_HEADER=y CONFIG_ARCH_SUNXI=y CONFIG_MACH_SUN50I=y CONFIG_DRAM_CLK=672
The ENABLE_ARM_SOC_BOOT0_HOOK option is a generic option shared with other boards. To allow alternative code to be inserted, we create another, now function specific config symbol on top of it to simplify later additions. No functional change at this time. Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- board/sunxi/Kconfig | 9 +++++++++ configs/pine64_plus_defconfig | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-)