[PATCH u-boot 1/2] powerpc/mpc85xx: socrates: Rename u-boot-socrates.bin to u-boot.bin

[PATCH u-boot 1/2] powerpc/mpc85xx: socrates: Rename u-boot-socrates.bin to u-boot.bin

[Pali Rohár]

U-Boot build process for socrates board produces final U-Boot binary in
file u-boot-socrates.bin (by binman) And as a bonus it produces two
unusable broken binaries u-boot-dtb.bin and u-boot.bin (by Makefile).

So do not build broken u-boot-dtb.bin and u-boot.bin binaries and rename
board specific u-boot-socrates.bin binary to u-boot.bin.

Renaming requires to define a new socrates specific Makefile target for
u-boot.bin (via binman) and also changing output name in socrates binman
config file.

With this change U-Boot build process for socrates board also produces
final U-Boot binary in file u-boot.bin.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 Makefile                              | 11 +++++++++++
 arch/powerpc/dts/socrates-u-boot.dtsi |  2 +-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index c977c906b036..117adb1cd8da 100644
--- a/Makefile
+++ b/Makefile
@@ -1201,22 +1201,30 @@ endif
 u-boot.bin: u-boot-fit-dtb.bin FORCE
 	$(call if_changed,copy)
 
+ifneq ($(CONFIG_TARGET_SOCRATES),y)
 u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
 	$(call if_changed,cat)
+endif
 
 else ifeq ($(CONFIG_OF_SEPARATE).$(CONFIG_OF_OMIT_DTB),y.)
+ifneq ($(CONFIG_TARGET_SOCRATES),y)
 u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
 	$(call if_changed,cat)
+endif
 
 ifneq ($(CONFIG_MPC85XX_HAVE_RESET_VECTOR)$(CONFIG_OF_SEPARATE),yy)
+ifneq ($(CONFIG_TARGET_SOCRATES),y)
 u-boot.bin: u-boot-dtb.bin FORCE
 	$(call if_changed,copy)
 endif
+endif
 
 else ifneq ($(CONFIG_MPC85XX_HAVE_RESET_VECTOR)$(CONFIG_OF_SEPARATE),yy)
+ifneq ($(CONFIG_TARGET_SOCRATES),y)
 u-boot.bin: u-boot-nodtb.bin FORCE
 	$(call if_changed,copy)
 endif
+endif
 
 # we call Makefile in arch/arm/mach-imx which
 # has targets which are dependent on targets defined
@@ -1601,6 +1609,9 @@ u-boot.bin: u-boot-nodtb.bin u-boot.dtb u-boot-br.bin FORCE
 OBJCOPYFLAGS_u-boot-br.bin := -O binary -j .bootpg -j .resetvec
 u-boot-br.bin: u-boot FORCE
 	$(call if_changed,objcopy)
+else ifeq ($(CONFIG_TARGET_SOCRATES),y)
+u-boot.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
+	$(call if_changed,binman)
 endif
 
 quiet_cmd_ldr = LD      $@
