diff mbox series

[v3,2/9] arm: dts: imx8mp: Add common u-boot dtsi

Message ID 20210707125804.3010652-3-t.remmet@phytec.de
State Accepted
Commit dafb164f61e642f429e37b5af42b7e59bbe47982
Delegated to: Stefano Babic
Headers show
Series phyCORE-i.MX8MP Update | expand

Commit Message

Teresa Remmet July 7, 2021, 12:57 p.m. UTC
Factor out the common node settings for dm-spl and dm-pre-reloc
and move them to imx8mp-u-boot.dtsi

Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
Reviewed-by: Fabio Estevam <festevam@gmail.com>
Reviewed-by: Heiko Schocher <hs@denx.de>
---
Changes in v3:
- Moved binman nodes to common imx8mp-u-boot.dtsi
Changes in v2:
- none

 arch/arm/dts/imx8mp-evk-u-boot.dtsi           | 143 +----------------
 .../imx8mp-phyboard-pollux-rdk-u-boot.dtsi    |  39 +----
 arch/arm/dts/imx8mp-u-boot.dtsi               | 149 ++++++++++++++++++
 3 files changed, 153 insertions(+), 178 deletions(-)
 create mode 100644 arch/arm/dts/imx8mp-u-boot.dtsi

Comments

Tim Harvey July 9, 2021, 2:47 p.m. UTC | #1
On Wed, Jul 7, 2021 at 5:58 AM Teresa Remmet <t.remmet@phytec.de> wrote:
>
> Factor out the common node settings for dm-spl and dm-pre-reloc
> and move them to imx8mp-u-boot.dtsi
>
> Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
> Reviewed-by: Fabio Estevam <festevam@gmail.com>
> Reviewed-by: Heiko Schocher <hs@denx.de>
> ---
> Changes in v3:
> - Moved binman nodes to common imx8mp-u-boot.dtsi
> Changes in v2:
> - none
>
>  arch/arm/dts/imx8mp-evk-u-boot.dtsi           | 143 +----------------
>  .../imx8mp-phyboard-pollux-rdk-u-boot.dtsi    |  39 +----
>  arch/arm/dts/imx8mp-u-boot.dtsi               | 149 ++++++++++++++++++
>  3 files changed, 153 insertions(+), 178 deletions(-)
>  create mode 100644 arch/arm/dts/imx8mp-u-boot.dtsi
>
> diff --git a/arch/arm/dts/imx8mp-evk-u-boot.dtsi b/arch/arm/dts/imx8mp-evk-u-boot.dtsi
> index 4162f41cffb6..2abcf1f03d4f 100644
> --- a/arch/arm/dts/imx8mp-evk-u-boot.dtsi
> +++ b/arch/arm/dts/imx8mp-evk-u-boot.dtsi
> @@ -3,11 +3,9 @@
>   * Copyright 2019 NXP
>   */
>
> -/ {
> -       binman: binman {
> -               multiple-images;
> -       };
> +#include "imx8mp-u-boot.dtsi"
>
> +/ {
>         wdt-reboot {
>                 compatible = "wdt-reboot";
>                 wdt = <&wdog1>;
> @@ -21,43 +19,6 @@
>         };
>  };
>
> -&{/soc@0} {
> -       u-boot,dm-pre-reloc;
> -       u-boot,dm-spl;
> -};
> -
> -&clk {
> -       u-boot,dm-spl;
> -       u-boot,dm-pre-reloc;
> -};
> -
> -&osc_32k {
> -       u-boot,dm-spl;
> -       u-boot,dm-pre-reloc;
> -};
> -
> -&osc_24m {
> -       u-boot,dm-spl;
> -       u-boot,dm-pre-reloc;
> -};
> -
> -&aips1 {
> -       u-boot,dm-spl;
> -       u-boot,dm-pre-reloc;
> -};
> -
> -&aips2 {
> -       u-boot,dm-spl;
> -};
> -
> -&aips3 {
> -       u-boot,dm-spl;
> -};
> -
> -&iomuxc {
> -       u-boot,dm-spl;
> -};
> -
>  &reg_usdhc2_vmmc {
>         u-boot,off-on-delay-us = <20000>;
>  };
> @@ -156,104 +117,4 @@
>         phy-reset-post-delay = <100>;
>  };
>
> -&binman {
> -        u-boot-spl-ddr {
> -               filename = "u-boot-spl-ddr.bin";
> -               pad-byte = <0xff>;
> -               align-size = <4>;
> -               align = <4>;
>
> -               u-boot-spl {
> -                       align-end = <4>;
> -               };
> -
> -               blob_1: blob-ext@1 {
> -                       filename = "lpddr4_pmu_train_1d_imem_202006.bin";
> -                       size = <0x8000>;
> -               };
> -
> -               blob_2: blob-ext@2 {
> -                       filename = "lpddr4_pmu_train_1d_dmem_202006.bin";
> -                       size = <0x4000>;
> -               };
> -
> -               blob_3: blob-ext@3 {
> -                       filename = "lpddr4_pmu_train_2d_imem_202006.bin";
> -                       size = <0x8000>;
> -               };
> -
> -               blob_4: blob-ext@4 {
> -                       filename = "lpddr4_pmu_train_2d_dmem_202006.bin";
> -                       size = <0x4000>;
> -               };
> -       };
> -
> -
> -       flash {
> -               mkimage {
> -                       args = "-n spl/u-boot-spl.cfgout -T imx8mimage -e 0x920000";
> -
> -                       blob {
> -                               filename = "u-boot-spl-ddr.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 = <0x970000>;
> -                                       entry = <0x970000>;
> -
> -                                       atf_blob: blob-ext {
> -                                               filename = "bl31.bin";
> -                                       };
> -                               };
> -
> -                               fdt {
> -                                       description = "NAME";
> -                                       type = "flat_dt";
> -                                       compression = "none";
> -
> -                                       uboot_fdt_blob: blob-ext {
> -                                               filename = "u-boot.dtb";
> -                                       };
> -                               };
> -                       };
> -
> -                       configurations {
> -                               default = "conf";
> -
> -                               conf {
> -                                       description = "NAME";
> -                                       firmware = "uboot";
> -                                       loadables = "atf";
> -                                       fdt = "fdt";
> -                               };
> -                       };
> -               };
> -       };
> -};
> diff --git a/arch/arm/dts/imx8mp-phyboard-pollux-rdk-u-boot.dtsi b/arch/arm/dts/imx8mp-phyboard-pollux-rdk-u-boot.dtsi
> index 20e7f63ff91f..6c1528934a98 100644
> --- a/arch/arm/dts/imx8mp-phyboard-pollux-rdk-u-boot.dtsi
> +++ b/arch/arm/dts/imx8mp-phyboard-pollux-rdk-u-boot.dtsi
> @@ -4,6 +4,8 @@
>   * Author: Teresa Remmet <t.remmet@phytec.de>
>   */
>
> +#include "imx8mp-u-boot.dtsi"
> +
>  / {
>         wdt-reboot {
>                 compatible = "wdt-reboot";
> @@ -12,43 +14,6 @@
>         };
>  };
>
> -&{/soc@0} {
> -       u-boot,dm-pre-reloc;
> -       u-boot,dm-spl;
> -};
> -
> -&clk {
> -       u-boot,dm-spl;
> -       u-boot,dm-pre-reloc;
> -};
> -
> -&osc_32k {
> -       u-boot,dm-spl;
> -       u-boot,dm-pre-reloc;
> -};
> -
> -&osc_24m {
> -       u-boot,dm-spl;
> -       u-boot,dm-pre-reloc;
> -};
> -
> -&aips1 {
> -       u-boot,dm-spl;
> -       u-boot,dm-pre-reloc;
> -};
> -
> -&aips2 {
> -       u-boot,dm-spl;
> -};
> -
> -&aips3 {
> -       u-boot,dm-spl;
> -};
> -
> -&iomuxc {
> -       u-boot,dm-spl;
> -};
> -
>  &reg_usdhc2_vmmc {
>         u-boot,dm-spl;
>  };
> diff --git a/arch/arm/dts/imx8mp-u-boot.dtsi b/arch/arm/dts/imx8mp-u-boot.dtsi
> new file mode 100644
> index 000000000000..d61346da3032
> --- /dev/null
> +++ b/arch/arm/dts/imx8mp-u-boot.dtsi
> @@ -0,0 +1,149 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2021 PHYTEC Messtechnik GmbH
> + * Author: Teresa Remmet <t.remmet@phytec.de>
> + */
> +
> +/ {
> +       binman: binman {
> +               multiple-images;
> +       };
> +};
> +
> +&{/soc@0} {
> +       u-boot,dm-pre-reloc;
> +       u-boot,dm-spl;
> +};
> +
> +&clk {
> +       u-boot,dm-spl;
> +       u-boot,dm-pre-reloc;
> +};
> +
> +&osc_32k {
> +       u-boot,dm-spl;
> +       u-boot,dm-pre-reloc;
> +};
> +
> +&osc_24m {
> +       u-boot,dm-spl;
> +       u-boot,dm-pre-reloc;
> +};
> +
> +&aips1 {
> +       u-boot,dm-spl;
> +       u-boot,dm-pre-reloc;
> +};
> +
> +&aips2 {
> +       u-boot,dm-spl;
> +};
> +
> +&aips3 {
> +       u-boot,dm-spl;
> +};
> +
> +&iomuxc {
> +       u-boot,dm-spl;
> +};
> +
> +&binman {
> +        u-boot-spl-ddr {
> +               filename = "u-boot-spl-ddr.bin";
> +               pad-byte = <0xff>;
> +               align-size = <4>;
> +               align = <4>;
> +
> +               u-boot-spl {
> +                       align-end = <4>;
> +               };
> +
> +               blob_1: blob-ext@1 {
> +                       filename = "lpddr4_pmu_train_1d_imem_202006.bin";
> +                       size = <0x8000>;
> +               };
> +
> +               blob_2: blob-ext@2 {
> +                       filename = "lpddr4_pmu_train_1d_dmem_202006.bin";
> +                       size = <0x4000>;
> +               };
> +
> +               blob_3: blob-ext@3 {
> +                       filename = "lpddr4_pmu_train_2d_imem_202006.bin";
> +                       size = <0x8000>;
> +               };
> +
> +               blob_4: blob-ext@4 {
> +                       filename = "lpddr4_pmu_train_2d_dmem_202006.bin";
> +                       size = <0x4000>;
> +               };
> +       };
> +
> +       flash {
> +               mkimage {
> +                       args = "-n spl/u-boot-spl.cfgout -T imx8mimage -e 0x920000";
> +
> +                       blob {
> +                               filename = "u-boot-spl-ddr.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 = <0x970000>;
> +                                       entry = <0x970000>;
> +
> +                                       atf_blob: blob-ext {
> +                                               filename = "bl31.bin";
> +                                       };
> +                               };
> +
> +                               fdt {
> +                                       description = "NAME";
> +                                       type = "flat_dt";
> +                                       compression = "none";
> +
> +                                       uboot_fdt_blob: blob-ext {
> +                                               filename = "u-boot.dtb";
> +                                       };
> +                               };
> +                       };
> +
> +                       configurations {
> +                               default = "conf";
> +
> +                               conf {
> +                                       description = "NAME";
> +                                       firmware = "uboot";
> +                                       loadables = "atf";
> +                                       fdt = "fdt";
> +                               };
> +                       };
> +               };
> +       };
> +};
> --
> 2.25.1
>

