* [PATCH] arm64: zynqmp: Use CONFIG_SPL_FS_LOAD_PAYLOAD_NAME in binman @ 2025-03-19 13:08 Michal Simek 2025-03-25 9:48 ` Quentin Schulz 0 siblings, 1 reply; 5+ messages in thread From: Michal Simek @ 2025-03-19 13:08 UTC (permalink / raw) To: u-boot, git; +Cc: Prasad Kummari, Tom Rini u-boot.itb name is coming via CONFIG_SPL_FS_LOAD_PAYLOAD_NAME and it's change will affect SD boot mode that's why start to use it. Signed-off-by: Michal Simek <michal.simek@amd.com> --- arch/arm/dts/zynqmp-binman-som.dts | 11 ++++++----- arch/arm/dts/zynqmp-binman.dts | 8 ++++---- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/arch/arm/dts/zynqmp-binman-som.dts b/arch/arm/dts/zynqmp-binman-som.dts index d5b63ef604ba..26a5a634a9c8 100644 --- a/arch/arm/dts/zynqmp-binman-som.dts +++ b/arch/arm/dts/zynqmp-binman-som.dts @@ -14,7 +14,7 @@ binman: binman { multiple-images; -#ifdef CONFIG_SPL +#if defined(CONFIG_SPL) fit-dtb.blob { filename = "fit-dtb.blob"; pad-byte = <0>; @@ -102,10 +102,10 @@ }; }; }; - - /* u-boot.itb generation in a static way */ +#if defined(CONFIG_SPL_FS_LOAD_PAYLOAD_NAME) + /* Generation in a static way */ itb { - filename = "u-boot.itb"; + filename = CONFIG_SPL_FS_LOAD_PAYLOAD_NAME; pad-byte = <0>; fit { @@ -227,12 +227,13 @@ }; blob-ext@2 { offset = <CONFIG_SYS_SPI_U_BOOT_OFFS>; - filename = "u-boot.itb"; + filename = CONFIG_SPL_FS_LOAD_PAYLOAD_NAME; }; fdtmap { }; }; #endif +#endif #endif }; }; diff --git a/arch/arm/dts/zynqmp-binman.dts b/arch/arm/dts/zynqmp-binman.dts index 252c2ad552b0..d2ccd9d32ec6 100644 --- a/arch/arm/dts/zynqmp-binman.dts +++ b/arch/arm/dts/zynqmp-binman.dts @@ -14,10 +14,10 @@ binman: binman { multiple-images; -#ifdef CONFIG_SPL - /* u-boot.itb generation in a static way */ +#if defined(CONFIG_SPL) && defined(CONFIG_SPL_FS_LOAD_PAYLOAD_NAME) + /* Generation in a static way */ itb { - filename = "u-boot.itb"; + filename = CONFIG_SPL_FS_LOAD_PAYLOAD_NAME; pad-byte = <0>; fit { @@ -196,7 +196,7 @@ }; blob-ext@2 { offset = <CONFIG_SYS_SPI_U_BOOT_OFFS>; - filename = "u-boot.itb"; + filename = CONFIG_SPL_FS_LOAD_PAYLOAD_NAME; }; fdtmap { }; -- 2.43.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] arm64: zynqmp: Use CONFIG_SPL_FS_LOAD_PAYLOAD_NAME in binman 2025-03-19 13:08 [PATCH] arm64: zynqmp: Use CONFIG_SPL_FS_LOAD_PAYLOAD_NAME in binman Michal Simek @ 2025-03-25 9:48 ` Quentin Schulz 2025-03-27 9:11 ` Michal Simek 0 siblings, 1 reply; 5+ messages in thread From: Quentin Schulz @ 2025-03-25 9:48 UTC (permalink / raw) To: Michal Simek, u-boot, git; +Cc: Prasad Kummari, Tom Rini Hi Michal, On 3/19/25 2:08 PM, Michal Simek wrote: > u-boot.itb name is coming via CONFIG_SPL_FS_LOAD_PAYLOAD_NAME and it's They are unrelated as far as I can tell? Isn't CONFIG_SPL_FS_LOAD_PAYLOAD_NAME the name of the U-Boot FIT that the SPL is going to load from some filesystem? u-boot.itb in the binman nodes below is the name of the generated U-Boot FIT on your disk. Your disk FS != SPL FS, no? I can get behind wanting the same name for the file on both FS, but the issue I have is the additional CONFIG_SPL_FS_LOAD_PAYLOAD_NAME #ifdef will prevent the generation of this U-Boot FIT image if the symbol isn't defined anymore (which is possible if SPL has no support for FS (ext4, fat, squashfs, semihosting)). U-Boot likely work just fine by trying to load u-boot.itb from another source no? (No experience with AMD, typically only Aarch64 for me, where u-boot.itb is usually loaded from a specific offset in the storage medium, outside of any FS). Basically, do we really need to NOT build U-Boot FIT if SPL_FS_LOAD_PAYLOAD_NAME is not defined? Cheers, Quentin ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] arm64: zynqmp: Use CONFIG_SPL_FS_LOAD_PAYLOAD_NAME in binman 2025-03-25 9:48 ` Quentin Schulz @ 2025-03-27 9:11 ` Michal Simek 2025-03-28 15:08 ` Quentin Schulz 0 siblings, 1 reply; 5+ messages in thread From: Michal Simek @ 2025-03-27 9:11 UTC (permalink / raw) To: Quentin Schulz, u-boot, git; +Cc: Prasad Kummari, Tom Rini Hi, On 3/25/25 10:48, Quentin Schulz wrote: > Hi Michal, > > On 3/19/25 2:08 PM, Michal Simek wrote: >> u-boot.itb name is coming via CONFIG_SPL_FS_LOAD_PAYLOAD_NAME and it's > > They are unrelated as far as I can tell? > > Isn't CONFIG_SPL_FS_LOAD_PAYLOAD_NAME the name of the U-Boot FIT that the SPL is > going to load from some filesystem? yes. > > u-boot.itb in the binman nodes below is the name of the generated U-Boot FIT on > your disk. > > Your disk FS != SPL FS, no? > > I can get behind wanting the same name for the file on both FS, but the issue I > have is the additional CONFIG_SPL_FS_LOAD_PAYLOAD_NAME #ifdef will prevent the > generation of this U-Boot FIT image if the symbol isn't defined anymore (which > is possible if SPL has no support for FS (ext4, fat, squashfs, semihosting)). U- > Boot likely work just fine by trying to load u-boot.itb from another source no? > (No experience with AMD, typically only Aarch64 for me, where u-boot.itb is > usually loaded from a specific offset in the storage medium, outside of any FS). > > Basically, do we really need to NOT build U-Boot FIT if SPL_FS_LOAD_PAYLOAD_NAME > is not defined? There are couple of things here. AMD/Xilinx evaluation boards are capable to boot from multiple devices. On ZynqMP we have one official supported flow which is FSBL or alternative SPL. In FSBL boot mode FIT image is not handled and boot image contains just u-boot.elf and that's it and no matter which boot mode you use. In SPL boot mode boot image (boot.bin) has only SPL and that's it. Then FIT image should be created to be able to start TF-A, U-Boot, etc. Then based on boot device there are in general two options which are raw mode (for example SPI) and then filesystem one (SD or EMMC). In RAW mode qspi image is created (maybe we can rename it because it can be also used for NAND for example) and image is expected at certain offset and name obviously doesn't matter. But in filesystem mode names matter and image should match CONFIG_SPL_FS_LOAD_PAYLOAD_NAME because that's the filename which SPL is looking for that's why these has to match. And if I go back when FSBL is used there is no reason to generate raw boot image as qspi.bin or fit image as u-boot.itb because all binaries are the part of boot.bin already. Please let me know if you have any other question. Thanks, Michal ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] arm64: zynqmp: Use CONFIG_SPL_FS_LOAD_PAYLOAD_NAME in binman 2025-03-27 9:11 ` Michal Simek @ 2025-03-28 15:08 ` Quentin Schulz 2025-03-28 15:19 ` Michal Simek 0 siblings, 1 reply; 5+ messages in thread From: Quentin Schulz @ 2025-03-28 15:08 UTC (permalink / raw) To: Michal Simek, u-boot, git; +Cc: Prasad Kummari, Tom Rini Hi Michal, On 3/27/25 10:11 AM, Michal Simek wrote: > Hi, > > On 3/25/25 10:48, Quentin Schulz wrote: >> Hi Michal, >> >> On 3/19/25 2:08 PM, Michal Simek wrote: >>> u-boot.itb name is coming via CONFIG_SPL_FS_LOAD_PAYLOAD_NAME and it's >> >> They are unrelated as far as I can tell? >> >> Isn't CONFIG_SPL_FS_LOAD_PAYLOAD_NAME the name of the U-Boot FIT that >> the SPL is going to load from some filesystem? > > yes. > >> >> u-boot.itb in the binman nodes below is the name of the generated U- >> Boot FIT on your disk. >> >> Your disk FS != SPL FS, no? >> >> I can get behind wanting the same name for the file on both FS, but >> the issue I have is the additional CONFIG_SPL_FS_LOAD_PAYLOAD_NAME >> #ifdef will prevent the generation of this U-Boot FIT image if the >> symbol isn't defined anymore (which is possible if SPL has no support >> for FS (ext4, fat, squashfs, semihosting)). U- Boot likely work just >> fine by trying to load u-boot.itb from another source no? (No >> experience with AMD, typically only Aarch64 for me, where u-boot.itb >> is usually loaded from a specific offset in the storage medium, >> outside of any FS). >> >> Basically, do we really need to NOT build U-Boot FIT if >> SPL_FS_LOAD_PAYLOAD_NAME is not defined? > > There are couple of things here. AMD/Xilinx evaluation boards are > capable to boot from multiple devices. > > On ZynqMP we have one official supported flow which is FSBL or > alternative SPL. > > In FSBL boot mode FIT image is not handled and boot image contains just > u-boot.elf and that's it and no matter which boot mode you use. > > In SPL boot mode boot image (boot.bin) has only SPL and that's it. Then > FIT image should be created to be able to start TF-A, U-Boot, etc. > > Then based on boot device there are in general two options which are raw > mode (for example SPI) and then filesystem one (SD or EMMC). > So no raw mode for anything that is not SPI? E.g. no raw mode on SD or eMMC? > In RAW mode qspi image is created (maybe we can rename it because it can > be also used for NAND for example) and image is expected at certain > offset and name obviously doesn't matter. > But you still need a u-boot.itb to be available during build (c.f. /binman/image/blob-ext@2 which points at u-boot.itb). Isn't this u-boot.itb generated by the /binman/itb image node? > But in filesystem mode names matter and image should match > CONFIG_SPL_FS_LOAD_PAYLOAD_NAME because that's the filename which SPL is > looking for that's why these has to match. > That's fine. I'm arguing about the fact that you are not only renaming the u-boot.itb to the expected filename on the filesystem but also entirely skipping building it if the filename is not provided, which is not what's done today. Essentially, I'm wondering if you don't want: """ diff --git a/arch/arm/dts/zynqmp-binman-som.dts b/arch/arm/dts/zynqmp-binman-som.dts index d5b63ef604b..e73f196611f 100644 --- a/arch/arm/dts/zynqmp-binman-som.dts +++ b/arch/arm/dts/zynqmp-binman-som.dts @@ -9,6 +9,12 @@ #include <config.h> +#if defined(CONFIG_SPL_FS_LOAD_PAYLOAD_NAME) +#define U_BOOT_ITB_FILENAME CONFIG_SPL_FS_LOAD_PAYLOAD_NAME +#else +#define U_BOOT_ITB_FILENAME "u-boot.itb" +#endif + /dts-v1/; / { binman: binman { @@ -105,7 +111,7 @@ /* u-boot.itb generation in a static way */ itb { - filename = "u-boot.itb"; + filename = U_BOOT_ITB_FILENAME; pad-byte = <0>; fit { @@ -227,7 +233,7 @@ }; blob-ext@2 { offset = <CONFIG_SYS_SPI_U_BOOT_OFFS>; - filename = "u-boot.itb"; + filename = U_BOOT_ITB_FILENAME; }; fdtmap { }; diff --git a/arch/arm/dts/zynqmp-binman.dts b/arch/arm/dts/zynqmp-binman.dts index 252c2ad552b..89dfbf4e936 100644 --- a/arch/arm/dts/zynqmp-binman.dts +++ b/arch/arm/dts/zynqmp-binman.dts @@ -9,6 +9,12 @@ #include <config.h> +#if defined(CONFIG_SPL_FS_LOAD_PAYLOAD_NAME) +#define U_BOOT_ITB_FILENAME CONFIG_SPL_FS_LOAD_PAYLOAD_NAME +#else +#define U_BOOT_ITB_FILENAME "u-boot.itb" +#endif + /dts-v1/; / { binman: binman { @@ -17,7 +23,7 @@ #ifdef CONFIG_SPL /* u-boot.itb generation in a static way */ itb { - filename = "u-boot.itb"; + filename = CONFIG_SPL_FS_LOAD_PAYLOAD_NAME; pad-byte = <0>; fit { @@ -196,7 +202,7 @@ }; blob-ext@2 { offset = <CONFIG_SYS_SPI_U_BOOT_OFFS>; - filename = "u-boot.itb"; + filename = U_BOOT_ITB_FILENAME; }; fdtmap { }; """ Cheers, Quentin ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] arm64: zynqmp: Use CONFIG_SPL_FS_LOAD_PAYLOAD_NAME in binman 2025-03-28 15:08 ` Quentin Schulz @ 2025-03-28 15:19 ` Michal Simek 0 siblings, 0 replies; 5+ messages in thread From: Michal Simek @ 2025-03-28 15:19 UTC (permalink / raw) To: Quentin Schulz, u-boot, git; +Cc: Prasad Kummari, Tom Rini On 3/28/25 16:08, Quentin Schulz wrote: > Hi Michal, > > On 3/27/25 10:11 AM, Michal Simek wrote: >> Hi, >> >> On 3/25/25 10:48, Quentin Schulz wrote: >>> Hi Michal, >>> >>> On 3/19/25 2:08 PM, Michal Simek wrote: >>>> u-boot.itb name is coming via CONFIG_SPL_FS_LOAD_PAYLOAD_NAME and it's >>> >>> They are unrelated as far as I can tell? >>> >>> Isn't CONFIG_SPL_FS_LOAD_PAYLOAD_NAME the name of the U-Boot FIT that the SPL >>> is going to load from some filesystem? >> >> yes. >> >>> >>> u-boot.itb in the binman nodes below is the name of the generated U- Boot FIT >>> on your disk. >>> >>> Your disk FS != SPL FS, no? >>> >>> I can get behind wanting the same name for the file on both FS, but the issue >>> I have is the additional CONFIG_SPL_FS_LOAD_PAYLOAD_NAME #ifdef will prevent >>> the generation of this U-Boot FIT image if the symbol isn't defined anymore >>> (which is possible if SPL has no support for FS (ext4, fat, squashfs, >>> semihosting)). U- Boot likely work just fine by trying to load u-boot.itb >>> from another source no? (No experience with AMD, typically only Aarch64 for >>> me, where u-boot.itb is usually loaded from a specific offset in the storage >>> medium, outside of any FS). >>> >>> Basically, do we really need to NOT build U-Boot FIT if >>> SPL_FS_LOAD_PAYLOAD_NAME is not defined? >> >> There are couple of things here. AMD/Xilinx evaluation boards are capable to >> boot from multiple devices. >> >> On ZynqMP we have one official supported flow which is FSBL or alternative SPL. >> >> In FSBL boot mode FIT image is not handled and boot image contains just u- >> boot.elf and that's it and no matter which boot mode you use. >> >> In SPL boot mode boot image (boot.bin) has only SPL and that's it. Then FIT >> image should be created to be able to start TF-A, U-Boot, etc. >> >> Then based on boot device there are in general two options which are raw mode >> (for example SPI) and then filesystem one (SD or EMMC). >> > > So no raw mode for anything that is not SPI? E.g. no raw mode on SD or eMMC? There is also NAND raw but it never gets popular. SD/EMMC no raw boot mode on ZynqMP. (Versal and never has it). > >> In RAW mode qspi image is created (maybe we can rename it because it can be >> also used for NAND for example) and image is expected at certain offset and >> name obviously doesn't matter. >> > > But you still need a u-boot.itb to be available during build (c.f. /binman/ > image/blob-ext@2 which points at u-boot.itb). Isn't this u-boot.itb generated by > the /binman/itb image node? it is and the patch is addressing it. > >> But in filesystem mode names matter and image should match >> CONFIG_SPL_FS_LOAD_PAYLOAD_NAME because that's the filename which SPL is >> looking for that's why these has to match. >> > > That's fine. I'm arguing about the fact that you are not only renaming the u- > boot.itb to the expected filename on the filesystem but also entirely skipping > building it if the filename is not provided, which is not what's done today. > > Essentially, I'm wondering if you don't want: > > """ > diff --git a/arch/arm/dts/zynqmp-binman-som.dts b/arch/arm/dts/zynqmp-binman- > som.dts > index d5b63ef604b..e73f196611f 100644 > --- a/arch/arm/dts/zynqmp-binman-som.dts > +++ b/arch/arm/dts/zynqmp-binman-som.dts > @@ -9,6 +9,12 @@ > > #include <config.h> > > +#if defined(CONFIG_SPL_FS_LOAD_PAYLOAD_NAME) > +#define U_BOOT_ITB_FILENAME CONFIG_SPL_FS_LOAD_PAYLOAD_NAME > +#else > +#define U_BOOT_ITB_FILENAME "u-boot.itb" > +#endif > + > /dts-v1/; > / { > binman: binman { > @@ -105,7 +111,7 @@ > > /* u-boot.itb generation in a static way */ > itb { > - filename = "u-boot.itb"; > + filename = U_BOOT_ITB_FILENAME; > pad-byte = <0>; > > fit { > @@ -227,7 +233,7 @@ > }; > blob-ext@2 { > offset = <CONFIG_SYS_SPI_U_BOOT_OFFS>; > - filename = "u-boot.itb"; > + filename = U_BOOT_ITB_FILENAME; > }; > fdtmap { > }; > diff --git a/arch/arm/dts/zynqmp-binman.dts b/arch/arm/dts/zynqmp-binman.dts > index 252c2ad552b..89dfbf4e936 100644 > --- a/arch/arm/dts/zynqmp-binman.dts > +++ b/arch/arm/dts/zynqmp-binman.dts > @@ -9,6 +9,12 @@ > > #include <config.h> > > +#if defined(CONFIG_SPL_FS_LOAD_PAYLOAD_NAME) > +#define U_BOOT_ITB_FILENAME CONFIG_SPL_FS_LOAD_PAYLOAD_NAME > +#else > +#define U_BOOT_ITB_FILENAME "u-boot.itb" > +#endif > + > /dts-v1/; > / { > binman: binman { > @@ -17,7 +23,7 @@ > #ifdef CONFIG_SPL > /* u-boot.itb generation in a static way */ > itb { > - filename = "u-boot.itb"; > + filename = CONFIG_SPL_FS_LOAD_PAYLOAD_NAME; > pad-byte = <0>; > > fit { > @@ -196,7 +202,7 @@ > }; > blob-ext@2 { > offset = <CONFIG_SYS_SPI_U_BOOT_OFFS>; > - filename = "u-boot.itb"; > + filename = U_BOOT_ITB_FILENAME; > }; > fdtmap { > }; > """ You are right that when FS functionality is disabled there is no reason not to build images for qspi which are not affected. I will send v2 to address it. Thanks for review, Michal ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-03-28 15:19 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-03-19 13:08 [PATCH] arm64: zynqmp: Use CONFIG_SPL_FS_LOAD_PAYLOAD_NAME in binman Michal Simek 2025-03-25 9:48 ` Quentin Schulz 2025-03-27 9:11 ` Michal Simek 2025-03-28 15:08 ` Quentin Schulz 2025-03-28 15:19 ` Michal Simek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox