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

[Jonas Karlman]

Hi Randolph,

On 6/29/2026 8:37 PM, Randolph Sapp wrote:
> On Mon Jun 29, 2026 at 1:14 PM CDT, Jonas Karlman wrote:
>> 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.
> 
> Not quite. I mean, we should ignore the region explicitly. As in, we should
> never even request to reserve regions outside of LMB.

How would you suggest we do that?

LMB memory map is internal to lmb and boot_fdt_handle_region() is the
actual function that tries to reserve regions from FDT and would be
where we test if a region is part of memory map or not, so why should it
not be able to do such test based on the error code returned from the
function is would end up calling anyway?

With above analysis I am even more convinced that this error message
probably could be downgraded to a debug message ;-)

In what scenarios do you expect this error message to be useful when FDT
contains a reserved region that is not part of LMB memory map?

Regards,
Jonas

> 
>>> 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?
> 
> Because under normal circumstances it should always be an error. Occasionally
> asking for a reservation you know is going to fail and assuming all failures of
> that type are OK is a little odd.
> 
>>>
>>> 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
> 
> Ah, thanks for the information.