Teresa,

I've noticed many of the imx8m boards migrating to using binman for
image packaging.

Doesn't this change from having a single flash.bin encompasing the SPL
and U-Boot proper to having split files? I noticed that happened with
imx8mm_evk for example when it switched to binman.

What are the benefits to using binman?

Thanks,

Tim
Heiko Schocher July 10, 2021, 12:23 p.m. UTC | #2
Hello Tim,

On 09.07.21 16:47, Tim Harvey wrote:
> On Wed, Jul 7, 2021 at 5:58 AM Teresa Remmet <t.remmet@phytec.de> wrote:
>>
>> Factor out the common node settings for dm-spl and dm-pre-reloc
>> and move them to imx8mp-u-boot.dtsi
>>
>> Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
>> Reviewed-by: Fabio Estevam <festevam@gmail.com>
>> Reviewed-by: Heiko Schocher <hs@denx.de>
>> ---
>> Changes in v3:
>> - Moved binman nodes to common imx8mp-u-boot.dtsi
>> Changes in v2:
>> - none
>>
>>  arch/arm/dts/imx8mp-evk-u-boot.dtsi           | 143 +----------------
>>  .../imx8mp-phyboard-pollux-rdk-u-boot.dtsi    |  39 +----
>>  arch/arm/dts/imx8mp-u-boot.dtsi               | 149 ++++++++++++++++++
>>  3 files changed, 153 insertions(+), 178 deletions(-)
>>  create mode 100644 arch/arm/dts/imx8mp-u-boot.dtsi
>>
>> diff --git a/arch/arm/dts/imx8mp-evk-u-boot.dtsi b/arch/arm/dts/imx8mp-evk-u-boot.dtsi
>> index 4162f41cffb6..2abcf1f03d4f 100644
>> --- a/arch/arm/dts/imx8mp-evk-u-boot.dtsi
>> +++ b/arch/arm/dts/imx8mp-evk-u-boot.dtsi
>> @@ -3,11 +3,9 @@
>>   * Copyright 2019 NXP
>>   */
>>
>> -/ {
>> -       binman: binman {
>> -               multiple-images;
>> -       };
>> +#include "imx8mp-u-boot.dtsi"
>>
>> +/ {
>>         wdt-reboot {
>>                 compatible = "wdt-reboot";
>>                 wdt = <&wdog1>;
>> @@ -21,43 +19,6 @@
>>         };
>>  };
>>
>> -&{/soc@0} {
>> -       u-boot,dm-pre-reloc;
>> -       u-boot,dm-spl;
>> -};
>> -
>> -&clk {
>> -       u-boot,dm-spl;
>> -       u-boot,dm-pre-reloc;
>> -};
>> -
>> -&osc_32k {
>> -       u-boot,dm-spl;
>> -       u-boot,dm-pre-reloc;
>> -};
>> -
>> -&osc_24m {
>> -       u-boot,dm-spl;
>> -       u-boot,dm-pre-reloc;
>> -};
>> -
>> -&aips1 {
>> -       u-boot,dm-spl;
>> -       u-boot,dm-pre-reloc;
>> -};
>> -
>> -&aips2 {
>> -       u-boot,dm-spl;
>> -};
>> -
>> -&aips3 {
>> -       u-boot,dm-spl;
>> -};
>> -
>> -&iomuxc {
>> -       u-boot,dm-spl;
>> -};
>> -
>>  &reg_usdhc2_vmmc {
>>         u-boot,off-on-delay-us = <20000>;
>>  };
>> @@ -156,104 +117,4 @@
>>         phy-reset-post-delay = <100>;
>>  };
>>
>> -&binman {
>> -        u-boot-spl-ddr {
>> -               filename = "u-boot-spl-ddr.bin";
>> -               pad-byte = <0xff>;
>> -               align-size = <4>;
>> -               align = <4>;
>>
>> -               u-boot-spl {
>> -                       align-end = <4>;
>> -               };
>> -
>> -               blob_1: blob-ext@1 {
>> -                       filename = "lpddr4_pmu_train_1d_imem_202006.bin";
>> -                       size = <0x8000>;
>> -               };
>> -
>> -               blob_2: blob-ext@2 {
>> -                       filename = "lpddr4_pmu_train_1d_dmem_202006.bin";
>> -                       size = <0x4000>;
>> -               };
>> -
>> -               blob_3: blob-ext@3 {
>> -                       filename = "lpddr4_pmu_train_2d_imem_202006.bin";
>> -                       size = <0x8000>;
>> -               };
>> -
>> -               blob_4: blob-ext@4 {
>> -                       filename = "lpddr4_pmu_train_2d_dmem_202006.bin";
>> -                       size = <0x4000>;
>> -               };
>> -       };
>> -
>> -
>> -       flash {
>> -               mkimage {
>> -                       args = "-n spl/u-boot-spl.cfgout -T imx8mimage -e 0x920000";
>> -
>> -                       blob {
>> -                               filename = "u-boot-spl-ddr.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 = <0x970000>;
>> -                                       entry = <0x970000>;
>> -
>> -                                       atf_blob: blob-ext {
>> -                                               filename = "bl31.bin";
>> -                                       };
>> -                               };
>> -
>> -                               fdt {
>> -                                       description = "NAME";
>> -                                       type = "flat_dt";
>> -                                       compression = "none";
>> -
>> -                                       uboot_fdt_blob: blob-ext {
>> -                                               filename = "u-boot.dtb";
>> -                                       };
>> -                               };
>> -                       };
>> -
>> -                       configurations {
>> -                               default = "conf";
>> -
>> -                               conf {
>> -                                       description = "NAME";
>> -                                       firmware = "uboot";
>> -                                       loadables = "atf";
>> -                                       fdt = "fdt";
>> -                               };
>> -                       };
>> -               };
>> -       };
>> -};
>> diff --git a/arch/arm/dts/imx8mp-phyboard-pollux-rdk-u-boot.dtsi b/arch/arm/dts/imx8mp-phyboard-pollux-rdk-u-boot.dtsi
>> index 20e7f63ff91f..6c1528934a98 100644
>> --- a/arch/arm/dts/imx8mp-phyboard-pollux-rdk-u-boot.dtsi
>> +++ b/arch/arm/dts/imx8mp-phyboard-pollux-rdk-u-boot.dtsi
>> @@ -4,6 +4,8 @@
>>   * Author: Teresa Remmet <t.remmet@phytec.de>
>>   */
>>
>> +#include "imx8mp-u-boot.dtsi"
>> +
>>  / {
>>         wdt-reboot {
>>                 compatible = "wdt-reboot";
>> @@ -12,43 +14,6 @@
>>         };
>>  };
>>
>> -&{/soc@0} {
>> -       u-boot,dm-pre-reloc;
>> -       u-boot,dm-spl;
>> -};
>> -
>> -&clk {
>> -       u-boot,dm-spl;
>> -       u-boot,dm-pre-reloc;
>> -};
>> -
>> -&osc_32k {
>> -       u-boot,dm-spl;
>> -       u-boot,dm-pre-reloc;
>> -};
>> -
>> -&osc_24m {
>> -       u-boot,dm-spl;
>> -       u-boot,dm-pre-reloc;
>> -};
>> -
>> -&aips1 {
>> -       u-boot,dm-spl;
>> -       u-boot,dm-pre-reloc;
>> -};
>> -
>> -&aips2 {
>> -       u-boot,dm-spl;
>> -};
>> -
>> -&aips3 {
>> -       u-boot,dm-spl;
>> -};
>> -
>> -&iomuxc {
>> -       u-boot,dm-spl;
>> -};
>> -
>>  &reg_usdhc2_vmmc {
>>         u-boot,dm-spl;
>>  };
>> diff --git a/arch/arm/dts/imx8mp-u-boot.dtsi b/arch/arm/dts/imx8mp-u-boot.dtsi
>> new file mode 100644
>> index 000000000000..d61346da3032
>> --- /dev/null
>> +++ b/arch/arm/dts/imx8mp-u-boot.dtsi
>> @@ -0,0 +1,149 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (C) 2021 PHYTEC Messtechnik GmbH
>> + * Author: Teresa Remmet <t.remmet@phytec.de>
>> + */
>> +
>> +/ {
>> +       binman: binman {
>> +               multiple-images;
>> +       };
>> +};
>> +
>> +&{/soc@0} {
>> +       u-boot,dm-pre-reloc;
>> +       u-boot,dm-spl;
>> +};
>> +
>> +&clk {
>> +       u-boot,dm-spl;
>> +       u-boot,dm-pre-reloc;
>> +};
>> +
>> +&osc_32k {
>> +       u-boot,dm-spl;
>> +       u-boot,dm-pre-reloc;
>> +};
>> +
>> +&osc_24m {
>> +       u-boot,dm-spl;
>> +       u-boot,dm-pre-reloc;
>> +};
>> +
>> +&aips1 {
>> +       u-boot,dm-spl;
>> +       u-boot,dm-pre-reloc;
>> +};
>> +
>> +&aips2 {
>> +       u-boot,dm-spl;
>> +};
>> +
>> +&aips3 {
>> +       u-boot,dm-spl;
>> +};
>> +
>> +&iomuxc {
>> +       u-boot,dm-spl;
>> +};
>> +
>> +&binman {
>> +        u-boot-spl-ddr {
>> +               filename = "u-boot-spl-ddr.bin";
>> +               pad-byte = <0xff>;
>> +               align-size = <4>;
>> +               align = <4>;
>> +
>> +               u-boot-spl {
>> +                       align-end = <4>;
>> +               };
>> +
>> +               blob_1: blob-ext@1 {
>> +                       filename = "lpddr4_pmu_train_1d_imem_202006.bin";
>> +                       size = <0x8000>;
>> +               };
>> +
>> +               blob_2: blob-ext@2 {
>> +                       filename = "lpddr4_pmu_train_1d_dmem_202006.bin";
>> +                       size = <0x4000>;
>> +               };
>> +
>> +               blob_3: blob-ext@3 {
>> +                       filename = "lpddr4_pmu_train_2d_imem_202006.bin";
>> +                       size = <0x8000>;
>> +               };
>> +
>> +               blob_4: blob-ext@4 {
>> +                       filename = "lpddr4_pmu_train_2d_dmem_202006.bin";
>> +                       size = <0x4000>;
>> +               };
>> +       };
>> +
>> +       flash {
>> +               mkimage {
>> +                       args = "-n spl/u-boot-spl.cfgout -T imx8mimage -e 0x920000";
>> +
>> +                       blob {
>> +                               filename = "u-boot-spl-ddr.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 = <0x970000>;
>> +                                       entry = <0x970000>;
>> +
>> +                                       atf_blob: blob-ext {
>> +                                               filename = "bl31.bin";
>> +                                       };
>> +                               };
>> +
>> +                               fdt {
>> +                                       description = "NAME";
>> +                                       type = "flat_dt";
>> +                                       compression = "none";
>> +
>> +                                       uboot_fdt_blob: blob-ext {
>> +                                               filename = "u-boot.dtb";
>> +                                       };
>> +                               };
>> +                       };
>> +
>> +                       configurations {
>> +                               default = "conf";
>> +
>> +                               conf {
>> +                                       description = "NAME";
>> +                                       firmware = "uboot";
>> +                                       loadables = "atf";
>> +                                       fdt = "fdt";
>> +                               };
>> +                       };
>> +               };
>> +       };
>> +};
>> --
>> 2.25.1
>>
> 
> Teresa,
> 
> I've noticed many of the imx8m boards migrating to using binman for
> image packaging.
> 
> Doesn't this change from having a single flash.bin encompasing the SPL
> and U-Boot proper to having split files? I noticed that happened with
> imx8mm_evk for example when it switched to binman.

Yes, but you can easy generate there a single image again.

> What are the benefits to using binman?

Beside the pros from binmal in general, I see the benefit in special
for imx8mp, that you can get all infos you need for signing the image
from within the image. No need to save some log output from U-Boot
build and parse this output ...

bye,
Heiko
Stefano Babic July 10, 2021, 3:53 p.m. UTC | #3
> Factor out the common node settings for dm-spl and dm-pre-reloc
> and move them to imx8mp-u-boot.dtsi
> Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
> Reviewed-by: Fabio Estevam <festevam@gmail.com>
> Reviewed-by: Heiko Schocher <hs@denx.de>
Applied to u-boot-imx, master, thanks !

Best regards,
Stefano Babic
Stefano Babic July 10, 2021, 7:36 p.m. UTC | #4
> Factor out the common node settings for dm-spl and dm-pre-reloc
> and move them to imx8mp-u-boot.dtsi
> Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
> Reviewed-by: Fabio Estevam <festevam@gmail.com>
> Reviewed-by: Heiko Schocher <hs@denx.de>
Applied to u-boot-imx, master, thanks !

Best regards,
Stefano Babic
Tim Harvey July 12, 2021, 4:42 p.m. UTC | #5
On Sat, Jul 10, 2021 at 5:23 AM Heiko Schocher <hs@denx.de> wrote:
>
> Hello Tim,
>
> On 09.07.21 16:47, Tim Harvey wrote:
> > On Wed, Jul 7, 2021 at 5:58 AM Teresa Remmet <t.remmet@phytec.de> wrote:
> >>
> >> Factor out the common node settings for dm-spl and dm-pre-reloc
> >> and move them to imx8mp-u-boot.dtsi
> >>
> >> Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
> >> Reviewed-by: Fabio Estevam <festevam@gmail.com>
> >> Reviewed-by: Heiko Schocher <hs@denx.de>
> >> ---
> >> Changes in v3:
> >> - Moved binman nodes to common imx8mp-u-boot.dtsi
> >> Changes in v2:
> >> - none
> >>
> >>  arch/arm/dts/imx8mp-evk-u-boot.dtsi           | 143 +----------------
> >>  .../imx8mp-phyboard-pollux-rdk-u-boot.dtsi    |  39 +----
> >>  arch/arm/dts/imx8mp-u-boot.dtsi               | 149 ++++++++++++++++++
> >>  3 files changed, 153 insertions(+), 178 deletions(-)
> >>  create mode 100644 arch/arm/dts/imx8mp-u-boot.dtsi
> >>
<snip>
> >>
> >
> > Teresa,
> >
> > I've noticed many of the imx8m boards migrating to using binman for
> > image packaging.
> >
> > Doesn't this change from having a single flash.bin encompasing the SPL
> > and U-Boot proper to having split files? I noticed that happened with
> > imx8mm_evk for example when it switched to binman.
>
> Yes, but you can easy generate there a single image again.
>
> > What are the benefits to using binman?
>
> Beside the pros from binmal in general, I see the benefit in special
> for imx8mp, that you can get all infos you need for signing the image
> from within the image. No need to save some log output from U-Boot
> build and parse this output ...
>

