| Message ID | 1356722226-23186-6-git-send-email-otavio@ossystems.com.br |
|---|---|
| State | Changes Requested |
| Delegated to: | Stefano Babic |
| Headers | show |
On Fri, Dec 28, 2012 at 05:17:05PM -0200, Otavio Salvador wrote: > Signed-off-by: Otavio Salvador <otavio@ossystems.com.br> > --- > Changes in v2: > - Allow use of dynamic/static ip > - Allow force use, or not, of fdt > - Change 'auto' to 'try' [snip] > @@ -169,13 +173,43 @@ > "loaduimage=fatload mmc ${mmcdev}:${mmcpart} ${loadaddr} ${uimage}\0" \ > "mmcboot=echo Booting from mmc ...; " \ > "run mmcargs; " \ > - "bootm\0" \ > + "if test ${boot_fdt} = yes; then " \ > + "if fatload mmc ${mmcdev}:${mmcpart} ${ftd_addr} ${ftd_file}; then " \ How about adding loadfdtfile after 'loaduimage' above? > + "bootm ${loadaddr} - ${ftd_addr}; " \ > + "else " \ > + "if test ${boot_fdt} = try; then " \ > + "bootm; " \ > + "else " \ > + "echo ERROR: Cannot load the DT, aborting...; " \ Strings must not be broken, so this is fine. But, you aren't really aborting. If you had a loop of "try mmcboot. Fail? Try netboot" it would continue. So the error message should perhaps just be about cannot load DT from mmc?
On Fri, Dec 28, 2012 at 05:17:05PM -0200, Otavio Salvador wrote: > Signed-off-by: Otavio Salvador <otavio@ossystems.com.br> > --- > Changes in v2: > - Allow use of dynamic/static ip > - Allow force use, or not, of fdt > - Change 'auto' to 'try' [snip] > + "if run ${get_cmd} ${ftd_addr} ${ftd_file}; then " \ Oh, and I forgot about this line. If you use space not tab at the end here, it's under 80 wide again.
On Mon, Jan 7, 2013 at 12:04 PM, Tom Rini <trini@ti.com> wrote: > On Fri, Dec 28, 2012 at 05:17:05PM -0200, Otavio Salvador wrote: > >> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br> >> --- >> Changes in v2: >> - Allow use of dynamic/static ip >> - Allow force use, or not, of fdt >> - Change 'auto' to 'try' > [snip] >> @@ -169,13 +173,43 @@ >> "loaduimage=fatload mmc ${mmcdev}:${mmcpart} ${loadaddr} ${uimage}\0" \ >> "mmcboot=echo Booting from mmc ...; " \ >> "run mmcargs; " \ >> - "bootm\0" \ >> + "if test ${boot_fdt} = yes; then " \ >> + "if fatload mmc ${mmcdev}:${mmcpart} ${ftd_addr} ${ftd_file}; then " \ > > How about adding loadfdtfile after 'loaduimage' above? The command for netboot is different so we'd need to have two vars for it. I think it will be confusing ... >> + "bootm ${loadaddr} - ${ftd_addr}; " \ >> + "else " \ >> + "if test ${boot_fdt} = try; then " \ >> + "bootm; " \ >> + "else " \ >> + "echo ERROR: Cannot load the DT, aborting...; " \ > > Strings must not be broken, so this is fine. But, you aren't really > aborting. If you had a loop of "try mmcboot. Fail? Try netboot" it > would continue. So the error message should perhaps just be about > cannot load DT from mmc? I don't have a strong opinion about this. I'd prefer to abort here so user really knows it failed. In case user wish it to not be fatal it can use the 'try' in boot_fdt. How I could abort here? -- Otavio Salvador O.S. Systems E-mail: otavio@ossystems.com.br http://www.ossystems.com.br Mobile: +55 53 9981-7854 http://projetos.ossystems.com.br
On Mon, Jan 7, 2013 at 12:06 PM, Tom Rini <trini@ti.com> wrote: > On Fri, Dec 28, 2012 at 05:17:05PM -0200, Otavio Salvador wrote: > >> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br> >> --- >> Changes in v2: >> - Allow use of dynamic/static ip >> - Allow force use, or not, of fdt >> - Change 'auto' to 'try' > [snip] >> + "if run ${get_cmd} ${ftd_addr} ${ftd_file}; then " \ > > Oh, and I forgot about this line. If you use space not tab at the end > here, it's under 80 wide again. Well, yes the code style is using tabs so I preferred to not mix both. I am more keen to keep it as is if it is not a requirement to get it applied. -- Otavio Salvador O.S. Systems E-mail: otavio@ossystems.com.br http://www.ossystems.com.br Mobile: +55 53 9981-7854 http://projetos.ossystems.com.br
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 01/07/2013 09:17 AM, Otavio Salvador wrote: > On Mon, Jan 7, 2013 at 12:04 PM, Tom Rini <trini@ti.com> wrote: >> On Fri, Dec 28, 2012 at 05:17:05PM -0200, Otavio Salvador wrote: >> >>> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br> --- >>> Changes in v2: - Allow use of dynamic/static ip - Allow force >>> use, or not, of fdt - Change 'auto' to 'try' >> [snip] >>> @@ -169,13 +173,43 @@ "loaduimage=fatload mmc >>> ${mmcdev}:${mmcpart} ${loadaddr} ${uimage}\0" \ "mmcboot=echo >>> Booting from mmc ...; " \ "run mmcargs; " \ - "bootm\0" \ + >>> "if test ${boot_fdt} = yes; then " \ + "if >>> fatload mmc ${mmcdev}:${mmcpart} ${ftd_addr} ${ftd_file}; then >>> " \ >> >> How about adding loadfdtfile after 'loaduimage' above? > > The command for netboot is different so we'd need to have two vars > for it. I think it will be confusing ... Yes, but there's already loaduimage that's fixed to FAT from MMC. So you're being consistent. >>> + "bootm ${loadaddr} - >>> ${ftd_addr}; " \ + "else " \ + "if test >>> ${boot_fdt} = try; then " \ + "bootm; " \ + "else " \ + "echo >>> ERROR: Cannot load the DT, aborting...; " \ >> >> Strings must not be broken, so this is fine. But, you aren't >> really aborting. If you had a loop of "try mmcboot. Fail? Try >> netboot" it would continue. So the error message should perhaps >> just be about cannot load DT from mmc? > > I don't have a strong opinion about this. I'd prefer to abort here > so user really knows it failed. In case user wish it to not be > fatal it can use the 'try' in boot_fdt. > > How I could abort here? Without firing up a board, maybe tossing in break? Or setting a variable (abortcmd) that you could test for in other clauses to make sure it's obeyed and we break out? - -- Tom -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with undefined - http://www.enigmail.net/ iQIcBAEBAgAGBQJQ6tm2AAoJENk4IS6UOR1WefAQALNWepGwYYkyUkoj2YfqQQf6 D23SyuZRAEUKxi7mkFZdBEDwyIHVvM8v4HEXkxMigKq6jd++xpGWqOvygUhu7NCK i4vfPXadgyt+3BLt5BGLLQDf1GNrsTpxjPuw+AEDsdt0hgo9fWSoOegpsO2q6kIj xSspwUpzBE6Dtzh9uomRw7J/o4uvDdRv7MC4krWB70SC9y8hN0p/syV4cYvjqLTp WWawT7qKfNwkfPH5tAmPb5b2V3JVGSMkvOMPuH3QoNSfhVTSE6EdKK1v3WWa933Z ucAGtJgDlPgRhTioIp78Nsi1JXwygHByA/ZYnxre9jimJZ5lxd8m5TpqwgnpkJN0 2sUtSg3WcSSqr+ckMXOswKPfT9wx0Vgy+elP/i4LlJ3d1uoDJufLt+v0nkKYHDR0 foC0udYrjSDZiA1kmX8u4w+sVGWAlAgawkiDSA3hYY1Y+GUeVzKGxwMNSvLrzrWq ga3aAWbr26KIZaBBjHRPlkPWzo8rvLDCWEWQkSBnF+o3Zccc5tebGIhYp/eMxt+T ewUCT0pW5HuXAeRvqNPwdiGDw2qrvFcrd6rnVpSNic/0rYyJRTUa4hmWv/FZm8PQ whCijnBCBVRnBc+M7K2YJTFcRlR4oC0GnLe6K7gDGT5fhHln11AbHa0san8DkrnE bzvjfkjxGtsaNKzR7wNV =Ew4G -----END PGP SIGNATURE-----
diff --git a/include/configs/mx6qsabrelite.h b/include/configs/mx6qsabrelite.h index ee86f9b..7ca12d9 100644 --- a/include/configs/mx6qsabrelite.h +++ b/include/configs/mx6qsabrelite.h @@ -157,6 +157,10 @@ "console=ttymxc1\0" \ "fdt_high=0xffffffff\0" \ "initrd_high=0xffffffff\0" \ + "ftd_file=imx6q-sabrelite.dtb\0" \ + "ftd_addr=0x11000000\0" \ + "boot_fdt=try\0" \ + "ip_dyn=yes\0" \ "mmcdev=0\0" \ "mmcpart=2\0" \ "mmcroot=/dev/mmcblk0p3 rootwait rw\0" \ @@ -169,13 +173,43 @@ "loaduimage=fatload mmc ${mmcdev}:${mmcpart} ${loadaddr} ${uimage}\0" \ "mmcboot=echo Booting from mmc ...; " \ "run mmcargs; " \ - "bootm\0" \ + "if test ${boot_fdt} = yes; then " \ + "if fatload mmc ${mmcdev}:${mmcpart} ${ftd_addr} ${ftd_file}; then " \ + "bootm ${loadaddr} - ${ftd_addr}; " \ + "else " \ + "if test ${boot_fdt} = try; then " \ + "bootm; " \ + "else " \ + "echo ERROR: Cannot load the DT, aborting...; " \ + "fi;\0" \ + "fi;\0" \ + "else " \ + "bootm; " \ + "fi;\0" \ "netargs=setenv bootargs console=${console},${baudrate} " \ "root=/dev/nfs " \ "ip=dhcp nfsroot=${serverip}:${nfsroot},v3,tcp\0" \ "netboot=echo Booting from net ...; " \ "run netargs; " \ - "dhcp ${uimage}; bootm\0" + "if test ${ip_dyn} = yes; then " \ + "setenv get_cmd dhcp; " \ + "else " \ + "setenv get_cmd tftp; " \ + "fi;\0" \ + "run ${get_cmd} ${uimage}; " \ + "if test ${boot_fdt} = no; then " \ + "if run ${get_cmd} ${ftd_addr} ${ftd_file}; then " \ + "bootm ${loadaddr} - ${ftd_addr}; " \ + "else " \ + "if test ${boot_fdt} = try; then " \ + "bootm; " \ + "else " \ + "echo ERROR: Cannot load the DT, aborting...; " \ + "fi;\0" \ + "fi;\0" \ + "else " \ + "bootm; " \ + "fi;\0" #define CONFIG_BOOTCOMMAND \ "mmc dev ${mmcdev};" \
Signed-off-by: Otavio Salvador <otavio@ossystems.com.br> --- Changes in v2: - Allow use of dynamic/static ip - Allow force use, or not, of fdt - Change 'auto' to 'try' include/configs/mx6qsabrelite.h | 38 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-)