All of lore.kernel.org
 help / color / mirror / Atom feed
* using binman fails boot
@ 2021-07-15 22:57 Tim Harvey
  2021-07-16  4:30 ` Simon Glass
  0 siblings, 1 reply; 14+ messages in thread
From: Tim Harvey @ 2021-07-15 22:57 UTC (permalink / raw)
  To: u-boot, Simon Glass

Greetings,

I'm taking a look at moving imx8mm-venice to use binman for packaging.
After doing so U-Boot proper fails to boot:

U-Boot SPL 2021.07-00475-g1126252f40 (Jul 15 2021 - 11:09:02 -0700)
GSC     : v58 0xf098 RST:VIN Thermal Protection Disabled
Model   : GW7300-00-B1B
Serial  : 852420
MFGDate : 10-26-2020
RTC     : 122
PMIC    : MP5416
DRAM    : LPDDR4 1 GiB
WDT:   Not starting
Trying to boot from MMC1
DTB     : imx8mm-venice-gw73xx-0x


U-Boot 2021.07-00475-g1126252f40 (Jul 15 2021 - 11:09:02 -0700)

CPU:   Freescale i.MX8MMQ rev1.0 1600 MHz (running at 1200 MHz)
CPU:   Industrial temperature grade (-40C to 105C) at 43C
Reset cause: POR
Model: Gateworks Venice GW73xx-0x i.MX8MM Development Kit
DRAM:  1 GiB
temp    : 38.3C
vdd_bat : 0.000V
vdd_vin : 15.731V
vdd_adc1: 0.000V
vdd_adc2: 0.000V
vdd_dram: 1.093V
vdd_1p2 : 1.193V
vdd_1p0 : 0.985V
vdd_2p5 : 2.470V
vdd_3p3 : 3.250V
vdd_0p95: 0.948V
vdd_1p8 : 1.799V
vdd_gsc : 3.262V
initcall sequence 000000007ffc4f58 failed at call 0000000040255910 (err=-2)
### ERROR ### Please RESET the board ###

Any ideas what this could be?

My patch in case I'm missing something obvious is:
diff --git a/arch/arm/dts/imx8mm-venice-u-boot.dtsi
b/arch/arm/dts/imx8mm-venice-u-boot.dtsi
index 42b2903f04..f738ea770e 100644
--- a/arch/arm/dts/imx8mm-venice-u-boot.dtsi
+++ b/arch/arm/dts/imx8mm-venice-u-boot.dtsi
@@ -6,6 +6,10 @@
 #include "imx8mm-u-boot.dtsi"

 / {
+       binman: binman {
+               multiple-images;
+       };
+
        wdt-reboot {
                compatible = "wdt-reboot";
                wdt = <&wdog1>;
@@ -68,3 +72,101 @@
 &wdog1 {
        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.bin";
+                       size = <0x8000>;
+               };
+
+               blob_2: blob-ext@2 {
+                       filename = "lpddr4_pmu_train_1d_dmem.bin";
+                       size = <0x4000>;
+               };
+
+               blob_3: blob-ext@3 {
+                       filename = "lpddr4_pmu_train_2d_imem.bin";
+                       size = <0x8000>;
+               };
+
+               blob_4: blob-ext@4 {
+                       filename = "lpddr4_pmu_train_2d_dmem.bin";
+                       size = <0x4000>;
+               };
+       };
+
+       flash {
+               mkimage {
+                       args = "-n spl/u-boot-spl.cfgout -T imx8mimage
-e 0x7e1000";
+
+                       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>;
+                       fit,fdt-list = "of-list";
+
+                       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 = <0x920000>;
+                                       entry = <0x920000>;
+
+                                       atf_blob: blob-ext {
+                                               filename = "bl31.bin";
+                                       };
+                               };
+
+                               @fdt-SEQ {
+                                       description = "NAME";
+                                       type = "flat_dt";
+                                       compression = "none";
+                               };
+                       };
+
+                       configurations {
+                               default = "@config-DEFAULT-SEQ";
+
+                               @config-SEQ {
+                                       description = "NAME";
+                                       firmware = "uboot";
+                                       loadables = "atf";
+                                       fdt = "fdt-SEQ";
+                               };
+                       };
+               };
+       };
+};
diff --git a/arch/arm/mach-imx/imx8m/Kconfig b/arch/arm/mach-imx/imx8m/Kconfig
index 78b6108ff5..71af1bd799 100644
--- a/arch/arm/mach-imx/imx8m/Kconfig
+++ b/arch/arm/mach-imx/imx8m/Kconfig
@@ -71,6 +71,7 @@ config TARGET_IMX8MM_ICORE_MX8MM

 config TARGET_IMX8MM_VENICE
        bool "Support Gateworks Venice iMX8M Mini module"
+       select BINMAN
        select IMX8MM
        select SUPPORT_SPL
        select IMX8M_LPDDR4
diff --git a/board/gateworks/venice/README b/board/gateworks/venice/README
index 6a0ab1ef10..95e35c4734 100644
--- a/board/gateworks/venice/README
+++ b/board/gateworks/venice/README
@@ -25,10 +25,18 @@ $ cp firmware-imx-8.9/firmware/ddr/synopsys/lpddr4*.bin .
 Build U-Boot
 ============
 $ make imx8mm_venice_defconfig
-$ make flash.bin CROSS_COMPILE=aarch64-linux-gnu- ATF_LOAD_ADDR=0x920000
+$ make CROSS_COMPILE=aarch64-linux-gnu- ATF_LOAD_ADDR=0x920000

 Update eMMC
 ===========
 => tftpboot $loadaddr flash.bin
 => setexpr blkcnt $filesize + 0x1ff && setexpr blkcnt $blkcnt / 0x200
 => mmc dev 2 && mmc write $loadaddr 0x42 $blkcnt
+=> tftpboot $loadaddr u-boot.itb
+=> setexpr blkcnt $filesize + 0x1ff && setexpr blkcnt $blkcnt / 0x200
+=> mmc dev 2 && mmc write $loadaddr 0x300 $blkcnt
+
+Update uSD:
+===========
+sudo dd if=flash.bin of=/dev/sd[x] bs=1024 seek=33 conv=notrunc
+sudo dd if=u-boot.itb of=/dev/sd[x] bs=1024 seek=384 conv=sync
diff --git a/board/gateworks/venice/imximage-8mm-lpddr4.cfg
b/board/gateworks/venice/imximage-8mm-lpddr4.cfg
new file mode 100644
index 0000000000..df3edb17ab
--- /dev/null
+++ b/board/gateworks/venice/imximage-8mm-lpddr4.cfg
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright 2021 Gateworks Corporation
+ */
+
+#define __ASSEMBLY__
+
+BOOT_FROM      sd
+LOADER         mkimage.flash.mkimage   0x7E1000
diff --git a/configs/imx8mm_venice_defconfig b/configs/imx8mm_venice_defconfig
index 883be27cd7..c6c790734a 100644
--- a/configs/imx8mm_venice_defconfig
+++ b/configs/imx8mm_venice_defconfig
@@ -26,9 +26,9 @@ CONFIG_DISTRO_DEFAULTS=y
 CONFIG_FIT=y
 CONFIG_FIT_EXTERNAL_OFFSET=0x3000
 CONFIG_SPL_LOAD_FIT=y
-CONFIG_SPL_FIT_GENERATOR="arch/arm/mach-imx/mkimage_fit_atf.sh"
+# CONFIG_USE_SPL_FIT_GENERATOR is not set
 CONFIG_OF_SYSTEM_SETUP=y
-CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=arch/arm/mach-imx/imx8m/imximage-8mm-lpddr4.cfg"
+CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=board/gateworks/venice/imximage-8mm-lpddr4.cfg"
 # CONFIG_USE_BOOTCOMMAND is not set
 CONFIG_USE_PREBOOT=y
 CONFIG_PREBOOT="gsc wd-disable"
@@ -62,7 +62,7 @@ CONFIG_CMD_EXT4_WRITE=y
 # CONFIG_SPL_EFI_PARTITION is not set
 CONFIG_OF_CONTROL=y
 CONFIG_SPL_OF_CONTROL=y
-CONFIG_OF_LIST="imx8mm-venice-gw71xx-0x imx8mm-venice-gw72xx-0x
imx8mm-venice-gw73xx-0x imx8mm-venice-gw7901 imx8mm-venice-gw7902"
+CONFIG_OF_LIST="imx8mm-venice imx8mm-venice-gw71xx-0x
imx8mm-venice-gw72xx-0x imx8mm-venice-gw73xx-0x imx8mm-venice-gw7901
imx8mm-venice-gw7902"
 CONFIG_ENV_IS_IN_MMC=y
 CONFIG_SYS_REDUNDAND_ENVIRONMENT=y
 CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG=y

A follow-on question is that I would like to investigate using binman
in the SPL to dynamically access the IMX8M ddr training blobs so that
we don't have to waste padding space taking them onto the end of the
SPL which is currently done. The lpddr4 training blobs I'm using
currently take up 57k without padding compared to 81k with padding.
The location of them is handled in ddr_load_train_firmware.

If I add the following to my SPL:
diff --git a/board/gateworks/venice/spl.c b/board/gateworks/venice/spl.c
index d0a490b0e6..62eb67fa5e 100644
--- a/board/gateworks/venice/spl.c
+++ b/board/gateworks/venice/spl.c
@@ -3,6 +3,7 @@
  * Copyright 2021 Gateworks Corporation
  */

+#include <binman_sym.h>
 #include <common.h>
 #include <cpu_func.h>
 #include <hang.h>
@@ -252,6 +253,8 @@ static int power_init_board(void)
        return 0;
 }

+binman_sym_declare(ulong, blob_1, image_pos);
+
 void board_init_f(ulong dummy)
 {
        struct udevice *dev;
@@ -291,6 +294,8 @@ void board_init_f(ulong dummy)
        gpio_request(PCIE_RSTN, "perst#");
        gpio_direction_output(PCIE_RSTN, 0);

+       printf("%s: blob_1:0x%0lx\n", __func__, binman_sym(ulong,
blob_1, image_pos));
+
        /* GSC */
        dram_sz = gsc_init(0);

I get 'blob_1:0x0' which is not what I expected.

If I understand correctly binman is using linker symbols to determine
where things are in the image? What I don't quite understand is what
symbols are valid to use assuming my dtsi above. The binman.rst docs
talk use 'u_boot_any' as an example which apparently can match
'u-boot.bin', 'u-boot.img', and 'u-boot-nodtb.bin' but I can't find
the code that somehow translates this meaning.

Best regards,

Tim

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: using binman fails boot
  2021-07-15 22:57 using binman fails boot Tim Harvey
@ 2021-07-16  4:30 ` Simon Glass
  2021-07-16 21:43   ` Tim Harvey
  0 siblings, 1 reply; 14+ messages in thread
From: Simon Glass @ 2021-07-16  4:30 UTC (permalink / raw)
  To: Tim Harvey; +Cc: u-boot

Hi Tim,

On Thu, 15 Jul 2021 at 16:58, Tim Harvey <tharvey@gateworks.com> wrote:
>
> Greetings,
>
> I'm taking a look at moving imx8mm-venice to use binman for packaging.
> After doing so U-Boot proper fails to boot:
>
> U-Boot SPL 2021.07-00475-g1126252f40 (Jul 15 2021 - 11:09:02 -0700)
> GSC     : v58 0xf098 RST:VIN Thermal Protection Disabled
> Model   : GW7300-00-B1B
> Serial  : 852420
> MFGDate : 10-26-2020
> RTC     : 122
> PMIC    : MP5416
> DRAM    : LPDDR4 1 GiB
> WDT:   Not starting
> Trying to boot from MMC1
> DTB     : imx8mm-venice-gw73xx-0x
>
>
> U-Boot 2021.07-00475-g1126252f40 (Jul 15 2021 - 11:09:02 -0700)
>
> CPU:   Freescale i.MX8MMQ rev1.0 1600 MHz (running at 1200 MHz)
> CPU:   Industrial temperature grade (-40C to 105C) at 43C
> Reset cause: POR
> Model: Gateworks Venice GW73xx-0x i.MX8MM Development Kit
> DRAM:  1 GiB
> temp    : 38.3C
> vdd_bat : 0.000V
> vdd_vin : 15.731V
> vdd_adc1: 0.000V
> vdd_adc2: 0.000V
> vdd_dram: 1.093V
> vdd_1p2 : 1.193V
> vdd_1p0 : 0.985V
> vdd_2p5 : 2.470V
> vdd_3p3 : 3.250V
> vdd_0p95: 0.948V
> vdd_1p8 : 1.799V
> vdd_gsc : 3.262V
> initcall sequence 000000007ffc4f58 failed at call 0000000040255910 (err=-2)
> ### ERROR ### Please RESET the board ###
>
> Any ideas what this could be?

I don't have much idea. What is the initcall that is failing? Can you
check u-boot.map ? That might give a clue as to what is failing. I
assume the DT is passed to U-Boot somehow from SPL?

>
> My patch in case I'm missing something obvious is:
> diff --git a/arch/arm/dts/imx8mm-venice-u-boot.dtsi
> b/arch/arm/dts/imx8mm-venice-u-boot.dtsi
> index 42b2903f04..f738ea770e 100644
> --- a/arch/arm/dts/imx8mm-venice-u-boot.dtsi
> +++ b/arch/arm/dts/imx8mm-venice-u-boot.dtsi
> @@ -6,6 +6,10 @@
>  #include "imx8mm-u-boot.dtsi"
>
>  / {
> +       binman: binman {
> +               multiple-images;
> +       };
> +
>         wdt-reboot {
>                 compatible = "wdt-reboot";
>                 wdt = <&wdog1>;
> @@ -68,3 +72,101 @@
>  &wdog1 {
>         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.bin";
> +                       size = <0x8000>;
> +               };
> +
> +               blob_2: blob-ext@2 {
> +                       filename = "lpddr4_pmu_train_1d_dmem.bin";
> +                       size = <0x4000>;
> +               };
> +
> +               blob_3: blob-ext@3 {
> +                       filename = "lpddr4_pmu_train_2d_imem.bin";
> +                       size = <0x8000>;
> +               };
> +
> +               blob_4: blob-ext@4 {
> +                       filename = "lpddr4_pmu_train_2d_dmem.bin";
> +                       size = <0x4000>;
> +               };
> +       };
> +
> +       flash {
> +               mkimage {
> +                       args = "-n spl/u-boot-spl.cfgout -T imx8mimage
> -e 0x7e1000";
> +
> +                       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>;
> +                       fit,fdt-list = "of-list";
> +
> +                       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 = <0x920000>;
> +                                       entry = <0x920000>;
> +
> +                                       atf_blob: blob-ext {
> +                                               filename = "bl31.bin";
> +                                       };
> +                               };
> +
> +                               @fdt-SEQ {
> +                                       description = "NAME";
> +                                       type = "flat_dt";
> +                                       compression = "none";
> +                               };
> +                       };
> +
> +                       configurations {
> +                               default = "@config-DEFAULT-SEQ";
> +
> +                               @config-SEQ {
> +                                       description = "NAME";
> +                                       firmware = "uboot";
> +                                       loadables = "atf";
> +                                       fdt = "fdt-SEQ";
> +                               };
> +                       };
> +               };
> +       };
> +};
> diff --git a/arch/arm/mach-imx/imx8m/Kconfig b/arch/arm/mach-imx/imx8m/Kconfig
> index 78b6108ff5..71af1bd799 100644
> --- a/arch/arm/mach-imx/imx8m/Kconfig
> +++ b/arch/arm/mach-imx/imx8m/Kconfig
> @@ -71,6 +71,7 @@ config TARGET_IMX8MM_ICORE_MX8MM
>
>  config TARGET_IMX8MM_VENICE
>         bool "Support Gateworks Venice iMX8M Mini module"
> +       select BINMAN
>         select IMX8MM
>         select SUPPORT_SPL
>         select IMX8M_LPDDR4
> diff --git a/board/gateworks/venice/README b/board/gateworks/venice/README
> index 6a0ab1ef10..95e35c4734 100644
> --- a/board/gateworks/venice/README
> +++ b/board/gateworks/venice/README
> @@ -25,10 +25,18 @@ $ cp firmware-imx-8.9/firmware/ddr/synopsys/lpddr4*.bin .
>  Build U-Boot
>  ============
>  $ make imx8mm_venice_defconfig
> -$ make flash.bin CROSS_COMPILE=aarch64-linux-gnu- ATF_LOAD_ADDR=0x920000
> +$ make CROSS_COMPILE=aarch64-linux-gnu- ATF_LOAD_ADDR=0x920000
>
>  Update eMMC
>  ===========
>  => tftpboot $loadaddr flash.bin
>  => setexpr blkcnt $filesize + 0x1ff && setexpr blkcnt $blkcnt / 0x200
>  => mmc dev 2 && mmc write $loadaddr 0x42 $blkcnt
> +=> tftpboot $loadaddr u-boot.itb
> +=> setexpr blkcnt $filesize + 0x1ff && setexpr blkcnt $blkcnt / 0x200
> +=> mmc dev 2 && mmc write $loadaddr 0x300 $blkcnt
> +
> +Update uSD:
> +===========
> +sudo dd if=flash.bin of=/dev/sd[x] bs=1024 seek=33 conv=notrunc
> +sudo dd if=u-boot.itb of=/dev/sd[x] bs=1024 seek=384 conv=sync
> diff --git a/board/gateworks/venice/imximage-8mm-lpddr4.cfg
> b/board/gateworks/venice/imximage-8mm-lpddr4.cfg
> new file mode 100644
> index 0000000000..df3edb17ab
> --- /dev/null
> +++ b/board/gateworks/venice/imximage-8mm-lpddr4.cfg
> @@ -0,0 +1,9 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright 2021 Gateworks Corporation
> + */
> +
> +#define __ASSEMBLY__
> +
> +BOOT_FROM      sd
> +LOADER         mkimage.flash.mkimage   0x7E1000
> diff --git a/configs/imx8mm_venice_defconfig b/configs/imx8mm_venice_defconfig
> index 883be27cd7..c6c790734a 100644
> --- a/configs/imx8mm_venice_defconfig
> +++ b/configs/imx8mm_venice_defconfig
> @@ -26,9 +26,9 @@ CONFIG_DISTRO_DEFAULTS=y
>  CONFIG_FIT=y
>  CONFIG_FIT_EXTERNAL_OFFSET=0x3000
>  CONFIG_SPL_LOAD_FIT=y
> -CONFIG_SPL_FIT_GENERATOR="arch/arm/mach-imx/mkimage_fit_atf.sh"
> +# CONFIG_USE_SPL_FIT_GENERATOR is not set
>  CONFIG_OF_SYSTEM_SETUP=y
> -CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=arch/arm/mach-imx/imx8m/imximage-8mm-lpddr4.cfg"
> +CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=board/gateworks/venice/imximage-8mm-lpddr4.cfg"
>  # CONFIG_USE_BOOTCOMMAND is not set
>  CONFIG_USE_PREBOOT=y
>  CONFIG_PREBOOT="gsc wd-disable"
> @@ -62,7 +62,7 @@ CONFIG_CMD_EXT4_WRITE=y
>  # CONFIG_SPL_EFI_PARTITION is not set
>  CONFIG_OF_CONTROL=y
>  CONFIG_SPL_OF_CONTROL=y
> -CONFIG_OF_LIST="imx8mm-venice-gw71xx-0x imx8mm-venice-gw72xx-0x
> imx8mm-venice-gw73xx-0x imx8mm-venice-gw7901 imx8mm-venice-gw7902"
> +CONFIG_OF_LIST="imx8mm-venice imx8mm-venice-gw71xx-0x
> imx8mm-venice-gw72xx-0x imx8mm-venice-gw73xx-0x imx8mm-venice-gw7901
> imx8mm-venice-gw7902"
>  CONFIG_ENV_IS_IN_MMC=y
>  CONFIG_SYS_REDUNDAND_ENVIRONMENT=y
>  CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG=y
>
> A follow-on question is that I would like to investigate using binman
> in the SPL to dynamically access the IMX8M ddr training blobs so that
> we don't have to waste padding space taking them onto the end of the
> SPL which is currently done. The lpddr4 training blobs I'm using
> currently take up 57k without padding compared to 81k with padding.
> The location of them is handled in ddr_load_train_firmware.
>
> If I add the following to my SPL:
> diff --git a/board/gateworks/venice/spl.c b/board/gateworks/venice/spl.c
> index d0a490b0e6..62eb67fa5e 100644
> --- a/board/gateworks/venice/spl.c
> +++ b/board/gateworks/venice/spl.c
> @@ -3,6 +3,7 @@
>   * Copyright 2021 Gateworks Corporation
>   */
>
> +#include <binman_sym.h>
>  #include <common.h>
>  #include <cpu_func.h>
>  #include <hang.h>
> @@ -252,6 +253,8 @@ static int power_init_board(void)
>         return 0;
>  }
>
> +binman_sym_declare(ulong, blob_1, image_pos);
> +
>  void board_init_f(ulong dummy)
>  {
>         struct udevice *dev;
> @@ -291,6 +294,8 @@ void board_init_f(ulong dummy)
>         gpio_request(PCIE_RSTN, "perst#");
>         gpio_direction_output(PCIE_RSTN, 0);
>
> +       printf("%s: blob_1:0x%0lx\n", __func__, binman_sym(ulong,
> blob_1, image_pos));
> +
>         /* GSC */
>         dram_sz = gsc_init(0);
>
> I get 'blob_1:0x0' which is not what I expected.
>
> If I understand correctly binman is using linker symbols to determine
> where things are in the image? What I don't quite understand is what
> symbols are valid to use assuming my dtsi above. The binman.rst docs
> talk use 'u_boot_any' as an example which apparently can match
> 'u-boot.bin', 'u-boot.img', and 'u-boot-nodtb.bin' but I can't find
> the code that somehow translates this meaning.

Actually any symbol can be used. It basically depends on the name of
the entry in your image description. So here it would be
blob-ext@1...I think that translates to blob_ext_1 but I'm not sure
about the @. You could try blob-ext-1 instead. It does not know about
phandles or labels.

If you pass BINMAN_VERBOSE=4 to the build you should see it talking
about writing symbols into the SPL image.

Regards,
Simon

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: using binman fails boot
  2021-07-16  4:30 ` Simon Glass