(+cc Simon)

Heiko,

And what are the pros from binman in general? I've read over
tools/binman/binman.rst so I'm assuming you mean what is described
there as benefits.

How do you get all the details needed for signing the image from binman?

If I make imx8mm_evk_defconfig which produces via binman flash.bin and
u-boot.itb I get the following:

$ ./tools/binman/binman ls -i flash.bin
binman: Cannot find FDT map in image
$ ./tools/binman/binman ls -i u-boot.itb
binman: Cannot find FDT map in image

I would very much like to understand how to use binman to get the
various offsets needed for signing an IMX image for use with HAB.

Thanks,

Tim
Simon Glass July 12, 2021, 7:43 p.m. UTC | #6
Hi Tim,

On Mon, 12 Jul 2021 at 10:42, Tim Harvey <tharvey@gateworks.com> wrote:
>
> On Sat, Jul 10, 2021 at 5:23 AM Heiko Schocher <hs@denx.de> wrote:
> >
> > Hello Tim,
> >
> > On 09.07.21 16:47, Tim Harvey wrote:
> > > On Wed, Jul 7, 2021 at 5:58 AM Teresa Remmet <t.remmet@phytec.de> wrote:
> > >>
> > >> Factor out the common node settings for dm-spl and dm-pre-reloc
> > >> and move them to imx8mp-u-boot.dtsi
> > >>
> > >> Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
> > >> Reviewed-by: Fabio Estevam <festevam@gmail.com>
> > >> Reviewed-by: Heiko Schocher <hs@denx.de>
> > >> ---
> > >> Changes in v3:
> > >> - Moved binman nodes to common imx8mp-u-boot.dtsi
> > >> Changes in v2:
> > >> - none
> > >>
> > >>  arch/arm/dts/imx8mp-evk-u-boot.dtsi           | 143 +----------------
> > >>  .../imx8mp-phyboard-pollux-rdk-u-boot.dtsi    |  39 +----
> > >>  arch/arm/dts/imx8mp-u-boot.dtsi               | 149 ++++++++++++++++++
> > >>  3 files changed, 153 insertions(+), 178 deletions(-)
> > >>  create mode 100644 arch/arm/dts/imx8mp-u-boot.dtsi
> > >>
> <snip>
> > >>
> > >
> > > Teresa,
> > >
> > > I've noticed many of the imx8m boards migrating to using binman for
> > > image packaging.
> > >
> > > Doesn't this change from having a single flash.bin encompasing the SPL
> > > and U-Boot proper to having split files? I noticed that happened with
> > > imx8mm_evk for example when it switched to binman.
> >
> > Yes, but you can easy generate there a single image again.
> >
> > > What are the benefits to using binman?
> >
> > Beside the pros from binmal in general, I see the benefit in special
> > for imx8mp, that you can get all infos you need for signing the image
> > from within the image. No need to save some log output from U-Boot
> > build and parse this output ...
> >
>
> (+cc Simon)
>
> Heiko,
>
> And what are the pros from binman in general? I've read over
> tools/binman/binman.rst so I'm assuming you mean what is described
> there as benefits.
>
> How do you get all the details needed for signing the image from binman?
>
> If I make imx8mm_evk_defconfig which produces via binman flash.bin and
> u-boot.itb I get the following:
>
> $ ./tools/binman/binman ls -i flash.bin
> binman: Cannot find FDT map in image
> $ ./tools/binman/binman ls -i u-boot.itb
> binman: Cannot find FDT map in image

As the message says, you need an 'fdtmap' in the image:

   fdtmap {
   }:

>
> I would very much like to understand how to use binman to get the
> various offsets needed for signing an IMX image for use with HAB.

You should be able to add signing support to binman for your use case.
See for example how vblock.py works.

Regards,
Simon
Tim Harvey July 12, 2021, 8:58 p.m. UTC | #7
On Mon, Jul 12, 2021 at 12:44 PM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Tim,
>
> On Mon, 12 Jul 2021 at 10:42, Tim Harvey <tharvey@gateworks.com> wrote:
> >
> > On Sat, Jul 10, 2021 at 5:23 AM Heiko Schocher <hs@denx.de> wrote:
> > >
> > > Hello Tim,
> > >
> > > On 09.07.21 16:47, Tim Harvey wrote:
> > > > On Wed, Jul 7, 2021 at 5:58 AM Teresa Remmet <t.remmet@phytec.de> wrote:
> > > >>
> > > >> Factor out the common node settings for dm-spl and dm-pre-reloc
> > > >> and move them to imx8mp-u-boot.dtsi
> > > >>
> > > >> Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
> > > >> Reviewed-by: Fabio Estevam <festevam@gmail.com>
> > > >> Reviewed-by: Heiko Schocher <hs@denx.de>
> > > >> ---
> > > >> Changes in v3:
> > > >> - Moved binman nodes to common imx8mp-u-boot.dtsi
> > > >> Changes in v2:
> > > >> - none
> > > >>
> > > >>  arch/arm/dts/imx8mp-evk-u-boot.dtsi           | 143 +----------------
> > > >>  .../imx8mp-phyboard-pollux-rdk-u-boot.dtsi    |  39 +----
> > > >>  arch/arm/dts/imx8mp-u-boot.dtsi               | 149 ++++++++++++++++++
> > > >>  3 files changed, 153 insertions(+), 178 deletions(-)
> > > >>  create mode 100644 arch/arm/dts/imx8mp-u-boot.dtsi
> > > >>
> > <snip>
> > > >>
> > > >
> > > > Teresa,
> > > >
> > > > I've noticed many of the imx8m boards migrating to using binman for
> > > > image packaging.
> > > >
> > > > Doesn't this change from having a single flash.bin encompasing the SPL
> > > > and U-Boot proper to having split files? I noticed that happened with
> > > > imx8mm_evk for example when it switched to binman.
> > >
> > > Yes, but you can easy generate there a single image again.
> > >
> > > > What are the benefits to using binman?
> > >
> > > Beside the pros from binmal in general, I see the benefit in special
> > > for imx8mp, that you can get all infos you need for signing the image
> > > from within the image. No need to save some log output from U-Boot
> > > build and parse this output ...
> > >
> >
> > (+cc Simon)
> >
> > Heiko,
> >
> > And what are the pros from binman in general? I've read over
> > tools/binman/binman.rst so I'm assuming you mean what is described
> > there as benefits.
> >
> > How do you get all the details needed for signing the image from binman?
> >
> > If I make imx8mm_evk_defconfig which produces via binman flash.bin and
> > u-boot.itb I get the following:
> >
> > $ ./tools/binman/binman ls -i flash.bin
> > binman: Cannot find FDT map in image
> > $ ./tools/binman/binman ls -i u-boot.itb
> > binman: Cannot find FDT map in image
>
> As the message says, you need an 'fdtmap' in the image:
>
>    fdtmap {
>    }:
>

Simon,

Sorry I still don't quite understand:

diff --git a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
index f200afac9f..c6d8932fa4 100644
--- a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
+++ b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
@@ -163,6 +163,9 @@
        itb {
                filename = "u-boot.itb";

+               fdtmap {
+               };
+
                fit {
                        description = "Configuration to load ATF before U-Boot";
                        #address-cells = <1>;


$ make imx8mm_evk_defconfig
$ make
$ ./tools/binman/binman ls -i u-boot.itb
Name          Image-pos  Size    Entry-type  Offset  Uncomp-size
------------------------------------------------------------------
main-section          0   a3875  section          0
  fdtmap              0     48d  fdtmap           0
  fit               48d   a33e8  fit            48d

For signing we need the loadaddr/offset/size of the components within
the FIT image. Since binman is calling mkimage to create FIT images it
seems outside the scope of binman to be able to dump details on the
actual FIT image. Perhaps mkimage or another tool is to be used for
that detail?

> >
> > I would very much like to understand how to use binman to get the
> > various offsets needed for signing an IMX image for use with HAB.
>
> You should be able to add signing support to binman for your use case.
> See for example how vblock.py works.

I see... so your saying instead of using an external tool to generate
a txt template with the offset/size values for the various blobs
instead we need to implement for example a habv4.py to add a habv4
entry and the python class would have access to the content handles to
determine the addr/size in order to create the text template file fed
to the NXP code signing tool?

Regards,

Tim
Simon Glass July 13, 2021, 8:17 p.m. UTC | #8
Hi Tim,

On Mon, 12 Jul 2021 at 14:58, Tim Harvey <tharvey@gateworks.com> wrote:
>
> On Mon, Jul 12, 2021 at 12:44 PM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Tim,
> >
> > On Mon, 12 Jul 2021 at 10:42, Tim Harvey <tharvey@gateworks.com> wrote:
> > >
> > > On Sat, Jul 10, 2021 at 5:23 AM Heiko Schocher <hs@denx.de> wrote:
> > > >
> > > > Hello Tim,
> > > >
> > > > On 09.07.21 16:47, Tim Harvey wrote:
> > > > > On Wed, Jul 7, 2021 at 5:58 AM Teresa Remmet <t.remmet@phytec.de> wrote:
> > > > >>
> > > > >> Factor out the common node settings for dm-spl and dm-pre-reloc
> > > > >> and move them to imx8mp-u-boot.dtsi
> > > > >>
> > > > >> Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
> > > > >> Reviewed-by: Fabio Estevam <festevam@gmail.com>
> > > > >> Reviewed-by: Heiko Schocher <hs@denx.de>
> > > > >> ---
> > > > >> Changes in v3:
> > > > >> - Moved binman nodes to common imx8mp-u-boot.dtsi
> > > > >> Changes in v2:
> > > > >> - none
> > > > >>
> > > > >>  arch/arm/dts/imx8mp-evk-u-boot.dtsi           | 143 +----------------
> > > > >>  .../imx8mp-phyboard-pollux-rdk-u-boot.dtsi    |  39 +----
> > > > >>  arch/arm/dts/imx8mp-u-boot.dtsi               | 149 ++++++++++++++++++
> > > > >>  3 files changed, 153 insertions(+), 178 deletions(-)
> > > > >>  create mode 100644 arch/arm/dts/imx8mp-u-boot.dtsi
> > > > >>
> > > <snip>
> > > > >>
> > > > >
> > > > > Teresa,
> > > > >
> > > > > I've noticed many of the imx8m boards migrating to using binman for
> > > > > image packaging.
> > > > >
> > > > > Doesn't this change from having a single flash.bin encompasing the SPL
> > > > > and U-Boot proper to having split files? I noticed that happened with
> > > > > imx8mm_evk for example when it switched to binman.
> > > >
> > > > Yes, but you can easy generate there a single image again.
> > > >
> > > > > What are the benefits to using binman?
> > > >
> > > > Beside the pros from binmal in general, I see the benefit in special
> > > > for imx8mp, that you can get all infos you need for signing the image
> > > > from within the image. No need to save some log output from U-Boot
> > > > build and parse this output ...
> > > >
> > >
> > > (+cc Simon)
> > >
> > > Heiko,
> > >
> > > And what are the pros from binman in general? I've read over
> > > tools/binman/binman.rst so I'm assuming you mean what is described
> > > there as benefits.
> > >
> > > How do you get all the details needed for signing the image from binman?
> > >
> > > If I make imx8mm_evk_defconfig which produces via binman flash.bin and
> > > u-boot.itb I get the following:
> > >
> > > $ ./tools/binman/binman ls -i flash.bin
> > > binman: Cannot find FDT map in image
> > > $ ./tools/binman/binman ls -i u-boot.itb
> > > binman: Cannot find FDT map in image
> >
> > As the message says, you need an 'fdtmap' in the image:
> >
> >    fdtmap {
> >    }:
> >
>
> Simon,
>
> Sorry I still don't quite understand:
>
> diff --git a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> index f200afac9f..c6d8932fa4 100644
> --- a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> +++ b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> @@ -163,6 +163,9 @@
>         itb {
>                 filename = "u-boot.itb";
>
> +               fdtmap {
> +               };
> +
>                 fit {
>                         description = "Configuration to load ATF before U-Boot";
>                         #address-cells = <1>;
>
>
> $ make imx8mm_evk_defconfig
> $ make
> $ ./tools/binman/binman ls -i u-boot.itb
> Name          Image-pos  Size    Entry-type  Offset  Uncomp-size
> ------------------------------------------------------------------
> main-section          0   a3875  section          0
>   fdtmap              0     48d  fdtmap           0
>   fit               48d   a33e8  fit            48d
>
> For signing we need the loadaddr/offset/size of the components within
> the FIT image. Since binman is calling mkimage to create FIT images it
> seems outside the scope of binman to be able to dump details on the
> actual FIT image. Perhaps mkimage or another tool is to be used for
> that detail?

Yes, binman does not look inside FITs at presnt. You may want to look
at dump_image?

>
> > >
> > > I would very much like to understand how to use binman to get the
> > > various offsets needed for signing an IMX image for use with HAB.
> >
> > You should be able to add signing support to binman for your use case.
> > See for example how vblock.py works.
>
> I see... so your saying instead of using an external tool to generate
> a txt template with the offset/size values for the various blobs
> instead we need to implement for example a habv4.py to add a habv4
> entry and the python class would have access to the content handles to
> determine the addr/size in order to create the text template file fed
> to the NXP code signing tool?

Yes that should work. It is in fact what binman is designed for
(producing images, with all the signing, etc.)

Regards,
Simon
Heiko Schocher July 19, 2021, 4:16 a.m. UTC | #9
Hello Tim,

On 12.07.21 18:42, Tim Harvey wrote:
> On Sat, Jul 10, 2021 at 5:23 AM Heiko Schocher <hs@denx.de> wrote:
>>
>> Hello Tim,
>>
>> On 09.07.21 16:47, Tim Harvey wrote:
>>> On Wed, Jul 7, 2021 at 5:58 AM Teresa Remmet <t.remmet@phytec.de> wrote:
>>>>
>>>> Factor out the common node settings for dm-spl and dm-pre-reloc
>>>> and move them to imx8mp-u-boot.dtsi
>>>>
>>>> Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
>>>> Reviewed-by: Fabio Estevam <festevam@gmail.com>
>>>> Reviewed-by: Heiko Schocher <hs@denx.de>
>>>> ---
>>>> Changes in v3:
>>>> - Moved binman nodes to common imx8mp-u-boot.dtsi
>>>> Changes in v2:
>>>> - none
>>>>
>>>>  arch/arm/dts/imx8mp-evk-u-boot.dtsi           | 143 +----------------
>>>>  .../imx8mp-phyboard-pollux-rdk-u-boot.dtsi    |  39 +----
>>>>  arch/arm/dts/imx8mp-u-boot.dtsi               | 149 ++++++++++++++++++
>>>>  3 files changed, 153 insertions(+), 178 deletions(-)
>>>>  create mode 100644 arch/arm/dts/imx8mp-u-boot.dtsi
>>>>
> <snip>
>>>>
>>>
>>> Teresa,
>>>
>>> I've noticed many of the imx8m boards migrating to using binman for
>>> image packaging.
>>>
>>> Doesn't this change from having a single flash.bin encompasing the SPL
>>> and U-Boot proper to having split files? I noticed that happened with
>>> imx8mm_evk for example when it switched to binman.
>>
>> Yes, but you can easy generate there a single image again.
>>
>>> What are the benefits to using binman?
>>
>> Beside the pros from binmal in general, I see the benefit in special
>> for imx8mp, that you can get all infos you need for signing the image
>> from within the image. No need to save some log output from U-Boot
>> build and parse this output ...
>>
> 
> (+cc Simon)
> 
> Heiko,
> 
> And what are the pros from binman in general? I've read over
> tools/binman/binman.rst so I'm assuming you mean what is described
> there as benefits.

Yes!

> How do you get all the details needed for signing the image from binman?

Not from binman directly, instead for getting the offset and length
of the images you want to sign, I simply used "fdtget" to read them
from the u-boot image:

(extract from my yocto class... WIP also):

dec2hex() {
        echo 0x$(printf '%x' $1)
}

# $1 ... fit image name
# $2 ... part of fit image
fit_get_off() {
        val=$(fdtget $1 /images/$2 data-position)
        # dec -> hex
        dec2hex $val
}

# $1 ... fit image name
# $2 ... part of fit image
fit_get_len() {
        val=$(fdtget $1 /images/$2 data-size)
        dec2hex $val
}

example for atf:

off=$(fit_get_off u-boot.itb atf)
len=$(fit_get_len u-boot.itb atf)

Hope this helps?

> If I make imx8mm_evk_defconfig which produces via binman flash.bin and
> u-boot.itb I get the following:
> 
> $ ./tools/binman/binman ls -i flash.bin
> binman: Cannot find FDT map in image
> $ ./tools/binman/binman ls -i u-boot.itb
> binman: Cannot find FDT map in image
> 
> I would very much like to understand how to use binman to get the
> various offsets needed for signing an IMX image for use with HAB.
> 
> Thanks,
> 
> Tim
> 

bye,
Heiko
Heiko Schocher July 19, 2021, 4:18 a.m. UTC | #10
Hi Tim, Simon,

On 13.07.21 22:17, Simon Glass wrote:
> Hi Tim,
> 
> On Mon, 12 Jul 2021 at 14:58, Tim Harvey <tharvey@gateworks.com> wrote:
>>
>> On Mon, Jul 12, 2021 at 12:44 PM Simon Glass <sjg@chromium.org> wrote:
>>>
>>> Hi Tim,
>>>
>>> On Mon, 12 Jul 2021 at 10:42, Tim Harvey <tharvey@gateworks.com> wrote:
>>>>
>>>> On Sat, Jul 10, 2021 at 5:23 AM Heiko Schocher <hs@denx.de> wrote:
>>>>>
>>>>> Hello Tim,
>>>>>
>>>>> On 09.07.21 16:47, Tim Harvey wrote:
>>>>>> On Wed, Jul 7, 2021 at 5:58 AM Teresa Remmet <t.remmet@phytec.de> wrote:
>>>>>>>
>>>>>>> Factor out the common node settings for dm-spl and dm-pre-reloc
>>>>>>> and move them to imx8mp-u-boot.dtsi
>>>>>>>
>>>>>>> Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
>>>>>>> Reviewed-by: Fabio Estevam <festevam@gmail.com>
>>>>>>> Reviewed-by: Heiko Schocher <hs@denx.de>
>>>>>>> ---
>>>>>>> Changes in v3:
>>>>>>> - Moved binman nodes to common imx8mp-u-boot.dtsi
>>>>>>> Changes in v2:
>>>>>>> - none
>>>>>>>
>>>>>>>  arch/arm/dts/imx8mp-evk-u-boot.dtsi           | 143 +----------------
>>>>>>>  .../imx8mp-phyboard-pollux-rdk-u-boot.dtsi    |  39 +----
>>>>>>>  arch/arm/dts/imx8mp-u-boot.dtsi               | 149 ++++++++++++++++++
>>>>>>>  3 files changed, 153 insertions(+), 178 deletions(-)
>>>>>>>  create mode 100644 arch/arm/dts/imx8mp-u-boot.dtsi
>>>>>>>
>>>> <snip>
>>>>>>>
>>>>>>
>>>>>> Teresa,
>>>>>>
>>>>>> I've noticed many of the imx8m boards migrating to using binman for
>>>>>> image packaging.
>>>>>>
>>>>>> Doesn't this change from having a single flash.bin encompasing the SPL
>>>>>> and U-Boot proper to having split files? I noticed that happened with
>>>>>> imx8mm_evk for example when it switched to binman.
>>>>>
>>>>> Yes, but you can easy generate there a single image again.
>>>>>
>>>>>> What are the benefits to using binman?
>>>>>
>>>>> Beside the pros from binmal in general, I see the benefit in special
>>>>> for imx8mp, that you can get all infos you need for signing the image
>>>>> from within the image. No need to save some log output from U-Boot
>>>>> build and parse this output ...
>>>>>
>>>>
>>>> (+cc Simon)
>>>>
>>>> Heiko,
>>>>
>>>> And what are the pros from binman in general? I've read over
>>>> tools/binman/binman.rst so I'm assuming you mean what is described
>>>> there as benefits.
>>>>
>>>> How do you get all the details needed for signing the image from binman?
>>>>
>>>> If I make imx8mm_evk_defconfig which produces via binman flash.bin and
>>>> u-boot.itb I get the following:
>>>>
>>>> $ ./tools/binman/binman ls -i flash.bin
>>>> binman: Cannot find FDT map in image
>>>> $ ./tools/binman/binman ls -i u-boot.itb
>>>> binman: Cannot find FDT map in image
>>>
>>> As the message says, you need an 'fdtmap' in the image:
>>>
>>>    fdtmap {
>>>    }:
>>>
>>
>> Simon,
>>
>> Sorry I still don't quite understand:
>>
>> diff --git a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
>> b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
>> index f200afac9f..c6d8932fa4 100644
>> --- a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
>> +++ b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
>> @@ -163,6 +163,9 @@
>>         itb {
>>                 filename = "u-boot.itb";
>>
>> +               fdtmap {
>> +               };
>> +
>>                 fit {
>>                         description = "Configuration to load ATF before U-Boot";
>>                         #address-cells = <1>;
>>
>>
>> $ make imx8mm_evk_defconfig
>> $ make
>> $ ./tools/binman/binman ls -i u-boot.itb
>> Name          Image-pos  Size    Entry-type  Offset  Uncomp-size
>> ------------------------------------------------------------------
>> main-section          0   a3875  section          0
>>   fdtmap              0     48d  fdtmap           0
>>   fit               48d   a33e8  fit            48d
>>
>> For signing we need the loadaddr/offset/size of the components within
>> the FIT image. Since binman is calling mkimage to create FIT images it
>> seems outside the scope of binman to be able to dump details on the
>> actual FIT image. Perhaps mkimage or another tool is to be used for
>> that detail?
> 
> Yes, binman does not look inside FITs at presnt. You may want to look
> at dump_image?
> 
>>
>>>>
>>>> I would very much like to understand how to use binman to get the
>>>> various offsets needed for signing an IMX image for use with HAB.
>>>
>>> You should be able to add signing support to binman for your use case.
>>> See for example how vblock.py works.
>>
>> I see... so your saying instead of using an external tool to generate
>> a txt template with the offset/size values for the various blobs
>> instead we need to implement for example a habv4.py to add a habv4
>> entry and the python class would have access to the content handles to
>> determine the addr/size in order to create the text template file fed
>> to the NXP code signing tool?
> 
> Yes that should work. It is in fact what binman is designed for
> (producing images, with all the signing, etc.)

Ah, sounds interesting!

@Tim: Do you have time to look into this topic?

bye,
Heiko
Fabio Estevam Aug. 13, 2021, 12:55 p.m. UTC | #11
Hi Heiko,

On Sat, Jul 10, 2021 at 9:23 AM Heiko Schocher <hs@denx.de> wrote:

> > Doesn't this change from having a single flash.bin encompasing the SPL
> > and U-Boot proper to having split files? I noticed that happened with
> > imx8mm_evk for example when it switched to binman.
>
> Yes, but you can easy generate there a single image again.

Yes, that would be preferable

Any suggestions on how we can have a single bootable image again?

Thanks,

Fabio Estevam
Heiko Schocher Aug. 16, 2021, 10:27 a.m. UTC | #12
Hello Fabio,

On 13.08.21 14:55, Fabio Estevam wrote:
> Hi Heiko,
> 
> On Sat, Jul 10, 2021 at 9:23 AM Heiko Schocher <hs@denx.de> wrote:
> 
>>> Doesn't this change from having a single flash.bin encompasing the SPL
>>> and U-Boot proper to having split files? I noticed that happened with
>>> imx8mm_evk for example when it switched to binman.
>>
>> Yes, but you can easy generate there a single image again.
> 
> Yes, that would be preferable
> 
> Any suggestions on how we can have a single bootable image again?

I am not a binman expert, but may you try to add in your u-boot.dtsi
in your "binman" node at the end:

        imx-boot {
                filename = "imx-boot.bin";
                pad-byte = <0x00>;

                spl: blob-ext@1 {
                        offset = <0x0>;
                        filename = "flash.bin";
                };
                uboot: blob-ext@2 {
                        offset = <0x59000>;
                        filename = "u-boot.itb";
                };
        };

bye,
Heiko
Fabio Estevam Aug. 16, 2021, 1:56 p.m. UTC | #13
Hi Heiko,

On Mon, Aug 16, 2021 at 7:27 AM Heiko Schocher <hs@denx.de> wrote:

> I am not a binman expert, but may you try to add in your u-boot.dtsi
> in your "binman" node at the end:
>
>         imx-boot {
>                 filename = "imx-boot.bin";
>                 pad-byte = <0x00>;
>
>                 spl: blob-ext@1 {
>                         offset = <0x0>;
>                         filename = "flash.bin";
>                 };
>                 uboot: blob-ext@2 {
>                         offset = <0x59000>;
>                         filename = "u-boot.itb";
>                 };

This helped!

I had to change the offset and now I can flash only flash.bin
and it boots fine:

--- a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
+++ b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
@@ -217,4 +217,18 @@
                        };
                };
        };
+
+        imx-boot {
+                filename = "flash.bin";
+                pad-byte = <0x00>;
+
+                spl: blob-ext@1 {
+                        offset = <0x0>;
+                        filename = "flash.bin";
+                };
+                uboot: blob-ext@2 {
+                        offset = <0x57c00>;
+                        filename = "u-boot.itb";
+                };
+        };
 };

I will submit the patch shortly.

Thanks
Frieder Schrempf Aug. 16, 2021, 2:09 p.m. UTC | #14
Hi Fabio,

On 16.08.21 15:56, Fabio Estevam wrote:
> Hi Heiko,
> 
> On Mon, Aug 16, 2021 at 7:27 AM Heiko Schocher <hs@denx.de> wrote:
> 
>> I am not a binman expert, but may you try to add in your u-boot.dtsi
>> in your "binman" node at the end:
>>
>>         imx-boot {
>>                 filename = "imx-boot.bin";
>>                 pad-byte = <0x00>;
>>
>>                 spl: blob-ext@1 {
>>                         offset = <0x0>;
>>                         filename = "flash.bin";
>>                 };
>>                 uboot: blob-ext@2 {
>>                         offset = <0x59000>;
>>                         filename = "u-boot.itb";
>>                 };
> 
> This helped!
> 
> I had to change the offset and now I can flash only flash.bin
> and it boots fine:
> 
> --- a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> +++ b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> @@ -217,4 +217,18 @@
>                         };
>                 };
>         };
> +
> +        imx-boot {
> +                filename = "flash.bin";
> +                pad-byte = <0x00>;
> +
> +                spl: blob-ext@1 {
> +                        offset = <0x0>;
> +                        filename = "flash.bin";
> +                };
> +                uboot: blob-ext@2 {
> +                        offset = <0x57c00>;
> +                        filename = "u-boot.itb";
> +                };
> +        };
>  };
> 
> I will submit the patch shortly.

