[U-Boot] [PATCH v3] integrator: pass a Device Tree by default

This, enabled the FDT library for the Integrators, updates the Integrator/CP default command to load and pass a Device Tree when booting the kernel from the on-board ethernet, define same environment for the Integrator/AP and move the load address around to something even.
Signed-off-by: Linus Walleij linus.walleij@linaro.org --- ChangeLog v2->v3: - Replace $servip by $serverip as is custom in other configs. ChangeLog v1->v2: - Skip definition of $loadaddr, rely on CONFIG_LOAD_ADDR instead. --- include/configs/integrator-common.h | 3 ++- include/configs/integratorap.h | 7 +++++-- include/configs/integratorcp.h | 9 ++++++--- 3 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/include/configs/integrator-common.h b/include/configs/integrator-common.h index 564b418..f4a182c 100644 --- a/include/configs/integrator-common.h +++ b/include/configs/integrator-common.h @@ -30,7 +30,7 @@ #define CONFIG_SYS_MEMTEST_END 0x10000000 #define CONFIG_SYS_HZ 1000 #define CONFIG_SYS_TIMERBASE 0x13000100 /* Timer1 */ -#define CONFIG_SYS_LOAD_ADDR 0x7fc0 /* default load address */ +#define CONFIG_SYS_LOAD_ADDR 0x800 /* default load address */ #define CONFIG_SYS_LONGHELP #define CONFIG_SYS_HUSH_PARSER #define CONFIG_SYS_CBSIZE 256 /* Console I/O Buffer Size*/ @@ -41,6 +41,7 @@
#define CONFIG_CMDLINE_TAG /* enable passing of ATAGs */ #define CONFIG_SETUP_MEMORY_TAGS +#define CONFIG_OF_LIBFDT /* enable passing a Device Tree */ #define CONFIG_MISC_INIT_R /* call misc_init_r during start up */
/* diff --git a/include/configs/integratorap.h b/include/configs/integratorap.h index c6907b5..6aaafba 100644 --- a/include/configs/integratorap.h +++ b/include/configs/integratorap.h @@ -62,9 +62,12 @@ */ #include <config_cmd_default.h>
-#define CONFIG_BOOTDELAY 2 +#define CONFIG_BOOTDELAY 0 #define CONFIG_BOOTARGS "root=/dev/mtdblock0 console=ttyAM0 console=tty" -#define CONFIG_BOOTCOMMAND "" +#define CONFIG_BOOTCOMMAND "setenv serverip 192.168.1.100 ; " \ + "setenv fdtaddr 0x00800000 ; " \ + "echo "\\$loadaddr = $loadaddr, \\$fdtaddr=$fdtaddr" ; " \ + "echo "load binaries then: bootm $loadaddr - $fdtaddr""
/* * Miscellaneous configurable options diff --git a/include/configs/integratorcp.h b/include/configs/integratorcp.h index ca02a6f..0844d18 100644 --- a/include/configs/integratorcp.h +++ b/include/configs/integratorcp.h @@ -60,11 +60,14 @@ #include <config_cmd_default.h>
#define CONFIG_BOOTDELAY 2 -#define CONFIG_BOOTARGS "root=/dev/mtdblock0 console=ttyAMA0 console=tty ip=dhcp netdev=27,0,0xfc800000,0xfc800010,eth0 video=clcdfb:0" -#define CONFIG_BOOTCOMMAND "tftpboot ; bootm" #define CONFIG_SERVERIP 192.168.1.100 #define CONFIG_IPADDR 192.168.1.104 -#define CONFIG_BOOTFILE "uImage" +#define CONFIG_BOOTARGS "root=/dev/mtdblock0 console=ttyAMA0 console=tty ip=dhcp netdev=27,0,0xfc800000,0xfc800010,eth0 video=clcdfb:0" +#define CONFIG_BOOTCOMMAND "setenv serverip 192.168.1.100 ; " \ + "setenv fdtaddr 0x00800000 ; " \ + "bootp $loadaddr $serverip:uImage ; " \ + "bootp $fdtaddr $serverip:integratorcp.dtb ; " \ + "bootm $loadaddr - $fdtaddr"
/* * Miscellaneous configurable options

Dear Linus Walleij,
In message 1363156087-23881-1-git-send-email-linus.walleij@linaro.org you wrote:
This, enabled the FDT library for the Integrators, updates the Integrator/CP default command to load and pass a Device Tree when booting the kernel from the on-board ethernet, define same environment for the Integrator/AP and move the load address around to something even.
Comment and code do not match.
-#define CONFIG_SYS_LOAD_ADDR 0x7fc0 /* default load address */ +#define CONFIG_SYS_LOAD_ADDR 0x800 /* default load address */
This appears to be an unrelated change. It should be clearly documented, especially as users who just update U-Boot on their board may have to make this change manually.
-#define CONFIG_BOOTDELAY 2 +#define CONFIG_BOOTDELAY 0
This is also an undocumented change, and one that is changing the board behaviour for all users. Is this really a good idea?
#define CONFIG_BOOTARGS "root=/dev/mtdblock0 console=ttyAM0 console=tty" -#define CONFIG_BOOTCOMMAND "" +#define CONFIG_BOOTCOMMAND "setenv serverip 192.168.1.100 ; " \
- "setenv fdtaddr 0x00800000 ; " \
- "echo "\\$loadaddr = $loadaddr, \\$fdtaddr=$fdtaddr" ; " \
- "echo "load binaries then: bootm $loadaddr - $fdtaddr""
We don't allow statis network parameter settings in config files,
Also, your boot command just echos some text, then - the comments claims it would do somethign else.
#define CONFIG_SERVERIP 192.168.1.100 #define CONFIG_IPADDR 192.168.1.104
Please get rid of these. We don't allow this in board config files.
-#define CONFIG_BOOTFILE "uImage" +#define CONFIG_BOOTARGS "root=/dev/mtdblock0 console=ttyAMA0 console=tty ip=dhcp netdev=27,0,0xfc800000,0xfc800010,eth0 video=clcdfb:0" +#define CONFIG_BOOTCOMMAND "setenv serverip 192.168.1.100 ; " \
NAK. We don't allow this in board config files.
- "setenv fdtaddr 0x00800000 ; " \
- "bootp $loadaddr $serverip:uImage ; " \
- "bootp $fdtaddr $serverip:integratorcp.dtb ; " \
- "bootm $loadaddr - $fdtaddr"
Is it intentional that integratorap.h and integratorcp.h are now configured so differently?
Best regards,
Wolfgang Denk

