[PATCH] boot: image-fdt: Restore suppression of irrelevant ERROR message

[Jonas Karlman]

Hi Randolph,

On 6/29/2026 7:33 PM, Randolph Sapp wrote:
> Hey, glad people are testing this. 
> 
> On Sat Jun 27, 2026 at 3:48 PM CDT, Jonas Karlman wrote:
>> The commit 623f6c5b6ab7 ("boot: image-fdt: free old dtb reservations")
>> removed the suppression of ERROR messages when -EINVAL is returned due
>> to the memory region not being part of the LMB memory map.
>>
>> This causes an irrelevant ERROR message during boot, e.g.:
>>
>>   Model: Radxa ROCK 3B
>>   [...]
>>   ERROR: reserving fdt memory region failed (addr=10f000 size=100 flags=2): -22
>>
>> or
>>
>>   Model: Rockchip RK3288 Asus Tinker Board S
>>   [...]
>>   ERROR: reserving fdt memory region failed (addr=fe000000 size=1000000 flags=4): -22
>>
>> FDT correctly contains reserved-memory for 10f000 or fe000000 and U-Boot
>> correctly does not make these regions available in the LMB memory map:
>>
>>   memory[0]      [0x200000-0xefffffff], 0xefe00000 bytes, flags: none
>>   memory[1]      [0x100000000-0x1ffffffff], 0x100000000 bytes, flags: none
>>
>> or
>>
>>   memory[0]      [0x0-0x7fffffff], 0x80000000 bytes, flags: none
>>
>> Ignore -EINVAL errors to restore suppression of this irrelevant ERROR
>> message when memory region is not part of the LMB memory map.
>>
>> Fixes: 623f6c5b6ab7 ("boot: image-fdt: free old dtb reservations")
>> Signed-off-by: Jonas Karlman <jonas at kwiboo.se>
>> ---
>>  boot/image-fdt.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/boot/image-fdt.c b/boot/image-fdt.c
>> index 9e0e0f93edd3..603294215b89 100644
>> --- a/boot/image-fdt.c
>> +++ b/boot/image-fdt.c
>> @@ -95,7 +95,7 @@ static void boot_fdt_handle_region(u64 addr, u64 size, u32 flags, bool free)
>>  		debug("   %s fdt memory region: addr=%llx size=%llx flags=%x\n",
>>  		      free ? "freed" : "reserved", (unsigned long long)addr,
>>  		      (unsigned long long)size, flags);
>> -	} else {
>> +	} else if (free || ret != -EINVAL) {
> 
> Seems like you messed up the free error reporting here.

Not sure what you mean, it is only lmb_alloc_mem() that documents:

  -EINVAL if the requested memory region is not part of the LMB memory map

and seems to now help print an verbose ERROR message during normal boot.

But maybe we should also ignore free errors?

> 
>>  		printf("ERROR: %s fdt memory region failed (addr=%llx size=%llx flags=%x): %ld\n",
>>  		       free ? "freeing" : "reserving", (unsigned long long)addr,
>>  		       (unsigned long long)size, flags, ret);
> 
> Got a few questions about this particular case though if you don't mind:
> 
> Why do you have reserved regions defined in areas that don't overlap with DDR?

E.g. fe000000-ffffffff or similar region is reserved for hw regs, on
boards with e.g. 4 GiB DRAM this area is typically unreachable, on a
2 GiB DRAM board there is no overlap.

The 10f000 is shmem used for SCMI, so should be defined in DT.

> Shouldn't those areas be unused anyway? Why don't we just explicitly ignore
> those reservations instead of forwarding them to an allocator that doesn't care?

That is what the net effect was prior to the changes in 623f6c5b6ab7
("boot: image-fdt: free old dtb reservations"). Rockchip platform
dynamically declare DRAM banks excluding the unusable ranges, and the
LMB allocator ignores ranges outside the declared DRAM banks.

> Why mask a bunch of potentially valid error cases for this explicit case?

That is what the code have been doing up until now, a different option
could be to change the error message to a debug message? Here I just
tried to do a limited revert to the old behavior.

Why should we print verbose ERROR messages for something that is not
always considered an error?

> 
> Why do none of the examples listed above have a memory node in dts? I've seen
> platforms that expect memory length to be defined by the bootloader, but they
> normally still place a dummy node in the tree at the correct starting address.

Rockchip platform dynamically check size of DRAM at runtime and declare
DRAM banks for the usable areas. Possible a fixup adds a memory node as
part of boot into OS.

Regards,
Jonas