| Message ID | 1501055837-15947-1-git-send-email-Ashish.Kumar@nxp.com |
|---|---|
| State | Superseded |
| Delegated to: | York Sun |
| Headers | show |
On 07/26/2017 12:57 AM, Ashish Kumar wrote: > Concatenation of u-boot-spl.bin and u-boot.img for NXP layerscape > platform SoC: LS1088A/LS2080A and their variants > > This patch also depricates UBOOT_BINLOAD in favour of SPL_PAYLOAD > > Signed-off-by: Ashish Kumar <Ashish.Kumar@nxp.com> > --- > > v2: > This is v2 version of https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.ozlabs.org%2Fpatch%2F755904%2F&data=01%7C01%7Cyork.sun%40nxp.com%7Cf780fc995f954b2198d308d4d3fbffb5%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0&sdata=kz47PT%2FPCGeBbhjJeucbreQefQFkUcTZqFSJ7vFRq60%3D&reserved=0 > > Also considers recommendation made in following mail chain > https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flinux.freescale.net%2Fpipermail%2Fu-boot%2F2016-September%2F041592.html&data=01%7C01%7Cyork.sun%40nxp.com%7Cf780fc995f954b2198d308d4d3fbffb5%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0&sdata=tds5BLj1lCHgJOfeJuJuFRsmNzOLZQHiOPKWHR%2BQqQg%3D&reserved=0 > to depricate UBOOT_BINLOAD > Bad practice to mix internal discussion thread. The archive is not accessible by external reviewers. > Makefile | 21 ++++++++++++++------- > 1 file changed, 14 insertions(+), 7 deletions(-) > > diff --git a/Makefile b/Makefile > index 4c4c8d8..ff22bda 100644 > --- a/Makefile > +++ b/Makefile > @@ -1030,8 +1030,20 @@ u-boot.dis: u-boot > ifdef CONFIG_TPL > SPL_PAYLOAD := tpl/u-boot-with-tpl.bin > else > +ifeq ($(ARCH),arm) > +ifdef CONFIG_OF_CONTROL > +SPL_PAYLOAD := u-boot-dtb.img > +else > +SPL_PAYLOAD := u-boot.img > +endif #ifdef CONFIG_OF_CONTROL > +else > +ifdef CONFIG_OF_CONTROL > +SPL_PAYLOAD := u-boot-dtb.bin > +else > SPL_PAYLOAD := u-boot.bin > -endif > +endif #ifdef CONFIG_OF_CONTROL > +endif #ifdef arm > +endif #ifdef CONFIG_TPL > Look like you are going back to check arch to decide raw format or not. Let's hear from other reviewers. York
On Wed, Jul 26, 2017 at 01:27:17PM +0530, Ashish Kumar wrote: > Concatenation of u-boot-spl.bin and u-boot.img for NXP layerscape > platform SoC: LS1088A/LS2080A and their variants > > This patch also depricates UBOOT_BINLOAD in favour of SPL_PAYLOAD > > Signed-off-by: Ashish Kumar <Ashish.Kumar@nxp.com> > --- > > v2: > This is v2 version of https://patchwork.ozlabs.org/patch/755904/ > > Also considers recommendation made in following mail chain > http://linux.freescale.net/pipermail/u-boot/2016-September/041592.html > to depricate UBOOT_BINLOAD > > Makefile | 21 ++++++++++++++------- > 1 file changed, 14 insertions(+), 7 deletions(-) > > diff --git a/Makefile b/Makefile > index 4c4c8d8..ff22bda 100644 > --- a/Makefile > +++ b/Makefile > @@ -1030,8 +1030,20 @@ u-boot.dis: u-boot > ifdef CONFIG_TPL > SPL_PAYLOAD := tpl/u-boot-with-tpl.bin > else > +ifeq ($(ARCH),arm) > +ifdef CONFIG_OF_CONTROL > +SPL_PAYLOAD := u-boot-dtb.img > +else > +SPL_PAYLOAD := u-boot.img > +endif #ifdef CONFIG_OF_CONTROL > +else > +ifdef CONFIG_OF_CONTROL > +SPL_PAYLOAD := u-boot-dtb.bin > +else > SPL_PAYLOAD := u-boot.bin ... so you're trying to make the logic such that on ARM we use .img and otherwise we use .bin ? Why are we not using .img everywhere? And I assume your else case is PowerPC, but I'm pretty sure other architectures are currently using .img as well. Thanks!
Hello Tom, Thanks for the comments, please see inline for respone. Regards Ashish -----Original Message----- From: Tom Rini [mailto:trini@konsulko.com] Sent: Thursday, July 27, 2017 3:36 AM To: Ashish Kumar <ashish.kumar@nxp.com> Cc: u-boot@lists.denx.de Subject: Re: [U-Boot] [Patch v2] Makefile: Concatenation of u-boot-spl.bin and u-boot.img On Wed, Jul 26, 2017 at 01:27:17PM +0530, Ashish Kumar wrote: > Concatenation of u-boot-spl.bin and u-boot.img for NXP layerscape > platform SoC: LS1088A/LS2080A and their variants > > This patch also depricates UBOOT_BINLOAD in favour of SPL_PAYLOAD > > Signed-off-by: Ashish Kumar <Ashish.Kumar@nxp.com> > --- > > v2: > This is v2 version of https://patchwork.ozlabs.org/patch/755904/ > > Also considers recommendation made in following mail chain > http://linux.freescale.net/pipermail/u-boot/2016-September/041592.html > to depricate UBOOT_BINLOAD > > Makefile | 21 ++++++++++++++------- > 1 file changed, 14 insertions(+), 7 deletions(-) > > diff --git a/Makefile b/Makefile > index 4c4c8d8..ff22bda 100644 > --- a/Makefile > +++ b/Makefile > @@ -1030,8 +1030,20 @@ u-boot.dis: u-boot > ifdef CONFIG_TPL > SPL_PAYLOAD := tpl/u-boot-with-tpl.bin else > +ifeq ($(ARCH),arm) > +ifdef CONFIG_OF_CONTROL > +SPL_PAYLOAD := u-boot-dtb.img > +else > +SPL_PAYLOAD := u-boot.img > +endif #ifdef CONFIG_OF_CONTROL > +else > +ifdef CONFIG_OF_CONTROL > +SPL_PAYLOAD := u-boot-dtb.bin > +else > SPL_PAYLOAD := u-boot.bin ... so you're trying to make the logic such that on ARM we use .img and otherwise we use .bin ? Why are we not using .img everywhere? And I assume your else case is PowerPC, but I'm pretty sure other architectures are currently using .img as well. Thanks! [Ashish Kumar] I am not sure what will break if I use .img for PowerPC. If other architectures are using .img as well, how about reversing the logic to check for powerpc and use .bin else use .img -- Tom
On Thu, Jul 27, 2017 at 04:30:53AM +0000, Ashish Kumar wrote: > Hello Tom, > > Thanks for the comments, please see inline for respone. > > Regards > Ashish > > -----Original Message----- > From: Tom Rini [mailto:trini@konsulko.com] > Sent: Thursday, July 27, 2017 3:36 AM > To: Ashish Kumar <ashish.kumar@nxp.com> > Cc: u-boot@lists.denx.de > Subject: Re: [U-Boot] [Patch v2] Makefile: Concatenation of u-boot-spl.bin and u-boot.img > > On Wed, Jul 26, 2017 at 01:27:17PM +0530, Ashish Kumar wrote: > > > Concatenation of u-boot-spl.bin and u-boot.img for NXP layerscape > > platform SoC: LS1088A/LS2080A and their variants > > > > This patch also depricates UBOOT_BINLOAD in favour of SPL_PAYLOAD > > > > Signed-off-by: Ashish Kumar <Ashish.Kumar@nxp.com> > > --- > > > > v2: > > This is v2 version of https://patchwork.ozlabs.org/patch/755904/ > > > > Also considers recommendation made in following mail chain > > http://linux.freescale.net/pipermail/u-boot/2016-September/041592.html > > to depricate UBOOT_BINLOAD > > > > Makefile | 21 ++++++++++++++------- > > 1 file changed, 14 insertions(+), 7 deletions(-) > > > > diff --git a/Makefile b/Makefile > > index 4c4c8d8..ff22bda 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -1030,8 +1030,20 @@ u-boot.dis: u-boot > > ifdef CONFIG_TPL > > SPL_PAYLOAD := tpl/u-boot-with-tpl.bin else > > +ifeq ($(ARCH),arm) > > +ifdef CONFIG_OF_CONTROL > > +SPL_PAYLOAD := u-boot-dtb.img > > +else > > +SPL_PAYLOAD := u-boot.img > > +endif #ifdef CONFIG_OF_CONTROL > > +else > > +ifdef CONFIG_OF_CONTROL > > +SPL_PAYLOAD := u-boot-dtb.bin > > +else > > SPL_PAYLOAD := u-boot.bin > > ... so you're trying to make the logic such that on ARM we use .img and otherwise we use .bin ? Why are we not using .img everywhere? And I assume your else case is PowerPC, but I'm pretty sure other architectures are currently using .img as well. Thanks! > [Ashish Kumar] I am not sure what will break if I use .img for PowerPC. > If other architectures are using .img as well, how about reversing the logic to check for powerpc and use .bin else use .img No, PowerPC should just use the .img like everyone else, and parse the "legacy" style header rather than jump to a raw location. Unless there's very very good reason, and then we need to make the logic that's doing this both clear and commented on, about why. Thanks!
diff --git a/Makefile b/Makefile index 4c4c8d8..ff22bda 100644 --- a/Makefile +++ b/Makefile @@ -1030,8 +1030,20 @@ u-boot.dis: u-boot ifdef CONFIG_TPL SPL_PAYLOAD := tpl/u-boot-with-tpl.bin else +ifeq ($(ARCH),arm) +ifdef CONFIG_OF_CONTROL +SPL_PAYLOAD := u-boot-dtb.img +else +SPL_PAYLOAD := u-boot.img +endif #ifdef CONFIG_OF_CONTROL +else +ifdef CONFIG_OF_CONTROL +SPL_PAYLOAD := u-boot-dtb.bin +else SPL_PAYLOAD := u-boot.bin -endif +endif #ifdef CONFIG_OF_CONTROL +endif #ifdef arm +endif #ifdef CONFIG_TPL OBJCOPYFLAGS_u-boot-with-spl.bin = -I binary -O binary \ --pad-to=$(CONFIG_SPL_PAD_TO) @@ -1194,16 +1206,11 @@ MKIMAGEFLAGS_u-boot-spl.pbl = -n $(srctree)/$(CONFIG_SYS_FSL_PBL_RCW:"%"=%) \ spl/u-boot-spl.pbl: spl/u-boot-spl.bin FORCE $(call if_changed,mkimage) -ifeq ($(ARCH),arm) -UBOOT_BINLOAD := u-boot.img -else -UBOOT_BINLOAD := u-boot.bin -endif OBJCOPYFLAGS_u-boot-with-spl-pbl.bin = -I binary -O binary --pad-to=$(CONFIG_SPL_PAD_TO) \ --gap-fill=0xff -u-boot-with-spl-pbl.bin: spl/u-boot-spl.pbl $(UBOOT_BINLOAD) FORCE +u-boot-with-spl-pbl.bin: spl/u-boot-spl.pbl $(SPL_PAYLOAD) FORCE $(call if_changed,pad_cat) # PPC4xx needs the SPL at the end of the image, since the reset vector
Concatenation of u-boot-spl.bin and u-boot.img for NXP layerscape platform SoC: LS1088A/LS2080A and their variants This patch also depricates UBOOT_BINLOAD in favour of SPL_PAYLOAD Signed-off-by: Ashish Kumar <Ashish.Kumar@nxp.com> --- v2: This is v2 version of https://patchwork.ozlabs.org/patch/755904/ Also considers recommendation made in following mail chain http://linux.freescale.net/pipermail/u-boot/2016-September/041592.html to depricate UBOOT_BINLOAD Makefile | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)