[U-Boot] [Patch v2] Makefile: Concatenation of u-boot-spl.bin and u-boot.img

[U-Boot] [Patch v2] Makefile: Concatenation of u-boot-spl.bin and u-boot.img

[Ashish Kumar]

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
-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
-- 
2.7.4

[U-Boot] [Patch v2] Makefile: Concatenation of u-boot-spl.bin and u-boot.img

[York Sun]

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

[U-Boot] [Patch v2] Makefile: Concatenation of u-boot-spl.bin and u-boot.img

[Tom Rini]

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!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170726/3d22cd57/attachment.sig>

[U-Boot] [Patch v2] Makefile: Concatenation of u-boot-spl.bin and u-boot.img

[Ashish Kumar]

Hello Tom,

Thanks for the comments, please see inline for respone.

Regards
Ashish

-----Original Message-----
From: Tom Rini [mailto:trini at konsulko.com] 
Sent: Thursday, July 27, 2017 3:36 AM
To: Ashish Kumar <ashish.kumar@nxp.com>
Cc: u-boot at 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

[U-Boot] [Patch v2] Makefile: Concatenation of u-boot-spl.bin and u-boot.img

[Tom Rini]

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 at konsulko.com] 
> Sent: Thursday, July 27, 2017 3:36 AM
> To: Ashish Kumar <ashish.kumar@nxp.com>
> Cc: u-boot at 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!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170727/7ecea309/attachment.sig>