[U-Boot-Users] Testing todays u-boot-fdt
Hi Jerry,
I gave the fdtlib of your git://cideas.us/pub/scm/u-boot/u-boot-fdt.git a try on my Icecube board. I got it built with the attached patch. libfdt was actually not made and the second hunk fixes a warning (=bug?). Then I was able to read and list a blob in memory:
=> fdt addr 20000 => fdt print / ...
A few other things didn't work, especially updating the FDT:
=> fdt addr 20000 10000 libfdt: FDT_ERR_BADVERSION
=> fdt env libfdt: FDT_ERR_BADVERSION => fdt bd_t libfdt: FDT_ERR_BADVERSION => fdt chosen libfdt: FDT_ERR_BADVERSION
=> fdt set /memory reg "<00000000 08000000>" Usage: fdt - flattened device tree utility commands
And "fdt mknode" seems not to be implemented.
Am I doing something wrong?
Wolfgang.
Wolfgang Grandegger wrote:
Hi Jerry,
I gave the fdtlib of your git://cideas.us/pub/scm/u-boot/u-boot-fdt.git a try on my Icecube board. I got it built with the attached patch. libfdt was actually not made and the second hunk fixes a warning (=bug?). Then I was able to read and list a blob in memory:
=> fdt addr 20000 => fdt print / ...
A few other things didn't work, especially updating the FDT:
=> fdt addr 20000 10000 libfdt: FDT_ERR_BADVERSION
=> fdt env libfdt: FDT_ERR_BADVERSION => fdt bd_t libfdt: FDT_ERR_BADVERSION => fdt chosen libfdt: FDT_ERR_BADVERSION
=> fdt set /memory reg "<00000000 08000000>" Usage: fdt - flattened device tree utility commands
And "fdt mknode" seems not to be implemented.
Am I doing something wrong?
Wolfgang.
Hi Wolfgang,
Ouch, that was a bad bug and very embarrassing. Just when you think you are perfect, a stupid pointer error jumps up and bits you. :-( I'll apply your patch. Thanks & sorry.
The bad version error is because you are running a version 16 blob and you need a version 17 blob to allow modifications. David Gibson has an intention of upconverting a v16 to v17 as part of libfdt, but neither of us has gotten around to doing it yet.
If you pick up the latest dtc, it compiles v17 by default now. The latest dtc also implements a -S <minsize> parameter so you can make extra space in the blob and not need to specify the length parameter with the "fdt addr" command (the length parameter for addr makes the blob longer - unnecessary with -S blobs). If you really want to be at the bleeding edge, you can apply this patch as well, but it is *not* necessary (the patch pads out the blob rather than leaving the extra space undefined): http://article.gmane.org/gmane.linux.ports.ppc64.devel/18741
See also: http://www.denx.de/wiki/UBoot/UBootFdtInfo (linked off the Custodian page).
On a related note, you will probably want a fdt_find_compatible_node() function added to libfdt. I looked at the kernel one and it looks like it would be pretty simple to adapt it to libfdt, but I have not gotten to it yet.
http://www.denx.de/wiki/view/UBoot/UBootFdtInfo#ToDo_libfdt 2. fdt_find_compatible_node() Ref: arch/powerpc/kernel/prom.c * Needed if we use fdt blobs to configure u-boot drivers
Best regards, gvb
Jerry Van Baren wrote:
Wolfgang Grandegger wrote:
Hi Jerry,
I gave the fdtlib of your git://cideas.us/pub/scm/u-boot/u-boot-fdt.git a try on my Icecube board. I got it built with the attached patch. libfdt was actually not made and the second hunk fixes a warning (=bug?). Then I was able to read and list a blob in memory:
=> fdt addr 20000 => fdt print / ...
A few other things didn't work, especially updating the FDT:
=> fdt addr 20000 10000 libfdt: FDT_ERR_BADVERSION
=> fdt env libfdt: FDT_ERR_BADVERSION => fdt bd_t libfdt: FDT_ERR_BADVERSION => fdt chosen libfdt: FDT_ERR_BADVERSION
=> fdt set /memory reg "<00000000 08000000>" Usage: fdt - flattened device tree utility commands
And "fdt mknode" seems not to be implemented.
Am I doing something wrong?
Wolfgang.
Hi Wolfgang,
Ouch, that was a bad bug and very embarrassing. Just when you think you are perfect, a stupid pointer error jumps up and bits you. :-( I'll apply your patch. Thanks & sorry.
Well, nobody is ...
The bad version error is because you are running a version 16 blob and you need a version 17 blob to allow modifications. David Gibson has an intention of upconverting a v16 to v17 as part of libfdt, but neither of us has gotten around to doing it yet.
OK, I can now update the FDT, apart from "mknode", but have still problems booting Linux-2.6.21-rc7. Should it already work?
If you pick up the latest dtc, it compiles v17 by default now. The latest dtc also implements a -S <minsize> parameter so you can make extra space in the blob and not need to specify the length parameter with the "fdt addr" command (the length parameter for addr makes the blob longer - unnecessary with -S blobs). If you really want to be at the bleeding edge, you can apply this patch as well, but it is *not* necessary (the patch pads out the blob rather than leaving the extra space undefined): http://article.gmane.org/gmane.linux.ports.ppc64.devel/18741
See also: http://www.denx.de/wiki/UBoot/UBootFdtInfo (linked off the Custodian page).
Ah, I was not yet aware of that link. It's very useful, indeed.
On a related note, you will probably want a fdt_find_compatible_node() function added to libfdt. I looked at the kernel one and it looks like it would be pretty simple to adapt it to libfdt, but I have not gotten to it yet.
http://www.denx.de/wiki/view/UBoot/UBootFdtInfo#ToDo_libfdt 2. fdt_find_compatible_node() Ref: arch/powerpc/kernel/prom.c * Needed if we use fdt blobs to configure u-boot drivers
OK.
Thanks.
Wolfgang.
Wolfgang Grandegger wrote:
Jerry Van Baren wrote:
Wolfgang Grandegger wrote:
Hi Jerry,
I gave the fdtlib of your git://cideas.us/pub/scm/u-boot/u-boot-fdt.git a try on my Icecube board. I got it built with the attached patch. libfdt was actually not made and the second hunk fixes a warning (=bug?). Then I was able to read and list a blob in memory:
[snip]
The bad version error is because you are running a version 16 blob and you need a version 17 blob to allow modifications. David Gibson has an intention of upconverting a v16 to v17 as part of libfdt, but neither of us has gotten around to doing it yet.
OK, I can now update the FDT, apart from "mknode", but have still problems booting Linux-2.6.21-rc7. Should it already work?
Probably not. I've only done this on the MPC8360eMDS.
Looking at the "ToDo" table it says that ./board/icecube/icecube.c needs modifications to ft_board_setup() and ./cpu/mpc5xxx/cpu.c needs modifications to ft_cpu_setup() (you should be able to copy what I did for ./board/mpc8360emds/mpc8360emds.c and ./cpu/mpc83xx/cpu.c with some minimal mods).
If that isn't the solution, I don't know where the problem lies.
[snip]
Thanks. Wolfgang.
Good luck, gvb
Jerry Van Baren wrote:
The bad version error is because you are running a version 16 blob and you need a version 17 blob to allow modifications. David Gibson has an intention of upconverting a v16 to v17 as part of libfdt, but neither of us has gotten around to doing it yet.
In that case, "libfdt: FDT_ERR_BADVERSION" is a really useless error message. Please change it to something like, "Cannot modify this version (v16) of the device tree".
Now that we support multiple version of the device tree, it is very important that error message are clear. On a working system, it's unlikely people will update their device tree. They'll just update the kernel and U-Boot, and so people using older versions of the device tree need to be told very clearly what they can and can't do.
Timur Tabi wrote:
Jerry Van Baren wrote:
The bad version error is because you are running a version 16 blob and you need a version 17 blob to allow modifications.
Why? U-Boot was able to modify v16 blobs, why can't libfdt do the same?
Hi Timur,
It had very limited capabilities: it "modified" the blob by creating a new blob, copying the old stuff to the new blob, and then adding the "chosen" node. For some values, it rewrote values in place but was unable to create new properties or nodes. Switching to libfdt and insisting on having a v17 blob (with spare space) totally removes those limitations.
libfdt allows modifying the blob in place, but only if it is v17. I've taken advantage of this. libfdt actually has four sets of capabilities: 1) Read-only 2) Write sequentially (effectively what was done previously) 3) Write in place (modifying pre-existing values - also previous) 4) Full featured write (requires v17 and available space)
I've implemented write/modifies using the full write capabilities because it is the simplest for implementing the fdt command and most flexible by far (doing a write modifies the value or creates it if it doesn't exist).
An improvment to the "set" command would be to do a write in place if the full featured write failed (i.e. not a v17 blob or no space left). Patches are welcome. ;-)
Best regards, gvb
Wolfgang Grandegger wrote:
Hi Jerry,
I gave the fdtlib of your git://cideas.us/pub/scm/u-boot/u-boot-fdt.git a try on my Icecube board. I got it built with the attached patch. libfdt was actually not made and the second hunk fixes a warning (=bug?). Then I was able to read and list a blob in memory:
[snip]
diff --git a/Makefile b/Makefile index 9a27bc2..94cda54 100644 --- a/Makefile +++ b/Makefile @@ -219,6 +219,7 @@ LIBS += $(shell if [ -d post/cpu/$(CPU) ]; then echo \ LIBS += $(shell if [ -d post/board/$(BOARDDIR) ]; then echo \ "post/board/$(BOARDDIR)/libpost$(BOARD).a"; fi) LIBS += common/libcommon.a +LIBS += libfdt/libfdt.a LIBS += $(BOARDLIBS)
LIBS := $(addprefix $(obj),$(LIBS))
Ahh, that is not a bug, you fixed the wrong file. You need to add libfdt/libfdt.a to your board's BOARDLIBS in your board's config file (see line after your addition - been there, did the same thing ;-). There is no point building libfdt.a if it isn't used.
Best regards, gvb
Jerry Van Baren wrote:
Wolfgang Grandegger wrote:
Hi Jerry,
I gave the fdtlib of your git://cideas.us/pub/scm/u-boot/u-boot-fdt.git a try on my Icecube board. I got it built with the attached patch. libfdt was actually not made and the second hunk fixes a warning (=bug?). Then I was able to read and list a blob in memory:
[snip]
diff --git a/Makefile b/Makefile index 9a27bc2..94cda54 100644 --- a/Makefile +++ b/Makefile @@ -219,6 +219,7 @@ LIBS += $(shell if [ -d post/cpu/$(CPU) ]; then echo \ LIBS += $(shell if [ -d post/board/$(BOARDDIR) ]; then echo \ "post/board/$(BOARDDIR)/libpost$(BOARD).a"; fi) LIBS += common/libcommon.a +LIBS += libfdt/libfdt.a LIBS += $(BOARDLIBS)
LIBS := $(addprefix $(obj),$(LIBS))
Ahh, that is not a bug, you fixed the wrong file. You need to add libfdt/libfdt.a to your board's BOARDLIBS in your board's config file (see line after your addition - been there, did the same thing ;-). There is no point building libfdt.a if it isn't used.
Ah, I was already wondering why it did work on your side. Nevertheless, touching two places to use the library is cumbersome. But adding ifdef's to the source code is also not nice.
Wolfgang.
In message 46276E94.6090204@smiths-aerospace.com you wrote:
LIBS += $(shell if [ -d post/board/$(BOARDDIR) ]; then echo \ "post/board/$(BOARDDIR)/libpost$(BOARD).a"; fi) LIBS += common/libcommon.a +LIBS += libfdt/libfdt.a LIBS += $(BOARDLIBS)
LIBS := $(addprefix $(obj),$(LIBS))
Ahh, that is not a bug, you fixed the wrong file. You need to add libfdt/libfdt.a to your board's BOARDLIBS in your board's config file (see line after your addition - been there, did the same thing ;-). There is no point building libfdt.a if it isn't used.
That's not the way things are done in U-Boot. Please fix this so that all U-Boot libraries are handled in the same way.
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
In message 46276E94.6090204@smiths-aerospace.com you wrote:
LIBS += $(shell if [ -d post/board/$(BOARDDIR) ]; then echo \ "post/board/$(BOARDDIR)/libpost$(BOARD).a"; fi) LIBS += common/libcommon.a +LIBS += libfdt/libfdt.a LIBS += $(BOARDLIBS)
LIBS := $(addprefix $(obj),$(LIBS))
Ahh, that is not a bug, you fixed the wrong file. You need to add libfdt/libfdt.a to your board's BOARDLIBS in your board's config file (see line after your addition - been there, did the same thing ;-). There is no point building libfdt.a if it isn't used.
That's not the way things are done in U-Boot. Please fix this so that all U-Boot libraries are handled in the same way.
Best regards,
Wolfgang Denk
Hi wd,
I found the technique in u-boot and thought BOARDLIBS was the approved method. Looks like it is a NAND thing.
$ grep -r BOARDLIBS * board/prodrive/pdnb3/config.mk:BOARDLIBS = $(obj)cpu/ixp/npe/libnpe.a board/nc650/config.mk:BOARDLIBS = $(obj)drivers/nand/libnand.a board/ixdp425/config.mk:BOARDLIBS = $(obj)cpu/ixp/npe/libnpe.a board/mpc8360emds/config.mk:BOARDLIBS = libfdt/libfdt.a doc/README.nand:specific config.mk file should also have "BOARDLIBS = doc/README.nand:necessary, but the config.mk should have "BOARDLIBS = include/configs/delta.h:/* Use the new NAND code. (BOARDLIBS = drivers/nand/libnand.a required) */ include/configs/zylonite.h:/* Use the new NAND code. (BOARDLIBS = drivers/nand/libnand.a required) */ Makefile:LIBS += $(BOARDLIBS)
Not a problem, I'll revert back to wg's Makefile change.
Best regards, gvb
Hello,
in message 4627F709.8060502@cideas.com you wrote:
I found the technique in u-boot and thought BOARDLIBS was the approved method. Looks like it is a NAND thing.
I see.
$ grep -r BOARDLIBS * board/prodrive/pdnb3/config.mk:BOARDLIBS = $(obj)cpu/ixp/npe/libnpe.a board/nc650/config.mk:BOARDLIBS = $(obj)drivers/nand/libnand.a board/ixdp425/config.mk:BOARDLIBS = $(obj)cpu/ixp/npe/libnpe.a board/mpc8360emds/config.mk:BOARDLIBS = libfdt/libfdt.a doc/README.nand:specific config.mk file should also have "BOARDLIBS = doc/README.nand:necessary, but the config.mk should have "BOARDLIBS = include/configs/delta.h:/* Use the new NAND code. (BOARDLIBS = drivers/nand/libnand.a required) */ include/configs/zylonite.h:/* Use the new NAND code. (BOARDLIBS = drivers/nand/libnand.a required) */ Makefile:LIBS += $(BOARDLIBS)
Thanks for pointing out. I will ask Stefan to clean this up.
Hi Stefan!
Not a problem, I'll revert back to wg's Makefile change.
Thanks.
Best regards,
Wolfgang Denk
Hi Jerry,
Jerry Van Baren wrote: [...]
Hi wd,
I found the technique in u-boot and thought BOARDLIBS was the approved method. Looks like it is a NAND thing.
$ grep -r BOARDLIBS * board/prodrive/pdnb3/config.mk:BOARDLIBS = $(obj)cpu/ixp/npe/libnpe.a board/nc650/config.mk:BOARDLIBS = $(obj)drivers/nand/libnand.a board/ixdp425/config.mk:BOARDLIBS = $(obj)cpu/ixp/npe/libnpe.a board/mpc8360emds/config.mk:BOARDLIBS = libfdt/libfdt.a doc/README.nand:specific config.mk file should also have "BOARDLIBS = doc/README.nand:necessary, but the config.mk should have "BOARDLIBS = include/configs/delta.h:/* Use the new NAND code. (BOARDLIBS = drivers/nand/libnand.a required) */ include/configs/zylonite.h:/* Use the new NAND code. (BOARDLIBS = drivers/nand/libnand.a required) */ Makefile:LIBS += $(BOARDLIBS)
Not a problem, I'll revert back to wg's Makefile change.
In U-Boot, it's common practice to use "ugly" ifdef's in the source files mainly to reduce compile time as shown in the attached patch.
Wolfgang.
Wolfgang Grandegger wrote:
Hi Jerry,
Jerry Van Baren wrote: [...]
Hi wd,
[snip]
In U-Boot, it's common practice to use "ugly" ifdef's in the source files mainly to reduce compile time as shown in the attached patch.
Wolfgang.
diff --git a/libfdt/fdt.c b/libfdt/fdt.c index 212b838..1ee67ad 100644 --- a/libfdt/fdt.c +++ b/libfdt/fdt.c @@ -16,6 +16,9 @@
- License along with this library; if not, write to the Free Software
- Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
*/ +#include "config.h" +#if CONFIG_OF_LIBFDT
#include "libfdt_env.h"
#include <fdt.h> @@ -83,3 +86,5 @@ int fdt_move(const void *fdt, void *buf, int bufsize) memmove(buf, fdt, fdt_totalsize(fdt)); return 0; }
+#endif /* CONFIG_OF_LIBFDT */
[snip more of the same]
Hi Wolfgang,
At this point I'm reluctant to do this. We've basically forked libfdt in that I've added and changed the source and David Gibson isn't in a position to accept the changes back into the original, but it is all generically useful libfdt code.
Adding the above to all of the files makes it that much more u-boot specific which will make it that much more difficult to unfork. On the other hand, it is "only" three lines per file.
Anyone care to weigh in on the issue? wd?
Best regards, gvb
In message 4628F431.1020901@smiths-aerospace.com you wrote:
At this point I'm reluctant to do this. We've basically forked libfdt
I perfectly understand your reluctance. It's ugly, and a poor worka- round for a problem that should besolved differently (i. e. by not compiling unneeded files at all).
If you want to do it right, it comes down to a rework of the configuration and build system. As much as I would like to see work being done on this, as much I am also aware that we have even more urgent tasks to solve at the moment (like getting the delays in patch processing down).
Adding the above to all of the files makes it that much more u-boot specific which will make it that much more difficult to unfork. On the other hand, it is "only" three lines per file.
Anyone care to weigh in on the issue? wd?
It's just 6 files, and the changes are trivial to do and to undo. Since Wolfgang G. already spent the effort to implement it, I suggect to add this. If we want to get rid of this later, a simple "patch -R" will probably be all that's needed.
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
In message 4628F431.1020901@smiths-aerospace.com you wrote:
At this point I'm reluctant to do this. We've basically forked libfdt
I perfectly understand your reluctance. It's ugly, and a poor worka- round for a problem that should besolved differently (i. e. by not compiling unneeded files at all).
If you want to do it right, it comes down to a rework of the configuration and build system. As much as I would like to see work being done on this, as much I am also aware that we have even more urgent tasks to solve at the moment (like getting the delays in patch processing down).
Adding the above to all of the files makes it that much more u-boot specific which will make it that much more difficult to unfork. On the other hand, it is "only" three lines per file.
Anyone care to weigh in on the issue? wd?
It's just 6 files, and the changes are trivial to do and to undo. Since Wolfgang G. already spent the effort to implement it, I suggect to add this. If we want to get rid of this later, a simple "patch -R" will probably be all that's needed.
Best regards, Wolfgang Denk
Hi Wolfgang,
...and there's the irony. With it as a library included by BOARDLIBS in the board config file, it is only compiled if it is called for in the board's config.mk.
It seems like it would be overall a win to have more of a real library approach. I have not gone down that path hardly at all, however, and it is likely to have briers that I'm not aware of. It also would take time. :-/
Having said that, I don't have any real issue in applying wg's patch. It isn't a big deal and will match the rest of u-boot methodology. At the end of the day, it is a lot easier than converting everything to true libraries. ;-]
Best regards, gvb
Wolfgang Grandegger wrote:
Hi Jerry,
I gave the fdtlib of your git://cideas.us/pub/scm/u-boot/u-boot-fdt.git a try on my Icecube board. I got it built with the attached patch. libfdt was actually not made and the second hunk fixes a warning (=bug?). Then I was able to read and list a blob in memory:
[snip]
Wolfgang.
Hi Wolfgang (Grendegger|Denk),
wg: Thanks for the patch, I've applied it (both the pointer bug fix and the Makefile change) and pushed it to the u-boot-fdt.
wd: Please merge the change, it is a serious bug which will cause unreliable bootm operation for "libfdt" builds (fortunately a limited audience at the moment :-/).
To be explicit, Wolfgang, please pull the *master* branch from: git://denx.de/git/u-boot-fdt.git
Thanks, gvb
In message 462835C4.3010705@gmail.com you wrote:
To be explicit, Wolfgang, please pull the *master* branch from: git://denx.de/git/u-boot-fdt.git
Done.
Best regards,
Wolfgang Denk
Wolfgang Grandegger wrote:
Hi Jerry,
I gave the fdtlib of your git://cideas.us/pub/scm/u-boot/u-boot-fdt.git a try on my Icecube board. I got it built with the attached patch. libfdt was actually not made and the second hunk fixes a warning (=bug?). Then I was able to read and list a blob in memory:
[snip]
And "fdt mknode" seems not to be implemented.
Am I doing something wrong?
Wolfgang.
I missed the last statement/question earlier: yes, fdt mknode is not implemented. I got a little carried away and put that in the help prematurely. I've added it to my "ToDo" list.
Unfortunately, I implemented a "move" command which collides on the first letter with "mknode". :-( ...and I was being so good at keeping the commands unique in the first letter.
Best regards, gvb
participants (6)
-
Jerry Van Baren -
Jerry Van Baren -
Jerry Van Baren -
Timur Tabi -
Wolfgang Denk -
Wolfgang Grandegger