@ 2021-07-16 21:43   ` Tim Harvey
  2021-07-16 22:11     ` Simon Glass
  0 siblings, 1 reply; 14+ messages in thread
From: Tim Harvey @ 2021-07-16 21:43 UTC (permalink / raw)
  To: Simon Glass; +Cc: u-boot

On Thu, Jul 15, 2021 at 9:30 PM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Tim,
>
> On Thu, 15 Jul 2021 at 16:58, Tim Harvey <tharvey@gateworks.com> wrote:
> >
> > Greetings,
> >
> > I'm taking a look at moving imx8mm-venice to use binman for packaging.
> > After doing so U-Boot proper fails to boot:
> >
> > U-Boot SPL 2021.07-00475-g1126252f40 (Jul 15 2021 - 11:09:02 -0700)
> > GSC     : v58 0xf098 RST:VIN Thermal Protection Disabled
> > Model   : GW7300-00-B1B
> > Serial  : 852420
> > MFGDate : 10-26-2020
> > RTC     : 122
> > PMIC    : MP5416
> > DRAM    : LPDDR4 1 GiB
> > WDT:   Not starting
> > Trying to boot from MMC1
> > DTB     : imx8mm-venice-gw73xx-0x
> >
> >
> > U-Boot 2021.07-00475-g1126252f40 (Jul 15 2021 - 11:09:02 -0700)
> >
> > CPU:   Freescale i.MX8MMQ rev1.0 1600 MHz (running at 1200 MHz)
> > CPU:   Industrial temperature grade (-40C to 105C) at 43C
> > Reset cause: POR
> > Model: Gateworks Venice GW73xx-0x i.MX8MM Development Kit
> > DRAM:  1 GiB
> > temp    : 38.3C
> > vdd_bat : 0.000V
> > vdd_vin : 15.731V
> > vdd_adc1: 0.000V
> > vdd_adc2: 0.000V
> > vdd_dram: 1.093V
> > vdd_1p2 : 1.193V
> > vdd_1p0 : 0.985V
> > vdd_2p5 : 2.470V
> > vdd_3p3 : 3.250V
> > vdd_0p95: 0.948V
> > vdd_1p8 : 1.799V
> > vdd_gsc : 3.262V
> > initcall sequence 000000007ffc4f58 failed at call 0000000040255910 (err=-2)
> > ### ERROR ### Please RESET the board ###
> >
> > Any ideas what this could be?
>
> I don't have much idea. What is the initcall that is failing? Can you
> check u-boot.map ? That might give a clue as to what is failing. I
> assume the DT is passed to U-Boot somehow from SPL?
>

Simon,

Thanks for the help!

The initcall addr doesn't match anything in u-boot.map (maybe
u-boot.map doesn't show what's in lib/binman.o?) but I was able to
track it down to initr_binman() failing due to
binman_init()->find_image_node(&binman->image)' returning -EINVAL.
This is because my imx8mm-venice-gw73xx-0x-uboot.dtsi doesn't have a
binman node (my CONFIG_DEFAULT_DEVICE_TREE did but not my actual
dtbs). So I have it working now!

> >
<snip>
> >
> > A follow-on question is that I would like to investigate using binman
> > in the SPL to dynamically access the IMX8M ddr training blobs so that
> > we don't have to waste padding space taking them onto the end of the
> > SPL which is currently done. The lpddr4 training blobs I'm using
> > currently take up 57k without padding compared to 81k with padding.
> > The location of them is handled in ddr_load_train_firmware.
> >
> > If I add the following to my SPL:
> > diff --git a/board/gateworks/venice/spl.c b/board/gateworks/venice/spl.c
> > index d0a490b0e6..62eb67fa5e 100644
> > --- a/board/gateworks/venice/spl.c
> > +++ b/board/gateworks/venice/spl.c
> > @@ -3,6 +3,7 @@
> >   * Copyright 2021 Gateworks Corporation
> >   */
> >
> > +#include <binman_sym.h>
> >  #include <common.h>
> >  #include <cpu_func.h>
> >  #include <hang.h>
> > @@ -252,6 +253,8 @@ static int power_init_board(void)
> >         return 0;
> >  }
> >
> > +binman_sym_declare(ulong, blob_1, image_pos);
> > +
> >  void board_init_f(ulong dummy)
> >  {
> >         struct udevice *dev;
> > @@ -291,6 +294,8 @@ void board_init_f(ulong dummy)
> >         gpio_request(PCIE_RSTN, "perst#");
> >         gpio_direction_output(PCIE_RSTN, 0);
> >
> > +       printf("%s: blob_1:0x%0lx\n", __func__, binman_sym(ulong,
> > blob_1, image_pos));
> > +
> >         /* GSC */
> >         dram_sz = gsc_init(0);
> >
> > I get 'blob_1:0x0' which is not what I expected.
> >
> > If I understand correctly binman is using linker symbols to determine
> > where things are in the image? What I don't quite understand is what
> > symbols are valid to use assuming my dtsi above. The binman.rst docs
> > talk use 'u_boot_any' as an example which apparently can match
> > 'u-boot.bin', 'u-boot.img', and 'u-boot-nodtb.bin' but I can't find
> > the code that somehow translates this meaning.
>
> Actually any symbol can be used. It basically depends on the name of
> the entry in your image description. So here it would be
> blob-ext@1...I think that translates to blob_ext_1 but I'm not sure
> about the @. You could try blob-ext-1 instead. It does not know about
> phandles or labels.
>
> If you pass BINMAN_VERBOSE=4 to the build you should see it talking
> about writing symbols into the SPL image.
>

For the following:
         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-ext@1 {
                        filename = "lpddr4_pmu_train_1d_imem.bin";
                        size = <0x8000>;
                };

                blob-ext@2 {
                        filename = "lpddr4_pmu_train_1d_dmem.bin";
                        size = <0x4000>;
                };

                blob-ext@3 {
                        filename = "lpddr4_pmu_train_2d_imem.bin";
                        size = <0x8000>;
                };

                blob-ext@4 {
                        filename = "lpddr4_pmu_train_2d_dmem.bin";
                        size = <0x4000>;
                };
        };

I tried 'blob_ext_1' and 'blob_ext1' and both formats resolve to 0x0.
The 'ext-blob' is an entry type supported by binman so if I had
multiple they must be called blob-ext@1, blob-ext@2, ... right?

The entry_name used in binman_sym_declare/binman_sym certainly can't
support non C varname characters so '-' and '@' characters must get
translated somewhere. Where would that be done in order to figure out
what to use?

BINMAN_VERBOSE=4 indeed prints out a tone of stuff but I'm not seeing
anything for 'blob' below that would seem to indicate one node name vs
another:
  BINMAN  flash.bin
Node '/binman/u-boot-spl-ddr/u-boot-spl': etype 'u-boot-spl':
u-boot-spl-expanded selected
Node '/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-nodtb': Packing:
offset=None, size=None, content_size=215d0
Node '/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-nodtb':    -
packed: offset=0x0, size=0x215d0, content_size=0x215d0,
next_offset=215d0
Node '/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-dtb': Packing:
offset=None, size=None, content_size=131c
Node '/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-dtb':    - packed:
offset=0x215d0, size=0x131c, content_size=0x131c, next_offset=228ec
Node '/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-nodtb': GetData: size 0x215d0
Node '/binman/u-boot-spl-ddr/u-boot-spl': GetPaddedDataForEntry: size None
Node '/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-dtb': GetData: size 0x131c
Node '/binman/u-boot-spl-ddr/u-boot-spl': GetPaddedDataForEntry: size None
Node '/binman/u-boot-spl-ddr/u-boot-spl': GetData: 2 entries, total size 0x228ec
 Node '/binman/u-boot-spl-ddr': GetPaddedDataForEntry: size None
Node '/binman/u-boot-spl-ddr/u-boot-spl': Packing: offset=None,
size=0x228ec, content_size=228ec
Node '/binman/u-boot-spl-ddr/u-boot-spl':    - packed: offset=0x0,
size=0x228ec, content_size=0x228ec, next_offset=228ec
Node '/binman/u-boot-spl-ddr/blob-ext@1': Packing: offset=None,
size=0x8000, content_size=7df4
Node '/binman/u-boot-spl-ddr/blob-ext@1':    - packed: offset=0x228ec,
size=0x8000, content_size=0x7df4, next_offset=2a8ec
Node '/binman/u-boot-spl-ddr/blob-ext@2': Packing: offset=None,
size=0x4000, content_size=684
Node '/binman/u-boot-spl-ddr/blob-ext@2':    - packed: offset=0x2a8ec,
size=0x4000, content_size=0x684, next_offset=2e8ec
Node '/binman/u-boot-spl-ddr/blob-ext@3': Packing: offset=None,
size=0x8000, content_size=5ac0
Node '/binman/u-boot-spl-ddr/blob-ext@3':    - packed: offset=0x2e8ec,
size=0x8000, content_size=0x5ac0, next_offset=368ec
Node '/binman/u-boot-spl-ddr/blob-ext@4': Packing: offset=None,
size=0x4000, content_size=564
Node '/binman/u-boot-spl-ddr/blob-ext@4':    - packed: offset=0x368ec,
size=0x4000, content_size=0x564, next_offset=3a8ec
Node '/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-nodtb': GetData: size 0x215d0
Node '/binman/u-boot-spl-ddr/u-boot-spl': GetPaddedDataForEntry: size 0x228ec
Node '/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-dtb': GetData: size 0x131c
Node '/binman/u-boot-spl-ddr/u-boot-spl': GetPaddedDataForEntry: size 0x228ec
Node '/binman/u-boot-spl-ddr/u-boot-spl': GetData: 2 entries, total size 0x228ec
 Node '/binman/u-boot-spl-ddr': GetPaddedDataForEntry: size None
Node '/binman/u-boot-spl-ddr/blob-ext@1': GetData: size 0x7df4
 Node '/binman/u-boot-spl-ddr': GetPaddedDataForEntry: size None
Node '/binman/u-boot-spl-ddr/blob-ext@2': GetData: size 0x684
 Node '/binman/u-boot-spl-ddr': GetPaddedDataForEntry: size None
Node '/binman/u-boot-spl-ddr/blob-ext@3': GetData: size 0x5ac0
 Node '/binman/u-boot-spl-ddr': GetPaddedDataForEntry: size None
Node '/binman/u-boot-spl-ddr/blob-ext@4': GetData: size 0x564
 Node '/binman/u-boot-spl-ddr': GetPaddedDataForEntry: size None
 Node '/binman/u-boot-spl-ddr': GetData: 5 entries, total size 0x3a8ec
 Node '/binman/u-boot-spl-ddr': GetPaddedDataForEntry: size 0x3a8ec
 Node '/binman/u-boot-spl-ddr': Packing: offset=None, size=0x3a8ec,
content_size=3a8ec
 Node '/binman/u-boot-spl-ddr':    - packed: offset=0x0, size=0x3a8ec,
content_size=0x3a8ec, next_offset=3a8ec
File ./u-boot.dtb.out: Update node '/binman/u-boot-spl-ddr' prop 'offset' to 0x0
File ./u-boot.dtb.out: Update node '/binman/u-boot-spl-ddr' prop
'size' to 0x3a8ec
File ./u-boot.dtb.out: Update node '/binman/u-boot-spl-ddr' prop
'image-pos' to 0x0
File ./u-boot.dtb.out: Update node '/binman/u-boot-spl-ddr/u-boot-spl'
prop 'offset' to 0x0
File ./u-boot.dtb.out: Update node '/binman/u-boot-spl-ddr/u-boot-spl'
prop 'size' to 0x228ec
File ./u-boot.dtb.out: Update node '/binman/u-boot-spl-ddr/u-boot-spl'
prop 'image-pos' to 0x0
File ./u-boot.dtb.out: Update node
'/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-nodtb' prop 'offset' to
0x0
File ./u-boot.dtb.out: Update node
'/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-nodtb' prop 'size' to
0x215d0
File ./u-boot.dtb.out: Update node
'/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-nodtb' prop 'image-pos'
to 0x0
File ./u-boot.dtb.out: Update node
'/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-dtb' prop 'offset' to
0x215d0
File ./u-boot.dtb.out: Update node
'/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-dtb' prop 'size' to
0x131c
File ./u-boot.dtb.out: Update node
'/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-dtb' prop 'image-pos' to
0x215d0
File ./u-boot.dtb.out: Update node '/binman/u-boot-spl-ddr/blob-ext@1'
prop 'offset' to 0x228ec
File ./u-boot.dtb.out: Update node '/binman/u-boot-spl-ddr/blob-ext@1'
prop 'size' to 0x8000
File ./u-boot.dtb.out: Update node '/binman/u-boot-spl-ddr/blob-ext@1'
prop 'image-pos' to 0x228ec
File ./u-boot.dtb.out: Update node '/binman/u-boot-spl-ddr/blob-ext@2'
prop 'offset' to 0x2a8ec
File ./u-boot.dtb.out: Update node '/binman/u-boot-spl-ddr/blob-ext@2'
prop 'size' to 0x4000
File ./u-boot.dtb.out: Update node '/binman/u-boot-spl-ddr/blob-ext@2'
prop 'image-pos' to 0x2a8ec
File ./u-boot.dtb.out: Update node '/binman/u-boot-spl-ddr/blob-ext@3'
prop 'offset' to 0x2e8ec
File ./u-boot.dtb.out: Update node '/binman/u-boot-spl-ddr/blob-ext@3'
prop 'size' to 0x8000
File ./u-boot.dtb.out: Update node '/binman/u-boot-spl-ddr/blob-ext@3'
prop 'image-pos' to 0x2e8ec
File ./u-boot.dtb.out: Update node '/binman/u-boot-spl-ddr/blob-ext@4'
prop 'offset' to 0x368ec
File ./u-boot.dtb.out: Update node '/binman/u-boot-spl-ddr/blob-ext@4'
prop 'size' to 0x4000
File ./u-boot.dtb.out: Update node '/binman/u-boot-spl-ddr/blob-ext@4'
prop 'image-pos' to 0x368ec
Pack completed after 1 pass(es)
Writing image to './u-boot-spl-ddr.bin'
Node '/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-nodtb': GetData: size 0x215d0
Node '/binman/u-boot-spl-ddr/u-boot-spl': GetPaddedDataForEntry: size 0x228ec
Node '/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-dtb': GetData: size 0x131c
Node '/binman/u-boot-spl-ddr/u-boot-spl': GetPaddedDataForEntry: size 0x228ec
Node '/binman/u-boot-spl-ddr/u-boot-spl': GetData: 2 entries, total size 0x228ec
 Node '/binman/u-boot-spl-ddr': GetPaddedDataForEntry: size 0x3a8ec
Node '/binman/u-boot-spl-ddr/blob-ext@1': GetData: size 0x7df4
 Node '/binman/u-boot-spl-ddr': GetPaddedDataForEntry: size 0x3a8ec
Node '/binman/u-boot-spl-ddr/blob-ext@2': GetData: size 0x684
 Node '/binman/u-boot-spl-ddr': GetPaddedDataForEntry: size 0x3a8ec
Node '/binman/u-boot-spl-ddr/blob-ext@3': GetData: size 0x5ac0
 Node '/binman/u-boot-spl-ddr': GetPaddedDataForEntry: size 0x3a8ec
Node '/binman/u-boot-spl-ddr/blob-ext@4': GetData: size 0x564
 Node '/binman/u-boot-spl-ddr': GetPaddedDataForEntry: size 0x3a8ec
 Node '/binman/u-boot-spl-ddr': GetData: 5 entries, total size 0x3a8ec
 Node '/binman/u-boot-spl-ddr': GetPaddedDataForEntry: size 0x3a8ec
Wrote 0x3a8ec bytes
Node '/binman/flash/mkimage/blob': GetData: size 0x3a8ec
  Node '/binman/flash/mkimage': Packing: offset=None, size=None,
content_size=3ca00
  Node '/binman/flash/mkimage':    - packed: offset=0x0, size=0x3ca00,
content_size=0x3ca00, next_offset=3ca00
  Node '/binman/flash/mkimage': GetData: size 0x3ca00
          Node '/binman/flash': GetPaddedDataForEntry: size None
          Node '/binman/flash': GetData: 1 entries, total size 0x3ca00
          Node '/binman/flash': GetPaddedDataForEntry: size 0x3ca00
          Node '/binman/flash': Packing: offset=None, size=0x3ca00,
content_size=3ca00
          Node '/binman/flash':    - packed: offset=0x0, size=0x3ca00,
content_size=0x3ca00, next_offset=3ca00
File ./u-boot.dtb.out: Update node '/binman/flash' prop 'offset' to 0x0
File ./u-boot.dtb.out: Update node '/binman/flash' prop 'size' to 0x3ca00
File ./u-boot.dtb.out: Update node '/binman/flash' prop 'image-pos' to 0x0
File ./u-boot.dtb.out: Update node '/binman/flash/mkimage' prop 'offset' to 0x0
File ./u-boot.dtb.out: Update node '/binman/flash/mkimage' prop 'size'
to 0x3ca00
File ./u-boot.dtb.out: Update node '/binman/flash/mkimage' prop
'image-pos' to 0x0
Pack completed after 1 pass(es)
Writing image to './flash.bin'
  Node '/binman/flash/mkimage': GetData: size 0x3ca00
          Node '/binman/flash': GetPaddedDataForEntry: size 0x3ca00
          Node '/binman/flash': GetData: 1 entries, total size 0x3ca00
          Node '/binman/flash': GetPaddedDataForEntry: size 0x3ca00
Wrote 0x3ca00 bytes
Node '/binman/itb/fit/images/uboot/blob-ext': Packing: offset=None,
size=None, content_size=aa978
Node '/binman/itb/fit/images/uboot/blob-ext':    - packed: offset=0x0,
size=0xaa978, content_size=0xaa978, next_offset=aa978
Node '/binman/itb/fit/images/uboot/blob-ext': GetData: size 0xaa978
Node '/binman/itb/fit/images/uboot': GetPaddedDataForEntry: size None
Node '/binman/itb/fit/images/uboot': GetData: 1 entries, total size 0xaa978
            Node '/binman/itb': GetPaddedDataForEntry: size None
Node '/binman/itb/fit/images/uboot': Packing: offset=None,
size=0xaa978, content_size=aa978
Node '/binman/itb/fit/images/uboot':    - packed: offset=0x0,
size=0xaa978, content_size=0xaa978, next_offset=aa978
Node '/binman/itb/fit/images/uboot/blob-ext': GetData: size 0xaa978
Node '/binman/itb/fit/images/uboot': GetPaddedDataForEntry: size 0xaa978
Node '/binman/itb/fit/images/uboot': GetData: 1 entries, total size 0xaa978
Node '/binman/itb/fit/images/atf/blob-ext': Packing: offset=None,
size=None, content_size=9159
Node '/binman/itb/fit/images/atf/blob-ext':    - packed: offset=0x0,
size=0x9159, content_size=0x9159, next_offset=9159
Node '/binman/itb/fit/images/atf/blob-ext': GetData: size 0x9159
Node '/binman/itb/fit/images/atf': GetPaddedDataForEntry: size None
Node '/binman/itb/fit/images/atf': GetData: 1 entries, total size 0x9159
            Node '/binman/itb': GetPaddedDataForEntry: size None
Node '/binman/itb/fit/images/atf': Packing: offset=None, size=0x9159,
content_size=9159
Node '/binman/itb/fit/images/atf':    - packed: offset=0x0,
size=0x9159, content_size=0x9159, next_offset=9159
Node '/binman/itb/fit/images/atf/blob-ext': GetData: size 0x9159
Node '/binman/itb/fit/images/atf': GetPaddedDataForEntry: size 0x9159
Node '/binman/itb/fit/images/atf': GetData: 1 entries, total size 0x9159
        Node '/binman/itb/fit': Packing: offset=None, size=None,
content_size=e5f64
        Node '/binman/itb/fit':    - packed: offset=0x0, size=0xe5f64,
content_size=0xe5f64, next_offset=e5f64
        Node '/binman/itb/fit': GetData: size 0xe5f64
            Node '/binman/itb': GetPaddedDataForEntry: size None
            Node '/binman/itb': GetData: 1 entries, total size 0xe5f64
            Node '/binman/itb': GetPaddedDataForEntry: size 0xe5f64
            Node '/binman/itb': Packing: offset=None, size=0xe5f64,
content_size=e5f64
            Node '/binman/itb':    - packed: offset=0x0, size=0xe5f64,
content_size=0xe5f64, next_offset=e5f64
File ./u-boot.dtb.out: Update node '/binman/itb' prop 'offset' to 0x0
File ./u-boot.dtb.out: Update node '/binman/itb' prop 'size' to 0xe5f64
File ./u-boot.dtb.out: Update node '/binman/itb' prop 'image-pos' to 0x0
File ./u-boot.dtb.out: Update node '/binman/itb/fit' prop 'offset' to 0x0
File ./u-boot.dtb.out: Update node '/binman/itb/fit' prop 'size' to 0xe5f64
File ./u-boot.dtb.out: Update node '/binman/itb/fit' prop 'image-pos' to 0x0
Pack completed after 1 pass(es)

Regards,

Tim

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: using binman fails boot
  2021-07-16 21:43   ` Tim Harvey
@ 2021-07-16 22:11     ` Simon Glass
  2021-07-16 23:15       ` Tim Harvey
  0 siblings, 1 reply; 14+ messages in thread
From: Simon Glass @ 2021-07-16 22:11 UTC (permalink / raw)
  To: Tim Harvey; +Cc: u-boot

() which has
Hi Tim,

On Fri, 16 Jul 2021 at 15:43, Tim Harvey <tharvey@gateworks.com> wrote:
>
> On Thu, Jul 15, 2021 at 9:30 PM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Tim,
> >
> > On Thu, 15 Jul 2021 at 16:58, Tim Harvey <tharvey@gateworks.com> wrote:
> > >
> > > Greetings,
> > >
> > > I'm taking a look at moving imx8mm-venice to use binman for packaging.
> > > After doing so U-Boot proper fails to boot:
> > >
> > > U-Boot SPL 2021.07-00475-g1126252f40 (Jul 15 2021 - 11:09:02 -0700)
> > > GSC     : v58 0xf098 RST:VIN Thermal Protection Disabled
> > > Model   : GW7300-00-B1B
> > > Serial  : 852420
> > > MFGDate : 10-26-2020
> > > RTC     : 122
> > > PMIC    : MP5416
> > > DRAM    : LPDDR4 1 GiB
> > > WDT:   Not starting
> > > Trying to boot from MMC1
> > > DTB     : imx8mm-venice-gw73xx-0x
> > >
> > >
> > > U-Boot 2021.07-00475-g1126252f40 (Jul 15 2021 - 11:09:02 -0700)
> > >
> > > CPU:   Freescale i.MX8MMQ rev1.0 1600 MHz (running at 1200 MHz)
> > > CPU:   Industrial temperature grade (-40C to 105C) at 43C
> > > Reset cause: POR
> > > Model: Gateworks Venice GW73xx-0x i.MX8MM Development Kit
> > > DRAM:  1 GiB
> > > temp    : 38.3C
> > > vdd_bat : 0.000V
> > > vdd_vin : 15.731V
> > > vdd_adc1: 0.000V
> > > vdd_adc2: 0.000V
> > > vdd_dram: 1.093V
> > > vdd_1p2 : 1.193V
> > > vdd_1p0 : 0.985V
> > > vdd_2p5 : 2.470V
> > > vdd_3p3 : 3.250V
> > > vdd_0p95: 0.948V
> > > vdd_1p8 : 1.799V
> > > vdd_gsc : 3.262V
> > > initcall sequence 000000007ffc4f58 failed at call 0000000040255910 (err=-2)
> > > ### ERROR ### Please RESET the board ###
> > >
> > > Any ideas what this could be?
> >
> > I don't have much idea. What is the initcall that is failing? Can you
> > check u-boot.map ? That might give a clue as to what is failing. I
> > assume the DT is passed to U-Boot somehow from SPL?
> >
>
> Simon,
>
> Thanks for the help!
>
> The initcall addr doesn't match anything in u-boot.map (maybe
> u-boot.map doesn't show what's in lib/binman.o?) but I was able to

It certainly should show up, but if you have CONFIG_LTO enabled lots
of functions disappear. Still if you get an initcall address I would
expect a function to be present. Make sure you use the unallocated
address.

> track it down to initr_binman() failing due to
> binman_init()->find_image_node(&binman->image)' returning -EINVAL.
> This is because my imx8mm-venice-gw73xx-0x-uboot.dtsi doesn't have a
> binman node (my CONFIG_DEFAULT_DEVICE_TREE did but not my actual
> dtbs). So I have it working now!

OK good progress! Perhaps we should put an error message in initr_binman() ?

>
> > >
> <snip>
> > >
> > > A follow-on question is that I would like to investigate using binman
> > > in the SPL to dynamically access the IMX8M ddr training blobs so that
> > > we don't have to waste padding space taking them onto the end of the
> > > SPL which is currently done. The lpddr4 training blobs I'm using
> > > currently take up 57k without padding compared to 81k with padding.
> > > The location of them is handled in ddr_load_train_firmware.
> > >
> > > If I add the following to my SPL:
> > > diff --git a/board/gateworks/venice/spl.c b/board/gateworks/venice/spl.c
> > > index d0a490b0e6..62eb67fa5e 100644
> > > --- a/board/gateworks/venice/spl.c
> > > +++ b/board/gateworks/venice/spl.c
> > > @@ -3,6 +3,7 @@
> > >   * Copyright 2021 Gateworks Corporation
> > >   */
> > >
> > > +#include <binman_sym.h>
> > >  #include <common.h>
> > >  #include <cpu_func.h>
> > >  #include <hang.h>
> > > @@ -252,6 +253,8 @@ static int power_init_board(void)
> > >         return 0;
> > >  }
> > >
> > > +binman_sym_declare(ulong, blob_1, image_pos);
> > > +
> > >  void board_init_f(ulong dummy)
> > >  {
> > >         struct udevice *dev;
> > > @@ -291,6 +294,8 @@ void board_init_f(ulong dummy)
> > >         gpio_request(PCIE_RSTN, "perst#");
> > >         gpio_direction_output(PCIE_RSTN, 0);
> > >
> > > +       printf("%s: blob_1:0x%0lx\n", __func__, binman_sym(ulong,
> > > blob_1, image_pos));
> > > +
> > >         /* GSC */
> > >         dram_sz = gsc_init(0);
> > >
> > > I get 'blob_1:0x0' which is not what I expected.
> > >
> > > If I understand correctly binman is using linker symbols to determine
> > > where things are in the image? What I don't quite understand is what
> > > symbols are valid to use assuming my dtsi above. The binman.rst docs
> > > talk use 'u_boot_any' as an example which apparently can match
> > > 'u-boot.bin', 'u-boot.img', and 'u-boot-nodtb.bin' but I can't find
> > > the code that somehow translates this meaning.
> >
> > Actually any symbol can be used. It basically depends on the name of
> > the entry in your image description. So here it would be
> > blob-ext@1...I think that translates to blob_ext_1 but I'm not sure
> > about the @. You could try blob-ext-1 instead. It does not know about
> > phandles or labels.
> >
> > If you pass BINMAN_VERBOSE=4 to the build you should see it talking
> > about writing symbols into the SPL image.
> >
>
> For the following:
>          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-ext@1 {
>                         filename = "lpddr4_pmu_train_1d_imem.bin";
>                         size = <0x8000>;
>                 };
>
>                 blob-ext@2 {
>                         filename = "lpddr4_pmu_train_1d_dmem.bin";
>                         size = <0x4000>;
>                 };
>
>                 blob-ext@3 {
>                         filename = "lpddr4_pmu_train_2d_imem.bin";
>                         size = <0x8000>;
>                 };
>
>                 blob-ext@4 {
>                         filename = "lpddr4_pmu_train_2d_dmem.bin";
>                         size = <0x4000>;
>                 };
>         };
>
> I tried 'blob_ext_1' and 'blob_ext1' and both formats resolve to 0x0.
> The 'ext-blob' is an entry type supported by binman so if I had
> multiple they must be called blob-ext@1, blob-ext@2, ... right?
>
> The entry_name used in binman_sym_declare/binman_sym certainly can't
> support non C varname characters so '-' and '@' characters must get
> translated somewhere. Where would that be done in order to figure out
> what to use?

If you want to look at the internals, see section.py LookupSymbol().

It takes the ELF symbol and replaces _ by - but does not (cannot)
replace _ with @. So I think you'll have to use - instead of @

I suppose we could do the search in the other direction (take the
entry and try to find the symbol that matches it), but I'd need to
think about it. A simple translation is easier.

In this case binman should really give an error for your chosen entry
name (blob-ext@4) but it doesn't know you are using it as a symbol. I
think it should complain about this (see the Warning in section.py
LookupSymbol()) but apparently it does not in your case.

If you can push your tree somewhere (with this problem) I'll see if I
can figure out why.

>
> BINMAN_VERBOSE=4 indeed prints out a tone of stuff but I'm not seeing
> anything for 'blob' below that would seem to indicate one node name vs
> another:

Oops you need BINMAN_VERBOSE=5 - see elf.py LookupAndWriteSymbols()
which has tout.Debug() which is level 5.

>   BINMAN  flash.bin
> Node '/binman/u-boot-spl-ddr/u-boot-spl': etype 'u-boot-spl':
> u-boot-spl-expanded selected
> Node '/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-nodtb': Packing:
> offset=None, size=None, content_size=215d0
> Node '/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-nodtb':    -
> packed: offset=0x0, size=0x215d0, content_size=0x215d0,
> next_offset=215d0
> Node '/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-dtb': Packing:
> offset=None, size=None, content_size=131c
> Node '/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-dtb':    - packed:
> offset=0x215d0, size=0x131c, content_size=0x131c, next_offset=228ec
> Node '/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-nodtb': GetData: size 0x215d0
> Node '/binman/u-boot-spl-ddr/u-boot-spl': GetPaddedDataForEntry: size None
> Node '/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-dtb': GetData: size 0x131c
> Node '/binman/u-boot-spl-ddr/u-boot-spl': GetPaddedDataForEntry: size None
> Node '/binman/u-boot-spl-ddr/u-boot-spl': GetData: 2 entries, total size 0x228ec
>  Node '/binman/u-boot-spl-ddr': GetPaddedDataForEntry: size None
> Node '/binman/u-boot-spl-ddr/u-boot-spl': Packing: offset=None,
> size=0x228ec, content_size=228ec
> Node '/binman/u-boot-spl-ddr/u-boot-spl':    - packed: offset=0x0,
> size=0x228ec, content_size=0x228ec, next_offset=228ec
> Node '/binman/u-boot-spl-ddr/blob-ext@1': Packing: offset=None,
> size=0x8000, content_size=7df4
> Node '/binman/u-boot-spl-ddr/blob-ext@1':    - packed: offset=0x228ec,
> size=0x8000, content_size=0x7df4, next_offset=2a8ec
> Node '/binman/u-boot-spl-ddr/blob-ext@2': Packing: offset=None,
> size=0x4000, content_size=684
> Node '/binman/u-boot-spl-ddr/blob-ext@2':    - packed: offset=0x2a8ec,
> size=0x4000, content_size=0x684, next_offset=2e8ec
> Node '/binman/u-boot-spl-ddr/blob-ext@3': Packing: offset=None,
> size=0x8000, content_size=5ac0
> Node '/binman/u-boot-spl-ddr/blob-ext@3':    - packed: offset=0x2e8ec,
> size=0x8000, content_size=0x5ac0, next_offset=368ec
> Node '/binman/u-boot-spl-ddr/blob-ext@4': Packing: offset=None,
> size=0x4000, content_size=564
> Node '/binman/u-boot-spl-ddr/blob-ext@4':    - packed: offset=0x368ec,
> size=0x4000, content_size=0x564, next_offset=3a8ec
> Node '/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-nodtb': GetData: size 0x215d0
> Node '/binman/u-boot-spl-ddr/u-boot-spl': GetPaddedDataForEntry: size 0x228ec
> Node '/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-dtb': GetData: size 0x131c
> Node '/binman/u-boot-spl-ddr/u-boot-spl': GetPaddedDataForEntry: size 0x228ec
> Node '/binman/u-boot-spl-ddr/u-boot-spl': GetData: 2 entries, total size 0x228ec
>  Node '/binman/u-boot-spl-ddr': GetPaddedDataForEntry: size None
> Node '/binman/u-boot-spl-ddr/blob-ext@1': GetData: size 0x7df4
>  Node '/binman/u-boot-spl-ddr': GetPaddedDataForEntry: size None
> Node '/binman/u-boot-spl-ddr/blob-ext@2': GetData: size 0x684
>  Node '/binman/u-boot-spl-ddr': GetPaddedDataForEntry: size None
> Node '/binman/u-boot-spl-ddr/blob-ext@3': GetData: size 0x5ac0
>  Node '/binman/u-boot-spl-ddr': GetPaddedDataForEntry: size None
> Node '/binman/u-boot-spl-ddr/blob-ext@4': GetData: size 0x564
>  Node '/binman/u-boot-spl-ddr': GetPaddedDataForEntry: size None
>  Node '/binman/u-boot-spl-ddr': GetData: 5 entries, total size 0x3a8ec
>  Node '/binman/u-boot-spl-ddr': GetPaddedDataForEntry: size 0x3a8ec
>  Node '/binman/u-boot-spl-ddr': Packing: offset=None, size=0x3a8ec,
> content_size=3a8ec
>  Node '/binman/u-boot-spl-ddr':    - packed: offset=0x0, size=0x3a8ec,
> content_size=0x3a8ec, next_offset=3a8ec
> File ./u-boot.dtb.out: Update node '/binman/u-boot-spl-ddr' prop 'offset' to 0x0
> File ./u-boot.dtb.out: Update node '/binman/u-boot-spl-ddr' prop
> 'size' to 0x3a8ec
> File ./u-boot.dtb.out: Update node '/binman/u-boot-spl-ddr' prop
> 'image-pos' to 0x0
> File ./u-boot.dtb.out: Update node '/binman/u-boot-spl-ddr/u-boot-spl'
> prop 'offset' to 0x0
> File ./u-boot.dtb.out: Update node '/binman/u-boot-spl-ddr/u-boot-spl'
> prop 'size' to 0x228ec
> File ./u-boot.dtb.out: Update node '/binman/u-boot-spl-ddr/u-boot-spl'
> prop 'image-pos' to 0x0
> File ./u-boot.dtb.out: Update node
> '/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-nodtb' prop 'offset' to
> 0x0
> File ./u-boot.dtb.out: Update node
> '/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-nodtb' prop 'size' to
> 0x215d0
> File ./u-boot.dtb.out: Update node
> '/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-nodtb' prop 'image-pos'
> to 0x0
> File ./u-boot.dtb.out: Update node
> '/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-dtb' prop 'offset' to
> 0x215d0
> File ./u-boot.dtb.out: Update node
> '/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-dtb' prop 'size' to
> 0x131c
> File ./u-boot.dtb.out: Update node
> '/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-dtb' prop 'image-pos' to
> 0x215d0
> File ./u-boot.dtb.out: Update node '/binman/u-boot-spl-ddr/blob-ext@1'
> prop 'offset' to 0x228ec
> File ./u-boot.dtb.out: Update node '/binman/u-boot-spl-ddr/blob-ext@1'
> prop 'size' to 0x8000
> File ./u-boot.dtb.out: Update node '/binman/u-boot-spl-ddr/blob-ext@1'
> prop 'image-pos' to 0x228ec
> File ./u-boot.dtb.out: Update node '/binman/u-boot-spl-ddr/blob-ext@2'
> prop 'offset' to 0x2a8ec
> File ./u-boot.dtb.out: Update node '/binman/u-boot-spl-ddr/blob-ext@2'
> prop 'size' to 0x4000
> File ./u-boot.dtb.out: Update node '/binman/u-boot-spl-ddr/blob-ext@2'
> prop 'image-pos' to 0x2a8ec
> File ./u-boot.dtb.out: Update node '/binman/u-boot-spl-ddr/blob-ext@3'
> prop 'offset' to 0x2e8ec
> File ./u-boot.dtb.out: Update node '/binman/u-boot-spl-ddr/blob-ext@3'
> prop 'size' to 0x8000
> File ./u-boot.dtb.out: Update node '/binman/u-boot-spl-ddr/blob-ext@3'
> prop 'image-pos' to 0x2e8ec
> File ./u-boot.dtb.out: Update node '/binman/u-boot-spl-ddr/blob-ext@4'
> prop 'offset' to 0x368ec
> File ./u-boot.dtb.out: Update node '/binman/u-boot-spl-ddr/blob-ext@4'
> prop 'size' to 0x4000
> File ./u-boot.dtb.out: Update node '/binman/u-boot-spl-ddr/blob-ext@4'
> prop 'image-pos' to 0x368ec
> Pack completed after 1 pass(es)
> Writing image to './u-boot-spl-ddr.bin'
> Node '/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-nodtb': GetData: size 0x215d0
> Node '/binman/u-boot-spl-ddr/u-boot-spl': GetPaddedDataForEntry: size 0x228ec
> Node '/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-dtb': GetData: size 0x131c
> Node '/binman/u-boot-spl-ddr/u-boot-spl': GetPaddedDataForEntry: size 0x228ec
> Node '/binman/u-boot-spl-ddr/u-boot-spl': GetData: 2 entries, total size 0x228ec
>  Node '/binman/u-boot-spl-ddr': GetPaddedDataForEntry: size 0x3a8ec
> Node '/binman/u-boot-spl-ddr/blob-ext@1': GetData: size 0x7df4
>  Node '/binman/u-boot-spl-ddr': GetPaddedDataForEntry: size 0x3a8ec
> Node '/binman/u-boot-spl-ddr/blob-ext@2': GetData: size 0x684
>  Node '/binman/u-boot-spl-ddr': GetPaddedDataForEntry: size 0x3a8ec
> Node '/binman/u-boot-spl-ddr/blob-ext@3': GetData: size 0x5ac0
>  Node '/binman/u-boot-spl-ddr': GetPaddedDataForEntry: size 0x3a8ec
> Node '/binman/u-boot-spl-ddr/blob-ext@4': GetData: size 0x564
>  Node '/binman/u-boot-spl-ddr': GetPaddedDataForEntry: size 0x3a8ec
>  Node '/binman/u-boot-spl-ddr': GetData: 5 entries, total size 0x3a8ec
>  Node '/binman/u-boot-spl-ddr': GetPaddedDataForEntry: size 0x3a8ec
> Wrote 0x3a8ec bytes
> Node '/binman/flash/mkimage/blob': GetData: size 0x3a8ec
>   Node '/binman/flash/mkimage': Packing: offset=None, size=None,
> content_size=3ca00
>   Node '/binman/flash/mkimage':    - packed: offset=0x0, size=0x3ca00,
> content_size=0x3ca00, next_offset=3ca00
>   Node '/binman/flash/mkimage': GetData: size 0x3ca00
>           Node '/binman/flash': GetPaddedDataForEntry: size None
>           Node '/binman/flash': GetData: 1 entries, total size 0x3ca00
>           Node '/binman/flash': GetPaddedDataForEntry: size 0x3ca00
>           Node '/binman/flash': Packing: offset=None, size=0x3ca00,
> content_size=3ca00
>           Node '/binman/flash':    - packed: offset=0x0, size=0x3ca00,
> content_size=0x3ca00, next_offset=3ca00
> File ./u-boot.dtb.out: Update node '/binman/flash' prop 'offset' to 0x0
> File ./u-boot.dtb.out: Update node '/binman/flash' prop 'size' to 0x3ca00
> File ./u-boot.dtb.out: Update node '/binman/flash' prop 'image-pos' to 0x0
> File ./u-boot.dtb.out: Update node '/binman/flash/mkimage' prop 'offset' to 0x0
> File ./u-boot.dtb.out: Update node '/binman/flash/mkimage' prop 'size'
> to 0x3ca00
> File ./u-boot.dtb.out: Update node '/binman/flash/mkimage' prop
> 'image-pos' to 0x0
> Pack completed after 1 pass(es)
> Writing image to './flash.bin'
>   Node '/binman/flash/mkimage': GetData: size 0x3ca00
>           Node '/binman/flash': GetPaddedDataForEntry: size 0x3ca00
>           Node '/binman/flash': GetData: 1 entries, total size 0x3ca00
>           Node '/binman/flash': GetPaddedDataForEntry: size 0x3ca00
> Wrote 0x3ca00 bytes
> Node '/binman/itb/fit/images/uboot/blob-ext': Packing: offset=None,
> size=None, content_size=aa978
> Node '/binman/itb/fit/images/uboot/blob-ext':    - packed: offset=0x0,
> size=0xaa978, content_size=0xaa978, next_offset=aa978
> Node '/binman/itb/fit/images/uboot/blob-ext': GetData: size 0xaa978
> Node '/binman/itb/fit/images/uboot': GetPaddedDataForEntry: size None
> Node '/binman/itb/fit/images/uboot': GetData: 1 entries, total size 0xaa978
>             Node '/binman/itb': GetPaddedDataForEntry: size None
> Node '/binman/itb/fit/images/uboot': Packing: offset=None,
> size=0xaa978, content_size=aa978
> Node '/binman/itb/fit/images/uboot':    - packed: offset=0x0,
> size=0xaa978, content_size=0xaa978, next_offset=aa978
> Node '/binman/itb/fit/images/uboot/blob-ext': GetData: size 0xaa978
> Node '/binman/itb/fit/images/uboot': GetPaddedDataForEntry: size 0xaa978
> Node '/binman/itb/fit/images/uboot': GetData: 1 entries, total size 0xaa978
> Node '/binman/itb/fit/images/atf/blob-ext': Packing: offset=None,
> size=None, content_size=9159
> Node '/binman/itb/fit/images/atf/blob-ext':    - packed: offset=0x0,
> size=0x9159, content_size=0x9159, next_offset=9159
> Node '/binman/itb/fit/images/atf/blob-ext': GetData: size 0x9159
> Node '/binman/itb/fit/images/atf': GetPaddedDataForEntry: size None
> Node '/binman/itb/fit/images/atf': GetData: 1 entries, total size 0x9159
>             Node '/binman/itb': GetPaddedDataForEntry: size None
> Node '/binman/itb/fit/images/atf': Packing: offset=None, size=0x9159,
> content_size=9159
> Node '/binman/itb/fit/images/atf':    - packed: offset=0x0,
> size=0x9159, content_size=0x9159, next_offset=9159
> Node '/binman/itb/fit/images/atf/blob-ext': GetData: size 0x9159
> Node '/binman/itb/fit/images/atf': GetPaddedDataForEntry: size 0x9159
> Node '/binman/itb/fit/images/atf': GetData: 1 entries, total size 0x9159
>         Node '/binman/itb/fit': Packing: offset=None, size=None,
> content_size=e5f64
>         Node '/binman/itb/fit':    - packed: offset=0x0, size=0xe5f64,
> content_size=0xe5f64, next_offset=e5f64
>         Node '/binman/itb/fit': GetData: size 0xe5f64
>             Node '/binman/itb': GetPaddedDataForEntry: size None
>             Node '/binman/itb': GetData: 1 entries, total size 0xe5f64
>             Node '/binman/itb': GetPaddedDataForEntry: size 0xe5f64
>             Node '/binman/itb': Packing: offset=None, size=0xe5f64,
> content_size=e5f64
>             Node '/binman/itb':    - packed: offset=0x0, size=0xe5f64,
> content_size=0xe5f64, next_offset=e5f64
> File ./u-boot.dtb.out: Update node '/binman/itb' prop 'offset' to 0x0
> File ./u-boot.dtb.out: Update node '/binman/itb' prop 'size' to 0xe5f64
> File ./u-boot.dtb.out: Update node '/binman/itb' prop 'image-pos' to 0x0
> File ./u-boot.dtb.out: Update node '/binman/itb/fit' prop 'offset' to 0x0
> File ./u-boot.dtb.out: Update node '/binman/itb/fit' prop 'size' to 0xe5f64
> File ./u-boot.dtb.out: Update node '/binman/itb/fit' prop 'image-pos' to 0x0
> Pack completed after 1 pass(es)
>
> Regards,

Regards,
SImon

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: using binman fails boot
  2021-07-16 22:11     ` Simon Glass
@ 2021-07-16 23:15       ` Tim Harvey
  2021-07-18  2:22         ` Simon Glass
  0 siblings, 1 reply; 14+ messages in thread
From: Tim Harvey @ 2021-07-16 23:15 UTC (permalink / raw)
  To: Simon Glass; +Cc: u-boot

On Fri, Jul 16, 2021 at 3:11 PM Simon Glass <sjg@chromium.org> wrote:
>
> () which has
> Hi Tim,
>
> On Fri, 16 Jul 2021 at 15:43, Tim Harvey <tharvey@gateworks.com> wrote:
> >
> > On Thu, Jul 15, 2021 at 9:30 PM Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Tim,
> > >
> > > On Thu, 15 Jul 2021 at 16:58, Tim Harvey <tharvey@gateworks.com> wrote:
> > > >
> > > > Greetings,
> > > >
> > > > I'm taking a look at moving imx8mm-venice to use binman for packaging.
> > > > After doing so U-Boot proper fails to boot:
> > > >
> > > > U-Boot SPL 2021.07-00475-g1126252f40 (Jul 15 2021 - 11:09:02 -0700)
> > > > GSC     : v58 0xf098 RST:VIN Thermal Protection Disabled
> > > > Model   : GW7300-00-B1B
> > > > Serial  : 852420
> > > > MFGDate : 10-26-2020
> > > > RTC     : 122
> > > > PMIC    : MP5416
> > > > DRAM    : LPDDR4 1 GiB
> > > > WDT:   Not starting
> > > > Trying to boot from MMC1
> > > > DTB     : imx8mm-venice-gw73xx-0x
> > > >
> > > >
> > > > U-Boot 2021.07-00475-g1126252f40 (Jul 15 2021 - 11:09:02 -0700)
> > > >
> > > > CPU:   Freescale i.MX8MMQ rev1.0 1600 MHz (running at 1200 MHz)
> > > > CPU:   Industrial temperature grade (-40C to 105C) at 43C
> > > > Reset cause: POR
> > > > Model: Gateworks Venice GW73xx-0x i.MX8MM Development Kit
> > > > DRAM:  1 GiB
> > > > temp    : 38.3C
> > > > vdd_bat : 0.000V
> > > > vdd_vin : 15.731V
> > > > vdd_adc1: 0.000V
> > > > vdd_adc2: 0.000V
> > > > vdd_dram: 1.093V
> > > > vdd_1p2 : 1.193V
> > > > vdd_1p0 : 0.985V
> > > > vdd_2p5 : 2.470V
> > > > vdd_3p3 : 3.250V
> > > > vdd_0p95: 0.948V
> > > > vdd_1p8 : 1.799V
> > > > vdd_gsc : 3.262V
> > > > initcall sequence 000000007ffc4f58 failed at call 0000000040255910 (err=-2)
> > > > ### ERROR ### Please RESET the board ###
> > > >
> > > > Any ideas what this could be?
> > >
> > > I don't have much idea. What is the initcall that is failing? Can you
> > > check u-boot.map ? That might give a clue as to what is failing. I
> > > assume the DT is passed to U-Boot somehow from SPL?
> > >
> >
> > Simon,
> >
> > Thanks for the help!
> >
> > The initcall addr doesn't match anything in u-boot.map (maybe
> > u-boot.map doesn't show what's in lib/binman.o?) but I was able to
>
> It certainly should show up, but if you have CONFIG_LTO enabled lots
> of functions disappear. Still if you get an initcall address I would
> expect a function to be present. Make sure you use the unallocated
> address.

I'm not sure what you mean by 'Make sure you use the unallocated address'

>
> > track it down to initr_binman() failing due to
> > binman_init()->find_image_node(&binman->image)' returning -EINVAL.
> > This is because my imx8mm-venice-gw73xx-0x-uboot.dtsi doesn't have a
> > binman node (my CONFIG_DEFAULT_DEVICE_TREE did but not my actual
> > dtbs). So I have it working now!
>
> OK good progress! Perhaps we should put an error message in initr_binman() ?
>

sure - I just sent a patch

> >
> > > >
> > <snip>
> > > >
> > > > A follow-on question is that I would like to investigate using binman
> > > > in the SPL to dynamically access the IMX8M ddr training blobs so that
> > > > we don't have to waste padding space taking them onto the end of the
> > > > SPL which is currently done. The lpddr4 training blobs I'm using
> > > > currently take up 57k without padding compared to 81k with padding.
> > > > The location of them is handled in ddr_load_train_firmware.
> > > >
> > > > If I add the following to my SPL:
> > > > diff --git a/board/gateworks/venice/spl.c b/board/gateworks/venice/spl.c
> > > > index d0a490b0e6..62eb67fa5e 100644
> > > > --- a/board/gateworks/venice/spl.c
> > > > +++ b/board/gateworks/venice/spl.c
> > > > @@ -3,6 +3,7 @@
> > > >   * Copyright 2021 Gateworks Corporation
> > > >   */
> > > >
> > > > +#include <binman_sym.h>
> > > >  #include <common.h>
> > > >  #include <cpu_func.h>
> > > >  #include <hang.h>
> > > > @@ -252,6 +253,8 @@ static int power_init_board(void)
> > > >         return 0;
> > > >  }
> > > >
> > > > +binman_sym_declare(ulong, blob_1, image_pos);
> > > > +
> > > >  void board_init_f(ulong dummy)
> > > >  {
> > > >         struct udevice *dev;
> > > > @@ -291,6 +294,8 @@ void board_init_f(ulong dummy)
> > > >         gpio_request(PCIE_RSTN, "perst#");
> > > >         gpio_direction_output(PCIE_RSTN, 0);
> > > >
> > > > +       printf("%s: blob_1:0x%0lx\n", __func__, binman_sym(ulong,
> > > > blob_1, image_pos));
> > > > +
> > > >         /* GSC */
> > > >         dram_sz = gsc_init(0);
> > > >
> > > > I get 'blob_1:0x0' which is not what I expected.
> > > >
> > > > If I understand correctly binman is using linker symbols to determine
> > > > where things are in the image? What I don't quite understand is what
> > > > symbols are valid to use assuming my dtsi above. The binman.rst docs
> > > > talk use 'u_boot_any' as an example which apparently can match
> > > > 'u-boot.bin', 'u-boot.img', and 'u-boot-nodtb.bin' but I can't find
> > > > the code that somehow translates this meaning.
> > >
> > > Actually any symbol can be used. It basically depends on the name of
> > > the entry in your image description. So here it would be
> > > blob-ext@1...I think that translates to blob_ext_1 but I'm not sure
> > > about the @. You could try blob-ext-1 instead. It does not know about
> > > phandles or labels.
> > >
> > > If you pass BINMAN_VERBOSE=4 to the build you should see it talking
> > > about writing symbols into the SPL image.
> > >
> >
> > For the following:
> >          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-ext@1 {
> >                         filename = "lpddr4_pmu_train_1d_imem.bin";
> >                         size = <0x8000>;
> >                 };
> >
> >                 blob-ext@2 {
> >                         filename = "lpddr4_pmu_train_1d_dmem.bin";
> >                         size = <0x4000>;
> >                 };
> >
> >                 blob-ext@3 {
> >                         filename = "lpddr4_pmu_train_2d_imem.bin";
> >                         size = <0x8000>;
> >                 };
> >
> >                 blob-ext@4 {
> >                         filename = "lpddr4_pmu_train_2d_dmem.bin";
> >                         size = <0x4000>;
> >                 };
> >         };
> >
> > I tried 'blob_ext_1' and 'blob_ext1' and both formats resolve to 0x0.
> > The 'ext-blob' is an entry type supported by binman so if I had
> > multiple they must be called blob-ext@1, blob-ext@2, ... right?
> >
> > The entry_name used in binman_sym_declare/binman_sym certainly can't
> > support non C varname characters so '-' and '@' characters must get
> > translated somewhere. Where would that be done in order to figure out
> > what to use?
>
> If you want to look at the internals, see section.py LookupSymbol().
>
> It takes the ELF symbol and replaces _ by - but does not (cannot)
> replace _ with @. So I think you'll have to use - instead of @
>
> I suppose we could do the search in the other direction (take the
> entry and try to find the symbol that matches it), but I'd need to
> think about it. A simple translation is easier.
>
> In this case binman should really give an error for your chosen entry
> name (blob-ext@4) but it doesn't know you are using it as a symbol. I
> think it should complain about this (see the Warning in section.py
> LookupSymbol()) but apparently it does not in your case.

But isn't blob-ext@4 a correct name? I can't use 'blob-ext-4' as
that's an unknown entry type.

>
> If you can push your tree somewhere (with this problem) I'll see if I
> can figure out why.
>

Sure, I pushed it to
https://github.com/Gateworks/uboot-venice/tree/WIP-venice-binman
make imx8mm_venice_defconfig
make

> >
> > BINMAN_VERBOSE=4 indeed prints out a tone of stuff but I'm not seeing
> > anything for 'blob' below that would seem to indicate one node name vs
> > another:
>
> Oops you need BINMAN_VERBOSE=5 - see elf.py LookupAndWriteSymbols()
> which has tout.Debug() which is level 5.
>

LookupAndWriteSymbols ends up doing nothing because
syms.get('__image_copy_start') returns None.

Thanks,

Tim

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: using binman fails boot
  2021-07-16 23:15       ` Tim Harvey
@ 2021-07-18  2:22         ` Simon Glass
  2021-07-19 23:23           ` Tim Harvey
  0 siblings, 1 reply; 14+ messages in thread
From: Simon Glass @ 2021-07-18  2:22 UTC (permalink / raw)
  To: Tim Harvey; +Cc: u-boot

Hi Tim,

On Fri, 16 Jul 2021 at 17:15, Tim Harvey <tharvey@gateworks.com> wrote:
>
> On Fri, Jul 16, 2021 at 3:11 PM Simon Glass <sjg@chromium.org> wrote:
> >
> > () which has
> > Hi Tim,
> >
> > On Fri, 16 Jul 2021 at 15:43, Tim Harvey <tharvey@gateworks.com> wrote:
> > >
> > > On Thu, Jul 15, 2021 at 9:30 PM Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Tim,
> > > >
> > > > On Thu, 15 Jul 2021 at 16:58, Tim Harvey <tharvey@gateworks.com> wrote:
> > > > >
> > > > > Greetings,
> > > > >
> > > > > I'm taking a look at moving imx8mm-venice to use binman for packaging.
> > > > > After doing so U-Boot proper fails to boot:
> > > > >
> > > > > U-Boot SPL 2021.07-00475-g1126252f40 (Jul 15 2021 - 11:09:02 -0700)
> > > > > GSC     : v58 0xf098 RST:VIN Thermal Protection Disabled
> > > > > Model   : GW7300-00-B1B
> > > > > Serial  : 852420
> > > > > MFGDate : 10-26-2020
> > > > > RTC     : 122
> > > > > PMIC    : MP5416
> > > > > DRAM    : LPDDR4 1 GiB
> > > > > WDT:   Not starting
> > > > > Trying to boot from MMC1
> > > > > DTB     : imx8mm-venice-gw73xx-0x
> > > > >
> > > > >
> > > > > U-Boot 2021.07-00475-g1126252f40 (Jul 15 2021 - 11:09:02 -0700)
> > > > >
> > > > > CPU:   Freescale i.MX8MMQ rev1.0 1600 MHz (running at 1200 MHz)
> > > > > CPU:   Industrial temperature grade (-40C to 105C) at 43C
> > > > > Reset cause: POR
> > > > > Model: Gateworks Venice GW73xx-0x i.MX8MM Development Kit
> > > > > DRAM:  1 GiB
> > > > > temp    : 38.3C
> > > > > vdd_bat : 0.000V
> > > > > vdd_vin : 15.731V
> > > > > vdd_adc1: 0.000V
> > > > > vdd_adc2: 0.000V
> > > > > vdd_dram: 1.093V
> > > > > vdd_1p2 : 1.193V
> > > > > vdd_1p0 : 0.985V
> > > > > vdd_2p5 : 2.470V
> > > > > vdd_3p3 : 3.250V
> > > > > vdd_0p95: 0.948V
> > > > > vdd_1p8 : 1.799V
> > > > > vdd_gsc : 3.262V
> > > > > initcall sequence 000000007ffc4f58 failed at call 0000000040255910 (err=-2)
> > > > > ### ERROR ### Please RESET the board ###
> > > > >
> > > > > Any ideas what this could be?
> > > >
> > > > I don't have much idea. What is the initcall that is failing? Can you
> > > > check u-boot.map ? That might give a clue as to what is failing. I
> > > > assume the DT is passed to U-Boot somehow from SPL?
> > > >
> > >
> > > Simon,
> > >
> > > Thanks for the help!
> > >
> > > The initcall addr doesn't match anything in u-boot.map (maybe
> > > u-boot.map doesn't show what's in lib/binman.o?) but I was able to
> >
> > It certainly should show up, but if you have CONFIG_LTO enabled lots
> > of functions disappear. Still if you get an initcall address I would
> > expect a function to be present. Make sure you use the unallocated
> > address.
>
> I'm not sure what you mean by 'Make sure you use the unallocated address'

Sorry I mean unrelocated address. After U-Boot relocates the addresses
change but it still prints out the original addresses.

>
> >
> > > track it down to initr_binman() failing due to
> > > binman_init()->find_image_node(&binman->image)' returning -EINVAL.
> > > This is because my imx8mm-venice-gw73xx-0x-uboot.dtsi doesn't have a
> > > binman node (my CONFIG_DEFAULT_DEVICE_TREE did but not my actual
> > > dtbs). So I have it working now!
> >
> > OK good progress! Perhaps we should put an error message in initr_binman() ?
> >
>
> sure - I just sent a patch
>
> > >
> > > > >
> > > <snip>
> > > > >
> > > > > A follow-on question is that I would like to investigate using binman
> > > > > in the SPL to dynamically access the IMX8M ddr training blobs so that
> > > > > we don't have to waste padding space taking them onto the end of the
> > > > > SPL which is currently done. The lpddr4 training blobs I'm using
> > > > > currently take up 57k without padding compared to 81k with padding.
> > > > > The location of them is handled in ddr_load_train_firmware.
> > > > >
> > > > > If I add the following to my SPL:
> > > > > diff --git a/board/gateworks/venice/spl.c b/board/gateworks/venice/spl.c
> > > > > index d0a490b0e6..62eb67fa5e 100644
> > > > > --- a/board/gateworks/venice/spl.c
> > > > > +++ b/board/gateworks/venice/spl.c
> > > > > @@ -3,6 +3,7 @@
> > > > >   * Copyright 2021 Gateworks Corporation
> > > > >   */
> > > > >
> > > > > +#include <binman_sym.h>
> > > > >  #include <common.h>
> > > > >  #include <cpu_func.h>
> > > > >  #include <hang.h>
> > > > > @@ -252,6 +253,8 @@ static int power_init_board(void)
> > > > >         return 0;
> > > > >  }
> > > > >
> > > > > +binman_sym_declare(ulong, blob_1, image_pos);
> > > > > +
> > > > >  void board_init_f(ulong dummy)
> > > > >  {
> > > > >         struct udevice *dev;
> > > > > @@ -291,6 +294,8 @@ void board_init_f(ulong dummy)
> > > > >         gpio_request(PCIE_RSTN, "perst#");
> > > > >         gpio_direction_output(PCIE_RSTN, 0);
> > > > >
> > > > > +       printf("%s: blob_1:0x%0lx\n", __func__, binman_sym(ulong,
> > > > > blob_1, image_pos));
> > > > > +
> > > > >         /* GSC */
> > > > >         dram_sz = gsc_init(0);
> > > > >
> > > > > I get 'blob_1:0x0' which is not what I expected.
> > > > >
> > > > > If I understand correctly binman is using linker symbols to determine
> > > > > where things are in the image? What I don't quite understand is what
> > > > > symbols are valid to use assuming my dtsi above. The binman.rst docs
> > > > > talk use 'u_boot_any' as an example which apparently can match
> > > > > 'u-boot.bin', 'u-boot.img', and 'u-boot-nodtb.bin' but I can't find
> > > > > the code that somehow translates this meaning.
> > > >
> > > > Actually any symbol can be used. It basically depends on the name of
> > > > the entry in your image description. So here it would be
> > > > blob-ext@1...I think that translates to blob_ext_1 but I'm not sure
> > > > about the @. You could try blob-ext-1 instead. It does not know about
> > > > phandles or labels.
> > > >
> > > > If you pass BINMAN_VERBOSE=4 to the build you should see it talking
> > > > about writing symbols into the SPL image.
> > > >
> > >
> > > For the following:
> > >          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-ext@1 {
> > >                         filename = "lpddr4_pmu_train_1d_imem.bin";
> > >                         size = <0x8000>;
> > >                 };
> > >
> > >                 blob-ext@2 {
> > >                         filename = "lpddr4_pmu_train_1d_dmem.bin";
> > >                         size = <0x4000>;
> > >                 };
> > >
> > >                 blob-ext@3 {
> > >                         filename = "lpddr4_pmu_train_2d_imem.bin";
> > >                         size = <0x8000>;
> > >                 };
> > >
> > >                 blob-ext@4 {
> > >                         filename = "lpddr4_pmu_train_2d_dmem.bin";
> > >                         size = <0x4000>;
> > >                 };
> > >         };
> > >
> > > I tried 'blob_ext_1' and 'blob_ext1' and both formats resolve to 0x0.
> > > The 'ext-blob' is an entry type supported by binman so if I had
> > > multiple they must be called blob-ext@1, blob-ext@2, ... right?
> > >
> > > The entry_name used in binman_sym_declare/binman_sym certainly can't
> > > support non C varname characters so '-' and '@' characters must get
> > > translated somewhere. Where would that be done in order to figure out
> > > what to use?
> >
> > If you want to look at the internals, see section.py LookupSymbol().
> >
> > It takes the ELF symbol and replaces _ by - but does not (cannot)
> > replace _ with @. So I think you'll have to use - instead of @
> >
> > I suppose we could do the search in the other direction (take the
> > entry and try to find the symbol that matches it), but I'd need to
> > think about it. A simple translation is easier.
> >
> > In this case binman should really give an error for your chosen entry
> > name (blob-ext@4) but it doesn't know you are using it as a symbol. I
> > think it should complain about this (see the Warning in section.py
> > LookupSymbol()) but apparently it does not in your case.
>
> But isn't blob-ext@4 a correct name? I can't use 'blob-ext-4' as
> that's an unknown entry type.

Well you can use any name and specify the type:

my-name {
   type = "blob-ext";
};

>
> >
> > If you can push your tree somewhere (with this problem) I'll see if I
> > can figure out why.
> >
>
> Sure, I pushed it to
> https://github.com/Gateworks/uboot-venice/tree/WIP-venice-binman
> make imx8mm_venice_defconfig
> make

OK

>
> > >
> > > BINMAN_VERBOSE=4 indeed prints out a tone of stuff but I'm not seeing
> > > anything for 'blob' below that would seem to indicate one node name vs
> > > another:
> >
> > Oops you need BINMAN_VERBOSE=5 - see elf.py LookupAndWriteSymbols()
> > which has tout.Debug() which is level 5.
> >
>
> LookupAndWriteSymbols ends up doing nothing because
> syms.get('__image_copy_start') returns None.

Well that is likely the problem.

Regards,
Simon

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: using binman fails boot
  2021-07-18  2:22         ` Simon Glass
@ 2021-07-19 23:23           ` Tim Harvey
  2021-07-23  3:07             ` Simon Glass
  0 siblings, 1 reply; 14+ messages in thread
From: Tim Harvey @ 2021-07-19 23:23 UTC (permalink / raw)
  To: Simon Glass; +Cc: u-boot

On Sat, Jul 17, 2021 at 7:22 PM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Tim,
>
> On Fri, 16 Jul 2021 at 17:15, Tim Harvey <tharvey@gateworks.com> wrote:
> >
> > On Fri, Jul 16, 2021 at 3:11 PM Simon Glass <sjg@chromium.org> wrote:
> > >
> > > () which has
> > > Hi Tim,
> > >
> > > On Fri, 16 Jul 2021 at 15:43, Tim Harvey <tharvey@gateworks.com> wrote:
> > > >
> > > > On Thu, Jul 15, 2021 at 9:30 PM Simon Glass <sjg@chromium.org> wrote:
> > > > >
> > > > > Hi Tim,
> > > > >
> > > > > On Thu, 15 Jul 2021 at 16:58, Tim Harvey <tharvey@gateworks.com> wrote:
> > > > > >
> > > > > > Greetings,
> > > > > >
> > > > > > I'm taking a look at moving imx8mm-venice to use binman for packaging.
> > > > > > After doing so U-Boot proper fails to boot:
> > > > > >
> > > > > > U-Boot SPL 2021.07-00475-g1126252f40 (Jul 15 2021 - 11:09:02 -0700)
> > > > > > GSC     : v58 0xf098 RST:VIN Thermal Protection Disabled
> > > > > > Model   : GW7300-00-B1B
> > > > > > Serial  : 852420
> > > > > > MFGDate : 10-26-2020
> > > > > > RTC     : 122
> > > > > > PMIC    : MP5416
> > > > > > DRAM    : LPDDR4 1 GiB
> > > > > > WDT:   Not starting
> > > > > > Trying to boot from MMC1
> > > > > > DTB     : imx8mm-venice-gw73xx-0x
> > > > > >
> > > > > >
> > > > > > U-Boot 2021.07-00475-g1126252f40 (Jul 15 2021 - 11:09:02 -0700)
> > > > > >
> > > > > > CPU:   Freescale i.MX8MMQ rev1.0 1600 MHz (running at 1200 MHz)
> > > > > > CPU:   Industrial temperature grade (-40C to 105C) at 43C
> > > > > > Reset cause: POR
> > > > > > Model: Gateworks Venice GW73xx-0x i.MX8MM Development Kit
> > > > > > DRAM:  1 GiB
> > > > > > temp    : 38.3C
> > > > > > vdd_bat : 0.000V
> > > > > > vdd_vin : 15.731V
> > > > > > vdd_adc1: 0.000V
> > > > > > vdd_adc2: 0.000V
> > > > > > vdd_dram: 1.093V
> > > > > > vdd_1p2 : 1.193V
> > > > > > vdd_1p0 : 0.985V
> > > > > > vdd_2p5 : 2.470V
> > > > > > vdd_3p3 : 3.250V
> > > > > > vdd_0p95: 0.948V
> > > > > > vdd_1p8 : 1.799V
> > > > > > vdd_gsc : 3.262V
> > > > > > initcall sequence 000000007ffc4f58 failed at call 0000000040255910 (err=-2)
> > > > > > ### ERROR ### Please RESET the board ###
> > > > > >
> > > > > > Any ideas what this could be?
> > > > >
> > > > > I don't have much idea. What is the initcall that is failing? Can you
> > > > > check u-boot.map ? That might give a clue as to what is failing. I
> > > > > assume the DT is passed to U-Boot somehow from SPL?
> > > > >
> > > >
> > > > Simon,
> > > >
> > > > Thanks for the help!
> > > >
> > > > The initcall addr doesn't match anything in u-boot.map (maybe
> > > > u-boot.map doesn't show what's in lib/binman.o?) but I was able to
> > >
> > > It certainly should show up, but if you have CONFIG_LTO enabled lots
> > > of functions disappear. Still if you get an initcall address I would
> > > expect a function to be present. Make sure you use the unallocated
> > > address.
> >
> > I'm not sure what you mean by 'Make sure you use the unallocated address'
>
> Sorry I mean unrelocated address. After U-Boot relocates the addresses
> change but it still prints out the original addresses.
>
> >
> > >
> > > > track it down to initr_binman() failing due to
> > > > binman_init()->find_image_node(&binman->image)' returning -EINVAL.
> > > > This is because my imx8mm-venice-gw73xx-0x-uboot.dtsi doesn't have a
> > > > binman node (my CONFIG_DEFAULT_DEVICE_TREE did but not my actual
> > > > dtbs). So I have it working now!
> > >
> > > OK good progress! Perhaps we should put an error message in initr_binman() ?
> > >
> >
> > sure - I just sent a patch
> >
> > > >
> > > > > >
> > > > <snip>
> > > > > >
> > > > > > A follow-on question is that I would like to investigate using binman
> > > > > > in the SPL to dynamically access the IMX8M ddr training blobs so that
> > > > > > we don't have to waste padding space taking them onto the end of the
> > > > > > SPL which is currently done. The lpddr4 training blobs I'm using
> > > > > > currently take up 57k without padding compared to 81k with padding.
> > > > > > The location of them is handled in ddr_load_train_firmware.
> > > > > >
> > > > > > If I add the following to my SPL:
> > > > > > diff --git a/board/gateworks/venice/spl.c b/board/gateworks/venice/spl.c
> > > > > > index d0a490b0e6..62eb67fa5e 100644
> > > > > > --- a/board/gateworks/venice/spl.c
> > > > > > +++ b/board/gateworks/venice/spl.c
> > > > > > @@ -3,6 +3,7 @@
> > > > > >   * Copyright 2021 Gateworks Corporation
> > > > > >   */
> > > > > >
> > > > > > +#include <binman_sym.h>
> > > > > >  #include <common.h>
> > > > > >  #include <cpu_func.h>
> > > > > >  #include <hang.h>
> > > > > > @@ -252,6 +253,8 @@ static int power_init_board(void)
> > > > > >         return 0;
> > > > > >  }
> > > > > >
> > > > > > +binman_sym_declare(ulong, blob_1, image_pos);
> > > > > > +
> > > > > >  void board_init_f(ulong dummy)
> > > > > >  {
> > > > > >         struct udevice *dev;
> > > > > > @@ -291,6 +294,8 @@ void board_init_f(ulong dummy)
> > > > > >         gpio_request(PCIE_RSTN, "perst#");
> > > > > >         gpio_direction_output(PCIE_RSTN, 0);
> > > > > >
> > > > > > +       printf("%s: blob_1:0x%0lx\n", __func__, binman_sym(ulong,
> > > > > > blob_1, image_pos));
> > > > > > +
> > > > > >         /* GSC */
> > > > > >         dram_sz = gsc_init(0);
> > > > > >
> > > > > > I get 'blob_1:0x0' which is not what I expected.
> > > > > >
> > > > > > If I understand correctly binman is using linker symbols to determine
> > > > > > where things are in the image? What I don't quite understand is what
> > > > > > symbols are valid to use assuming my dtsi above. The binman.rst docs
> > > > > > talk use 'u_boot_any' as an example which apparently can match
> > > > > > 'u-boot.bin', 'u-boot.img', and 'u-boot-nodtb.bin' but I can't find
> > > > > > the code that somehow translates this meaning.
> > > > >
> > > > > Actually any symbol can be used. It basically depends on the name of
> > > > > the entry in your image description. So here it would be
> > > > > blob-ext@1...I think that translates to blob_ext_1 but I'm not sure
> > > > > about the @. You could try blob-ext-1 instead. It does not know about
> > > > > phandles or labels.
> > > > >
> > > > > If you pass BINMAN_VERBOSE=4 to the build you should see it talking
> > > > > about writing symbols into the SPL image.
> > > > >
> > > >
> > > > For the following:
> > > >          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-ext@1 {
> > > >                         filename = "lpddr4_pmu_train_1d_imem.bin";
> > > >                         size = <0x8000>;
> > > >                 };
> > > >
> > > >                 blob-ext@2 {
> > > >                         filename = "lpddr4_pmu_train_1d_dmem.bin";
> > > >                         size = <0x4000>;
> > > >                 };
> > > >
> > > >                 blob-ext@3 {
> > > >                         filename = "lpddr4_pmu_train_2d_imem.bin";
> > > >                         size = <0x8000>;
> > > >                 };
> > > >
> > > >                 blob-ext@4 {
> > > >                         filename = "lpddr4_pmu_train_2d_dmem.bin";
> > > >                         size = <0x4000>;
> > > >                 };
> > > >         };
> > > >
> > > > I tried 'blob_ext_1' and 'blob_ext1' and both formats resolve to 0x0.
> > > > The 'ext-blob' is an entry type supported by binman so if I had
> > > > multiple they must be called blob-ext@1, blob-ext@2, ... right?
> > > >
> > > > The entry_name used in binman_sym_declare/binman_sym certainly can't
> > > > support non C varname characters so '-' and '@' characters must get
> > > > translated somewhere. Where would that be done in order to figure out
> > > > what to use?
> > >
> > > If you want to look at the internals, see section.py LookupSymbol().
> > >
> > > It takes the ELF symbol and replaces _ by - but does not (cannot)
> > > replace _ with @. So I think you'll have to use - instead of @
> > >
> > > I suppose we could do the search in the other direction (take the
> > > entry and try to find the symbol that matches it), but I'd need to
> > > think about it. A simple translation is easier.
> > >
> > > In this case binman should really give an error for your chosen entry
> > > name (blob-ext@4) but it doesn't know you are using it as a symbol. I
> > > think it should complain about this (see the Warning in section.py
> > > LookupSymbol()) but apparently it does not in your case.
> >
> > But isn't blob-ext@4 a correct name? I can't use 'blob-ext-4' as
> > that's an unknown entry type.
>
> Well you can use any name and specify the type:
>
> my-name {
>    type = "blob-ext";
> };
>

