On 9/5/20 6:10 PM, Samuel Holland 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 <s...@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).
>
> Is there a way to make binman also write the FIT without the SPL? Would that
> require duplicating the whole binman node?
>
>> 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.
>
>> 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).
>
>> 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.
>
>> +#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.
>> + };
>> + };
>> +
>> + @fdt-SEQ {
>> + description = "NAME";
>> + type = "flat_dt";
>> + compression = "none";
>> + };
>> + };
>> +
>> + configurations {
>> + default = "config-1";
>
> I would expect @DEFAULT-SEQ here.
>
>> + @config-SEQ {
>> + description = "NAME";
>> + firmware = "uboot";
>> + loadables = "atf";
>> + fdt = "fdt-SEQ";
>> + };
>> + };
>> + };
>> +#else
>> u-boot-img {
>> offset = <CONFIG_SPL_PAD_TO>;
>> };
>> +#endif
>> };
>> };
>>
>
