RE: [RFC] imx: imx8qm-rom7720: switch to binman

[ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>]

Hello Oliver,

> -----Original Message-----
> From: Oliver Graute <oliver.graute@kococonnector.com>
> Sent: Monday, January 10, 2022 11:31 AM
> To: Oliver Graute <oliver.graute@kococonnector.com>; Stefano Babic
> <sbabic@denx.de>; Fabio Estevam <festevam@gmail.com>; NXP i.MX U-Boot Team
> <uboot-imx@nxp.com>
> Cc: aford173@gmail.com; peng.fan@nxp.com; marcel.ziswiler@toradex.com;
> tharvey@gateworks.com; ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>;
> u-boot@lists.denx.de
> Subject: [RFC] imx: imx8qm-rom7720: switch to binman
> 
> Switch to use binman to pack images

Please don't forget S-o-B, checkpatch does complain about it. ;)
> 
> ---
> 
> Run into this issue on the first try to get binman working:
> 
> make[1]: Nothing to be done for 'SPL'.
>   BINMAN  flash.bin
>   MKIMAGE flash.bin
> Error: spl/u-boot-spl.cfgout[16] - Invalid command (LOADER)
> arch/arm/mach-imx/Makefile:190: recipe for target 'flash.bin' failed
> make[1]: *** [flash.bin] Error 1
> make[1]: *** Deleting file 'flash.bin'
> Makefile:1519: recipe for target 'flash.bin' failed
> make: *** [flash.bin] Error 2
> 
> Some clue howto fix that?

This might be due to the fact that you're using the same u-boot-nodtb.bin binary twice: once in binman section, and once in ITB. You can try to rename the binman entry to a different name, this might do the trick. See [1] for sample of imx8mq_evk conversion.

> 
>  arch/arm/dts/imx8qm-rom7720-a1.dts            |  1 +
>  arch/arm/dts/imx8qm-u-boot.dtsi               | 95 +++++++++++++++++++
>  arch/arm/mach-imx/imx8/Kconfig                |  1 +
>  .../advantech/imx8qm_rom7720_a1/imximage.cfg  | 16 +---
>  configs/imx8qm_rom7720_a1_4G_defconfig        |  2 +-

Is there any documentation available for this board that
needs adaptions with new build instructions? I was not able
to locate any doc for this board at all...