diff --git a/arch/powerpc/dts/socrates-u-boot.dtsi b/arch/powerpc/dts/socrates-u-boot.dtsi
index 14a7c245dc46..eff413f5b4a2 100644
--- a/arch/powerpc/dts/socrates-u-boot.dtsi
+++ b/arch/powerpc/dts/socrates-u-boot.dtsi
@@ -5,7 +5,7 @@
  */
 / {
 	binman {
-		filename = "u-boot-socrates.bin";
+		filename = "u-boot.bin";
 		pad-byte = <0xff>;
 		// Place dtb one sector before u-boot-nodtb.bin
 		blob {
-- 
2.20.1

[PATCH u-boot 2/2] Makefile: Build working u-boot-dtb.bin target also for mpc85xx

[Pali Rohár]

U-Boot build process currently always produces broken u-boot-dtb.bin binary
for PowerPC mpc85xx architecture on boards which needs mpc85xx reset
vector. For these boards this (intermediate) binary is not used as input
for any other Makefile target on this architecture, so there is no real
problem with it.

But it is not a good idea to produce broken binaries during build phase. So
try to improve it. Binary u-boot-dtb.bin should contains u-boot code with
DTB blob. Such binary for those boards is build by binman. So change binman
output file name from u-boot.bin to u-boot-dtb.bin and then let generic
Makefile rule to generate final u-boot.bin from u-boot-dtb.bin. And finally
disable generic u-boot-dtb.bin rule for mpc85xx.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 Makefile                              | 17 ++++++++---------
 arch/powerpc/dts/socrates-u-boot.dtsi |  2 +-
 arch/powerpc/dts/u-boot.dtsi          |  2 +-
 3 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/Makefile b/Makefile
index 117adb1cd8da..3c0f783f633e 100644
--- a/Makefile
+++ b/Makefile
@@ -1201,30 +1201,29 @@ endif
 u-boot.bin: u-boot-fit-dtb.bin FORCE
 	$(call if_changed,copy)
 
+ifneq ($(CONFIG_MPC85XX_HAVE_RESET_VECTOR)$(CONFIG_OF_SEPARATE),yy)
 ifneq ($(CONFIG_TARGET_SOCRATES),y)
 u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
 	$(call if_changed,cat)
 endif
+endif
 
 else ifeq ($(CONFIG_OF_SEPARATE).$(CONFIG_OF_OMIT_DTB),y.)
+
+ifneq ($(CONFIG_MPC85XX_HAVE_RESET_VECTOR)$(CONFIG_OF_SEPARATE),yy)
 ifneq ($(CONFIG_TARGET_SOCRATES),y)
 u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
 	$(call if_changed,cat)
 endif
+endif
 
-ifneq ($(CONFIG_MPC85XX_HAVE_RESET_VECTOR)$(CONFIG_OF_SEPARATE),yy)
-ifneq ($(CONFIG_TARGET_SOCRATES),y)
 u-boot.bin: u-boot-dtb.bin FORCE
 	$(call if_changed,copy)
-endif
-endif
 
-else ifneq ($(CONFIG_MPC85XX_HAVE_RESET_VECTOR)$(CONFIG_OF_SEPARATE),yy)
-ifneq ($(CONFIG_TARGET_SOCRATES),y)
+else
 u-boot.bin: u-boot-nodtb.bin FORCE
 	$(call if_changed,copy)
 endif
-endif
 
 # we call Makefile in arch/arm/mach-imx which
 # has targets which are dependent on targets defined
@@ -1603,14 +1602,14 @@ u-boot-with-nand-spl.sfp: u-boot-spl-padx4.sfp u-boot.img FORCE
 endif
 
 ifeq ($(CONFIG_MPC85XX_HAVE_RESET_VECTOR)$(CONFIG_OF_SEPARATE),yy)
-u-boot.bin: u-boot-nodtb.bin u-boot.dtb u-boot-br.bin FORCE
+u-boot-dtb.bin: u-boot-nodtb.bin u-boot.dtb u-boot-br.bin FORCE
 	$(call if_changed,binman)
 
 OBJCOPYFLAGS_u-boot-br.bin := -O binary -j .bootpg -j .resetvec
 u-boot-br.bin: u-boot FORCE
 	$(call if_changed,objcopy)
 else ifeq ($(CONFIG_TARGET_SOCRATES),y)
-u-boot.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
+u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
 	$(call if_changed,binman)
 endif
 
diff --git a/arch/powerpc/dts/socrates-u-boot.dtsi b/arch/powerpc/dts/socrates-u-boot.dtsi
index eff413f5b4a2..ba2e56d35675 100644
--- a/arch/powerpc/dts/socrates-u-boot.dtsi
+++ b/arch/powerpc/dts/socrates-u-boot.dtsi
@@ -5,7 +5,7 @@
  */
 / {
 	binman {
-		filename = "u-boot.bin";
+		filename = "u-boot-dtb.bin";
 		pad-byte = <0xff>;
 		// Place dtb one sector before u-boot-nodtb.bin
 		blob {
diff --git a/arch/powerpc/dts/u-boot.dtsi b/arch/powerpc/dts/u-boot.dtsi
index b4b5257362e5..b69c08808781 100644
--- a/arch/powerpc/dts/u-boot.dtsi
+++ b/arch/powerpc/dts/u-boot.dtsi
@@ -9,7 +9,7 @@
 
 / {
 	binman {
-		filename = "u-boot.bin";
+		filename = "u-boot-dtb.bin";
 		skip-at-start = <CONFIG_TEXT_BASE>;
 		sort-by-offset;
 		pad-byte = <0xff>;
-- 
2.20.1

Re: [PATCH u-boot 2/2] Makefile: Build working u-boot-dtb.bin target also for mpc85xx

[Simon Glass]

Hi Pali,

On Wed, 28 Dec 2022 at 14:06, Pali Rohár <pali@kernel.org> wrote:
>
> U-Boot build process currently always produces broken u-boot-dtb.bin binary
> for PowerPC mpc85xx architecture on boards which needs mpc85xx reset
> vector. For these boards this (intermediate) binary is not used as input
> for any other Makefile target on this architecture, so there is no real
> problem with it.
>
> But it is not a good idea to produce broken binaries during build phase. So
> try to improve it. Binary u-boot-dtb.bin should contains u-boot code with
> DTB blob. Such binary for those boards is build by binman. So change binman
> output file name from u-boot.bin to u-boot-dtb.bin and then let generic
> Makefile rule to generate final u-boot.bin from u-boot-dtb.bin. And finally
> disable generic u-boot-dtb.bin rule for mpc85xx.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  Makefile                              | 17 ++++++++---------
>  arch/powerpc/dts/socrates-u-boot.dtsi |  2 +-
>  arch/powerpc/dts/u-boot.dtsi          |  2 +-
>  3 files changed, 10 insertions(+), 11 deletions(-)

Can you instead use a new filename, like u-boot-powerpc.bin for this?

I'm not  fan of adding SoC-specific rules in the Makefile - in fact
one of the goals of binman is to drop these.

Regards,
Simon

Re: [PATCH u-boot 2/2] Makefile: Build working u-boot-dtb.bin target also for mpc85xx

[Pali Rohár]

On Thursday 29 December 2022 16:39:11 Simon Glass wrote:
> Hi Pali,
> 
> On Wed, 28 Dec 2022 at 14:06, Pali Rohár <pali@kernel.org> wrote:
> >
> > U-Boot build process currently always produces broken u-boot-dtb.bin binary
> > for PowerPC mpc85xx architecture on boards which needs mpc85xx reset
> > vector. For these boards this (intermediate) binary is not used as input
> > for any other Makefile target on this architecture, so there is no real
> > problem with it.
> >
> > But it is not a good idea to produce broken binaries during build phase. So
> > try to improve it. Binary u-boot-dtb.bin should contains u-boot code with
> > DTB blob. Such binary for those boards is build by binman. So change binman
> > output file name from u-boot.bin to u-boot-dtb.bin and then let generic
> > Makefile rule to generate final u-boot.bin from u-boot-dtb.bin. And finally
> > disable generic u-boot-dtb.bin rule for mpc85xx.
> >
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > ---
> >  Makefile                              | 17 ++++++++---------
> >  arch/powerpc/dts/socrates-u-boot.dtsi |  2 +-
> >  arch/powerpc/dts/u-boot.dtsi          |  2 +-
> >  3 files changed, 10 insertions(+), 11 deletions(-)
> 
> Can you instead use a new filename, like u-boot-powerpc.bin for this?

This would lead to the same situation as described here:
https://patchwork.ozlabs.org/project/uboot/patch/20221228181839.22003-1-pali@kernel.org/

> I'm not  fan of adding SoC-specific rules in the Makefile - in fact
> one of the goals of binman is to drop these.

In this case it would be better to build u-boot-dts.bin only by binman
(for all platforms) instead of cat-ing rules in Makefile.

Re: [PATCH u-boot 2/2] Makefile: Build working u-boot-dtb.bin target also for mpc85xx

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1962 bytes --]

On Fri, Dec 30, 2022 at 01:48:11PM +0100, Pali Rohár wrote:
> On Thursday 29 December 2022 16:39:11 Simon Glass wrote:
> > Hi Pali,
> > 
> > On Wed, 28 Dec 2022 at 14:06, Pali Rohár <pali@kernel.org> wrote:
> > >
> > > U-Boot build process currently always produces broken u-boot-dtb.bin binary
> > > for PowerPC mpc85xx architecture on boards which needs mpc85xx reset
> > > vector. For these boards this (intermediate) binary is not used as input
> > > for any other Makefile target on this architecture, so there is no real
> > > problem with it.
> > >
> > > But it is not a good idea to produce broken binaries during build phase. So
> > > try to improve it. Binary u-boot-dtb.bin should contains u-boot code with
> > > DTB blob. Such binary for those boards is build by binman. So change binman
> > > output file name from u-boot.bin to u-boot-dtb.bin and then let generic
> > > Makefile rule to generate final u-boot.bin from u-boot-dtb.bin. And finally
> > > disable generic u-boot-dtb.bin rule for mpc85xx.
> > >
> > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > ---
> > >  Makefile                              | 17 ++++++++---------
> > >  arch/powerpc/dts/socrates-u-boot.dtsi |  2 +-
> > >  arch/powerpc/dts/u-boot.dtsi          |  2 +-
> > >  3 files changed, 10 insertions(+), 11 deletions(-)
> > 
> > Can you instead use a new filename, like u-boot-powerpc.bin for this?
> 
> This would lead to the same situation as described here:
> https://patchwork.ozlabs.org/project/uboot/patch/20221228181839.22003-1-pali@kernel.org/
> 
> > I'm not  fan of adding SoC-specific rules in the Makefile - in fact
> > one of the goals of binman is to drop these.
> 
> In this case it would be better to build u-boot-dts.bin only by binman
> (for all platforms) instead of cat-ing rules in Makefile.

This would also be an easier path forward perhaps for making sure that
the dtb is always 8 byte aligned?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH u-boot 2/2] Makefile: Build working u-boot-dtb.bin target also for mpc85xx

[Pali Rohár]

On Friday 30 December 2022 10:21:04 Tom Rini wrote:
> > In this case it would be better to build u-boot-dts.bin only by binman
> > (for all platforms) instead of cat-ing rules in Makefile.
> 
> This would also be an easier path forward perhaps for making sure that
> the dtb is always 8 byte aligned?

Well, no. With DTB the problem is that it is not put to the correct
offset as can be specified in linker script. So moving this code from
Makefile to binman also moves this problem to another location.
8 byte alignment is just subset of the "correct offset" problem.

Re: [PATCH u-boot 2/2] Makefile: Build working u-boot-dtb.bin target also for mpc85xx

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 882 bytes --]

On Fri, Dec 30, 2022 at 04:24:43PM +0100, Pali Rohár wrote:
> On Friday 30 December 2022 10:21:04 Tom Rini wrote:
> > > In this case it would be better to build u-boot-dts.bin only by binman
> > > (for all platforms) instead of cat-ing rules in Makefile.
> > 
> > This would also be an easier path forward perhaps for making sure that
> > the dtb is always 8 byte aligned?
> 
> Well, no. With DTB the problem is that it is not put to the correct
> offset as can be specified in linker script. So moving this code from
> Makefile to binman also moves this problem to another location.
> 8 byte alignment is just subset of the "correct offset" problem.

Right, the high level answer is binman is intended to be the tool to
assemble binaries, and has to deal with "make sure binary X is at offset
Y, which also has a linker symbol for run-time references".

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH u-boot 2/2] Makefile: Build working u-boot-dtb.bin target also for mpc85xx

[Pali Rohár]

On Friday 30 December 2022 10:41:47 Tom Rini wrote:
> On Fri, Dec 30, 2022 at 04:24:43PM +0100, Pali Rohár wrote:
> > On Friday 30 December 2022 10:21:04 Tom Rini wrote:
> > > > In this case it would be better to build u-boot-dts.bin only by binman
> > > > (for all platforms) instead of cat-ing rules in Makefile.
> > > 
> > > This would also be an easier path forward perhaps for making sure that
> > > the dtb is always 8 byte aligned?
> > 
> > Well, no. With DTB the problem is that it is not put to the correct
> > offset as can be specified in linker script. So moving this code from
> > Makefile to binman also moves this problem to another location.
> > 8 byte alignment is just subset of the "correct offset" problem.
> 
> Right, the high level answer is binman is intended to be the tool to
> assemble binaries, and has to deal with "make sure binary X is at offset
> Y, which also has a linker symbol for run-time references".

Ok, if this tool has access to ELF/linker symbols (or will have in
future in case it does not have yet) then this problem could be solved
here.

Re: [PATCH u-boot 2/2] Makefile: Build working u-boot-dtb.bin target also for mpc85xx

[Simon Glass]

Hi Pali,

On Fri, 30 Dec 2022 at 09:44, Pali Rohár <pali@kernel.org> wrote:
>
> On Friday 30 December 2022 10:41:47 Tom Rini wrote:
> > On Fri, Dec 30, 2022 at 04:24:43PM +0100, Pali Rohár wrote:
> > > On Friday 30 December 2022 10:21:04 Tom Rini wrote:
> > > > > In this case it would be better to build u-boot-dts.bin only by binman
> > > > > (for all platforms) instead of cat-ing rules in Makefile.
> > > >
> > > > This would also be an easier path forward perhaps for making sure that
> > > > the dtb is always 8 byte aligned?
> > >
> > > Well, no. With DTB the problem is that it is not put to the correct
> > > offset as can be specified in linker script. So moving this code from
> > > Makefile to binman also moves this problem to another location.
> > > 8 byte alignment is just subset of the "correct offset" problem.
> >
> > Right, the high level answer is binman is intended to be the tool to
> > assemble binaries, and has to deal with "make sure binary X is at offset
> > Y, which also has a linker symbol for run-time references".
>
> Ok, if this tool has access to ELF/linker symbols (or will have in
> future in case it does not have yet) then this problem could be solved
> here.

It does have this access and already updates symbols in some cases.
See [1]. I am a little nervous about a complete move to binman in this
area even for simple things like u-boot.bin, since it would set off
yet another migration. But perhaps most boards don't actually use
u-boot.bin anyway?

Part of me thinks we should solve this in the .lds files, since
otherwise we are blurring the line between building and packaging.

Regards,
SImon

[1] https://u-boot.readthedocs.io/en/latest/develop/package/binman.html#access-to-binman-entry-offsets-at-run-time-symbols

Re: [PATCH u-boot 2/2] Makefile: Build working u-boot-dtb.bin target also for mpc85xx

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2001 bytes --]

On Fri, Dec 30, 2022 at 09:49:08AM -0600, Simon Glass wrote:
> Hi Pali,
> 
> On Fri, 30 Dec 2022 at 09:44, Pali Rohár <pali@kernel.org> wrote:
> >
> > On Friday 30 December 2022 10:41:47 Tom Rini wrote:
> > > On Fri, Dec 30, 2022 at 04:24:43PM +0100, Pali Rohár wrote:
> > > > On Friday 30 December 2022 10:21:04 Tom Rini wrote:
> > > > > > In this case it would be better to build u-boot-dts.bin only by binman
> > > > > > (for all platforms) instead of cat-ing rules in Makefile.
> > > > >
> > > > > This would also be an easier path forward perhaps for making sure that
> > > > > the dtb is always 8 byte aligned?
> > > >
> > > > Well, no. With DTB the problem is that it is not put to the correct
> > > > offset as can be specified in linker script. So moving this code from
> > > > Makefile to binman also moves this problem to another location.
> > > > 8 byte alignment is just subset of the "correct offset" problem.
> > >
> > > Right, the high level answer is binman is intended to be the tool to
> > > assemble binaries, and has to deal with "make sure binary X is at offset
> > > Y, which also has a linker symbol for run-time references".
> >
> > Ok, if this tool has access to ELF/linker symbols (or will have in
> > future in case it does not have yet) then this problem could be solved
> > here.
> 
> It does have this access and already updates symbols in some cases.
> See [1]. I am a little nervous about a complete move to binman in this
> area even for simple things like u-boot.bin, since it would set off
> yet another migration. But perhaps most boards don't actually use
> u-boot.bin anyway?

Well, we should just convert everyone at once since we should get
identical binaries before / after.

> Part of me thinks we should solve this in the .lds files, since
> otherwise we are blurring the line between building and packaging.

I thought the thread about fixing the alignment of the dtb noted
problems with that approach?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH u-boot 2/2] Makefile: Build working u-boot-dtb.bin target also for mpc85xx

[Pali Rohár]

On Friday 30 December 2022 09:49:08 Simon Glass wrote:
> Hi Pali,
> 
> On Fri, 30 Dec 2022 at 09:44, Pali Rohár <pali@kernel.org> wrote:
> >
> > On Friday 30 December 2022 10:41:47 Tom Rini wrote:
> > > On Fri, Dec 30, 2022 at 04:24:43PM +0100, Pali Rohár wrote:
> > > > On Friday 30 December 2022 10:21:04 Tom Rini wrote:
> > > > > > In this case it would be better to build u-boot-dts.bin only by binman
> > > > > > (for all platforms) instead of cat-ing rules in Makefile.
> > > > >
> > > > > This would also be an easier path forward perhaps for making sure that
> > > > > the dtb is always 8 byte aligned?
> > > >
> > > > Well, no. With DTB the problem is that it is not put to the correct
> > > > offset as can be specified in linker script. So moving this code from
> > > > Makefile to binman also moves this problem to another location.
> > > > 8 byte alignment is just subset of the "correct offset" problem.
> > >
> > > Right, the high level answer is binman is intended to be the tool to
> > > assemble binaries, and has to deal with "make sure binary X is at offset
> > > Y, which also has a linker symbol for run-time references".
> >
> > Ok, if this tool has access to ELF/linker symbols (or will have in
> > future in case it does not have yet) then this problem could be solved
> > here.
> 
> It does have this access and already updates symbols in some cases.
> See [1].
> [1] https://u-boot.readthedocs.io/en/latest/develop/package/binman.html#access-to-binman-entry-offsets-at-run-time-symbols

I just do not see how to do it, but ok, maybe something more is needed.

> I am a little nervous about a complete move to binman in this
> area even for simple things like u-boot.bin, since it would set off
> yet another migration.

Yes, it sounds like a big change.

> But perhaps most boards don't actually use u-boot.bin anyway?

I think that most boards _use_ u-boot.bin. Or wrap u-boot.bin into some
own container (by mkimage).

> Part of me thinks we should solve this in the .lds files, since
> otherwise we are blurring the line between building and packaging.

Linker script files in any case would have to be adjusted / fixed to
align _end symbol. Without it ELF symbol would not be correct and so
obviously any solution depending on ELF symbols would not work...

As I wrote recently, I proposed alternative solution without binman:
https://lore.kernel.org/u-boot/20221217235913.w7ihsktbplbp2j7z@pali/

> Regards,
> SImon
> 

Re: [PATCH u-boot 2/2] Makefile: Build working u-boot-dtb.bin target also for mpc85xx

[Simon Glass]

Hi Pali,

On Fri, 30 Dec 2022 at 10:06, Pali Rohár <pali@kernel.org> wrote:
>
> On Friday 30 December 2022 09:49:08 Simon Glass wrote:
> > Hi Pali,
> >
> > On Fri, 30 Dec 2022 at 09:44, Pali Rohár <pali@kernel.org> wrote:
> > >
> > > On Friday 30 December 2022 10:41:47 Tom Rini wrote:
> > > > On Fri, Dec 30, 2022 at 04:24:43PM +0100, Pali Rohár wrote:
> > > > > On Friday 30 December 2022 10:21:04 Tom Rini wrote:
> > > > > > > In this case it would be better to build u-boot-dts.bin only by binman
> > > > > > > (for all platforms) instead of cat-ing rules in Makefile.
> > > > > >
> > > > > > This would also be an easier path forward perhaps for making sure that
> > > > > > the dtb is always 8 byte aligned?
> > > > >
> > > > > Well, no. With DTB the problem is that it is not put to the correct
> > > > > offset as can be specified in linker script. So moving this code from
> > > > > Makefile to binman also moves this problem to another location.
> > > > > 8 byte alignment is just subset of the "correct offset" problem.
> > > >
> > > > Right, the high level answer is binman is intended to be the tool to
> > > > assemble binaries, and has to deal with "make sure binary X is at offset
> > > > Y, which also has a linker symbol for run-time references".
> > >
> > > Ok, if this tool has access to ELF/linker symbols (or will have in
> > > future in case it does not have yet) then this problem could be solved
> > > here.
> >
> > It does have this access and already updates symbols in some cases.
> > See [1].
> > [1] https://u-boot.readthedocs.io/en/latest/develop/package/binman.html#access-to-binman-entry-offsets-at-run-time-symbols
>
> I just do not see how to do it, but ok, maybe something more is needed.
>
> > I am a little nervous about a complete move to binman in this
> > area even for simple things like u-boot.bin, since it would set off
> > yet another migration.
>
> Yes, it sounds like a big change.
>
> > But perhaps most boards don't actually use u-boot.bin anyway?
>
> I think that most boards _use_ u-boot.bin. Or wrap u-boot.bin into some
> own container (by mkimage).
>
> > Part of me thinks we should solve this in the .lds files, since
> > otherwise we are blurring the line between building and packaging.
>
> Linker script files in any case would have to be adjusted / fixed to
> align _end symbol. Without it ELF symbol would not be correct and so
> obviously any solution depending on ELF symbols would not work...
>
> As I wrote recently, I proposed alternative solution without binman:
> https://lore.kernel.org/u-boot/20221217235913.w7ihsktbplbp2j7z@pali/

I think that would work. It is a little like the BINARY_SIZE_CHECK
thing we have. It is the closest thing to what we have. As you saw I
was happy with your original 'trunc' solution too.

After some more thought, perhaps the binman solution makes sense, so
long as we do what you say above (make the _end symbol aligned). Of
course binman can fix that up, but it feels more like a build thing
than a packaging thing to me. It could be quite confusing for people
to see a symbol change between build-time and run-time.

So the steps would be something like this:

1. Update the align before _end in each .lds to use a constant like
#define END_ALIGN  8
2. Update binman's dtb etypes to align to 8 bytes
3. Add a common binman image for u-boot.bin (used by every board)
4. Enable binman by default
5. Drop the current 'cat' rules in Makefile, so that only u-boot and
u-boot-nodtb.bin are produced
6. Optionally consider moving .img files to binman also

Regards,
Simon

Re: [PATCH u-boot 2/2] Makefile: Build working u-boot-dtb.bin target also for mpc85xx

[Pali Rohár]

On Friday 30 December 2022 11:43:44 Simon Glass wrote:
> Hi Pali,
> 
> On Fri, 30 Dec 2022 at 10:06, Pali Rohár <pali@kernel.org> wrote:
> >
> > On Friday 30 December 2022 09:49:08 Simon Glass wrote:
> > > Hi Pali,
> > >
> > > On Fri, 30 Dec 2022 at 09:44, Pali Rohár <pali@kernel.org> wrote:
> > > >
> > > > On Friday 30 December 2022 10:41:47 Tom Rini wrote:
> > > > > On Fri, Dec 30, 2022 at 04:24:43PM +0100, Pali Rohár wrote:
> > > > > > On Friday 30 December 2022 10:21:04 Tom Rini wrote:
> > > > > > > > In this case it would be better to build u-boot-dts.bin only by binman
> > > > > > > > (for all platforms) instead of cat-ing rules in Makefile.
> > > > > > >
> > > > > > > This would also be an easier path forward perhaps for making sure that
> > > > > > > the dtb is always 8 byte aligned?
> > > > > >
> > > > > > Well, no. With DTB the problem is that it is not put to the correct
> > > > > > offset as can be specified in linker script. So moving this code from
> > > > > > Makefile to binman also moves this problem to another location.
> > > > > > 8 byte alignment is just subset of the "correct offset" problem.
> > > > >
> > > > > Right, the high level answer is binman is intended to be the tool to
> > > > > assemble binaries, and has to deal with "make sure binary X is at offset
> > > > > Y, which also has a linker symbol for run-time references".
> > > >
> > > > Ok, if this tool has access to ELF/linker symbols (or will have in
> > > > future in case it does not have yet) then this problem could be solved
> > > > here.
> > >
> > > It does have this access and already updates symbols in some cases.
> > > See [1].
> > > [1] https://u-boot.readthedocs.io/en/latest/develop/package/binman.html#access-to-binman-entry-offsets-at-run-time-symbols
> >
> > I just do not see how to do it, but ok, maybe something more is needed.
> >
> > > I am a little nervous about a complete move to binman in this
> > > area even for simple things like u-boot.bin, since it would set off
> > > yet another migration.
> >
> > Yes, it sounds like a big change.
> >
> > > But perhaps most boards don't actually use u-boot.bin anyway?
> >
> > I think that most boards _use_ u-boot.bin. Or wrap u-boot.bin into some
> > own container (by mkimage).
> >
> > > Part of me thinks we should solve this in the .lds files, since
> > > otherwise we are blurring the line between building and packaging.
> >
> > Linker script files in any case would have to be adjusted / fixed to
> > align _end symbol. Without it ELF symbol would not be correct and so
> > obviously any solution depending on ELF symbols would not work...
> >
> > As I wrote recently, I proposed alternative solution without binman:
> > https://lore.kernel.org/u-boot/20221217235913.w7ihsktbplbp2j7z@pali/
> 
> I think that would work. It is a little like the BINARY_SIZE_CHECK
> thing we have. It is the closest thing to what we have. As you saw I
> was happy with your original 'trunc' solution too.
> 
> After some more thought, perhaps the binman solution makes sense, so
> long as we do what you say above (make the _end symbol aligned). Of
> course binman can fix that up, but it feels more like a build thing
> than a packaging thing to me.

Yes, this is build thing/issue, not packaging one.

> It could be quite confusing for people
> to see a symbol change between build-time and run-time.

Yes, symbol change is confusing. So I would propose to not change any
symbol between build and run time.

> So the steps would be something like this:
> 
> 1. Update the align before _end in each .lds to use a constant like
> #define END_ALIGN  8
> 2. Update binman's dtb etypes to align to 8 bytes

This is not enough. Some boards/platform may have stricter alignment
(e.g. to SD card sector size = 512 bytes). So binman should not align
image and instead of that, it should read _exact_ address of _end
symbol and put DTB etype to this address. Step 1. already ensures that
_end is aligned to 8 bytes.

> 3. Add a common binman image for u-boot.bin (used by every board)

It should be u-boot-dtb.bin (not u-boot.bin). At least this is the
current file name. (See this my patch series again, which aligns this
naming also for powerpc/mpc85xx).

> 4. Enable binman by default
> 5. Drop the current 'cat' rules in Makefile, so that only u-boot and
> u-boot-nodtb.bin are produced

(cat rule is only for u-boot-dtb.bin)

> 6. Optionally consider moving .img files to binman also
> 
> Regards,
> Simon

With slightly modified/improved step 2. I agree that this improves
current situation. And should these issues.

Re: [PATCH u-boot 2/2] Makefile: Build working u-boot-dtb.bin target also for mpc85xx

[Simon Glass]

Hi Pali,

On Fri, 30 Dec 2022 at 11:55, Pali Rohár <pali@kernel.org> wrote:
>
> On Friday 30 December 2022 11:43:44 Simon Glass wrote:
> > Hi Pali,
> >
> > On Fri, 30 Dec 2022 at 10:06, Pali Rohár <pali@kernel.org> wrote:
> > >
> > > On Friday 30 December 2022 09:49:08 Simon Glass wrote:
> > > > Hi Pali,
> > > >
> > > > On Fri, 30 Dec 2022 at 09:44, Pali Rohár <pali@kernel.org> wrote:
> > > > >
> > > > > On Friday 30 December 2022 10:41:47 Tom Rini wrote:
> > > > > > On Fri, Dec 30, 2022 at 04:24:43PM +0100, Pali Rohár wrote:
> > > > > > > On Friday 30 December 2022 10:21:04 Tom Rini wrote:
> > > > > > > > > In this case it would be better to build u-boot-dts.bin only by binman
> > > > > > > > > (for all platforms) instead of cat-ing rules in Makefile.
> > > > > > > >
> > > > > > > > This would also be an easier path forward perhaps for making sure that
> > > > > > > > the dtb is always 8 byte aligned?
> > > > > > >
> > > > > > > Well, no. With DTB the problem is that it is not put to the correct
> > > > > > > offset as can be specified in linker script. So moving this code from
> > > > > > > Makefile to binman also moves this problem to another location.
> > > > > > > 8 byte alignment is just subset of the "correct offset" problem.
> > > > > >
> > > > > > Right, the high level answer is binman is intended to be the tool to
> > > > > > assemble binaries, and has to deal with "make sure binary X is at offset
> > > > > > Y, which also has a linker symbol for run-time references".
> > > > >
> > > > > Ok, if this tool has access to ELF/linker symbols (or will have in
> > > > > future in case it does not have yet) then this problem could be solved
> > > > > here.
> > > >
> > > > It does have this access and already updates symbols in some cases.
> > > > See [1].
> > > > [1] https://u-boot.readthedocs.io/en/latest/develop/package/binman.html#access-to-binman-entry-offsets-at-run-time-symbols
> > >
> > > I just do not see how to do it, but ok, maybe something more is needed.
> > >
> > > > I am a little nervous about a complete move to binman in this
> > > > area even for simple things like u-boot.bin, since it would set off
> > > > yet another migration.
> > >
> > > Yes, it sounds like a big change.
> > >
> > > > But perhaps most boards don't actually use u-boot.bin anyway?
> > >
> > > I think that most boards _use_ u-boot.bin. Or wrap u-boot.bin into some
> > > own container (by mkimage).
> > >
> > > > Part of me thinks we should solve this in the .lds files, since
> > > > otherwise we are blurring the line between building and packaging.
> > >
> > > Linker script files in any case would have to be adjusted / fixed to
> > > align _end symbol. Without it ELF symbol would not be correct and so
> > > obviously any solution depending on ELF symbols would not work...
> > >
> > > As I wrote recently, I proposed alternative solution without binman:
> > > https://lore.kernel.org/u-boot/20221217235913.w7ihsktbplbp2j7z@pali/
> >
> > I think that would work. It is a little like the BINARY_SIZE_CHECK
> > thing we have. It is the closest thing to what we have. As you saw I
> > was happy with your original 'trunc' solution too.
> >
> > After some more thought, perhaps the binman solution makes sense, so
> > long as we do what you say above (make the _end symbol aligned). Of
> > course binman can fix that up, but it feels more like a build thing
> > than a packaging thing to me.
>
> Yes, this is build thing/issue, not packaging one.
>
> > It could be quite confusing for people
> > to see a symbol change between build-time and run-time.
>
> Yes, symbol change is confusing. So I would propose to not change any
> symbol between build and run time.
>
> > So the steps would be something like this:
> >
> > 1. Update the align before _end in each .lds to use a constant like
> > #define END_ALIGN  8
> > 2. Update binman's dtb etypes to align to 8 bytes
>
> This is not enough. Some boards/platform may have stricter alignment
> (e.g. to SD card sector size = 512 bytes). So binman should not align
> image and instead of that, it should read _exact_ address of _end
> symbol and put DTB etype to this address. Step 1. already ensures that
> _end is aligned to 8 bytes.

OK, we can do that I suppose, perhaps with a new binman property:

u_boot: u-boot-nodtb {
};
u-boot-dtb {
   align-to-sym = <&u_boot> ,"_end";
};

or using expanding just:

u-boot {
   align-dtb-to-sym = <&u_boot> ,"_end";
};

(which we could perhaps make the default?)

>
> > 3. Add a common binman image for u-boot.bin (used by every board)
>
> It should be u-boot-dtb.bin (not u-boot.bin). At least this is the
> current file name. (See this my patch series again, which aligns this
> naming also for powerpc/mpc85xx).

We changed this 6 years ago and I'm not keen on going back.

ad1ecd2063d fdt: Build a U-Boot binary without device tree

>
> > 4. Enable binman by default
> > 5. Drop the current 'cat' rules in Makefile, so that only u-boot and
> > u-boot-nodtb.bin are produced
>
> (cat rule is only for u-boot-dtb.bin)
>
> > 6. Optionally consider moving .img files to binman also
> >
> > Regards,
> > Simon
>
> With slightly modified/improved step 2. I agree that this improves
> current situation. And should these issues.

Regards,
Simon


[1] https://u-boot.readthedocs.io/en/latest/develop/package/binman.html#expanded-entries

Re: [PATCH u-boot 2/2] Makefile: Build working u-boot-dtb.bin target also for mpc85xx

[Pali Rohár]

On Friday 30 December 2022 12:51:03 Simon Glass wrote:
> Hi Pali,
> 
> On Fri, 30 Dec 2022 at 11:55, Pali Rohár <pali@kernel.org> wrote:
> >
> > On Friday 30 December 2022 11:43:44 Simon Glass wrote:
> > > Hi Pali,
> > >
> > > On Fri, 30 Dec 2022 at 10:06, Pali Rohár <pali@kernel.org> wrote:
> > > >
> > > > On Friday 30 December 2022 09:49:08 Simon Glass wrote:
> > > > > Hi Pali,
> > > > >
> > > > > On Fri, 30 Dec 2022 at 09:44, Pali Rohár <pali@kernel.org> wrote:
> > > > > >
> > > > > > On Friday 30 December 2022 10:41:47 Tom Rini wrote:
> > > > > > > On Fri, Dec 30, 2022 at 04:24:43PM +0100, Pali Rohár wrote:
> > > > > > > > On Friday 30 December 2022 10:21:04 Tom Rini wrote:
> > > > > > > > > > In this case it would be better to build u-boot-dts.bin only by binman
> > > > > > > > > > (for all platforms) instead of cat-ing rules in Makefile.
> > > > > > > > >
> > > > > > > > > This would also be an easier path forward perhaps for making sure that
> > > > > > > > > the dtb is always 8 byte aligned?
> > > > > > > >
> > > > > > > > Well, no. With DTB the problem is that it is not put to the correct
> > > > > > > > offset as can be specified in linker script. So moving this code from
> > > > > > > > Makefile to binman also moves this problem to another location.
> > > > > > > > 8 byte alignment is just subset of the "correct offset" problem.
> > > > > > >
> > > > > > > Right, the high level answer is binman is intended to be the tool to
> > > > > > > assemble binaries, and has to deal with "make sure binary X is at offset
> > > > > > > Y, which also has a linker symbol for run-time references".
> > > > > >
> > > > > > Ok, if this tool has access to ELF/linker symbols (or will have in
> > > > > > future in case it does not have yet) then this problem could be solved
> > > > > > here.
> > > > >
> > > > > It does have this access and already updates symbols in some cases.
> > > > > See [1].
> > > > > [1] https://u-boot.readthedocs.io/en/latest/develop/package/binman.html#access-to-binman-entry-offsets-at-run-time-symbols
> > > >
> > > > I just do not see how to do it, but ok, maybe something more is needed.
> > > >
> > > > > I am a little nervous about a complete move to binman in this
> > > > > area even for simple things like u-boot.bin, since it would set off
> > > > > yet another migration.
> > > >
> > > > Yes, it sounds like a big change.
> > > >
> > > > > But perhaps most boards don't actually use u-boot.bin anyway?
> > > >
> > > > I think that most boards _use_ u-boot.bin. Or wrap u-boot.bin into some
> > > > own container (by mkimage).
> > > >
> > > > > Part of me thinks we should solve this in the .lds files, since
> > > > > otherwise we are blurring the line between building and packaging.
> > > >
> > > > Linker script files in any case would have to be adjusted / fixed to
> > > > align _end symbol. Without it ELF symbol would not be correct and so
> > > > obviously any solution depending on ELF symbols would not work...
> > > >
> > > > As I wrote recently, I proposed alternative solution without binman:
> > > > https://lore.kernel.org/u-boot/20221217235913.w7ihsktbplbp2j7z@pali/
> > >
> > > I think that would work. It is a little like the BINARY_SIZE_CHECK
> > > thing we have. It is the closest thing to what we have. As you saw I
> > > was happy with your original 'trunc' solution too.
> > >
> > > After some more thought, perhaps the binman solution makes sense, so
> > > long as we do what you say above (make the _end symbol aligned). Of
> > > course binman can fix that up, but it feels more like a build thing
> > > than a packaging thing to me.
> >
> > Yes, this is build thing/issue, not packaging one.
> >
> > > It could be quite confusing for people
> > > to see a symbol change between build-time and run-time.
> >
> > Yes, symbol change is confusing. So I would propose to not change any
> > symbol between build and run time.
> >
> > > So the steps would be something like this:
> > >
> > > 1. Update the align before _end in each .lds to use a constant like
> > > #define END_ALIGN  8
> > > 2. Update binman's dtb etypes to align to 8 bytes
> >
> > This is not enough. Some boards/platform may have stricter alignment
> > (e.g. to SD card sector size = 512 bytes). So binman should not align
> > image and instead of that, it should read _exact_ address of _end
> > symbol and put DTB etype to this address. Step 1. already ensures that
> > _end is aligned to 8 bytes.
> 
> OK, we can do that I suppose, perhaps with a new binman property:
> 
> u_boot: u-boot-nodtb {
> };
> u-boot-dtb {
>    align-to-sym = <&u_boot> ,"_end";
> };
> 
> or using expanding just:
> 
> u-boot {
>    align-dtb-to-sym = <&u_boot> ,"_end";
> };
> 
> (which we could perhaps make the default?)

Something like that looks reasonable. And must be enabled by default.

I'm not sure if calling it "align" is a good idea. Because if property
is in u-boot node then it it is "padding". And if that property is in
dtb node then it is "placing" binary to the absolute address.

From reading binman.html documentation I see that for placing image at
absolute address there is already "offset" property. And for padding
there is "pad-after" property. What about consistency?

So maybe 'pad-after-to-sym = "_end";' in u-boot node?
Or 'offset-from-sym = <&u_boot>, "_end";' in dtb node?

> >
> > > 3. Add a common binman image for u-boot.bin (used by every board)
> >
> > It should be u-boot-dtb.bin (not u-boot.bin). At least this is the
> > current file name. (See this my patch series again, which aligns this
> > naming also for powerpc/mpc85xx).
> 
> We changed this 6 years ago and I'm not keen on going back.
> 
> ad1ecd2063d fdt: Build a U-Boot binary without device tree

I just do not understand you because in that commit is exactly what I
wrote. In file u-boot-dtb.bin is u-boot binary with DTB and in file
u-boot-nodtb.bin is u-boot binary without DTB.

So binman should take input files u-boot-nodtb.bin and DTB binary. And
should produce output file u-boot-dtb.bin. Is there any issue with it?

> >
> > > 4. Enable binman by default
> > > 5. Drop the current 'cat' rules in Makefile, so that only u-boot and
> > > u-boot-nodtb.bin are produced
> >
> > (cat rule is only for u-boot-dtb.bin)
> >
> > > 6. Optionally consider moving .img files to binman also
> > >
> > > Regards,
> > > Simon
> >
> > With slightly modified/improved step 2. I agree that this improves
> > current situation. And should these issues.
> 
> Regards,
> Simon
> 
> 
> [1] https://u-boot.readthedocs.io/en/latest/develop/package/binman.html#expanded-entries

Re: [PATCH u-boot 2/2] Makefile: Build working u-boot-dtb.bin target also for mpc85xx

[Simon Glass]

Hi Pali,

On Fri, 30 Dec 2022 at 13:12, Pali Rohár <pali@kernel.org> wrote:
>
> On Friday 30 December 2022 12:51:03 Simon Glass wrote:
> > Hi Pali,
> >
> > On Fri, 30 Dec 2022 at 11:55, Pali Rohár <pali@kernel.org> wrote:
> > >
> > > On Friday 30 December 2022 11:43:44 Simon Glass wrote:
> > > > Hi Pali,
> > > >
> > > > On Fri, 30 Dec 2022 at 10:06, Pali Rohár <pali@kernel.org> wrote:
> > > > >
> > > > > On Friday 30 December 2022 09:49:08 Simon Glass wrote:
> > > > > > Hi Pali,
> > > > > >
> > > > > > On Fri, 30 Dec 2022 at 09:44, Pali Rohár <pali@kernel.org> wrote:
> > > > > > >
> > > > > > > On Friday 30 December 2022 10:41:47 Tom Rini wrote:
> > > > > > > > On Fri, Dec 30, 2022 at 04:24:43PM +0100, Pali Rohár wrote:
> > > > > > > > > On Friday 30 December 2022 10:21:04 Tom Rini wrote:
> > > > > > > > > > > In this case it would be better to build u-boot-dts.bin only by binman
> > > > > > > > > > > (for all platforms) instead of cat-ing rules in Makefile.
> > > > > > > > > >
> > > > > > > > > > This would also be an easier path forward perhaps for making sure that
> > > > > > > > > > the dtb is always 8 byte aligned?
> > > > > > > > >
> > > > > > > > > Well, no. With DTB the problem is that it is not put to the correct
> > > > > > > > > offset as can be specified in linker script. So moving this code from
> > > > > > > > > Makefile to binman also moves this problem to another location.
> > > > > > > > > 8 byte alignment is just subset of the "correct offset" problem.
> > > > > > > >
> > > > > > > > Right, the high level answer is binman is intended to be the tool to
> > > > > > > > assemble binaries, and has to deal with "make sure binary X is at offset
> > > > > > > > Y, which also has a linker symbol for run-time references".
> > > > > > >
> > > > > > > Ok, if this tool has access to ELF/linker symbols (or will have in
> > > > > > > future in case it does not have yet) then this problem could be solved
> > > > > > > here.
> > > > > >
> > > > > > It does have this access and already updates symbols in some cases.
> > > > > > See [1].
> > > > > > [1] https://u-boot.readthedocs.io/en/latest/develop/package/binman.html#access-to-binman-entry-offsets-at-run-time-symbols
> > > > >
> > > > > I just do not see how to do it, but ok, maybe something more is needed.
> > > > >
> > > > > > I am a little nervous about a complete move to binman in this
> > > > > > area even for simple things like u-boot.bin, since it would set off
> > > > > > yet another migration.
> > > > >
> > > > > Yes, it sounds like a big change.
> > > > >
> > > > > > But perhaps most boards don't actually use u-boot.bin anyway?
> > > > >
> > > > > I think that most boards _use_ u-boot.bin. Or wrap u-boot.bin into some
> > > > > own container (by mkimage).
> > > > >
> > > > > > Part of me thinks we should solve this in the .lds files, since
> > > > > > otherwise we are blurring the line between building and packaging.
> > > > >
> > > > > Linker script files in any case would have to be adjusted / fixed to
> > > > > align _end symbol. Without it ELF symbol would not be correct and so
> > > > > obviously any solution depending on ELF symbols would not work...
> > > > >
> > > > > As I wrote recently, I proposed alternative solution without binman:
> > > > > https://lore.kernel.org/u-boot/20221217235913.w7ihsktbplbp2j7z@pali/
> > > >
> > > > I think that would work. It is a little like the BINARY_SIZE_CHECK
> > > > thing we have. It is the closest thing to what we have. As you saw I
> > > > was happy with your original 'trunc' solution too.
> > > >
> > > > After some more thought, perhaps the binman solution makes sense, so
> > > > long as we do what you say above (make the _end symbol aligned). Of
> > > > course binman can fix that up, but it feels more like a build thing
> > > > than a packaging thing to me.
> > >
> > > Yes, this is build thing/issue, not packaging one.
> > >
> > > > It could be quite confusing for people
> > > > to see a symbol change between build-time and run-time.
> > >
> > > Yes, symbol change is confusing. So I would propose to not change any
> > > symbol between build and run time.
> > >
> > > > So the steps would be something like this:
> > > >
> > > > 1. Update the align before _end in each .lds to use a constant like
> > > > #define END_ALIGN  8
> > > > 2. Update binman's dtb etypes to align to 8 bytes
> > >
> > > This is not enough. Some boards/platform may have stricter alignment
> > > (e.g. to SD card sector size = 512 bytes). So binman should not align
> > > image and instead of that, it should read _exact_ address of _end
> > > symbol and put DTB etype to this address. Step 1. already ensures that
> > > _end is aligned to 8 bytes.
> >
> > OK, we can do that I suppose, perhaps with a new binman property:
> >
> > u_boot: u-boot-nodtb {
> > };
> > u-boot-dtb {
> >    align-to-sym = <&u_boot> ,"_end";
> > };
> >
> > or using expanding just:
> >
> > u-boot {
> >    align-dtb-to-sym = <&u_boot> ,"_end";
> > };
> >
> > (which we could perhaps make the default?)
>
> Something like that looks reasonable. And must be enabled by default.
>
> I'm not sure if calling it "align" is a good idea. Because if property
> is in u-boot node then it it is "padding". And if that property is in
> dtb node then it is "placing" binary to the absolute address.
>
> From reading binman.html documentation I see that for placing image at
> absolute address there is already "offset" property. And for padding
> there is "pad-after" property. What about consistency?
>
> So maybe 'pad-after-to-sym = "_end";' in u-boot node?
> Or 'offset-from-sym = <&u_boot>, "_end";' in dtb node?

Yes that makes sense. I'll take a look at this at some point.

>
> > >
> > > > 3. Add a common binman image for u-boot.bin (used by every board)
> > >
> > > It should be u-boot-dtb.bin (not u-boot.bin). At least this is the
> > > current file name. (See this my patch series again, which aligns this
> > > naming also for powerpc/mpc85xx).
> >
> > We changed this 6 years ago and I'm not keen on going back.
> >
> > ad1ecd2063d fdt: Build a U-Boot binary without device tree
>
> I just do not understand you because in that commit is exactly what I
> wrote. In file u-boot-dtb.bin is u-boot binary with DTB and in file
> u-boot-nodtb.bin is u-boot binary without DTB.
>
> So binman should take input files u-boot-nodtb.bin and DTB binary. And
> should produce output file u-boot-dtb.bin. Is there any issue with it?

Actually u-boot-dtb.bin is a hangover from that commit, left in to
allow people to adjust. So I think we should remove creation of
u-boot-dtb.bin

>
> > >
> > > > 4. Enable binman by default
> > > > 5. Drop the current 'cat' rules in Makefile, so that only u-boot and
> > > > u-boot-nodtb.bin are produced
> > >
> > > (cat rule is only for u-boot-dtb.bin)
> > >
> > > > 6. Optionally consider moving .img files to binman also
> > > >
> > > > Regards,
> > > > Simon
> > >
> > > With slightly modified/improved step 2. I agree that this improves
> > > current situation. And should these issues.
> >
> > Regards,
> > Simon
> >
> >
> > [1] https://u-boot.readthedocs.io/en/latest/develop/package/binman.html#expanded-entries

Regards,
Simon

Re: [PATCH u-boot 2/2] Makefile: Build working u-boot-dtb.bin target also for mpc85xx

[Pali Rohár]

On Tuesday 03 January 2023 11:02:17 Simon Glass wrote:
> > > > > 3. Add a common binman image for u-boot.bin (used by every board)
> > > >
> > > > It should be u-boot-dtb.bin (not u-boot.bin). At least this is the
> > > > current file name. (See this my patch series again, which aligns this
> > > > naming also for powerpc/mpc85xx).
> > >
> > > We changed this 6 years ago and I'm not keen on going back.
> > >
> > > ad1ecd2063d fdt: Build a U-Boot binary without device tree
> >
> > I just do not understand you because in that commit is exactly what I
> > wrote. In file u-boot-dtb.bin is u-boot binary with DTB and in file
> > u-boot-nodtb.bin is u-boot binary without DTB.
> >
> > So binman should take input files u-boot-nodtb.bin and DTB binary. And
> > should produce output file u-boot-dtb.bin. Is there any issue with it?
> 
> Actually u-boot-dtb.bin is a hangover from that commit, left in to
> allow people to adjust. So I think we should remove creation of
> u-boot-dtb.bin

Ok, complete remove of u-boot-dtb.bin creation also works.

Re: [PATCH u-boot 2/2] Makefile: Build working u-boot-dtb.bin target also for mpc85xx

[Simon Glass]

Hi Pali,

On Tue, 3 Jan 2023 at 10:05, Pali Rohár <pali@kernel.org> wrote:
>
> On Tuesday 03 January 2023 11:02:17 Simon Glass wrote:
> > > > > > 3. Add a common binman image for u-boot.bin (used by every board)
> > > > >
> > > > > It should be u-boot-dtb.bin (not u-boot.bin). At least this is the
> > > > > current file name. (See this my patch series again, which aligns this
> > > > > naming also for powerpc/mpc85xx).
> > > >
> > > > We changed this 6 years ago and I'm not keen on going back.
> > > >
> > > > ad1ecd2063d fdt: Build a U-Boot binary without device tree
> > >
> > > I just do not understand you because in that commit is exactly what I
> > > wrote. In file u-boot-dtb.bin is u-boot binary with DTB and in file
> > > u-boot-nodtb.bin is u-boot binary without DTB.
> > >
> > > So binman should take input files u-boot-nodtb.bin and DTB binary. And
> > > should produce output file u-boot-dtb.bin. Is there any issue with it?
> >
> > Actually u-boot-dtb.bin is a hangover from that commit, left in to
> > allow people to adjust. So I think we should remove creation of
> > u-boot-dtb.bin
>
> Ok, complete remove of u-boot-dtb.bin creation also works.

OK good.


- Simon

Re: [PATCH u-boot 2/2] Makefile: Build working u-boot-dtb.bin target also for mpc85xx

[Pali Rohár]

On Friday 06 January 2023 17:13:41 Simon Glass wrote:
> Hi Pali,
> 
> On Tue, 3 Jan 2023 at 10:05, Pali Rohár <pali@kernel.org> wrote:
> >
> > On Tuesday 03 January 2023 11:02:17 Simon Glass wrote:
> > > > > > > 3. Add a common binman image for u-boot.bin (used by every board)
> > > > > >
> > > > > > It should be u-boot-dtb.bin (not u-boot.bin). At least this is the
> > > > > > current file name. (See this my patch series again, which aligns this
> > > > > > naming also for powerpc/mpc85xx).
> > > > >
> > > > > We changed this 6 years ago and I'm not keen on going back.
> > > > >
> > > > > ad1ecd2063d fdt: Build a U-Boot binary without device tree
> > > >
> > > > I just do not understand you because in that commit is exactly what I
> > > > wrote. In file u-boot-dtb.bin is u-boot binary with DTB and in file
> > > > u-boot-nodtb.bin is u-boot binary without DTB.
> > > >
> > > > So binman should take input files u-boot-nodtb.bin and DTB binary. And
> > > > should produce output file u-boot-dtb.bin. Is there any issue with it?
> > >
> > > Actually u-boot-dtb.bin is a hangover from that commit, left in to
> > > allow people to adjust. So I think we should remove creation of
> > > u-boot-dtb.bin
> >
> > Ok, complete remove of u-boot-dtb.bin creation also works.
> 
> OK good.
> 
> 
> - Simon

I would suggest to take patch 1/2 to have all mpc85xx boards standard
output file u-boot.bin. And prevent to repeat issue that building of the
final image with custom name was unintentionally turned off as a side
effect of some change.

Re: [PATCH u-boot 2/2] Makefile: Build working u-boot-dtb.bin target also for mpc85xx

[Heiko Schocher]

Hello Pali,

On 11.01.23 01:12, Pali Rohár wrote:
> On Friday 06 January 2023 17:13:41 Simon Glass wrote:
>> Hi Pali,
>>
>> On Tue, 3 Jan 2023 at 10:05, Pali Rohár <pali@kernel.org> wrote:
>>>
>>> On Tuesday 03 January 2023 11:02:17 Simon Glass wrote:
>>>>>>>> 3. Add a common binman image for u-boot.bin (used by every board)
>>>>>>>
>>>>>>> It should be u-boot-dtb.bin (not u-boot.bin). At least this is the
>>>>>>> current file name. (See this my patch series again, which aligns this
>>>>>>> naming also for powerpc/mpc85xx).
>>>>>>
>>>>>> We changed this 6 years ago and I'm not keen on going back.
>>>>>>
>>>>>> ad1ecd2063d fdt: Build a U-Boot binary without device tree
>>>>>
>>>>> I just do not understand you because in that commit is exactly what I
>>>>> wrote. In file u-boot-dtb.bin is u-boot binary with DTB and in file
>>>>> u-boot-nodtb.bin is u-boot binary without DTB.
>>>>>
>>>>> So binman should take input files u-boot-nodtb.bin and DTB binary. And
>>>>> should produce output file u-boot-dtb.bin. Is there any issue with it?
>>>>
>>>> Actually u-boot-dtb.bin is a hangover from that commit, left in to
>>>> allow people to adjust. So I think we should remove creation of
>>>> u-boot-dtb.bin
>>>
>>> Ok, complete remove of u-boot-dtb.bin creation also works.
>>
>> OK good.
>>
>>
>> - Simon
> 
> I would suggest to take patch 1/2 to have all mpc85xx boards standard
> output file u-boot.bin. And prevent to repeat issue that building of the
> final image with custom name was unintentionally turned off as a side
> effect of some change.
> 

I just tried the patches (based on my rework of socrates board):

  http://patchwork.ozlabs.org/project/uboot/patch/20221228200437.30971-1-pali@kernel.org/
  http://patchwork.ozlabs.org/project/uboot/patch/20221228200437.30971-2-pali@kernel.org/


and they work fine for me...

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs@denx.de

Re: [PATCH u-boot 2/2] Makefile: Build working u-boot-dtb.bin target also for mpc85xx

[Heiko Schocher]

Hello Pali, Tom,

I just tried azure build with my socrates board updates based on
v2023.01 and the 2 patches from Pali:

http://patchwork.ozlabs.org/project/uboot/patch/20221228200437.30971-1-pali@kernel.org/
http://patchwork.ozlabs.org/project/uboot/patch/20221228200437.30971-2-pali@kernel.org/

and get errors for powerpc build:

https://dev.azure.com/hs0298/110c3e42-44d5-4db4-9bd5-8a8bbead15f3/_apis/build/builds/93/logs/601

socrates board builds fine ... my patches are socrates board specfic,
so hopefully no impact for other boards ...

@Tom: Do you know if v2023.01 builds fine for powerpc
@Pali: Did you made a global build with your 2 patches?

for reference, you find my tree here:

https://github.com/hsdenx/u-boot-test/tree/socrates-2023.01-v1

Thanks!

bye,
Heiko

-- 
DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs@denx.de

Re: [PATCH u-boot 2/2] Makefile: Build working u-boot-dtb.bin target also for mpc85xx

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2182 bytes --]

On Wed, Jan 11, 2023 at 01:52:24PM +0100, Heiko Schocher wrote:
> Hello Pali, Tom,
> 
> I just tried azure build with my socrates board updates based on
> v2023.01 and the 2 patches from Pali:
> 
> http://patchwork.ozlabs.org/project/uboot/patch/20221228200437.30971-1-pali@kernel.org/
> http://patchwork.ozlabs.org/project/uboot/patch/20221228200437.30971-2-pali@kernel.org/
> 
> and get errors for powerpc build:
> 
> https://dev.azure.com/hs0298/110c3e42-44d5-4db4-9bd5-8a8bbead15f3/_apis/build/builds/93/logs/601
> 
> socrates board builds fine ... my patches are socrates board specfic,
> so hopefully no impact for other boards ...

socrates is one of the two failing boards, in that link:
2023-01-11T12:16:04.4930367Z    powerpc:  w+   MPC837XERDB kmcoge5ne
kmeter1 kmopti2 kmsupx5 kmtepr2 tuge1 tuxx1 MPC8548CDS MPC8548CDS_36BIT
MPC8548CDS_legacy P1010RDB-PA_36BIT_NAND P1010RDB-PA_36BIT_NOR
P1010RDB-PA_36BIT_SDCARD P1010RDB-PA_36BIT_SPIFLASH P1010RDB-PA_NAND
P1010RDB-PA_NOR P1010RDB-PA_SDCARD P1010RDB-PA_SPIFLASH
P1010RDB-PB_36BIT_NAND P1010RDB-PB_36BIT_NOR P1010RDB-PB_36BIT_SDCARD
P1010RDB-PB_36BIT_SPIFLASH P1010RDB-PB_NAND P1010RDB-PB_NOR
P1010RDB-PB_SDCARD P1010RDB-PB_SPIFLASH P1020RDB-PC P1020RDB-PC_36BIT
P1020RDB-PC_36BIT_NAND P1020RDB-PC_36BIT_SDCARD
P1020RDB-PC_36BIT_SPIFLASH P1020RDB-PC_NAND P1020RDB-PC_SDCARD
P1020RDB-PC_SPIFLASH P1020RDB-PD P1020RDB-PD_NAND P1020RDB-PD_SDCARD
P1020RDB-PD_SPIFLASH P2020RDB-PC P2020RDB-PC_36BIT
P2020RDB-PC_36BIT_NAND P2020RDB-PC_36BIT_SDCARD
P2020RDB-PC_36BIT_SPIFLASH P2020RDB-PC_NAND P2020RDB-PC_SDCARD
P2020RDB-PC_SPIFLASH P2041RDB P2041RDB_NAND P2041RDB_SDCARD
P2041RDB_SPIFLASH T1024RDB T1024RDB_NAND T1024RDB_SDCARD
T1024RDB_SPIFLASH T1042D4RDB T1042D4RDB_NAND T1042D4RDB_SDCARD
T1042D4RDB_SPIFLASH T2080QDS T2080QDS_NAND T2080QDS_SDCARD
T2080QDS_SECURE_BOOT T2080QDS_SPIFLASH T2080QDS_SRIO_PCIE_BOOT T2080RDB
T2080RDB_NAND T2080RDB_revD T2080RDB_revD_NAND T2080RDB_revD_SDCARD
T2080RDB_revD_SPIFLASH T2080RDB_SDCARD T2080RDB_SPIFLASH T4240RDB
T4240RDB_SDCARD +   socrates kmcent2

> @Tom: Do you know if v2023.01 builds fine for powerpc

Yes, CI is passing.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH u-boot 2/2] Makefile: Build working u-boot-dtb.bin target also for mpc85xx