If you prepare a patch I would prefer to have separate names for the SPL image and the final concatenated image. So we don't overwrite the SPL image and can still use it on its own if needed.
But rather than choosing a new name for the complete image (as Heiko did above), I would like to rename the SPL image to something more meaningful like spl.bin first and then use flash.bin for the full image.
What do you think?

Thanks
Frieder
Frieder Schrempf Aug. 16, 2021, 2:18 p.m. UTC | #15
On 16.08.21 16:09, Frieder Schrempf wrote:
> Hi Fabio,
> 
> On 16.08.21 15:56, Fabio Estevam wrote:
>> Hi Heiko,
>>
>> On Mon, Aug 16, 2021 at 7:27 AM Heiko Schocher <hs@denx.de> wrote:
>>
>>> I am not a binman expert, but may you try to add in your u-boot.dtsi
>>> in your "binman" node at the end:
>>>
>>>         imx-boot {
>>>                 filename = "imx-boot.bin";
>>>                 pad-byte = <0x00>;
>>>
>>>                 spl: blob-ext@1 {
>>>                         offset = <0x0>;
>>>                         filename = "flash.bin";
>>>                 };
>>>                 uboot: blob-ext@2 {
>>>                         offset = <0x59000>;
>>>                         filename = "u-boot.itb";
>>>                 };
>>
>> This helped!
>>
>> I had to change the offset and now I can flash only flash.bin
>> and it boots fine:
>>
>> --- a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
>> +++ b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
>> @@ -217,4 +217,18 @@
>>                         };
>>                 };
>>         };
>> +
>> +        imx-boot {
>> +                filename = "flash.bin";
>> +                pad-byte = <0x00>;
>> +
>> +                spl: blob-ext@1 {
>> +                        offset = <0x0>;
>> +                        filename = "flash.bin";
>> +                };
>> +                uboot: blob-ext@2 {
>> +                        offset = <0x57c00>;
>> +                        filename = "u-boot.itb";
>> +                };
>> +        };
>>  };
>>
>> I will submit the patch shortly.
> 
> If you prepare a patch I would prefer to have separate names for the SPL image and the final concatenated image. So we don't overwrite the SPL image and can still use it on its own if needed.
> But rather than choosing a new name for the complete image (as Heiko did above), I would like to rename the SPL image to something more meaningful like spl.bin first and then use flash.bin for the full image.
> What do you think?

