Re: [U-Boot-Users] libfdt release?
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
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
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 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%40...
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.
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%40...
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 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
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.
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
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
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
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
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
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.
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 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.
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.
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
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.
participants (8)
-
David Gibson -
Detlev Zundel -
Jerry Van Baren -
Jerry Van Baren -
Jerry Van Baren -
Kumar Gala -
Kumar Gala -
Wolfgang Grandegger