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

[Wolfgang Denk]

Dear Linus Walleij,

In message <1363156087-23881-1-git-send-email-linus.walleij at 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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
He who hesitates is not only lost, but miles from the next exit.