And it would also be nice if we could share the common parts of the binman config between the different boards to avoid having this in each boards dtsi and to avoid having diverging configurations, where e. g. flash.bin refers to different kinds of images.
Fabio Estevam Aug. 16, 2021, 3:10 p.m. UTC | #16
Hi Frieder,

On Mon, Aug 16, 2021 at 11:18 AM Frieder Schrempf
<frieder.schrempf@kontron.de> wrote:

> > If you prepare a patch I would prefer to have separate names for the SPL image and the final concatenated image. So we don't overwrite the SPL image and can still use it on its own if needed.
> > But rather than choosing a new name for the complete image (as Heiko did above), I would like to rename the SPL image to something more meaningful like spl.bin first and then use flash.bin for the full image.
> > What do you think?

It seems we cannot rename the SPL to spl.bin as this would cause
breakage to i.MX8MM boards that do not use binman, such as
verdin-imx8mm_defconfig.

> And it would also be nice if we could share the common parts of the binman config between the different boards to avoid having this in each boards dtsi and to avoid having diverging configurations, where e. g. flash.bin refers to different kinds of images.

Agreed.

Let me try to send a proposal for imx8mm-evk first. If it gets
accepted, then I can try to make a common approach.

Regards,

Fabio Estevam
Marcel Ziswiler Aug. 18, 2021, 8:17 a.m. UTC | #17
Hi Fabio and Frieder

On Mon, 2021-08-16 at 12:10 -0300, Fabio Estevam wrote:
> Hi Frieder,
> 
> On Mon, Aug 16, 2021 at 11:18 AM Frieder Schrempf
> <frieder.schrempf@kontron.de> wrote:
> 
> > > If you prepare a patch I would prefer to have separate names for the SPL image and the final concatenated
> > > image. So we don't overwrite the SPL image and can still use it on its own if needed.
> > > But rather than choosing a new name for the complete image (as Heiko did above), I would like to rename
> > > the SPL image to something more meaningful like spl.bin first and then use flash.bin for the full image.
> > > What do you think?
> 
> It seems we cannot rename the SPL to spl.bin as this would cause
> breakage to i.MX8MM boards that do not use binman, such as
> verdin-imx8mm_defconfig.

I am actually just about to send our conversion to using binman for this as well (;-p).

> > And it would also be nice if we could share the common parts of the binman config between the different
> > boards to avoid having this in each boards dtsi and to avoid having diverging configurations, where e. g.
> > flash.bin refers to different kinds of images.
> 
> Agreed.

I am also all in for this idea. Should I still be sending my patch set converting Verdin iMX8M Mini separately
first? Maybe that would be the easiest and we could then transition them all to a common approach, not?

> Let me try to send a proposal for imx8mm-evk first. If it gets
> accepted, then I can try to make a common approach.

Yes, that's kinda what I was thinking. Thanks!

> Regards,
> 
> Fabio Estevam

Cheers

Marcel
diff mbox series

Patch

diff --git a/arch/arm/dts/imx8mp-evk-u-boot.dtsi b/arch/arm/dts/imx8mp-evk-u-boot.dtsi
index 4162f41cffb6..2abcf1f03d4f 100644
--- a/arch/arm/dts/imx8mp-evk-u-boot.dtsi
+++ b/arch/arm/dts/imx8mp-evk-u-boot.dtsi
@@ -3,11 +3,9 @@ 
  * Copyright 2019 NXP
  */
 
-/ {
-	binman: binman {
-		multiple-images;
-	};
+#include "imx8mp-u-boot.dtsi"
 
+/ {
 	wdt-reboot {
 		compatible = "wdt-reboot";
 		wdt = <&wdog1>;
@@ -21,43 +19,6 @@ 
 	};
 };
 
-&{/soc@0} {
-	u-boot,dm-pre-reloc;
-	u-boot,dm-spl;
-};
-
-&clk {
-	u-boot,dm-spl;
-	u-boot,dm-pre-reloc;
-};
-
-&osc_32k {
-	u-boot,dm-spl;
-	u-boot,dm-pre-reloc;
-};
-
-&osc_24m {
-	u-boot,dm-spl;
-	u-boot,dm-pre-reloc;
-};
-
-&aips1 {
-	u-boot,dm-spl;
-	u-boot,dm-pre-reloc;
-};
-
-&aips2 {
-	u-boot,dm-spl;
-};
-
-&aips3 {
-	u-boot,dm-spl;
-};
-
-&iomuxc {
-	u-boot,dm-spl;
-};
-
 &reg_usdhc2_vmmc {
 	u-boot,off-on-delay-us = <20000>;
 };
@@ -156,104 +117,4 @@ 
 	phy-reset-post-delay = <100>;
 };
 
-&binman {
-	 u-boot-spl-ddr {
-		filename = "u-boot-spl-ddr.bin";
-		pad-byte = <0xff>;
-		align-size = <4>;
-		align = <4>;
 
-		u-boot-spl {
-			align-end = <4>;
-		};
-
-		blob_1: blob-ext@1 {
-			filename = "lpddr4_pmu_train_1d_imem_202006.bin";
-			size = <0x8000>;
-		};
-
-		blob_2: blob-ext@2 {
-			filename = "lpddr4_pmu_train_1d_dmem_202006.bin";
-			size = <0x4000>;
-		};
-
-		blob_3: blob-ext@3 {
-			filename = "lpddr4_pmu_train_2d_imem_202006.bin";
-			size = <0x8000>;
-		};
-
-		blob_4: blob-ext@4 {
-			filename = "lpddr4_pmu_train_2d_dmem_202006.bin";
-			size = <0x4000>;
-		};
-	};
-
-
-       flash {
-		mkimage {
-			args = "-n spl/u-boot-spl.cfgout -T imx8mimage -e 0x920000";
-
-			blob {
-				filename = "u-boot-spl-ddr.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 = <0x970000>;
-					entry = <0x970000>;
-
-					atf_blob: blob-ext {
-						filename = "bl31.bin";
-					};
-				};
-
-				fdt {
-					description = "NAME";
-					type = "flat_dt";
-					compression = "none";
-
-					uboot_fdt_blob: blob-ext {
-						filename = "u-boot.dtb";
-					};
-				};
-			};
-
-			configurations {
-				default = "conf";
-
-				conf {
-					description = "NAME";
-					firmware = "uboot";
-					loadables = "atf";
-					fdt = "fdt";
-				};
-			};
-		};
-	};
-};
diff --git a/arch/arm/dts/imx8mp-phyboard-pollux-rdk-u-boot.dtsi b/arch/arm/dts/imx8mp-phyboard-pollux-rdk-u-boot.dtsi
index 20e7f63ff91f..6c1528934a98 100644
--- a/arch/arm/dts/imx8mp-phyboard-pollux-rdk-u-boot.dtsi
+++ b/arch/arm/dts/imx8mp-phyboard-pollux-rdk-u-boot.dtsi
@@ -4,6 +4,8 @@ 
  * Author: Teresa Remmet <t.remmet@phytec.de>
  */
 
+#include "imx8mp-u-boot.dtsi"
+
 / {
 	wdt-reboot {
 		compatible = "wdt-reboot";
@@ -12,43 +14,6 @@ 
 	};
 };
 
-&{/soc@0} {
-	u-boot,dm-pre-reloc;
-	u-boot,dm-spl;
-};
-
-&clk {
-	u-boot,dm-spl;
-	u-boot,dm-pre-reloc;
-};
-
-&osc_32k {
-	u-boot,dm-spl;
-	u-boot,dm-pre-reloc;
-};
-
-&osc_24m {
-	u-boot,dm-spl;
-	u-boot,dm-pre-reloc;
-};
-
-&aips1 {
-	u-boot,dm-spl;
-	u-boot,dm-pre-reloc;
-};
-
-&aips2 {
-	u-boot,dm-spl;
-};
-
-&aips3 {
-	u-boot,dm-spl;
-};
-
-&iomuxc {
-	u-boot,dm-spl;
-};
-
 &reg_usdhc2_vmmc {
 	u-boot,dm-spl;
 };
diff --git a/arch/arm/dts/imx8mp-u-boot.dtsi b/arch/arm/dts/imx8mp-u-boot.dtsi
new file mode 100644
index 000000000000..d61346da3032
--- /dev/null
+++ b/arch/arm/dts/imx8mp-u-boot.dtsi
@@ -0,0 +1,149 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2021 PHYTEC Messtechnik GmbH
+ * Author: Teresa Remmet <t.remmet@phytec.de>
+ */
+
+/ {
+	binman: binman {
+		multiple-images;
+	};
+};
+
+&{/soc@0} {
+	u-boot,dm-pre-reloc;
+	u-boot,dm-spl;
+};
+
+&clk {
+	u-boot,dm-spl;
+	u-boot,dm-pre-reloc;
+};
+
+&osc_32k {
+	u-boot,dm-spl;
+	u-boot,dm-pre-reloc;
+};
+
+&osc_24m {
+	u-boot,dm-spl;
+	u-boot,dm-pre-reloc;
+};
+
+&aips1 {
+	u-boot,dm-spl;
+	u-boot,dm-pre-reloc;
+};
+
+&aips2 {
+	u-boot,dm-spl;
+};
+
+&aips3 {
+	u-boot,dm-spl;
+};
+
+&iomuxc {
+	u-boot,dm-spl;
+};
+
+&binman {
+	 u-boot-spl-ddr {
+		filename = "u-boot-spl-ddr.bin";
+		pad-byte = <0xff>;
+		align-size = <4>;
+		align = <4>;
+
+		u-boot-spl {
+			align-end = <4>;
+		};
+
+		blob_1: blob-ext@1 {
+			filename = "lpddr4_pmu_train_1d_imem_202006.bin";
+			size = <0x8000>;
+		};
+
+		blob_2: blob-ext@2 {
+			filename = "lpddr4_pmu_train_1d_dmem_202006.bin";
+			size = <0x4000>;
+		};
+
+		blob_3: blob-ext@3 {
+			filename = "lpddr4_pmu_train_2d_imem_202006.bin";
+			size = <0x8000>;
+		};
+
+		blob_4: blob-ext@4 {
+			filename = "lpddr4_pmu_train_2d_dmem_202006.bin";
+			size = <0x4000>;
+		};
+	};
+
+	flash {
+		mkimage {
+			args = "-n spl/u-boot-spl.cfgout -T imx8mimage -e 0x920000";
+
+			blob {
+				filename = "u-boot-spl-ddr.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 = <0x970000>;
+					entry = <0x970000>;
+
+					atf_blob: blob-ext {
+						filename = "bl31.bin";
+					};
+				};
+
+				fdt {
+					description = "NAME";
+					type = "flat_dt";
+					compression = "none";
+
+					uboot_fdt_blob: blob-ext {
+						filename = "u-boot.dtb";
+					};
+				};
+			};
+
+			configurations {
+				default = "conf";
+
+				conf {
+					description = "NAME";
+					firmware = "uboot";
+					loadables = "atf";
+					fdt = "fdt";
+				};
+			};
+		};
+	};
+};