| Message ID | 20190402085727.14552-10-sr@denx.de |
|---|---|
| State | Superseded |
| Delegated to: | Eugen Hristev |
| Headers | show |
| Series | [U-Boot,01/13,v3] arm: at91: Makefile: Compile lowlevel_init only when really necessary | expand |
On 02.04.2019 11:57, Stefan Roese wrote: > > This patch adds the CONFIG_SPL_IMAGE option to select the SPL image that > shall be used to generate the combined SPL + U-Boot image. The default > value is the current value "spl/u-boot-spl.bin". > > This patch also sets CONFIG_SPL_IMAGE to "spl/boot.bin" for AT91 targets > which use SPL NAND support (boot from NAND). For these build targets the > combined image "u-boot-with-spl.bin" is now automatically generated and > can be programmed into NAND as one single image (vs. SPL image and U-Boot > as 2 separate images). > > Signed-off-by: Stefan Roese <sr@denx.de> > Cc: Heiko Schocher <hs@denx.de> > Cc: Andreas Bießmann <andreas@biessmann.org> > Cc: Eugen Hristev <eugen.hristev@microchip.com> > Reviewed-by: Heiko Schocher <hs@denx.de> > Tested on the taurus board: > Tested-by: Heiko Schocher <hs@denx.de> > --- > v3: > - No change > > v2: > - No change > > Kconfig | 10 ++++++++++ > Makefile | 4 +++- > 2 files changed, 13 insertions(+), 1 deletion(-) > > diff --git a/Kconfig b/Kconfig > index 305b265ed7..7c2b86f1f3 100644 > --- a/Kconfig > +++ b/Kconfig > @@ -224,6 +224,15 @@ config BUILD_ROM > which are not shipped in the U-Boot source tree. > Please, see doc/README.x86 for details. > > +config SPL_IMAGE > + string "SPL image filename that is generated" > + default "spl/boot.bin" if ARCH_AT91 && SPL_NAND_SUPPORT > + default "spl/u-boot-spl.bin" > + help > + The SPL image filename that is generated by the build process. > + This image might be used to generated a combined image with > + SPL and main U-Boot proper as well. > + Hi Stefan, If I try to just use menuconfig and change this value to something else, build fails. The purpose of this Kconfig is to name the SPL filename as per the config's desire ? Or in fact is "which binary file to use to make the Combined SPL+U-BOOT mega image" ? Eugen > config BUILD_TARGET > string "Build target special images" > default "u-boot-with-spl.sfp" if TARGET_SOCFPGA_ARRIA10 > @@ -232,6 +241,7 @@ config BUILD_TARGET > default "u-boot-elf.srec" if RCAR_GEN3 > default "u-boot.itb" if SPL_LOAD_FIT && ARCH_SUNXI > default "u-boot.kwb" if KIRKWOOD > + default "u-boot-with-spl.bin" if ARCH_AT91 && SPL_NAND_SUPPORT > help > Some SoCs need special image types (e.g. U-Boot binary > with a special header) as build targets. By defining > diff --git a/Makefile b/Makefile > index c1af9307b3..077bb6634e 100644 > --- a/Makefile > +++ b/Makefile > @@ -1225,9 +1225,11 @@ else > SPL_PAYLOAD := u-boot.bin > endif > > +SPL_IMAGE := $(CONFIG_SPL_IMAGE:"%"=%) > + > OBJCOPYFLAGS_u-boot-with-spl.bin = -I binary -O binary \ > --pad-to=$(CONFIG_SPL_PAD_TO) > -u-boot-with-spl.bin: spl/u-boot-spl.bin $(SPL_PAYLOAD) FORCE > +u-boot-with-spl.bin: $(SPL_IMAGE) $(SPL_PAYLOAD) FORCE > $(call if_changed,pad_cat) > > ifeq ($(CONFIG_ARCH_LPC32XX)$(CONFIG_SPL),yy) >
On 03.04.2019 14:03, Eugen Hristev wrote: > > > On 02.04.2019 11:57, Stefan Roese wrote: > >> >> This patch adds the CONFIG_SPL_IMAGE option to select the SPL image that >> shall be used to generate the combined SPL + U-Boot image. The default >> value is the current value "spl/u-boot-spl.bin". >> >> This patch also sets CONFIG_SPL_IMAGE to "spl/boot.bin" for AT91 targets >> which use SPL NAND support (boot from NAND). For these build targets the >> combined image "u-boot-with-spl.bin" is now automatically generated and >> can be programmed into NAND as one single image (vs. SPL image and U-Boot >> as 2 separate images). >> >> Signed-off-by: Stefan Roese <sr@denx.de> >> Cc: Heiko Schocher <hs@denx.de> >> Cc: Andreas Bießmann <andreas@biessmann.org> >> Cc: Eugen Hristev <eugen.hristev@microchip.com> >> Reviewed-by: Heiko Schocher <hs@denx.de> >> Tested on the taurus board: >> Tested-by: Heiko Schocher <hs@denx.de> >> --- >> v3: >> - No change >> >> v2: >> - No change >> >> Kconfig | 10 ++++++++++ >> Makefile | 4 +++- >> 2 files changed, 13 insertions(+), 1 deletion(-) >> >> diff --git a/Kconfig b/Kconfig >> index 305b265ed7..7c2b86f1f3 100644 >> --- a/Kconfig >> +++ b/Kconfig >> @@ -224,6 +224,15 @@ config BUILD_ROM >> which are not shipped in the U-Boot source tree. >> Please, see doc/README.x86 for details. >> +config SPL_IMAGE >> + string "SPL image filename that is generated" >> + default "spl/boot.bin" if ARCH_AT91 && SPL_NAND_SUPPORT >> + default "spl/u-boot-spl.bin" >> + help >> + The SPL image filename that is generated by the build process. >> + This image might be used to generated a combined image with >> + SPL and main U-Boot proper as well. >> + > > Hi Stefan, > > If I try to just use menuconfig and change this value to something else, > build fails. The purpose of this Kconfig is to name the SPL filename as > per the config's desire ? > > Or in fact is "which binary file to use to make the Combined SPL+U-BOOT > mega image" ? Ok, the commit message explains it pretty well, but the Kconfig help is totally different... hence the confusion sorry. So we need to either make a choice submenu in this Kconfig : either this or that; or, do exactly what the Kconfig says: rename the output binary into the value of this Kconfig. Does this make sense ? > > Eugen > >> config BUILD_TARGET >> string "Build target special images" >> default "u-boot-with-spl.sfp" if TARGET_SOCFPGA_ARRIA10 >> @@ -232,6 +241,7 @@ config BUILD_TARGET >> default "u-boot-elf.srec" if RCAR_GEN3 >> default "u-boot.itb" if SPL_LOAD_FIT && ARCH_SUNXI >> default "u-boot.kwb" if KIRKWOOD >> + default "u-boot-with-spl.bin" if ARCH_AT91 && SPL_NAND_SUPPORT >> help >> Some SoCs need special image types (e.g. U-Boot binary >> with a special header) as build targets. By defining >> diff --git a/Makefile b/Makefile >> index c1af9307b3..077bb6634e 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -1225,9 +1225,11 @@ else >> SPL_PAYLOAD := u-boot.bin >> endif >> +SPL_IMAGE := $(CONFIG_SPL_IMAGE:"%"=%) >> + >> OBJCOPYFLAGS_u-boot-with-spl.bin = -I binary -O binary \ >> --pad-to=$(CONFIG_SPL_PAD_TO) >> -u-boot-with-spl.bin: spl/u-boot-spl.bin $(SPL_PAYLOAD) FORCE >> +u-boot-with-spl.bin: $(SPL_IMAGE) $(SPL_PAYLOAD) FORCE >> $(call if_changed,pad_cat) >> ifeq ($(CONFIG_ARCH_LPC32XX)$(CONFIG_SPL),yy) >>
On 03.04.19 13:08, Eugen.Hristev@microchip.com wrote: > > > On 02.04.2019 11:57, Stefan Roese wrote: > >> >> This patch adds the CONFIG_SPL_IMAGE option to select the SPL image that >> shall be used to generate the combined SPL + U-Boot image. The default >> value is the current value "spl/u-boot-spl.bin". >> >> This patch also sets CONFIG_SPL_IMAGE to "spl/boot.bin" for AT91 targets >> which use SPL NAND support (boot from NAND). For these build targets the >> combined image "u-boot-with-spl.bin" is now automatically generated and >> can be programmed into NAND as one single image (vs. SPL image and U-Boot >> as 2 separate images). >> >> Signed-off-by: Stefan Roese <sr@denx.de> >> Cc: Heiko Schocher <hs@denx.de> >> Cc: Andreas Bießmann <andreas@biessmann.org> >> Cc: Eugen Hristev <eugen.hristev@microchip.com> >> Reviewed-by: Heiko Schocher <hs@denx.de> >> Tested on the taurus board: >> Tested-by: Heiko Schocher <hs@denx.de> >> --- >> v3: >> - No change >> >> v2: >> - No change >> >> Kconfig | 10 ++++++++++ >> Makefile | 4 +++- >> 2 files changed, 13 insertions(+), 1 deletion(-) >> >> diff --git a/Kconfig b/Kconfig >> index 305b265ed7..7c2b86f1f3 100644 >> --- a/Kconfig >> +++ b/Kconfig >> @@ -224,6 +224,15 @@ config BUILD_ROM >> which are not shipped in the U-Boot source tree. >> Please, see doc/README.x86 for details. >> >> +config SPL_IMAGE >> + string "SPL image filename that is generated" >> + default "spl/boot.bin" if ARCH_AT91 && SPL_NAND_SUPPORT >> + default "spl/u-boot-spl.bin" >> + help >> + The SPL image filename that is generated by the build process. >> + This image might be used to generated a combined image with >> + SPL and main U-Boot proper as well. >> + > > Hi Stefan, > > If I try to just use menuconfig and change this value to something else, > build fails. The purpose of this Kconfig is to name the SPL filename as > per the config's desire ? > > Or in fact is "which binary file to use to make the Combined SPL+U-BOOT > mega image" ? Its not meant to change the SPL image filename to an arbitrary name. Its meant to select one of the already supported values of the build system. I should probably re-phrase the Kconfig text to better reflect this. Thanks, Stefan
On 03.04.19 13:11, Eugen.Hristev@microchip.com wrote: > > > On 03.04.2019 14:03, Eugen Hristev wrote: >> >> >> On 02.04.2019 11:57, Stefan Roese wrote: >> >>> >>> This patch adds the CONFIG_SPL_IMAGE option to select the SPL image that >>> shall be used to generate the combined SPL + U-Boot image. The default >>> value is the current value "spl/u-boot-spl.bin". >>> >>> This patch also sets CONFIG_SPL_IMAGE to "spl/boot.bin" for AT91 targets >>> which use SPL NAND support (boot from NAND). For these build targets the >>> combined image "u-boot-with-spl.bin" is now automatically generated and >>> can be programmed into NAND as one single image (vs. SPL image and U-Boot >>> as 2 separate images). >>> >>> Signed-off-by: Stefan Roese <sr@denx.de> >>> Cc: Heiko Schocher <hs@denx.de> >>> Cc: Andreas Bießmann <andreas@biessmann.org> >>> Cc: Eugen Hristev <eugen.hristev@microchip.com> >>> Reviewed-by: Heiko Schocher <hs@denx.de> >>> Tested on the taurus board: >>> Tested-by: Heiko Schocher <hs@denx.de> >>> --- >>> v3: >>> - No change >>> >>> v2: >>> - No change >>> >>> Kconfig | 10 ++++++++++ >>> Makefile | 4 +++- >>> 2 files changed, 13 insertions(+), 1 deletion(-) >>> >>> diff --git a/Kconfig b/Kconfig >>> index 305b265ed7..7c2b86f1f3 100644 >>> --- a/Kconfig >>> +++ b/Kconfig >>> @@ -224,6 +224,15 @@ config BUILD_ROM >>> which are not shipped in the U-Boot source tree. >>> Please, see doc/README.x86 for details. >>> +config SPL_IMAGE >>> + string "SPL image filename that is generated" >>> + default "spl/boot.bin" if ARCH_AT91 && SPL_NAND_SUPPORT >>> + default "spl/u-boot-spl.bin" >>> + help >>> + The SPL image filename that is generated by the build process. >>> + This image might be used to generated a combined image with >>> + SPL and main U-Boot proper as well. >>> + >> >> Hi Stefan, >> >> If I try to just use menuconfig and change this value to something else, >> build fails. The purpose of this Kconfig is to name the SPL filename as >> per the config's desire ? >> >> Or in fact is "which binary file to use to make the Combined SPL+U-BOOT >> mega image" ? > > Ok, the commit message explains it pretty well, but the Kconfig help is > totally different... hence the confusion sorry. > So we need to either make a choice submenu in this Kconfig : either this > or that; or, do exactly what the Kconfig says: rename the output binary > into the value of this Kconfig. > > Does this make sense ? You are suggesting to add this SPL_IMAGE as choice instead of its current implementation? That might be better, yes. Let me look into this... Thanks, Stefan
Hi Eugen, On 03.04.19 13:21, Stefan Roese wrote: <snip> >>> If I try to just use menuconfig and change this value to something else, >>> build fails. The purpose of this Kconfig is to name the SPL filename as >>> per the config's desire ? >>> >>> Or in fact is "which binary file to use to make the Combined SPL+U-BOOT >>> mega image" ? >> >> Ok, the commit message explains it pretty well, but the Kconfig help is >> totally different... hence the confusion sorry. >> So we need to either make a choice submenu in this Kconfig : either this >> or that; or, do exactly what the Kconfig says: rename the output binary >> into the value of this Kconfig. >> >> Does this make sense ? > > You are suggesting to add this SPL_IMAGE as choice instead of its > current implementation? That might be better, yes. Let me look into > this... I did look into this and I would prefer to stay with the original implementation of the defaults. Changing this into a Kconfig choice makes this a bit more complex in the Kconfig. Additionally and even more important (for my personal feeling), the original implementation is more in line with the BUILD_TARGET implemenation directly below this new implementation. So if you don't object, I would like to send a new version with the original implementation but with a "better" (more correct) description in Kconfig. What do you think? Thanks, Stefan
On 03.04.2019 15:22, Stefan Roese wrote: > External E-Mail > > > Hi Eugen, > > On 03.04.19 13:21, Stefan Roese wrote: > > <snip> > >>>> If I try to just use menuconfig and change this value to something >>>> else, >>>> build fails. The purpose of this Kconfig is to name the SPL filename as >>>> per the config's desire ? >>>> >>>> Or in fact is "which binary file to use to make the Combined SPL+U-BOOT >>>> mega image" ? >>> >>> Ok, the commit message explains it pretty well, but the Kconfig help is >>> totally different... hence the confusion sorry. >>> So we need to either make a choice submenu in this Kconfig : either this >>> or that; or, do exactly what the Kconfig says: rename the output binary >>> into the value of this Kconfig. >>> >>> Does this make sense ? >> >> You are suggesting to add this SPL_IMAGE as choice instead of its >> current implementation? That might be better, yes. Let me look into >> this... > > I did look into this and I would prefer to stay with the original > implementation of the defaults. Changing this into a Kconfig choice > makes this a bit more complex in the Kconfig. Additionally and even > more important (for my personal feeling), the original implementation > is more in line with the BUILD_TARGET implemenation directly below > this new implementation. > > So if you don't object, I would like to send a new version with the > original implementation but with a "better" (more correct) > description in Kconfig. > > What do you think? This would imply that your new description has to state that if an incorrect binary name is configured here, the build will fail (?) > > Thanks, > Stefan
On 03.04.19 14:31, Eugen.Hristev@microchip.com wrote: > > > On 03.04.2019 15:22, Stefan Roese wrote: >> External E-Mail >> >> >> Hi Eugen, >> >> On 03.04.19 13:21, Stefan Roese wrote: >> >> <snip> >> >>>>> If I try to just use menuconfig and change this value to something >>>>> else, >>>>> build fails. The purpose of this Kconfig is to name the SPL filename as >>>>> per the config's desire ? >>>>> >>>>> Or in fact is "which binary file to use to make the Combined SPL+U-BOOT >>>>> mega image" ? >>>> >>>> Ok, the commit message explains it pretty well, but the Kconfig help is >>>> totally different... hence the confusion sorry. >>>> So we need to either make a choice submenu in this Kconfig : either this >>>> or that; or, do exactly what the Kconfig says: rename the output binary >>>> into the value of this Kconfig. >>>> >>>> Does this make sense ? >>> >>> You are suggesting to add this SPL_IMAGE as choice instead of its >>> current implementation? That might be better, yes. Let me look into >>> this... >> >> I did look into this and I would prefer to stay with the original >> implementation of the defaults. Changing this into a Kconfig choice >> makes this a bit more complex in the Kconfig. Additionally and even >> more important (for my personal feeling), the original implementation >> is more in line with the BUILD_TARGET implemenation directly below >> this new implementation. >> >> So if you don't object, I would like to send a new version with the >> original implementation but with a "better" (more correct) >> description in Kconfig. >> >> What do you think? > > This would imply that your new description has to state that if an > incorrect binary name is configured here, the build will fail (?) No. My main reasoning here is, that this is also not included in the description of "BUILD_TARGET" below. Here you can also change the selected value (via Kconfig help, no user input needed) to a new value that is not supported, which also results in a build error. I would like to not make this overly complex here. If a user wants to do something stupid by defining this (or some other Kconfig option) to an unsupported value, we can't really stop him. If you really think this is necessary, I will add a sentence to the Kconfig text, to only select "supported" values here. But again, I would prefer to not do this. Thanks, Stefan
On 03.04.2019 15:38, Stefan Roese wrote: > External E-Mail > > > On 03.04.19 14:31, Eugen.Hristev@microchip.com wrote: >> >> >> On 03.04.2019 15:22, Stefan Roese wrote: >>> External E-Mail >>> >>> >>> Hi Eugen, >>> >>> On 03.04.19 13:21, Stefan Roese wrote: >>> >>> <snip> >>> >>>>>> If I try to just use menuconfig and change this value to something >>>>>> else, >>>>>> build fails. The purpose of this Kconfig is to name the SPL >>>>>> filename as >>>>>> per the config's desire ? >>>>>> >>>>>> Or in fact is "which binary file to use to make the Combined >>>>>> SPL+U-BOOT >>>>>> mega image" ? >>>>> >>>>> Ok, the commit message explains it pretty well, but the Kconfig >>>>> help is >>>>> totally different... hence the confusion sorry. >>>>> So we need to either make a choice submenu in this Kconfig : either >>>>> this >>>>> or that; or, do exactly what the Kconfig says: rename the output >>>>> binary >>>>> into the value of this Kconfig. >>>>> >>>>> Does this make sense ? >>>> >>>> You are suggesting to add this SPL_IMAGE as choice instead of its >>>> current implementation? That might be better, yes. Let me look into >>>> this... >>> >>> I did look into this and I would prefer to stay with the original >>> implementation of the defaults. Changing this into a Kconfig choice >>> makes this a bit more complex in the Kconfig. Additionally and even >>> more important (for my personal feeling), the original implementation >>> is more in line with the BUILD_TARGET implemenation directly below >>> this new implementation. >>> >>> So if you don't object, I would like to send a new version with the >>> original implementation but with a "better" (more correct) >>> description in Kconfig. >>> >>> What do you think? >> >> This would imply that your new description has to state that if an >> incorrect binary name is configured here, the build will fail (?) > > No. My main reasoning here is, that this is also not included in the > description of "BUILD_TARGET" below. Here you can also change the > selected value (via Kconfig help, no user input needed) to a new value > that is not supported, which also results in a build error. > > I would like to not make this overly complex here. If a user wants > to do something stupid by defining this (or some other Kconfig > option) to an unsupported value, we can't really stop him. > > If you really think this is necessary, I will add a sentence to the > Kconfig text, to only select "supported" values here. But again, I > would prefer to not do this. > > Thanks, > Stefan Allright, but we have to make sure that everyone understands what this option does when they try to change it. So, a bit of rework of the help text is needed.
diff --git a/Kconfig b/Kconfig index 305b265ed7..7c2b86f1f3 100644 --- a/Kconfig +++ b/Kconfig @@ -224,6 +224,15 @@ config BUILD_ROM which are not shipped in the U-Boot source tree. Please, see doc/README.x86 for details. +config SPL_IMAGE + string "SPL image filename that is generated" + default "spl/boot.bin" if ARCH_AT91 && SPL_NAND_SUPPORT + default "spl/u-boot-spl.bin" + help + The SPL image filename that is generated by the build process. + This image might be used to generated a combined image with + SPL and main U-Boot proper as well. + config BUILD_TARGET string "Build target special images" default "u-boot-with-spl.sfp" if TARGET_SOCFPGA_ARRIA10 @@ -232,6 +241,7 @@ config BUILD_TARGET default "u-boot-elf.srec" if RCAR_GEN3 default "u-boot.itb" if SPL_LOAD_FIT && ARCH_SUNXI default "u-boot.kwb" if KIRKWOOD + default "u-boot-with-spl.bin" if ARCH_AT91 && SPL_NAND_SUPPORT help Some SoCs need special image types (e.g. U-Boot binary with a special header) as build targets. By defining diff --git a/Makefile b/Makefile index c1af9307b3..077bb6634e 100644 --- a/Makefile +++ b/Makefile @@ -1225,9 +1225,11 @@ else SPL_PAYLOAD := u-boot.bin endif +SPL_IMAGE := $(CONFIG_SPL_IMAGE:"%"=%) + OBJCOPYFLAGS_u-boot-with-spl.bin = -I binary -O binary \ --pad-to=$(CONFIG_SPL_PAD_TO) -u-boot-with-spl.bin: spl/u-boot-spl.bin $(SPL_PAYLOAD) FORCE +u-boot-with-spl.bin: $(SPL_IMAGE) $(SPL_PAYLOAD) FORCE $(call if_changed,pad_cat) ifeq ($(CONFIG_ARCH_LPC32XX)$(CONFIG_SPL),yy)