Re: [PATCH 2/3] rockchip: Fix SPL link error for Radxa ROCK Pi 4

[Jonas Karlman <jonas@kwiboo.se>]

Hi Quentin and Naoki,

On 2025-02-05 12:37, Quentin Schulz wrote:
> Hi Naoki,
> 
> On 1/29/25 2:25 PM, FUKAUMI Naoki wrote:
>> If CONFIG_EFI_CAPSULE_FIRMWARE_RAW, CONFIG_SPL_ENV_SUPPORT, and
>> CONFIG_SPL_DFU are enabled, linking u-boot-spl will fail.
>>
>>    LD      spl/u-boot-spl
>> ld.bfd: drivers/dfu/dfu.o: in function `dfu_init_env_entities':
>> /home/radxa/u-boot/drivers/dfu/dfu.c:173:(.text.dfu_init_env_entities+0x24): undefined reference to `set_dfu_alt_info'
>>
>> Compile mach-rockchip/board.c for SPL.
>>
>> Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>
>> ---
>>   arch/arm/mach-rockchip/Makefile | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-rockchip/Makefile b/arch/arm/mach-rockchip/Makefile
>> index 5e7edc99cdc..e697df7ebcb 100644
>> --- a/arch/arm/mach-rockchip/Makefile
>> +++ b/arch/arm/mach-rockchip/Makefile
>> @@ -22,10 +22,10 @@ ifeq ($(CONFIG_XPL_BUILD)$(CONFIG_TPL_BUILD),)
>>   # we can have the preprocessor correctly recognise both 0x0 and 0
>>   # meaning "turn it off".
>>   obj-y += boot_mode.o
>> -obj-$(CONFIG_ROCKCHIP_COMMON_BOARD) += board.o
>>   endif
>>   
>>   ifeq ($(CONFIG_TPL_BUILD),)
>> +obj-$(CONFIG_ROCKCHIP_COMMON_BOARD) += board.o
> 
> This is probably a tiny bit too enthusiastic.
> 
> This means the whole content of board.c will be compiled for SPL whereas 
> it currently is compiled only for proper.
> 
> This means we need to be careful about what's in board.c.
> 
> Most IS_ENABLED(CONFIG_<config>) should probably be changed to 
> CONFIG_IS_ENABLED(<config>) so that it handles SPL symbols too and 
> doesn't attempt to build stuff for SPL when only proper symbols are 
> selected (and not their SPL counterpart).
> 
> E.g.
> 
> - EFI_PARTITION
> - SYS_DCACHE_OFF
> - USB_GADGET
> - DM_USB_GADGET
> 
> seems to have SPL symbols too, but
> 
> - EFI_HAVE_CAPSULE_SUPPORT
> - ROCKCHIP_USB2_PHY
> - FASTBOOT
> - MISC_INIT_R
> - ROCKCHIP_EFUSE (though does only build for proper (see the use of 
> $(XPL_) in Makefile))
> - ROCKCHIP_OTP (though does only build for proper (see the use of 
> $(XPL_) in Makefile))
> - BOARD_RNG_SEED
> - RNG_ROCKCHIP
> 
> only have proper symbols.
> 
> Maybe it makes more sense to move the efi-specific part of board.c into 
> efi.c and build that one whenever we actually need it?
> 
> What do you think?

I fully agree, please move the efi-specific parts away from board.c.

In current state it is only the rock-pi-4c-rk3399 target that have use
of set_dfu_alt_info(), rockchip_capsule_update_board_setup() is broken
for rock-pi-4-rk3399, the code check for radxa,rockpi4b, and board
target use radxa,rockpi4a :-)

Regards,
Jonas

> 
> Cheers,
> Quentin