On Wed, Mar 13, 2013 at 8:14 AM, Wolfgang Denk wd@denx.de wrote:
-#define CONFIG_SYS_LOAD_ADDR 0x7fc0 /* default load address */ +#define CONFIG_SYS_LOAD_ADDR 0x800 /* default load address */
This appears to be an unrelated change. It should be clearly documented, especially as users who just update U-Boot on their board may have to make this change manually.
OK moved this out to a separate patch. Not very important anyway, I just never understood the very weird offset 0x7fc0, because when I tested it any address sort of works...
Any hints on why it is (historically) set to 0x7fc0?
-#define CONFIG_BOOTDELAY 2 +#define CONFIG_BOOTDELAY 0
This is also an undocumented change, and one that is changing the board behaviour for all users. Is this really a good idea?
I just skipped this. Basically this is because there is no default boot script on the Integrator/AP anyway, but let's take that as a separate patch.
#define CONFIG_BOOTARGS "root=/dev/mtdblock0 console=ttyAM0 console=tty" -#define CONFIG_BOOTCOMMAND "" +#define CONFIG_BOOTCOMMAND "setenv serverip 192.168.1.100 ; " \
- "setenv fdtaddr 0x00800000 ; " \
- "echo "\\$loadaddr = $loadaddr, \\$fdtaddr=$fdtaddr" ; " \
- "echo "load binaries then: bootm $loadaddr - $fdtaddr""
We don't allow statis network parameter settings in config files,
Also, your boot command just echos some text, then - the comments claims it would do somethign else.
OK I'll update the comment. The only idea with the echoed text is to be helpful since you have to enter the boot sequence manually on the Integrator/AP (since ethernet is not always available, as it is on PCI).
#define CONFIG_SERVERIP 192.168.1.100 #define CONFIG_IPADDR 192.168.1.104
Please get rid of these. We don't allow this in board config files.
I'll try to convert this to using DHCP.
-#define CONFIG_BOOTFILE "uImage" +#define CONFIG_BOOTARGS "root=/dev/mtdblock0 console=ttyAMA0 console=tty ip=dhcp netdev=27,0,0xfc800000,0xfc800010,eth0 video=clcdfb:0" +#define CONFIG_BOOTCOMMAND "setenv serverip 192.168.1.100 ; " \
NAK. We don't allow this in board config files.
OK same thing.
- "setenv fdtaddr 0x00800000 ; " \
- "bootp $loadaddr $serverip:uImage ; " \
- "bootp $fdtaddr $serverip:integratorcp.dtb ; " \
- "bootm $loadaddr - $fdtaddr"
Is it intentional that integratorap.h and integratorcp.h are now configured so differently?
Yes, the Integrator/CP has an ethernet adapter (SMSC91x) that we know will always be available and preferred boot path.
The Integrator/AP has an optional Ethernet PCI card, but we cannot rely on that always being present.
Thanks, Linus Walleij

Dear Linus,
In message CACRpkdZ2XCBU9QvFUg=c_2E=QQP=fmRaup-EnEWx3hWzqvtqcA@mail.gmail.com you wrote:
-#define CONFIG_SYS_LOAD_ADDR 0x7fc0 /* default load address */ +#define CONFIG_SYS_LOAD_ADDR 0x800 /* default load address */
This appears to be an unrelated change. It should be clearly documented, especially as users who just update U-Boot on their board may have to make this change manually.
OK moved this out to a separate patch. Not very important anyway, I just never understood the very weird offset 0x7fc0, because when I tested it any address sort of works...
Any hints on why it is (historically) set to 0x7fc0?
I can only speculate - 0x40 = 64 bytes is the size of the old legacy uImage header; with this load address the payload would be aligned at 0x8000. Eventually this was used to avoid a memcpy in case of uncompressed kernel images?
#define CONFIG_SERVERIP 192.168.1.100 #define CONFIG_IPADDR 192.168.1.104
Please get rid of these. We don't allow this in board config files.
I'll try to convert this to using DHCP.
Thanks.
- "setenv fdtaddr 0x00800000 ; " \
- "bootp $loadaddr $serverip:uImage ; " \
- "bootp $fdtaddr $serverip:integratorcp.dtb ; " \
- "bootm $loadaddr - $fdtaddr"
Is it intentional that integratorap.h and integratorcp.h are now configured so differently?
Yes, the Integrator/CP has an ethernet adapter (SMSC91x) that we know will always be available and preferred boot path.
The Integrator/AP has an optional Ethernet PCI card, but we cannot rely on that always being present.
Hm... that's a change of behaviour to all users, which is not really nice. But OK, you're the board maintainer...
Best regards,
Wolfgang Denk
participants (2)
-
Linus Walleij
-
Wolfgang Denk