[U-Boot-Users] libfdt release?

[U-Boot-Users] libfdt release?

[Jerry Van Baren]

Kumar Gala wrote:
>>> Yeah I was going through the u-boot mods and noticed a few additions.
>>>
>>> Pulled from the u-boot git commit logs:
>>>
>>> * add convenience function fdt_find_and_setprop()
>>
>> I don't think this belongs in libfdt itself - in the bootwrapper we'd
>> do this in devtree.c.  u-boot can keep it's function outside of
>> libfdt.
> 
> Agreed, I think u-boot should remove this in the future as the one user 
> should move to a more generic mechanism for handling "fixups".

Makes sense.  NBD.

>>> * Add fdt_find_node_by_type() and fdt_find_compatible_node() to LIBFDT
>>
>> fdt_find_node_by_type() should be subsumed by
>> fdt_node_offset_by_prop_value() which we already have upstream.  I'm
>> working on a fdt_node_offset_by_compatible().

OK.

>>> * Replace fdt_node_offset() with fdt_find_node_by_path().
>>
>> There is no fdt_node_offset(), fdt_path_offset() they mean, maybe.
>> And why a gratuitous name change...?
> 
> Jerry?

We needed the function (Detlev, IIRC).  The gratuitous name change was 
intended to be more descriptive.  Perhaps if the correct original name 
were used, we wouldn't have needed a gratuitous name change. ;-)

>>> * 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.

>>> * 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).

> David, one thing that would be extremely useful is "API" comments in 
> libfdt.h for each of the functions.

Add my vote too. ;-)

> - k

HTH,
gvb

[U-Boot-Users] libfdt release?

[Detlev Zundel]

Hi Jerry,

>>>> * Replace fdt_node_offset() with fdt_find_node_by_path().
>>>
>>> There is no fdt_node_offset(), fdt_path_offset() they mean, maybe.
>>> And why a gratuitous name change...?
>> 
>> Jerry?
>
> We needed the function (Detlev, IIRC).  The gratuitous name change was 
> intended to be more descriptive.  Perhaps if the correct original name 
> were used, we wouldn't have needed a gratuitous name change. ;-)

Just for the records, unfortunately I could only help with minor
theoretical comments during the time that Wolfgang Grandegger actually
wrote code making use of all this, so it might have been his code
needing the functions.

Cheers
  Detlev

-- 
C hasn't changed much since the 1970s. And let's face it it's ugly.
Can't we do better? C++? (Sorry, never mind.)
                                    -- Rob Pike
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot-Users] libfdt release?

[David Gibson]

On Thu, Oct 11, 2007 at 07:35:45PM -0400, Jerry Van Baren wrote:
[snip]
> >>>* Replace fdt_node_offset() with fdt_find_node_by_path().
> >>
> >>There is no fdt_node_offset(), fdt_path_offset() they mean, maybe.
> >>And why a gratuitous name change...?
> >
> >Jerry?
> 
> We needed the function (Detlev, IIRC).  The gratuitous name change was 
> intended to be more descriptive.  Perhaps if the correct original name 
> were used, we wouldn't have needed a gratuitous name change. ;-)

Heh.

Do bear in mind in future that the naming conventions in libfdt aren't
arbitrary.  I've tried to put "offset" in the name of everything that
returns a structure block offset as a hint to that return convention.
Additionally it's supposed to mitigate the big libfdt gotcha - that
offsets aren't handles, so the offset for a node can change if you
alter another node.

> >>>* 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.

> >>>* 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.

> >David, one thing that would be extremely useful is "API" comments in 
> >libfdt.h for each of the functions.
> 
> Add my vote too. ;-)

Yeah, I know...

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[U-Boot-Users] libfdt release?

[Wolfgang Grandegger]

