| Message ID | 1299022621-2780-1-git-send-email-jkridner@beagleboard.org |
|---|---|
| State | Superseded |
| Delegated to: | Sandeep Paulraj |
| Headers | show |
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
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 >
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 >>
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
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 /*