diff mbox series

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

Message ID 20221228200437.30971-2-pali@kernel.org
State Changes Requested
Delegated to: Tom Rini
Headers show
Series [u-boot,1/2] powerpc/mpc85xx: socrates: Rename u-boot-socrates.bin to u-boot.bin | expand

Commit Message

Pali Rohár Dec. 28, 2022, 8:04 p.m. UTC
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(-)

Comments

Simon Glass Dec. 29, 2022, 10:39 p.m. UTC | #1
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
Pali Rohár Dec. 30, 2022, 12:48 p.m. UTC | #2
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.
Tom Rini Dec. 30, 2022, 3:21 p.m. UTC | #3
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?
Pali Rohár Dec. 30, 2022, 3:24 p.m. UTC | #4
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.
Tom Rini Dec. 30, 2022, 3:41 p.m. UTC | #5
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".
Pali Rohár Dec. 30, 2022, 3:44 p.m. UTC | #6
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.
Simon Glass Dec. 30, 2022, 3:49 p.m. UTC | #7
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
Pali Rohár Dec. 30, 2022, 4:06 p.m. UTC | #8
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
>
Tom Rini Dec. 30, 2022, 4:06 p.m. UTC | #9
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?
Simon Glass Dec. 30, 2022, 5:43 p.m. UTC | #10
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
Pali Rohár Dec. 30, 2022, 5:55 p.m. UTC | #11
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.
Simon Glass Dec. 30, 2022, 6:51 p.m. UTC | #12
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
Pali Rohár Dec. 30, 2022, 7:12 p.m. UTC | #13
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
Simon Glass Jan. 3, 2023, 5:02 p.m. UTC | #14
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
Pali Rohár Jan. 3, 2023, 5:05 p.m. UTC | #15
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.
Simon Glass Jan. 7, 2023, 12:13 a.m. UTC | #16
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
Pali Rohár Jan. 11, 2023, 12:12 a.m. UTC | #17
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.
Heiko Schocher Jan. 11, 2023, 10:08 a.m. UTC | #18
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
Heiko Schocher Jan. 11, 2023, 12:52 p.m. UTC | #19
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
Tom Rini Jan. 11, 2023, 2:01 p.m. UTC | #20
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.
Pali Rohár Jan. 11, 2023, 5:55 p.m. UTC | #21
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
Pali Rohár Jan. 11, 2023, 6:02 p.m. UTC | #22
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
> 
>
Pali Rohár Jan. 11, 2023, 6:13 p.m. UTC | #23
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
> > 
> >
Heiko Schocher Jan. 12, 2023, 5:27 a.m. UTC | #24
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
Heiko Schocher Jan. 12, 2023, 6:27 a.m. UTC | #25
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
>>>
>>>
Heiko Schocher Jan. 12, 2023, 10:50 a.m. UTC | #26
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
Pali Rohár Jan. 12, 2023, 5:39 p.m. UTC | #27
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
Heiko Schocher Jan. 13, 2023, 5:52 a.m. UTC | #28
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
diff mbox series

Patch

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