[PATCH 6/8] sunxi: board: Set fdtfile to match the DT chosen by SPL

[Samuel Holland]

On 9/21/20 7:43 PM, André Przywara wrote:
> On 03/09/2020 06:07, Samuel Holland wrote:
> 
> Hi,
> 
>> Previously, fdtfile was always the value in CONFIG_DEFAULT_DEVICE_TREE.
>> This meant that, regardless of the DT chosen by SPL (either by changing
>> the header in the image or by the selection code at runtime), Linux
>> always used the default DT.
>>
>> By using the name from the SPL header (which, because of the previous
>> commit, always matches the DT used by U-Boot proper), Linux also sees
>> the same board as U-Boot/SPL, even if the boot script later loads a DT
>> from disk.
> 
> I strongly frown upon proliferating the broken way of explicitly loading
> a DT from somewhere, when the DT embedded in U-Boot should be all we
> will ever need.

The embedded DT is only "all you ever need" if 1) your DT is 100% complete and
accurate (ha, ha, ha) or 2) you re-flash U-Boot every time the device tree
changes, which is risky and an unnecessary waste of flash memory write cycles.

Having a built-in DT that's always available and mostly works is quite useful,
but complaining that distros and users want to update their DTs without
patching, recompiling, and re-flashing U-Boot is frankly ridiculous. Especially
considering that the U-Boot DTs are usually out of date, and users might not
want to update the U-Boot code for various reasons.

> But making the selected DT available to U-Boot scripts doesn't really
> hurt or prevent us from doing it properly, so:
> 
>> Signed-off-by: Samuel Holland <samuel at sholland.org>
> 
> One nit below, with that:
> Reviewed-by: Andre Przywara <andre.przywara at arm.com>
> 
> Cheers,
> Andre
> 
>> ---
>>  board/sunxi/board.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
>> index eaa40a1ea96..5457b28e135 100644
>> --- a/board/sunxi/board.c
>> +++ b/board/sunxi/board.c
>> @@ -870,6 +870,7 @@ static void setup_environment(const void *fdt)
>>  
>>  int misc_init_r(void)
>>  {
>> +	const char *spl_dt_name;
>>  	uint boot;
>>  
>>  	env_set("fel_booted", NULL);
>> @@ -888,6 +889,16 @@ int misc_init_r(void)
>>  		env_set("mmc_bootdev", "1");
>>  	}
>>  
>> +	/* Set fdtfile to match the FIT configuration chosen in SPL. */
>> +	spl_dt_name = get_spl_dt_name();
>> +	if (spl_dt_name) {
>> +		char *prefix = IS_ENABLED(CONFIG_ARM64) ? "allwinner/" : "";
>> +		char str[64];
> 
> The longest name (including the directory name) is 49 characters so far,
> so let's hope that people don't go crazy with their DT names. Shall we
> check the return value of snprintf() and at least complain? In contrast
> to strncpy(), snprintf() is safe, but might still truncate the name.

What kind of complaint were you thinking of? Trying to actually use a truncated
$fdtfile would produce a "file not found" error. Maybe that would be obvious enough?

Cheers,
Samuel

> Cheers,
> Andre.
> 
>> +
>> +		snprintf(str, sizeof(str), "%s%s.dtb", prefix, spl_dt_name);
>> +		env_set("fdtfile", str);
>> +	}
>> +
>>  	setup_environment(gd->fdt_blob);
>>  
>>  #ifdef CONFIG_USB_ETHER
>>
>