[Pali Rohár]

On Wednesday 11 January 2023 09:01:37 Tom Rini wrote:
> On Wed, Jan 11, 2023 at 01:52:24PM +0100, Heiko Schocher wrote:
> > Hello Pali, Tom,
> > 
> > I just tried azure build with my socrates board updates based on
> > v2023.01 and the 2 patches from Pali:
> > 
> > http://patchwork.ozlabs.org/project/uboot/patch/20221228200437.30971-1-pali@kernel.org/
> > http://patchwork.ozlabs.org/project/uboot/patch/20221228200437.30971-2-pali@kernel.org/

At the time when I sent those two patches to ML, I checked that P1/P2
powerpc boards and also socrates board compiles successfully.

Now I imported those two patches on top of the current master branch and
they still compiles without any problems for socrates board.

> > and get errors for powerpc build:
> > 
> > https://dev.azure.com/hs0298/110c3e42-44d5-4db4-9bd5-8a8bbead15f3/_apis/build/builds/93/logs/601
> > 
> > socrates board builds fine ... my patches are socrates board specfic,
> > so hopefully no impact for other boards ...

From that build log it looks like that u-boot fails for socrates and
kmcent2 board. Which is strange as you said that too that socrates is
building fine...

> socrates is one of the two failing boards, in that link:
> 2023-01-11T12:16:04.4930367Z    powerpc:  w+   MPC837XERDB kmcoge5ne
> kmeter1 kmopti2 kmsupx5 kmtepr2 tuge1 tuxx1 MPC8548CDS MPC8548CDS_36BIT
> MPC8548CDS_legacy P1010RDB-PA_36BIT_NAND P1010RDB-PA_36BIT_NOR
> P1010RDB-PA_36BIT_SDCARD P1010RDB-PA_36BIT_SPIFLASH P1010RDB-PA_NAND
> P1010RDB-PA_NOR P1010RDB-PA_SDCARD P1010RDB-PA_SPIFLASH
> P1010RDB-PB_36BIT_NAND P1010RDB-PB_36BIT_NOR P1010RDB-PB_36BIT_SDCARD
> P1010RDB-PB_36BIT_SPIFLASH P1010RDB-PB_NAND P1010RDB-PB_NOR
> P1010RDB-PB_SDCARD P1010RDB-PB_SPIFLASH P1020RDB-PC P1020RDB-PC_36BIT
> P1020RDB-PC_36BIT_NAND P1020RDB-PC_36BIT_SDCARD
> P1020RDB-PC_36BIT_SPIFLASH P1020RDB-PC_NAND P1020RDB-PC_SDCARD
> P1020RDB-PC_SPIFLASH P1020RDB-PD P1020RDB-PD_NAND P1020RDB-PD_SDCARD
> P1020RDB-PD_SPIFLASH P2020RDB-PC P2020RDB-PC_36BIT
> P2020RDB-PC_36BIT_NAND P2020RDB-PC_36BIT_SDCARD
> P2020RDB-PC_36BIT_SPIFLASH P2020RDB-PC_NAND P2020RDB-PC_SDCARD
> P2020RDB-PC_SPIFLASH P2041RDB P2041RDB_NAND P2041RDB_SDCARD
> P2041RDB_SPIFLASH T1024RDB T1024RDB_NAND T1024RDB_SDCARD
> T1024RDB_SPIFLASH T1042D4RDB T1042D4RDB_NAND T1042D4RDB_SDCARD
> T1042D4RDB_SPIFLASH T2080QDS T2080QDS_NAND T2080QDS_SDCARD
> T2080QDS_SECURE_BOOT T2080QDS_SPIFLASH T2080QDS_SRIO_PCIE_BOOT T2080RDB
> T2080RDB_NAND T2080RDB_revD T2080RDB_revD_NAND T2080RDB_revD_SDCARD
> T2080RDB_revD_SPIFLASH T2080RDB_SDCARD T2080RDB_SPIFLASH T4240RDB
> T4240RDB_SDCARD +   socrates kmcent2
> 
> > @Tom: Do you know if v2023.01 builds fine for powerpc
> 
> Yes, CI is passing.
> 
> -- 
> Tom