David Gibson wrote:
> On Thu, Oct 11, 2007 at 07:35:45PM -0400, Jerry Van Baren wrote:
> [snip]
>>>>> * Replace fdt_node_offset() with fdt_find_node_by_path().
>>>> There is no fdt_node_offset(), fdt_path_offset() they mean, maybe.
>>>> And why a gratuitous name change...?
>>> Jerry?
>> We needed the function (Detlev, IIRC).  The gratuitous name change was 
>> intended to be more descriptive.  Perhaps if the correct original name 
>> were used, we wouldn't have needed a gratuitous name change. ;-)
> 
> Heh.
> 
> Do bear in mind in future that the naming conventions in libfdt aren't
> arbitrary.  I've tried to put "offset" in the name of everything that
> returns a structure block offset as a hint to that return convention.
> Additionally it's supposed to mitigate the big libfdt gotcha - that
> offsets aren't handles, so the offset for a node can change if you
> alter another node.

I proposed the name change some time ago on this list:

http://sourceforge.net/mailarchive/forum.php?thread_name=4639F61F.2050506%40grandegger.com&forum_name=u-boot-users

And they have been accepted and nobody complained ;-). The main reason 
for the change was to make them (almost) compatible with the names used 
by Linux. I think that's useful when we start using FDT for configuring 
U-Boot.

Wolfgang.

[U-Boot-Users] libfdt release?

[Jerry Van Baren]

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

[U-Boot-Users] libfdt release?

[David Gibson]

On Fri, Oct 12, 2007 at 09:16:01AM +0200, Wolfgang Grandegger wrote:
> David Gibson wrote:
> >On Thu, Oct 11, 2007 at 07:35:45PM -0400, Jerry Van Baren wrote:
> >[snip]
> >>>>>* Replace fdt_node_offset() with fdt_find_node_by_path().
> >>>>There is no fdt_node_offset(), fdt_path_offset() they mean, maybe.
> >>>>And why a gratuitous name change...?
> >>>Jerry?
> >>We needed the function (Detlev, IIRC).  The gratuitous name change was 
> >>intended to be more descriptive.  Perhaps if the correct original name 
> >>were used, we wouldn't have needed a gratuitous name change. ;-)
> >
> >Heh.
> >
> >Do bear in mind in future that the naming conventions in libfdt aren't
> >arbitrary.  I've tried to put "offset" in the name of everything that
> >returns a structure block offset as a hint to that return convention.
> >Additionally it's supposed to mitigate the big libfdt gotcha - that
> >offsets aren't handles, so the offset for a node can change if you
> >alter another node.
> 
> I proposed the name change some time ago on this list:
> 
> http://sourceforge.net/mailarchive/forum.php?thread_name=4639F61F.2050506%40grandegger.com&forum_name=u-boot-users
> 
> And they have been accepted and nobody complained ;-).

Well, of course not; I'm not on the u-boot list.

> The main reason 
> for the change was to make them (almost) compatible with the names used 
> by Linux. I think that's useful when we start using FDT for configuring 
> U-Boot.

Well, you have my reasons for *not* doing that, above.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[U-Boot-Users] libfdt release?

[David Gibson]

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.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[U-Boot-Users] libfdt release?

[Kumar Gala]

Guys,

Can you tell me where we stand on changes/additions to libfdt vs u- 
boot.  I'd really like to see an updated libfdt imported into u-boot  
for the next window and am happy to work on the u-boot modifications  
as long as I understand what exactly they are.

Having fdt_get_name() and fdt_get_path() will be really useful for  
some fixup of device node code I've got and will allow us to drop the  
hard coded/explicit PATHs to nodes from <config.h>.

I'd rather see us take a clean libfdt drop rather than pulling in  
bits and pieces.

- k

[U-Boot-Users] libfdt release?

[Jerry Van Baren]

Kumar Gala wrote:
> Guys,
> 
> Can you tell me where we stand on changes/additions to libfdt vs u- 
> boot.  I'd really like to see an updated libfdt imported into u-boot  
> for the next window and am happy to work on the u-boot modifications  
> as long as I understand what exactly they are.
> 
> Having fdt_get_name() and fdt_get_path() will be really useful for  
> some fixup of device node code I've got and will allow us to drop the  
> hard coded/explicit PATHs to nodes from <config.h>.
> 
> I'd rather see us take a clean libfdt drop rather than pulling in  
> bits and pieces.
> 
> - k

