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

[Alexander Holler]

Hello,

Am 02.03.2011 16:44, schrieb Jason Kridner:
> On Tue, Mar 1, 2011 at 9:54 PM, Alexander Holler<holler at ahsoftware.de>  wrote:
>> Hello Jason,
>>
>> On 02.03.2011 00:37, Jason Kridner wrote:
>>
>>> From: Alexander Holler<holler at 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 at 60
>>> run loaduimage
>>> run mmcboot
>>> -----------------------------------
>>> you could create a file named uEnv.txt and use that instead of boot.scr:
>>> -----------------------------------
>>> dvimode=1024x768-16 at 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 at 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 at 60".


>>> Signed-off-by: Jason Kridner<jkridner at beagleboard.org>
>>> Cc: Alexander Holler<holler at 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
>>