Re: [PATCH u-boot 2/2] Makefile: Build working u-boot-dtb.bin target also for mpc85xx

[Pali Rohár]

On Wednesday 11 January 2023 18:55:40 Pali Rohár wrote:
> On Wednesday 11 January 2023 09:01:37 Tom Rini wrote:
> > On Wed, Jan 11, 2023 at 01:52:24PM +0100, Heiko Schocher wrote:
> > > Hello Pali, Tom,
> > > 
> > > I just tried azure build with my socrates board updates based on
> > > v2023.01 and the 2 patches from Pali:
> > > 
> > > http://patchwork.ozlabs.org/project/uboot/patch/20221228200437.30971-1-pali@kernel.org/
> > > http://patchwork.ozlabs.org/project/uboot/patch/20221228200437.30971-2-pali@kernel.org/
> 
> At the time when I sent those two patches to ML, I checked that P1/P2
> powerpc boards and also socrates board compiles successfully.
> 
> Now I imported those two patches on top of the current master branch and
> they still compiles without any problems for socrates board.
> 
> > > and get errors for powerpc build:
> > > 
> > > https://dev.azure.com/hs0298/110c3e42-44d5-4db4-9bd5-8a8bbead15f3/_apis/build/builds/93/logs/601
> > > 
> > > socrates board builds fine ... my patches are socrates board specfic,
> > > so hopefully no impact for other boards ...
> 
> From that build log it looks like that u-boot fails for socrates and
> kmcent2 board. Which is strange as you said that too that socrates is
> building fine...

