From: Timur Tabi <timur@freescale.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] libfdt: make fdt_increase_size() available to everyone
Date: Tue, 18 May 2010 10:32:18 -0500 [thread overview]
Message-ID: <4BF2B302.2030909@freescale.com> (raw)
In-Reply-To: <20100518151841.B6F18E6D663@gemini.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
next prev parent reply other threads:[~2010-05-18 15:32 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-05-10 21:26 [U-Boot] [PATCH 2/3] libfdt: make fdt_increase_size() available to everyone Timur Tabi
2010-05-10 21:30 ` Timur Tabi
2010-05-13 17:46 ` Kumar Gala
2010-05-16 2:11 ` [U-Boot] " Gerald Van Baren
2010-05-16 4:13 ` Timur Tabi
2010-05-17 11:24 ` Jerry Van Baren
2010-05-17 13:16 ` Wolfgang Denk
2010-05-17 14:16 ` Timur Tabi
2010-05-17 20:33 ` Wolfgang Denk
2010-05-18 14:10 ` Timur Tabi
2010-05-18 15:18 ` Wolfgang Denk
2010-05-18 15:32 ` Timur Tabi [this message]
2010-05-18 22:20 ` Wolfgang Denk
2010-05-19 0:51 ` Timur Tabi
2010-05-19 6:54 ` Wolfgang Denk
2010-05-19 14:57 ` Timur Tabi
2010-05-19 15:14 ` Wolfgang Denk
2010-05-19 15:34 ` Timur Tabi
2010-05-19 15:44 ` Wolfgang Denk
2010-05-19 21:46 ` Kumar Gala
2010-05-19 22:06 ` Wolfgang Denk
2010-05-19 22:12 ` Timur Tabi
2010-05-20 8:28 ` Wolfgang Denk
2010-05-20 11:44 ` Timur Tabi
2010-05-25 16:07 ` Timur Tabi
2010-05-25 17:50 ` Wolfgang Denk
2010-05-25 18:01 ` Timur Tabi
2010-05-25 19:15 ` Wolfgang Denk
2010-05-25 19:28 ` Timur Tabi
2010-05-25 22:11 ` Wolfgang Denk
2010-05-26 15:11 ` Timur Tabi
2010-05-26 16:11 ` Wolfgang Denk
2010-05-26 16:38 ` Timur Tabi
2010-05-26 17:23 ` Scott Wood
2010-05-26 17:56 ` Timur Tabi
2010-05-26 18:06 ` Scott Wood
2010-05-26 18:23 ` Timur Tabi
2010-05-26 19:14 ` Wolfgang Denk
2010-05-26 19:08 ` Wolfgang Denk
2010-05-20 13:34 ` Kumar Gala
2010-05-20 12:58 ` Timur Tabi
2010-05-17 11:25 ` [U-Boot] [PATCH 2/3] " Jerry Van Baren
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4BF2B302.2030909@freescale.com \
--to=timur@freescale.com \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.