Patchwork [U-Boot,RFC,v2,02/15] at91: Correct CONFIG_AUTOBOOT_PROMPT definition for pm9263

login
register
mail settings
Submitter Simon Glass
Date Feb. 24, 2013, 5:26 p.m.
Message ID <1361726773-18639-3-git-send-email-sjg@chromium.org>
Download mbox | patch
Permalink /patch/222784/
State Superseded, archived
Headers show

Comments

Simon Glass - Feb. 24, 2013, 5:26 p.m.
This is not currently used, since autoboot is not enabled for this
board, but the string is missing a parameter. Add it.


Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None

 include/configs/pm9263.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Joe Hershberger - Feb. 24, 2013, 7:53 p.m.
Hi Simon,

On Sun, Feb 24, 2013 at 11:26 AM, Simon Glass <sjg@chromium.org> wrote:
> This is not currently used, since autoboot is not enabled for this
> board, but the string is missing a parameter. Add it.
>

Why not enable autoboot for this board so that this setting gets testing?

>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> Changes in v2: None
>
>  include/configs/pm9263.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/configs/pm9263.h b/include/configs/pm9263.h
> index b60a9ad..6f6ddfa 100644
> --- a/include/configs/pm9263.h
> +++ b/include/configs/pm9263.h
> @@ -355,7 +355,7 @@
>
>  #define CONFIG_BOOTCOMMAND             "run flashboot"
>  #define CONFIG_ROOTPATH                        "/ronetix/rootfs"
> -#define CONFIG_AUTOBOOT_PROMPT         "autoboot in %d seconds\n"
> +#define CONFIG_AUTOBOOT_PROMPT         "autoboot in %d seconds\n", bootdelay
>
>  #define CONFIG_CON_ROT                 "fbcon=rotate:3 "
>  #define CONFIG_BOOTARGS                        "root=/dev/mtdblock4 rootfstype=jffs2 "\
> --
> 1.8.1.3
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
Simon Glass - Feb. 26, 2013, 5:28 a.m.
Hi Joe,

On Sun, Feb 24, 2013 at 11:53 AM, Joe Hershberger
<joe.hershberger@gmail.com> wrote:
> Hi Simon,
>
> On Sun, Feb 24, 2013 at 11:26 AM, Simon Glass <sjg@chromium.org> wrote:
>> This is not currently used, since autoboot is not enabled for this
>> board, but the string is missing a parameter. Add it.
>>
>
> Why not enable autoboot for this board so that this setting gets testing?

Actually with autoconf this setting is tested, which is how I found
the problem. The old code used #ifdef and so the problem was masked.

Or do you think I should enable because that was probably the board
vendor's indent?

Regards,
Simon

>
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>> Changes in v2: None
>>
>>  include/configs/pm9263.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/configs/pm9263.h b/include/configs/pm9263.h
>> index b60a9ad..6f6ddfa 100644
>> --- a/include/configs/pm9263.h
>> +++ b/include/configs/pm9263.h
>> @@ -355,7 +355,7 @@
>>
>>  #define CONFIG_BOOTCOMMAND             "run flashboot"
>>  #define CONFIG_ROOTPATH                        "/ronetix/rootfs"
>> -#define CONFIG_AUTOBOOT_PROMPT         "autoboot in %d seconds\n"
>> +#define CONFIG_AUTOBOOT_PROMPT         "autoboot in %d seconds\n", bootdelay
>>
>>  #define CONFIG_CON_ROT                 "fbcon=rotate:3 "
>>  #define CONFIG_BOOTARGS                        "root=/dev/mtdblock4 rootfstype=jffs2 "\
>> --
>> 1.8.1.3
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot@lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
Joe Hershberger - Feb. 27, 2013, 8:40 a.m.
Hi Simon,

On Mon, Feb 25, 2013 at 11:28 PM, Simon Glass <sjg@chromium.org> wrote:
> Hi Joe,
>
> On Sun, Feb 24, 2013 at 11:53 AM, Joe Hershberger
> <joe.hershberger@gmail.com> wrote:
>> Hi Simon,
>>
>> On Sun, Feb 24, 2013 at 11:26 AM, Simon Glass <sjg@chromium.org> wrote:
>>> This is not currently used, since autoboot is not enabled for this
>>> board, but the string is missing a parameter. Add it.
>>>
>>
>> Why not enable autoboot for this board so that this setting gets testing?
>
> Actually with autoconf this setting is tested, which is how I found
> the problem. The old code used #ifdef and so the problem was masked.
>
> Or do you think I should enable because that was probably the board
> vendor's indent?

What I was thinking is that if the board sets the configuration for
this, then they surely want this feature to work.  It should be
enabled by default for this board (unless the maintainer explicitly
declares no, in which case the configuration should be removed
completely).
Simon Glass - Feb. 28, 2013, 2:19 p.m.
+Ilko Iliev <iliev@ronetix.at> again

Hi Joe,

On Wed, Feb 27, 2013 at 12:40 AM, Joe Hershberger
<joe.hershberger@gmail.com> wrote:
> Hi Simon,
>
> On Mon, Feb 25, 2013 at 11:28 PM, Simon Glass <sjg@chromium.org> wrote:
>> Hi Joe,
>>
>> On Sun, Feb 24, 2013 at 11:53 AM, Joe Hershberger
>> <joe.hershberger@gmail.com> wrote:
>>> Hi Simon,
>>>
>>> On Sun, Feb 24, 2013 at 11:26 AM, Simon Glass <sjg@chromium.org> wrote:
>>>> This is not currently used, since autoboot is not enabled for this
>>>> board, but the string is missing a parameter. Add it.
>>>>
>>>
>>> Why not enable autoboot for this board so that this setting gets testing?
>>
>> Actually with autoconf this setting is tested, which is how I found
>> the problem. The old code used #ifdef and so the problem was masked.
>>
>> Or do you think I should enable because that was probably the board
>> vendor's indent?
>
> What I was thinking is that if the board sets the configuration for
> this, then they surely want this feature to work.  It should be
> enabled by default for this board (unless the maintainer explicitly
> declares no, in which case the configuration should be removed
> completely).

I don't disagree with you, but I think that is a matter for the board
maintainer. I found a minor problem in the config file which I fixed.
But I don't really think I want to enable CONFIG_AUTOBOOT_KEYED,
select some values for CONFIG_AUTOBOOT_DELAY_STR2, etc. as well. For
my purposes I just need the board to continue to boot.

Somehow we dropped Ilko Iliev <iliev@ronetix.at> off the Cc here, so
have re-added.

Regards,
Simon

Patch

diff --git a/include/configs/pm9263.h b/include/configs/pm9263.h
index b60a9ad..6f6ddfa 100644
--- a/include/configs/pm9263.h
+++ b/include/configs/pm9263.h
@@ -355,7 +355,7 @@ 
 
 #define CONFIG_BOOTCOMMAND		"run flashboot"
 #define CONFIG_ROOTPATH			"/ronetix/rootfs"
-#define CONFIG_AUTOBOOT_PROMPT		"autoboot in %d seconds\n"
+#define CONFIG_AUTOBOOT_PROMPT		"autoboot in %d seconds\n", bootdelay
 
 #define CONFIG_CON_ROT			"fbcon=rotate:3 "
 #define CONFIG_BOOTARGS			"root=/dev/mtdblock4 rootfstype=jffs2 "\