kmcent2 is expected to fail with my above two patches on top of the
v2023.01 without this kmcent2 commit which is now already in master:
https://source.denx.de/u-boot/u-boot/-/commit/499fe577c8011dd8a9184548c419db42aef079a7

> > socrates is one of the two failing boards, in that link:
> > 2023-01-11T12:16:04.4930367Z    powerpc:  w+   MPC837XERDB kmcoge5ne
> > kmeter1 kmopti2 kmsupx5 kmtepr2 tuge1 tuxx1 MPC8548CDS MPC8548CDS_36BIT
> > MPC8548CDS_legacy P1010RDB-PA_36BIT_NAND P1010RDB-PA_36BIT_NOR
> > P1010RDB-PA_36BIT_SDCARD P1010RDB-PA_36BIT_SPIFLASH P1010RDB-PA_NAND
> > P1010RDB-PA_NOR P1010RDB-PA_SDCARD P1010RDB-PA_SPIFLASH
> > P1010RDB-PB_36BIT_NAND P1010RDB-PB_36BIT_NOR P1010RDB-PB_36BIT_SDCARD
> > P1010RDB-PB_36BIT_SPIFLASH P1010RDB-PB_NAND P1010RDB-PB_NOR
> > P1010RDB-PB_SDCARD P1010RDB-PB_SPIFLASH P1020RDB-PC P1020RDB-PC_36BIT
> > P1020RDB-PC_36BIT_NAND P1020RDB-PC_36BIT_SDCARD
> > P1020RDB-PC_36BIT_SPIFLASH P1020RDB-PC_NAND P1020RDB-PC_SDCARD
> > P1020RDB-PC_SPIFLASH P1020RDB-PD P1020RDB-PD_NAND P1020RDB-PD_SDCARD
> > P1020RDB-PD_SPIFLASH P2020RDB-PC P2020RDB-PC_36BIT
> > P2020RDB-PC_36BIT_NAND P2020RDB-PC_36BIT_SDCARD
> > P2020RDB-PC_36BIT_SPIFLASH P2020RDB-PC_NAND P2020RDB-PC_SDCARD
> > P2020RDB-PC_SPIFLASH P2041RDB P2041RDB_NAND P2041RDB_SDCARD
> > P2041RDB_SPIFLASH T1024RDB T1024RDB_NAND T1024RDB_SDCARD
> > T1024RDB_SPIFLASH T1042D4RDB T1042D4RDB_NAND T1042D4RDB_SDCARD
> > T1042D4RDB_SPIFLASH T2080QDS T2080QDS_NAND T2080QDS_SDCARD
> > T2080QDS_SECURE_BOOT T2080QDS_SPIFLASH T2080QDS_SRIO_PCIE_BOOT T2080RDB
> > T2080RDB_NAND T2080RDB_revD T2080RDB_revD_NAND T2080RDB_revD_SDCARD
> > T2080RDB_revD_SPIFLASH T2080RDB_SDCARD T2080RDB_SPIFLASH T4240RDB
> > T4240RDB_SDCARD +   socrates kmcent2
> > 
> > > @Tom: Do you know if v2023.01 builds fine for powerpc
> > 
> > Yes, CI is passing.
> > 
> > -- 
> > Tom
> 
> 

Re: [PATCH u-boot 2/2] Makefile: Build working u-boot-dtb.bin target also for mpc85xx

[Pali Rohár]

On Wednesday 11 January 2023 19:02:38 Pali Rohár wrote:
> On Wednesday 11 January 2023 18:55:40 Pali Rohár wrote:
> > On Wednesday 11 January 2023 09:01:37 Tom Rini wrote:
> > > On Wed, Jan 11, 2023 at 01:52:24PM +0100, Heiko Schocher wrote:
> > > > Hello Pali, Tom,
> > > > 
> > > > I just tried azure build with my socrates board updates based on
> > > > v2023.01 and the 2 patches from Pali:
> > > > 
> > > > http://patchwork.ozlabs.org/project/uboot/patch/20221228200437.30971-1-pali@kernel.org/
> > > > http://patchwork.ozlabs.org/project/uboot/patch/20221228200437.30971-2-pali@kernel.org/
> > 
> > At the time when I sent those two patches to ML, I checked that P1/P2
> > powerpc boards and also socrates board compiles successfully.
> > 
> > Now I imported those two patches on top of the current master branch and
> > they still compiles without any problems for socrates board.
> > 
> > > > and get errors for powerpc build:
> > > > 
> > > > https://dev.azure.com/hs0298/110c3e42-44d5-4db4-9bd5-8a8bbead15f3/_apis/build/builds/93/logs/601
> > > > 
> > > > socrates board builds fine ... my patches are socrates board specfic,
> > > > so hopefully no impact for other boards ...
> > 
> > From that build log it looks like that u-boot fails for socrates and
> > kmcent2 board. Which is strange as you said that too that socrates is
> > building fine...
> 
> kmcent2 is expected to fail with my above two patches on top of the
> v2023.01 without this kmcent2 commit which is now already in master:
> https://source.denx.de/u-boot/u-boot/-/commit/499fe577c8011dd8a9184548c419db42aef079a7

And now I think I see the reason why it is failing also for socrates
board. Error in the build log is:

2023-01-11T12:16:04.4937207Z +binman: [Errno 2] No such file or directory: 'u-boot.dtb'
2023-01-11T12:16:04.4937685Z +make[1]: *** [Makefile:1613: u-boot-dtb.bin] Error 1

u-boot.dtb builds make by Makefile rule:

u-boot.dtb: dts/dt.dtb
	$(call cmd,copy)

But socrates-u-boot.dtsi has specified that use dts/dt.dtb and this
dependency is also specified in Makefile.

And it looks like that binman needs also u-boot.dtb file. So it is
possible to hit a race condition, that make builds u-boot.dtb later than
rule for binman.

I would suggest to try to apply this patch, which should instruct make
to do not call binman until u-boot.dtb is correctly built:

diff --git a/Makefile b/Makefile
index 3c76486a620e..5d2ef8cc81c5 100644
--- a/Makefile
+++ b/Makefile
@@ -1603,7 +1603,7 @@ OBJCOPYFLAGS_u-boot-br.bin := -O binary -j .bootpg -j .resetvec
 u-boot-br.bin: u-boot FORCE
 	$(call if_changed,objcopy)
 else ifeq ($(CONFIG_TARGET_SOCRATES),y)
-u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
+u-boot-dtb.bin: u-boot-nodtb.bin u-boot.dtb FORCE
 	$(call if_changed,binman)
 endif
 
