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

[Jonas Karlman]

Hi Randolph,

On 6/29/2026 10:19 PM, Randolph Sapp wrote:
> On Mon Jun 29, 2026 at 2:08 PM CDT, Jonas Karlman wrote:
>> 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?
> 
> What about when the FDT contains a reserved region that is part of the LMB
> memory map, but failed to initialize the address pointer correctly?

Please elaborate in what scenario this could happen, any possible
consequence of such error and how this specific error message help
end-users.

> 
> My point is this error code is not specifically reserved for the case you are
> describing and may adopt additional functionality in the future. It already has
> one additional case tied to it *today*.

I do not understand, lmb_alloc_mem() is documented as:

  When the allocation is of type @LMB_MEM_ALLOC_ADDR, the return value can
  be -EINVAL if the requested memory region is not part of the LMB memory
  map, and -EEXIST if the requested region is already allocated.

The old behavior prior to the offending commit correctly ignored this
return value to not produce a verbose ERROR message for regions not part
of the memory map.

The change in this patch restores logging to such expected behavior.
Do you prefer a full revert of the offending commit instead of reverting
logging to old behavior, a downgrade from printf() to debug() or some
other way to remove this erroneous verbose ERROR message?

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.
>