Patchwork [U-Boot,7/9] mx53loco: Add support to dynamically choose between ftd use or not

login
register
mail settings
Submitter Otavio Salvador
Date Dec. 21, 2012, 4:59 p.m.
Message ID <1356109151-6623-8-git-send-email-otavio@ossystems.com.br>
Download mbox | patch
Permalink /patch/207848/
State Changes Requested
Delegated to: Stefano Babic
Headers show

Comments

Otavio Salvador - Dec. 21, 2012, 4:59 p.m.
Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
---
 include/configs/mx53loco.h |   15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)
Stefano Babic - Dec. 26, 2012, 11:31 a.m.
On 21/12/2012 17:59, Otavio Salvador wrote:
> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
> ---
>  include/configs/mx53loco.h |   15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 

Hi Otavio,

> diff --git a/include/configs/mx53loco.h b/include/configs/mx53loco.h
> index 4cfdc0b..505957c 100644
> --- a/include/configs/mx53loco.h
> +++ b/include/configs/mx53loco.h
> @@ -118,6 +118,8 @@
>  #define CONFIG_EXTRA_ENV_SETTINGS \
>  	"script=boot.scr\0" \
>  	"uimage=uImage\0" \
> +	"ftd_file=imx53-qsb.dtb\0" \
> +	"ftd_addr=0x71000000\0" \
>  	"mmcdev=0\0" \
>  	"mmcpart=2\0" \
>  	"mmcroot=/dev/mmcblk0p3 rw rootwait\0" \
> @@ -129,13 +131,22 @@
>  	"loaduimage=fatload mmc ${mmcdev}:${mmcpart} ${loadaddr} ${uimage}\0" \
>  	"mmcboot=echo Booting from mmc ...; " \
>  		"run mmcargs; " \
> -		"bootm\0" \
> +		"if fatload mmc ${mmcdev}:${mmcpart} ${ftd_addr} ${ftd_file}; then " \
> +			"bootm ${loadaddr} - ${ftd_addr}; " \
> +		"else " \
> +			"bootm; " \
> +		"fi;\0" \
>  	"netargs=setenv bootargs console=ttymxc0,${baudrate} " \
>  		"root=/dev/nfs " \
>  		"ip=dhcp nfsroot=${serverip}:${nfsroot},v3,tcp\0" \
>  	"netboot=echo Booting from net ...; " \
>  		"run netargs; " \
> -		"dhcp ${uimage}; bootm\0" \
> +		"dhcp ${uimage}; " \
> +		"if dhcp ${ftd_addr} ${ftd_file}; then " \
> +			"bootm ${loadaddr} - ${ftd_addr}; " \
> +		"else " \
> +			"bootm; " \
> +		"fi;\0"
>  

Only to remark that this is the correct behavior. The kernel with fdt is
booted independently if this is wanted or not, but if the fdt file is
loaded successfully from MMC or network. This can have some drawback
effects if, for example, the fdt is simply stored on the TFTP server,
but we want to boot a kernel without DT. For example, when someone wants
to test both kernels or there are multiple instances of the same board
type (mx53loco in this case) loading from the same TFTP server.

In your patch, the behavior depends if the fdt is simply present on the
media, but this does not always mean that the file must be loaded.
Should be not better to set variable as flag to force the desired
behavior and to be sure that the system does not boot in a different way
as the user thinks ?

In other words, something like :

	"if boot_fdt; then " \
		"if dhcp ${ftd_addr} ${ftd_file}; then " \
			"bootm ${loadaddr} - ${ftd_addr}; " \
		"else " \
			"echo Error: I cannot boot a DT kernel; \"
		"fi; \"
	" else bootm; fi;\0"

Best regards,
Stefano Babic
Fabio Estevam - Dec. 26, 2012, 12:14 p.m.
On Wed, Dec 26, 2012 at 9:31 AM, Stefano Babic <sbabic@denx.de> wrote:

> Only to remark that this is the correct behavior. The kernel with fdt is
> booted independently if this is wanted or not, but if the fdt file is
> loaded successfully from MMC or network. This can have some drawback
> effects if, for example, the fdt is simply stored on the TFTP server,
> but we want to boot a kernel without DT. For example, when someone wants
> to test both kernels or there are multiple instances of the same board
> type (mx53loco in this case) loading from the same TFTP server.
>
> In your patch, the behavior depends if the fdt is simply present on the
> media, but this does not always mean that the file must be loaded.
> Should be not better to set variable as flag to force the desired
> behavior and to be sure that the system does not boot in a different way
> as the user thinks ?
>
> In other words, something like :
>
>         "if boot_fdt; then " \
>                 "if dhcp ${ftd_addr} ${ftd_file}; then " \
>                         "bootm ${loadaddr} - ${ftd_addr}; " \
>                 "else " \
>                         "echo Error: I cannot boot a DT kernel; \"
>                 "fi; \"
>         " else bootm; fi;\0"


Yes, I think this is a good idea and would allow us to easily switch
from dt to non-dt kernel during tests.

Regards,

Fabio Estevam
Otavio Salvador - Dec. 26, 2012, 6:23 p.m.
On Wed, Dec 26, 2012 at 10:14 AM, Fabio Estevam <festevam@gmail.com> wrote:
> On Wed, Dec 26, 2012 at 9:31 AM, Stefano Babic <sbabic@denx.de> wrote:
>
>> Only to remark that this is the correct behavior. The kernel with fdt is
>> booted independently if this is wanted or not, but if the fdt file is
>> loaded successfully from MMC or network. This can have some drawback
>> effects if, for example, the fdt is simply stored on the TFTP server,
>> but we want to boot a kernel without DT. For example, when someone wants
>> to test both kernels or there are multiple instances of the same board
>> type (mx53loco in this case) loading from the same TFTP server.
>>
>> In your patch, the behavior depends if the fdt is simply present on the
>> media, but this does not always mean that the file must be loaded.
>> Should be not better to set variable as flag to force the desired
>> behavior and to be sure that the system does not boot in a different way
>> as the user thinks ?
>>
>> In other words, something like :
>>
>>         "if boot_fdt; then " \
>>                 "if dhcp ${ftd_addr} ${ftd_file}; then " \
>>                         "bootm ${loadaddr} - ${ftd_addr}; " \
>>                 "else " \
>>                         "echo Error: I cannot boot a DT kernel; \"
>>                 "fi; \"
>>         " else bootm; fi;\0"
>
>
> Yes, I think this is a good idea and would allow us to easily switch
> from dt to non-dt kernel during tests.

I like the idea but maybe we could have three states?

boot_fdt=yes

It'd imply force fdt and would behave as you said above;

boot_fdt=no

Would ignore the fdt completely

boot_fdt=auto

The current code, which try to load fdt and do not fail otherwise.

Comments?

--
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
Stefano Babic - Dec. 27, 2012, 9:44 a.m.
On 26/12/2012 19:23, Otavio Salvador wrote:
> On Wed, Dec 26, 2012 at 10:14 AM, Fabio Estevam <festevam@gmail.com> wrote:
>> On Wed, Dec 26, 2012 at 9:31 AM, Stefano Babic <sbabic@denx.de> wrote:
>>
>>> Only to remark that this is the correct behavior. The kernel with fdt is
>>> booted independently if this is wanted or not, but if the fdt file is
>>> loaded successfully from MMC or network. This can have some drawback
>>> effects if, for example, the fdt is simply stored on the TFTP server,
>>> but we want to boot a kernel without DT. For example, when someone wants
>>> to test both kernels or there are multiple instances of the same board
>>> type (mx53loco in this case) loading from the same TFTP server.
>>>
>>> In your patch, the behavior depends if the fdt is simply present on the
>>> media, but this does not always mean that the file must be loaded.
>>> Should be not better to set variable as flag to force the desired
>>> behavior and to be sure that the system does not boot in a different way
>>> as the user thinks ?
>>>
>>> In other words, something like :
>>>
>>>         "if boot_fdt; then " \
>>>                 "if dhcp ${ftd_addr} ${ftd_file}; then " \
>>>                         "bootm ${loadaddr} - ${ftd_addr}; " \
>>>                 "else " \
>>>                         "echo Error: I cannot boot a DT kernel; \"
>>>                 "fi; \"
>>>         " else bootm; fi;\0"
>>
>>
>> Yes, I think this is a good idea and would allow us to easily switch
>> from dt to non-dt kernel during tests.
> 
> I like the idea but maybe we could have three states?
> 
> boot_fdt=yes
> 
> It'd imply force fdt and would behave as you said above;
> 
> boot_fdt=no
> 
> Would ignore the fdt completely
> 
> boot_fdt=auto
> 
> The current code, which try to load fdt and do not fail otherwise.
> 
> Comments?

Reading again your environment, it seems to me that you strictly bind
the network configuration with fdt. But they are two distinct setup.
Let's say, if I desire to start a fdt kernel but I have a static ip
address. In this case, I should load with "tftp" instead using "dhcp".
So the necessity to have a three state is due to the fact you are
setting more as one state in one shot: fdt or not, and dhcp or not. IMHO
you need two variables to manage them independently. I think I did
something like this in another board, but I do not remember which one ;-(.

So you can have:
	boot_fdt
	ip_dyn

to discriminate what the script should do.

         "if boot_fdt; then " \
		"if ip_dyn;then" \
	           "if dhcp ${ftd_addr} ${ftd_file}; then " \
        	       "bootm ${loadaddr} - ${ftd_addr}; " \
                    "else " \
                       "echo Error: I cannot boot a DT kernel; \"
		    "fi;" \
		"else tftp ${ftd_addr} ${ftd_file}
			.....

Regards,
Stefano

Patch

diff --git a/include/configs/mx53loco.h b/include/configs/mx53loco.h
index 4cfdc0b..505957c 100644
--- a/include/configs/mx53loco.h
+++ b/include/configs/mx53loco.h
@@ -118,6 +118,8 @@ 
 #define CONFIG_EXTRA_ENV_SETTINGS \
 	"script=boot.scr\0" \
 	"uimage=uImage\0" \
+	"ftd_file=imx53-qsb.dtb\0" \
+	"ftd_addr=0x71000000\0" \
 	"mmcdev=0\0" \
 	"mmcpart=2\0" \
 	"mmcroot=/dev/mmcblk0p3 rw rootwait\0" \
@@ -129,13 +131,22 @@ 
 	"loaduimage=fatload mmc ${mmcdev}:${mmcpart} ${loadaddr} ${uimage}\0" \
 	"mmcboot=echo Booting from mmc ...; " \
 		"run mmcargs; " \
-		"bootm\0" \
+		"if fatload mmc ${mmcdev}:${mmcpart} ${ftd_addr} ${ftd_file}; then " \
+			"bootm ${loadaddr} - ${ftd_addr}; " \
+		"else " \
+			"bootm; " \
+		"fi;\0" \
 	"netargs=setenv bootargs console=ttymxc0,${baudrate} " \
 		"root=/dev/nfs " \
 		"ip=dhcp nfsroot=${serverip}:${nfsroot},v3,tcp\0" \
 	"netboot=echo Booting from net ...; " \
 		"run netargs; " \
-		"dhcp ${uimage}; bootm\0" \
+		"dhcp ${uimage}; " \
+		"if dhcp ${ftd_addr} ${ftd_file}; then " \
+			"bootm ${loadaddr} - ${ftd_addr}; " \
+		"else " \
+			"bootm; " \
+		"fi;\0"
 
 #define CONFIG_BOOTCOMMAND \
 	"mmc dev ${mmcdev}; if mmc rescan; then " \