[PATCH v3 08/12] sunxi: Convert 64-bit boards to use binman
Simon Glass
sjg at chromium.org
Sun Sep 6 02:18:03 CEST 2020
Hi Samuel,
On Sat, 5 Sep 2020 at 17:10, Samuel Holland <samuel at sholland.org> wrote:
>
> On 9/1/20 6:14 AM, Simon Glass wrote:
> > At present 64-bit sunxi boards use the Makefile to create a FIT, using
> > USE_SPL_FIT_GENERATOR. This is deprecated.
> >
> > Update sunxi to use binman instead.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
> >
> > (no changes since v2)
> >
> > Changes in v2:
> > - Add a 'fit-fdt-list' property
> > - Fix 'board' typo in commit message
> >
> > Kconfig | 3 +-
> > Makefile | 18 ++--------
> > arch/arm/dts/sunxi-u-boot.dtsi | 61 +++++++++++++++++++++++++++++++++-
> > 3 files changed, 63 insertions(+), 19 deletions(-)
> >
> > diff --git a/Kconfig b/Kconfig
> > index 883e3f71d01..837b2f517ae 100644
> > --- a/Kconfig
> > +++ b/Kconfig
> > @@ -659,12 +659,11 @@ config SPL_FIT_SOURCE
> >
> > config USE_SPL_FIT_GENERATOR
> > bool "Use a script to generate the .its script"
> > - default y if SPL_FIT
> > + default y if SPL_FIT && !ARCH_SUNXI
>
> Now `make u-boot.itb` doesn't work.
>
> u-boot.itb is helpful to have because, with CONFIG_OF_LIST, it can be shared
> across all boards of a platform. Only SPL is board-specific (on arm64 sunxi, at
> least).
It is generated, just with a different filename.
>
> Is there a way to make binman also write the FIT without the SPL? Would that
> require duplicating the whole binman node?
Yes it would. We could get more complicated and allow an image to
build on another perhaps. I'm not sure what is easiest here.
>
> > config SPL_FIT_GENERATOR
> > string ".its file generator script for U-Boot FIT image"
> > depends on USE_SPL_FIT_GENERATOR
> > - default "board/sunxi/mksunxi_fit_atf.sh" if SPL_LOAD_FIT && ARCH_SUNXI
> > default "arch/arm/mach-rockchip/make_fit_atf.py" if SPL_LOAD_FIT && ARCH_ROCKCHIP
> > default "arch/arm/mach-zynqmp/mkimage_fit_atf.sh" if SPL_LOAD_FIT && ARCH_ZYNQMP
> > default "arch/riscv/lib/mkimage_fit_opensbi.sh" if SPL_LOAD_FIT && RISCV
> > diff --git a/Makefile b/Makefile
> > index 5b4e60496d6..65024c74089 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -923,11 +923,6 @@ INPUTS-$(CONFIG_REMAKE_ELF) += u-boot.elf
> > INPUTS-$(CONFIG_EFI_APP) += u-boot-app.efi
> > INPUTS-$(CONFIG_EFI_STUB) += u-boot-payload.efi
> >
> > -# Build a combined spl + u-boot image for sunxi
> > -ifeq ($(CONFIG_ARCH_SUNXI)$(CONFIG_ARM64)$(CONFIG_SPL),yyy)
> > -INPUTS-y += u-boot-sunxi-with-spl.bin
> > -endif
> > -
> > # Generate this input file for binman
> > ifeq ($(CONFIG_SPL),)
> > INPUTS-$(CONFIG_ARCH_MEDIATEK) += u-boot-mtk.bin
> > @@ -1024,13 +1019,9 @@ PHONY += inputs
> > inputs: $(INPUTS-y)
> >
> > all: .binman_stamp inputs
> > -# Hack for sunxi which doesn't have a proper binman definition for
> > -# 64-bit boards
> > -ifneq ($(CONFIG_ARCH_SUNXI)$(CONFIG_ARM64),yy)
> > ifeq ($(CONFIG_BINMAN),y)
> > $(call if_changed,binman)
> > endif
> > -endif
> >
> > # Timestamp file to make sure that binman always runs
> > .binman_stamp: FORCE
> > @@ -1336,6 +1327,8 @@ cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \
> > $(if $(BINMAN_VERBOSE),-v$(BINMAN_VERBOSE)) \
> > build -u -d u-boot.dtb -O . -m --allow-missing \
> > -I . -I $(srctree) -I $(srctree)/board/$(BOARDDIR) \
> > + -I arch/$(ARCH)/dts -a of-list=$(CONFIG_OF_LIST) \
> > + -a atf-bl31-path=${BL31} \
> > $(BINMAN_$(@F))
> >
> > OBJCOPYFLAGS_u-boot.ldr.hex := -I binary -O ihex
> > @@ -1625,13 +1618,6 @@ u-boot-x86-reset16.bin: u-boot FORCE
> >
> > endif # CONFIG_X86
> >
> > -ifneq ($(CONFIG_ARCH_SUNXI),)
> > -ifeq ($(CONFIG_ARM64),y)
> > -u-boot-sunxi-with-spl.bin: spl/sunxi-spl.bin u-boot.itb FORCE
> > - $(call if_changed,cat)
> > -endif
> > -endif
> > -
>
> Now `make u-boot-sunxi-with-spl.bin` doesn't work.
>
> This is less of an issue, but still probably breaks some scripts. It breaks
> mine, at least.
Why do you specify a target? Doesn't it build the file without the target?
One problem with buildman is that there is no definitely of what files
it will produce when run, or at least there is, but it is in the
binman description itself.
This means that 'make clean' doesn't work fully, for example. I can
think of a few ways to implement this. One would be to put a list of
target files into a text file and have 'make clean' use that. We could
also have an option to tell binman to produce a list of files it would
generate if run. Then we might be able to tell binman to generate a
particular file.
>
> > OBJCOPYFLAGS_u-boot-app.efi := $(OBJCOPYFLAGS_EFI)
> > u-boot-app.efi: u-boot FORCE
> > $(call if_changed,zobjcopy)
> > diff --git a/arch/arm/dts/sunxi-u-boot.dtsi b/arch/arm/dts/sunxi-u-boot.dtsi
> > index fdd4c80aa46..1d1c3691099 100644
> > --- a/arch/arm/dts/sunxi-u-boot.dtsi
> > +++ b/arch/arm/dts/sunxi-u-boot.dtsi
> > @@ -5,14 +5,73 @@
> > mmc1 = &mmc2;
> > };
> >
> > - binman {
> > + binman: binman {
> > + multiple-images;
> > + };
> > +};
> > +
> > +&binman {
> > + u-boot-sunxi-with-spl {
> > filename = "u-boot-sunxi-with-spl.bin";
> > pad-byte = <0xff>;
>
> style: blank line here (and above "atf" and "@config-SEQ" below).
I'll send a fix-up patch.
>
> > blob {
> > filename = "spl/sunxi-spl.bin";
> > };
> > +#ifdef CONFIG_ARM64
> > + fit {
> > + description = "Configuration to load ATF before U-Boot";
> > + #address-cells = <1>;
> > + fit,fdt-list = "of-list";
> > +
> > + images {
> > + uboot {
> > + description = "U-Boot (64-bit)";
> > + type = "standalone";
> > + arch = "arm64";
> > + compression = "none";
> > + load = <0x4a000000>;
> > +
> > + u-boot-nodtb {
> > + };
> > + };
> > + atf {
> > + description = "ARM Trusted Firmware";
> > + type = "firmware";
> > + arch = "arm64";
> > + compression = "none";
> > +/* TODO: Do this with an overwrite in this board's dtb? */
>
> This address is determined by the physical SRAM layout, so it is per-SoC, not
> per-board. I would suggest omitting load/entry here entirely, and putting them
> in the $SOC-u-boot.dtsi.
That's fine also. I just don't like having #ifdefs here.
>
> > +#ifdef CONFIG_MACH_SUN50I_H6
> > + load = <0x104000>;
> > + entry = <0x104000>;
> > +#else
> > + load = <0x44000>;
> > + entry = <0x44000>;
> > +#endif
> > + atf-bl31 {
> > + };
> > + };
> > +
> > + @fdt-SEQ {
> > + description = "NAME";
> > + type = "flat_dt";
> > + compression = "none";
> > + };
> > + };
> > +
> > + configurations {
> > + default = "config-1";
>
> I would expect @DEFAULT-SEQ here.
For some reason the old script just used config-1.
>
> > + @config-SEQ {
> > + description = "NAME";
> > + firmware = "uboot";
> > + loadables = "atf";
> > + fdt = "fdt-SEQ";
> > + };
> > + };
> > + };
> > +#else
> > u-boot-img {
> > offset = <CONFIG_SPL_PAD_TO>;
> > };
> > +#endif
> > };
> > };
> >
>
Regards,
Simon
More information about the U-Boot
mailing list