[U-Boot-Users] libfdt release?

[Jerry Van Baren <gerald.vanbaren@smiths-aerospace.com>]

David Gibson wrote:
> On Thu, Oct 11, 2007 at 07:35:45PM -0400, Jerry Van Baren wrote:

[snip]

>>>>> * Add some utilities to manipulate the reserved memory map. (think
>>>>> your recent patch my cover that)
>>>> Should do.
>>>>
>>>>> * libfdt: Make fdt_check_header() public
>>>> Hrm.  Why?
>>> Jerry?
>> Because we needed to check the validity of the header outside of libfdt 
>> and decide whether we wanted to use the blob or not.
> 
> That seems reasonable.  I'll look at exporting that.  Although.. I'm a
> little surprised that u-boot doesn't just to an fdt_open_into() as the
> first thing, which would include the check.

Yes, that would be the alternative, but fdt_open_into() potentially 
changes things (copies the blob) and doing an fdt_open_into() just to 
check the validity of the header is not intuitive.

While it is true that, by giving the same start and end address, you can 
avoid doing the actual copy, it isn't obvious that the read/write 
doesn't actually occur if source == dest... the code goes through all 
the motions and relies on the underlying memory utility library to short 
circuit the actual operation.  If the underlying memory utility library 
*doesn't* short circuit the operation, we could have severe problems 
with blobs that are burned into flash (potential write protection 
exceptions).

I submit that is a lot of ugly to avoid a simple export of an already 
existing, useful, function.

>>>>> * libfdt: Enhanced and published fdt_next_tag()
>>>> Hrm.  I'm not categorically opposed to publishing fdt_next_tag(), but
>>>> I'm disinclined to do so without good reason.
>>> Jerry?
>>>
>>> Hopefully Jerry will comment on why the additions were made to u-boot.
>> I needed it to implement my u-boot "fdt print" command, to step through 
>> the tags so I could print them out in a human readable format (it 
>> actually is dtc-input-compatible and, in most cases, matches the 
>> original dtc input source code).
> 
> Hrm, I see.  For debugging purposes, essentially.
> 
> I have been thinking for some time that I needed to add user-accesible
> traversal functions - I just haven't managed to come up with an
> interface I like, yet.

More than debugging purposes, the "fdt print" command is extremely 
useful for the u-boot command line to see what you have to start with, 
figure out what parameters need to be modified, and format a correct 
string to do the modification.  This is doubly important when the blob 
was created /n/ steps before you got it (a board manufacturer, reseller, 
or some poor luser) and you have no clue what the blob is to start with.

It would be very useful for the kernel as well, to implement the /proc 
blob display.  Again, on the face of it, your "debugging purposes" would 
apply, but it is essential debugging, not nicety debugging.  I suspect 
kernel developers are going to be a lot happier getting an ascii dump of 
/proc/fdt/asciiblob (sorry, forgot where it lives) than a mime encoded 
binary lump that they then have to decode (only to find out that the 
poor luser didn't dd or mime encode it properly).

In common/cmd_fdt.c line 551 (line 601 is where the traversal starts) 
you can see how I used it to walk through the blob:
<http://www.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=blob;f=common/cmd_fdt.c;h=571b8f14d56f3bc97dcd49eb8465cfdc9988d786;hb=HEAD#l551>

IMHO it is an ideal user-accessible traversal function.  It is simple 
yet sufficient.  It does what you need with very little fuss required of 
the user, yet it can be used in lots of unexpected ways (that's a *good* 
thing here).  For instance, in my "print" example, I add a looping 
construct with a nesting depth variable and viola, an elegant print 
function that can start anywhere in the blob, print out a syntactically 
correct ASCII representation, etc. etc.

Thanks,
gvb