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

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)

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

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!

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!

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

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.

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.
participants (2)
-
Alex Kiernan
-
Tom Rini