Patchwork [U-Boot] board/bsc913x: Add config flag for bootdelay

login
register
mail settings
Submitter harninder rai
Date June 17, 2013, 11:48 a.m.
Message ID <1371469712-12678-1-git-send-email-harninder.rai@freescale.com>
Download mbox | patch
Permalink /patch/251822/
State Rejected
Delegated to: Andy Fleming
Headers show

Comments

harninder rai - June 17, 2013, 11:48 a.m.
Keep the value of CONFIG_BOOTDELAY as -1 to disable autoboot

Signed-off-by: Harninder Rai <harninder.rai@freescale.com>
---
 include/configs/BSC9131RDB.h |    1 +
 include/configs/BSC9132QDS.h |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)
Albert ARIBAUD - June 23, 2013, 7:07 a.m.
Hi Harninder,

On Mon, 17 Jun 2013 17:18:32 +0530, Harninder Rai
<harninder.rai@freescale.com> wrote:

> Keep the value of CONFIG_BOOTDELAY as -1 to disable autoboot
> 
> Signed-off-by: Harninder Rai <harninder.rai@freescale.com>
> ---
>  include/configs/BSC9131RDB.h |    1 +
>  include/configs/BSC9132QDS.h |    1 +
>  2 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/include/configs/BSC9131RDB.h b/include/configs/BSC9131RDB.h
> index fd076e0..c56b119 100644
> --- a/include/configs/BSC9131RDB.h
> +++ b/include/configs/BSC9131RDB.h
> @@ -393,6 +393,7 @@ extern unsigned long get_sdram_size(void);
>  #define CONFIG_UBOOTPATH	"u-boot.bin" /* U-Boot image on TFTP server */
>  
>  #define CONFIG_BAUDRATE		115200
> +#define CONFIG_BOOTDELAY	-1 /* Disable autoboot */
>  
>  #define	CONFIG_EXTRA_ENV_SETTINGS				\
>  	"netdev=eth0\0"						\
> diff --git a/include/configs/BSC9132QDS.h b/include/configs/BSC9132QDS.h
> index 9d15d0e..d54900a 100644
> --- a/include/configs/BSC9132QDS.h
> +++ b/include/configs/BSC9132QDS.h
> @@ -607,6 +607,7 @@ combinations. this should be removed later
>  #define CONFIG_UBOOTPATH	"u-boot.bin"
>  
>  #define CONFIG_BAUDRATE		115200
> +#define CONFIG_BOOTDELAY	-1 /* Disable autoboot */
>  
>  #ifdef CONFIG_SDCARD
>  #define CONFIG_DEF_HWCONFIG	"hwconfig=usb1:dr_mode=host,phy_type=ulpi\0"

Plase fix commit message: you're not simply adding a boot delay flag,
you're in fact disabling autoboot, which is quite different.

Also, when touching on a board's code, you should Cc: the board's
maintainer(s). Please do so for V2 of your patch -- and if you get a
non-delivery status (e.g. a maintainer's mail address does not exist any
more), please follow up on your patch submission to let us know.

Amicalement,
Rai Harninder-B01044 - June 24, 2013, 9:28 a.m.
Hello Albert,


Plase fix commit message: you're not simply adding a boot delay flag, you're in fact disabling autoboot, which is quite different.
[Rai Harninder-B01044] "Disable autoboot through bootdelay config flag". Is this ok?

