Re: [PATCH 2/8] configs: imx8mm_data_modul_edm_sbc: not select SPL_RAM_DEVICE

From: Alper Nebi Yasak <alpernebiyasak@gmail.com>
To: Marek Vasut <marex@denx.de>
Cc: u-boot@lists.denx.de, Peng Fan <peng.fan@nxp.com>,
	"Peng Fan (OSS)" <peng.fan@oss.nxp.com>,
	sbabic@denx.de, festevam@gmail.com, trini@konsulko.com
Subject: Re: [PATCH 2/8] configs: imx8mm_data_modul_edm_sbc: not select SPL_RAM_DEVICE
Date: Tue, 7 Jun 2022 23:54:20 +0300	[thread overview]
Message-ID: <0b62a04b-8472-8f49-82ca-d1ba684d2e16@gmail.com> (raw)
In-Reply-To: <c842cee2-38c4-d704-7db8-af25552971f9@denx.de>

On 07/06/2022 21:11, Marek Vasut wrote:
> On 6/7/22 19:26, Alper Nebi Yasak wrote:
>> On 06/06/2022 17:07, Marek Vasut wrote:
>>> On 6/3/22 09:17, Peng Fan (OSS) wrote:
>>>> From: Peng Fan <peng.fan@nxp.com>
>>>>
>>>> i.MX8M use FIT image, not RAW image. And to support binman symbols,
>>>> u_boot_any could be optimized if RAW image is not selected, otherwise
>>>> there will be build failure. So not select SPL_RAW_DEVICE
>>>
>>> Is it RAW device/image or RAM device/image ? There seem to be some
>>> confusion here. It also seems this might break start of U-Boot by JTAG
>>> upload, right ?
>>
>> The previous patch disables SPL_RAW_IMAGE_SUPPORT for i.MX8M boards
>> (already disabled for this), this one disables SPL_RAM_DEVICE (was only
>> enabled for this). Both need to be disabled for this series in its
>> current form to avoid a binman symbol-related error, but this commit
>> message suffers from being copy-paste of the previous one.
> 
> OK, I am really confused now. What binman symbol error ?

When BINMAN_SYMBOLS is enabled, some binman 'u_boot_any' symbols are
declared in common/spl/spl.c. They are marked as 'unused', but if
anything actually uses them they survive optimization and show up in the
spl/u-boot-spl ELF file. When binman is building an image including a
'u-boot-spl' entry, it sees these symbols and tries to fill them in with
appropriate values about 'u-boot'-like entries. If there is no such
entry in the same image as the 'u-boot-spl', binman raises an error
about this.

In this case, SPL_RAW_IMAGE_SUPPORT and SPL_RAM_DEVICE both enable code
that uses the 'u_boot_any' symbols. However, the i.MX8M binman image
descriptions try to specify the binman images in a modular way, and one
of the images has a 'u-boot-spl' without any 'u-boot'-like entries.
Which means enabling the configs ends up triggering the error above.

See my reply to an earlier version [1] (this is actually v6 of this
series) about what could be done to solve this properly. Also see
previous discussions [2][3] for more info.

[1] Re: [PATCH V4 1/8] spl: guard u_boot_any with X86
https://lore.kernel.org/u-boot/334898af-f495-acb5-0c5a-1f4a9acce66e@gmail.com/

[2] using binman fails boot
https://lore.kernel.org/u-boot/CAJ+vNU0BZDr2q0ZPQkoQBP1eBhbYmQfJMYraSgOvWXwZ=yFReQ@mail.gmail.com/T/#u

[3] imx8: ls1028a: Drop raw image support
https://lore.kernel.org/u-boot/20210801205951.2202789-1-sjg@chromium.org/T/#u

> Is this some attempt at papering over a bug ?
Simon recommended disabling SPL_RAW_IMAGE_SUPPORT to resolve this error
in the previous discussions [3]. But I'm leaning towards saying yes
here, I think it's a workaround to something that should change on the
binman side. I'm not exactly sure what or how. Just now I thought maybe
we could add a BINMAN_SYMBOLS_FOR_NEXT_PHASE config or so, which would
enable the 'u_boot_any' etc. symbols instead of BINMAN_SYMBOLS.

Regardless, I think this version is a good enough step in the right
direction and more changes can still be done afterwards, so I'm not
asking for a new version.