Ok - I understand.

> >
> > >
> > > If you can push your tree somewhere (with this problem) I'll see if I
> > > can figure out why.
> > >
> >
> > Sure, I pushed it to
> > https://github.com/Gateworks/uboot-venice/tree/WIP-venice-binman
> > make imx8mm_venice_defconfig
> > make
>
> OK
>
> >
> > > >
> > > > BINMAN_VERBOSE=4 indeed prints out a tone of stuff but I'm not seeing
> > > > anything for 'blob' below that would seem to indicate one node name vs
> > > > another:
> > >
> > > Oops you need BINMAN_VERBOSE=5 - see elf.py LookupAndWriteSymbols()
> > > which has tout.Debug() which is level 5.
> > >
> >
> > LookupAndWriteSymbols ends up doing nothing because
> > syms.get('__image_copy_start') returns None.
>
> Well that is likely the problem.
>

Here are the syms getting passed to LookupAndWriteSymbols:

syms: OrderedDict([('zimage.c.482a543f', Symbol(section='.debug_info',
address=9039, size=0, weak=True)), ('image.c.f5c25828',
Symbol(section='.debug_info', address=149598, size=0, weak=True)),
('image_fdt.c.d0cf71b8', Symbol(section='.debug_info', address=158369,
size=0, weak=True)), ('image_fit.c.c45e696c',
Symbol(section='.debug_info', address=162759, size=0, weak=True)),
('spl_parse_image_header', Symbol(section='.text', address=8308244,
size=100, weak=False)), ('fit_image_get_address',
Symbol(section='.text', address=8317036, size=136, weak=False)),
('spl_fit_image_get_os', Symbol(section='.text', address=8317172,
size=92, weak=False)), ('spl_fit_get_image_node',
Symbol(section='.text', address=8318196, size=180, weak=False)),
('spl_load_fit_image', Symbol(section='.text', address=8329124,
size=552, weak=False)), ('spl_mmc_load_image', Symbol(section='.text',
address=8331700, size=796, weak=False)), ('spl_sdp_load_image',
Symbol(section='.text', address=8334688, size=2052, weak=False)),
('.binman_sym', Symbol(section='.binman_sym', address=8392576, size=0,
weak=False)), ('_binman_u_boot_any_prop_image_pos',
Symbol(section='.binman_sym', address=8392576, size=8, weak=False)),
('_binman_blob1_prop_image_pos', Symbol(section='.binman_sym',
address=8392584, size=8, weak=False)),
('_binman_blob2_prop_image_pos', Symbol(section='.binman_sym',
address=8392592, size=8, weak=False)),
('_binman_blob3_prop_image_pos', Symbol(section='.binman_sym',
address=8392600, size=8, weak=False)),
('_binman_blob4_prop_image_pos', Symbol(section='.binman_sym',
address=8392608, size=8, weak=False)),
('_u_boot_list_2_spl_image_loader_2_BOOT_DEVICE_BOARD0spl_sdp_load_image',
Symbol(section='.u_boot_list', address=8396392, size=24, weak=False)),
('_u_boot_list_2_spl_image_loader_2_BOOT_DEVICE_MMC10spl_mmc_load_image',
Symbol(section='.u_boot_list', address=8396416, size=24, weak=False)),
('_u_boot_list_2_spl_image_loader_2_BOOT_DEVICE_MMC20spl_mmc_load_image',
Symbol(section='.u_boot_list', address=8396440, size=24, weak=False)),
('_u_boot_list_2_spl_image_loader_2_BOOT_DEVICE_MMC2_20spl_mmc_load_image',
Symbol(section='.u_boot_list', address=8396464, size=24, weak=False)),
('.image_copy_end', Symbol(section='.image_copy_end', address=8398288,
size=0, weak=False)), ('_image_binary_end', Symbol(section='.end',
address=8398288, size=0, weak=False))])

Tim

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: using binman fails boot
  2021-07-19 23:23           ` Tim Harvey
@ 2021-07-23  3:07             ` Simon Glass
  2021-07-23 21:06               ` Tim Harvey
  0 siblings, 1 reply; 14+ messages in thread
From: Simon Glass @ 2021-07-23  3:07 UTC (permalink / raw)
  To: Tim Harvey; +Cc: u-boot

Hi Tim,

On Mon, 19 Jul 2021 at 17:23, Tim Harvey <tharvey@gateworks.com> wrote:
>
> On Sat, Jul 17, 2021 at 7:22 PM Simon Glass <sjg@chromium.org> wrote:
> >

[..]

> > > But isn't blob-ext@4 a correct name? I can't use 'blob-ext-4' as
> > > that's an unknown entry type.
> >
> > Well you can use any name and specify the type:
> >
> > my-name {
> >    type = "blob-ext";
> > };
> >
>
> Ok - I understand.
>
> > >
> > > >
> > > > If you can push your tree somewhere (with this problem) I'll see if I
> > > > can figure out why.
> > > >
> > >
> > > Sure, I pushed it to
> > > https://github.com/Gateworks/uboot-venice/tree/WIP-venice-binman
> > > make imx8mm_venice_defconfig
> > > make
> >
> > OK
> >
> > >
> > > > >
> > > > > BINMAN_VERBOSE=4 indeed prints out a tone of stuff but I'm not seeing
> > > > > anything for 'blob' below that would seem to indicate one node name vs
> > > > > another:
> > > >
> > > > Oops you need BINMAN_VERBOSE=5 - see elf.py LookupAndWriteSymbols()
> > > > which has tout.Debug() which is level 5.
> > > >
> > >
> > > LookupAndWriteSymbols ends up doing nothing because
> > > syms.get('__image_copy_start') returns None.
> >
> > Well that is likely the problem.

I sent a patch to make binman report this as an error.

I pushed the resulting tree to:

https://github.com/sjg20/u-boot/tree/try-tim

Now the error is:

binman: Section '/binman/u-boot-spl-ddr': Symbol
'_binman_u_boot_any_prop_image_pos'

   in entry '/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-nodtb':
Entry 'u-boot-any' not found in list
(u-boot-spl-nodtb,u-boot-spl-dtb,u-boot-spl,blob-ext@1,blob-ext@2,blob-ext@3,blob-ext@4,main-section)

The problem seems to be that you are asking binman to generate three
independent images. U-Boot is in a FIT which is not in the same image
as SPL. So it is not possible to locate the flash offset of U-Boot
(with in the FIT).

Can you give me a bit more info about your intent here? Is it to load
U-Boot from the FIT? I so, I suppose it is possible to make binman
access an independent image, if it is told where it starts.

But why is everything not in one image?

Regards,
Simon

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: using binman fails boot
  2021-07-23  3:07             ` Simon Glass
@ 2021-07-23 21:06               ` Tim Harvey
  2021-07-23 21:41                 ` Simon Glass
  0 siblings, 1 reply; 14+ messages in thread
From: Tim Harvey @ 2021-07-23 21:06 UTC (permalink / raw)
  To: Simon Glass; +Cc: u-boot

On Thu, Jul 22, 2021 at 8:07 PM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Tim,
>
> On Mon, 19 Jul 2021 at 17:23, Tim Harvey <tharvey@gateworks.com> wrote:
> >
> > On Sat, Jul 17, 2021 at 7:22 PM Simon Glass <sjg@chromium.org> wrote:
> > >
>
> [..]
>
> > > > But isn't blob-ext@4 a correct name? I can't use 'blob-ext-4' as
> > > > that's an unknown entry type.
> > >
> > > Well you can use any name and specify the type:
> > >
> > > my-name {
> > >    type = "blob-ext";
> > > };
> > >
> >
> > Ok - I understand.
> >
> > > >
> > > > >
> > > > > If you can push your tree somewhere (with this problem) I'll see if I
> > > > > can figure out why.
> > > > >
> > > >
> > > > Sure, I pushed it to
> > > > https://github.com/Gateworks/uboot-venice/tree/WIP-venice-binman
> > > > make imx8mm_venice_defconfig
> > > > make
> > >
> > > OK
> > >
> > > >
> > > > > >
> > > > > > BINMAN_VERBOSE=4 indeed prints out a tone of stuff but I'm not seeing
> > > > > > anything for 'blob' below that would seem to indicate one node name vs
> > > > > > another:
> > > > >
> > > > > Oops you need BINMAN_VERBOSE=5 - see elf.py LookupAndWriteSymbols()
> > > > > which has tout.Debug() which is level 5.
> > > > >
> > > >
> > > > LookupAndWriteSymbols ends up doing nothing because
> > > > syms.get('__image_copy_start') returns None.
> > >
> > > Well that is likely the problem.
>
> I sent a patch to make binman report this as an error.
>
> I pushed the resulting tree to:
>
> https://github.com/sjg20/u-boot/tree/try-tim
>
> Now the error is:
>
> binman: Section '/binman/u-boot-spl-ddr': Symbol
> '_binman_u_boot_any_prop_image_pos'
>
>    in entry '/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-nodtb':
> Entry 'u-boot-any' not found in list
> (u-boot-spl-nodtb,u-boot-spl-dtb,u-boot-spl,blob-ext@1,blob-ext@2,blob-ext@3,blob-ext@4,main-section)
>
> The problem seems to be that you are asking binman to generate three
> independent images. U-Boot is in a FIT which is not in the same image
> as SPL. So it is not possible to locate the flash offset of U-Boot
> (with in the FIT).
>
> Can you give me a bit more info about your intent here? Is it to load
> U-Boot from the FIT? I so, I suppose it is possible to make binman
> access an independent image, if it is told where it starts.
>
> But why is everything not in one image?
>

Simon,

I would rather have 1 image. I was going off of the imx8mm_evk switch
to binman which creates the separate images.

The whole point of what I'm investigating here has to do with the SPL.
OCRAM is at a premium and the current way the IMX8M is handling DDR
firmware is to tack it on after the code in the SPL image and it gets
padded to make it easy to locate which is a huge waste of space. I
figured we can use binman to locate the blobs without the padding.

So, if you take 'just' the spl image here:
        spl: 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.bin";
                        size = <0x8000>;
                };

                blob_2: blob-ext@2 {
                        filename = "lpddr4_pmu_train_1d_dmem.bin";
                        size = <0x4000>;
                };

                blob_3: blob-ext@3 {
                        filename = "lpddr4_pmu_train_2d_imem.bin";
                        size = <0x8000>;
                };

                blob_4: blob-ext@4 {
                        filename = "lpddr4_pmu_train_2d_dmem.bin";
                        size = <0x4000>;
                };
        };

My intention is to remove the size arguments above which are currently
forcing wasted padding and locate the blobs at runtime with binman.

Based on your other patch it it would seem I'm missing something from
my lds to add __image_copy_start yet in
arch/arm/cpu/armv8/u-boot-spl.lds I see:
        .text : {
                . = ALIGN(8);
                *(.__image_copy_start)
                CPUDIR/start.o (.text*)
                *(.text*)
        } >.sram

My understanding of linker files is pretty slim so perhaps there's
something missing above.

Regards,

Tim

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: using binman fails boot
  2021-07-23 21:06               ` Tim Harvey
@ 2021-07-23 21:41                 ` Simon Glass
  2021-07-23 22:51                   ` Tim Harvey
  0 siblings, 1 reply; 14+ messages in thread
From: Simon Glass @ 2021-07-23 21:41 UTC (permalink / raw)
  To: Tim Harvey; +Cc: u-boot

Hi Tim,

On Fri, 23 Jul 2021 at 15:06, Tim Harvey <tharvey@gateworks.com> wrote:
>
> On Thu, Jul 22, 2021 at 8:07 PM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Tim,
> >
> > On Mon, 19 Jul 2021 at 17:23, Tim Harvey <tharvey@gateworks.com> wrote:
> > >
> > > On Sat, Jul 17, 2021 at 7:22 PM Simon Glass <sjg@chromium.org> wrote:
> > > >
> >
> > [..]
> >
> > > > > But isn't blob-ext@4 a correct name? I can't use 'blob-ext-4' as
> > > > > that's an unknown entry type.
> > > >
> > > > Well you can use any name and specify the type:
> > > >
> > > > my-name {
> > > >    type = "blob-ext";
> > > > };
> > > >
> > >
> > > Ok - I understand.
> > >
> > > > >
> > > > > >
> > > > > > If you can push your tree somewhere (with this problem) I'll see if I
> > > > > > can figure out why.
> > > > > >
> > > > >
> > > > > Sure, I pushed it to
> > > > > https://github.com/Gateworks/uboot-venice/tree/WIP-venice-binman
> > > > > make imx8mm_venice_defconfig
> > > > > make
> > > >
> > > > OK
> > > >
> > > > >
> > > > > > >
> > > > > > > BINMAN_VERBOSE=4 indeed prints out a tone of stuff but I'm not seeing
> > > > > > > anything for 'blob' below that would seem to indicate one node name vs
> > > > > > > another:
> > > > > >
> > > > > > Oops you need BINMAN_VERBOSE=5 - see elf.py LookupAndWriteSymbols()
> > > > > > which has tout.Debug() which is level 5.
> > > > > >
> > > > >
> > > > > LookupAndWriteSymbols ends up doing nothing because
> > > > > syms.get('__image_copy_start') returns None.
> > > >
> > > > Well that is likely the problem.
> >
> > I sent a patch to make binman report this as an error.
> >
> > I pushed the resulting tree to:
> >
> > https://github.com/sjg20/u-boot/tree/try-tim
> >
> > Now the error is:
> >
> > binman: Section '/binman/u-boot-spl-ddr': Symbol
> > '_binman_u_boot_any_prop_image_pos'
> >
> >    in entry '/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-nodtb':
> > Entry 'u-boot-any' not found in list
> > (u-boot-spl-nodtb,u-boot-spl-dtb,u-boot-spl,blob-ext@1,blob-ext@2,blob-ext@3,blob-ext@4,main-section)
> >
> > The problem seems to be that you are asking binman to generate three
> > independent images. U-Boot is in a FIT which is not in the same image
> > as SPL. So it is not possible to locate the flash offset of U-Boot
> > (with in the FIT).
> >
> > Can you give me a bit more info about your intent here? Is it to load
> > U-Boot from the FIT? I so, I suppose it is possible to make binman
> > access an independent image, if it is told where it starts.
> >
> > But why is everything not in one image?
> >
>
> Simon,
>
> I would rather have 1 image. I was going off of the imx8mm_evk switch
> to binman which creates the separate images.

Well at present you are loading a FIT into RAM, I think? Is it coming
from flash?

If you load a FIT containing U-Boot then you don't need the binman
symbol stuff, since SPL looks in the FIT for the location of U-Boot.
There isn't much benefit in having binman point to U-Boot within the
FIT, since we already have code to find it. It might save a few bytes
of code, but it would be confusing...I'm not sure if that is worth the
hassle.

If you want a single image, then you might not want FIT at all...just
use binman.

It really depends what you want.

>
> The whole point of what I'm investigating here has to do with the SPL.
> OCRAM is at a premium and the current way the IMX8M is handling DDR
> firmware is to tack it on after the code in the SPL image and it gets
> padded to make it easy to locate which is a huge waste of space. I
> figured we can use binman to locate the blobs without the padding.
>
> So, if you take 'just' the spl image here:
>         spl: 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.bin";
>                         size = <0x8000>;
>                 };
>
>                 blob_2: blob-ext@2 {
>                         filename = "lpddr4_pmu_train_1d_dmem.bin";
>                         size = <0x4000>;
>                 };
>
>                 blob_3: blob-ext@3 {
>                         filename = "lpddr4_pmu_train_2d_imem.bin";
>                         size = <0x8000>;
>                 };
>
>                 blob_4: blob-ext@4 {
>                         filename = "lpddr4_pmu_train_2d_dmem.bin";
>                         size = <0x4000>;
>                 };
>         };
>
> My intention is to remove the size arguments above which are currently
> forcing wasted padding and locate the blobs at runtime with binman.

Well you can just remove them.

>
> Based on your other patch it it would seem I'm missing something from
> my lds to add __image_copy_start yet in
> arch/arm/cpu/armv8/u-boot-spl.lds I see:
>         .text : {
>                 . = ALIGN(8);
>                 *(.__image_copy_start)
>                 CPUDIR/start.o (.text*)
>                 *(.text*)
>         } >.sram
>
> My understanding of linker files is pretty slim so perhaps there's
> something missing above.

Yes you need to define the value of the __image_copy_start symbol, so:

.text: {
    __image_copy_start = .;

See for example arch/arm/cpu/u-boot-spl.lds

Regards,
SImon

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: using binman fails boot
  2021-07-23 21:41                 ` Simon Glass
@ 2021-07-23 22:51                   ` Tim Harvey
  2021-07-24 22:01                     ` Simon Glass
  0 siblings, 1 reply; 14+ messages in thread
From: Tim Harvey @ 2021-07-23 22:51 UTC (permalink / raw)
  To: Simon Glass; +Cc: u-boot

On Fri, Jul 23, 2021 at 2:41 PM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Tim,
>
> On Fri, 23 Jul 2021 at 15:06, Tim Harvey <tharvey@gateworks.com> wrote:
> >
> > On Thu, Jul 22, 2021 at 8:07 PM Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Tim,
> > >
> > > On Mon, 19 Jul 2021 at 17:23, Tim Harvey <tharvey@gateworks.com> wrote:
> > > >
> > > > On Sat, Jul 17, 2021 at 7:22 PM Simon Glass <sjg@chromium.org> wrote:
> > > > >
> > >
> > > [..]
> > >
> > > > > > But isn't blob-ext@4 a correct name? I can't use 'blob-ext-4' as
> > > > > > that's an unknown entry type.
> > > > >
> > > > > Well you can use any name and specify the type:
> > > > >
> > > > > my-name {
> > > > >    type = "blob-ext";
> > > > > };
> > > > >
> > > >
> > > > Ok - I understand.
> > > >
> > > > > >
> > > > > > >
> > > > > > > If you can push your tree somewhere (with this problem) I'll see if I
> > > > > > > can figure out why.
> > > > > > >
> > > > > >
> > > > > > Sure, I pushed it to
> > > > > > https://github.com/Gateworks/uboot-venice/tree/WIP-venice-binman
> > > > > > make imx8mm_venice_defconfig
> > > > > > make
> > > > >
> > > > > OK
> > > > >
> > > > > >
> > > > > > > >
> > > > > > > > BINMAN_VERBOSE=4 indeed prints out a tone of stuff but I'm not seeing
> > > > > > > > anything for 'blob' below that would seem to indicate one node name vs
> > > > > > > > another:
> > > > > > >
> > > > > > > Oops you need BINMAN_VERBOSE=5 - see elf.py LookupAndWriteSymbols()
> > > > > > > which has tout.Debug() which is level 5.
> > > > > > >
> > > > > >
> > > > > > LookupAndWriteSymbols ends up doing nothing because
> > > > > > syms.get('__image_copy_start') returns None.
> > > > >
> > > > > Well that is likely the problem.
> > >
> > > I sent a patch to make binman report this as an error.
> > >
> > > I pushed the resulting tree to:
> > >
> > > https://github.com/sjg20/u-boot/tree/try-tim
> > >
> > > Now the error is:
> > >
> > > binman: Section '/binman/u-boot-spl-ddr': Symbol
> > > '_binman_u_boot_any_prop_image_pos'
> > >
> > >    in entry '/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-nodtb':
> > > Entry 'u-boot-any' not found in list
> > > (u-boot-spl-nodtb,u-boot-spl-dtb,u-boot-spl,blob-ext@1,blob-ext@2,blob-ext@3,blob-ext@4,main-section)
> > >
> > > The problem seems to be that you are asking binman to generate three
> > > independent images. U-Boot is in a FIT which is not in the same image
> > > as SPL. So it is not possible to locate the flash offset of U-Boot
> > > (with in the FIT).
> > >
> > > Can you give me a bit more info about your intent here? Is it to load
> > > U-Boot from the FIT? I so, I suppose it is possible to make binman
> > > access an independent image, if it is told where it starts.
> > >
> > > But why is everything not in one image?
> > >
> >
> > Simon,
> >
> > I would rather have 1 image. I was going off of the imx8mm_evk switch
> > to binman which creates the separate images.
>
> Well at present you are loading a FIT into RAM, I think? Is it coming
> from flash?
>
> If you load a FIT containing U-Boot then you don't need the binman
> symbol stuff, since SPL looks in the FIT for the location of U-Boot.
> There isn't much benefit in having binman point to U-Boot within the
> FIT, since we already have code to find it. It might save a few bytes
> of code, but it would be confusing...I'm not sure if that is worth the
> hassle.
>
> If you want a single image, then you might not want FIT at all...just
> use binman.
>
> It really depends what you want.

Maybe my terminology is all wrong or I'm not making myself clear. I'm
trying to access data inside the SPL binary in board_init_f() 'before'
the SPL has done anything at all with FIT.

I'm using FIT because I have multiple board models (ie multiple DTB's)
supported by a single U-Boot 'board'.

So my boot goes like this:
IMX8M BOOT ROM fetches flash.bin (SPL) from eMMC into OCRAM
SPL configures PMIC and DRAM based on runtime detection of board model
  - at this point in time SPL is using a generic imx8mm-venice.dts
that just supports i2c/uart2/emmc which are common to all venice
boards
  - pmic config is done without dm because we don't have the
board-specific dtb yet which defines the pmic
  - DRAM config is done based on eeprom bytes that specify the DRAM
size/density/etc
  - DRAM config includes loading the 'blobs' to the M4 CPU - these are
the blobs I want to locate in the SPL
SPL locates FIT and starts chugging through it (I don't claim to fully
understand this part)
  - board_fit_config_name_match() is called for each DTB found and I
return a success if the DTB matches the board model found via I2C
EEPROM
SPL loads ATF and executes it
ATF executes? U-Boot (not super clear on all of this either)
U-Boot Proper runs with the board-specific dtb (not imx8mm-venice.dtb
but imx8mm-venice-gwxxxx.dtb)

>
> >
> > The whole point of what I'm investigating here has to do with the SPL.
> > OCRAM is at a premium and the current way the IMX8M is handling DDR
> > firmware is to tack it on after the code in the SPL image and it gets
> > padded to make it easy to locate which is a huge waste of space. I
> > figured we can use binman to locate the blobs without the padding.
> >
> > So, if you take 'just' the spl image here:
> >         spl: 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.bin";
> >                         size = <0x8000>;
> >                 };
> >
> >                 blob_2: blob-ext@2 {
> >                         filename = "lpddr4_pmu_train_1d_dmem.bin";
> >                         size = <0x4000>;
> >                 };
> >
> >                 blob_3: blob-ext@3 {
> >                         filename = "lpddr4_pmu_train_2d_imem.bin";
> >                         size = <0x8000>;
> >                 };
> >
> >                 blob_4: blob-ext@4 {
> >                         filename = "lpddr4_pmu_train_2d_dmem.bin";
> >                         size = <0x4000>;
> >                 };
> >         };
> >
> > My intention is to remove the size arguments above which are currently
> > forcing wasted padding and locate the blobs at runtime with binman.
>
> Well you can just remove them.

Not right now because the imx8 dram config expects them to be
following the DDR code and specific sizes... its dumb code that ends
up wasiting 24K of SPL/OCRAM with padding which is why I want to
improve that.

see ddr_load_train_firmware
https://elixir.bootlin.com/u-boot/latest/source/drivers/ddr/imx/imx8m/helper.c#L29

>
> >
> > Based on your other patch it it would seem I'm missing something from
> > my lds to add __image_copy_start yet in
> > arch/arm/cpu/armv8/u-boot-spl.lds I see:
> >         .text : {
> >                 . = ALIGN(8);
> >                 *(.__image_copy_start)
> >                 CPUDIR/start.o (.text*)
> >                 *(.text*)
> >         } >.sram
> >
> > My understanding of linker files is pretty slim so perhaps there's
> > something missing above.
>
> Yes you need to define the value of the __image_copy_start symbol, so:
>
> .text: {
>     __image_copy_start = .;
>
> See for example arch/arm/cpu/u-boot-spl.lds
>

Honestly what I 'really' want to do is get the SPL to load all the
dram config/blobs from flash and completely move them out of the SPL
that gets loaded into OCRAM so that I don't overflow the OCRAM with
DRAM configs when we add new boards. So maybe I'll just start focusing
on that.

I was thinking FIT would be a good approach for that but I haven't dug
into how the SPL processes the FIT yet...if it requires DRAM to do so
then I can't really go that route and maybe it's just too complex for
what I want anyway.

Tim

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: using binman fails boot
  2021-07-23 22:51                   ` Tim Harvey
@ 2021-07-24 22:01                     ` Simon Glass
  2021-07-26 18:42                       ` Tim Harvey
  0 siblings, 1 reply; 14+ messages in thread
From: Simon Glass @ 2021-07-24 22:01 UTC (permalink / raw)
  To: Tim Harvey; +Cc: u-boot

Hi Tim,

On Fri, 23 Jul 2021 at 16:52, Tim Harvey <tharvey@gateworks.com> wrote:
>
> On Fri, Jul 23, 2021 at 2:41 PM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Tim,
> >
> > On Fri, 23 Jul 2021 at 15:06, Tim Harvey <tharvey@gateworks.com> wrote:
> > >
> > > On Thu, Jul 22, 2021 at 8:07 PM Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Tim,
> > > >
> > > > On Mon, 19 Jul 2021 at 17:23, Tim Harvey <tharvey@gateworks.com> wrote:
> > > > >
> > > > > On Sat, Jul 17, 2021 at 7:22 PM Simon Glass <sjg@chromium.org> wrote:
> > > > > >
> > > >
> > > > [..]
> > > >
> > > > > > > But isn't blob-ext@4 a correct name? I can't use 'blob-ext-4' as
> > > > > > > that's an unknown entry type.
> > > > > >
> > > > > > Well you can use any name and specify the type:
> > > > > >
> > > > > > my-name {
> > > > > >    type = "blob-ext";
> > > > > > };
> > > > > >
> > > > >
> > > > > Ok - I understand.
> > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > If you can push your tree somewhere (with this problem) I'll see if I
> > > > > > > > can figure out why.
> > > > > > > >
> > > > > > >
> > > > > > > Sure, I pushed it to
> > > > > > > https://github.com/Gateworks/uboot-venice/tree/WIP-venice-binman
> > > > > > > make imx8mm_venice_defconfig
> > > > > > > make
> > > > > >
> > > > > > OK
> > > > > >
> > > > > > >
> > > > > > > > >
> > > > > > > > > BINMAN_VERBOSE=4 indeed prints out a tone of stuff but I'm not seeing
> > > > > > > > > anything for 'blob' below that would seem to indicate one node name vs
> > > > > > > > > another:
> > > > > > > >
> > > > > > > > Oops you need BINMAN_VERBOSE=5 - see elf.py LookupAndWriteSymbols()
> > > > > > > > which has tout.Debug() which is level 5.
> > > > > > > >
> > > > > > >
> > > > > > > LookupAndWriteSymbols ends up doing nothing because
> > > > > > > syms.get('__image_copy_start') returns None.
> > > > > >
> > > > > > Well that is likely the problem.
> > > >
> > > > I sent a patch to make binman report this as an error.
> > > >
> > > > I pushed the resulting tree to:
> > > >
> > > > https://github.com/sjg20/u-boot/tree/try-tim
> > > >
> > > > Now the error is:
> > > >
> > > > binman: Section '/binman/u-boot-spl-ddr': Symbol
> > > > '_binman_u_boot_any_prop_image_pos'
> > > >
> > > >    in entry '/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-nodtb':
> > > > Entry 'u-boot-any' not found in list
> > > > (u-boot-spl-nodtb,u-boot-spl-dtb,u-boot-spl,blob-ext@1,blob-ext@2,blob-ext@3,blob-ext@4,main-section)
> > > >
> > > > The problem seems to be that you are asking binman to generate three
> > > > independent images. U-Boot is in a FIT which is not in the same image
> > > > as SPL. So it is not possible to locate the flash offset of U-Boot
> > > > (with in the FIT).
> > > >
> > > > Can you give me a bit more info about your intent here? Is it to load
> > > > U-Boot from the FIT? I so, I suppose it is possible to make binman
> > > > access an independent image, if it is told where it starts.
> > > >
> > > > But why is everything not in one image?
> > > >
> > >
> > > Simon,
> > >
> > > I would rather have 1 image. I was going off of the imx8mm_evk switch
> > > to binman which creates the separate images.
> >
> > Well at present you are loading a FIT into RAM, I think? Is it coming
> > from flash?
> >
> > If you load a FIT containing U-Boot then you don't need the binman
> > symbol stuff, since SPL looks in the FIT for the location of U-Boot.
> > There isn't much benefit in having binman point to U-Boot within the
> > FIT, since we already have code to find it. It might save a few bytes
> > of code, but it would be confusing...I'm not sure if that is worth the
> > hassle.
> >
> > If you want a single image, then you might not want FIT at all...just
> > use binman.
> >
> > It really depends what you want.
>
> Maybe my terminology is all wrong or I'm not making myself clear. I'm
> trying to access data inside the SPL binary in board_init_f() 'before'
> the SPL has done anything at all with FIT.
>
> I'm using FIT because I have multiple board models (ie multiple DTB's)
> supported by a single U-Boot 'board'.

OK I see.

Well in that case the problem is the use of
CONFIG_SPL_RAW_IMAGE_SUPPORT which causes spl_set_header_raw_uboot()
to try to find U-Boot's binman symbol, which doesn't exist.

Also the naming of your sections need a tweak, as mentioned.

I've pushed a new trree to:

https://github.com/sjg20/u-boot/tree/try-tim

>
> So my boot goes like this:
> IMX8M BOOT ROM fetches flash.bin (SPL) from eMMC into OCRAM
> SPL configures PMIC and DRAM based on runtime detection of board model
>   - at this point in time SPL is using a generic imx8mm-venice.dts
> that just supports i2c/uart2/emmc which are common to all venice
> boards
>   - pmic config is done without dm because we don't have the
> board-specific dtb yet which defines the pmic
>   - DRAM config is done based on eeprom bytes that specify the DRAM
> size/density/etc
>   - DRAM config includes loading the 'blobs' to the M4 CPU - these are
> the blobs I want to locate in the SPL
> SPL locates FIT and starts chugging through it (I don't claim to fully
> understand this part)
>   - board_fit_config_name_match() is called for each DTB found and I
> return a success if the DTB matches the board model found via I2C
> EEPROM
> SPL loads ATF and executes it
> ATF executes? U-Boot (not super clear on all of this either)
> U-Boot Proper runs with the board-specific dtb (not imx8mm-venice.dtb
> but imx8mm-venice-gwxxxx.dtb)

OK, got it.


>
> >
> > >
> > > The whole point of what I'm investigating here has to do with the SPL.
> > > OCRAM is at a premium and the current way the IMX8M is handling DDR
> > > firmware is to tack it on after the code in the SPL image and it gets
> > > padded to make it easy to locate which is a huge waste of space. I
> > > figured we can use binman to locate the blobs without the padding.
> > >
> > > So, if you take 'just' the spl image here:
> > >         spl: 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.bin";
> > >                         size = <0x8000>;
> > >                 };
> > >
> > >                 blob_2: blob-ext@2 {
> > >                         filename = "lpddr4_pmu_train_1d_dmem.bin";
> > >                         size = <0x4000>;
> > >                 };
> > >
> > >                 blob_3: blob-ext@3 {
> > >                         filename = "lpddr4_pmu_train_2d_imem.bin";
> > >                         size = <0x8000>;
> > >                 };
> > >
> > >                 blob_4: blob-ext@4 {
> > >                         filename = "lpddr4_pmu_train_2d_dmem.bin";
> > >                         size = <0x4000>;
> > >                 };
> > >         };
> > >
> > > My intention is to remove the size arguments above which are currently
> > > forcing wasted padding and locate the blobs at runtime with binman.
> >
> > Well you can just remove them.
>
> Not right now because the imx8 dram config expects them to be
> following the DDR code and specific sizes... its dumb code that ends
> up wasiting 24K of SPL/OCRAM with padding which is why I want to
> improve that.
>
> see ddr_load_train_firmware
> https://elixir.bootlin.com/u-boot/latest/source/drivers/ddr/imx/imx8m/helper.c#L29

OK well if you can update that code to read the size from a binman
symbol, perhaps that will help.

>
> >
> > >
> > > Based on your other patch it it would seem I'm missing something from
> > > my lds to add __image_copy_start yet in
> > > arch/arm/cpu/armv8/u-boot-spl.lds I see:
> > >         .text : {
> > >                 . = ALIGN(8);
> > >                 *(.__image_copy_start)
> > >                 CPUDIR/start.o (.text*)
> > >                 *(.text*)
> > >         } >.sram
> > >
> > > My understanding of linker files is pretty slim so perhaps there's
> > > something missing above.
> >
> > Yes you need to define the value of the __image_copy_start symbol, so:
> >
> > .text: {
> >     __image_copy_start = .;
> >
> > See for example arch/arm/cpu/u-boot-spl.lds
> >
>
> Honestly what I 'really' want to do is get the SPL to load all the
> dram config/blobs from flash and completely move them out of the SPL
> that gets loaded into OCRAM so that I don't overflow the OCRAM with
> DRAM configs when we add new boards. So maybe I'll just start focusing
> on that.
>
> I was thinking FIT would be a good approach for that but I haven't dug
> into how the SPL processes the FIT yet...if it requires DRAM to do so
> then I can't really go that route and maybe it's just too complex for
> what I want anyway.

FIT is fine, but I suppose it is also possible to use binman for
everything. But we do encourage FIT since it is a standard boot
method.

Regards,
Simon

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: using binman fails boot
  2021-07-24 22:01                     ` Simon Glass
@ 2021-07-26 18:42                       ` Tim Harvey
  2021-07-28  2:46                         ` Simon Glass
  0 siblings, 1 reply; 14+ messages in thread
From: Tim Harvey @ 2021-07-26 18:42 UTC (permalink / raw)
  To: Simon Glass; +Cc: u-boot

On Sat, Jul 24, 2021 at 3:01 PM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Tim,
>
> On Fri, 23 Jul 2021 at 16:52, Tim Harvey <tharvey@gateworks.com> wrote:
> >
> > On Fri, Jul 23, 2021 at 2:41 PM Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Tim,
> > >
> > > On Fri, 23 Jul 2021 at 15:06, Tim Harvey <tharvey@gateworks.com> wrote:
> > > >
> > > > On Thu, Jul 22, 2021 at 8:07 PM Simon Glass <sjg@chromium.org> wrote:
> > > > >
> > > > > Hi Tim,
> > > > >
> > > > > On Mon, 19 Jul 2021 at 17:23, Tim Harvey <tharvey@gateworks.com> wrote:
> > > > > >
> > > > > > On Sat, Jul 17, 2021 at 7:22 PM Simon Glass <sjg@chromium.org> wrote:
> > > > > > >
> > > > >
> > > > > [..]
> > > > >
> > > > > > > > But isn't blob-ext@4 a correct name? I can't use 'blob-ext-4' as
> > > > > > > > that's an unknown entry type.
> > > > > > >
> > > > > > > Well you can use any name and specify the type:
> > > > > > >
> > > > > > > my-name {
> > > > > > >    type = "blob-ext";
> > > > > > > };
> > > > > > >
> > > > > >
> > > > > > Ok - I understand.
> > > > > >
> > > > > > > >
> > > > > > > > >
> > > > > > > > > If you can push your tree somewhere (with this problem) I'll see if I
> > > > > > > > > can figure out why.
> > > > > > > > >
> > > > > > > >
> > > > > > > > Sure, I pushed it to
> > > > > > > > https://github.com/Gateworks/uboot-venice/tree/WIP-venice-binman
> > > > > > > > make imx8mm_venice_defconfig
> > > > > > > > make
> > > > > > >
> > > > > > > OK
> > > > > > >
> > > > > > > >
> > > > > > > > > >
> > > > > > > > > > BINMAN_VERBOSE=4 indeed prints out a tone of stuff but I'm not seeing
> > > > > > > > > > anything for 'blob' below that would seem to indicate one node name vs
> > > > > > > > > > another:
> > > > > > > > >
> > > > > > > > > Oops you need BINMAN_VERBOSE=5 - see elf.py LookupAndWriteSymbols()
> > > > > > > > > which has tout.Debug() which is level 5.
> > > > > > > > >
> > > > > > > >
> > > > > > > > LookupAndWriteSymbols ends up doing nothing because
> > > > > > > > syms.get('__image_copy_start') returns None.
> > > > > > >
> > > > > > > Well that is likely the problem.
> > > > >
> > > > > I sent a patch to make binman report this as an error.
> > > > >
> > > > > I pushed the resulting tree to:
> > > > >
> > > > > https://github.com/sjg20/u-boot/tree/try-tim
> > > > >
> > > > > Now the error is:
> > > > >
> > > > > binman: Section '/binman/u-boot-spl-ddr': Symbol
> > > > > '_binman_u_boot_any_prop_image_pos'
> > > > >
> > > > >    in entry '/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-nodtb':
> > > > > Entry 'u-boot-any' not found in list
> > > > > (u-boot-spl-nodtb,u-boot-spl-dtb,u-boot-spl,blob-ext@1,blob-ext@2,blob-ext@3,blob-ext@4,main-section)
> > > > >
> > > > > The problem seems to be that you are asking binman to generate three
> > > > > independent images. U-Boot is in a FIT which is not in the same image
> > > > > as SPL. So it is not possible to locate the flash offset of U-Boot
> > > > > (with in the FIT).
> > > > >
> > > > > Can you give me a bit more info about your intent here? Is it to load
> > > > > U-Boot from the FIT? I so, I suppose it is possible to make binman
> > > > > access an independent image, if it is told where it starts.
> > > > >
> > > > > But why is everything not in one image?
> > > > >
> > > >
> > > > Simon,
> > > >
> > > > I would rather have 1 image. I was going off of the imx8mm_evk switch
> > > > to binman which creates the separate images.
> > >
> > > Well at present you are loading a FIT into RAM, I think? Is it coming
> > > from flash?
> > >
> > > If you load a FIT containing U-Boot then you don't need the binman
> > > symbol stuff, since SPL looks in the FIT for the location of U-Boot.
> > > There isn't much benefit in having binman point to U-Boot within the
> > > FIT, since we already have code to find it. It might save a few bytes
> > > of code, but it would be confusing...I'm not sure if that is worth the
> > > hassle.
> > >
> > > If you want a single image, then you might not want FIT at all...just
> > > use binman.
> > >
> > > It really depends what you want.
> >
> > Maybe my terminology is all wrong or I'm not making myself clear. I'm
> > trying to access data inside the SPL binary in board_init_f() 'before'
> > the SPL has done anything at all with FIT.
> >
> > I'm using FIT because I have multiple board models (ie multiple DTB's)
> > supported by a single U-Boot 'board'.
>
> OK I see.
>
> Well in that case the problem is the use of
> CONFIG_SPL_RAW_IMAGE_SUPPORT which causes spl_set_header_raw_uboot()
> to try to find U-Boot's binman symbol, which doesn't exist.
>
> Also the naming of your sections need a tweak, as mentioned.
>
> I've pushed a new trree to:
>
> https://github.com/sjg20/u-boot/tree/try-tim

