[U-Boot] [PATCH] CONFIG_BOOTDELAY default should not affect runtime

[Joe Hershberger]

Hi Tom,

On Mon, Feb 18, 2013 at 11:20 AM, Tom Rini <trini at ti.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 02/09/2013 11:21 AM, Otavio Salvador wrote:
>> Hello Wolfgang,
>>
>> On Sat, Feb 9, 2013 at 4:54 AM, Wolfgang Denk <wd at denx.de> wrote:
>>> Dear Joe Hershberger,
>>>
>>> In message
>>> <1360355280-1197-1-git-send-email-joe.hershberger at ni.com> you
>>> wrote:
>>>> Because the code that handles bootdelay is compiled in
>>>> conditionally based on the default value, you are restricted in
>>>> the default, regardless of what you want the runtime options to
>>>> be.
>>>>
>>>> Change the source to always check if any default is given so
>>>> that other values can be selected and used at runtime.
>>>>
>>>> Signed-off-by: Joe Hershberger <joe.hershberger at ni.com> ---
>>>> common/main.c | 14 ++++++-------- 1 file changed, 6
>>>> insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/common/main.c b/common/main.c index
>>>> e2d2e09..0973c59 100644 --- a/common/main.c +++
>>>> b/common/main.c @@ -95,7 +95,7 @@ extern void mdm_init(void);
>>>> /* defined in board.c */ * Watch for 'delay' seconds for
>>>> autoboot stop or autoboot delay string. * returns: 0 -  no key
>>>> string, allow autoboot 1 - got key string, abort */ -#if
>>>> defined(CONFIG_BOOTDELAY) && (CONFIG_BOOTDELAY >= 0) +#if
>>>> defined(CONFIG_BOOTDELAY)
>>>
>>> Careful!! This is probably changing behaviour of a number of
>>> boards significantly.
>>>
>>> we have to check if we really want this, and if yes, we have to
>>> announce it and provide a grace period (eventually using
>>> doc/feature-removal-schedule.txt ?)
>>
>> It seems the CONFIG_BOOTDELAY as < 0 is not very common:
>>
>> ~/hacking/u-boot% git grep CONFIG_BOOTDELAY | egrep 'BOOTDELAY\s*
>> \-[0-9]' include/configs/RPXsuper.h:#define CONFIG_BOOTDELAY
>> -1 include/configs/ep8260.h:#define CONFIG_BOOTDELAY        -1
>> include/configs/espt.h:#define CONFIG_BOOTDELAY        -1
>> include/configs/scb9328.h:#define CONFIG_BOOTDELAY   -1
>> include/configs/sh7763rdp.h:#define CONFIG_BOOTDELAY        -1
>
> I count 49 boards with git grep -E
> 'CONFIG_BOOTDELAY[[:blank:]]+-[0-9]' so it's not _that_ uncommon.
>
>> So maybe those could have CONFIG_BOOTDELAY undefined keeping them
>> working as before?
>
> The problem is that as I read the README, we document CONFIG_BOOTDELAY
> as having a valid value range of from -2 to sane positive value.  So
> yes, if we want to change this we need to (a) change the README too
> and (b) give some sort of heads-up.
>
> Off the top of my head, we could change to:
> CONFIG_AUTOBOOT_DISABLED and CONFIG_FORCE_AUTOBOOT_NO_DELAY and update
> doc/README.autoboot as well and add some sanity #warning checks for a
> release or two in some common generated config related file or
> something like that.

The change has no effect on the behavior, it simply changes what is
compiled in.  If the default value is not changed, then the behavior
is unchanged.  The only impact this may have on existing boards is to
make the code size slightly larger.  If that is an issue for some
reason, then CONFIG_BOOTDELAY can be undefined.

If we want to add other config options, that's fine, but that's more
involved that I was hoping for.

Thanks,
-Joe