Dear Timur Tabi,
In message 4BF2B302.2030909@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