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

Timur Tabi
2010-05-18T15:32:18Z

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