[PATCH V4 0/8] arm64: binman: use binman symbols for imx

From: Peng Fan peng.fan@nxp.com
V4: Fix three boards build failure
V3: Add R-b/T-b Fix build warning
V2: resolve some CI failure include patch 7
binman symbol is a good feature, but only used on X86 for now. This patchset is to use it for i.MX8M platform.
The current imx8m ddr phy firmware consumes lots of space, because we pad them to the largest 32KB and 16KB for IMEM and DMEM.
With this patchset we use binman symbols to get firmware location and size, we could save near 36KB with i.MX8MP-EVK.
Please help check and test
Peng Fan (8): spl: guard u_boot_any with X86 arm: dts: imx8m: update binman ddr firmware node name imx: imx8mm-icore: migrate to use BINMAN armv8: u-boot-spl.lds: mark __image_copy_start as symbol tools: binman: section: replace @ with - ddr: imx8m: helper: load ddr firmware according to binman symbols arm: dts: imx8m: shrink ddr firmware size to actual file size binman_sym: guard with CONFIG_SPL_BINMAN_SYMBOLS
arch/arm/cpu/armv8/u-boot-spl.lds | 2 +- arch/arm/dts/imx8mm-u-boot.dtsi | 16 +++--- arch/arm/dts/imx8mn-beacon-kit-u-boot.dtsi | 8 +-- .../dts/imx8mn-bsh-smm-s2-u-boot-common.dtsi | 4 +- arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi | 8 +-- arch/arm/dts/imx8mn-evk-u-boot.dtsi | 8 +-- .../dts/imx8mn-var-som-symphony-u-boot.dtsi | 16 +++--- arch/arm/dts/imx8mn-venice-u-boot.dtsi | 16 +++--- arch/arm/dts/imx8mp-u-boot.dtsi | 8 +-- arch/arm/dts/imx8mq-cm-u-boot.dtsi | 8 +-- arch/arm/dts/imx8mq-u-boot.dtsi | 16 +++--- arch/arm/mach-imx/imx8m/Kconfig | 1 + .../mach-imx/imx8m/imximage-8mm-lpddr4.cfg | 10 +--- common/spl/spl.c | 8 ++- common/spl/spl_ram.c | 4 ++ configs/imx8mm-icore-mx8mm-ctouch2_defconfig | 2 +- configs/imx8mm-icore-mx8mm-edimm2.2_defconfig | 2 +- drivers/ddr/imx/imx8m/helper.c | 51 ++++++++++++++++--- include/binman_sym.h | 2 +- tools/binman/etype/section.py | 2 +- tools/binman/test/u_boot_binman_syms.c | 1 + tools/binman/test/u_boot_binman_syms_size.c | 1 + 22 files changed, 116 insertions(+), 78 deletions(-)

