From mboxrd@z Thu Jan 1 00:00:00 1970 From: Derald D. Woods Date: Mon, 10 Dec 2018 07:01:15 -0600 Subject: [U-Boot] [PATCH] ARM: at91: Fix 'boot.bin' generation when CONFIG_SD_BOOT is enabled In-Reply-To: References: <20181208194905.21014-1-woods.technical@gmail.com> Message-ID: <20181210130114.GA7182@ethiopia> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Mon, Dec 10, 2018 at 08:32:33AM +0000, Eugen.Hristev at microchip.com wrote: > > > On 08.12.2018 21:49, Derald D. Woods wrote: > > On AT91 platforms configured for SD_BOOT, this commit avoids the > > generation of the PMECC header used for booting from NAND flash. This > > issue was found by attempting to boot the SAMA5D3-XPLD board with the > > 'sama5d3_xplained_mmc_defconfig'. > > > > [PMECC Reference] > > http://www.at91.com/linux4sam/bin/view/Linux4SAM/AT91Bootstrap > > > > [Mailing List Thread] > > https://lists.denx.de/pipermail/u-boot/2018-December/350666.html > > > > Fixes: 5541543f ("configs: at91: Remove CONFIG_SYS_EXTRA_OPTIONS assignment") > > Reported-by: Daniel Evans > > Cc: Robert Nelson > > Cc: Eugen Hristev > > Cc: Wenyou Yang > > Signed-off-by: Derald D. Woods > > --- > > scripts/Makefile.spl | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl > > index 22bd8f7c27..e727cb610f 100644 > > --- a/scripts/Makefile.spl > > +++ b/scripts/Makefile.spl > > @@ -166,10 +166,12 @@ ifeq ($(CONFIG_SYS_SOC),"at91") > > MKIMAGEFLAGS_boot.bin = -T atmelimage > > > > ifeq ($(CONFIG_SPL_GENERATE_ATMEL_PMECC_HEADER),y) > > +ifneq ($(CONFIG_SD_BOOT),y) > > Hi Derald, > > Thanks for your patch, however, I don't like that we do not use the > CONFIG_SPL_GENERATE_ATMEL_PMECC_HEADER anymore... isn't this config > supposed to say whether we are going to generate the header or not ? > That is not what is happening with this patch. SPL_GENERATE_ATMEL_PMECC_HEADER is not removed. The config still serves its orignal intent. If SD_BOOT is configured, then NAND is not being used. In this non-NAND case, the header is not needed. > Checking if "not sd-boot" doesn't look like a good option... we may use > SPI boot or QSPI or some other type at some point and the issue will > still be there. > This location and method would work for those nod-NAND cases also. See below. > I would rather fix the original patch by Wenyou, namely move the #ifdef > below to not have the GENERATE_ATMEL_PMECC enabled for SDBOOT. > > Does this sound good for you? > If this SPL_GENERATE_ATMEL_PMECC_HEADER only needs to be there for NAND, why not guard the 'ifdef ($(CONFIG_NAND_BOOT,y))'? Would this be better? Basically we could replace the 'ifneq ($(CONFIG_SD_BOOT),y)' with the more appropriate 'ifeq ($(CONFIG_NAND_BOOT),y)'. This would actually allow the future use-cases to be added as they become available and can be shown to actually boot with the header applied. I will put together a proper version 2 of my patch later today. [patch v2] ------------------------------------------------------------------------ diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl index 22bd8f7..e727cb6 100644 --- a/scripts/Makefile.spl +++ b/scripts/Makefile.spl @@ -166,10 +166,12 @@ ifeq ($(CONFIG_SYS_SOC),"at91") MKIMAGEFLAGS_boot.bin = -T atmelimage ifeq ($(CONFIG_SPL_GENERATE_ATMEL_PMECC_HEADER),y) +ifeq ($(CONFIG_NAND_BOOT),y) MKIMAGEFLAGS_boot.bin += -n $(shell $(obj)/../tools/atmel_pmecc_params) boot.bin: $(obj)/../tools/atmel_pmecc_params endif +endif boot.bin: $(obj)/u-boot-spl.bin FORCE $(call if_changed,mkimage) ------------------------------------------------------------------------ This would allow other configurations to 'opt-in' to applying the header. Which I think is the direction this is truly heading. Also, the SPL_GENERATE_ATMEL_PMECC_HEADER needs transitioning to Kconfig. Having the logic in "Makefile.spl" would help with that work too. Derald > Thanks again, > > Eugen > > > MKIMAGEFLAGS_boot.bin += -n $(shell $(obj)/../tools/atmel_pmecc_params) > > > > boot.bin: $(obj)/../tools/atmel_pmecc_params > > endif > > +endif > > > > boot.bin: $(obj)/u-boot-spl.bin FORCE > > $(call if_changed,mkimage) > >