Patchwork [U-Boot,v2,5/6] mx6qsabrelite: Add support to dynamically choose between ftd use or not

login
register
mail settings
Submitter Otavio Salvador
Date Dec. 28, 2012, 7:17 p.m.
Message ID <1356722226-23186-6-git-send-email-otavio@ossystems.com.br>
Download mbox | patch
Permalink /patch/208530/
State Changes Requested
Delegated to: Stefano Babic
Headers show

Comments

Otavio Salvador - Dec. 28, 2012, 7:17 p.m.
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(-)
Tom Rini - Jan. 7, 2013, 2:04 p.m.
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?
Tom Rini - Jan. 7, 2013, 2:06 p.m.
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.
Otavio Salvador - Jan. 7, 2013, 2:17 p.m.
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
Otavio Salvador - Jan. 7, 2013, 2:18 p.m.
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
Tom Rini - Jan. 7, 2013, 2:20 p.m.
-----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-----

Patch

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};" \