From: Peng Fan peng.fan@nxp.com
set the symbol as weak not work if LTO is enabled. Since u_boot_any is only used on X86 for now, so guard it with X86, otherwise build break if we use BINMAN_SYMBOLS on i.MX.
Tested-by: Tim Harvey tharvey@gateworks.com #imx8m[m,n,p]-venice Signed-off-by: Peng Fan peng.fan@nxp.com --- common/spl/spl.c | 8 ++++++-- common/spl/spl_ram.c | 4 ++++ 2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/common/spl/spl.c b/common/spl/spl.c index c8c463f80bd..4b28180467a 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -50,7 +50,7 @@ DECLARE_GLOBAL_DATA_PTR;
u32 *boot_params_ptr = NULL;
-#if CONFIG_IS_ENABLED(BINMAN_SYMBOLS) +#if CONFIG_IS_ENABLED(BINMAN_SYMBOLS) && CONFIG_IS_ENABLED(X86) /* See spl.h for information about this */ binman_sym_declare(ulong, u_boot_any, image_pos); binman_sym_declare(ulong, u_boot_any, size); @@ -148,7 +148,7 @@ void spl_fixup_fdt(void *fdt_blob) #endif }
-#if CONFIG_IS_ENABLED(BINMAN_SYMBOLS) +#if CONFIG_IS_ENABLED(BINMAN_SYMBOLS) && CONFIG_IS_ENABLED(X86) ulong spl_get_image_pos(void) { #ifdef CONFIG_VPL @@ -221,7 +221,11 @@ __weak struct image_header *spl_get_load_buffer(ssize_t offset, size_t size)
void spl_set_header_raw_uboot(struct spl_image_info *spl_image) { +#if CONFIG_IS_ENABLED(X86) ulong u_boot_pos = binman_sym(ulong, u_boot_any, image_pos); +#else + ulong u_boot_pos = BINMAN_SYM_MISSING; +#endif
spl_image->size = CONFIG_SYS_MONITOR_LEN;
diff --git a/common/spl/spl_ram.c b/common/spl/spl_ram.c index 82964592571..083b14102ee 100644 --- a/common/spl/spl_ram.c +++ b/common/spl/spl_ram.c @@ -70,7 +70,11 @@ static int spl_ram_load_image(struct spl_image_info *spl_image, load.read = spl_ram_load_read; spl_load_simple_fit(spl_image, &load, 0, header); } else { +#if CONFIG_IS_ENABLED(X86) ulong u_boot_pos = binman_sym(ulong, u_boot_any, image_pos); +#else + ulong u_boot_pos = BINMAN_SYM_MISSING; +#endif
debug("Legacy image\n"); /*

On Fri, May 20, 2022 at 10:10:40PM +0800, Peng Fan (OSS) wrote:
From: Peng Fan peng.fan@nxp.com
set the symbol as weak not work if LTO is enabled. Since u_boot_any is only used on X86 for now, so guard it with X86, otherwise build break if we use BINMAN_SYMBOLS on i.MX.
Tested-by: Tim Harvey tharvey@gateworks.com #imx8m[m,n,p]-venice Signed-off-by: Peng Fan peng.fan@nxp.com
common/spl/spl.c | 8 ++++++-- common/spl/spl_ram.c | 4 ++++ 2 files changed, 10 insertions(+), 2 deletions(-)
I think we long term need to figure this out and address it so LTO works. But for now can you please guard this with a test on LTO instead, so it's clear where the problem is?

Subject: Re: [PATCH V4 1/8] spl: guard u_boot_any with X86
On Fri, May 20, 2022 at 10:10:40PM +0800, Peng Fan (OSS) wrote:
From: Peng Fan peng.fan@nxp.com
set the symbol as weak not work if LTO is enabled. Since u_boot_any is only used on X86 for now, so guard it with X86, otherwise build break if we use BINMAN_SYMBOLS on i.MX.
Tested-by: Tim Harvey tharvey@gateworks.com #imx8m[m,n,p]-venice Signed-off-by: Peng Fan peng.fan@nxp.com
common/spl/spl.c | 8 ++++++-- common/spl/spl_ram.c | 4 ++++ 2 files changed, 10 insertions(+), 2 deletions(-)
I think we long term need to figure this out and address it so LTO works. But for now can you please guard this with a test on LTO instead, so it's clear where the problem is?
Sorry, I could not get your point about guard with a test on LTO.
Actually binman weak symbol will report a warning log if there is no u_boot_any binman symbol. Since only X86 use it, I guard with X86.
Thanks, Peng.
-- Tom

On Sat, May 21, 2022 at 08:33:56AM +0000, Peng Fan wrote:
Subject: Re: [PATCH V4 1/8] spl: guard u_boot_any with X86
On Fri, May 20, 2022 at 10:10:40PM +0800, Peng Fan (OSS) wrote:
From: Peng Fan peng.fan@nxp.com
set the symbol as weak not work if LTO is enabled. Since u_boot_any is only used on X86 for now, so guard it with X86, otherwise build break if we use BINMAN_SYMBOLS on i.MX.
Tested-by: Tim Harvey tharvey@gateworks.com #imx8m[m,n,p]-venice Signed-off-by: Peng Fan peng.fan@nxp.com
common/spl/spl.c | 8 ++++++-- common/spl/spl_ram.c | 4 ++++ 2 files changed, 10 insertions(+), 2 deletions(-)
I think we long term need to figure this out and address it so LTO works. But for now can you please guard this with a test on LTO instead, so it's clear where the problem is?
Sorry, I could not get your point about guard with a test on LTO.
Actually binman weak symbol will report a warning log if there is no u_boot_any binman symbol. Since only X86 use it, I guard with X86.
Why are you mentioning LTO in the commit message? When I read the commit message it sounds like you're saying the problem is that LTO doesn't like how this symbol is handled, but if LTO was disabled, everything would be fine. If it's not LTO-related, please re-word the message instead.

On 21/05/2022 15:05, Tom Rini wrote:
On Sat, May 21, 2022 at 08:33:56AM +0000, Peng Fan wrote:
Subject: Re: [PATCH V4 1/8] spl: guard u_boot_any with X86
On Fri, May 20, 2022 at 10:10:40PM +0800, Peng Fan (OSS) wrote:
From: Peng Fan peng.fan@nxp.com
set the symbol as weak not work if LTO is enabled. Since u_boot_any is only used on X86 for now, so guard it with X86, otherwise build break if we use BINMAN_SYMBOLS on i.MX.
Tested-by: Tim Harvey tharvey@gateworks.com #imx8m[m,n,p]-venice Signed-off-by: Peng Fan peng.fan@nxp.com
common/spl/spl.c | 8 ++++++-- common/spl/spl_ram.c | 4 ++++ 2 files changed, 10 insertions(+), 2 deletions(-)
I think we long term need to figure this out and address it so LTO works. But for now can you please guard this with a test on LTO instead, so it's clear where the problem is?
Sorry, I could not get your point about guard with a test on LTO.
Actually binman weak symbol will report a warning log if there is no u_boot_any binman symbol. Since only X86 use it, I guard with X86.
Why are you mentioning LTO in the commit message? When I read the commit message it sounds like you're saying the problem is that LTO doesn't like how this symbol is handled, but if LTO was disabled, everything would be fine. If it's not LTO-related, please re-word the message instead.
It looks like we should be able to change things in common/spl/spl.c to:
#if CONFIG_IS_ENABLED(BINMAN_SYMBOLS) /* See spl.h for information about this */ binman_sym_declare_optional(ulong, u_boot_any, image_pos); binman_sym_declare_optional(ulong, u_boot_any, size); #endif
which would mark the symbol as 'weak' and turn the error into a warning on the binman side. But that is somehow being undone by LTO.
I'm trying to build for imx8mm-beacon with that change instead of this patch. With CONFIG_LTO=y, build fails and spl/u-boot-spl.sym has:
00000000007fbe28 l O .binman_sym 0000000000000008 _binman_u_boot_any_prop_image_pos
Looks like the size symbol is optimized out. With CONFIG_LTO unset, the build succeeds and the same file has:
00000000007fe90c w O .binman_sym 0000000000000008 _binman_u_boot_any_prop_image_pos 00000000007fe904 w O .binman_sym 0000000000000008 _binman_u_boot_any_prop_size
I don't know much about linking stuff, so this is as deep as I could dig...

On Sun, May 22, 2022 at 04:56:08PM +0300, Alper Nebi Yasak wrote:
On 21/05/2022 15:05, Tom Rini wrote:
On Sat, May 21, 2022 at 08:33:56AM +0000, Peng Fan wrote:
Subject: Re: [PATCH V4 1/8] spl: guard u_boot_any with X86
On Fri, May 20, 2022 at 10:10:40PM +0800, Peng Fan (OSS) wrote:
From: Peng Fan peng.fan@nxp.com
set the symbol as weak not work if LTO is enabled. Since u_boot_any is only used on X86 for now, so guard it with X86, otherwise build break if we use BINMAN_SYMBOLS on i.MX.
Tested-by: Tim Harvey tharvey@gateworks.com #imx8m[m,n,p]-venice Signed-off-by: Peng Fan peng.fan@nxp.com
common/spl/spl.c | 8 ++++++-- common/spl/spl_ram.c | 4 ++++ 2 files changed, 10 insertions(+), 2 deletions(-)
I think we long term need to figure this out and address it so LTO works. But for now can you please guard this with a test on LTO instead, so it's clear where the problem is?
Sorry, I could not get your point about guard with a test on LTO.
Actually binman weak symbol will report a warning log if there is no u_boot_any binman symbol. Since only X86 use it, I guard with X86.
Why are you mentioning LTO in the commit message? When I read the commit message it sounds like you're saying the problem is that LTO doesn't like how this symbol is handled, but if LTO was disabled, everything would be fine. If it's not LTO-related, please re-word the message instead.
It looks like we should be able to change things in common/spl/spl.c to:
#if CONFIG_IS_ENABLED(BINMAN_SYMBOLS) /* See spl.h for information about this */ binman_sym_declare_optional(ulong, u_boot_any, image_pos); binman_sym_declare_optional(ulong, u_boot_any, size); #endif
which would mark the symbol as 'weak' and turn the error into a warning on the binman side. But that is somehow being undone by LTO.
So, looking at binman_sym_declare_optional we tell the linker that it's weak and might even be unused. LTO gets very aggressive about finding things that aren't used in the resulting binary and discarding them. Typically we have the problem of a function that is used but it's hard for LTO to see it, so we give it the "used" attribute. But for something we're already saying is "unused" this would be wrong. So why do we mark things as unused here? I assume it's marked weak as it could be overridden at link time by a definition elsewhere.

On 22/05/2022 17:50, Tom Rini wrote:
On Sun, May 22, 2022 at 04:56:08PM +0300, Alper Nebi Yasak wrote:
It looks like we should be able to change things in common/spl/spl.c to:
#if CONFIG_IS_ENABLED(BINMAN_SYMBOLS) /* See spl.h for information about this */ binman_sym_declare_optional(ulong, u_boot_any, image_pos); binman_sym_declare_optional(ulong, u_boot_any, size); #endif
which would mark the symbol as 'weak' and turn the error into a warning on the binman side. But that is somehow being undone by LTO.
So, looking at binman_sym_declare_optional we tell the linker that it's weak and might even be unused. LTO gets very aggressive about finding things that aren't used in the resulting binary and discarding them. Typically we have the problem of a function that is used but it's hard for LTO to see it, so we give it the "used" attribute. But for something we're already saying is "unused" this would be wrong. So why do we mark things as unused here? I assume it's marked weak as it could be overridden at link time by a definition elsewhere.
I don't know, but the GCC manual says marking things 'unused' will suppress a warning if the variable is actually unused. I guess binman symbols may become unused due to some other config options being unset, and it's done for those cases?
I think the optional symbols are marked weak just to signal the python side that they are optional. If I understand it right, binman:
1. Packs an image using the finalized 'spl/u-boot-spl.bin' 2. Figures out the image-pos and size values of every entry 3. Inspects the 'spl/u-boot-spl' ELF file for declared binman symbols 4. Raises an error if any non-'weak' symbol's value is undetermined 5. Updates the u-boot-spl.bin (in memory) with the calculated values 6. Recreates the final image based on that updated bin file.

Subject: Re: [PATCH V4 1/8] spl: guard u_boot_any with X86
On 21/05/2022 15:05, Tom Rini wrote:
On Sat, May 21, 2022 at 08:33:56AM +0000, Peng Fan wrote:
Subject: Re: [PATCH V4 1/8] spl: guard u_boot_any with X86
On Fri, May 20, 2022 at 10:10:40PM +0800, Peng Fan (OSS) wrote:
From: Peng Fan peng.fan@nxp.com
set the symbol as weak not work if LTO is enabled. Since u_boot_any is only used on X86 for now, so guard it with X86, otherwise build break if we use BINMAN_SYMBOLS on i.MX.
Tested-by: Tim Harvey tharvey@gateworks.com #imx8m[m,n,p]-venice Signed-off-by: Peng Fan peng.fan@nxp.com
common/spl/spl.c | 8 ++++++-- common/spl/spl_ram.c | 4 ++++ 2 files changed, 10 insertions(+), 2 deletions(-)
I think we long term need to figure this out and address it so LTO works. But for now can you please guard this with a test on LTO instead, so it's clear where the problem is?
Sorry, I could not get your point about guard with a test on LTO.
Actually binman weak symbol will report a warning log if there is no u_boot_any binman symbol. Since only X86 use it, I guard with X86.
Why are you mentioning LTO in the commit message? When I read the commit message it sounds like you're saying the problem is that LTO doesn't like how this symbol is handled, but if LTO was disabled, everything would be fine. If it's not LTO-related, please re-word the message instead.
It looks like we should be able to change things in common/spl/spl.c to:
#if CONFIG_IS_ENABLED(BINMAN_SYMBOLS) /* See spl.h for information about this */ binman_sym_declare_optional(ulong, u_boot_any, image_pos); binman_sym_declare_optional(ulong, u_boot_any, size); #endif
which would mark the symbol as 'weak' and turn the error into a warning on the binman side. But that is somehow being undone by LTO.
I'm trying to build for imx8mm-beacon with that change instead of this patch. With CONFIG_LTO=y, build fails and spl/u-boot-spl.sym has:
00000000007fbe28 l O .binman_sym 0000000000000008
_binman_u_boot_any_prop_image_pos
Looks like the size symbol is optimized out. With CONFIG_LTO unset, the build succeeds and the same file has:
00000000007fe90c w O .binman_sym 0000000000000008
_binman_u_boot_any_prop_image_pos
00000000007fe904 w O .binman_sym 0000000000000008
_binman_u_boot_any_prop_size
I don't know much about linking stuff, so this is as deep as I could dig...
Thanks for the detailed sharing. Yes, this is the issue I try to work around. Some i.MX boards has LTO set, this leads me to use X86 as an extra guard.
Thanks, Peng.

Subject: Re: [PATCH V4 1/8] spl: guard u_boot_any with X86
On Sat, May 21, 2022 at 08:33:56AM +0000, Peng Fan wrote:
Subject: Re: [PATCH V4 1/8] spl: guard u_boot_any with X86
On Fri, May 20, 2022 at 10:10:40PM +0800, Peng Fan (OSS) wrote:
From: Peng Fan peng.fan@nxp.com
set the symbol as weak not work if LTO is enabled. Since u_boot_any is only used on X86 for now, so guard it with X86, otherwise build break if we use BINMAN_SYMBOLS on i.MX.
Tested-by: Tim Harvey tharvey@gateworks.com #imx8m[m,n,p]-venice Signed-off-by: Peng Fan peng.fan@nxp.com
common/spl/spl.c | 8 ++++++-- common/spl/spl_ram.c | 4 ++++ 2 files changed, 10 insertions(+), 2 deletions(-)
I think we long term need to figure this out and address it so LTO works. But for now can you please guard this with a test on LTO instead, so it's clear where the problem is?
Sorry, I could not get your point about guard with a test on LTO.
Actually binman weak symbol will report a warning log if there is no u_boot_any binman symbol. Since only X86 use it, I guard with X86.
Why are you mentioning LTO in the commit message? When I read the commit message it sounds like you're saying the problem is that LTO doesn't like how this symbol is handled, but if LTO was disabled, everything would be fine. If it's not LTO-related, please re-word the message instead.
Sorry, I could reword the commit message, but currently I have no better idea to address the issue unless use X86 as a guard in the code as this patch does. If you agree the code in this patch, I could reword commit msg and send v5.
Thanks, Peng.
-- Tom

On Mon, May 23, 2022 at 06:28:44AM +0000, Peng Fan (OSS) wrote:
Subject: Re: [PATCH V4 1/8] spl: guard u_boot_any with X86
On Sat, May 21, 2022 at 08:33:56AM +0000, Peng Fan wrote:
Subject: Re: [PATCH V4 1/8] spl: guard u_boot_any with X86
On Fri, May 20, 2022 at 10:10:40PM +0800, Peng Fan (OSS) wrote:
From: Peng Fan peng.fan@nxp.com
set the symbol as weak not work if LTO is enabled. Since u_boot_any is only used on X86 for now, so guard it with X86, otherwise build break if we use BINMAN_SYMBOLS on i.MX.
Tested-by: Tim Harvey tharvey@gateworks.com #imx8m[m,n,p]-venice Signed-off-by: Peng Fan peng.fan@nxp.com
common/spl/spl.c | 8 ++++++-- common/spl/spl_ram.c | 4 ++++ 2 files changed, 10 insertions(+), 2 deletions(-)
I think we long term need to figure this out and address it so LTO works. But for now can you please guard this with a test on LTO instead, so it's clear where the problem is?
Sorry, I could not get your point about guard with a test on LTO.
Actually binman weak symbol will report a warning log if there is no u_boot_any binman symbol. Since only X86 use it, I guard with X86.
Why are you mentioning LTO in the commit message? When I read the commit message it sounds like you're saying the problem is that LTO doesn't like how this symbol is handled, but if LTO was disabled, everything would be fine. If it's not LTO-related, please re-word the message instead.
Sorry, I could reword the commit message, but currently I have no better idea to address the issue unless use X86 as a guard in the code as this patch does. If you agree the code in this patch, I could reword commit msg and send v5.
Well, lets see what Alper says in the other part of the thread. I'd really like to solve this not work around this. But I'll take documenting the problem for the person that has X86 && LTO as good enough for the moment.

On 23/05/2022 17:10, Tom Rini wrote:
On Mon, May 23, 2022 at 06:28:44AM +0000, Peng Fan (OSS) wrote:
Subject: Re: [PATCH V4 1/8] spl: guard u_boot_any with X86
Why are you mentioning LTO in the commit message? When I read the commit message it sounds like you're saying the problem is that LTO doesn't like how this symbol is handled, but if LTO was disabled, everything would be fine. If it's not LTO-related, please re-word the message instead.
Sorry, I could reword the commit message, but currently I have no better idea to address the issue unless use X86 as a guard in the code as this patch does. If you agree the code in this patch, I could reword commit msg and send v5.
Well, lets see what Alper says in the other part of the thread. I'd really like to solve this not work around this. But I'll take documenting the problem for the person that has X86 && LTO as good enough for the moment.
Alternatively, I think we can decide that all binman symbols are optional, and not raise an error when we can't find a value for them. They are de-facto optional anyway: people can use the non-binman-image build artifacts (which binman doesn't update wrt. symbols), thus the code that uses binman symbols should assume their values can be missing.

On 20/05/2022 17:10, Peng Fan (OSS) wrote:
From: Peng Fan peng.fan@nxp.com
set the symbol as weak not work if LTO is enabled. Since u_boot_any is only used on X86 for now, so guard it with X86, otherwise build break if we use BINMAN_SYMBOLS on i.MX.
Tested-by: Tim Harvey tharvey@gateworks.com #imx8m[m,n,p]-venice Signed-off-by: Peng Fan peng.fan@nxp.com
common/spl/spl.c | 8 ++++++-- common/spl/spl_ram.c | 4 ++++ 2 files changed, 10 insertions(+), 2 deletions(-)
[...]
If I apply the series without this patch and try to build imx8mm-beacon, I get the following error, and I'm assuming that's what you're trying to solve here:
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 (...)
The immediate cause for this is that your 'u-boot-spl-ddr' image has no 'u-boot'-like entry, meaning there's no reasonable value to write into the non-optional 'u_boot_any' symbol.
(I think it might be OK to make this symbol optional, but LTO breaks that as you found out. The rest of this email is about my thoughts before I realized the LTO problem, but they're still relevant.)
It looks like binman images were designed to be monolithic instead of modular, and it's assumed you'd have one fully-specified image for your 'flash.bin' where 'u-boot-spl' would know about (for example) a 'u-boot' in a FIT entry. (I would like things to be modular eventually, though).
I tried to merge things into a single binman image, but mkimage is trying to read 'u-boot-spl-ddr.bin' which would no longer exist. I see this is because it's specified in 'spl/u-boot-spl.cfgout' as a LOADER file in a later patch.
Maybe 'imx8mimage.c' can be changed to use mkimage's -d argument as the LOADER instead? That's how binman passes the input data to mkimage, but that seems to be ignored in this case. Then we can merge things into a single image which will be able to set the 'u_boot_any' symbols without error.
Otherwise, the mkimage file reading error can be solved by creating a new binman 'imx8m-image' entry type that creates the 'cfgout' and calls mkimage with it. (Binman images would still need to be merged).

From: Peng Fan peng.fan@nxp.com
We are migrating to use BINMAN SYMBOLS, the current name is not a valid binman type, so update to use blob-ext@[1,2,3,4].
Tested-by: Tim Harvey tharvey@gateworks.com #imx8m[m,n,p]-venice Signed-off-by: Peng Fan peng.fan@nxp.com --- arch/arm/dts/imx8mm-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mn-bsh-smm-s2-u-boot-common.dtsi | 4 ++-- arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mn-venice-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mq-u-boot.dtsi | 8 ++++---- 5 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/arch/arm/dts/imx8mm-u-boot.dtsi b/arch/arm/dts/imx8mm-u-boot.dtsi index 9f66cdb65a9..5de55a2d80b 100644 --- a/arch/arm/dts/imx8mm-u-boot.dtsi +++ b/arch/arm/dts/imx8mm-u-boot.dtsi @@ -39,25 +39,25 @@ filename = "u-boot-spl.bin"; };
- 1d-imem { + imem_1d: blob-ext@1 { filename = "lpddr4_pmu_train_1d_imem.bin"; size = <0x8000>; type = "blob-ext"; };
- 1d-dmem { + dmem_1d: blob-ext@2 { filename = "lpddr4_pmu_train_1d_dmem.bin"; size = <0x4000>; type = "blob-ext"; };
- 2d-imem { + imem_2d: blob-ext@3 { filename = "lpddr4_pmu_train_2d_imem.bin"; size = <0x8000>; type = "blob-ext"; };
- 2d-dmem { + dmem_2d: blob-ext@4 { filename = "lpddr4_pmu_train_2d_dmem.bin"; size = <0x4000>; type = "blob-ext"; diff --git a/arch/arm/dts/imx8mn-bsh-smm-s2-u-boot-common.dtsi b/arch/arm/dts/imx8mn-bsh-smm-s2-u-boot-common.dtsi index 46a9d7fd78b..5a52b73d7e9 100644 --- a/arch/arm/dts/imx8mn-bsh-smm-s2-u-boot-common.dtsi +++ b/arch/arm/dts/imx8mn-bsh-smm-s2-u-boot-common.dtsi @@ -111,13 +111,13 @@ filename = "u-boot-spl.bin"; };
- 1d-imem { + imem_1d: blob-ext@1 { filename = "ddr3_imem_1d.bin"; size = <0x8000>; type = "blob-ext"; };
- 1d_dmem { + dmem_1d: blob-ext@2 { filename = "ddr3_dmem_1d.bin"; size = <0x4000>; type = "blob-ext"; diff --git a/arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi b/arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi index 6e37622cca7..001e725f568 100644 --- a/arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi +++ b/arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi @@ -130,25 +130,25 @@ filename = "u-boot-spl.bin"; };
- 1d-imem { + blob_1: blob-ext@1 { filename = "ddr4_imem_1d.bin"; size = <0x8000>; type = "blob-ext"; };
- 1d_dmem { + blob_2: blob-ext@2 { filename = "ddr4_dmem_1d.bin"; size = <0x4000>; type = "blob-ext"; };
- 2d_imem { + blob_3: blob-ext@3 { filename = "ddr4_imem_2d.bin"; size = <0x8000>; type = "blob-ext"; };
- 2d_dmem { + blob_4: blob-ext@4 { filename = "ddr4_dmem_2d.bin"; size = <0x4000>; type = "blob-ext"; diff --git a/arch/arm/dts/imx8mn-venice-u-boot.dtsi b/arch/arm/dts/imx8mn-venice-u-boot.dtsi index 35819553879..67922146963 100644 --- a/arch/arm/dts/imx8mn-venice-u-boot.dtsi +++ b/arch/arm/dts/imx8mn-venice-u-boot.dtsi @@ -126,25 +126,25 @@ filename = "u-boot-spl.bin"; };
- 1d-imem { + imem_1d: blob-ext@1 { filename = "lpddr4_pmu_train_1d_imem.bin"; size = <0x8000>; type = "blob-ext"; };
- 1d_dmem { + dmem_1d: blob-ext@2 { filename = "lpddr4_pmu_train_1d_dmem.bin"; size = <0x4000>; type = "blob-ext"; };
- 2d_imem { + imem_2d: blob-ext@3 { filename = "lpddr4_pmu_train_2d_imem.bin"; size = <0x8000>; type = "blob-ext"; };
- 2d_dmem { + dmem_2d: blob-ext@4 { filename = "lpddr4_pmu_train_2d_dmem.bin"; size = <0x4000>; type = "blob-ext"; diff --git a/arch/arm/dts/imx8mq-u-boot.dtsi b/arch/arm/dts/imx8mq-u-boot.dtsi index 912a3d4a356..389414ad26f 100644 --- a/arch/arm/dts/imx8mq-u-boot.dtsi +++ b/arch/arm/dts/imx8mq-u-boot.dtsi @@ -46,25 +46,25 @@ filename = "u-boot-spl.bin"; };
- 1d-imem { + imem_1d: blob-ext@1 { filename = "lpddr4_pmu_train_1d_imem.bin"; size = <0x8000>; type = "blob-ext"; };
- 1d-dmem { + dmem_1d: blob-ext@2 { filename = "lpddr4_pmu_train_1d_dmem.bin"; size = <0x4000>; type = "blob-ext"; };
- 2d-imem { + imem_2d: blob-ext@3 { filename = "lpddr4_pmu_train_2d_imem.bin"; size = <0x8000>; type = "blob-ext"; };
- 2d-dmem { + dmem_2d: blob-ext@4 { filename = "lpddr4_pmu_train_2d_dmem.bin"; size = <0x4000>; type = "blob-ext";

On 20/05/2022 17:10, Peng Fan (OSS) wrote:
From: Peng Fan peng.fan@nxp.com
We are migrating to use BINMAN SYMBOLS, the current name is not a valid binman type, so update to use blob-ext@[1,2,3,4].
Tested-by: Tim Harvey tharvey@gateworks.com #imx8m[m,n,p]-venice Signed-off-by: Peng Fan peng.fan@nxp.com
arch/arm/dts/imx8mm-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mn-bsh-smm-s2-u-boot-common.dtsi | 4 ++-- arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mn-venice-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mq-u-boot.dtsi | 8 ++++---- 5 files changed, 18 insertions(+), 18 deletions(-)
I think descriptive entry names are better in general, so I prefer the old names to a bunch of 'blob-ext's. Symbol resolution is done using the entry names anyway, I don't see why this change would be necessary.
In any case, the names and labels should be consistent across the different dts files, assuming the blobs have the same purpose / meaning. I see that labels starting with digits cause a dtc parse error, so the entries could be `imem_1d: imem-1d { ... };` and so on.
Also, the blobs essentially look the same to me aside from their filenames, just duplicated over different imx8m* variants and boards. Maybe binman parts could be unified into a single dtsi file later on? (Filename differences can be handled by new 'blob_named_by_arg's btw.)
diff --git a/arch/arm/dts/imx8mm-u-boot.dtsi b/arch/arm/dts/imx8mm-u-boot.dtsi index 9f66cdb65a9..5de55a2d80b 100644 --- a/arch/arm/dts/imx8mm-u-boot.dtsi +++ b/arch/arm/dts/imx8mm-u-boot.dtsi @@ -39,25 +39,25 @@ filename = "u-boot-spl.bin"; };
1d-imem {
};imem_1d: blob-ext@1 { filename = "lpddr4_pmu_train_1d_imem.bin"; size = <0x8000>; type = "blob-ext";
1d-dmem {
};dmem_1d: blob-ext@2 { filename = "lpddr4_pmu_train_1d_dmem.bin"; size = <0x4000>; type = "blob-ext";
2d-imem {
};imem_2d: blob-ext@3 { filename = "lpddr4_pmu_train_2d_imem.bin"; size = <0x8000>; type = "blob-ext";
2d-dmem {
dmem_2d: blob-ext@4 { filename = "lpddr4_pmu_train_2d_dmem.bin"; size = <0x4000>; type = "blob-ext";
diff --git a/arch/arm/dts/imx8mn-bsh-smm-s2-u-boot-common.dtsi b/arch/arm/dts/imx8mn-bsh-smm-s2-u-boot-common.dtsi index 46a9d7fd78b..5a52b73d7e9 100644 --- a/arch/arm/dts/imx8mn-bsh-smm-s2-u-boot-common.dtsi +++ b/arch/arm/dts/imx8mn-bsh-smm-s2-u-boot-common.dtsi @@ -111,13 +111,13 @@ filename = "u-boot-spl.bin"; };
1d-imem {
};imem_1d: blob-ext@1 { filename = "ddr3_imem_1d.bin"; size = <0x8000>; type = "blob-ext";
1d_dmem {
dmem_1d: blob-ext@2 { filename = "ddr3_dmem_1d.bin"; size = <0x4000>; type = "blob-ext";
diff --git a/arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi b/arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi index 6e37622cca7..001e725f568 100644 --- a/arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi +++ b/arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi @@ -130,25 +130,25 @@ filename = "u-boot-spl.bin"; };
1d-imem {
};blob_1: blob-ext@1 { filename = "ddr4_imem_1d.bin"; size = <0x8000>; type = "blob-ext";
1d_dmem {
};blob_2: blob-ext@2 { filename = "ddr4_dmem_1d.bin"; size = <0x4000>; type = "blob-ext";
2d_imem {
};blob_3: blob-ext@3 { filename = "ddr4_imem_2d.bin"; size = <0x8000>; type = "blob-ext";
2d_dmem {
blob_4: blob-ext@4 { filename = "ddr4_dmem_2d.bin"; size = <0x4000>; type = "blob-ext";
diff --git a/arch/arm/dts/imx8mn-venice-u-boot.dtsi b/arch/arm/dts/imx8mn-venice-u-boot.dtsi index 35819553879..67922146963 100644 --- a/arch/arm/dts/imx8mn-venice-u-boot.dtsi +++ b/arch/arm/dts/imx8mn-venice-u-boot.dtsi @@ -126,25 +126,25 @@ filename = "u-boot-spl.bin"; };
1d-imem {
};imem_1d: blob-ext@1 { filename = "lpddr4_pmu_train_1d_imem.bin"; size = <0x8000>; type = "blob-ext";
1d_dmem {
};dmem_1d: blob-ext@2 { filename = "lpddr4_pmu_train_1d_dmem.bin"; size = <0x4000>; type = "blob-ext";
2d_imem {
};imem_2d: blob-ext@3 { filename = "lpddr4_pmu_train_2d_imem.bin"; size = <0x8000>; type = "blob-ext";
2d_dmem {
dmem_2d: blob-ext@4 { filename = "lpddr4_pmu_train_2d_dmem.bin"; size = <0x4000>; type = "blob-ext";
diff --git a/arch/arm/dts/imx8mq-u-boot.dtsi b/arch/arm/dts/imx8mq-u-boot.dtsi index 912a3d4a356..389414ad26f 100644 --- a/arch/arm/dts/imx8mq-u-boot.dtsi +++ b/arch/arm/dts/imx8mq-u-boot.dtsi @@ -46,25 +46,25 @@ filename = "u-boot-spl.bin"; };
1d-imem {
};imem_1d: blob-ext@1 { filename = "lpddr4_pmu_train_1d_imem.bin"; size = <0x8000>; type = "blob-ext";
1d-dmem {
};dmem_1d: blob-ext@2 { filename = "lpddr4_pmu_train_1d_dmem.bin"; size = <0x4000>; type = "blob-ext";
2d-imem {
};imem_2d: blob-ext@3 { filename = "lpddr4_pmu_train_2d_imem.bin"; size = <0x8000>; type = "blob-ext";
2d-dmem {
dmem_2d: blob-ext@4 { filename = "lpddr4_pmu_train_2d_dmem.bin"; size = <0x4000>; type = "blob-ext";

Subject: Re: [PATCH V4 2/8] arm: dts: imx8m: update binman ddr firmware node name
On 20/05/2022 17:10, Peng Fan (OSS) wrote:
From: Peng Fan peng.fan@nxp.com
We are migrating to use BINMAN SYMBOLS, the current name is not a valid binman type, so update to use blob-ext@[1,2,3,4].
Tested-by: Tim Harvey tharvey@gateworks.com #imx8m[m,n,p]-venice Signed-off-by: Peng Fan peng.fan@nxp.com
arch/arm/dts/imx8mm-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mn-bsh-smm-s2-u-boot-common.dtsi | 4 ++-- arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mn-venice-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mq-u-boot.dtsi | 8 ++++---- 5 files changed, 18 insertions(+), 18 deletions(-)
I think descriptive entry names are better in general, so I prefer the old names to a bunch of 'blob-ext's. Symbol resolution is done using the entry names anyway, I don't see why this change would be necessary.
Binman understand blob-ext, it not understand 1d-imem. If keep id-imem, or else, need to add several new binman types which I would not prefer.
Thanks, Peng.
In any case, the names and labels should be consistent across the different dts files, assuming the blobs have the same purpose / meaning. I see that labels starting with digits cause a dtc parse error, so the entries could be `imem_1d: imem-1d { ... };` and so on.
Also, the blobs essentially look the same to me aside from their filenames, just duplicated over different imx8m* variants and boards. Maybe binman parts could be unified into a single dtsi file later on? (Filename differences can be handled by new 'blob_named_by_arg's btw.)
diff --git a/arch/arm/dts/imx8mm-u-boot.dtsi b/arch/arm/dts/imx8mm-u-boot.dtsi index 9f66cdb65a9..5de55a2d80b 100644 --- a/arch/arm/dts/imx8mm-u-boot.dtsi +++ b/arch/arm/dts/imx8mm-u-boot.dtsi @@ -39,25 +39,25 @@ filename = "u-boot-spl.bin"; };
1d-imem {
};imem_1d: blob-ext@1 { filename = "lpddr4_pmu_train_1d_imem.bin"; size = <0x8000>; type = "blob-ext";
1d-dmem {
};dmem_1d: blob-ext@2 { filename = "lpddr4_pmu_train_1d_dmem.bin"; size = <0x4000>; type = "blob-ext";
2d-imem {
};imem_2d: blob-ext@3 { filename = "lpddr4_pmu_train_2d_imem.bin"; size = <0x8000>; type = "blob-ext";
2d-dmem {
dmem_2d: blob-ext@4 { filename = "lpddr4_pmu_train_2d_dmem.bin"; size = <0x4000>; type = "blob-ext";
diff --git a/arch/arm/dts/imx8mn-bsh-smm-s2-u-boot-common.dtsi b/arch/arm/dts/imx8mn-bsh-smm-s2-u-boot-common.dtsi index 46a9d7fd78b..5a52b73d7e9 100644 --- a/arch/arm/dts/imx8mn-bsh-smm-s2-u-boot-common.dtsi +++ b/arch/arm/dts/imx8mn-bsh-smm-s2-u-boot-common.dtsi @@ -111,13 +111,13 @@ filename = "u-boot-spl.bin"; };
1d-imem {
};imem_1d: blob-ext@1 { filename = "ddr3_imem_1d.bin"; size = <0x8000>; type = "blob-ext";
1d_dmem {
dmem_1d: blob-ext@2 { filename = "ddr3_dmem_1d.bin"; size = <0x4000>; type = "blob-ext";
diff --git a/arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi b/arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi index 6e37622cca7..001e725f568 100644 --- a/arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi +++ b/arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi @@ -130,25 +130,25 @@ filename = "u-boot-spl.bin"; };
1d-imem {
};blob_1: blob-ext@1 { filename = "ddr4_imem_1d.bin"; size = <0x8000>; type = "blob-ext";
1d_dmem {
};blob_2: blob-ext@2 { filename = "ddr4_dmem_1d.bin"; size = <0x4000>; type = "blob-ext";
2d_imem {
};blob_3: blob-ext@3 { filename = "ddr4_imem_2d.bin"; size = <0x8000>; type = "blob-ext";
2d_dmem {
blob_4: blob-ext@4 { filename = "ddr4_dmem_2d.bin"; size = <0x4000>; type = "blob-ext";
diff --git a/arch/arm/dts/imx8mn-venice-u-boot.dtsi b/arch/arm/dts/imx8mn-venice-u-boot.dtsi index 35819553879..67922146963 100644 --- a/arch/arm/dts/imx8mn-venice-u-boot.dtsi +++ b/arch/arm/dts/imx8mn-venice-u-boot.dtsi @@ -126,25 +126,25 @@ filename = "u-boot-spl.bin"; };
1d-imem {
};imem_1d: blob-ext@1 { filename = "lpddr4_pmu_train_1d_imem.bin"; size = <0x8000>; type = "blob-ext";
1d_dmem {
};dmem_1d: blob-ext@2 { filename = "lpddr4_pmu_train_1d_dmem.bin"; size = <0x4000>; type = "blob-ext";
2d_imem {
};imem_2d: blob-ext@3 { filename = "lpddr4_pmu_train_2d_imem.bin"; size = <0x8000>; type = "blob-ext";
2d_dmem {
dmem_2d: blob-ext@4 { filename = "lpddr4_pmu_train_2d_dmem.bin"; size = <0x4000>; type = "blob-ext";
diff --git a/arch/arm/dts/imx8mq-u-boot.dtsi b/arch/arm/dts/imx8mq-u-boot.dtsi index 912a3d4a356..389414ad26f 100644 --- a/arch/arm/dts/imx8mq-u-boot.dtsi +++ b/arch/arm/dts/imx8mq-u-boot.dtsi @@ -46,25 +46,25 @@ filename = "u-boot-spl.bin"; };
1d-imem {
};imem_1d: blob-ext@1 { filename = "lpddr4_pmu_train_1d_imem.bin"; size = <0x8000>; type = "blob-ext";
1d-dmem {
};dmem_1d: blob-ext@2 { filename = "lpddr4_pmu_train_1d_dmem.bin"; size = <0x4000>; type = "blob-ext";
2d-imem {
};imem_2d: blob-ext@3 { filename = "lpddr4_pmu_train_2d_imem.bin"; size = <0x8000>; type = "blob-ext";
2d-dmem {
dmem_2d: blob-ext@4 { filename = "lpddr4_pmu_train_2d_dmem.bin"; size = <0x4000>; type = "blob-ext";

On 23/05/2022 10:01, Peng Fan (OSS) wrote:
Subject: Re: [PATCH V4 2/8] arm: dts: imx8m: update binman ddr firmware node name
On 20/05/2022 17:10, Peng Fan (OSS) wrote:
From: Peng Fan peng.fan@nxp.com
We are migrating to use BINMAN SYMBOLS, the current name is not a valid binman type, so update to use blob-ext@[1,2,3,4].
Tested-by: Tim Harvey tharvey@gateworks.com #imx8m[m,n,p]-venice Signed-off-by: Peng Fan peng.fan@nxp.com
arch/arm/dts/imx8mm-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mn-bsh-smm-s2-u-boot-common.dtsi | 4 ++-- arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mn-venice-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mq-u-boot.dtsi | 8 ++++---- 5 files changed, 18 insertions(+), 18 deletions(-)
I think descriptive entry names are better in general, so I prefer the old names to a bunch of 'blob-ext's. Symbol resolution is done using the entry names anyway, I don't see why this change would be necessary.
Binman understand blob-ext, it not understand 1d-imem. If keep id-imem, or else, need to add several new binman types which I would not prefer.
It would work as long as you set `type = "blob-ext";` in the entry block.
Please test this diff on top of your series:
diff --git a/arch/arm/dts/imx8mm-u-boot.dtsi b/arch/arm/dts/imx8mm-u-boot.dtsi index 19a2da30f512..8c48678625d9 100644 --- a/arch/arm/dts/imx8mm-u-boot.dtsi +++ b/arch/arm/dts/imx8mm-u-boot.dtsi @@ -39,25 +39,25 @@ filename = "u-boot-spl.bin"; };
- imem_1d: blob-ext@1 { + ddr-1d-imem-fw { filename = "lpddr4_pmu_train_1d_imem.bin"; align-end = <4>; type = "blob-ext"; };
- dmem_1d: blob-ext@2 { + ddr-1d-dmem-fw { filename = "lpddr4_pmu_train_1d_dmem.bin"; align-end = <4>; type = "blob-ext"; };
- imem_2d: blob-ext@3 { + ddr-2d-imem-fw { filename = "lpddr4_pmu_train_2d_imem.bin"; align-end = <4>; type = "blob-ext"; };
- dmem_2d: blob-ext@4 { + ddr-2d-dmem-fw { filename = "lpddr4_pmu_train_2d_dmem.bin"; align-end = <4>; type = "blob-ext"; diff --git a/arch/arm/dts/imx8mn-beacon-kit-u-boot.dtsi b/arch/arm/dts/imx8mn-beacon-kit-u-boot.dtsi index 0f89c8f677fa..dc7afc05e8f9 100644 --- a/arch/arm/dts/imx8mn-beacon-kit-u-boot.dtsi +++ b/arch/arm/dts/imx8mn-beacon-kit-u-boot.dtsi @@ -143,24 +143,28 @@ align-end = <4>; };
- blob_1: blob-ext@1 { + ddr-1d-imem-fw { filename = "lpddr4_pmu_train_1d_imem.bin"; align-end = <4>; + type = "blob-ext"; };
- blob_2: blob-ext@2 { + ddr-1d-dmem-fw { filename = "lpddr4_pmu_train_1d_dmem.bin"; align-end = <4>; + type = "blob-ext"; };
- blob_3: blob-ext@3 { + ddr-2d-imem-fw { filename = "lpddr4_pmu_train_2d_imem.bin"; align-end = <4>; + type = "blob-ext"; };
- blob_4: blob-ext@4 { + ddr-2d-dmem-fw { filename = "lpddr4_pmu_train_2d_dmem.bin"; align-end = <4>; + type = "blob-ext"; }; };
diff --git a/arch/arm/dts/imx8mn-bsh-smm-s2-u-boot-common.dtsi b/arch/arm/dts/imx8mn-bsh-smm-s2-u-boot-common.dtsi index 5a52b73d7e9c..dc4cec250efa 100644 --- a/arch/arm/dts/imx8mn-bsh-smm-s2-u-boot-common.dtsi +++ b/arch/arm/dts/imx8mn-bsh-smm-s2-u-boot-common.dtsi @@ -111,13 +111,13 @@ filename = "u-boot-spl.bin"; };
- imem_1d: blob-ext@1 { + ddr-1d-imem-fw { filename = "ddr3_imem_1d.bin"; size = <0x8000>; type = "blob-ext"; };
- dmem_1d: blob-ext@2 { + ddr-1d-dmem-fw { filename = "ddr3_dmem_1d.bin"; size = <0x4000>; type = "blob-ext"; diff --git a/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi b/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi index 6f3d5cfe3c18..3f02b4aac135 100644 --- a/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi +++ b/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi @@ -151,24 +151,28 @@ align-end = <4>; };
- blob_1: blob-ext@1 { + ddr-1d-imem-fw { filename = "ddr4_imem_1d_201810.bin"; align-end = <4>; + type = "blob-ext"; };
- blob_2: blob-ext@2 { + ddr-1d-dmem-fw { filename = "ddr4_dmem_1d_201810.bin"; align-end = <4>; + type = "blob-ext"; };
- blob_3: blob-ext@3 { + ddr-2d-imem-fw { filename = "ddr4_imem_2d_201810.bin"; align-end = <4>; + type = "blob-ext"; };
- blob_4: blob-ext@4 { + ddr-2d-dmem-fw { filename = "ddr4_dmem_2d_201810.bin"; align-end = <4>; + type = "blob-ext"; }; };
diff --git a/arch/arm/dts/imx8mn-evk-u-boot.dtsi b/arch/arm/dts/imx8mn-evk-u-boot.dtsi index 4f6dcf307b28..4d55905736d0 100644 --- a/arch/arm/dts/imx8mn-evk-u-boot.dtsi +++ b/arch/arm/dts/imx8mn-evk-u-boot.dtsi @@ -36,24 +36,28 @@ align-end = <4>; };
- blob_1: blob-ext@1 { + ddr-1d-imem-fw { filename = "lpddr4_pmu_train_1d_imem.bin"; align-end = <4>; + type = "blob-ext"; };
- blob_2: blob-ext@2 { + ddr-1d-dmem-fw { filename = "lpddr4_pmu_train_1d_dmem.bin"; align-end = <4>; + type = "blob-ext"; };
- blob_3: blob-ext@3 { + ddr-2d-imem-fw { filename = "lpddr4_pmu_train_2d_imem.bin"; align-end = <4>; + type = "blob-ext"; };
- blob_4: blob-ext@4 { + ddr-2d-dmem-fw { filename = "lpddr4_pmu_train_2d_dmem.bin"; align-end = <4>; + type = "blob-ext"; }; };
diff --git a/arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi b/arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi index 32ef79292886..ed1ab10ded37 100644 --- a/arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi +++ b/arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi @@ -130,25 +130,25 @@ filename = "u-boot-spl.bin"; };
- blob_1: blob-ext@1 { + ddr-1d-imem-fw { filename = "ddr4_imem_1d.bin"; align-end = <4>; type = "blob-ext"; };
- blob_2: blob-ext@2 { + ddr-1d-dmem-fw { filename = "ddr4_dmem_1d.bin"; align-end = <4>; type = "blob-ext"; };
- blob_3: blob-ext@3 { + ddr-2d-imem-fw { filename = "ddr4_imem_2d.bin"; align-end = <4>; type = "blob-ext"; };
- blob_4: blob-ext@4 { + ddr-2d-dmem-fw { filename = "ddr4_dmem_2d.bin"; align-end = <4>; type = "blob-ext"; diff --git a/arch/arm/dts/imx8mn-venice-u-boot.dtsi b/arch/arm/dts/imx8mn-venice-u-boot.dtsi index 03a9eb8516f9..6d6535fdbfeb 100644 --- a/arch/arm/dts/imx8mn-venice-u-boot.dtsi +++ b/arch/arm/dts/imx8mn-venice-u-boot.dtsi @@ -122,25 +122,25 @@ filename = "u-boot-spl.bin"; };
- imem_1d: blob-ext@1 { + ddr-1d-imem-fw { filename = "lpddr4_pmu_train_1d_imem.bin"; align-end = <4>; type = "blob-ext"; };
- dmem_1d: blob-ext@2 { + ddr-1d-dmem-fw { filename = "lpddr4_pmu_train_1d_dmem.bin"; align-end = <4>; type = "blob-ext"; };
- imem_2d: blob-ext@3 { + ddr-2d-imem-fw { filename = "lpddr4_pmu_train_2d_imem.bin"; align-end = <4>; type = "blob-ext"; };
- dmem_2d: blob-ext@4 { + ddr-2d-dmem-fw { filename = "lpddr4_pmu_train_2d_dmem.bin"; align-end = <4>; type = "blob-ext"; diff --git a/arch/arm/dts/imx8mp-u-boot.dtsi b/arch/arm/dts/imx8mp-u-boot.dtsi index 274515a010ee..6fd4de2c9615 100644 --- a/arch/arm/dts/imx8mp-u-boot.dtsi +++ b/arch/arm/dts/imx8mp-u-boot.dtsi @@ -61,24 +61,28 @@ align-end = <4>; };
- blob_1: blob-ext@1 { + ddr-1d-imem-fw { filename = "lpddr4_pmu_train_1d_imem_202006.bin"; align-end = <4>; + type = "blob-ext"; };
- blob_2: blob-ext@2 { + ddr-1d-dmem-fw { filename = "lpddr4_pmu_train_1d_dmem_202006.bin"; align-end = <4>; + type = "blob-ext"; };
- blob_3: blob-ext@3 { + ddr-2d-imem-fw { filename = "lpddr4_pmu_train_2d_imem_202006.bin"; align-end = <4>; + type = "blob-ext"; };
- blob_4: blob-ext@4 { + ddr-2d-dmem-fw { filename = "lpddr4_pmu_train_2d_dmem_202006.bin"; align-end = <4>; + type = "blob-ext"; }; };
diff --git a/arch/arm/dts/imx8mq-cm-u-boot.dtsi b/arch/arm/dts/imx8mq-cm-u-boot.dtsi index 1b66e76ee1dc..81063083bd40 100644 --- a/arch/arm/dts/imx8mq-cm-u-boot.dtsi +++ b/arch/arm/dts/imx8mq-cm-u-boot.dtsi @@ -20,24 +20,28 @@ align-end = <4>; };
- blob_1: blob-ext@1 { + ddr-1d-imem-fw { filename = "lpddr4_pmu_train_1d_imem.bin"; align-end = <4>; + type = "blob-ext"; };
- blob_2: blob-ext@2 { + ddr-1d-dmem-fw { filename = "lpddr4_pmu_train_1d_dmem.bin"; align-end = <4>; + type = "blob-ext"; };
- blob_3: blob-ext@3 { + ddr-2d-imem-fw { filename = "lpddr4_pmu_train_2d_imem.bin"; align-end = <4>; + type = "blob-ext"; };
- blob_4: blob-ext@4 { + ddr-2d-dmem-fw { filename = "lpddr4_pmu_train_2d_dmem.bin"; align-end = <4>; + type = "blob-ext"; }; };
diff --git a/arch/arm/dts/imx8mq-u-boot.dtsi b/arch/arm/dts/imx8mq-u-boot.dtsi index e683e9184725..000ee545ee7f 100644 --- a/arch/arm/dts/imx8mq-u-boot.dtsi +++ b/arch/arm/dts/imx8mq-u-boot.dtsi @@ -22,25 +22,25 @@ filename = "u-boot-spl.bin"; };
- imem_1d: blob-ext@1 { + ddr-1d-imem-fw { filename = "lpddr4_pmu_train_1d_imem.bin"; align-end = <4>; type = "blob-ext"; };
- dmem_1d: blob-ext@2 { + ddr-1d-dmem-fw { filename = "lpddr4_pmu_train_1d_dmem.bin"; align-end = <4>; type = "blob-ext"; };
- imem_2d: blob-ext@3 { + ddr-2d-imem-fw { filename = "lpddr4_pmu_train_2d_imem.bin"; align-end = <4>; type = "blob-ext"; };
- dmem_2d: blob-ext@4 { + ddr-2d-dmem-fw { filename = "lpddr4_pmu_train_2d_dmem.bin"; align-end = <4>; type = "blob-ext"; diff --git a/drivers/ddr/imx/imx8m/helper.c b/drivers/ddr/imx/imx8m/helper.c index b3bd57531b76..3764cffabc90 100644 --- a/drivers/ddr/imx/imx8m/helper.c +++ b/drivers/ddr/imx/imx8m/helper.c @@ -26,18 +26,18 @@ DECLARE_GLOBAL_DATA_PTR; #define DMEM_OFFSET_ADDR 0x00054000 #define DDR_TRAIN_CODE_BASE_ADDR IP2APB_DDRPHY_IPS_BASE_ADDR(0)
-binman_sym_declare(ulong, blob_ext_1, image_pos); -binman_sym_declare(ulong, blob_ext_1, size); +binman_sym_declare(ulong, ddr_1d_imem_fw, image_pos); +binman_sym_declare(ulong, ddr_1d_imem_fw, size);
-binman_sym_declare(ulong, blob_ext_2, image_pos); -binman_sym_declare(ulong, blob_ext_2, size); +binman_sym_declare(ulong, ddr_1d_dmem_fw, image_pos); +binman_sym_declare(ulong, ddr_1d_dmem_fw, size);
#if !IS_ENABLED(CONFIG_IMX8M_DDR3L) -binman_sym_declare(ulong, blob_ext_3, image_pos); -binman_sym_declare(ulong, blob_ext_3, size); +binman_sym_declare(ulong, ddr_2d_imem_fw, image_pos); +binman_sym_declare(ulong, ddr_2d_imem_fw, size);
-binman_sym_declare(ulong, blob_ext_4, image_pos); -binman_sym_declare(ulong, blob_ext_4, size); +binman_sym_declare(ulong, ddr_2d_dmem_fw, image_pos); +binman_sym_declare(ulong, ddr_2d_dmem_fw, size); #endif
/* We need PHY iMEM PHY is 32KB padded */ @@ -59,27 +59,27 @@ void ddr_load_train_firmware(enum fw_type type) } #endif
+ dmem_start = imem_start + imem_len; + if (CONFIG_IS_ENABLED(BINMAN_SYMBOLS)) { switch (type) { case FW_1D_IMAGE: - imem_start = binman_sym(ulong, blob_ext_1, image_pos); - imem_len = binman_sym(ulong, blob_ext_1, size); - dmem_start = binman_sym(ulong, blob_ext_2, image_pos); - dmem_len = binman_sym(ulong, blob_ext_2, size); + imem_start = binman_sym(ulong, ddr_1d_imem_fw, image_pos); + imem_len = binman_sym(ulong, ddr_1d_imem_fw, size); + dmem_start = binman_sym(ulong, ddr_1d_dmem_fw, image_pos); + dmem_len = binman_sym(ulong, ddr_1d_dmem_fw, size); break; case FW_2D_IMAGE: #if !IS_ENABLED(CONFIG_IMX8M_DDR3L) - imem_start = binman_sym(ulong, blob_ext_3, image_pos); - imem_len = binman_sym(ulong, blob_ext_3, size); - dmem_start = binman_sym(ulong, blob_ext_4, image_pos); - dmem_len = binman_sym(ulong, blob_ext_4, size); + imem_start = binman_sym(ulong, ddr_1d_dmem_fw, image_pos); + imem_len = binman_sym(ulong, ddr_1d_dmem_fw, size); + dmem_start = binman_sym(ulong, ddr_2d_dmem_fw, image_pos); + dmem_len = binman_sym(ulong, ddr_2d_dmem_fw, size); #endif break; } }
- dmem_start = imem_start + imem_len; - pr_from32 = imem_start; pr_to32 = DDR_TRAIN_CODE_BASE_ADDR + 4 * IMEM_OFFSET_ADDR; for (i = 0x0; i < imem_len; ) { diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index e3f362b442b8..bd67238b9199 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -875,7 +875,7 @@ def _CollectEntries(self, entries, entries_by_name, add_entry): entries[entry.GetPath()] = entry for entry in to_add.values(): self._CollectEntries(entries, entries_by_name, entry) - entries_by_name[add_entry.name.replace('@', '-')] = add_entry + entries_by_name[add_entry.name] = add_entry
def MissingArgs(self, entry, missing): """Report a missing argument, if enabled

From: Peng Fan peng.fan@nxp.com
Use BINMAN instead of imx specific packing method.
Signed-off-by: Peng Fan peng.fan@nxp.com --- arch/arm/mach-imx/imx8m/Kconfig | 1 + arch/arm/mach-imx/imx8m/imximage-8mm-lpddr4.cfg | 10 +--------- configs/imx8mm-icore-mx8mm-ctouch2_defconfig | 2 +- configs/imx8mm-icore-mx8mm-edimm2.2_defconfig | 2 +- 4 files changed, 4 insertions(+), 11 deletions(-)
diff --git a/arch/arm/mach-imx/imx8m/Kconfig b/arch/arm/mach-imx/imx8m/Kconfig index 24299ae037f..75ad7aab081 100644 --- a/arch/arm/mach-imx/imx8m/Kconfig +++ b/arch/arm/mach-imx/imx8m/Kconfig @@ -68,6 +68,7 @@ config TARGET_IMX8MM_EVK
config TARGET_IMX8MM_ICORE_MX8MM bool "Engicam i.Core MX8M Mini SOM" + select BINMAN select IMX8MM select SUPPORT_SPL select IMX8M_LPDDR4 diff --git a/arch/arm/mach-imx/imx8m/imximage-8mm-lpddr4.cfg b/arch/arm/mach-imx/imx8m/imximage-8mm-lpddr4.cfg index e06d53ef417..5dcb8ae72f0 100644 --- a/arch/arm/mach-imx/imx8m/imximage-8mm-lpddr4.cfg +++ b/arch/arm/mach-imx/imx8m/imximage-8mm-lpddr4.cfg @@ -3,13 +3,5 @@ * Copyright 2019 NXP */
- -FIT BOOT_FROM sd -LOADER spl/u-boot-spl-ddr.bin 0x7E1000 -SECOND_LOADER u-boot.itb 0x40200000 0x60000 - -DDR_FW lpddr4_pmu_train_1d_imem.bin -DDR_FW lpddr4_pmu_train_1d_dmem.bin -DDR_FW lpddr4_pmu_train_2d_imem.bin -DDR_FW lpddr4_pmu_train_2d_dmem.bin +LOADER u-boot-spl-ddr.bin 0x7E1000 diff --git a/configs/imx8mm-icore-mx8mm-ctouch2_defconfig b/configs/imx8mm-icore-mx8mm-ctouch2_defconfig index d95a74a7237..dcb12e5d026 100644 --- a/configs/imx8mm-icore-mx8mm-ctouch2_defconfig +++ b/configs/imx8mm-icore-mx8mm-ctouch2_defconfig @@ -20,7 +20,7 @@ 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_DEFAULT_FDT_FILE="imx8mm-icore-mx8mm-ctouch2.dtb" CONFIG_SPL_BOARD_INIT=y diff --git a/configs/imx8mm-icore-mx8mm-edimm2.2_defconfig b/configs/imx8mm-icore-mx8mm-edimm2.2_defconfig index 43c697a39d8..22acf7317b4 100644 --- a/configs/imx8mm-icore-mx8mm-edimm2.2_defconfig +++ b/configs/imx8mm-icore-mx8mm-edimm2.2_defconfig @@ -20,7 +20,7 @@ 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_DEFAULT_FDT_FILE="imx8mm-icore-mx8mm-edimm2.2.dtb" CONFIG_SPL_BOARD_INIT=y

On 20/05/2022 17:10, Peng Fan (OSS) wrote:
From: Peng Fan peng.fan@nxp.com
Use BINMAN instead of imx specific packing method.
Signed-off-by: Peng Fan peng.fan@nxp.com
arch/arm/mach-imx/imx8m/Kconfig | 1 + arch/arm/mach-imx/imx8m/imximage-8mm-lpddr4.cfg | 10 +---------
Maybe this could be done for other imximage*.cfg files as well?
configs/imx8mm-icore-mx8mm-ctouch2_defconfig | 2 +- configs/imx8mm-icore-mx8mm-edimm2.2_defconfig | 2 +- 4 files changed, 4 insertions(+), 11 deletions(-)
diff --git a/arch/arm/mach-imx/imx8m/Kconfig b/arch/arm/mach-imx/imx8m/Kconfig index 24299ae037f..75ad7aab081 100644 --- a/arch/arm/mach-imx/imx8m/Kconfig +++ b/arch/arm/mach-imx/imx8m/Kconfig @@ -68,6 +68,7 @@ config TARGET_IMX8MM_EVK
config TARGET_IMX8MM_ICORE_MX8MM bool "Engicam i.Core MX8M Mini SOM"
- select BINMAN select IMX8MM select SUPPORT_SPL select IMX8M_LPDDR4
diff --git a/arch/arm/mach-imx/imx8m/imximage-8mm-lpddr4.cfg b/arch/arm/mach-imx/imx8m/imximage-8mm-lpddr4.cfg index e06d53ef417..5dcb8ae72f0 100644 --- a/arch/arm/mach-imx/imx8m/imximage-8mm-lpddr4.cfg +++ b/arch/arm/mach-imx/imx8m/imximage-8mm-lpddr4.cfg @@ -3,13 +3,5 @@
- Copyright 2019 NXP
*/
-FIT BOOT_FROM sd -LOADER spl/u-boot-spl-ddr.bin 0x7E1000 -SECOND_LOADER u-boot.itb 0x40200000 0x60000
-DDR_FW lpddr4_pmu_train_1d_imem.bin -DDR_FW lpddr4_pmu_train_1d_dmem.bin -DDR_FW lpddr4_pmu_train_2d_imem.bin -DDR_FW lpddr4_pmu_train_2d_dmem.bin +LOADER u-boot-spl-ddr.bin 0x7E1000 diff --git a/configs/imx8mm-icore-mx8mm-ctouch2_defconfig b/configs/imx8mm-icore-mx8mm-ctouch2_defconfig index d95a74a7237..dcb12e5d026 100644 --- a/configs/imx8mm-icore-mx8mm-ctouch2_defconfig +++ b/configs/imx8mm-icore-mx8mm-ctouch2_defconfig @@ -20,7 +20,7 @@ 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_DEFAULT_FDT_FILE="imx8mm-icore-mx8mm-ctouch2.dtb" CONFIG_SPL_BOARD_INIT=y diff --git a/configs/imx8mm-icore-mx8mm-edimm2.2_defconfig b/configs/imx8mm-icore-mx8mm-edimm2.2_defconfig index 43c697a39d8..22acf7317b4 100644 --- a/configs/imx8mm-icore-mx8mm-edimm2.2_defconfig +++ b/configs/imx8mm-icore-mx8mm-edimm2.2_defconfig @@ -20,7 +20,7 @@ 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_DEFAULT_FDT_FILE="imx8mm-icore-mx8mm-edimm2.2.dtb" CONFIG_SPL_BOARD_INIT=y

Subject: Re: [PATCH V4 3/8] imx: imx8mm-icore: migrate to use BINMAN
On 20/05/2022 17:10, Peng Fan (OSS) wrote:
From: Peng Fan peng.fan@nxp.com
Use BINMAN instead of imx specific packing method.
Signed-off-by: Peng Fan peng.fan@nxp.com
arch/arm/mach-imx/imx8m/Kconfig | 1 + arch/arm/mach-imx/imx8m/imximage-8mm-lpddr4.cfg | 10 +---------
Maybe this could be done for other imximage*.cfg files as well?
Yes, there are follow up patches to clean up the cfg files, this patch is for binman and avoid build break for these two boards.
Thanks, Peng.
configs/imx8mm-icore-mx8mm-ctouch2_defconfig | 2 +- configs/imx8mm-icore-mx8mm-edimm2.2_defconfig | 2 +- 4 files changed, 4 insertions(+), 11 deletions(-)
diff --git a/arch/arm/mach-imx/imx8m/Kconfig b/arch/arm/mach-imx/imx8m/Kconfig index 24299ae037f..75ad7aab081 100644 --- a/arch/arm/mach-imx/imx8m/Kconfig +++ b/arch/arm/mach-imx/imx8m/Kconfig @@ -68,6 +68,7 @@ config TARGET_IMX8MM_EVK
config TARGET_IMX8MM_ICORE_MX8MM bool "Engicam i.Core MX8M Mini SOM"
- select BINMAN select IMX8MM select SUPPORT_SPL select IMX8M_LPDDR4
diff --git a/arch/arm/mach-imx/imx8m/imximage-8mm-lpddr4.cfg b/arch/arm/mach-imx/imx8m/imximage-8mm-lpddr4.cfg index e06d53ef417..5dcb8ae72f0 100644 --- a/arch/arm/mach-imx/imx8m/imximage-8mm-lpddr4.cfg +++ b/arch/arm/mach-imx/imx8m/imximage-8mm-lpddr4.cfg @@ -3,13 +3,5 @@
- Copyright 2019 NXP
*/
-FIT BOOT_FROM sd -LOADER spl/u-boot-spl-ddr.bin 0x7E1000 -SECOND_LOADER u-boot.itb 0x40200000 0x60000
-DDR_FW lpddr4_pmu_train_1d_imem.bin -DDR_FW lpddr4_pmu_train_1d_dmem.bin -DDR_FW lpddr4_pmu_train_2d_imem.bin -DDR_FW lpddr4_pmu_train_2d_dmem.bin +LOADER u-boot-spl-ddr.bin 0x7E1000 diff --git a/configs/imx8mm-icore-mx8mm-ctouch2_defconfig b/configs/imx8mm-icore-mx8mm-ctouch2_defconfig index d95a74a7237..dcb12e5d026 100644 --- a/configs/imx8mm-icore-mx8mm-ctouch2_defconfig +++ b/configs/imx8mm-icore-mx8mm-ctouch2_defconfig @@ -20,7 +20,7 @@ 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_DEFAULT_FDT_FILE="imx8mm-icore-mx8mm-ctouch2.dtb" CONFIG_SPL_BOARD_INIT=y diff --git a/configs/imx8mm-icore-mx8mm-edimm2.2_defconfig b/configs/imx8mm-icore-mx8mm-edimm2.2_defconfig index 43c697a39d8..22acf7317b4 100644 --- a/configs/imx8mm-icore-mx8mm-edimm2.2_defconfig +++ b/configs/imx8mm-icore-mx8mm-edimm2.2_defconfig @@ -20,7 +20,7 @@ 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_DEFAULT_FDT_FILE="imx8mm-icore-mx8mm-edimm2.2.dtb" CONFIG_SPL_BOARD_INIT=y

From: Peng Fan peng.fan@nxp.com
In arch/arm/lib/sections.c there is below code: char __image_copy_start[0] __section(".__image_copy_start"); But actually 'objdump -t spl/u-boot-spl' not able to find out symbol '__image_copy_start' for binman update image-pos/size.
So update link file
Tested-by: Tim Harvey tharvey@gateworks.com #imx8m[m,n,p]-venice Signed-off-by: Peng Fan peng.fan@nxp.com --- arch/arm/cpu/armv8/u-boot-spl.lds | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/cpu/armv8/u-boot-spl.lds b/arch/arm/cpu/armv8/u-boot-spl.lds index 730eb93dbc3..9b1e7d46287 100644 --- a/arch/arm/cpu/armv8/u-boot-spl.lds +++ b/arch/arm/cpu/armv8/u-boot-spl.lds @@ -23,7 +23,7 @@ SECTIONS { .text : { . = ALIGN(8); - *(.__image_copy_start) + __image_copy_start = .; CPUDIR/start.o (.text*) *(.text*) } >.sram

On Fri, May 20, 2022 at 10:10:43PM +0800, Peng Fan (OSS) wrote:
From: Peng Fan peng.fan@nxp.com
In arch/arm/lib/sections.c there is below code: char __image_copy_start[0] __section(".__image_copy_start"); But actually 'objdump -t spl/u-boot-spl' not able to find out symbol '__image_copy_start' for binman update image-pos/size.
So update link file
Tested-by: Tim Harvey tharvey@gateworks.com #imx8m[m,n,p]-venice Signed-off-by: Peng Fan peng.fan@nxp.com
Reviewed-by: Tom Rini trini@konsulko.com

From: Peng Fan peng.fan@nxp.com
In arch/arm/dts/imx8mp-u-boot.dtsi, there are blob-ext@1, blob-ext@2 and etc which is for packing ddr phy firmware. However we could not declare symbol name such as 'binman_sym_declare(ulong, blob_ext@1, image_pos)', because '@' is not allowed, so we choose to declare the symbol 'binman_sym_declare(ulong, blob_ext_1, image_pos);' with '@' replaced with '_'. It does not impact if there is no '@' in section name.
Tested-by: Tim Harvey tharvey@gateworks.com #imx8m[m,n,p]-venice Reviewed-by: Tom Rini trini@konsulko.com Signed-off-by: Peng Fan peng.fan@nxp.com --- tools/binman/etype/section.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index bd67238b919..e3f362b442b 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -875,7 +875,7 @@ class Entry_section(Entry): entries[entry.GetPath()] = entry for entry in to_add.values(): self._CollectEntries(entries, entries_by_name, entry) - entries_by_name[add_entry.name] = add_entry + entries_by_name[add_entry.name.replace('@', '-')] = add_entry
def MissingArgs(self, entry, missing): """Report a missing argument, if enabled

On 20/05/2022 17:10, Peng Fan (OSS) wrote:
From: Peng Fan peng.fan@nxp.com
In arch/arm/dts/imx8mp-u-boot.dtsi, there are blob-ext@1, blob-ext@2 and etc which is for packing ddr phy firmware. However we could not declare symbol name such as 'binman_sym_declare(ulong, blob_ext@1, image_pos)', because '@' is not allowed, so we choose to declare the symbol 'binman_sym_declare(ulong, blob_ext_1, image_pos);' with '@' replaced with '_'. It does not impact if there is no '@' in section name.
Tested-by: Tim Harvey tharvey@gateworks.com #imx8m[m,n,p]-venice Reviewed-by: Tom Rini trini@konsulko.com Signed-off-by: Peng Fan peng.fan@nxp.com
tools/binman/etype/section.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
This shouldn't be necessary if you keep the old names for the binman entries and use `binman_sym_declare(ulong, imem_1d, image_pos);` etc. for the symbols.
diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index bd67238b919..e3f362b442b 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -875,7 +875,7 @@ class Entry_section(Entry): entries[entry.GetPath()] = entry for entry in to_add.values(): self._CollectEntries(entries, entries_by_name, entry)
entries_by_name[add_entry.name] = add_entry
entries_by_name[add_entry.name.replace('@', '-')] = add_entry
The correct place to do this would be LookupSymbol() in binman/etype/section.py, but I'm not convinced this should be done at all. I'd say if an entry is important enough to have a symbol for it, it should have a unique, descriptive, non-@ name.
def MissingArgs(self, entry, missing): """Report a missing argument, if enabled

Subject: Re: [PATCH V4 5/8] tools: binman: section: replace @ with -
On 20/05/2022 17:10, Peng Fan (OSS) wrote:
From: Peng Fan peng.fan@nxp.com
In arch/arm/dts/imx8mp-u-boot.dtsi, there are blob-ext@1, blob-ext@2 and etc which is for packing ddr phy firmware. However we could not declare symbol name such as 'binman_sym_declare(ulong, blob_ext@1, image_pos)', because '@' is not allowed, so we choose to declare the symbol 'binman_sym_declare(ulong, blob_ext_1, image_pos);' with '@' replaced with '_'. It does not impact if there is no '@' in section name.
Tested-by: Tim Harvey tharvey@gateworks.com #imx8m[m,n,p]-venice Reviewed-by: Tom Rini trini@konsulko.com Signed-off-by: Peng Fan peng.fan@nxp.com
tools/binman/etype/section.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
This shouldn't be necessary if you keep the old names for the binman entries and use `binman_sym_declare(ulong, imem_1d, image_pos);` etc. for the symbols.
No, as I replied in the other mail, imem_1d is not a binman type.
diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index bd67238b919..e3f362b442b 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -875,7 +875,7 @@ class Entry_section(Entry): entries[entry.GetPath()] = entry for entry in to_add.values(): self._CollectEntries(entries, entries_by_name, entry)
entries_by_name[add_entry.name] = add_entry
entries_by_name[add_entry.name.replace('@', '-')] = add_entry
The correct place to do this would be LookupSymbol() in binman/etype/section.py, but I'm not convinced this should be done at all. I'd say if an entry is important enough to have a symbol for it, it should have a unique, descriptive, non-@ name.
Since blob_ext@[1,2,3] is used, this is to replace and generate symbols as blob_ext_[1,2,3]
Thanks, Peng.
def MissingArgs(self, entry, missing): """Report a missing argument, if enabled

From: Peng Fan peng.fan@nxp.com
By reading binman symbols, we no need hard coded IMEM_LEN/DMEM_LEN after we update the binman dtsi to drop 0x8000/0x4000 length for the firmware.
And that could save binary size for many KBs.
Tested-by: Tim Harvey tharvey@gateworks.com #imx8m[m,n,p]-venice Signed-off-by: Peng Fan peng.fan@nxp.com --- drivers/ddr/imx/imx8m/helper.c | 51 ++++++++++++++++++++++++++++------ 1 file changed, 43 insertions(+), 8 deletions(-)
diff --git a/drivers/ddr/imx/imx8m/helper.c b/drivers/ddr/imx/imx8m/helper.c index f23904bf712..b3bd57531b7 100644 --- a/drivers/ddr/imx/imx8m/helper.c +++ b/drivers/ddr/imx/imx8m/helper.c @@ -4,6 +4,7 @@ */
#include <common.h> +#include <binman_sym.h> #include <log.h> #include <spl.h> #include <asm/global_data.h> @@ -25,15 +26,30 @@ DECLARE_GLOBAL_DATA_PTR; #define DMEM_OFFSET_ADDR 0x00054000 #define DDR_TRAIN_CODE_BASE_ADDR IP2APB_DDRPHY_IPS_BASE_ADDR(0)
+binman_sym_declare(ulong, blob_ext_1, image_pos); +binman_sym_declare(ulong, blob_ext_1, size); + +binman_sym_declare(ulong, blob_ext_2, image_pos); +binman_sym_declare(ulong, blob_ext_2, size); + +#if !IS_ENABLED(CONFIG_IMX8M_DDR3L) +binman_sym_declare(ulong, blob_ext_3, image_pos); +binman_sym_declare(ulong, blob_ext_3, size); + +binman_sym_declare(ulong, blob_ext_4, image_pos); +binman_sym_declare(ulong, blob_ext_4, size); +#endif + /* We need PHY iMEM PHY is 32KB padded */ void ddr_load_train_firmware(enum fw_type type) { u32 tmp32, i; u32 error = 0; unsigned long pr_to32, pr_from32; - unsigned long fw_offset = type ? IMEM_2D_OFFSET : 0; - unsigned long imem_start = (unsigned long)&_end + fw_offset; - unsigned long dmem_start; + uint32_t fw_offset = type ? IMEM_2D_OFFSET : 0; + uint32_t imem_start = (unsigned long)&_end + fw_offset; + uint32_t dmem_start; + uint32_t imem_len = IMEM_LEN, dmem_len = DMEM_LEN;
#ifdef CONFIG_SPL_OF_CONTROL if (gd->fdt_blob && !fdt_check_header(gd->fdt_blob)) { @@ -43,11 +59,30 @@ void ddr_load_train_firmware(enum fw_type type) } #endif
- dmem_start = imem_start + IMEM_LEN; + if (CONFIG_IS_ENABLED(BINMAN_SYMBOLS)) { + switch (type) { + case FW_1D_IMAGE: + imem_start = binman_sym(ulong, blob_ext_1, image_pos); + imem_len = binman_sym(ulong, blob_ext_1, size); + dmem_start = binman_sym(ulong, blob_ext_2, image_pos); + dmem_len = binman_sym(ulong, blob_ext_2, size); + break; + case FW_2D_IMAGE: +#if !IS_ENABLED(CONFIG_IMX8M_DDR3L) + imem_start = binman_sym(ulong, blob_ext_3, image_pos); + imem_len = binman_sym(ulong, blob_ext_3, size); + dmem_start = binman_sym(ulong, blob_ext_4, image_pos); + dmem_len = binman_sym(ulong, blob_ext_4, size); +#endif + break; + } + } + + dmem_start = imem_start + imem_len;
pr_from32 = imem_start; pr_to32 = DDR_TRAIN_CODE_BASE_ADDR + 4 * IMEM_OFFSET_ADDR; - for (i = 0x0; i < IMEM_LEN; ) { + for (i = 0x0; i < imem_len; ) { tmp32 = readl(pr_from32); writew(tmp32 & 0x0000ffff, pr_to32); pr_to32 += 4; @@ -59,7 +94,7 @@ void ddr_load_train_firmware(enum fw_type type)
pr_from32 = dmem_start; pr_to32 = DDR_TRAIN_CODE_BASE_ADDR + 4 * DMEM_OFFSET_ADDR; - for (i = 0x0; i < DMEM_LEN; ) { + for (i = 0x0; i < dmem_len; ) { tmp32 = readl(pr_from32); writew(tmp32 & 0x0000ffff, pr_to32); pr_to32 += 4; @@ -72,7 +107,7 @@ void ddr_load_train_firmware(enum fw_type type) debug("check ddr_pmu_train_imem code\n"); pr_from32 = imem_start; pr_to32 = DDR_TRAIN_CODE_BASE_ADDR + 4 * IMEM_OFFSET_ADDR; - for (i = 0x0; i < IMEM_LEN; ) { + for (i = 0x0; i < imem_len; ) { tmp32 = (readw(pr_to32) & 0x0000ffff); pr_to32 += 4; tmp32 += ((readw(pr_to32) & 0x0000ffff) << 16); @@ -93,7 +128,7 @@ void ddr_load_train_firmware(enum fw_type type) debug("check ddr4_pmu_train_dmem code\n"); pr_from32 = dmem_start; pr_to32 = DDR_TRAIN_CODE_BASE_ADDR + 4 * DMEM_OFFSET_ADDR; - for (i = 0x0; i < DMEM_LEN;) { + for (i = 0x0; i < dmem_len;) { tmp32 = (readw(pr_to32) & 0x0000ffff); pr_to32 += 4; tmp32 += ((readw(pr_to32) & 0x0000ffff) << 16);

On 20/05/2022 17:10, Peng Fan (OSS) wrote:
From: Peng Fan peng.fan@nxp.com
By reading binman symbols, we no need hard coded IMEM_LEN/DMEM_LEN after we update the binman dtsi to drop 0x8000/0x4000 length for the firmware.
And that could save binary size for many KBs.
Tested-by: Tim Harvey tharvey@gateworks.com #imx8m[m,n,p]-venice Signed-off-by: Peng Fan peng.fan@nxp.com
drivers/ddr/imx/imx8m/helper.c | 51 ++++++++++++++++++++++++++++------ 1 file changed, 43 insertions(+), 8 deletions(-)
diff --git a/drivers/ddr/imx/imx8m/helper.c b/drivers/ddr/imx/imx8m/helper.c index f23904bf712..b3bd57531b7 100644 --- a/drivers/ddr/imx/imx8m/helper.c +++ b/drivers/ddr/imx/imx8m/helper.c @@ -4,6 +4,7 @@ */
#include <common.h> +#include <binman_sym.h> #include <log.h> #include <spl.h> #include <asm/global_data.h> @@ -25,15 +26,30 @@ DECLARE_GLOBAL_DATA_PTR; #define DMEM_OFFSET_ADDR 0x00054000 #define DDR_TRAIN_CODE_BASE_ADDR IP2APB_DDRPHY_IPS_BASE_ADDR(0)
+binman_sym_declare(ulong, blob_ext_1, image_pos); +binman_sym_declare(ulong, blob_ext_1, size);
+binman_sym_declare(ulong, blob_ext_2, image_pos); +binman_sym_declare(ulong, blob_ext_2, size);
+#if !IS_ENABLED(CONFIG_IMX8M_DDR3L) +binman_sym_declare(ulong, blob_ext_3, image_pos); +binman_sym_declare(ulong, blob_ext_3, size);
+binman_sym_declare(ulong, blob_ext_4, image_pos); +binman_sym_declare(ulong, blob_ext_4, size); +#endif
(Descriptive entry names would make these more meaningful.)
/* We need PHY iMEM PHY is 32KB padded */ void ddr_load_train_firmware(enum fw_type type) { u32 tmp32, i; u32 error = 0; unsigned long pr_to32, pr_from32;
- unsigned long fw_offset = type ? IMEM_2D_OFFSET : 0;
- unsigned long imem_start = (unsigned long)&_end + fw_offset;
- unsigned long dmem_start;
- uint32_t fw_offset = type ? IMEM_2D_OFFSET : 0;
- uint32_t imem_start = (unsigned long)&_end + fw_offset;
- uint32_t dmem_start;
- uint32_t imem_len = IMEM_LEN, dmem_len = DMEM_LEN;
#ifdef CONFIG_SPL_OF_CONTROL if (gd->fdt_blob && !fdt_check_header(gd->fdt_blob)) { @@ -43,11 +59,30 @@ void ddr_load_train_firmware(enum fw_type type) } #endif
- dmem_start = imem_start + IMEM_LEN;
- if (CONFIG_IS_ENABLED(BINMAN_SYMBOLS)) {
switch (type) {
case FW_1D_IMAGE:
imem_start = binman_sym(ulong, blob_ext_1, image_pos);
imem_len = binman_sym(ulong, blob_ext_1, size);
dmem_start = binman_sym(ulong, blob_ext_2, image_pos);
dmem_len = binman_sym(ulong, blob_ext_2, size);
break;
case FW_2D_IMAGE:
+#if !IS_ENABLED(CONFIG_IMX8M_DDR3L)
imem_start = binman_sym(ulong, blob_ext_3, image_pos);
imem_len = binman_sym(ulong, blob_ext_3, size);
dmem_start = binman_sym(ulong, blob_ext_4, image_pos);
dmem_len = binman_sym(ulong, blob_ext_4, size);
+#endif
break;
}
- }
- dmem_start = imem_start + imem_len;
This overwrites the values from binman symbols, which looks wrong to me. I think this line should appear before the if block.
pr_from32 = imem_start; pr_to32 = DDR_TRAIN_CODE_BASE_ADDR + 4 * IMEM_OFFSET_ADDR;
- for (i = 0x0; i < IMEM_LEN; ) {
- for (i = 0x0; i < imem_len; ) { tmp32 = readl(pr_from32); writew(tmp32 & 0x0000ffff, pr_to32); pr_to32 += 4;
@@ -59,7 +94,7 @@ void ddr_load_train_firmware(enum fw_type type)
pr_from32 = dmem_start; pr_to32 = DDR_TRAIN_CODE_BASE_ADDR + 4 * DMEM_OFFSET_ADDR;
- for (i = 0x0; i < DMEM_LEN; ) {
- for (i = 0x0; i < dmem_len; ) { tmp32 = readl(pr_from32); writew(tmp32 & 0x0000ffff, pr_to32); pr_to32 += 4;
@@ -72,7 +107,7 @@ void ddr_load_train_firmware(enum fw_type type) debug("check ddr_pmu_train_imem code\n"); pr_from32 = imem_start; pr_to32 = DDR_TRAIN_CODE_BASE_ADDR + 4 * IMEM_OFFSET_ADDR;
- for (i = 0x0; i < IMEM_LEN; ) {
- for (i = 0x0; i < imem_len; ) { tmp32 = (readw(pr_to32) & 0x0000ffff); pr_to32 += 4; tmp32 += ((readw(pr_to32) & 0x0000ffff) << 16);
@@ -93,7 +128,7 @@ void ddr_load_train_firmware(enum fw_type type) debug("check ddr4_pmu_train_dmem code\n"); pr_from32 = dmem_start; pr_to32 = DDR_TRAIN_CODE_BASE_ADDR + 4 * DMEM_OFFSET_ADDR;
- for (i = 0x0; i < DMEM_LEN;) {
- for (i = 0x0; i < dmem_len;) { tmp32 = (readw(pr_to32) & 0x0000ffff); pr_to32 += 4; tmp32 += ((readw(pr_to32) & 0x0000ffff) << 16);

Subject: Re: [PATCH V4 6/8] ddr: imx8m: helper: load ddr firmware according to binman symbols
On 20/05/2022 17:10, Peng Fan (OSS) wrote:
From: Peng Fan peng.fan@nxp.com
By reading binman symbols, we no need hard coded IMEM_LEN/DMEM_LEN after we update the binman dtsi to drop 0x8000/0x4000 length for the
firmware.
And that could save binary size for many KBs.
Tested-by: Tim Harvey tharvey@gateworks.com #imx8m[m,n,p]-venice Signed-off-by: Peng Fan peng.fan@nxp.com
drivers/ddr/imx/imx8m/helper.c | 51 ++++++++++++++++++++++++++++------ 1 file changed, 43 insertions(+), 8 deletions(-)
diff --git a/drivers/ddr/imx/imx8m/helper.c b/drivers/ddr/imx/imx8m/helper.c index f23904bf712..b3bd57531b7
100644
--- a/drivers/ddr/imx/imx8m/helper.c +++ b/drivers/ddr/imx/imx8m/helper.c @@ -4,6 +4,7 @@ */
#include <common.h> +#include <binman_sym.h> #include <log.h> #include <spl.h> #include <asm/global_data.h> @@ -25,15 +26,30 @@ DECLARE_GLOBAL_DATA_PTR; #define
DMEM_OFFSET_ADDR
0x00054000 #define DDR_TRAIN_CODE_BASE_ADDR IP2APB_DDRPHY_IPS_BASE_ADDR(0)
+binman_sym_declare(ulong, blob_ext_1, image_pos); +binman_sym_declare(ulong, blob_ext_1, size);
+binman_sym_declare(ulong, blob_ext_2, image_pos); +binman_sym_declare(ulong, blob_ext_2, size);
+#if !IS_ENABLED(CONFIG_IMX8M_DDR3L) +binman_sym_declare(ulong, blob_ext_3, image_pos); +binman_sym_declare(ulong, blob_ext_3, size);
+binman_sym_declare(ulong, blob_ext_4, image_pos); +binman_sym_declare(ulong, blob_ext_4, size); #endif
(Descriptive entry names would make these more meaningful.)
/* We need PHY iMEM PHY is 32KB padded */ void ddr_load_train_firmware(enum fw_type type) { u32 tmp32, i; u32 error = 0; unsigned long pr_to32, pr_from32;
- unsigned long fw_offset = type ? IMEM_2D_OFFSET : 0;
- unsigned long imem_start = (unsigned long)&_end + fw_offset;
- unsigned long dmem_start;
- uint32_t fw_offset = type ? IMEM_2D_OFFSET : 0;
- uint32_t imem_start = (unsigned long)&_end + fw_offset;
- uint32_t dmem_start;
- uint32_t imem_len = IMEM_LEN, dmem_len = DMEM_LEN;
#ifdef CONFIG_SPL_OF_CONTROL if (gd->fdt_blob && !fdt_check_header(gd->fdt_blob)) { @@ -43,11 +59,30 @@ void ddr_load_train_firmware(enum fw_type type) } #endif
- dmem_start = imem_start + IMEM_LEN;
- if (CONFIG_IS_ENABLED(BINMAN_SYMBOLS)) {
switch (type) {
case FW_1D_IMAGE:
imem_start = binman_sym(ulong, blob_ext_1, image_pos);
imem_len = binman_sym(ulong, blob_ext_1, size);
dmem_start = binman_sym(ulong, blob_ext_2, image_pos);
dmem_len = binman_sym(ulong, blob_ext_2, size);
break;
case FW_2D_IMAGE:
+#if !IS_ENABLED(CONFIG_IMX8M_DDR3L)
imem_start = binman_sym(ulong, blob_ext_3, image_pos);
imem_len = binman_sym(ulong, blob_ext_3, size);
dmem_start = binman_sym(ulong, blob_ext_4, image_pos);
dmem_len = binman_sym(ulong, blob_ext_4, size); #endif
break;
}
- }
- dmem_start = imem_start + imem_len;
This overwrites the values from binman symbols, which looks wrong to me. I think this line should appear before the if block.
It does not matter actually. Put it before if block is ok.
Regards, Peng.
pr_from32 = imem_start; pr_to32 = DDR_TRAIN_CODE_BASE_ADDR + 4 * IMEM_OFFSET_ADDR;
- for (i = 0x0; i < IMEM_LEN; ) {
- for (i = 0x0; i < imem_len; ) { tmp32 = readl(pr_from32); writew(tmp32 & 0x0000ffff, pr_to32); pr_to32 += 4;
@@ -59,7 +94,7 @@ void ddr_load_train_firmware(enum fw_type type)
pr_from32 = dmem_start; pr_to32 = DDR_TRAIN_CODE_BASE_ADDR + 4 * DMEM_OFFSET_ADDR;
- for (i = 0x0; i < DMEM_LEN; ) {
- for (i = 0x0; i < dmem_len; ) { tmp32 = readl(pr_from32); writew(tmp32 & 0x0000ffff, pr_to32); pr_to32 += 4;
@@ -72,7 +107,7 @@ void ddr_load_train_firmware(enum fw_type type) debug("check ddr_pmu_train_imem code\n"); pr_from32 = imem_start; pr_to32 = DDR_TRAIN_CODE_BASE_ADDR + 4 * IMEM_OFFSET_ADDR;
- for (i = 0x0; i < IMEM_LEN; ) {
- for (i = 0x0; i < imem_len; ) { tmp32 = (readw(pr_to32) & 0x0000ffff); pr_to32 += 4; tmp32 += ((readw(pr_to32) & 0x0000ffff) << 16); @@ -93,7 +128,7
@@
void ddr_load_train_firmware(enum fw_type type) debug("check ddr4_pmu_train_dmem code\n"); pr_from32 = dmem_start; pr_to32 = DDR_TRAIN_CODE_BASE_ADDR + 4 * DMEM_OFFSET_ADDR;
- for (i = 0x0; i < DMEM_LEN;) {
- for (i = 0x0; i < dmem_len;) { tmp32 = (readw(pr_to32) & 0x0000ffff); pr_to32 += 4; tmp32 += ((readw(pr_to32) & 0x0000ffff) << 16);

From: Peng Fan peng.fan@nxp.com
After we switch to use BINMAN_SYMBOLS, there is no need to pad the file size to 0x8000 and 0x4000. After we use BINMAN_SYMBOLS, the u-boot-spl-ddr.bin shrink about 36KB with i.MX8MP-EVK.
Tested-by: Tim Harvey tharvey@gateworks.com #imx8m[m,n,p]-venice Signed-off-by: Peng Fan peng.fan@nxp.com --- arch/arm/dts/imx8mm-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mn-beacon-kit-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mn-evk-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mn-venice-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mp-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mq-cm-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mq-u-boot.dtsi | 8 ++++---- 9 files changed, 36 insertions(+), 36 deletions(-)
diff --git a/arch/arm/dts/imx8mm-u-boot.dtsi b/arch/arm/dts/imx8mm-u-boot.dtsi index 5de55a2d80b..19a2da30f51 100644 --- a/arch/arm/dts/imx8mm-u-boot.dtsi +++ b/arch/arm/dts/imx8mm-u-boot.dtsi @@ -41,25 +41,25 @@
imem_1d: blob-ext@1 { filename = "lpddr4_pmu_train_1d_imem.bin"; - size = <0x8000>; + align-end = <4>; type = "blob-ext"; };
dmem_1d: blob-ext@2 { filename = "lpddr4_pmu_train_1d_dmem.bin"; - size = <0x4000>; + align-end = <4>; type = "blob-ext"; };
imem_2d: blob-ext@3 { filename = "lpddr4_pmu_train_2d_imem.bin"; - size = <0x8000>; + align-end = <4>; type = "blob-ext"; };
dmem_2d: blob-ext@4 { filename = "lpddr4_pmu_train_2d_dmem.bin"; - size = <0x4000>; + align-end = <4>; type = "blob-ext"; }; }; diff --git a/arch/arm/dts/imx8mn-beacon-kit-u-boot.dtsi b/arch/arm/dts/imx8mn-beacon-kit-u-boot.dtsi index eb1dd8debba..e1740fa31a6 100644 --- a/arch/arm/dts/imx8mn-beacon-kit-u-boot.dtsi +++ b/arch/arm/dts/imx8mn-beacon-kit-u-boot.dtsi @@ -149,22 +149,22 @@
blob_1: blob-ext@1 { filename = "lpddr4_pmu_train_1d_imem.bin"; - size = <0x8000>; + align-end = <4>; };
blob_2: blob-ext@2 { filename = "lpddr4_pmu_train_1d_dmem.bin"; - size = <0x4000>; + align-end = <4>; };
blob_3: blob-ext@3 { filename = "lpddr4_pmu_train_2d_imem.bin"; - size = <0x8000>; + align-end = <4>; };
blob_4: blob-ext@4 { filename = "lpddr4_pmu_train_2d_dmem.bin"; - size = <0x4000>; + align-end = <4>; }; };
diff --git a/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi b/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi index 4d0ecb07d4f..1fe2d0fd507 100644 --- a/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi +++ b/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi @@ -157,22 +157,22 @@
blob_1: blob-ext@1 { filename = "ddr4_imem_1d_201810.bin"; - size = <0x8000>; + align-end = <4>; };
blob_2: blob-ext@2 { filename = "ddr4_dmem_1d_201810.bin"; - size = <0x4000>; + align-end = <4>; };
blob_3: blob-ext@3 { filename = "ddr4_imem_2d_201810.bin"; - size = <0x8000>; + align-end = <4>; };
blob_4: blob-ext@4 { filename = "ddr4_dmem_2d_201810.bin"; - size = <0x4000>; + align-end = <4>; }; };
diff --git a/arch/arm/dts/imx8mn-evk-u-boot.dtsi b/arch/arm/dts/imx8mn-evk-u-boot.dtsi index 3db46d4cbcb..4f6dcf307b2 100644 --- a/arch/arm/dts/imx8mn-evk-u-boot.dtsi +++ b/arch/arm/dts/imx8mn-evk-u-boot.dtsi @@ -38,22 +38,22 @@
blob_1: blob-ext@1 { filename = "lpddr4_pmu_train_1d_imem.bin"; - size = <0x8000>; + align-end = <4>; };
blob_2: blob-ext@2 { filename = "lpddr4_pmu_train_1d_dmem.bin"; - size = <0x4000>; + align-end = <4>; };
blob_3: blob-ext@3 { filename = "lpddr4_pmu_train_2d_imem.bin"; - size = <0x8000>; + align-end = <4>; };
blob_4: blob-ext@4 { filename = "lpddr4_pmu_train_2d_dmem.bin"; - size = <0x4000>; + align-end = <4>; }; };
diff --git a/arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi b/arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi index 001e725f568..32ef7929288 100644 --- a/arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi +++ b/arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi @@ -132,25 +132,25 @@
blob_1: blob-ext@1 { filename = "ddr4_imem_1d.bin"; - size = <0x8000>; + align-end = <4>; type = "blob-ext"; };
blob_2: blob-ext@2 { filename = "ddr4_dmem_1d.bin"; - size = <0x4000>; + align-end = <4>; type = "blob-ext"; };
blob_3: blob-ext@3 { filename = "ddr4_imem_2d.bin"; - size = <0x8000>; + align-end = <4>; type = "blob-ext"; };
blob_4: blob-ext@4 { filename = "ddr4_dmem_2d.bin"; - size = <0x4000>; + align-end = <4>; type = "blob-ext"; }; }; diff --git a/arch/arm/dts/imx8mn-venice-u-boot.dtsi b/arch/arm/dts/imx8mn-venice-u-boot.dtsi index 67922146963..e3a0b170347 100644 --- a/arch/arm/dts/imx8mn-venice-u-boot.dtsi +++ b/arch/arm/dts/imx8mn-venice-u-boot.dtsi @@ -128,25 +128,25 @@
imem_1d: blob-ext@1 { filename = "lpddr4_pmu_train_1d_imem.bin"; - size = <0x8000>; + align-end = <4>; type = "blob-ext"; };
dmem_1d: blob-ext@2 { filename = "lpddr4_pmu_train_1d_dmem.bin"; - size = <0x4000>; + align-end = <4>; type = "blob-ext"; };
imem_2d: blob-ext@3 { filename = "lpddr4_pmu_train_2d_imem.bin"; - size = <0x8000>; + align-end = <4>; type = "blob-ext"; };
dmem_2d: blob-ext@4 { filename = "lpddr4_pmu_train_2d_dmem.bin"; - size = <0x4000>; + align-end = <4>; type = "blob-ext"; }; }; diff --git a/arch/arm/dts/imx8mp-u-boot.dtsi b/arch/arm/dts/imx8mp-u-boot.dtsi index 20edd90cfad..274515a010e 100644 --- a/arch/arm/dts/imx8mp-u-boot.dtsi +++ b/arch/arm/dts/imx8mp-u-boot.dtsi @@ -63,22 +63,22 @@
blob_1: blob-ext@1 { filename = "lpddr4_pmu_train_1d_imem_202006.bin"; - size = <0x8000>; + align-end = <4>; };
blob_2: blob-ext@2 { filename = "lpddr4_pmu_train_1d_dmem_202006.bin"; - size = <0x4000>; + align-end = <4>; };
blob_3: blob-ext@3 { filename = "lpddr4_pmu_train_2d_imem_202006.bin"; - size = <0x8000>; + align-end = <4>; };
blob_4: blob-ext@4 { filename = "lpddr4_pmu_train_2d_dmem_202006.bin"; - size = <0x4000>; + align-end = <4>; }; };
diff --git a/arch/arm/dts/imx8mq-cm-u-boot.dtsi b/arch/arm/dts/imx8mq-cm-u-boot.dtsi index e2f4b0e740d..9538a04ed97 100644 --- a/arch/arm/dts/imx8mq-cm-u-boot.dtsi +++ b/arch/arm/dts/imx8mq-cm-u-boot.dtsi @@ -30,22 +30,22 @@
blob_1: blob-ext@1 { filename = "lpddr4_pmu_train_1d_imem.bin"; - size = <0x8000>; + align-end = <4>; };
blob_2: blob-ext@2 { filename = "lpddr4_pmu_train_1d_dmem.bin"; - size = <0x4000>; + align-end = <4>; };
blob_3: blob-ext@3 { filename = "lpddr4_pmu_train_2d_imem.bin"; - size = <0x8000>; + align-end = <4>; };
blob_4: blob-ext@4 { filename = "lpddr4_pmu_train_2d_dmem.bin"; - size = <0x4000>; + align-end = <4>; }; };
diff --git a/arch/arm/dts/imx8mq-u-boot.dtsi b/arch/arm/dts/imx8mq-u-boot.dtsi index 389414ad26f..1495869fcd2 100644 --- a/arch/arm/dts/imx8mq-u-boot.dtsi +++ b/arch/arm/dts/imx8mq-u-boot.dtsi @@ -48,25 +48,25 @@
imem_1d: blob-ext@1 { filename = "lpddr4_pmu_train_1d_imem.bin"; - size = <0x8000>; + align-end = <4>; type = "blob-ext"; };
dmem_1d: blob-ext@2 { filename = "lpddr4_pmu_train_1d_dmem.bin"; - size = <0x4000>; + align-end = <4>; type = "blob-ext"; };
imem_2d: blob-ext@3 { filename = "lpddr4_pmu_train_2d_imem.bin"; - size = <0x8000>; + align-end = <4>; type = "blob-ext"; };
dmem_2d: blob-ext@4 { filename = "lpddr4_pmu_train_2d_dmem.bin"; - size = <0x4000>; + align-end = <4>; type = "blob-ext"; }; };

On 20/05/2022 17:10, Peng Fan (OSS) wrote:
From: Peng Fan peng.fan@nxp.com
After we switch to use BINMAN_SYMBOLS, there is no need to pad the file size to 0x8000 and 0x4000. After we use BINMAN_SYMBOLS, the u-boot-spl-ddr.bin shrink about 36KB with i.MX8MP-EVK.
Tested-by: Tim Harvey tharvey@gateworks.com #imx8m[m,n,p]-venice Signed-off-by: Peng Fan peng.fan@nxp.com
arch/arm/dts/imx8mm-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mn-beacon-kit-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mn-evk-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mn-venice-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mp-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mq-cm-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mq-u-boot.dtsi | 8 ++++----
Probably can be done for 'imx8mn-bsh-smm-s2-u-boot-common.dtsi' as well.
9 files changed, 36 insertions(+), 36 deletions(-)
diff --git a/arch/arm/dts/imx8mm-u-boot.dtsi b/arch/arm/dts/imx8mm-u-boot.dtsi index 5de55a2d80b..19a2da30f51 100644 --- a/arch/arm/dts/imx8mm-u-boot.dtsi +++ b/arch/arm/dts/imx8mm-u-boot.dtsi @@ -41,25 +41,25 @@
imem_1d: blob-ext@1 { filename = "lpddr4_pmu_train_1d_imem.bin";
size = <0x8000>;
align-end = <4>; type = "blob-ext";
};
dmem_1d: blob-ext@2 { filename = "lpddr4_pmu_train_1d_dmem.bin";
size = <0x4000>;
align-end = <4>; type = "blob-ext";
};
imem_2d: blob-ext@3 { filename = "lpddr4_pmu_train_2d_imem.bin";
size = <0x8000>;
align-end = <4>; type = "blob-ext";
};
dmem_2d: blob-ext@4 { filename = "lpddr4_pmu_train_2d_dmem.bin";
size = <0x4000>;
}; };align-end = <4>; type = "blob-ext";
diff --git a/arch/arm/dts/imx8mn-beacon-kit-u-boot.dtsi b/arch/arm/dts/imx8mn-beacon-kit-u-boot.dtsi index eb1dd8debba..e1740fa31a6 100644 --- a/arch/arm/dts/imx8mn-beacon-kit-u-boot.dtsi +++ b/arch/arm/dts/imx8mn-beacon-kit-u-boot.dtsi @@ -149,22 +149,22 @@
blob_1: blob-ext@1 { filename = "lpddr4_pmu_train_1d_imem.bin";
size = <0x8000>;
align-end = <4>;
};
blob_2: blob-ext@2 { filename = "lpddr4_pmu_train_1d_dmem.bin";
size = <0x4000>;
align-end = <4>;
};
blob_3: blob-ext@3 { filename = "lpddr4_pmu_train_2d_imem.bin";
size = <0x8000>;
align-end = <4>;
};
blob_4: blob-ext@4 { filename = "lpddr4_pmu_train_2d_dmem.bin";
size = <0x4000>;
}; };align-end = <4>;
diff --git a/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi b/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi index 4d0ecb07d4f..1fe2d0fd507 100644 --- a/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi +++ b/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi @@ -157,22 +157,22 @@
blob_1: blob-ext@1 { filename = "ddr4_imem_1d_201810.bin";
size = <0x8000>;
align-end = <4>;
};
blob_2: blob-ext@2 { filename = "ddr4_dmem_1d_201810.bin";
size = <0x4000>;
align-end = <4>;
};
blob_3: blob-ext@3 { filename = "ddr4_imem_2d_201810.bin";
size = <0x8000>;
align-end = <4>;
};
blob_4: blob-ext@4 { filename = "ddr4_dmem_2d_201810.bin";
size = <0x4000>;
}; };align-end = <4>;
diff --git a/arch/arm/dts/imx8mn-evk-u-boot.dtsi b/arch/arm/dts/imx8mn-evk-u-boot.dtsi index 3db46d4cbcb..4f6dcf307b2 100644 --- a/arch/arm/dts/imx8mn-evk-u-boot.dtsi +++ b/arch/arm/dts/imx8mn-evk-u-boot.dtsi @@ -38,22 +38,22 @@
blob_1: blob-ext@1 { filename = "lpddr4_pmu_train_1d_imem.bin";
size = <0x8000>;
align-end = <4>;
};
blob_2: blob-ext@2 { filename = "lpddr4_pmu_train_1d_dmem.bin";
size = <0x4000>;
align-end = <4>;
};
blob_3: blob-ext@3 { filename = "lpddr4_pmu_train_2d_imem.bin";
size = <0x8000>;
align-end = <4>;
};
blob_4: blob-ext@4 { filename = "lpddr4_pmu_train_2d_dmem.bin";
size = <0x4000>;
}; };align-end = <4>;
diff --git a/arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi b/arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi index 001e725f568..32ef7929288 100644 --- a/arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi +++ b/arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi @@ -132,25 +132,25 @@
blob_1: blob-ext@1 { filename = "ddr4_imem_1d.bin";
size = <0x8000>;
align-end = <4>; type = "blob-ext";
};
blob_2: blob-ext@2 { filename = "ddr4_dmem_1d.bin";
size = <0x4000>;
align-end = <4>; type = "blob-ext";
};
blob_3: blob-ext@3 { filename = "ddr4_imem_2d.bin";
size = <0x8000>;
align-end = <4>; type = "blob-ext";
};
blob_4: blob-ext@4 { filename = "ddr4_dmem_2d.bin";
size = <0x4000>;
}; };align-end = <4>; type = "blob-ext";
diff --git a/arch/arm/dts/imx8mn-venice-u-boot.dtsi b/arch/arm/dts/imx8mn-venice-u-boot.dtsi index 67922146963..e3a0b170347 100644 --- a/arch/arm/dts/imx8mn-venice-u-boot.dtsi +++ b/arch/arm/dts/imx8mn-venice-u-boot.dtsi @@ -128,25 +128,25 @@
imem_1d: blob-ext@1 { filename = "lpddr4_pmu_train_1d_imem.bin";
size = <0x8000>;
align-end = <4>; type = "blob-ext";
};
dmem_1d: blob-ext@2 { filename = "lpddr4_pmu_train_1d_dmem.bin";
size = <0x4000>;
align-end = <4>; type = "blob-ext";
};
imem_2d: blob-ext@3 { filename = "lpddr4_pmu_train_2d_imem.bin";
size = <0x8000>;
align-end = <4>; type = "blob-ext";
};
dmem_2d: blob-ext@4 { filename = "lpddr4_pmu_train_2d_dmem.bin";
size = <0x4000>;
}; };align-end = <4>; type = "blob-ext";
diff --git a/arch/arm/dts/imx8mp-u-boot.dtsi b/arch/arm/dts/imx8mp-u-boot.dtsi index 20edd90cfad..274515a010e 100644 --- a/arch/arm/dts/imx8mp-u-boot.dtsi +++ b/arch/arm/dts/imx8mp-u-boot.dtsi @@ -63,22 +63,22 @@
blob_1: blob-ext@1 { filename = "lpddr4_pmu_train_1d_imem_202006.bin";
size = <0x8000>;
align-end = <4>;
};
blob_2: blob-ext@2 { filename = "lpddr4_pmu_train_1d_dmem_202006.bin";
size = <0x4000>;
align-end = <4>;
};
blob_3: blob-ext@3 { filename = "lpddr4_pmu_train_2d_imem_202006.bin";
size = <0x8000>;
align-end = <4>;
};
blob_4: blob-ext@4 { filename = "lpddr4_pmu_train_2d_dmem_202006.bin";
size = <0x4000>;
}; };align-end = <4>;
diff --git a/arch/arm/dts/imx8mq-cm-u-boot.dtsi b/arch/arm/dts/imx8mq-cm-u-boot.dtsi index e2f4b0e740d..9538a04ed97 100644 --- a/arch/arm/dts/imx8mq-cm-u-boot.dtsi +++ b/arch/arm/dts/imx8mq-cm-u-boot.dtsi @@ -30,22 +30,22 @@
blob_1: blob-ext@1 { filename = "lpddr4_pmu_train_1d_imem.bin";
size = <0x8000>;
align-end = <4>;
};
blob_2: blob-ext@2 { filename = "lpddr4_pmu_train_1d_dmem.bin";
size = <0x4000>;
align-end = <4>;
};
blob_3: blob-ext@3 { filename = "lpddr4_pmu_train_2d_imem.bin";
size = <0x8000>;
align-end = <4>;
};
blob_4: blob-ext@4 { filename = "lpddr4_pmu_train_2d_dmem.bin";
size = <0x4000>;
}; };align-end = <4>;
diff --git a/arch/arm/dts/imx8mq-u-boot.dtsi b/arch/arm/dts/imx8mq-u-boot.dtsi index 389414ad26f..1495869fcd2 100644 --- a/arch/arm/dts/imx8mq-u-boot.dtsi +++ b/arch/arm/dts/imx8mq-u-boot.dtsi @@ -48,25 +48,25 @@
imem_1d: blob-ext@1 { filename = "lpddr4_pmu_train_1d_imem.bin";
size = <0x8000>;
align-end = <4>; type = "blob-ext";
};
dmem_1d: blob-ext@2 { filename = "lpddr4_pmu_train_1d_dmem.bin";
size = <0x4000>;
align-end = <4>; type = "blob-ext";
};
imem_2d: blob-ext@3 { filename = "lpddr4_pmu_train_2d_imem.bin";
size = <0x8000>;
align-end = <4>; type = "blob-ext";
};
dmem_2d: blob-ext@4 { filename = "lpddr4_pmu_train_2d_dmem.bin";
size = <0x4000>;
}; };align-end = <4>; type = "blob-ext";

Hi
Il lun 23 mag 2022, 23:13 Alper Nebi Yasak alpernebiyasak@gmail.com ha scritto:
On 20/05/2022 17:10, Peng Fan (OSS) wrote:
From: Peng Fan peng.fan@nxp.com
After we switch to use BINMAN_SYMBOLS, there is no need to pad the file size to 0x8000 and 0x4000. After we use BINMAN_SYMBOLS, the u-boot-spl-ddr.bin shrink about 36KB with i.MX8MP-EVK.
Tested-by: Tim Harvey tharvey@gateworks.com #imx8m[m,n,p]-venice Signed-off-by: Peng Fan peng.fan@nxp.com
arch/arm/dts/imx8mm-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mn-beacon-kit-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mn-evk-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mn-venice-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mp-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mq-cm-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mq-u-boot.dtsi | 8 ++++----
Probably can be done for 'imx8mn-bsh-smm-s2-u-boot-common.dtsi' as well.
Let us test.
I did not find time
Michael
9 files changed, 36 insertions(+), 36 deletions(-)
diff --git a/arch/arm/dts/imx8mm-u-boot.dtsi
b/arch/arm/dts/imx8mm-u-boot.dtsi
index 5de55a2d80b..19a2da30f51 100644 --- a/arch/arm/dts/imx8mm-u-boot.dtsi +++ b/arch/arm/dts/imx8mm-u-boot.dtsi @@ -41,25 +41,25 @@
imem_1d: blob-ext@1 { filename = "lpddr4_pmu_train_1d_imem.bin";
size = <0x8000>;
align-end = <4>; type = "blob-ext"; }; dmem_1d: blob-ext@2 { filename = "lpddr4_pmu_train_1d_dmem.bin";
size = <0x4000>;
align-end = <4>; type = "blob-ext"; }; imem_2d: blob-ext@3 { filename = "lpddr4_pmu_train_2d_imem.bin";
size = <0x8000>;
align-end = <4>; type = "blob-ext"; }; dmem_2d: blob-ext@4 { filename = "lpddr4_pmu_train_2d_dmem.bin";
size = <0x4000>;
align-end = <4>; type = "blob-ext"; }; };
diff --git a/arch/arm/dts/imx8mn-beacon-kit-u-boot.dtsi
b/arch/arm/dts/imx8mn-beacon-kit-u-boot.dtsi
index eb1dd8debba..e1740fa31a6 100644 --- a/arch/arm/dts/imx8mn-beacon-kit-u-boot.dtsi +++ b/arch/arm/dts/imx8mn-beacon-kit-u-boot.dtsi @@ -149,22 +149,22 @@
blob_1: blob-ext@1 { filename = "lpddr4_pmu_train_1d_imem.bin";
size = <0x8000>;
align-end = <4>; }; blob_2: blob-ext@2 { filename = "lpddr4_pmu_train_1d_dmem.bin";
size = <0x4000>;
align-end = <4>; }; blob_3: blob-ext@3 { filename = "lpddr4_pmu_train_2d_imem.bin";
size = <0x8000>;
align-end = <4>; }; blob_4: blob-ext@4 { filename = "lpddr4_pmu_train_2d_dmem.bin";
size = <0x4000>;
align-end = <4>; }; };
diff --git a/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi
b/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi
index 4d0ecb07d4f..1fe2d0fd507 100644 --- a/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi +++ b/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi @@ -157,22 +157,22 @@
blob_1: blob-ext@1 { filename = "ddr4_imem_1d_201810.bin";
size = <0x8000>;
align-end = <4>; }; blob_2: blob-ext@2 { filename = "ddr4_dmem_1d_201810.bin";
size = <0x4000>;
align-end = <4>; }; blob_3: blob-ext@3 { filename = "ddr4_imem_2d_201810.bin";
size = <0x8000>;
align-end = <4>; }; blob_4: blob-ext@4 { filename = "ddr4_dmem_2d_201810.bin";
size = <0x4000>;
align-end = <4>; }; };
diff --git a/arch/arm/dts/imx8mn-evk-u-boot.dtsi
b/arch/arm/dts/imx8mn-evk-u-boot.dtsi
index 3db46d4cbcb..4f6dcf307b2 100644 --- a/arch/arm/dts/imx8mn-evk-u-boot.dtsi +++ b/arch/arm/dts/imx8mn-evk-u-boot.dtsi @@ -38,22 +38,22 @@
blob_1: blob-ext@1 { filename = "lpddr4_pmu_train_1d_imem.bin";
size = <0x8000>;
align-end = <4>; }; blob_2: blob-ext@2 { filename = "lpddr4_pmu_train_1d_dmem.bin";
size = <0x4000>;
align-end = <4>; }; blob_3: blob-ext@3 { filename = "lpddr4_pmu_train_2d_imem.bin";
size = <0x8000>;
align-end = <4>; }; blob_4: blob-ext@4 { filename = "lpddr4_pmu_train_2d_dmem.bin";
size = <0x4000>;
align-end = <4>; }; };
diff --git a/arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi
b/arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi
index 001e725f568..32ef7929288 100644 --- a/arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi +++ b/arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi @@ -132,25 +132,25 @@
blob_1: blob-ext@1 { filename = "ddr4_imem_1d.bin";
size = <0x8000>;
align-end = <4>; type = "blob-ext"; }; blob_2: blob-ext@2 { filename = "ddr4_dmem_1d.bin";
size = <0x4000>;
align-end = <4>; type = "blob-ext"; }; blob_3: blob-ext@3 { filename = "ddr4_imem_2d.bin";
size = <0x8000>;
align-end = <4>; type = "blob-ext"; }; blob_4: blob-ext@4 { filename = "ddr4_dmem_2d.bin";
size = <0x4000>;
align-end = <4>; type = "blob-ext"; }; };
diff --git a/arch/arm/dts/imx8mn-venice-u-boot.dtsi
b/arch/arm/dts/imx8mn-venice-u-boot.dtsi
index 67922146963..e3a0b170347 100644 --- a/arch/arm/dts/imx8mn-venice-u-boot.dtsi +++ b/arch/arm/dts/imx8mn-venice-u-boot.dtsi @@ -128,25 +128,25 @@
imem_1d: blob-ext@1 { filename = "lpddr4_pmu_train_1d_imem.bin";
size = <0x8000>;
align-end = <4>; type = "blob-ext"; }; dmem_1d: blob-ext@2 { filename = "lpddr4_pmu_train_1d_dmem.bin";
size = <0x4000>;
align-end = <4>; type = "blob-ext"; }; imem_2d: blob-ext@3 { filename = "lpddr4_pmu_train_2d_imem.bin";
size = <0x8000>;
align-end = <4>; type = "blob-ext"; }; dmem_2d: blob-ext@4 { filename = "lpddr4_pmu_train_2d_dmem.bin";
size = <0x4000>;
align-end = <4>; type = "blob-ext"; }; };
diff --git a/arch/arm/dts/imx8mp-u-boot.dtsi
b/arch/arm/dts/imx8mp-u-boot.dtsi
index 20edd90cfad..274515a010e 100644 --- a/arch/arm/dts/imx8mp-u-boot.dtsi +++ b/arch/arm/dts/imx8mp-u-boot.dtsi @@ -63,22 +63,22 @@
blob_1: blob-ext@1 { filename = "lpddr4_pmu_train_1d_imem_202006.bin";
size = <0x8000>;
align-end = <4>; }; blob_2: blob-ext@2 { filename = "lpddr4_pmu_train_1d_dmem_202006.bin";
size = <0x4000>;
align-end = <4>; }; blob_3: blob-ext@3 { filename = "lpddr4_pmu_train_2d_imem_202006.bin";
size = <0x8000>;
align-end = <4>; }; blob_4: blob-ext@4 { filename = "lpddr4_pmu_train_2d_dmem_202006.bin";
size = <0x4000>;
align-end = <4>; }; };
diff --git a/arch/arm/dts/imx8mq-cm-u-boot.dtsi
b/arch/arm/dts/imx8mq-cm-u-boot.dtsi
index e2f4b0e740d..9538a04ed97 100644 --- a/arch/arm/dts/imx8mq-cm-u-boot.dtsi +++ b/arch/arm/dts/imx8mq-cm-u-boot.dtsi @@ -30,22 +30,22 @@
blob_1: blob-ext@1 { filename = "lpddr4_pmu_train_1d_imem.bin";
size = <0x8000>;
align-end = <4>; }; blob_2: blob-ext@2 { filename = "lpddr4_pmu_train_1d_dmem.bin";
size = <0x4000>;
align-end = <4>; }; blob_3: blob-ext@3 { filename = "lpddr4_pmu_train_2d_imem.bin";
size = <0x8000>;
align-end = <4>; }; blob_4: blob-ext@4 { filename = "lpddr4_pmu_train_2d_dmem.bin";
size = <0x4000>;
align-end = <4>; }; };
diff --git a/arch/arm/dts/imx8mq-u-boot.dtsi
b/arch/arm/dts/imx8mq-u-boot.dtsi
index 389414ad26f..1495869fcd2 100644 --- a/arch/arm/dts/imx8mq-u-boot.dtsi +++ b/arch/arm/dts/imx8mq-u-boot.dtsi @@ -48,25 +48,25 @@
imem_1d: blob-ext@1 { filename = "lpddr4_pmu_train_1d_imem.bin";
size = <0x8000>;
align-end = <4>; type = "blob-ext"; }; dmem_1d: blob-ext@2 { filename = "lpddr4_pmu_train_1d_dmem.bin";
size = <0x4000>;
align-end = <4>; type = "blob-ext"; }; imem_2d: blob-ext@3 { filename = "lpddr4_pmu_train_2d_imem.bin";
size = <0x8000>;
align-end = <4>; type = "blob-ext"; }; dmem_2d: blob-ext@4 { filename = "lpddr4_pmu_train_2d_dmem.bin";
size = <0x4000>;
align-end = <4>; type = "blob-ext"; }; };

From: Peng Fan peng.fan@nxp.com
There is case that CONFIG_BINMAN is defined, but CONFIG_SPL_BINMAN_SYMBOLS is not defined. In that case, there will be build failure. So use CONFIG_SPL_BINMAN_SYMBOLS to guard the macros, and define CONFIG_SPL_BINMAN_SYMBOLS in binman syms test.
Tested-by: Tim Harvey tharvey@gateworks.com #imx8m[m,n,p]-venice Signed-off-by: Peng Fan peng.fan@nxp.com --- include/binman_sym.h | 2 +- tools/binman/test/u_boot_binman_syms.c | 1 + tools/binman/test/u_boot_binman_syms_size.c | 1 + 3 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/binman_sym.h b/include/binman_sym.h index 72e6765fe52..548d8f5654c 100644 --- a/include/binman_sym.h +++ b/include/binman_sym.h @@ -13,7 +13,7 @@
#define BINMAN_SYM_MISSING (-1UL)
-#ifdef CONFIG_BINMAN +#ifdef CONFIG_SPL_BINMAN_SYMBOLS
/** * binman_symname() - Internal function to get a binman symbol name diff --git a/tools/binman/test/u_boot_binman_syms.c b/tools/binman/test/u_boot_binman_syms.c index 37fc339ce84..f4a4d1f6846 100644 --- a/tools/binman/test/u_boot_binman_syms.c +++ b/tools/binman/test/u_boot_binman_syms.c @@ -6,6 +6,7 @@ */
#define CONFIG_BINMAN +#define CONFIG_SPL_BINMAN_SYMBOLS #include <binman_sym.h>
binman_sym_declare(unsigned long, u_boot_spl_any, offset); diff --git a/tools/binman/test/u_boot_binman_syms_size.c b/tools/binman/test/u_boot_binman_syms_size.c index 7224bc1863c..3a01d8ca4be 100644 --- a/tools/binman/test/u_boot_binman_syms_size.c +++ b/tools/binman/test/u_boot_binman_syms_size.c @@ -6,6 +6,7 @@ */
#define CONFIG_BINMAN +#define CONFIG_SPL_BINMAN_SYMBOLS #include <binman_sym.h>
binman_sym_declare(char, u_boot_spl, pos);

On 20/05/2022 17:10, Peng Fan (OSS) wrote:
From: Peng Fan peng.fan@nxp.com
There is case that CONFIG_BINMAN is defined, but CONFIG_SPL_BINMAN_SYMBOLS is not defined. In that case, there will be build failure. So use CONFIG_SPL_BINMAN_SYMBOLS to guard the macros, and define CONFIG_SPL_BINMAN_SYMBOLS in binman syms test.
I guess they should be CONFIG_IS_ENABLED(BINMAN_SYMBOLS) instead, as there's also a CONFIG_TPL_BINMAN_SYMBOLS.
Tested-by: Tim Harvey tharvey@gateworks.com #imx8m[m,n,p]-venice Signed-off-by: Peng Fan peng.fan@nxp.com
include/binman_sym.h | 2 +- tools/binman/test/u_boot_binman_syms.c | 1 + tools/binman/test/u_boot_binman_syms_size.c | 1 + 3 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/binman_sym.h b/include/binman_sym.h index 72e6765fe52..548d8f5654c 100644 --- a/include/binman_sym.h +++ b/include/binman_sym.h @@ -13,7 +13,7 @@
#define BINMAN_SYM_MISSING (-1UL)
-#ifdef CONFIG_BINMAN +#ifdef CONFIG_SPL_BINMAN_SYMBOLS
/**
- binman_symname() - Internal function to get a binman symbol name
diff --git a/tools/binman/test/u_boot_binman_syms.c b/tools/binman/test/u_boot_binman_syms.c index 37fc339ce84..f4a4d1f6846 100644 --- a/tools/binman/test/u_boot_binman_syms.c +++ b/tools/binman/test/u_boot_binman_syms.c @@ -6,6 +6,7 @@ */
#define CONFIG_BINMAN +#define CONFIG_SPL_BINMAN_SYMBOLS #include <binman_sym.h>
binman_sym_declare(unsigned long, u_boot_spl_any, offset); diff --git a/tools/binman/test/u_boot_binman_syms_size.c b/tools/binman/test/u_boot_binman_syms_size.c index 7224bc1863c..3a01d8ca4be 100644 --- a/tools/binman/test/u_boot_binman_syms_size.c +++ b/tools/binman/test/u_boot_binman_syms_size.c @@ -6,6 +6,7 @@ */
#define CONFIG_BINMAN +#define CONFIG_SPL_BINMAN_SYMBOLS #include <binman_sym.h>
binman_sym_declare(char, u_boot_spl, pos);

Subject: Re: [PATCH V4 8/8] binman_sym: guard with CONFIG_SPL_BINMAN_SYMBOLS
On 20/05/2022 17:10, Peng Fan (OSS) wrote:
From: Peng Fan peng.fan@nxp.com
There is case that CONFIG_BINMAN is defined, but CONFIG_SPL_BINMAN_SYMBOLS is not defined. In that case, there will be build failure. So use CONFIG_SPL_BINMAN_SYMBOLS to guard the macros, and define CONFIG_SPL_BINMAN_SYMBOLS in binman syms test.
I guess they should be CONFIG_IS_ENABLED(BINMAN_SYMBOLS) instead, as there's also a CONFIG_TPL_BINMAN_SYMBOLS.
No. CONFIG_IS_ENABLED not work, because there is no defconfig generated.
Regards, Peng.
Tested-by: Tim Harvey tharvey@gateworks.com #imx8m[m,n,p]-venice Signed-off-by: Peng Fan peng.fan@nxp.com
include/binman_sym.h | 2 +- tools/binman/test/u_boot_binman_syms.c | 1 + tools/binman/test/u_boot_binman_syms_size.c | 1 + 3 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/binman_sym.h b/include/binman_sym.h index 72e6765fe52..548d8f5654c 100644 --- a/include/binman_sym.h +++ b/include/binman_sym.h @@ -13,7 +13,7 @@
#define BINMAN_SYM_MISSING (-1UL)
-#ifdef CONFIG_BINMAN +#ifdef CONFIG_SPL_BINMAN_SYMBOLS
/**
- binman_symname() - Internal function to get a binman symbol name
diff --git a/tools/binman/test/u_boot_binman_syms.c b/tools/binman/test/u_boot_binman_syms.c index 37fc339ce84..f4a4d1f6846 100644 --- a/tools/binman/test/u_boot_binman_syms.c +++ b/tools/binman/test/u_boot_binman_syms.c @@ -6,6 +6,7 @@ */
#define CONFIG_BINMAN +#define CONFIG_SPL_BINMAN_SYMBOLS #include <binman_sym.h>
binman_sym_declare(unsigned long, u_boot_spl_any, offset); diff --git a/tools/binman/test/u_boot_binman_syms_size.c b/tools/binman/test/u_boot_binman_syms_size.c index 7224bc1863c..3a01d8ca4be 100644 --- a/tools/binman/test/u_boot_binman_syms_size.c +++ b/tools/binman/test/u_boot_binman_syms_size.c @@ -6,6 +6,7 @@ */
#define CONFIG_BINMAN +#define CONFIG_SPL_BINMAN_SYMBOLS #include <binman_sym.h>
binman_sym_declare(char, u_boot_spl, pos);
participants (5)
-
Alper Nebi Yasak
-
Michael Nazzareno Trimarchi
-
Peng Fan
-
Peng Fan (OSS)
-
Tom Rini