Hi Kumar,

My plan for the next merge window is to pull a fresh copy of The 
Official LIBFDT into u-boot-fdt.  From what David has said, libfdt has 
been filled out with some functions that we independently invented 
and/or there are better ways of doing things, if only we were smarter.

I have not had time to actually pull a copy of the current libfdt from 
the kernel/dtc repository and evaluate where we stand.

I would also like to see improvements to the 83xx CPU handling to match 
what Grant did on the 5xxx sub-repo.

My problem is that I had zero "u-boot" time available last month.  I 
have hopes for some time this month, but it is questionable.  Maybe 
November...

I don't anticipate a problem unforking libfdt, but it all takes time. 
Whatever you can contribute towards this, I'm happy to queue up for the 
next merge window.  If you have the time and desire to take over 
custodian responsibilities for u-boot-fdt, we can discuss that too.

Best regards,
gvb

[U-Boot-Users] libfdt release?

[Kumar Gala]

On Oct 15, 2007, at 1:51 PM, Jerry Van Baren wrote:

> Kumar Gala wrote:
>> Guys,
>>
>> Can you tell me where we stand on changes/additions to libfdt vs u-
>> boot.  I'd really like to see an updated libfdt imported into u-boot
>> for the next window and am happy to work on the u-boot modifications
>> as long as I understand what exactly they are.
>>
>> Having fdt_get_name() and fdt_get_path() will be really useful for
>> some fixup of device node code I've got and will allow us to drop the
>> hard coded/explicit PATHs to nodes from <config.h>.
>>
>> I'd rather see us take a clean libfdt drop rather than pulling in
>> bits and pieces.
>>
>> - k
>
> Hi Kumar,
>
> My plan for the next merge window is to pull a fresh copy of The
> Official LIBFDT into u-boot-fdt.  From what David has said, libfdt has
> been filled out with some functions that we independently invented
> and/or there are better ways of doing things, if only we were smarter.
>
> I have not had time to actually pull a copy of the current libfdt from
> the kernel/dtc repository and evaluate where we stand.
>
> I would also like to see improvements to the 83xx CPU handling to  
> match
> what Grant did on the 5xxx sub-repo.

Have you looked at my post: RFC: flat dev tree fixup code?

It takes the 83xx style and removes the hard coded paths.  This could  
easily be converted into explicit function calls if desired, with  
some caveats.  My biggest issue is wanting to remove OF_CPU, OF_SOC,  
etc. from u-boot.

> My problem is that I had zero "u-boot" time available last month.  I
> have hopes for some time this month, but it is questionable.  Maybe
> November...

I understand, and thus my offer of trying to help out.  I dont know  
all the history of what we have in u-boot today vs the libfdt that  
was imported.  Once someone can clearly say here are some things that  
we need to handle in u-boot based I'm happy to try and work on those  
items.

> I don't anticipate a problem unforking libfdt, but it all takes time.
> Whatever you can contribute towards this, I'm happy to queue up for  
> the
> next merge window.  If you have the time and desire to take over
> custodian responsibilities for u-boot-fdt, we can discuss that too.

I don't really have time to be custodian.  The linuxppc stuff takes  
up more than enough time.

- k

[U-Boot-Users] libfdt release?

[Wolfgang Grandegger]

Kumar Gala wrote:
> Guys,
> 
> Can you tell me where we stand on changes/additions to libfdt vs u- 
> boot.  I'd really like to see an updated libfdt imported into u-boot  
> for the next window and am happy to work on the u-boot modifications  
> as long as I understand what exactly they are.
> 
> Having fdt_get_name() and fdt_get_path() will be really useful for  
> some fixup of device node code I've got and will allow us to drop the  
> hard coded/explicit PATHs to nodes from <config.h>.
> 
> I'd rather see us take a clean libfdt drop rather than pulling in  
> bits and pieces.

Yes, that would be nice but the libfdt for U-Boot may still need to be 
extended, especially for dynamic configuration. Therefore I would 
appreciate a discussion on what else we need for that purpose and how we 
handle (separate) common and extended libfdt functions for U-Boot. For 
the dynamic configuration, I'm clearly in favor of function names 
similar to the one commonly used in Linux (with the prefix fdt:):

   of_find_node_by_path
   of_find_node_by_type
   of_find_node_by_phandle
   of_find_compatible_node
   of_device_is_compatible
   of_get_property

Wolfgang.

[U-Boot-Users] libfdt release?

[Jerry Van Baren]

Kumar Gala wrote:
> 
> On Oct 15, 2007, at 1:51 PM, Jerry Van Baren wrote:
> 
>> Kumar Gala wrote:
>>> Guys,
>>>
>>> Can you tell me where we stand on changes/additions to libfdt vs u-
>>> boot.  I'd really like to see an updated libfdt imported into u-boot
>>> for the next window and am happy to work on the u-boot modifications
>>> as long as I understand what exactly they are.
>>>
>>> Having fdt_get_name() and fdt_get_path() will be really useful for
>>> some fixup of device node code I've got and will allow us to drop the
>>> hard coded/explicit PATHs to nodes from <config.h>.
>>>
>>> I'd rather see us take a clean libfdt drop rather than pulling in
>>> bits and pieces.
>>>
>>> - k
>>
>> Hi Kumar,
>>
>> My plan for the next merge window is to pull a fresh copy of The
>> Official LIBFDT into u-boot-fdt.  From what David has said, libfdt has
>> been filled out with some functions that we independently invented
>> and/or there are better ways of doing things, if only we were smarter.
>>
>> I have not had time to actually pull a copy of the current libfdt from
>> the kernel/dtc repository and evaluate where we stand.
>>
>> I would also like to see improvements to the 83xx CPU handling to match
>> what Grant did on the 5xxx sub-repo.
> 
> Have you looked at my post: RFC: flat dev tree fixup code?
> 
> It takes the 83xx style and removes the hard coded paths.  This could 
> easily be converted into explicit function calls if desired, with some 
> caveats.  My biggest issue is wanting to remove OF_CPU, OF_SOC, etc. 
> from u-boot.

I've seen it and filed it for When I Have Time.  I agree with the 
concept but have not had time to look at it in detail.  I would like to 
unfork libfdt first... do your changes work with David's current libfdt? 
  Do they work with the current u-boot libfdt?

Your RFC is based off the 83xx method.  Grant Likely convinced me that 
his method of fixup in the 5xxx tree is better (cut the table 
indirection as unnecessary obfuscation, do direct calls instead).  Your 
removal of direct references to CPU and SOC nodes still applies, but I 
have a goal of switching to Grant's direct call methodology.  Perhaps 
you can reroll your patch to use 5xxx as the template and kill two 
bugs^W birds with one patch?

The FDT fixup that your RFC addresses is actually part of the 
CPU-specific code (83xx, 85xx, 5xxx, etc.) and your RFC really should be 
aimed at those custodians (Kim, Jon, Grant, etc.).

Depending on the state of u-boot libfdt and the DTC libfdt, it may also 
require coordination with and update via u-boot-fdt.

>> My problem is that I had zero "u-boot" time available last month.  I
>> have hopes for some time this month, but it is questionable.  Maybe
>> November...
> 
> I understand, and thus my offer of trying to help out.  I dont know all 
> the history of what we have in u-boot today vs the libfdt that was 
> imported.  Once someone can clearly say here are some things that we 
> need to handle in u-boot based I'm happy to try and work on those items.

I would like to see libfdt unforked first before we pile a bunch more 
dependencies on top of libfdt, increasing the unforking work.

>> I don't anticipate a problem unforking libfdt, but it all takes time.
>> Whatever you can contribute towards this, I'm happy to queue up for the
>> next merge window.  If you have the time and desire to take over
>> custodian responsibilities for u-boot-fdt, we can discuss that too.
> 
> I don't really have time to be custodian.  The linuxppc stuff takes up 
> more than enough time.

Dang, called my bluff.  ;-)

gvb

[U-Boot-Users] libfdt release?

[Kumar Gala]

On Oct 15, 2007, at 2:23 PM, Jerry Van Baren wrote:

> Kumar Gala wrote:
>> On Oct 15, 2007, at 1:51 PM, Jerry Van Baren wrote:
>>> Kumar Gala wrote:
>>>> Guys,
>>>>
>>>> Can you tell me where we stand on changes/additions to libfdt vs u-
>>>> boot.  I'd really like to see an updated libfdt imported into u- 
>>>> boot
>>>> for the next window and am happy to work on the u-boot  
>>>> modifications
>>>> as long as I understand what exactly they are.
>>>>
>>>> Having fdt_get_name() and fdt_get_path() will be really useful for
>>>> some fixup of device node code I've got and will allow us to  
>>>> drop the
>>>> hard coded/explicit PATHs to nodes from <config.h>.
>>>>
>>>> I'd rather see us take a clean libfdt drop rather than pulling in
>>>> bits and pieces.
>>>>
>>>> - k
>>>
>>> Hi Kumar,
>>>
>>> My plan for the next merge window is to pull a fresh copy of The
>>> Official LIBFDT into u-boot-fdt.  From what David has said,  
>>> libfdt has
>>> been filled out with some functions that we independently invented
>>> and/or there are better ways of doing things, if only we were  
>>> smarter.
>>>
>>> I have not had time to actually pull a copy of the current libfdt  
>>> from
>>> the kernel/dtc repository and evaluate where we stand.
>>>
>>> I would also like to see improvements to the 83xx CPU handling to  
>>> match
>>> what Grant did on the 5xxx sub-repo.
>> Have you looked at my post: RFC: flat dev tree fixup code?
>> It takes the 83xx style and removes the hard coded paths.  This  
>> could easily be converted into explicit function calls if desired,  
>> with some caveats.  My biggest issue is wanting to remove OF_CPU,  
>> OF_SOC, etc. from u-boot.
>
> I've seen it and filed it for When I Have Time.  I agree with the  
> concept but have not had time to look at it in detail.  I would  
> like to unfork libfdt first... do your changes work with David's  
> current libfdt?  Do they work with the current u-boot libfdt?

A mix.  I grabbed some functions out of David's libfdt to get the  
name of the node based on offset.  Thus my desire to get david's  
libfdt into u-boot.

> Your RFC is based off the 83xx method.  Grant Likely convinced me  
> that his method of fixup in the 5xxx tree is better (cut the table  
> indirection as unnecessary obfuscation, do direct calls instead).   
> Your removal of direct references to CPU and SOC nodes still  
> applies, but I have a goal of switching to Grant's direct call  
> methodology.  Perhaps you can reroll your patch to use 5xxx as the  
> template and kill two bugs^W birds with one patch?

The one problem I had with direct calls is you really need/want to  
iterate over a set of nodes not just a single one in some cases (like  
fixing up CPU nodes).

> The FDT fixup that your RFC addresses is actually part of the CPU- 
> specific code (83xx, 85xx, 5xxx, etc.) and your RFC really should  
> be aimed at those custodians (Kim, Jon, Grant, etc.).

It was RFC for anyone and everyone to comment on :)

The fixup code itself was generic, the table was specific to 85xx.

> Depending on the state of u-boot libfdt and the DTC libfdt, it may  
> also require coordination with and update via u-boot-fdt.

agreed.

>>> My problem is that I had zero "u-boot" time available last month.  I
>>> have hopes for some time this month, but it is questionable.  Maybe
>>> November...
>> I understand, and thus my offer of trying to help out.  I dont  
>> know all the history of what we have in u-boot today vs the libfdt  
>> that was imported.  Once someone can clearly say here are some  
>> things that we need to handle in u-boot based I'm happy to try and  
>> work on those items.
>
> I would like to see libfdt unforked first before we pile a bunch  
> more dependencies on top of libfdt, increasing the unforking work.

Agreed.

>>> I don't anticipate a problem unforking libfdt, but it all takes  
>>> time.
>>> Whatever you can contribute towards this, I'm happy to queue up  
>>> for the
>>> next merge window.  If you have the time and desire to take over
>>> custodian responsibilities for u-boot-fdt, we can discuss that too.
>> I don't really have time to be custodian.  The linuxppc stuff  
>> takes up more than enough time.
>
> Dang, called my bluff.  ;-)

good try :)

- k

[U-Boot-Users] libfdt release?

[David Gibson]

On Mon, Oct 15, 2007 at 09:21:54PM +0200, Wolfgang Grandegger wrote:
> Kumar Gala wrote:
> >Guys,
> >
> >Can you tell me where we stand on changes/additions to libfdt vs u- 
> >boot.  I'd really like to see an updated libfdt imported into u-boot  
> >for the next window and am happy to work on the u-boot modifications  
> >as long as I understand what exactly they are.
> >
> >Having fdt_get_name() and fdt_get_path() will be really useful for  
> >some fixup of device node code I've got and will allow us to drop the  
> >hard coded/explicit PATHs to nodes from <config.h>.
> >
> >I'd rather see us take a clean libfdt drop rather than pulling in  
> >bits and pieces.
> 
> Yes, that would be nice but the libfdt for U-Boot may still need to be 
> extended, especially for dynamic configuration. Therefore I would 
> appreciate a discussion on what else we need for that purpose and how we 
> handle (separate) common and extended libfdt functions for U-Boot. For 

Again, I'm happy to add functionality to core libfdt if u-boot needs
it (as long as it isn't fundamentally u-boot specific, of course).

> the dynamic configuration, I'm clearly in favor of function names 
> similar to the one commonly used in Linux (with the prefix fdt:):
> 
>   of_find_node_by_path
>   of_find_node_by_type
>   of_find_node_by_phandle
>   of_find_compatible_node
>   of_device_is_compatible
>   of_get_property

Tough.  The names I have are staying for the reasons I've already
mentioned.  And because I want to keep the libfdt API reasonably
stable.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[U-Boot-Users] libfdt release?

[Wolfgang Grandegger]

David Gibson wrote:
> On Mon, Oct 15, 2007 at 09:21:54PM +0200, Wolfgang Grandegger wrote:
>> Kumar Gala wrote:
>>> Guys,
>>>
>>> Can you tell me where we stand on changes/additions to libfdt vs u- 
>>> boot.  I'd really like to see an updated libfdt imported into u-boot  
>>> for the next window and am happy to work on the u-boot modifications  
>>> as long as I understand what exactly they are.
>>>
>>> Having fdt_get_name() and fdt_get_path() will be really useful for  
>>> some fixup of device node code I've got and will allow us to drop the  
>>> hard coded/explicit PATHs to nodes from <config.h>.
>>>
>>> I'd rather see us take a clean libfdt drop rather than pulling in  
>>> bits and pieces.
>> Yes, that would be nice but the libfdt for U-Boot may still need to be 
>> extended, especially for dynamic configuration. Therefore I would 
>> appreciate a discussion on what else we need for that purpose and how we 
>> handle (separate) common and extended libfdt functions for U-Boot. For 
> 
> Again, I'm happy to add functionality to core libfdt if u-boot needs
> it (as long as it isn't fundamentally u-boot specific, of course).

The functions below are not really U-Boot specific.

>> the dynamic configuration, I'm clearly in favor of function names 
>> similar to the one commonly used in Linux (with the prefix fdt:):
>>
>>   of_find_node_by_path
>>   of_find_node_by_type
>>   of_find_node_by_phandle
>>   of_find_compatible_node
>>   of_device_is_compatible
>>   of_get_property
> 
> Tough.  The names I have are staying for the reasons I've already
> mentioned.  And because I want to keep the libfdt API reasonably
> stable.

OK, just my personal opinion on gratuitous name changes. What functions 
are missing in the libfdt? How can I help to get them ported to the 
generic libfdt?

Wolfgang.

[U-Boot-Users] libfdt release?

[David Gibson]

On Tue, Oct 16, 2007 at 09:03:32AM +0200, Wolfgang Grandegger wrote:
> David Gibson wrote:
> >On Mon, Oct 15, 2007 at 09:21:54PM +0200, Wolfgang Grandegger wrote:
> >>Kumar Gala wrote:
> >>>Guys,
> >>>
> >>>Can you tell me where we stand on changes/additions to libfdt vs u- 
> >>>boot.  I'd really like to see an updated libfdt imported into u-boot  
> >>>for the next window and am happy to work on the u-boot modifications  
> >>>as long as I understand what exactly they are.
> >>>
> >>>Having fdt_get_name() and fdt_get_path() will be really useful for  
> >>>some fixup of device node code I've got and will allow us to drop the  
> >>>hard coded/explicit PATHs to nodes from <config.h>.
> >>>
> >>>I'd rather see us take a clean libfdt drop rather than pulling in  
> >>>bits and pieces.
> >>Yes, that would be nice but the libfdt for U-Boot may still need to be 
> >>extended, especially for dynamic configuration. Therefore I would 
> >>appreciate a discussion on what else we need for that purpose and how we 
> >>handle (separate) common and extended libfdt functions for U-Boot. For 
> >
> >Again, I'm happy to add functionality to core libfdt if u-boot needs
> >it (as long as it isn't fundamentally u-boot specific, of course).
> 
> The functions below are not really U-Boot specific.

Sure, and we have nearly all of them.

> >>the dynamic configuration, I'm clearly in favor of function names 
> >>similar to the one commonly used in Linux (with the prefix fdt:):
> >>
> >>  of_find_node_by_path
> >>  of_find_node_by_type
> >>  of_find_node_by_phandle
> >>  of_find_compatible_node
> >>  of_device_is_compatible
> >>  of_get_property
> >
> >Tough.  The names I have are staying for the reasons I've already
> >mentioned.  And because I want to keep the libfdt API reasonably
> >stable.
> 
> OK, just my personal opinion on gratuitous name changes. What functions 
> are missing in the libfdt? How can I help to get them ported to the 
> generic libfdt?

There are functions for all of these, either already in libfdt, or
included in a patch I sent to jdl today.

 * of_find_node_by_path => fdt_path_offset()

 * of_find_node_by_type => fdt_node_offset_by_prop_value() 
 * of_find_node_by_phandle => fdt_node_offset_by_prop_value()

Although I suspect the only reason u-boot needs the first of these is
because of the broken early Freescale bindings which use device_type
in ways they shouldn't.

 * of_find_compatible_node => fdt_node_offset_by_compatible()
 * of_device_is_compatible => fdt_node_check_compatible()

Both in today's patch to jdl.  Beware that fdt_node_check_compatible()
has reversed sense from of_device_is_compatible() (returns 0 if the
node *is* compatible).  That's so it can return error codes in the
normal way if passed a junk tree or offset.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[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

[U-Boot-Users] libfdt release?

[David Gibson]

On Tue, Oct 23, 2007 at 09:39:14AM -0500, Kumar Gala wrote:
[snip]
> 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.)

Oops, fdt_check_header() fell off my radar.  I'll send a patch to
publish that today.  I already have a patch in the works to publish
fdt_next_tag() - Jerry's print example convinced me it's a good idea,
plus I thought of a useful testcase I can implement with it.

The rest of the things I know about are already in there, I think.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson