Re: [PATCH V4 1/8] spl: guard u_boot_any with X86

[Alper Nebi Yasak <alpernebiyasak@gmail.com>]

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).