[U-Boot] [PATCH 1/2] env: Include bootdelay in environment if negative

[U-Boot] [PATCH 1/2] env: Include bootdelay in environment if negative

[Alex Kiernan]

The test for (CONFIG_BOOTDELAY >= 0) has been in U-Boot since the
beginning, but the meaning of it has changed over time. Allow the
default to be set for any value, including -ve ones. This allows
(for example) CONFIG_ENV_IS_NOWHERE to have values for bootdelay in
its compiled in environment.

The only thing this changes is where the default for bootdelay can be
fetched from; before this change you get a compiled in default, after
you'll pull it from the default value in the environment, but both values
will be the same. Also if there's a value set in the environment then
that will take precedence (as before).

Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
---

 include/env_default.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/env_default.h b/include/env_default.h
index 54d8124793..bd600cfa44 100644
--- a/include/env_default.h
+++ b/include/env_default.h
@@ -40,7 +40,7 @@ const uchar default_environment[] = {
 #ifdef	CONFIG_NFSBOOTCOMMAND
 	"nfsboot="	CONFIG_NFSBOOTCOMMAND		"\0"
 #endif
-#if defined(CONFIG_BOOTDELAY) && (CONFIG_BOOTDELAY >= 0)
+#if defined(CONFIG_BOOTDELAY)
 	"bootdelay="	__stringify(CONFIG_BOOTDELAY)	"\0"
 #endif
 #if defined(CONFIG_BAUDRATE) && (CONFIG_BAUDRATE >= 0)
-- 
2.17.1

[U-Boot] [PATCH 2/2] bootmenu: Extend BOOTDELAY help text

[Alex Kiernan]

Extend BOOTDELAY help text to cover its additional usage within the
bootmenu command.

Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
---

 common/Kconfig | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/common/Kconfig b/common/Kconfig
index 4c7a1a9af8..81e88ea77c 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -182,6 +182,10 @@ config BOOTDELAY
 	  set to -1 to disable autoboot.
 	  set to -2 to autoboot with no delay and not check for abort
 
+	  If this value is >= 0 then it is also used for the default delay
+	  before starting the default entry in bootmenu. If it is < 0 then
+	  a default value of 10s is used.
+
 	  See doc/README.autoboot for details.
 
 config USE_BOOTARGS
-- 
2.17.1

[U-Boot] [U-Boot, 1/2] env: Include bootdelay in environment if negative

[Tom Rini]

On Thu, Jul 05, 2018 at 12:38:15PM +0000, Alex Kiernan wrote:

> The test for (CONFIG_BOOTDELAY >= 0) has been in U-Boot since the
> beginning, but the meaning of it has changed over time. Allow the
> default to be set for any value, including -ve ones. This allows
> (for example) CONFIG_ENV_IS_NOWHERE to have values for bootdelay in
> its compiled in environment.
> 
> The only thing this changes is where the default for bootdelay can be
> fetched from; before this change you get a compiled in default, after
> you'll pull it from the default value in the environment, but both values
> will be the same. Also if there's a value set in the environment then
> that will take precedence (as before).
> 
> Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180720/7d4d1a5a/attachment.sig>

[U-Boot] [U-Boot,2/2] bootmenu: Extend BOOTDELAY help text

[Tom Rini]

On Thu, Jul 05, 2018 at 12:38:16PM +0000, Alex Kiernan wrote:

> Extend BOOTDELAY help text to cover its additional usage within the
> bootmenu command.
> 
> Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180720/07bb1c5d/attachment.sig>

[U-Boot] [U-Boot, 1/2] env: Include bootdelay in environment if negative

[Alex Kiernan]

On Fri, Jul 20, 2018 at 11:34 PM Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Jul 05, 2018 at 12:38:15PM +0000, Alex Kiernan wrote:
>
> > The test for (CONFIG_BOOTDELAY >= 0) has been in U-Boot since the
> > beginning, but the meaning of it has changed over time. Allow the
> > default to be set for any value, including -ve ones. This allows
> > (for example) CONFIG_ENV_IS_NOWHERE to have values for bootdelay in
> > its compiled in environment.
> >
> > The only thing this changes is where the default for bootdelay can be
> > fetched from; before this change you get a compiled in default, after
> > you'll pull it from the default value in the environment, but both values
> > will be the same. Also if there's a value set in the environment then
> > that will take precedence (as before).
> >
> > Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
>
> Applied to u-boot/master, thanks!
>

I've just realised that if you had an existing hardcoded bootdelay= to
cover the case where it wasn't being inserted through env_defaults you
would now end up with two.

However checking there's 6 boards (9 configs) which possibly have this
problem, but none of them have a -ve value for CONFIG_BOOTDELAY. If I
revert this change and then grep out bootdelay from the generated
environments for those configs, 7 have duplicates before this change
(the last two are odroid-c2 and odroid-xu3 which are fine):

cl-som-am57x/uboot.env: bootdelay=2
cl-som-am57x/uboot.env: bootdelay=3
cm_t54/uboot.env: bootdelay=3
cm_t54/uboot.env: bootdelay=3
display5_factory/uboot.env: bootdelay=3
display5_factory/uboot.env: bootdelay=1
display5/uboot.env: bootdelay=2
display5/uboot.env: bootdelay=1
nas220/uboot.env: bootdelay=3
nas220/uboot.env: bootdelay=-1
odroid/uboot.env: bootdelay=2
odroid/uboot.env: bootdelay=0
vinco/uboot.env: bootdelay=3
vinco/uboot.env: bootdelay=0

I don't think there's anything here for me to fix up, but the
maintainers of those boards probably want to figure out which of the
bootdelay values they wanted.

I wonder if we want some QA process off the end of the build (or in
buildman?) which verifies the extracted environment for duplicate
entries.

--
Alex Kiernan

[U-Boot] [U-Boot, 1/2] env: Include bootdelay in environment if negative

[Tom Rini]

On Thu, Jul 26, 2018 at 09:31:08AM +0100, Alex Kiernan wrote:
> On Fri, Jul 20, 2018 at 11:34 PM Tom Rini <trini@konsulko.com> wrote:
> >
> > On Thu, Jul 05, 2018 at 12:38:15PM +0000, Alex Kiernan wrote:
> >
> > > The test for (CONFIG_BOOTDELAY >= 0) has been in U-Boot since the
> > > beginning, but the meaning of it has changed over time. Allow the
> > > default to be set for any value, including -ve ones. This allows
> > > (for example) CONFIG_ENV_IS_NOWHERE to have values for bootdelay in
> > > its compiled in environment.
> > >
> > > The only thing this changes is where the default for bootdelay can be
> > > fetched from; before this change you get a compiled in default, after
> > > you'll pull it from the default value in the environment, but both values
> > > will be the same. Also if there's a value set in the environment then
> > > that will take precedence (as before).
> > >
> > > Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
> >
> > Applied to u-boot/master, thanks!
> >
> 
> I've just realised that if you had an existing hardcoded bootdelay= to
> cover the case where it wasn't being inserted through env_defaults you
> would now end up with two.
> 
> However checking there's 6 boards (9 configs) which possibly have this
> problem, but none of them have a -ve value for CONFIG_BOOTDELAY. If I
> revert this change and then grep out bootdelay from the generated
> environments for those configs, 7 have duplicates before this change
> (the last two are odroid-c2 and odroid-xu3 which are fine):
> 
> cl-som-am57x/uboot.env: bootdelay=2
> cl-som-am57x/uboot.env: bootdelay=3
> cm_t54/uboot.env: bootdelay=3
> cm_t54/uboot.env: bootdelay=3
> display5_factory/uboot.env: bootdelay=3
> display5_factory/uboot.env: bootdelay=1
> display5/uboot.env: bootdelay=2
> display5/uboot.env: bootdelay=1
> nas220/uboot.env: bootdelay=3
> nas220/uboot.env: bootdelay=-1
> odroid/uboot.env: bootdelay=2
> odroid/uboot.env: bootdelay=0
> vinco/uboot.env: bootdelay=3
> vinco/uboot.env: bootdelay=0

Ah, good catch.  In the end, it's probably OK.

> I don't think there's anything here for me to fix up, but the
> maintainers of those boards probably want to figure out which of the
> bootdelay values they wanted.
> 
> I wonder if we want some QA process off the end of the build (or in
> buildman?) which verifies the extracted environment for duplicate
> entries.

That would be a great addition to buildman, yes.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180726/797ca8e4/attachment.sig>

[U-Boot] [U-Boot, 1/2] env: Include bootdelay in environment if negative

[Alex Kiernan]

On Fri, Jul 27, 2018 at 1:26 AM Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Jul 26, 2018 at 09:31:08AM +0100, Alex Kiernan wrote:
> > On Fri, Jul 20, 2018 at 11:34 PM Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Thu, Jul 05, 2018 at 12:38:15PM +0000, Alex Kiernan wrote:
> > >
> > > > The test for (CONFIG_BOOTDELAY >= 0) has been in U-Boot since the
> > > > beginning, but the meaning of it has changed over time. Allow the
> > > > default to be set for any value, including -ve ones. This allows
> > > > (for example) CONFIG_ENV_IS_NOWHERE to have values for bootdelay in
> > > > its compiled in environment.
> > > >
> > > > The only thing this changes is where the default for bootdelay can be
> > > > fetched from; before this change you get a compiled in default, after
> > > > you'll pull it from the default value in the environment, but both values
> > > > will be the same. Also if there's a value set in the environment then
> > > > that will take precedence (as before).
> > > >
> > > > Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
> > >
> > > Applied to u-boot/master, thanks!
> > >
> >
> > I've just realised that if you had an existing hardcoded bootdelay= to
> > cover the case where it wasn't being inserted through env_defaults you
> > would now end up with two.
> >
> > However checking there's 6 boards (9 configs) which possibly have this
> > problem, but none of them have a -ve value for CONFIG_BOOTDELAY. If I
> > revert this change and then grep out bootdelay from the generated
> > environments for those configs, 7 have duplicates before this change
> > (the last two are odroid-c2 and odroid-xu3 which are fine):
> >
> > cl-som-am57x/uboot.env: bootdelay=2
> > cl-som-am57x/uboot.env: bootdelay=3
> > cm_t54/uboot.env: bootdelay=3
> > cm_t54/uboot.env: bootdelay=3
> > display5_factory/uboot.env: bootdelay=3
> > display5_factory/uboot.env: bootdelay=1
> > display5/uboot.env: bootdelay=2
> > display5/uboot.env: bootdelay=1
> > nas220/uboot.env: bootdelay=3
> > nas220/uboot.env: bootdelay=-1
> > odroid/uboot.env: bootdelay=2
> > odroid/uboot.env: bootdelay=0
> > vinco/uboot.env: bootdelay=3
> > vinco/uboot.env: bootdelay=0
>
> Ah, good catch.  In the end, it's probably OK.
>

I guess last match will win, so you end up with what you've put in
your board file, which is likely what's intended.

> > I don't think there's anything here for me to fix up, but the
> > maintainers of those boards probably want to figure out which of the
> > bootdelay values they wanted.
> >
> > I wonder if we want some QA process off the end of the build (or in
> > buildman?) which verifies the extracted environment for duplicate
> > entries.
>
> That would be a great addition to buildman, yes.
>

Have we got any warning machinery in buildman? I can only find stuff
which is reporting warnings from the tools, not from itself.

-- 
Alex Kiernan