Patchwork [U-Boot,v2] omap3_beagle: enable the use of a plain text file named uEnv.txt instead of boot.scr

login
register
mail settings
Submitter Jason Kridner
Date March 1, 2011, 11:37 p.m.
Message ID <1299022621-2780-1-git-send-email-jkridner@beagleboard.org>
Download mbox | patch
Permalink /patch/85012/
State Superseded
Delegated to: Sandeep Paulraj
Headers show

Comments

Jason Kridner - March 1, 2011, 11:37 p.m.
From: Alexander Holler <holler@ahsoftware.de>

Using the new env import command it is possible to use plain text files instead
of script-images. Plain text files are much easier to handle.

E.g. If your boot.scr contains the following:
-----------------------------------
setenv dvimode 1024x768-16@60
run loaduimage
run mmcboot
-----------------------------------
you could create a file named uEnv.txt and use that instead of boot.scr:
-----------------------------------
dvimode=1024x768-16@60
uenvcmd=run loaduimage; run mmcboot
-----------------------------------
The variable uenvcmd (if existent) will be executed (using run) after uEnv.txt
was loaded. If uenvcmd doesn't exist the default boot sequence will be started,
therefore you could just use
-----------------------------------
dvimode=1024x768-16@60
-----------------------------------
as uEnv.txt because loaduimage and mmcboot is part of the default boot sequence.

For backwards compatibility the use of boot.scr is still supported.
---
Changes for v2:
  - Eliminated else redundant clause that would be ignored if boot
    succeeds.

Signed-off-by: Jason Kridner <jkridner@beagleboard.org>
Cc: Alexander Holler <holler@ahsoftware.de>
---
 include/configs/omap3_beagle.h |   26 ++++++++++++++++++--------
 1 files changed, 18 insertions(+), 8 deletions(-)
Alexander Holler - March 2, 2011, 2:54 a.m.
Hello Jason,

On 02.03.2011 00:37, Jason Kridner wrote:

> From: Alexander Holler<holler@ahsoftware.de>
>
> Using the new env import command it is possible to use plain text files instead
> of script-images. Plain text files are much easier to handle.
>
> E.g. If your boot.scr contains the following:
> -----------------------------------
> setenv dvimode 1024x768-16@60
> run loaduimage
> run mmcboot
> -----------------------------------
> you could create a file named uEnv.txt and use that instead of boot.scr:
> -----------------------------------
> dvimode=1024x768-16@60
> uenvcmd=run loaduimage; run mmcboot
> -----------------------------------
> The variable uenvcmd (if existent) will be executed (using run) after uEnv.txt
> was loaded. If uenvcmd doesn't exist the default boot sequence will be started,
> therefore you could just use
> -----------------------------------
> dvimode=1024x768-16@60
> -----------------------------------
> as uEnv.txt because loaduimage and mmcboot is part of the default boot sequence.
>
> For backwards compatibility the use of boot.scr is still supported.
> ---
> Changes for v2:
>    - Eliminated else redundant clause that would be ignored if boot
>      succeeds.


If I interpret your change correctly, your v2 would use uEnv.txt and 
boot.scr if both are existent. I think this would only lead to confusion.
My target was to get rid of boot.scr and to therefor boot.scr would be 
ignored if uEnv.txt exists. I don't see any reason why boot.scr should 
be still used when uEnv.txt exists.


>
> Signed-off-by: Jason Kridner<jkridner@beagleboard.org>
> Cc: Alexander Holler<holler@ahsoftware.de>
> ---
>   include/configs/omap3_beagle.h |   26 ++++++++++++++++++--------
>   1 files changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h
> index f151e98..b7f5480 100644
> --- a/include/configs/omap3_beagle.h
> +++ b/include/configs/omap3_beagle.h
> @@ -229,6 +229,9 @@
>   	"loadbootscript=fatload mmc ${mmcdev} ${loadaddr} boot.scr\0" \
>   	"bootscript=echo Running bootscript from mmc ...; " \
>   		"source ${loadaddr}\0" \
> +	"loadbootenv=fatload mmc ${mmcdev} ${loadaddr} uEnv.txt\0" \
> +	"importbootenv=echo Importing environment from mmc ...; " \
> +		"env import -t $loadaddr $filesize\0" \
>   	"loaduimage=fatload mmc ${mmcdev} ${loadaddr} uImage\0" \
>   	"mmcboot=echo Booting from mmc ...; " \
>   		"run mmcargs; " \
> @@ -240,15 +243,22 @@
>
>   #define CONFIG_BOOTCOMMAND \
>   	"if mmc rescan ${mmcdev}; then " \
> +		"echo SD/MMC found on device ${mmcdev};" \
> +		"if run loadbootenv; then " \
> +			"run importbootenv;" \
> +		"fi;" \
> +		"if test -n $uenvcmd; then " \
> +			"echo Running uenvcmd ...;" \
> +			"run uenvcmd;" \
> +		"fi;" \
>   		"if run loadbootscript; then " \
> -			"run bootscript; " \
> -		"else " \
> -			"if run loaduimage; then " \
> -				"run mmcboot; " \
> -			"else run nandboot; " \
> -			"fi; " \
> -		"fi; " \
> -	"else run nandboot; fi"
> +			"run bootscript;" \
> +		"fi;" \
> +		"if run loaduimage; then " \
> +			"run mmcboot;" \
> +		"fi;" \
> +	"fi;" \
> +	"run nandboot;" \
>
>   #define CONFIG_AUTO_COMPLETE		1
>   /*

Regards,

Alexander
Jason Kridner - March 2, 2011, 3:44 p.m.
On Tue, Mar 1, 2011 at 9:54 PM, Alexander Holler <holler@ahsoftware.de> wrote:
> Hello Jason,
>
> On 02.03.2011 00:37, Jason Kridner wrote:
>
>> From: Alexander Holler<holler@ahsoftware.de>
>>
>> Using the new env import command it is possible to use plain text files
>> instead
>> of script-images. Plain text files are much easier to handle.
>>
>> E.g. If your boot.scr contains the following:
>> -----------------------------------
>> setenv dvimode 1024x768-16@60
>> run loaduimage
>> run mmcboot
>> -----------------------------------
>> you could create a file named uEnv.txt and use that instead of boot.scr:
>> -----------------------------------
>> dvimode=1024x768-16@60
>> uenvcmd=run loaduimage; run mmcboot
>> -----------------------------------
>> The variable uenvcmd (if existent) will be executed (using run) after
>> uEnv.txt
>> was loaded. If uenvcmd doesn't exist the default boot sequence will be
>> started,
>> therefore you could just use
>> -----------------------------------
>> dvimode=1024x768-16@60
>> -----------------------------------
>> as uEnv.txt because loaduimage and mmcboot is part of the default boot
>> sequence.
>>
>> For backwards compatibility the use of boot.scr is still supported.
>> ---
>> Changes for v2:
>>   - Eliminated else redundant clause that would be ignored if boot
>>     succeeds.
>
>
> If I interpret your change correctly, your v2 would use uEnv.txt and
> boot.scr if both are existent. I think this would only lead to confusion.
> My target was to get rid of boot.scr and to therefor boot.scr would be
> ignored if uEnv.txt exists. I don't see any reason why boot.scr should be
> still used when uEnv.txt exists.

if uenvcmd results in a successful boot, then boot.scr would never get executed.

Again, my concern is that this default logic gets overly complex.  A
policy of applying linear attempt-then-fall-through will make patches
to this logic apply cleanly as new boot sources are added,
specifically if we ever get the USER button support upstream.

I understand the goal of only using uEnv.txt and not using ucmdenv
such that the u-boot default variables can be used to perform the
boot.  I think this goal can easily be achieved by deleting boot.scr.
If boot.scr exists, I don't follow that it would be any less confusing
to the user if it were to *not* be executed following uEnv.txt.

>
>
>>
>> Signed-off-by: Jason Kridner<jkridner@beagleboard.org>
>> Cc: Alexander Holler<holler@ahsoftware.de>
>> ---
>>  include/configs/omap3_beagle.h |   26 ++++++++++++++++++--------
>>  1 files changed, 18 insertions(+), 8 deletions(-)
>>
>> diff --git a/include/configs/omap3_beagle.h
>> b/include/configs/omap3_beagle.h
>> index f151e98..b7f5480 100644
>> --- a/include/configs/omap3_beagle.h
>> +++ b/include/configs/omap3_beagle.h
>> @@ -229,6 +229,9 @@
>>        "loadbootscript=fatload mmc ${mmcdev} ${loadaddr} boot.scr\0" \
>>        "bootscript=echo Running bootscript from mmc ...; " \
>>                "source ${loadaddr}\0" \
>> +       "loadbootenv=fatload mmc ${mmcdev} ${loadaddr} uEnv.txt\0" \
>> +       "importbootenv=echo Importing environment from mmc ...; " \
>> +               "env import -t $loadaddr $filesize\0" \
>>        "loaduimage=fatload mmc ${mmcdev} ${loadaddr} uImage\0" \
>>        "mmcboot=echo Booting from mmc ...; " \
>>                "run mmcargs; " \
>> @@ -240,15 +243,22 @@
>>
>>  #define CONFIG_BOOTCOMMAND \
>>        "if mmc rescan ${mmcdev}; then " \
>> +               "echo SD/MMC found on device ${mmcdev};" \
>> +               "if run loadbootenv; then " \
>> +                       "run importbootenv;" \
>> +               "fi;" \
>> +               "if test -n $uenvcmd; then " \
>> +                       "echo Running uenvcmd ...;" \
>> +                       "run uenvcmd;" \
>> +               "fi;" \
>>                "if run loadbootscript; then " \
>> -                       "run bootscript; " \
>> -               "else " \
>> -                       "if run loaduimage; then " \
>> -                               "run mmcboot; " \
>> -                       "else run nandboot; " \
>> -                       "fi; " \
>> -               "fi; " \
>> -       "else run nandboot; fi"
>> +                       "run bootscript;" \
>> +               "fi;" \
>> +               "if run loaduimage; then " \
>> +                       "run mmcboot;" \
>> +               "fi;" \
>> +       "fi;" \
>> +       "run nandboot;" \
>>
>>  #define CONFIG_AUTO_COMPLETE          1
>>  /*
>
> Regards,
>
> Alexander
>
Alexander Holler - March 2, 2011, 5:47 p.m.
Hello,

Am 02.03.2011 16:44, schrieb Jason Kridner:
> On Tue, Mar 1, 2011 at 9:54 PM, Alexander Holler<holler@ahsoftware.de>  wrote:
>> Hello Jason,
>>
>> On 02.03.2011 00:37, Jason Kridner wrote:
>>
>>> From: Alexander Holler<holler@ahsoftware.de>
>>>
>>> Using the new env import command it is possible to use plain text files
>>> instead
>>> of script-images. Plain text files are much easier to handle.
>>>
>>> E.g. If your boot.scr contains the following:
>>> -----------------------------------
>>> setenv dvimode 1024x768-16@60
>>> run loaduimage
>>> run mmcboot
>>> -----------------------------------
>>> you could create a file named uEnv.txt and use that instead of boot.scr:
>>> -----------------------------------
>>> dvimode=1024x768-16@60
>>> uenvcmd=run loaduimage; run mmcboot
>>> -----------------------------------
>>> The variable uenvcmd (if existent) will be executed (using run) after
>>> uEnv.txt
>>> was loaded. If uenvcmd doesn't exist the default boot sequence will be
>>> started,
>>> therefore you could just use
>>> -----------------------------------
>>> dvimode=1024x768-16@60
>>> -----------------------------------
>>> as uEnv.txt because loaduimage and mmcboot is part of the default boot
>>> sequence.
>>>
>>> For backwards compatibility the use of boot.scr is still supported.
>>> ---
>>> Changes for v2:
>>>    - Eliminated else redundant clause that would be ignored if boot
>>>      succeeds.
>>
>>
>> If I interpret your change correctly, your v2 would use uEnv.txt and
>> boot.scr if both are existent. I think this would only lead to confusion.
>> My target was to get rid of boot.scr and to therefor boot.scr would be
>> ignored if uEnv.txt exists. I don't see any reason why boot.scr should be
>> still used when uEnv.txt exists.
>
> if uenvcmd results in a successful boot, then boot.scr would never get executed.

But that requires that uenvcmd is set/used which makes uEnv.txt 
unnecessarily complex and will not be possible to define/change e.g. 
only the (u-boot) environment variable dvimode.

> Again, my concern is that this default logic gets overly complex.  A
> policy of applying linear attempt-then-fall-through will make patches
> to this logic apply cleanly as new boot sources are added,
> specifically if we ever get the USER button support upstream.

We could just delete the logic for boot.scr. Another approach would be 
to print a warning like "The usage of boot.scr is deprecated and will be 
removed with the next release!" if boot.scr will be used and delete the 
stuff for boot.scr with the next release.

> I understand the goal of only using uEnv.txt and not using ucmdenv
> such that the u-boot default variables can be used to perform the
> boot.  I think this goal can easily be achieved by deleting boot.scr.
> If boot.scr exists, I don't follow that it would be any less confusing
> to the user if it were to *not* be executed following uEnv.txt.

Again, I think such an approach will only confuse people because stuff 
from two files might be used. And what is used will depend on what 
u-boot is used. There might be people with an u-boot in NAND, for which 
a boot.scr on an SD-card still is required. But if you want to serve 
people with an new u-boot too, you'll need an uEnv.txt too. And with 
your logic that has to use an uencmd, which makes uEnv.txt more 
complicate than needed.

And most people never see the bootcmd, but they see uEnv.txt and/or 
boot.scr because they have to define the resolution for their monitor there.

So I still would prefer to leave bootcmd currently a bit more 
complicated, with the advantage that only a simple uEnv.txt is possible 
which just contains e.g. "dvimode=1024x768-16@60".


>>> Signed-off-by: Jason Kridner<jkridner@beagleboard.org>
>>> Cc: Alexander Holler<holler@ahsoftware.de>
>>> ---
>>>   include/configs/omap3_beagle.h |   26 ++++++++++++++++++--------
>>>   1 files changed, 18 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/include/configs/omap3_beagle.h
>>> b/include/configs/omap3_beagle.h
>>> index f151e98..b7f5480 100644
>>> --- a/include/configs/omap3_beagle.h
>>> +++ b/include/configs/omap3_beagle.h
>>> @@ -229,6 +229,9 @@
>>>         "loadbootscript=fatload mmc ${mmcdev} ${loadaddr} boot.scr\0" \
>>>         "bootscript=echo Running bootscript from mmc ...; " \
>>>                 "source ${loadaddr}\0" \
>>> +       "loadbootenv=fatload mmc ${mmcdev} ${loadaddr} uEnv.txt\0" \
>>> +       "importbootenv=echo Importing environment from mmc ...; " \
>>> +               "env import -t $loadaddr $filesize\0" \
>>>         "loaduimage=fatload mmc ${mmcdev} ${loadaddr} uImage\0" \
>>>         "mmcboot=echo Booting from mmc ...; " \
>>>                 "run mmcargs; " \
>>> @@ -240,15 +243,22 @@
>>>
>>>   #define CONFIG_BOOTCOMMAND \
>>>         "if mmc rescan ${mmcdev}; then " \
>>> +               "echo SD/MMC found on device ${mmcdev};" \
>>> +               "if run loadbootenv; then " \
>>> +                       "run importbootenv;" \
>>> +               "fi;" \
>>> +               "if test -n $uenvcmd; then " \
>>> +                       "echo Running uenvcmd ...;" \
>>> +                       "run uenvcmd;" \
>>> +               "fi;" \
>>>                 "if run loadbootscript; then " \
>>> -                       "run bootscript; " \
>>> -               "else " \
>>> -                       "if run loaduimage; then " \
>>> -                               "run mmcboot; " \
>>> -                       "else run nandboot; " \
>>> -                       "fi; " \
>>> -               "fi; " \
>>> -       "else run nandboot; fi"
>>> +                       "run bootscript;" \
>>> +               "fi;" \
>>> +               "if run loaduimage; then " \
>>> +                       "run mmcboot;" \
>>> +               "fi;" \
>>> +       "fi;" \
>>> +       "run nandboot;" \
>>>
>>>   #define CONFIG_AUTO_COMPLETE          1
>>>   /*
>>
>> Regards,
>>
>> Alexander
>>
Alexander Holler - March 2, 2011, 7:55 p.m.
Hello,

just a last word. I don't have to decide which patch is used and I don't 
have any problem if v2 is used. I'm able to modify bootcmd like I wish 
and can live with any version.
I've justed wanted to make clear why I haven't used a simpler approach.

Anyway, if v2 will be used, please just remove my name as author, I 
don't want to be responsible for the possible arising confusion.
Everyone can still use as much from that patch as he wishes, please just 
don't use my name as author for a modified patch with a changed logic. ;)

Regards,

Alexander

Patch

diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h
index f151e98..b7f5480 100644
--- a/include/configs/omap3_beagle.h
+++ b/include/configs/omap3_beagle.h
@@ -229,6 +229,9 @@ 
 	"loadbootscript=fatload mmc ${mmcdev} ${loadaddr} boot.scr\0" \
 	"bootscript=echo Running bootscript from mmc ...; " \
 		"source ${loadaddr}\0" \
+	"loadbootenv=fatload mmc ${mmcdev} ${loadaddr} uEnv.txt\0" \
+	"importbootenv=echo Importing environment from mmc ...; " \
+		"env import -t $loadaddr $filesize\0" \
 	"loaduimage=fatload mmc ${mmcdev} ${loadaddr} uImage\0" \
 	"mmcboot=echo Booting from mmc ...; " \
 		"run mmcargs; " \
@@ -240,15 +243,22 @@ 
 
 #define CONFIG_BOOTCOMMAND \
 	"if mmc rescan ${mmcdev}; then " \
+		"echo SD/MMC found on device ${mmcdev};" \
+		"if run loadbootenv; then " \
+			"run importbootenv;" \
+		"fi;" \
+		"if test -n $uenvcmd; then " \
+			"echo Running uenvcmd ...;" \
+			"run uenvcmd;" \
+		"fi;" \
 		"if run loadbootscript; then " \
-			"run bootscript; " \
-		"else " \
-			"if run loaduimage; then " \
-				"run mmcboot; " \
-			"else run nandboot; " \
-			"fi; " \
-		"fi; " \
-	"else run nandboot; fi"
+			"run bootscript;" \
+		"fi;" \
+		"if run loaduimage; then " \
+			"run mmcboot;" \
+		"fi;" \
+	"fi;" \
+	"run nandboot;" \
 
 #define CONFIG_AUTO_COMPLETE		1
 /*