From mboxrd@z Thu Jan 1 00:00:00 1970 From: Timur Tabi Date: Tue, 18 May 2010 10:32:18 -0500 Subject: [U-Boot] libfdt: make fdt_increase_size() available to everyone In-Reply-To: <20100518151841.B6F18E6D663@gemini.denx.de> References: <1273526815-1091-1-git-send-email-timur@freescale.com> <1273975868.3244.9.camel@xps> <20100517131650.1060EE6D663@gemini.denx.de> <4BF14FBF.3040900@freescale.com> <20100517203355.2B215E6D663@gemini.denx.de> <4BF29FE9.1070305@freescale.com> <20100518151841.B6F18E6D663@gemini.denx.de> Message-ID: <4BF2B302.2030909@freescale.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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