diff --git a/arch/powerpc/dts/socrates-u-boot.dtsi b/arch/powerpc/dts/socrates-u-boot.dtsi
index ba2e56d35675..f6af611b513c 100644
--- a/arch/powerpc/dts/socrates-u-boot.dtsi
+++ b/arch/powerpc/dts/socrates-u-boot.dtsi
@@ -9,7 +9,7 @@
 		pad-byte = <0xff>;
 		// Place dtb one sector before u-boot-nodtb.bin
 		blob {
-			filename = "dts/dt.dtb";
+			filename = "u-boot.dtb";
 		};
 		u-boot-nodtb {
 			filename = "u-boot-nodtb.bin";


Heiko, could you try to put commit 499fe577c8011dd8a9184548c419db42aef079a7
and above patch to your branch and retest it again?

> > > socrates is one of the two failing boards, in that link:
> > > 2023-01-11T12:16:04.4930367Z    powerpc:  w+   MPC837XERDB kmcoge5ne
> > > kmeter1 kmopti2 kmsupx5 kmtepr2 tuge1 tuxx1 MPC8548CDS MPC8548CDS_36BIT
> > > MPC8548CDS_legacy P1010RDB-PA_36BIT_NAND P1010RDB-PA_36BIT_NOR
> > > P1010RDB-PA_36BIT_SDCARD P1010RDB-PA_36BIT_SPIFLASH P1010RDB-PA_NAND
> > > P1010RDB-PA_NOR P1010RDB-PA_SDCARD P1010RDB-PA_SPIFLASH
> > > P1010RDB-PB_36BIT_NAND P1010RDB-PB_36BIT_NOR P1010RDB-PB_36BIT_SDCARD
> > > P1010RDB-PB_36BIT_SPIFLASH P1010RDB-PB_NAND P1010RDB-PB_NOR
> > > P1010RDB-PB_SDCARD P1010RDB-PB_SPIFLASH P1020RDB-PC P1020RDB-PC_36BIT
> > > P1020RDB-PC_36BIT_NAND P1020RDB-PC_36BIT_SDCARD
> > > P1020RDB-PC_36BIT_SPIFLASH P1020RDB-PC_NAND P1020RDB-PC_SDCARD
> > > P1020RDB-PC_SPIFLASH P1020RDB-PD P1020RDB-PD_NAND P1020RDB-PD_SDCARD
> > > P1020RDB-PD_SPIFLASH P2020RDB-PC P2020RDB-PC_36BIT
> > > P2020RDB-PC_36BIT_NAND P2020RDB-PC_36BIT_SDCARD
> > > P2020RDB-PC_36BIT_SPIFLASH P2020RDB-PC_NAND P2020RDB-PC_SDCARD
> > > P2020RDB-PC_SPIFLASH P2041RDB P2041RDB_NAND P2041RDB_SDCARD
> > > P2041RDB_SPIFLASH T1024RDB T1024RDB_NAND T1024RDB_SDCARD
> > > T1024RDB_SPIFLASH T1042D4RDB T1042D4RDB_NAND T1042D4RDB_SDCARD
> > > T1042D4RDB_SPIFLASH T2080QDS T2080QDS_NAND T2080QDS_SDCARD
> > > T2080QDS_SECURE_BOOT T2080QDS_SPIFLASH T2080QDS_SRIO_PCIE_BOOT T2080RDB
> > > T2080RDB_NAND T2080RDB_revD T2080RDB_revD_NAND T2080RDB_revD_SDCARD
> > > T2080RDB_revD_SPIFLASH T2080RDB_SDCARD T2080RDB_SPIFLASH T4240RDB
> > > T4240RDB_SDCARD +   socrates kmcent2
> > > 
> > > > @Tom: Do you know if v2023.01 builds fine for powerpc
> > > 
> > > Yes, CI is passing.
> > > 
> > > -- 
> > > Tom
> > 
> > 

Re: [PATCH u-boot 2/2] Makefile: Build working u-boot-dtb.bin target also for mpc85xx

[Heiko Schocher]

Hello Pali,

On 11.01.23 19:13, Pali Rohár wrote:
> On Wednesday 11 January 2023 19:02:38 Pali Rohár wrote:
>> On Wednesday 11 January 2023 18:55:40 Pali Rohár wrote:
>>> On Wednesday 11 January 2023 09:01:37 Tom Rini wrote:
>>>> On Wed, Jan 11, 2023 at 01:52:24PM +0100, Heiko Schocher wrote:
>>>>> Hello Pali, Tom,
>>>>>
>>>>> I just tried azure build with my socrates board updates based on
>>>>> v2023.01 and the 2 patches from Pali:
>>>>>
>>>>> http://patchwork.ozlabs.org/project/uboot/patch/20221228200437.30971-1-pali@kernel.org/
>>>>> http://patchwork.ozlabs.org/project/uboot/patch/20221228200437.30971-2-pali@kernel.org/
>>>
>>> At the time when I sent those two patches to ML, I checked that P1/P2
>>> powerpc boards and also socrates board compiles successfully.
>>>
>>> Now I imported those two patches on top of the current master branch and
>>> they still compiles without any problems for socrates board.
>>>
>>>>> and get errors for powerpc build:
>>>>>
>>>>> https://dev.azure.com/hs0298/110c3e42-44d5-4db4-9bd5-8a8bbead15f3/_apis/build/builds/93/logs/601
>>>>>
>>>>> socrates board builds fine ... my patches are socrates board specfic,
>>>>> so hopefully no impact for other boards ...
>>>
>>> From that build log it looks like that u-boot fails for socrates and
>>> kmcent2 board. Which is strange as you said that too that socrates is
>>> building fine...

misreaded azure output, so socrates is failing because missing u-boot.dtb,
sorry. Interesting is, that my yocto build works ...

>>
>> kmcent2 is expected to fail with my above two patches on top of the
>> v2023.01 without this kmcent2 commit which is now already in master:
>> https://source.denx.de/u-boot/u-boot/-/commit/499fe577c8011dd8a9184548c419db42aef079a7
> 
> And now I think I see the reason why it is failing also for socrates
> board. Error in the build log is:
> 
> 2023-01-11T12:16:04.4937207Z +binman: [Errno 2] No such file or directory: 'u-boot.dtb'
> 2023-01-11T12:16:04.4937685Z +make[1]: *** [Makefile:1613: u-boot-dtb.bin] Error 1

Yup.

> u-boot.dtb builds make by Makefile rule:
> 
> u-boot.dtb: dts/dt.dtb
> 	$(call cmd,copy)
> 
> But socrates-u-boot.dtsi has specified that use dts/dt.dtb and this
> dependency is also specified in Makefile.
> 
> And it looks like that binman needs also u-boot.dtb file. So it is
> possible to hit a race condition, that make builds u-boot.dtb later than
> rule for binman.

Exactly over this I stumbled yesterday in the evening and I made a local
fix:

diff --git a/Makefile b/Makefile
index fb1454552a..60f5cffccd 100644
--- a/Makefile
+++ b/Makefile
@@ -1609,7 +1609,7 @@ OBJCOPYFLAGS_u-boot-br.bin := -O binary -j .bootpg -j .resetvec
 u-boot-br.bin: u-boot FORCE
        $(call if_changed,objcopy)
 else ifeq ($(CONFIG_TARGET_SOCRATES),y)
-u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
+u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb u-boot.dtb FORCE
        $(call if_changed,binman)
 endif


> I would suggest to try to apply this patch, which should instruct make
> to do not call binman until u-boot.dtb is correctly built:
> 
> diff --git a/Makefile b/Makefile
> index 3c76486a620e..5d2ef8cc81c5 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1603,7 +1603,7 @@ OBJCOPYFLAGS_u-boot-br.bin := -O binary -j .bootpg -j .resetvec
>  u-boot-br.bin: u-boot FORCE
>  	$(call if_changed,objcopy)
>  else ifeq ($(CONFIG_TARGET_SOCRATES),y)
> -u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
> +u-boot-dtb.bin: u-boot-nodtb.bin u-boot.dtb FORCE
>  	$(call if_changed,binman)
>  endif
>  
> diff --git a/arch/powerpc/dts/socrates-u-boot.dtsi b/arch/powerpc/dts/socrates-u-boot.dtsi
> index ba2e56d35675..f6af611b513c 100644
> --- a/arch/powerpc/dts/socrates-u-boot.dtsi
> +++ b/arch/powerpc/dts/socrates-u-boot.dtsi
> @@ -9,7 +9,7 @@
>  		pad-byte = <0xff>;
>  		// Place dtb one sector before u-boot-nodtb.bin
>  		blob {
> -			filename = "dts/dt.dtb";
> +			filename = "u-boot.dtb";
>  		};
>  		u-boot-nodtb {
>  			filename = "u-boot-nodtb.bin";
> 
> 
> Heiko, could you try to put commit 499fe577c8011dd8a9184548c419db42aef079a7
> and above patch to your branch and retest it again?

Of course! Just have to wait until other azure build finished...

In the meantime I rebased my patchset to current HEAD, so above
commit is already in, and I had to adapt some patches for socrates.

I write the results hopefully soon.

bye,
Heikp
>>>> socrates is one of the two failing boards, in that link:
>>>> 2023-01-11T12:16:04.4930367Z    powerpc:  w+   MPC837XERDB kmcoge5ne
>>>> kmeter1 kmopti2 kmsupx5 kmtepr2 tuge1 tuxx1 MPC8548CDS MPC8548CDS_36BIT
>>>> MPC8548CDS_legacy P1010RDB-PA_36BIT_NAND P1010RDB-PA_36BIT_NOR
>>>> P1010RDB-PA_36BIT_SDCARD P1010RDB-PA_36BIT_SPIFLASH P1010RDB-PA_NAND
>>>> P1010RDB-PA_NOR P1010RDB-PA_SDCARD P1010RDB-PA_SPIFLASH
>>>> P1010RDB-PB_36BIT_NAND P1010RDB-PB_36BIT_NOR P1010RDB-PB_36BIT_SDCARD
>>>> P1010RDB-PB_36BIT_SPIFLASH P1010RDB-PB_NAND P1010RDB-PB_NOR
>>>> P1010RDB-PB_SDCARD P1010RDB-PB_SPIFLASH P1020RDB-PC P1020RDB-PC_36BIT
>>>> P1020RDB-PC_36BIT_NAND P1020RDB-PC_36BIT_SDCARD
>>>> P1020RDB-PC_36BIT_SPIFLASH P1020RDB-PC_NAND P1020RDB-PC_SDCARD
>>>> P1020RDB-PC_SPIFLASH P1020RDB-PD P1020RDB-PD_NAND P1020RDB-PD_SDCARD
>>>> P1020RDB-PD_SPIFLASH P2020RDB-PC P2020RDB-PC_36BIT
>>>> P2020RDB-PC_36BIT_NAND P2020RDB-PC_36BIT_SDCARD
>>>> P2020RDB-PC_36BIT_SPIFLASH P2020RDB-PC_NAND P2020RDB-PC_SDCARD
>>>> P2020RDB-PC_SPIFLASH P2041RDB P2041RDB_NAND P2041RDB_SDCARD
>>>> P2041RDB_SPIFLASH T1024RDB T1024RDB_NAND T1024RDB_SDCARD
>>>> T1024RDB_SPIFLASH T1042D4RDB T1042D4RDB_NAND T1042D4RDB_SDCARD
>>>> T1042D4RDB_SPIFLASH T2080QDS T2080QDS_NAND T2080QDS_SDCARD
>>>> T2080QDS_SECURE_BOOT T2080QDS_SPIFLASH T2080QDS_SRIO_PCIE_BOOT T2080RDB
>>>> T2080RDB_NAND T2080RDB_revD T2080RDB_revD_NAND T2080RDB_revD_SDCARD
>>>> T2080RDB_revD_SPIFLASH T2080RDB_SDCARD T2080RDB_SPIFLASH T4240RDB
>>>> T4240RDB_SDCARD +   socrates kmcent2
>>>>
>>>>> @Tom: Do you know if v2023.01 builds fine for powerpc
>>>>
>>>> Yes, CI is passing.
>>>>
>>>> -- 
>>>> Tom
>>>
>>>

-- 
DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs@denx.de

Re: [PATCH u-boot 2/2] Makefile: Build working u-boot-dtb.bin target also for mpc85xx

[Heiko Schocher]

Hello Pali,

On 12.01.23 07:27, Heiko Schocher wrote:
> Hello Pali,
> 
> On 11.01.23 19:13, Pali Rohár wrote:
>> On Wednesday 11 January 2023 19:02:38 Pali Rohár wrote:
>>> On Wednesday 11 January 2023 18:55:40 Pali Rohár wrote:
>>>> On Wednesday 11 January 2023 09:01:37 Tom Rini wrote:
>>>>> On Wed, Jan 11, 2023 at 01:52:24PM +0100, Heiko Schocher wrote:
>>>>>> Hello Pali, Tom,
>>>>>>
>>>>>> I just tried azure build with my socrates board updates based on
>>>>>> v2023.01 and the 2 patches from Pali:
>>>>>>
>>>>>> http://patchwork.ozlabs.org/project/uboot/patch/20221228200437.30971-1-pali@kernel.org/
>>>>>> http://patchwork.ozlabs.org/project/uboot/patch/20221228200437.30971-2-pali@kernel.org/
>>>>
>>>> At the time when I sent those two patches to ML, I checked that P1/P2
>>>> powerpc boards and also socrates board compiles successfully.
>>>>
>>>> Now I imported those two patches on top of the current master branch and
>>>> they still compiles without any problems for socrates board.
>>>>
>>>>>> and get errors for powerpc build:
>>>>>>
>>>>>> https://dev.azure.com/hs0298/110c3e42-44d5-4db4-9bd5-8a8bbead15f3/_apis/build/builds/93/logs/601
>>>>>>
>>>>>> socrates board builds fine ... my patches are socrates board specfic,
>>>>>> so hopefully no impact for other boards ...
>>>>
>>>> From that build log it looks like that u-boot fails for socrates and
>>>> kmcent2 board. Which is strange as you said that too that socrates is
>>>> building fine...
> 
> misreaded azure output, so socrates is failing because missing u-boot.dtb,
> sorry. Interesting is, that my yocto build works ...
> 
>>>
>>> kmcent2 is expected to fail with my above two patches on top of the
>>> v2023.01 without this kmcent2 commit which is now already in master:
>>> https://source.denx.de/u-boot/u-boot/-/commit/499fe577c8011dd8a9184548c419db42aef079a7
>>
>> And now I think I see the reason why it is failing also for socrates
>> board. Error in the build log is:
>>
>> 2023-01-11T12:16:04.4937207Z +binman: [Errno 2] No such file or directory: 'u-boot.dtb'
>> 2023-01-11T12:16:04.4937685Z +make[1]: *** [Makefile:1613: u-boot-dtb.bin] Error 1
> 
> Yup.
> 
>> u-boot.dtb builds make by Makefile rule:
>>
>> u-boot.dtb: dts/dt.dtb
>> 	$(call cmd,copy)
>>
>> But socrates-u-boot.dtsi has specified that use dts/dt.dtb and this
>> dependency is also specified in Makefile.
>>
>> And it looks like that binman needs also u-boot.dtb file. So it is
>> possible to hit a race condition, that make builds u-boot.dtb later than
>> rule for binman.
> 
> Exactly over this I stumbled yesterday in the evening and I made a local
> fix:
> 
> diff --git a/Makefile b/Makefile
> index fb1454552a..60f5cffccd 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1609,7 +1609,7 @@ OBJCOPYFLAGS_u-boot-br.bin := -O binary -j .bootpg -j .resetvec
>  u-boot-br.bin: u-boot FORCE
>         $(call if_changed,objcopy)
>  else ifeq ($(CONFIG_TARGET_SOCRATES),y)
> -u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
> +u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb u-boot.dtb FORCE
>         $(call if_changed,binman)
>  endif
> 
> 
>> I would suggest to try to apply this patch, which should instruct make
>> to do not call binman until u-boot.dtb is correctly built:
>>
>> diff --git a/Makefile b/Makefile
>> index 3c76486a620e..5d2ef8cc81c5 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1603,7 +1603,7 @@ OBJCOPYFLAGS_u-boot-br.bin := -O binary -j .bootpg -j .resetvec
>>  u-boot-br.bin: u-boot FORCE
>>  	$(call if_changed,objcopy)
>>  else ifeq ($(CONFIG_TARGET_SOCRATES),y)
>> -u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
>> +u-boot-dtb.bin: u-boot-nodtb.bin u-boot.dtb FORCE
>>  	$(call if_changed,binman)
>>  endif
>>  
>> diff --git a/arch/powerpc/dts/socrates-u-boot.dtsi b/arch/powerpc/dts/socrates-u-boot.dtsi
>> index ba2e56d35675..f6af611b513c 100644
>> --- a/arch/powerpc/dts/socrates-u-boot.dtsi
>> +++ b/arch/powerpc/dts/socrates-u-boot.dtsi
>> @@ -9,7 +9,7 @@
>>  		pad-byte = <0xff>;
>>  		// Place dtb one sector before u-boot-nodtb.bin
>>  		blob {
>> -			filename = "dts/dt.dtb";
>> +			filename = "u-boot.dtb";
>>  		};
>>  		u-boot-nodtb {
>>  			filename = "u-boot-nodtb.bin";
>>
>>
>> Heiko, could you try to put commit 499fe577c8011dd8a9184548c419db42aef079a7
>> and above patch to your branch and retest it again?
> 
> Of course! Just have to wait until other azure build finished...
> 
> In the meantime I rebased my patchset to current HEAD, so above
> commit is already in, and I had to adapt some patches for socrates.
> 
> I write the results hopefully soon.

Azure build successfully finished:
https://dev.azure.com/hs0298/hs/_build/results?buildId=95&view=results

U-Boot tree (contains the discussed fix on top):
https://github.com/hsdenx/u-boot-test/commits/socrates-2023.01-v1

So, would you send a v2 of your 2 patches, including the above fix?

If so, I pick them than up, and run a second azure build with them,
before I post the socrates board updates...

Else I can add the fix to my series...

Thanks!

bye,
Heiko

-- 
DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs@denx.de

Re: [PATCH u-boot 2/2] Makefile: Build working u-boot-dtb.bin target also for mpc85xx

[Pali Rohár]

On Thursday 12 January 2023 11:50:32 Heiko Schocher wrote:
> Hello Pali,
> 
> On 12.01.23 07:27, Heiko Schocher wrote:
> > Hello Pali,
> > 
> > On 11.01.23 19:13, Pali Rohár wrote:
> >> On Wednesday 11 January 2023 19:02:38 Pali Rohár wrote:
> >>> On Wednesday 11 January 2023 18:55:40 Pali Rohár wrote:
> >>>> On Wednesday 11 January 2023 09:01:37 Tom Rini wrote:
> >>>>> On Wed, Jan 11, 2023 at 01:52:24PM +0100, Heiko Schocher wrote:
> >>>>>> Hello Pali, Tom,
> >>>>>>
> >>>>>> I just tried azure build with my socrates board updates based on
> >>>>>> v2023.01 and the 2 patches from Pali:
> >>>>>>
> >>>>>> http://patchwork.ozlabs.org/project/uboot/patch/20221228200437.30971-1-pali@kernel.org/
> >>>>>> http://patchwork.ozlabs.org/project/uboot/patch/20221228200437.30971-2-pali@kernel.org/
> >>>>
> >>>> At the time when I sent those two patches to ML, I checked that P1/P2
> >>>> powerpc boards and also socrates board compiles successfully.
> >>>>
> >>>> Now I imported those two patches on top of the current master branch and
> >>>> they still compiles without any problems for socrates board.
> >>>>
> >>>>>> and get errors for powerpc build:
> >>>>>>
> >>>>>> https://dev.azure.com/hs0298/110c3e42-44d5-4db4-9bd5-8a8bbead15f3/_apis/build/builds/93/logs/601
> >>>>>>
> >>>>>> socrates board builds fine ... my patches are socrates board specfic,
> >>>>>> so hopefully no impact for other boards ...
> >>>>
> >>>> From that build log it looks like that u-boot fails for socrates and
> >>>> kmcent2 board. Which is strange as you said that too that socrates is
> >>>> building fine...
> > 
> > misreaded azure output, so socrates is failing because missing u-boot.dtb,
> > sorry. Interesting is, that my yocto build works ...
> > 
> >>>
> >>> kmcent2 is expected to fail with my above two patches on top of the
> >>> v2023.01 without this kmcent2 commit which is now already in master:
> >>> https://source.denx.de/u-boot/u-boot/-/commit/499fe577c8011dd8a9184548c419db42aef079a7
> >>
> >> And now I think I see the reason why it is failing also for socrates
> >> board. Error in the build log is:
> >>
> >> 2023-01-11T12:16:04.4937207Z +binman: [Errno 2] No such file or directory: 'u-boot.dtb'
> >> 2023-01-11T12:16:04.4937685Z +make[1]: *** [Makefile:1613: u-boot-dtb.bin] Error 1
> > 
> > Yup.
> > 
> >> u-boot.dtb builds make by Makefile rule:
> >>
> >> u-boot.dtb: dts/dt.dtb
> >> 	$(call cmd,copy)
> >>
> >> But socrates-u-boot.dtsi has specified that use dts/dt.dtb and this
> >> dependency is also specified in Makefile.
> >>
> >> And it looks like that binman needs also u-boot.dtb file. So it is
> >> possible to hit a race condition, that make builds u-boot.dtb later than
> >> rule for binman.
> > 
> > Exactly over this I stumbled yesterday in the evening and I made a local
> > fix:
> > 
> > diff --git a/Makefile b/Makefile
> > index fb1454552a..60f5cffccd 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1609,7 +1609,7 @@ OBJCOPYFLAGS_u-boot-br.bin := -O binary -j .bootpg -j .resetvec
> >  u-boot-br.bin: u-boot FORCE
> >         $(call if_changed,objcopy)
> >  else ifeq ($(CONFIG_TARGET_SOCRATES),y)
> > -u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
> > +u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb u-boot.dtb FORCE
> >         $(call if_changed,binman)
> >  endif
> > 
> > 
> >> I would suggest to try to apply this patch, which should instruct make
> >> to do not call binman until u-boot.dtb is correctly built:
> >>
> >> diff --git a/Makefile b/Makefile
> >> index 3c76486a620e..5d2ef8cc81c5 100644
> >> --- a/Makefile
> >> +++ b/Makefile
> >> @@ -1603,7 +1603,7 @@ OBJCOPYFLAGS_u-boot-br.bin := -O binary -j .bootpg -j .resetvec
> >>  u-boot-br.bin: u-boot FORCE
> >>  	$(call if_changed,objcopy)
> >>  else ifeq ($(CONFIG_TARGET_SOCRATES),y)
> >> -u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
> >> +u-boot-dtb.bin: u-boot-nodtb.bin u-boot.dtb FORCE
> >>  	$(call if_changed,binman)
> >>  endif
> >>  
> >> diff --git a/arch/powerpc/dts/socrates-u-boot.dtsi b/arch/powerpc/dts/socrates-u-boot.dtsi
> >> index ba2e56d35675..f6af611b513c 100644
> >> --- a/arch/powerpc/dts/socrates-u-boot.dtsi
> >> +++ b/arch/powerpc/dts/socrates-u-boot.dtsi
> >> @@ -9,7 +9,7 @@
> >>  		pad-byte = <0xff>;
> >>  		// Place dtb one sector before u-boot-nodtb.bin
> >>  		blob {
> >> -			filename = "dts/dt.dtb";
> >> +			filename = "u-boot.dtb";
> >>  		};
> >>  		u-boot-nodtb {
> >>  			filename = "u-boot-nodtb.bin";
> >>
> >>
> >> Heiko, could you try to put commit 499fe577c8011dd8a9184548c419db42aef079a7
> >> and above patch to your branch and retest it again?
> > 
> > Of course! Just have to wait until other azure build finished...
> > 
> > In the meantime I rebased my patchset to current HEAD, so above
> > commit is already in, and I had to adapt some patches for socrates.
> > 
> > I write the results hopefully soon.
> 
> Azure build successfully finished:
> https://dev.azure.com/hs0298/hs/_build/results?buildId=95&view=results
> 
> U-Boot tree (contains the discussed fix on top):
> https://github.com/hsdenx/u-boot-test/commits/socrates-2023.01-v1

Perfect, so it really fixed above issue.

> So, would you send a v2 of your 2 patches, including the above fix?

Above fix should be squashed into patch 1/2. I can do it and send v2.

But the question is if you want to include and merge also patch 2/2?

> If so, I pick them than up, and run a second azure build with them,
> before I post the socrates board updates...
> 
> Else I can add the fix to my series...
> 
> Thanks!
> 
> bye,
> Heiko
> 
> -- 
> DENX Software Engineering GmbH,      Managing Director: Erika Unter
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs@denx.de

Re: [PATCH u-boot 2/2] Makefile: Build working u-boot-dtb.bin target also for mpc85xx

[Heiko Schocher]

Hello Pali,

On 12.01.23 18:39, Pali Rohár wrote:
> On Thursday 12 January 2023 11:50:32 Heiko Schocher wrote:
>> Hello Pali,
>>
>> On 12.01.23 07:27, Heiko Schocher wrote:
>>> Hello Pali,
>>>
>>> On 11.01.23 19:13, Pali Rohár wrote:
>>>> On Wednesday 11 January 2023 19:02:38 Pali Rohár wrote:
>>>>> On Wednesday 11 January 2023 18:55:40 Pali Rohár wrote:
>>>>>> On Wednesday 11 January 2023 09:01:37 Tom Rini wrote:
>>>>>>> On Wed, Jan 11, 2023 at 01:52:24PM +0100, Heiko Schocher wrote:
>>>>>>>> Hello Pali, Tom,
>>>>>>>>
>>>>>>>> I just tried azure build with my socrates board updates based on
>>>>>>>> v2023.01 and the 2 patches from Pali:
>>>>>>>>
>>>>>>>> http://patchwork.ozlabs.org/project/uboot/patch/20221228200437.30971-1-pali@kernel.org/
>>>>>>>> http://patchwork.ozlabs.org/project/uboot/patch/20221228200437.30971-2-pali@kernel.org/
>>>>>>
>>>>>> At the time when I sent those two patches to ML, I checked that P1/P2
>>>>>> powerpc boards and also socrates board compiles successfully.
>>>>>>
>>>>>> Now I imported those two patches on top of the current master branch and
>>>>>> they still compiles without any problems for socrates board.
>>>>>>
>>>>>>>> and get errors for powerpc build:
>>>>>>>>
>>>>>>>> https://dev.azure.com/hs0298/110c3e42-44d5-4db4-9bd5-8a8bbead15f3/_apis/build/builds/93/logs/601
>>>>>>>>
>>>>>>>> socrates board builds fine ... my patches are socrates board specfic,
>>>>>>>> so hopefully no impact for other boards ...
>>>>>>
>>>>>> From that build log it looks like that u-boot fails for socrates and
>>>>>> kmcent2 board. Which is strange as you said that too that socrates is
>>>>>> building fine...
>>>
>>> misreaded azure output, so socrates is failing because missing u-boot.dtb,
>>> sorry. Interesting is, that my yocto build works ...
>>>
>>>>>
>>>>> kmcent2 is expected to fail with my above two patches on top of the
>>>>> v2023.01 without this kmcent2 commit which is now already in master:
>>>>> https://source.denx.de/u-boot/u-boot/-/commit/499fe577c8011dd8a9184548c419db42aef079a7
>>>>
>>>> And now I think I see the reason why it is failing also for socrates
>>>> board. Error in the build log is:
>>>>
>>>> 2023-01-11T12:16:04.4937207Z +binman: [Errno 2] No such file or directory: 'u-boot.dtb'
>>>> 2023-01-11T12:16:04.4937685Z +make[1]: *** [Makefile:1613: u-boot-dtb.bin] Error 1
>>>
>>> Yup.
>>>
>>>> u-boot.dtb builds make by Makefile rule:
>>>>
>>>> u-boot.dtb: dts/dt.dtb
>>>> 	$(call cmd,copy)
>>>>
>>>> But socrates-u-boot.dtsi has specified that use dts/dt.dtb and this
>>>> dependency is also specified in Makefile.
>>>>
>>>> And it looks like that binman needs also u-boot.dtb file. So it is
>>>> possible to hit a race condition, that make builds u-boot.dtb later than
>>>> rule for binman.
>>>
>>> Exactly over this I stumbled yesterday in the evening and I made a local
>>> fix:
>>>
>>> diff --git a/Makefile b/Makefile
>>> index fb1454552a..60f5cffccd 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -1609,7 +1609,7 @@ OBJCOPYFLAGS_u-boot-br.bin := -O binary -j .bootpg -j .resetvec
>>>  u-boot-br.bin: u-boot FORCE
>>>         $(call if_changed,objcopy)
>>>  else ifeq ($(CONFIG_TARGET_SOCRATES),y)
>>> -u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
>>> +u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb u-boot.dtb FORCE
>>>         $(call if_changed,binman)
>>>  endif
>>>
>>>
>>>> I would suggest to try to apply this patch, which should instruct make
>>>> to do not call binman until u-boot.dtb is correctly built:
>>>>
>>>> diff --git a/Makefile b/Makefile
>>>> index 3c76486a620e..5d2ef8cc81c5 100644
>>>> --- a/Makefile
>>>> +++ b/Makefile
>>>> @@ -1603,7 +1603,7 @@ OBJCOPYFLAGS_u-boot-br.bin := -O binary -j .bootpg -j .resetvec
>>>>  u-boot-br.bin: u-boot FORCE
>>>>  	$(call if_changed,objcopy)
>>>>  else ifeq ($(CONFIG_TARGET_SOCRATES),y)
>>>> -u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
>>>> +u-boot-dtb.bin: u-boot-nodtb.bin u-boot.dtb FORCE
>>>>  	$(call if_changed,binman)
>>>>  endif
>>>>  
>>>> diff --git a/arch/powerpc/dts/socrates-u-boot.dtsi b/arch/powerpc/dts/socrates-u-boot.dtsi
>>>> index ba2e56d35675..f6af611b513c 100644
>>>> --- a/arch/powerpc/dts/socrates-u-boot.dtsi
>>>> +++ b/arch/powerpc/dts/socrates-u-boot.dtsi
>>>> @@ -9,7 +9,7 @@
>>>>  		pad-byte = <0xff>;
>>>>  		// Place dtb one sector before u-boot-nodtb.bin
>>>>  		blob {
>>>> -			filename = "dts/dt.dtb";
>>>> +			filename = "u-boot.dtb";
>>>>  		};
>>>>  		u-boot-nodtb {
>>>>  			filename = "u-boot-nodtb.bin";
>>>>
>>>>
>>>> Heiko, could you try to put commit 499fe577c8011dd8a9184548c419db42aef079a7
>>>> and above patch to your branch and retest it again?
>>>
>>> Of course! Just have to wait until other azure build finished...
>>>
>>> In the meantime I rebased my patchset to current HEAD, so above
>>> commit is already in, and I had to adapt some patches for socrates.
>>>
>>> I write the results hopefully soon.
>>
>> Azure build successfully finished:
>> https://dev.azure.com/hs0298/hs/_build/results?buildId=95&view=results
>>
>> U-Boot tree (contains the discussed fix on top):
>> https://github.com/hsdenx/u-boot-test/commits/socrates-2023.01-v1
> 
> Perfect, so it really fixed above issue.

Yep!

>> So, would you send a v2 of your 2 patches, including the above fix?
> 
> Above fix should be squashed into patch 1/2. I can do it and send v2.

Great, so I wait for it and retest my patches with it.

> But the question is if you want to include and merge also patch 2/2?

I tested with this patch ... yes.

bye,
Heiko
>> If so, I pick them than up, and run a second azure build with them,
>> before I post the socrates board updates...
>>
>> Else I can add the fix to my series...
>>
>> Thanks!
>>
>> bye,
>> Heiko
>>
>> -- 
>> DENX Software Engineering GmbH,      Managing Director: Erika Unter
>> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>> Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs@denx.de

-- 
DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs@denx.de

Re: [PATCH u-boot 2/2] Makefile: Build working u-boot-dtb.bin target also for mpc85xx

[Heiko Schocher]

Hello Tom,

On 11.01.23 15:01, Tom Rini wrote:
> On Wed, Jan 11, 2023 at 01:52:24PM +0100, Heiko Schocher wrote:
>> Hello Pali, Tom,
>>
>> I just tried azure build with my socrates board updates based on
>> v2023.01 and the 2 patches from Pali:
>>
>> http://patchwork.ozlabs.org/project/uboot/patch/20221228200437.30971-1-pali@kernel.org/
>> http://patchwork.ozlabs.org/project/uboot/patch/20221228200437.30971-2-pali@kernel.org/
>>
>> and get errors for powerpc build:
>>
>> https://dev.azure.com/hs0298/110c3e42-44d5-4db4-9bd5-8a8bbead15f3/_apis/build/builds/93/logs/601
>>
>> socrates board builds fine ... my patches are socrates board specfic,
>> so hopefully no impact for other boards ...
> 
> socrates is one of the two failing boards, in that link:
> 2023-01-11T12:16:04.4930367Z    powerpc:  w+   MPC837XERDB kmcoge5ne
> kmeter1 kmopti2 kmsupx5 kmtepr2 tuge1 tuxx1 MPC8548CDS MPC8548CDS_36BIT
> MPC8548CDS_legacy P1010RDB-PA_36BIT_NAND P1010RDB-PA_36BIT_NOR
> P1010RDB-PA_36BIT_SDCARD P1010RDB-PA_36BIT_SPIFLASH P1010RDB-PA_NAND
> P1010RDB-PA_NOR P1010RDB-PA_SDCARD P1010RDB-PA_SPIFLASH
> P1010RDB-PB_36BIT_NAND P1010RDB-PB_36BIT_NOR P1010RDB-PB_36BIT_SDCARD
> P1010RDB-PB_36BIT_SPIFLASH P1010RDB-PB_NAND P1010RDB-PB_NOR
> P1010RDB-PB_SDCARD P1010RDB-PB_SPIFLASH P1020RDB-PC P1020RDB-PC_36BIT
> P1020RDB-PC_36BIT_NAND P1020RDB-PC_36BIT_SDCARD
> P1020RDB-PC_36BIT_SPIFLASH P1020RDB-PC_NAND P1020RDB-PC_SDCARD
> P1020RDB-PC_SPIFLASH P1020RDB-PD P1020RDB-PD_NAND P1020RDB-PD_SDCARD
> P1020RDB-PD_SPIFLASH P2020RDB-PC P2020RDB-PC_36BIT
> P2020RDB-PC_36BIT_NAND P2020RDB-PC_36BIT_SDCARD
> P2020RDB-PC_36BIT_SPIFLASH P2020RDB-PC_NAND P2020RDB-PC_SDCARD
> P2020RDB-PC_SPIFLASH P2041RDB P2041RDB_NAND P2041RDB_SDCARD
> P2041RDB_SPIFLASH T1024RDB T1024RDB_NAND T1024RDB_SDCARD
> T1024RDB_SPIFLASH T1042D4RDB T1042D4RDB_NAND T1042D4RDB_SDCARD
> T1042D4RDB_SPIFLASH T2080QDS T2080QDS_NAND T2080QDS_SDCARD
> T2080QDS_SECURE_BOOT T2080QDS_SPIFLASH T2080QDS_SRIO_PCIE_BOOT T2080RDB
> T2080RDB_NAND T2080RDB_revD T2080RDB_revD_NAND T2080RDB_revD_SDCARD
> T2080RDB_revD_SPIFLASH T2080RDB_SDCARD T2080RDB_SPIFLASH T4240RDB
> T4240RDB_SDCARD +   socrates kmcent2

Yep, noted this later ... and have local a fix for socrates
board, but first just started a build with patches from pali
to be sure, that I did not introduced some mistake ...
> 
>> @Tom: Do you know if v2023.01 builds fine for powerpc
> 
> Yes, CI is passing.

Ok, fine, so I do not start one ... just started one with patches
from pali.

Thanks for the info!

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs@denx.de

[PATCH v2 u-boot 1/3] powerpc/mpc85xx: socrates: Use u-boot.dtb instead of dts/dt.dtb

[Pali Rohár]

binman uses DTB file u-boot.dtb for building u-boot binaries. So use same
DTB file for u-boot runtime on socrates board, instead of dts/dt.dtb file.

Note that u-boot.dtb is generated by simple Makefile rule which copies
dts/dt.dtb to u-boot.dtb. So both files are same.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 arch/powerpc/dts/socrates-u-boot.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/dts/socrates-u-boot.dtsi b/arch/powerpc/dts/socrates-u-boot.dtsi
index 14a7c245dc46..92e89a11bc01 100644
--- a/arch/powerpc/dts/socrates-u-boot.dtsi
+++ b/arch/powerpc/dts/socrates-u-boot.dtsi
@@ -9,7 +9,7 @@
 		pad-byte = <0xff>;
 		// Place dtb one sector before u-boot-nodtb.bin
 		blob {
-			filename = "dts/dt.dtb";
+			filename = "u-boot.dtb";
 		};
 		u-boot-nodtb {
 			filename = "u-boot-nodtb.bin";
-- 
2.20.1

[PATCH v2 u-boot 2/3] powerpc/mpc85xx: socrates: Rename u-boot-socrates.bin to u-boot.bin

[Pali Rohár]

U-Boot build process for socrates board produces final U-Boot binary in
file u-boot-socrates.bin (by binman) And as a bonus it produces two
unusable broken binaries u-boot-dtb.bin and u-boot.bin (by Makefile).

So do not build broken u-boot-dtb.bin and u-boot.bin binaries and rename
board specific u-boot-socrates.bin binary to u-boot.bin.

Renaming requires to define a new socrates specific Makefile target for
u-boot.bin (via binman) and also changing output name in socrates binman
config file.

With this change U-Boot build process for socrates board also produces
final U-Boot binary in file u-boot.bin.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
Added make dependency on u-boot.dtb
---
 Makefile                              | 11 +++++++++++
 arch/powerpc/dts/socrates-u-boot.dtsi |  2 +-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index a4a14d5d35a8..5473bea25332 100644
--- a/Makefile
+++ b/Makefile
@@ -1195,22 +1195,30 @@ endif
 u-boot.bin: u-boot-fit-dtb.bin FORCE
 	$(call if_changed,copy)
 
+ifneq ($(CONFIG_TARGET_SOCRATES),y)
 u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
 	$(call if_changed,cat)
+endif
 
 else ifeq ($(CONFIG_OF_SEPARATE).$(CONFIG_OF_OMIT_DTB),y.)
+ifneq ($(CONFIG_TARGET_SOCRATES),y)
 u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
 	$(call if_changed,cat)
+endif
 
 ifneq ($(CONFIG_MPC85XX_HAVE_RESET_VECTOR)$(CONFIG_OF_SEPARATE),yy)
+ifneq ($(CONFIG_TARGET_SOCRATES),y)
 u-boot.bin: u-boot-dtb.bin FORCE
 	$(call if_changed,copy)
 endif
+endif
 
 else ifneq ($(CONFIG_MPC85XX_HAVE_RESET_VECTOR)$(CONFIG_OF_SEPARATE),yy)
+ifneq ($(CONFIG_TARGET_SOCRATES),y)
 u-boot.bin: u-boot-nodtb.bin FORCE
 	$(call if_changed,copy)
 endif
+endif
 
 # we call Makefile in arch/arm/mach-imx which
 # has targets which are dependent on targets defined
@@ -1595,6 +1603,9 @@ u-boot.bin: u-boot-nodtb.bin u-boot.dtb u-boot-br.bin FORCE
 OBJCOPYFLAGS_u-boot-br.bin := -O binary -j .bootpg -j .resetvec
 u-boot-br.bin: u-boot FORCE
 	$(call if_changed,objcopy)
+else ifeq ($(CONFIG_TARGET_SOCRATES),y)
+u-boot.bin: u-boot-nodtb.bin u-boot.dtb FORCE
+	$(call if_changed,binman)
 endif
 
 quiet_cmd_ldr = LD      $@
diff --git a/arch/powerpc/dts/socrates-u-boot.dtsi b/arch/powerpc/dts/socrates-u-boot.dtsi
index 92e89a11bc01..b012201a32bd 100644
--- a/arch/powerpc/dts/socrates-u-boot.dtsi
+++ b/arch/powerpc/dts/socrates-u-boot.dtsi
@@ -5,7 +5,7 @@
  */
 / {
 	binman {
-		filename = "u-boot-socrates.bin";
+		filename = "u-boot.bin";
 		pad-byte = <0xff>;
 		// Place dtb one sector before u-boot-nodtb.bin
 		blob {
-- 
2.20.1

Re: [PATCH v2 u-boot 2/3] powerpc/mpc85xx: socrates: Rename u-boot-socrates.bin to u-boot.bin

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2257 bytes --]

On Sat, Jan 14, 2023 at 12:11:22AM +0100, Pali Rohár wrote:
> U-Boot build process for socrates board produces final U-Boot binary in
> file u-boot-socrates.bin (by binman) And as a bonus it produces two
> unusable broken binaries u-boot-dtb.bin and u-boot.bin (by Makefile).
> 
> So do not build broken u-boot-dtb.bin and u-boot.bin binaries and rename
> board specific u-boot-socrates.bin binary to u-boot.bin.
> 
> Renaming requires to define a new socrates specific Makefile target for
> u-boot.bin (via binman) and also changing output name in socrates binman
> config file.
> 
> With this change U-Boot build process for socrates board also produces
> final U-Boot binary in file u-boot.bin.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
> Added make dependency on u-boot.dtb
> ---
>  Makefile                              | 11 +++++++++++
>  arch/powerpc/dts/socrates-u-boot.dtsi |  2 +-
>  2 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index a4a14d5d35a8..5473bea25332 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1195,22 +1195,30 @@ endif
>  u-boot.bin: u-boot-fit-dtb.bin FORCE
>  	$(call if_changed,copy)
>  
> +ifneq ($(CONFIG_TARGET_SOCRATES),y)
>  u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
>  	$(call if_changed,cat)
> +endif
>  
>  else ifeq ($(CONFIG_OF_SEPARATE).$(CONFIG_OF_OMIT_DTB),y.)
> +ifneq ($(CONFIG_TARGET_SOCRATES),y)
>  u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
>  	$(call if_changed,cat)
> +endif
>  
>  ifneq ($(CONFIG_MPC85XX_HAVE_RESET_VECTOR)$(CONFIG_OF_SEPARATE),yy)
> +ifneq ($(CONFIG_TARGET_SOCRATES),y)
>  u-boot.bin: u-boot-dtb.bin FORCE
>  	$(call if_changed,copy)
>  endif
> +endif
>  
>  else ifneq ($(CONFIG_MPC85XX_HAVE_RESET_VECTOR)$(CONFIG_OF_SEPARATE),yy)
> +ifneq ($(CONFIG_TARGET_SOCRATES),y)
>  u-boot.bin: u-boot-nodtb.bin FORCE
>  	$(call if_changed,copy)
>  endif
> +endif

Simon's point from before still stands. This is the opposite of what we
want. There must not be CONFIG_TARGET_ logic introduced to the
top-level Makefile. socrate is "just" another mpc85xx platform, it
doesn't have a special ROM, we need to adjust it back to acting like
other platforms.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v2 u-boot 2/3] powerpc/mpc85xx: socrates: Rename u-boot-socrates.bin to u-boot.bin

[Pali Rohár]

On Friday 13 January 2023 18:16:03 Tom Rini wrote:
> On Sat, Jan 14, 2023 at 12:11:22AM +0100, Pali Rohár wrote:
> > U-Boot build process for socrates board produces final U-Boot binary in
> > file u-boot-socrates.bin (by binman) And as a bonus it produces two
> > unusable broken binaries u-boot-dtb.bin and u-boot.bin (by Makefile).
> > 
> > So do not build broken u-boot-dtb.bin and u-boot.bin binaries and rename
> > board specific u-boot-socrates.bin binary to u-boot.bin.
> > 
> > Renaming requires to define a new socrates specific Makefile target for
> > u-boot.bin (via binman) and also changing output name in socrates binman
> > config file.
> > 
> > With this change U-Boot build process for socrates board also produces
> > final U-Boot binary in file u-boot.bin.
> > 
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > ---
> > Added make dependency on u-boot.dtb
> > ---
> >  Makefile                              | 11 +++++++++++
> >  arch/powerpc/dts/socrates-u-boot.dtsi |  2 +-
> >  2 files changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Makefile b/Makefile
> > index a4a14d5d35a8..5473bea25332 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1195,22 +1195,30 @@ endif
> >  u-boot.bin: u-boot-fit-dtb.bin FORCE
> >  	$(call if_changed,copy)
> >  
> > +ifneq ($(CONFIG_TARGET_SOCRATES),y)
> >  u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
> >  	$(call if_changed,cat)
> > +endif
> >  
> >  else ifeq ($(CONFIG_OF_SEPARATE).$(CONFIG_OF_OMIT_DTB),y.)
> > +ifneq ($(CONFIG_TARGET_SOCRATES),y)
> >  u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
> >  	$(call if_changed,cat)
> > +endif
> >  
> >  ifneq ($(CONFIG_MPC85XX_HAVE_RESET_VECTOR)$(CONFIG_OF_SEPARATE),yy)
> > +ifneq ($(CONFIG_TARGET_SOCRATES),y)
> >  u-boot.bin: u-boot-dtb.bin FORCE
> >  	$(call if_changed,copy)
> >  endif
> > +endif
> >  
> >  else ifneq ($(CONFIG_MPC85XX_HAVE_RESET_VECTOR)$(CONFIG_OF_SEPARATE),yy)
> > +ifneq ($(CONFIG_TARGET_SOCRATES),y)
> >  u-boot.bin: u-boot-nodtb.bin FORCE
> >  	$(call if_changed,copy)
> >  endif
> > +endif
> 
> Simon's point from before still stands. This is the opposite of what we
> want. There must not be CONFIG_TARGET_ logic introduced to the
> top-level Makefile. socrate is "just" another mpc85xx platform, it
> doesn't have a special ROM, we need to adjust it back to acting like
> other platforms.

socrates has its own flash layout, own build procedure and purpose of
this patch is just to prevent another breakage (like it was done in the
past) by throwing make errors.

Trying to adjust board code and changing its layout is really not up to
me. I do not have this board.

One there is generic binman build rules from make then it can be
switches to that generic binman rule. Until it happen there is not
better option...

Re: [PATCH v2 u-boot 2/3] powerpc/mpc85xx: socrates: Rename u-boot-socrates.bin to u-boot.bin

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 3442 bytes --]

On Sat, Jan 14, 2023 at 10:12:06PM +0100, Pali Rohár wrote:
> On Friday 13 January 2023 18:16:03 Tom Rini wrote:
> > On Sat, Jan 14, 2023 at 12:11:22AM +0100, Pali Rohár wrote:
> > > U-Boot build process for socrates board produces final U-Boot binary in
> > > file u-boot-socrates.bin (by binman) And as a bonus it produces two
> > > unusable broken binaries u-boot-dtb.bin and u-boot.bin (by Makefile).
> > > 
> > > So do not build broken u-boot-dtb.bin and u-boot.bin binaries and rename
> > > board specific u-boot-socrates.bin binary to u-boot.bin.
> > > 
> > > Renaming requires to define a new socrates specific Makefile target for
> > > u-boot.bin (via binman) and also changing output name in socrates binman
> > > config file.
> > > 
> > > With this change U-Boot build process for socrates board also produces
> > > final U-Boot binary in file u-boot.bin.
> > > 
> > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > ---
> > > Added make dependency on u-boot.dtb
> > > ---
> > >  Makefile                              | 11 +++++++++++
> > >  arch/powerpc/dts/socrates-u-boot.dtsi |  2 +-
> > >  2 files changed, 12 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/Makefile b/Makefile
> > > index a4a14d5d35a8..5473bea25332 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -1195,22 +1195,30 @@ endif
> > >  u-boot.bin: u-boot-fit-dtb.bin FORCE
> > >  	$(call if_changed,copy)
> > >  
> > > +ifneq ($(CONFIG_TARGET_SOCRATES),y)
> > >  u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
> > >  	$(call if_changed,cat)
> > > +endif
> > >  
> > >  else ifeq ($(CONFIG_OF_SEPARATE).$(CONFIG_OF_OMIT_DTB),y.)
> > > +ifneq ($(CONFIG_TARGET_SOCRATES),y)
> > >  u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
> > >  	$(call if_changed,cat)
> > > +endif
> > >  
> > >  ifneq ($(CONFIG_MPC85XX_HAVE_RESET_VECTOR)$(CONFIG_OF_SEPARATE),yy)
> > > +ifneq ($(CONFIG_TARGET_SOCRATES),y)
> > >  u-boot.bin: u-boot-dtb.bin FORCE
> > >  	$(call if_changed,copy)
> > >  endif
> > > +endif
> > >  
> > >  else ifneq ($(CONFIG_MPC85XX_HAVE_RESET_VECTOR)$(CONFIG_OF_SEPARATE),yy)
> > > +ifneq ($(CONFIG_TARGET_SOCRATES),y)
> > >  u-boot.bin: u-boot-nodtb.bin FORCE
> > >  	$(call if_changed,copy)
> > >  endif
> > > +endif
> > 
> > Simon's point from before still stands. This is the opposite of what we
> > want. There must not be CONFIG_TARGET_ logic introduced to the
> > top-level Makefile. socrate is "just" another mpc85xx platform, it
> > doesn't have a special ROM, we need to adjust it back to acting like
> > other platforms.
> 
> socrates has its own flash layout, own build procedure and purpose of
> this patch is just to prevent another breakage (like it was done in the
> past) by throwing make errors.
> 
> Trying to adjust board code and changing its layout is really not up to
> me. I do not have this board.
> 
> One there is generic binman build rules from make then it can be
> switches to that generic binman rule. Until it happen there is not
> better option...

Yes, it should be Heiko, as the board maintainer, dealing with fixing
this part. I don't understand the flash layout, and the partition table
laid out in arch/powerpc/dts/socrates.dts confuses things even more to
me. But, the board maintainer should be able to sort this all out.
Because we do not want to add CONFIG_TARGET_ logic to the top-level
Makefile.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v2 u-boot 2/3] powerpc/mpc85xx: socrates: Rename u-boot-socrates.bin to u-boot.bin

[Heiko Schocher]

Hello Tom, Pali,

On 14.01.23 22:24, Tom Rini wrote:
> On Sat, Jan 14, 2023 at 10:12:06PM +0100, Pali Rohár wrote:
>> On Friday 13 January 2023 18:16:03 Tom Rini wrote:
>>> On Sat, Jan 14, 2023 at 12:11:22AM +0100, Pali Rohár wrote:
>>>> U-Boot build process for socrates board produces final U-Boot binary in
>>>> file u-boot-socrates.bin (by binman) And as a bonus it produces two
>>>> unusable broken binaries u-boot-dtb.bin and u-boot.bin (by Makefile).
>>>>
>>>> So do not build broken u-boot-dtb.bin and u-boot.bin binaries and rename
>>>> board specific u-boot-socrates.bin binary to u-boot.bin.
>>>>
>>>> Renaming requires to define a new socrates specific Makefile target for
>>>> u-boot.bin (via binman) and also changing output name in socrates binman
>>>> config file.
>>>>
>>>> With this change U-Boot build process for socrates board also produces
>>>> final U-Boot binary in file u-boot.bin.
>>>>
>>>> Signed-off-by: Pali Rohár <pali@kernel.org>
>>>> ---
>>>> Added make dependency on u-boot.dtb
>>>> ---
>>>>  Makefile                              | 11 +++++++++++
>>>>  arch/powerpc/dts/socrates-u-boot.dtsi |  2 +-
>>>>  2 files changed, 12 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Makefile b/Makefile
>>>> index a4a14d5d35a8..5473bea25332 100644
>>>> --- a/Makefile
>>>> +++ b/Makefile
>>>> @@ -1195,22 +1195,30 @@ endif
>>>>  u-boot.bin: u-boot-fit-dtb.bin FORCE
>>>>  	$(call if_changed,copy)
>>>>  
>>>> +ifneq ($(CONFIG_TARGET_SOCRATES),y)
>>>>  u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
>>>>  	$(call if_changed,cat)
>>>> +endif
>>>>  
>>>>  else ifeq ($(CONFIG_OF_SEPARATE).$(CONFIG_OF_OMIT_DTB),y.)
>>>> +ifneq ($(CONFIG_TARGET_SOCRATES),y)
>>>>  u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
>>>>  	$(call if_changed,cat)
>>>> +endif
>>>>  
>>>>  ifneq ($(CONFIG_MPC85XX_HAVE_RESET_VECTOR)$(CONFIG_OF_SEPARATE),yy)
>>>> +ifneq ($(CONFIG_TARGET_SOCRATES),y)
>>>>  u-boot.bin: u-boot-dtb.bin FORCE
>>>>  	$(call if_changed,copy)
>>>>  endif
>>>> +endif
>>>>  
>>>>  else ifneq ($(CONFIG_MPC85XX_HAVE_RESET_VECTOR)$(CONFIG_OF_SEPARATE),yy)
>>>> +ifneq ($(CONFIG_TARGET_SOCRATES),y)
>>>>  u-boot.bin: u-boot-nodtb.bin FORCE
>>>>  	$(call if_changed,copy)
>>>>  endif
>>>> +endif
>>>
>>> Simon's point from before still stands. This is the opposite of what we
>>> want. There must not be CONFIG_TARGET_ logic introduced to the
>>> top-level Makefile. socrate is "just" another mpc85xx platform, it
>>> doesn't have a special ROM, we need to adjust it back to acting like
>>> other platforms.
>>
>> socrates has its own flash layout, own build procedure and purpose of
>> this patch is just to prevent another breakage (like it was done in the
>> past) by throwing make errors.
>>
>> Trying to adjust board code and changing its layout is really not up to
>> me. I do not have this board.
>>
>> One there is generic binman build rules from make then it can be
>> switches to that generic binman rule. Until it happen there is not
>> better option...
> 
> Yes, it should be Heiko, as the board maintainer, dealing with fixing
> this part. I don't understand the flash layout, and the partition table
> laid out in arch/powerpc/dts/socrates.dts confuses things even more to
> me. But, the board maintainer should be able to sort this all out.
> Because we do not want to add CONFIG_TARGET_ logic to the top-level
> Makefile.

Seems I missed this point from Simon... I take a look into it!

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs@denx.de

[PATCH v2 u-boot 3/3] Makefile: Build working u-boot-dtb.bin target also for mpc85xx

[Pali Rohár]

U-Boot build process currently always produces broken u-boot-dtb.bin binary
for PowerPC mpc85xx architecture on boards which needs mpc85xx reset
vector. For these boards this (intermediate) binary is not used as input
for any other Makefile target on this architecture, so there is no real
problem with it.

But it is not a good idea to produce broken binaries during build phase. So
try to improve it. Binary u-boot-dtb.bin should contains u-boot code with
DTB blob. Such binary for those boards is build by binman. So change binman
output file name from u-boot.bin to u-boot-dtb.bin and then let generic
Makefile rule to generate final u-boot.bin from u-boot-dtb.bin. And finally
disable generic u-boot-dtb.bin rule for mpc85xx.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 Makefile                              | 17 ++++++++---------
 arch/powerpc/dts/socrates-u-boot.dtsi |  2 +-
 arch/powerpc/dts/u-boot.dtsi          |  2 +-
 3 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/Makefile b/Makefile
index 5473bea25332..5d2ef8cc81c5 100644
--- a/Makefile
+++ b/Makefile
@@ -1195,30 +1195,29 @@ endif
 u-boot.bin: u-boot-fit-dtb.bin FORCE
 	$(call if_changed,copy)
 
+ifneq ($(CONFIG_MPC85XX_HAVE_RESET_VECTOR)$(CONFIG_OF_SEPARATE),yy)
 ifneq ($(CONFIG_TARGET_SOCRATES),y)
 u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
 	$(call if_changed,cat)
 endif
+endif
 
 else ifeq ($(CONFIG_OF_SEPARATE).$(CONFIG_OF_OMIT_DTB),y.)
+
+ifneq ($(CONFIG_MPC85XX_HAVE_RESET_VECTOR)$(CONFIG_OF_SEPARATE),yy)
 ifneq ($(CONFIG_TARGET_SOCRATES),y)
 u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
 	$(call if_changed,cat)
 endif
+endif
 
-ifneq ($(CONFIG_MPC85XX_HAVE_RESET_VECTOR)$(CONFIG_OF_SEPARATE),yy)
-ifneq ($(CONFIG_TARGET_SOCRATES),y)
 u-boot.bin: u-boot-dtb.bin FORCE
 	$(call if_changed,copy)
-endif
-endif
 
-else ifneq ($(CONFIG_MPC85XX_HAVE_RESET_VECTOR)$(CONFIG_OF_SEPARATE),yy)
-ifneq ($(CONFIG_TARGET_SOCRATES),y)
+else
 u-boot.bin: u-boot-nodtb.bin FORCE
 	$(call if_changed,copy)
 endif
-endif
 
 # we call Makefile in arch/arm/mach-imx which
 # has targets which are dependent on targets defined
@@ -1597,14 +1596,14 @@ u-boot-with-nand-spl.sfp: u-boot-spl-padx4.sfp u-boot.img FORCE
 endif
 
 ifeq ($(CONFIG_MPC85XX_HAVE_RESET_VECTOR)$(CONFIG_OF_SEPARATE),yy)
-u-boot.bin: u-boot-nodtb.bin u-boot.dtb u-boot-br.bin FORCE
+u-boot-dtb.bin: u-boot-nodtb.bin u-boot.dtb u-boot-br.bin FORCE
 	$(call if_changed,binman)
 
 OBJCOPYFLAGS_u-boot-br.bin := -O binary -j .bootpg -j .resetvec
 u-boot-br.bin: u-boot FORCE
 	$(call if_changed,objcopy)
 else ifeq ($(CONFIG_TARGET_SOCRATES),y)
-u-boot.bin: u-boot-nodtb.bin u-boot.dtb FORCE
+u-boot-dtb.bin: u-boot-nodtb.bin u-boot.dtb FORCE
 	$(call if_changed,binman)
 endif
 
diff --git a/arch/powerpc/dts/socrates-u-boot.dtsi b/arch/powerpc/dts/socrates-u-boot.dtsi
index b012201a32bd..f6af611b513c 100644
--- a/arch/powerpc/dts/socrates-u-boot.dtsi
+++ b/arch/powerpc/dts/socrates-u-boot.dtsi
@@ -5,7 +5,7 @@
  */
 / {
 	binman {
-		filename = "u-boot.bin";
+		filename = "u-boot-dtb.bin";
 		pad-byte = <0xff>;
 		// Place dtb one sector before u-boot-nodtb.bin
 		blob {
diff --git a/arch/powerpc/dts/u-boot.dtsi b/arch/powerpc/dts/u-boot.dtsi
index 6b7375cff215..c39ab6f0cacd 100644
--- a/arch/powerpc/dts/u-boot.dtsi
+++ b/arch/powerpc/dts/u-boot.dtsi
@@ -9,7 +9,7 @@
 
 / {
 	binman {
-		filename = "u-boot.bin";
+		filename = "u-boot-dtb.bin";
 		skip-at-start = <CONFIG_TEXT_BASE>;
 		sort-by-offset;
 		pad-byte = <0xff>;
-- 
2.20.1