[U-Boot-Users] libfdt release?

[Kumar Gala]

On Oct 14, 2007, at 11:12 PM, David Gibson wrote:

> On Fri, Oct 12, 2007 at 07:58:28AM -0400, Jerry Van Baren wrote:
>> 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.
>
> Yeah, fair enough.  I'll export the 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.
>
> That would be debugging purposes.  I didn't mean to imply that
> debugging purposes were unimportant..
>
>> 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).
>
> Uh.. how did mime encoding get into this!?  We already have
> /proc/device-tree giving the unpacked device tree.  I'm not sure if we
> currently export the blob or not, but if we did it should be as a raw
> binary.  A user can point dtc at that to decode it, there's absolutely
> no call for the kernel to do so.
>
>> 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.
>
> The existing function was already sufficient - and would have
> discouraged at least one stupidity in the way you use it in this
> example.
>
> If you want to look into the details of the returned tags, it's easy
> to use fdt_offset_ptr() to look at the data in place as either a
> struct fdt_node_header or struct fdt_property (or anything else).
>
> Using that approach for properties, you wouldn't need to *re*-iterate
> from the last BEGIN_NODE with fdt_getprop() to find the property that
> next_tag() just found anyway.  Which would in turn entirely eliminate
> the need for offstack.
>
> Oh, *and* current libfdt has fdt_get_name() which makes it even easier
> to get a node's name, given it's offset.

What's the status of getting some of the changes agreed to into the  
official libfdt?

(A recent git pull it appears things like fdt_check_header are still  
private, etc.)

- k