On Sun, Nov 10, 2024 at 6:49 PM Marek Vasut <[email protected]> wrote:
>
> On 11/10/24 6:21 PM, Adam Ford wrote:
> > On Sun, Nov 10, 2024 at 10:42 AM Marek Vasut <[email protected]> wrote:
> >>
> >> On 11/10/24 2:15 PM, Adam Ford wrote:
> >>> On Sat, Nov 9, 2024 at 7:34 PM Marek Vasut <[email protected]> wrote:
> >>>>
> >>>> On 11/9/24 9:06 PM, Adam Ford wrote:
> >>>>> When FSPI_CONF_HEADER is set, the binary needs to be built such
> >>>>> that there is a configuration file located at 0x400 and the start
> >>>>> of the file that would normally be flash.bin starts at 0x1000.
> >>>>> This used to be done properly until the device tree was converted to
> >>>>> nxp_imx8mimage.
> >>>>>
> >>>>> Building these with the offsets built into the binman device tree
> >>>>> changes impacts how the actual image is built and the locations
> >>>>> of the various blobs aren't fetched properly and booting fails.
> >>>>>
> >>>>> Fix this by building a standard image as if it were to boot from
> >>>>> eMMC or SD, then use that image as the input for a second image
> >>>>
> >>>> This seems like a workaround for some broken offset calculation in
> >>>> binman ?
> >>>
> >>> This used to work until it was migrated to nxp_imx8mimage.
> >>> The blobs appear to be at the proper offsets, but the contents of
> >>> what's stored at those offsets are not the same.
> >>
> >> I know, this is what Lukasz reported too.
> >>
> >>> If you're going to claim there is a bug somewhere, I would argue that
> >>> it's somewhere i nxp_imx8mimage
> >>
> >> I agree with that claim. Well, by extension, the problem might also be
> >> in binman itself.
> >>
> >>> . However, if you look at this series,
> >>> the added benefit is the ability for Nano to be able to build both a
> >>> SD/eMMC image and FSPI images with one config which allows for the
> >>> elimination of extra defconfig files. I am guessing Plus would have a
> >>> similar benefit since they have similar bootloaders.
> >> This I do not agree with. If the intent is to generate two images, then
> >> there should be two full binman descriptors, one for each image (one for
> >> flash-plain.bin and one for flash-fspi.bin or some such naming).
> >>
> >> Can you try and fix the FSPI generation first, so an FSPI compatible
> >
> > I am not a python programmer, and I couldn't determine what was going on.
> I am hoping Simon could offer some input here ...
>
> Can you try the attached diff on MX8MM (use "git show -w" to view the
> diff better) ? It will generate two files, flash.bin and flash-fspi.bin
> , the later should have the fspi header and maybe even correct offsets?
I reset my branch to to U-Boot master from wedneday a7a96a37cbd8
"Merge https://source.denx.de/u-boot/custodians/u-boot-riscv";)
I verified the FCFB header is present. Unfortunately, when I burn the
FSPI on my 8MM and attempt to boot, nothing happens.
However, I changed the "nxp,boot-from" parameter to "fspi" and it booted!
U-Boot SPL 2025.01-rc1-00168-ga7a96a37cbd8-dirty (Nov 10 2024 - 19:27:21 -0600)
WDT: Started watchdog@30280000 with servicing every 1000ms (60s timeout)
SEC0: RNG instantiated
Trying to boot from NOR
<snip>
I looked at your patch, and noticed your FIXME. Once we get the code
working, we'll likely need a way to pass the header offset, because
it's different between Mini (0x0) and Nano / Plus (0x400).
I'd like to suggest we #iifndef the section filename where "flash.bin"
currently sits, and remove it if we are building for flexspi. This
way we get what you originally requested, which is a single binary.
I have attached my diff file, so you can see my proposal. I am happy
to test either Mini or Nano, but I am traveling this week starting
Wednesday afternoon (US Central time) until Sunday night, so I won't
be able to test in that window.
Let me know how/if you want to proceed.
Thanks for looking into this.
adam
>
> Applies on top of 56accc56b9aa ("bios_emulator: fix first argument of
> pci_{read,write}_config_* function calls") .
>
> I noticed that there is some dependency issue where fspi_header.bin may
> not be generated yet when binman is triggered -- that needs to be fixed.
> You can detect the failure by running 'hexdump -C flash-fspi.bin | head'
> , if there is no FCFB header then this failure occurred. The easiest way
> to work around this is to run 'rm flash.bin ; make flash.bin'. The real
> fix is likely a matter of some Makefile tweak.
diff --git a/arch/arm/dts/imx8mm-u-boot.dtsi b/arch/arm/dts/imx8mm-u-boot.dtsi
index d31bc82253..71501a5796 100644
--- a/arch/arm/dts/imx8mm-u-boot.dtsi
+++ b/arch/arm/dts/imx8mm-u-boot.dtsi
@@ -42,165 +42,170 @@
};
&binman {
+#ifdef CONFIG_FSPI_CONF_HEADER
filename = "flash.bin";
section {
- pad-byte = <0x00>;
-
-#ifdef CONFIG_FSPI_CONF_HEADER
fspi_conf_block {
filename = CONFIG_FSPI_CONF_FILE;
type = "blob-ext";
size = <0x1000>;
};
-#endif
-#ifdef CONFIG_IMX_HAB
- nxp-imx8mcst@0 {
- filename = "u-boot-spl-mkimage.signed.bin";
- nxp,loader-address = <CONFIG_SPL_TEXT_BASE>;
- nxp,unlock;
- args; /* Needed by mkimage etype superclass */
+ section {
#endif
-
- binman_imx_spl: nxp-imx8mimage {
- filename = "u-boot-spl-mkimage.bin";
- nxp,boot-from = "sd";
- nxp,rom-version = <1>;
- nxp,loader-address = <CONFIG_SPL_TEXT_BASE>;
- args; /* Needed by mkimage etype superclass */
-
- section {
- align = <4>;
- align-size = <4>;
- filename = "u-boot-spl-ddr.bin";
- pad-byte = <0xff>;
-
- u-boot-spl {
- align-end = <4>;
- filename = "u-boot-spl.bin";
- };
-
- ddr-1d-imem-fw {
- filename = "lpddr4_pmu_train_1d_imem.bin";
- align-end = <4>;
- type = "blob-ext";
- };
-
- ddr-1d-dmem-fw {
- filename = "lpddr4_pmu_train_1d_dmem.bin";
- align-end = <4>;
- type = "blob-ext";
- };
-
- ddr-2d-imem-fw {
- filename = "lpddr4_pmu_train_2d_imem.bin";
- align-end = <4>;
- type = "blob-ext";
- };
-
- ddr-2d-dmem-fw {
- filename = "lpddr4_pmu_train_2d_dmem.bin";
- align-end = <4>;
- type = "blob-ext";
- };
- };
- };
-#ifdef CONFIG_IMX_HAB
- };
-
- nxp-imx8mcst@1 {
- filename = "u-boot-fit.signed.bin";
- nxp,loader-address = <CONFIG_SPL_LOAD_FIT_ADDRESS>;
#ifdef CONFIG_FSPI_CONF_HEADER
- offset = <0x58C00>;
-#else
- offset = <0x57c00>;
+ filename = "flash.bin";
#endif
+ section {
+ pad-byte = <0x00>;
- args; /* Needed by mkimage etype superclass */
+#ifdef CONFIG_IMX_HAB
+ nxp-imx8mcst@0 {
+ filename = "u-boot-spl-mkimage.signed.bin";
+ nxp,loader-address = <CONFIG_SPL_TEXT_BASE>;
+ nxp,unlock;
+ args; /* Needed by mkimage etype superclass */
#endif
- binman_imx_fit: fit {
- description = "Configuration to load ATF before U-Boot";
- filename = "u-boot.itb";
-#ifndef CONFIG_IMX_HAB
- fit,external-offset = <CONFIG_FIT_EXTERNAL_OFFSET>;
-#endif
- fit,fdt-list = "of-list";
- #address-cells = <1>;
+ binman_imx_spl: nxp-imx8mimage {
+ filename = "u-boot-spl-mkimage.bin";
#ifdef CONFIG_FSPI_CONF_HEADER
- offset = <0x58C00>;
+ nxp,boot-from = "fspi";
#else
- offset = <0x57c00>;
+ nxp,boot-from = "sd";
#endif
-
- images {
- uboot {
- arch = "arm64";
- compression = "none";
- description = "U-Boot (64-bit)";
- load = <CONFIG_TEXT_BASE>;
- type = "standalone";
-
- uboot-blob {
- filename = "u-boot-nodtb.bin";
- type = "blob-ext";
+ nxp,rom-version = <1>;
+ nxp,loader-address = <CONFIG_SPL_TEXT_BASE>;
+ args; /* Needed by mkimage etype superclass */
+
+ section {
+ align = <4>;
+ align-size = <4>;
+ filename = "u-boot-spl-ddr.bin";
+ pad-byte = <0xff>;
+
+ u-boot-spl {
+ align-end = <4>;
+ filename = "u-boot-spl.bin";
+ };
+
+ ddr-1d-imem-fw {
+ filename = "lpddr4_pmu_train_1d_imem.bin";
+ align-end = <4>;
+ type = "blob-ext";
+ };
+
+ ddr-1d-dmem-fw {
+ filename = "lpddr4_pmu_train_1d_dmem.bin";
+ align-end = <4>;
+ type = "blob-ext";
+ };
+
+ ddr-2d-imem-fw {
+ filename = "lpddr4_pmu_train_2d_imem.bin";
+ align-end = <4>;
+ type = "blob-ext";
+ };
+
+ ddr-2d-dmem-fw {
+ filename = "lpddr4_pmu_train_2d_dmem.bin";
+ align-end = <4>;
+ type = "blob-ext";
+ };
};
};
+#ifdef CONFIG_IMX_HAB
+ };
-#ifndef CONFIG_ARMV8_PSCI
- atf {
- arch = "arm64";
- compression = "none";
- description = "ARM Trusted Firmware";
- entry = <0x920000>;
- load = <0x920000>;
- type = "firmware";
-
- atf-blob {
- filename = "bl31.bin";
- type = "atf-bl31";
- };
- };
+ nxp-imx8mcst@1 {
+ filename = "u-boot-fit.signed.bin";
+ nxp,loader-address = <CONFIG_SPL_LOAD_FIT_ADDRESS>;
+ offset = <0x57c00>;
+
+ args; /* Needed by mkimage etype superclass */
#endif
- binman_fip: fip {
- arch = "arm64";
- compression = "none";
- description = "Trusted Firmware FIP";
- load = <0x40310000>;
- type = "firmware";
- };
+ binman_imx_fit: fit {
+ description = "Configuration to load ATF before U-Boot";
+ filename = "u-boot.itb";
+#ifndef CONFIG_IMX_HAB
+ fit,external-offset = <CONFIG_FIT_EXTERNAL_OFFSET>;
+#endif
+ fit,fdt-list = "of-list";
+ #address-cells = <1>;
+ offset = <0x57c00>;
+
+ images {
+ uboot {
+ arch = "arm64";
+ compression = "none";
+ description = "U-Boot (64-bit)";
+ load = <CONFIG_TEXT_BASE>;
+ type = "standalone";
+
+ uboot-blob {
+ filename = "u-boot-nodtb.bin";
+ type = "blob-ext";
+ };
+ };
- @fdt-SEQ {
- compression = "none";
- description = "NAME";
- type = "flat_dt";
+#ifndef CONFIG_ARMV8_PSCI
+ atf {
+ arch = "arm64";
+ compression = "none";
+ description = "ARM Trusted Firmware";
+ entry = <0x920000>;
+ load = <0x920000>;
+ type = "firmware";
+
+ atf-blob {
+ filename = "bl31.bin";
+ type = "atf-bl31";
+ };
+ };
+#endif
- uboot-fdt-blob {
- filename = "u-boot.dtb";
- type = "blob-ext";
+ binman_fip: fip {
+ arch = "arm64";
+ compression = "none";
+ description = "Trusted Firmware FIP";
+ load = <0x40310000>;
+ type = "firmware";
+ };
+
+ @fdt-SEQ {
+ compression = "none";
+ description = "NAME";
+ type = "flat_dt";
+
+ uboot-fdt-blob {
+ filename = "u-boot.dtb";
+ type = "blob-ext";
+ };
+ };
};
- };
- };
- configurations {
- default = "@config-DEFAULT-SEQ";
+ configurations {
+ default = "@config-DEFAULT-SEQ";
- @config-SEQ {
- description = "NAME";
- fdt = "fdt-SEQ";
- firmware = "uboot";
+ @config-SEQ {
+ description = "NAME";
+ fdt = "fdt-SEQ";
+ firmware = "uboot";
#ifndef CONFIG_ARMV8_PSCI
- loadables = "atf";
+ loadables = "atf";
#endif
+ };
+ };
};
+#ifdef CONFIG_IMX_HAB
};
+#endif
};
-#ifdef CONFIG_IMX_HAB
+#ifdef CONFIG_FSPI_CONF_HEADER
};
-#endif
};
+#endif
};
&clk {
diff --git a/tools/binman/etype/nxp_imx8mimage.py b/tools/binman/etype/nxp_imx8mimage.py
index 8ad177b3b6..e57da68079 100644
--- a/tools/binman/etype/nxp_imx8mimage.py
+++ b/tools/binman/etype/nxp_imx8mimage.py
@@ -72,4 +72,6 @@ class Entry_nxp_imx8mimage(Entry_mkimage):
return
upto += entry.size
+ # FIXME: Maybe ?
+ image_pos = 0
Entry_section.SetImagePos(self, image_pos)
