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

Marek Vasut
2022-06-07T22:55:02Z

On 6/7/22 22:54, Alper Nebi Yasak wrote:
> 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.

I still don't see why we should randomly damage board configs to work
around what looks like a bug in binman -- are we now implementing
workarounds instead of trying to fully understand issues and implement
proper fixes for those ?

Why can't we fix binman instead ?