Hi Samuel,
On Sat, 5 Sep 2020 at 19:49, Samuel Holland samuel@sholland.org wrote:
Simon,
On 9/5/20 7:18 PM, Simon Glass wrote:
On Sat, 5 Sep 2020 at 17:10, Samuel Holland samuel@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@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_SUNXINow `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.
Thanks. From looking at the code and comparing with u-boot-sunxi-with-spl.bin, it seems that u-boot-sunxi-with-spl.fit.fit is the "final" ITB file. My only hesitation is that it seems like an implementation detail, but I guess it's fine for now.
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.
u-boot-sunxi-with-spl.fit.fit is good enough for my purposes, but others may have an opinion.
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 && RISCVdiff --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?
Yes, the file is built either way. I provide a specific target to avoid building other files I don't need -- for example, a plain `make` used to rebuild the hello world EFI application every time.
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.
Yes, having binman generate a Makefile fragment would be ideal. That would also solve binman being forced to re-run every `make` invocation. For now a plain `make`/`make all` is fine. The forced rebuilds seem to be better under control now.
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.
I tried implementing this, and I ran into the problem that sunxi doesn't define CONFIG_SYS_VENDOR. So this file (from CONFIG_SYS_SOC) and the board-DTS-specific file are the only two magic u-boot.dtsi files available, and I don't think we want a u-boot.dtsi for every board.
No that would be painful.
A possible improvement would be defining BL31_ADDR (and later SCP_ADDR) as macros at the top of the file, like the shell script does.
Yes that might be better I suppose.
+#ifdef CONFIG_MACH_SUN50I_H6
load = <0x104000>;entry = <0x104000>;+#else
load = <0x44000>;entry = <0x44000>;+#endif
atf-bl31 {Another regression: you need
filename = "bl31.bin";
here to match the behavior when the environment variable is not defined.
That would be better to go in the code. If U-Boot passes an empty filename to binman then it needs special handling, if we want a default.
(merging the threads here)
What special handling are you thinking of? In blob_named_by_arg.py, the default filename is only overwritten from the arg if the arg is not empty. So the code already does the right thing, matching the baseline script: if no environment variable was defined, use a file with the default name in the current directory. It just needs a default name defined here.
Well Entry_aft_bl31 uses Entry_named_blob_by_arg which uses Entry_blob which reads the filename. So we would need to do some refactoring to avoid overriding the default filename, as I read it.
};};@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.
Probably because determining the index of CONFIG_DEFAULT_DEVICE_TREE in CONFIG_OF_LIST in POSIX sh is nontrivial. sunxi SPL always explicitly chooses the config matching CONFIG_DEFAULT_DEVICE_TREE, so "1" vs "DEFAULT-SEQ" would only make a difference if CONFIG_DEFAULT_DEVICE_TREE was not included in CONFIG_OF_LIST. (If using "DEFAULT-SEQ", that would be an error, so it would require that the SPL and FIT were built separately).
OK.
@config-SEQ {description = "NAME";firmware = "uboot";loadables = "atf";fdt = "fdt-SEQ";};};};+#else u-boot-img { offset = <CONFIG_SPL_PAD_TO>; }; +#endif }; };
Regards, Simon