| Message ID | 20230113231123.27427-2-pali@kernel.org |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [v2,u-boot,1/3] powerpc/mpc85xx: socrates: Use u-boot.dtb instead of dts/dt.dtb | expand |
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.
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...
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.
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
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 {
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(-)