>  5 files changed, 100 insertions(+), 15 deletions(-)
>  create mode 100644 arch/arm/dts/imx8qm-u-boot.dtsi
> 
> diff --git a/arch/arm/dts/imx8qm-rom7720-a1.dts b/arch/arm/dts/imx8qm-rom7720-
> a1.dts
> index d1f2fff869..332d441c6d 100644
> --- a/arch/arm/dts/imx8qm-rom7720-a1.dts
> +++ b/arch/arm/dts/imx8qm-rom7720-a1.dts
> @@ -10,6 +10,7 @@
>  /memreserve/ 0x80000000 0x00020000;
> 
>  #include "fsl-imx8qm.dtsi"
> +#include "imx8qm-u-boot.dtsi"
> 
>  / {
>         model = "Advantech iMX8QM Qseven series";
> diff --git a/arch/arm/dts/imx8qm-u-boot.dtsi b/arch/arm/dts/imx8qm-u-boot.dtsi
> new file mode 100644
> index 0000000000..77e5cd6989
> --- /dev/null
> +++ b/arch/arm/dts/imx8qm-u-boot.dtsi
> @@ -0,0 +1,95 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2021 NXP
> + */
> +
> +/ {
> +       binman: binman {
> +               multiple-images;
> +       };
> +
> +};
> +
> +&binman {
> +       u-boot-nodtb {
> +               filename = "u-boot-nodtb.bin";
> +               pad-byte = <0xff>;
> +               align-size = <4>;
> +               align = <4>;
> +
> +               u-boot-spl {
> +                       align-end = <4>;
> +               };

What I see is missing in this section is SECO and SCFW binaries.

Can i.MX8QM boot without those? Otherwise generated image will be unusable.

> +
> +       };
> +
> +       flash {
> +               mkimage {
> +                       args = "-n spl/u-boot-spl.cfgout -T imx8mimage -e
> 0x100000";
> +
> +                       blob {
> +                               filename = "u-boot-nodtb.bin";
> +                       };
> +
> +               };
> +
> +       };
> +
> +       itb {
> +               filename = "u-boot.itb";
> +
> +               fit {
> +                       description = "Configuration to load ATF before U-Boot";
> +                       #address-cells = <1>;
> +                       fit,external-offset = <CONFIG_FIT_EXTERNAL_OFFSET>;
> +
> +                       images {
> +                               uboot {
> +                                       description = "U-Boot (64-bit)";
> +                                       type = "standalone";
> +                                       arch = "arm64";
> +                                       compression = "none";
> +                                       load = <CONFIG_SYS_TEXT_BASE>;
> +
> +                                       uboot_blob: blob-ext {
> +                                               filename = "u-boot-nodtb.bin";
> +                                       };
> +                               };
> +
> +                               atf {
> +                                       description = "ARM Trusted Firmware";
> +                                       type = "firmware";
> +                                       arch = "arm64";
> +                                       compression = "none";
> +                                       load = <0x80000000>;
> +                                       entry = <0x80000000>;
> +
> +                                       atf_blob: blob-ext {
> +                                               filename = "bl31.bin";
> +                                       };
> +                               };
> +
> +                               fdt {
> +                                       description = "imx8qm-rom7720-a1";

You can use "NAME" here I suppose.

> +                                       type = "flat_dt";
> +                                       compression = "none";
> +
> +                                       uboot_fdt_blob: blob-ext {
> +                                               filename = "u-boot.dtb";
> +                                       };
> +                               };
> +                       };
> +
> +                       configurations {
> +                               default = "conf";
> +
> +                               conf {
> +                                       description = "imx8qm-rom7720-a1";

And as well here, "NAME".

> +                                       firmware = "uboot";
> +                                       loadables = "atf";
> +                                       fdt = "fdt";
> +                               };
> +                       };
> +               };
> +       };
> +};

This binman configuration suggests that there are 2 binaries that
should be populated onto the bootable media: flash.bin and
u-boot.itb. Can you please convert them to use a single binary?
Sample code for this is also in [1].

> diff --git a/arch/arm/mach-imx/imx8/Kconfig b/arch/arm/mach-imx/imx8/Kconfig
> index b43739e5c6..b42b985cdf 100644
> --- a/arch/arm/mach-imx/imx8/Kconfig
> +++ b/arch/arm/mach-imx/imx8/Kconfig
> @@ -81,6 +81,7 @@ config TARGET_CONGA_QMX8
> 
>  config TARGET_IMX8QM_ROM7720_A1
>         bool "Support i.MX8QM ROM-7720-A1"
> +       select BINMAN
>         select BOARD_LATE_INIT
>         select SUPPORT_SPL
>         select IMX8QM
> diff --git a/board/advantech/imx8qm_rom7720_a1/imximage.cfg
> b/board/advantech/imx8qm_rom7720_a1/imximage.cfg
> index e324c7ca37..30556ff9d7 100644
> --- a/board/advantech/imx8qm_rom7720_a1/imximage.cfg
> +++ b/board/advantech/imx8qm_rom7720_a1/imximage.cfg
> @@ -5,17 +5,5 @@
> 
>  #define __ASSEMBLY__
> 
> -/* Boot from SD, sector size 0x400 */
> -BOOT_FROM SD 0x400
> -/* SoC type IMX8QM */
> -SOC_TYPE IMX8QM
> -/* Append seco container image */
> -APPEND mx8qm-ahab-container.img
> -/* Create the 2nd container */
> -CONTAINER
> -/* Add scfw image with exec attribute */
> -IMAGE SCU mx8qm-val-scfw-tcm.bin
> -/* Add ATF image with exec attribute */
> -IMAGE A35 bl31.bin 0x80000000
> -/* Add U-Boot image with load attribute */
> -DATA A35 u-boot-dtb.bin 0x80020000
> +BOOT_FROM      sd
> +LOADER         u-boot-dtb.bin  0x10000
> diff --git a/configs/imx8qm_rom7720_a1_4G_defconfig
> b/configs/imx8qm_rom7720_a1_4G_defconfig
> index fb43fa1962..07ea6e191d 100644
> --- a/configs/imx8qm_rom7720_a1_4G_defconfig
> +++ b/configs/imx8qm_rom7720_a1_4G_defconfig
> @@ -18,7 +18,7 @@ CONFIG_SPL=y
>  CONFIG_SYS_LOAD_ADDR=0x80280000
>  CONFIG_FIT=y
>  CONFIG_SPL_LOAD_FIT=y
> -CONFIG_SPL_FIT_GENERATOR="arch/arm/mach-imx/mkimage_fit_atf.sh"
> +# CONFIG_SPL_FIT_GENERATOR is not set
>  CONFIG_BOOTDELAY=3
>  CONFIG_LOG=y
>  CONFIG_BOARD_EARLY_INIT_F=y
> --
> 2.17.1

-- andrey

Link: [1]: https://lore.kernel.org/u-boot/20211203161802.12699-1-andrey.zhizhikin@leica-geosystems.com/