Re: [PATCH] Kbuild: include arch/ Makefile before other directories

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

On 13.06.23 11:16, Sascha Hauer wrote:
> On Mon, Jun 05, 2023 at 08:37:19AM +0200, Ahmad Fatoum wrote:
>> Unless overridden by SORT*, LD will place sections matched by wildcards
>> in the order they were seen in the link. So far, this meant that
>> arch/ initcalls and device trees compiled into barebox proper, were
>> always last.
>>
>> For platforms with PBL or with only one device tree in barebox proper,
>> this didn't matter much, but when enabling the of_manipulation selftest,
>> a second device tree would be built into barebox on kvx, openrisc and
>> some MIPS.
>>
>> Because all directories appear before arch/, this had the effect that
>> on kvx, openrisc and some MIPS, __dtb_start would end up pointing at the
>> test's device tree instead of the board device tree breaking the build.
>>
>> Switching the affected platforms to use PBL would be one fix for the
>> issue, but there's a simple one: let's do what Linux does in its
>> top-level Kbuild file and have arch/ not be the last in sequence.
>>
>> This may fause fallout for code that depended on specific ordering of
>> initcalls or other linker-defined lists, which would need to be fixed.
> 
> I am afraid this causes regressions that take a long time to realize and
> to sort out.
>
> We could do something along the following lines to pick the correct dtb.
> This is only meant as an example, it likely needs some sugar to work
> correctly in all cases.

Doesn't work for kvx, which doesn't set CONFIG_BUILTIN_DTB_NAME
in the defconfig. I am fine with an alternative that lets me run
selftests on openrisc.

Cheers,
Ahmad

> 
> Sascha
> 
> ----------------------------------8<------------------------------
> 
> diff --git a/arch/mips/boot/Makefile b/arch/mips/boot/Makefile
> index d1e27b5e6b..e114cc5d8a 100644
> --- a/arch/mips/boot/Makefile
> +++ b/arch/mips/boot/Makefile
> @@ -3,6 +3,7 @@
>  obj-y += start.o
>  obj-y += main_entry.o
>  
> +CFLAGS_dtb.o += -DBUILTIN_DTB_SYMBOL=__dtb_$(patsubst "%",%,$(subst -,_,$(CONFIG_BUILTIN_DTB_NAME)))_start
>  obj-$(CONFIG_OFDEVICE) += dtb.o
>  
>  pbl-y += main_entry-pbl.o
> diff --git a/arch/mips/boot/dtb.c b/arch/mips/boot/dtb.c
> index ece1494e5f..3afcd20a0e 100644
> --- a/arch/mips/boot/dtb.c
> +++ b/arch/mips/boot/dtb.c
> @@ -34,7 +34,7 @@ int of_add_memory_bank(struct device_node *node, bool dump, int r,
>  	return ret;
>  }
>  
> -extern char __dtb_start[];
> +extern char BUILTIN_DTB_SYMBOL[];
>  
>  static int of_mips_init(void)
>  {
> @@ -42,7 +42,7 @@ static int of_mips_init(void)
>  
>  	fdt = glob_fdt;
>  	if (!fdt)
> -		fdt = __dtb_start;
> +		fdt = BUILTIN_DTB_SYMBOL;
>  
>  	return barebox_register_fdt(fdt);
>  }

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |