[U-Boot] libfdt: make fdt_increase_size() available to everyone

Timur Tabi timur at freescale.com
Tue May 18 17:32:18 CEST 2010


Wolfgang Denk wrote:

>> We can never guarantee this.  The code that calls fdt_increase_size() will
>> just have to ensure that there is enough room.
> 
> Such an "ensure that there is enough room" is exactly what I'm asking
> for.

Maybe I don't understand what you're getting at.  My point is that, whenever
someone writes code that calls fdt_increase_size(), *that* person needs to
provide the assurance, not me.  Within fdt_increase_size(), there is no way
to ensure anything.  And since my patch only deals with fdt_increase_size()
itself, I don't understand what you want from *me* within the context of
*this* patch.

>> If the fdt is in NOR flash, then boot_relocate_fdt() will copy it to RAM via
>> lmb_alloc_base(), which uses CONFIG_SYS_FDT_PAD to determine how much extra
>> room is needed.
> 
> Hm... it seems that not a single board uses this setting, 

That's because the default has been sufficient so far.

> which also
> happens to be completely undocumented.

That's got nothing to do with me.  I didn't write the code that uses
CONFIG_SYS_FDT_PAD.

> For me lmb_alloc_base() is just another form of malloc()...

True, but it's not the same as malloc().  For example, I cannot use
realloc() to make the allocation bigger.  If all fdts were allocated via
malloc(), then I could modify fdt_increase_size() to call realloc().

>> So for case #1, we can use CONFIG_SYS_FDT_PAD.  For case #2, we just have to
>> assume that when the user did "tftp c0000 my.dtb", that he knows what he's
>> doing.
> 
> I'm not sure if we have the same intentions here.
> 
> I think, that for case #1 the available size is known, so we can check
> if we exceed the limits. And this is what we should do then.

But within fdt_increase_size(), how do I know if the memory is allocated via
lmb_alloc_base()?  The heap management data structure for an lmb is managed
external to the heap itself.

>> I assume that fdt_increase_size() will only be used to increase the
>> available space by a significant amount.  One example of this (and the
>> reason I posted this patch in the first place), is to embed firmware inside
>> the device tree.  A new binding for Freescale QE firmware allows for this,
>> and I have code in-house which implements this.
> 
> If we are talking about such substantial increments then it is all
> the more important to check for available room.

And again, the point *I* am trying to make is that it's okay to put the onus
of that check on the *caller* of fdt_increase_size(), and not on
fdt_increase_size() itself.

>> So I say that a particular board will know whether it needs to significantly
>> expand the available amount of RAM beyond the default CONFIG_SYS_FDT_PAD.
>> Therefore, we can define a new value of CONFIG_SYS_FDT_PAD in the board
>> header files for those boards that need it.
> 
> No. We should check if the programmed value is sufficient.

But that is only meaningful if the fdt is allocated via an lmb, which is not
true in case #2.  In case #2, there is no allocation of memory, so there's
no way to know within fdt_increase_size()!

-- 
Timur Tabi
Linux kernel developer at Freescale


More information about the U-Boot mailing list