Simon,

Thanks - this appears to give me some real offsets:

U-Boot SPL 2021.07-00329-gb3d23cad33-dirty (Jul 26 2021 - 11:19:30 -0700)
board_init_f: blob_1:0x8038dc
board_init_f: blob_2:0x80b8dc
board_init_f: blob_3:0x80f8dc
board_init_f: blob_4:0x8178dc

CONFIG_SPL_TEXT_BASE=0x7e1000 so subtracting that from above matches
the offsets of the blobs in u-boot-spl-ddr.map

This should work nicely... I'll continue working on my end goal.

>
> >
> > So my boot goes like this:
> > IMX8M BOOT ROM fetches flash.bin (SPL) from eMMC into OCRAM
> > SPL configures PMIC and DRAM based on runtime detection of board model
> >   - at this point in time SPL is using a generic imx8mm-venice.dts
> > that just supports i2c/uart2/emmc which are common to all venice
> > boards
> >   - pmic config is done without dm because we don't have the
> > board-specific dtb yet which defines the pmic
> >   - DRAM config is done based on eeprom bytes that specify the DRAM
> > size/density/etc
> >   - DRAM config includes loading the 'blobs' to the M4 CPU - these are
> > the blobs I want to locate in the SPL
> > SPL locates FIT and starts chugging through it (I don't claim to fully
> > understand this part)
> >   - board_fit_config_name_match() is called for each DTB found and I
> > return a success if the DTB matches the board model found via I2C
> > EEPROM
> > SPL loads ATF and executes it
> > ATF executes? U-Boot (not super clear on all of this either)
> > U-Boot Proper runs with the board-specific dtb (not imx8mm-venice.dtb
> > but imx8mm-venice-gwxxxx.dtb)
>
> OK, got it.
>
>
> >
> > >
> > > >
> > > > The whole point of what I'm investigating here has to do with the SPL.
> > > > OCRAM is at a premium and the current way the IMX8M is handling DDR
> > > > firmware is to tack it on after the code in the SPL image and it gets
> > > > padded to make it easy to locate which is a huge waste of space. I
> > > > figured we can use binman to locate the blobs without the padding.
> > > >
> > > > So, if you take 'just' the spl image here:
> > > >         spl: 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.bin";
> > > >                         size = <0x8000>;
> > > >                 };
> > > >
> > > >                 blob_2: blob-ext@2 {
> > > >                         filename = "lpddr4_pmu_train_1d_dmem.bin";
> > > >                         size = <0x4000>;
> > > >                 };
> > > >
> > > >                 blob_3: blob-ext@3 {
> > > >                         filename = "lpddr4_pmu_train_2d_imem.bin";
> > > >                         size = <0x8000>;
> > > >                 };
> > > >
> > > >                 blob_4: blob-ext@4 {
> > > >                         filename = "lpddr4_pmu_train_2d_dmem.bin";
> > > >                         size = <0x4000>;
> > > >                 };
> > > >         };
> > > >
> > > > My intention is to remove the size arguments above which are currently
> > > > forcing wasted padding and locate the blobs at runtime with binman.
> > >
> > > Well you can just remove them.
> >
> > Not right now because the imx8 dram config expects them to be
> > following the DDR code and specific sizes... its dumb code that ends
> > up wasiting 24K of SPL/OCRAM with padding which is why I want to
> > improve that.
> >
> > see ddr_load_train_firmware
> > https://elixir.bootlin.com/u-boot/latest/source/drivers/ddr/imx/imx8m/helper.c#L29
>
> OK well if you can update that code to read the size from a binman
> symbol, perhaps that will help.
>
> >
> > >
> > > >
> > > > Based on your other patch it it would seem I'm missing something from
> > > > my lds to add __image_copy_start yet in
> > > > arch/arm/cpu/armv8/u-boot-spl.lds I see:
> > > >         .text : {
> > > >                 . = ALIGN(8);
> > > >                 *(.__image_copy_start)
> > > >                 CPUDIR/start.o (.text*)
> > > >                 *(.text*)
> > > >         } >.sram
> > > >
> > > > My understanding of linker files is pretty slim so perhaps there's
> > > > something missing above.
> > >
> > > Yes you need to define the value of the __image_copy_start symbol, so:
> > >
> > > .text: {
> > >     __image_copy_start = .;
> > >
> > > See for example arch/arm/cpu/u-boot-spl.lds
> > >
> >
> > Honestly what I 'really' want to do is get the SPL to load all the
> > dram config/blobs from flash and completely move them out of the SPL
> > that gets loaded into OCRAM so that I don't overflow the OCRAM with
> > DRAM configs when we add new boards. So maybe I'll just start focusing
> > on that.
> >
> > I was thinking FIT would be a good approach for that but I haven't dug
> > into how the SPL processes the FIT yet...if it requires DRAM to do so
> > then I can't really go that route and maybe it's just too complex for
> > what I want anyway.
>
> FIT is fine, but I suppose it is also possible to use binman for
> everything. But we do encourage FIT since it is a standard boot
> method.

The next step of what I was interested in was to move those blobs
outside of the spl binary completely and if I do that the binman
solution no longer works as the blobs would be outside of flash.bin so
I'm not sure how I could make use of binman there.

Tim

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: using binman fails boot
  2021-07-26 18:42                       ` Tim Harvey
@ 2021-07-28  2:46                         ` Simon Glass
  0 siblings, 0 replies; 14+ messages in thread
From: Simon Glass @ 2021-07-28  2:46 UTC (permalink / raw)
  To: Tim Harvey; +Cc: u-boot

Hi Tim,

On Mon, 26 Jul 2021 at 12:42, Tim Harvey <tharvey@gateworks.com> wrote:
>
> On Sat, Jul 24, 2021 at 3:01 PM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Tim,
> >
> > On Fri, 23 Jul 2021 at 16:52, Tim Harvey <tharvey@gateworks.com> wrote:
> > >
> > > On Fri, Jul 23, 2021 at 2:41 PM Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Tim,
> > > >
> > > > On Fri, 23 Jul 2021 at 15:06, Tim Harvey <tharvey@gateworks.com> wrote:
> > > > >
> > > > > On Thu, Jul 22, 2021 at 8:07 PM Simon Glass <sjg@chromium.org> wrote:
> > > > > >
> > > > > > Hi Tim,
> > > > > >
> > > > > > On Mon, 19 Jul 2021 at 17:23, Tim Harvey <tharvey@gateworks.com> wrote:
> > > > > > >
> > > > > > > On Sat, Jul 17, 2021 at 7:22 PM Simon Glass <sjg@chromium.org> wrote:
> > > > > > > >
> > > > > >
> > > > > > [..]
> > > > > >
> > > > > > > > > But isn't blob-ext@4 a correct name? I can't use 'blob-ext-4' as
> > > > > > > > > that's an unknown entry type.
> > > > > > > >
> > > > > > > > Well you can use any name and specify the type:
> > > > > > > >
> > > > > > > > my-name {
> > > > > > > >    type = "blob-ext";
> > > > > > > > };
> > > > > > > >
> > > > > > >
> > > > > > > Ok - I understand.
> > > > > > >
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > If you can push your tree somewhere (with this problem) I'll see if I
> > > > > > > > > > can figure out why.
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > Sure, I pushed it to
> > > > > > > > > https://github.com/Gateworks/uboot-venice/tree/WIP-venice-binman
> > > > > > > > > make imx8mm_venice_defconfig
> > > > > > > > > make
> > > > > > > >
> > > > > > > > OK
> > > > > > > >
> > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > BINMAN_VERBOSE=4 indeed prints out a tone of stuff but I'm not seeing
> > > > > > > > > > > anything for 'blob' below that would seem to indicate one node name vs
> > > > > > > > > > > another:
> > > > > > > > > >
> > > > > > > > > > Oops you need BINMAN_VERBOSE=5 - see elf.py LookupAndWriteSymbols()
> > > > > > > > > > which has tout.Debug() which is level 5.
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > LookupAndWriteSymbols ends up doing nothing because
> > > > > > > > > syms.get('__image_copy_start') returns None.
> > > > > > > >
> > > > > > > > Well that is likely the problem.
> > > > > >
> > > > > > I sent a patch to make binman report this as an error.
> > > > > >
> > > > > > I pushed the resulting tree to:
> > > > > >
> > > > > > https://github.com/sjg20/u-boot/tree/try-tim
> > > > > >
> > > > > > Now the error is:
> > > > > >
> > > > > > binman: Section '/binman/u-boot-spl-ddr': Symbol
> > > > > > '_binman_u_boot_any_prop_image_pos'
> > > > > >
> > > > > >    in entry '/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-nodtb':
> > > > > > Entry 'u-boot-any' not found in list
> > > > > > (u-boot-spl-nodtb,u-boot-spl-dtb,u-boot-spl,blob-ext@1,blob-ext@2,blob-ext@3,blob-ext@4,main-section)
> > > > > >
> > > > > > The problem seems to be that you are asking binman to generate three
> > > > > > independent images. U-Boot is in a FIT which is not in the same image
> > > > > > as SPL. So it is not possible to locate the flash offset of U-Boot
> > > > > > (with in the FIT).
> > > > > >
> > > > > > Can you give me a bit more info about your intent here? Is it to load
> > > > > > U-Boot from the FIT? I so, I suppose it is possible to make binman
> > > > > > access an independent image, if it is told where it starts.
> > > > > >
> > > > > > But why is everything not in one image?
> > > > > >
> > > > >
> > > > > Simon,
> > > > >
> > > > > I would rather have 1 image. I was going off of the imx8mm_evk switch
> > > > > to binman which creates the separate images.
> > > >
> > > > Well at present you are loading a FIT into RAM, I think? Is it coming
> > > > from flash?
> > > >
> > > > If you load a FIT containing U-Boot then you don't need the binman
> > > > symbol stuff, since SPL looks in the FIT for the location of U-Boot.
> > > > There isn't much benefit in having binman point to U-Boot within the
> > > > FIT, since we already have code to find it. It might save a few bytes
> > > > of code, but it would be confusing...I'm not sure if that is worth the
> > > > hassle.
> > > >
> > > > If you want a single image, then you might not want FIT at all...just
> > > > use binman.
> > > >
> > > > It really depends what you want.
> > >
> > > Maybe my terminology is all wrong or I'm not making myself clear. I'm
> > > trying to access data inside the SPL binary in board_init_f() 'before'
> > > the SPL has done anything at all with FIT.
> > >
> > > I'm using FIT because I have multiple board models (ie multiple DTB's)
> > > supported by a single U-Boot 'board'.
> >
> > OK I see.
> >
> > Well in that case the problem is the use of
> > CONFIG_SPL_RAW_IMAGE_SUPPORT which causes spl_set_header_raw_uboot()
> > to try to find U-Boot's binman symbol, which doesn't exist.
> >
> > Also the naming of your sections need a tweak, as mentioned.
> >
> > I've pushed a new trree to:
> >
> > https://github.com/sjg20/u-boot/tree/try-tim
>
> Simon,
>
> Thanks - this appears to give me some real offsets:
>
> U-Boot SPL 2021.07-00329-gb3d23cad33-dirty (Jul 26 2021 - 11:19:30 -0700)
> board_init_f: blob_1:0x8038dc
> board_init_f: blob_2:0x80b8dc
> board_init_f: blob_3:0x80f8dc
> board_init_f: blob_4:0x8178dc
>
> CONFIG_SPL_TEXT_BASE=0x7e1000 so subtracting that from above matches
> the offsets of the blobs in u-boot-spl-ddr.map
>
> This should work nicely... I'll continue working on my end goal.

OK good.

>
> >
> > >
> > > So my boot goes like this:
> > > IMX8M BOOT ROM fetches flash.bin (SPL) from eMMC into OCRAM
> > > SPL configures PMIC and DRAM based on runtime detection of board model
> > >   - at this point in time SPL is using a generic imx8mm-venice.dts
> > > that just supports i2c/uart2/emmc which are common to all venice
> > > boards
> > >   - pmic config is done without dm because we don't have the
> > > board-specific dtb yet which defines the pmic
> > >   - DRAM config is done based on eeprom bytes that specify the DRAM
> > > size/density/etc
> > >   - DRAM config includes loading the 'blobs' to the M4 CPU - these are
> > > the blobs I want to locate in the SPL
> > > SPL locates FIT and starts chugging through it (I don't claim to fully
> > > understand this part)
> > >   - board_fit_config_name_match() is called for each DTB found and I
> > > return a success if the DTB matches the board model found via I2C
> > > EEPROM
> > > SPL loads ATF and executes it
> > > ATF executes? U-Boot (not super clear on all of this either)
> > > U-Boot Proper runs with the board-specific dtb (not imx8mm-venice.dtb
> > > but imx8mm-venice-gwxxxx.dtb)
> >
> > OK, got it.
> >
> >
> > >
> > > >
> > > > >
> > > > > The whole point of what I'm investigating here has to do with the SPL.
> > > > > OCRAM is at a premium and the current way the IMX8M is handling DDR
> > > > > firmware is to tack it on after the code in the SPL image and it gets
> > > > > padded to make it easy to locate which is a huge waste of space. I
> > > > > figured we can use binman to locate the blobs without the padding.
> > > > >
> > > > > So, if you take 'just' the spl image here:
> > > > >         spl: 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.bin";
> > > > >                         size = <0x8000>;
> > > > >                 };
> > > > >
> > > > >                 blob_2: blob-ext@2 {
> > > > >                         filename = "lpddr4_pmu_train_1d_dmem.bin";
> > > > >                         size = <0x4000>;
> > > > >                 };
> > > > >
> > > > >                 blob_3: blob-ext@3 {
> > > > >                         filename = "lpddr4_pmu_train_2d_imem.bin";
> > > > >                         size = <0x8000>;
> > > > >                 };
> > > > >
> > > > >                 blob_4: blob-ext@4 {
> > > > >                         filename = "lpddr4_pmu_train_2d_dmem.bin";
> > > > >                         size = <0x4000>;
> > > > >                 };
> > > > >         };
> > > > >
> > > > > My intention is to remove the size arguments above which are currently
> > > > > forcing wasted padding and locate the blobs at runtime with binman.
> > > >
> > > > Well you can just remove them.
> > >
> > > Not right now because the imx8 dram config expects them to be
> > > following the DDR code and specific sizes... its dumb code that ends
> > > up wasiting 24K of SPL/OCRAM with padding which is why I want to
> > > improve that.
> > >
> > > see ddr_load_train_firmware
> > > https://elixir.bootlin.com/u-boot/latest/source/drivers/ddr/imx/imx8m/helper.c#L29
> >
> > OK well if you can update that code to read the size from a binman
> > symbol, perhaps that will help.
> >
> > >
> > > >
> > > > >
> > > > > Based on your other patch it it would seem I'm missing something from
> > > > > my lds to add __image_copy_start yet in
> > > > > arch/arm/cpu/armv8/u-boot-spl.lds I see:
> > > > >         .text : {
> > > > >                 . = ALIGN(8);
> > > > >                 *(.__image_copy_start)
> > > > >                 CPUDIR/start.o (.text*)
> > > > >                 *(.text*)
> > > > >         } >.sram
> > > > >
> > > > > My understanding of linker files is pretty slim so perhaps there's
> > > > > something missing above.
> > > >
> > > > Yes you need to define the value of the __image_copy_start symbol, so:
> > > >
> > > > .text: {
> > > >     __image_copy_start = .;
> > > >
> > > > See for example arch/arm/cpu/u-boot-spl.lds
> > > >
> > >
> > > Honestly what I 'really' want to do is get the SPL to load all the
> > > dram config/blobs from flash and completely move them out of the SPL
> > > that gets loaded into OCRAM so that I don't overflow the OCRAM with
> > > DRAM configs when we add new boards. So maybe I'll just start focusing
> > > on that.
> > >
> > > I was thinking FIT would be a good approach for that but I haven't dug
> > > into how the SPL processes the FIT yet...if it requires DRAM to do so
> > > then I can't really go that route and maybe it's just too complex for
> > > what I want anyway.
> >
> > FIT is fine, but I suppose it is also possible to use binman for
> > everything. But we do encourage FIT since it is a standard boot
> > method.
>
> The next step of what I was interested in was to move those blobs
> outside of the spl binary completely and if I do that the binman
> solution no longer works as the blobs would be outside of flash.bin so
> I'm not sure how I could make use of binman there.

Where are you planning to put them? Still in flash or on another
medium altogether? If the latter, binman cannot help you at present,
although I suppose we could add a way for it to point into other
images.  If the former, then can you put everything in one image?

Regards,
SImon

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2021-07-28  2:46 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-15 22:57 using binman fails boot Tim Harvey
2021-07-16  4:30 ` Simon Glass
2021-07-16 21:43   ` Tim Harvey
2021-07-16 22:11     ` Simon Glass
2021-07-16 23:15       ` Tim Harvey
2021-07-18  2:22         ` Simon Glass
2021-07-19 23:23           ` Tim Harvey
2021-07-23  3:07             ` Simon Glass
2021-07-23 21:06               ` Tim Harvey
2021-07-23 21:41                 ` Simon Glass
2021-07-23 22:51                   ` Tim Harvey
2021-07-24 22:01                     ` Simon Glass
2021-07-26 18:42                       ` Tim Harvey
2021-07-28  2:46                         ` Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.