Also, when touching on a board's code, you should Cc: the board's maintainer(s). Please do so for V2 of your patch -- and if you get a non-delivery status (e.g. a maintainer's mail address does not exist any more), please follow up on your patch submission to let us know.
[Rai Harninder-B01044] The board's maintainer is Andy Fleming who is already included in cc


Harry++
Albert ARIBAUD - June 24, 2013, 5:11 p.m.
Hi Rai,

On Mon, 24 Jun 2013 09:28:26 +0000, Rai Harninder-B01044
<B01044@freescale.com> wrote:

> Hello Albert,
> 
> 
> Plase fix commit message: you're not simply adding a boot delay flag, you're in fact disabling autoboot, which is quite different.
> [Rai Harninder-B01044] "Disable autoboot through bootdelay config flag". Is this ok?

(weird way of quoting messages; makes it really hard to see who said
what. Can't your mail client use the standard ">" line prefix?)

As for the commit summary, it is fine by me.

> Also, when touching on a board's code, you should Cc: the board's maintainer(s). Please do so for V2 of your patch -- and if you get a non-delivery status (e.g. a maintainer's mail address does not exist any more), please follow up on your patch submission to let us know.
> [Rai Harninder-B01044] The board's maintainer is Andy Fleming who is already included in cc

Are you not mixing up BSC9131RDB and BSC9132QDS maintainership with
MPC85xx tree custodianship?

Andy is the MPC85xx custodian, but has no entry in the MAINTAINERS
file, which OTOH lists Poonam Aggrwal <poonam.aggrwal@freescale.com> as
the BSC9131RDB maintainer, and Naveen Burmi <NaveenBurmi@freescale.com>
as the BSC9132QDS one.

> Harry++

Amicalement,
Scott Wood - June 25, 2013, 12:01 a.m.
On 06/24/2013 04:28:26 AM, Rai Harninder-B01044 wrote:
> Hello Albert,
> 
> 
> Plase fix commit message: you're not simply adding a boot delay flag,  
> you're in fact disabling autoboot, which is quite different.
> [Rai Harninder-B01044] "Disable autoboot through bootdelay config  
> flag". Is this ok?

But the patch isn't disabling autoboot.  It was already compile-time  
disabled, since CONFIG_BOOTDELAY wasn't specified at all.  The patch is  
allowing it to be enabled at runtime, without enabling it by default.

-Scott
Rai Harninder-B01044 - June 25, 2013, 12:38 p.m.
Hi Albert,

(weird way of quoting messages; makes it really hard to see who said what. Can't your mail client use the standard ">" line prefix?)
> Fixed

As for the commit summary, it is fine by me.
> Scott has commented on the same. Can you please have a look and suggest?


Are you not mixing up BSC9131RDB and BSC9132QDS maintainership with MPC85xx tree custodianship?
> 913x fall under 85xx platforms only (though their naming convention is quite different) for which Andy has the custodianship. I will anyways include both Poonam and Naveen in the next rev of the patch (if any)

> Harry++
Scott Wood - June 25, 2013, 5:45 p.m.
On 06/25/2013 07:38:20 AM, Rai Harninder-B01044 wrote:
> Hi Albert,
> 
> (weird way of quoting messages; makes it really hard to see who said  
> what. Can't your mail client use the standard ">" line prefix?)
> > Fixed

No, it's not fixed.  "> " is supposed to go before lines that you're  
quoting -- not before your own lines.

-Scott

Patch

diff --git a/include/configs/BSC9131RDB.h b/include/configs/BSC9131RDB.h
index fd076e0..c56b119 100644
--- a/include/configs/BSC9131RDB.h
+++ b/include/configs/BSC9131RDB.h
@@ -393,6 +393,7 @@  extern unsigned long get_sdram_size(void);
 #define CONFIG_UBOOTPATH	"u-boot.bin" /* U-Boot image on TFTP server */
 
 #define CONFIG_BAUDRATE		115200
+#define CONFIG_BOOTDELAY	-1 /* Disable autoboot */
 
 #define	CONFIG_EXTRA_ENV_SETTINGS				\
 	"netdev=eth0\0"						\
diff --git a/include/configs/BSC9132QDS.h b/include/configs/BSC9132QDS.h
index 9d15d0e..d54900a 100644
--- a/include/configs/BSC9132QDS.h
+++ b/include/configs/BSC9132QDS.h
@@ -607,6 +607,7 @@  combinations. this should be removed later
 #define CONFIG_UBOOTPATH	"u-boot.bin"
 
 #define CONFIG_BAUDRATE		115200
+#define CONFIG_BOOTDELAY	-1 /* Disable autoboot */
 
 #ifdef CONFIG_SDCARD
 #define CONFIG_DEF_HWCONFIG	"hwconfig=usb1:dr_mode=host,phy_type=ulpi\0"