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

[Wolfgang Denk]

Dear Timur Tabi,

In message <4BF2B302.2030909 at freescale.com> you 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.

Your problem is that you are too much focussed on "your patch" only.
You need to keep your eyes open for the bigger whole.  Your patch has
the potential of causing nasty crashes, and I ask you to prevent this.
This may require changes outside the current scope of "your 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.

Right. But your patch is supposed to change this.

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

No. But you are trying to get code into mainline where the current
code is insufficient. So you also got to extend the code to make
yourpatch acceptable.

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

If you don't know this in fdt_increase_size(), then either make it
known there (for example by extending other code to pass on such
information), or add such checking to other parts of the code in the
call chain.

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

OK. I have no problem with that. I am just missing this other part of
the required changes in your patch.

> > 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()!

Maybe. But there is still case #1, where we have a real problem, and I
will not accept your patch without seing this problem properly
addressed.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Do not underestimate the value of print statements for debugging.