[PATCH 3/3] rockchip: Fix binman error

[FUKAUMI Naoki]

Hi,

On 2/9/25 23:27, Simon Glass wrote:
> Hi Quentin,
> 
> On Thu, 6 Feb 2025 at 09:54, Quentin Schulz <quentin.schulz at cherry.de> wrote:
>>
>> Hi Simon,
>>
>> On 2/6/25 1:46 PM, Simon Glass wrote:
>>> Hi Quentin,
>>>
>>> On Wed, 5 Feb 2025 at 06:00, Quentin Schulz <quentin.schulz at cherry.de> wrote:
>>>>
>>>> Hi Naoki,
>>>>
>>>> On 1/29/25 2:25 PM, FUKAUMI Naoki wrote:
>>>>> After adding the below configs, binman produces an error.
>>>>>
>>>>>     CONFIG_SPL_DFU
>>>>>     CONFIG_SPL_ENV_SUPPORT
>>>>>     CONFIG_SPL_USB_DWC3_GENERIC
>>>>>     CONFIG_SPL_USB_GADGET
>>>>>
>>>>> binman: Section '/binman/simple-bin-spi': Symbol '_binman_u_boot_any_prop_size'
>>>>>       in entry '/binman/simple-bin-spi/mkimage/u-boot-spl/u-boot-spl-nodtb': Entry 'u-boot-any' not found in list (u-boot-tpl-nodtb,u-boot-tpl-dtb,u-boot-tpl,u-boot-spl-nodtb,u-boot-spl-dtb,u-boot-spl,mkimage,fit,simple-bin-spi)
>>>>>
>>>>> Add "no-write-symbols;" to u-boot-tpl and u-boot-spl.
>>>>>
>>>>
>>>> I'll be frank with you, I have no clue what's happening :)
>>>
>>> I means that simple-bin-spi has a binary inside it, u-boot-spl, which
>>> uses binman_sym_declare(ulong, u_boot_any, size). Most likely it is
>>> the one at the top of common/spl.c
>>>
>>> This symbol is filled in (by binman, at build-time) with the size of
>>> the U-Boot binary. If there is no such U-Boot image then it cannot do
>>> this.
>>>
>>
>> Why is it not failing right now? Why does it fail when you enable
>> CONFIG_SPL_DFU, CONFIG_SPL_ENV_SUPPORT, CONFIG_SPL_USB_DWC3_GENERIC,
>> CONFIG_SPL_USB_GADGET for Rock-Pi-4?
> 
> I added these to the end of configs/rock-pi-4-rk3399_defconfig
> 
> CONFIG_SPL_DFU=y
> CONFIG_SPL_ENV_SUPPORT=y
> CONFIG_SPL_USB_DWC3_GENERIC=y
> CONFIG_SPL_USB_GADGET=y
> CONFIG_DM_USB=y
> 
> then build and don't see these errors. Can you please give a little
> more detail so I can repeat this?

This occurs when adding the above CONFIG to the following defconfig and 
building on today's master and next branch.

  rock-4c-plus-rk3399_defconfig
  rock-4se-rk3399_defconfig
  rock-pi-4-rk3399_defconfig
  rock-pi-4c-rk3399_defconfig

Best regards,

--
FUKAUMI Naoki
Radxa Computer (Shenzhen) Co., Ltd.

>> This doesn't generate the
>> u-boot.itb file we need for simple-bin-spi as we anyway take the fit
>> blob generated by simple-bin instead.
>>
>> It would make sense to always fail if this relies on u-boot.itb to be
>> built for simple-bin-spi as well?
>>
>> I suggested to use only blobs from simple-bin in simple-bin-spi but
>> according to
>> https://docs.u-boot.org/en/latest/develop/package/binman.html#image-dependencies
>> this is undefined behavior. Maybe we should use a template
>> (https://docs.u-boot.org/en/latest/develop/package/binman.html#templates)
>> and rebuild everything for the SPI image as well. I saw such a patch in
>> your VBE part H series I believe, maybe it makes sense to have this
>> merged separately?
> 
> Yes I am adding a template in that series, but I would rather get that
> submitted, since Binman does currently process images in series.
> 
>>
>> This feels somewhat wrong though, if I'm not mistaken every binary used
>> in simple-bin and simple-bin-spi is the same, to the exception of how
>> the idbloader.img is generated (-T rkspi/rksd), so it doesn't make a lot
>> of sense to me to go through a complete rebuild of all binaries, instead
>> of just running mkimage a second time with the same set of binaries. But
>> if that's the only way to have something with defined behavior AND
>> working, then so be it.
> 
> For now at least I think it is best to keep the images independent,
> with a template to reduce the duplication.
> 
> Regards.
> 
> Simon
>