[U-Boot] [PATCH] mx53loco: Define CONFIG_BOARD_LATE_INIT

newer
[U-Boot] [PATCH] MX28: Move the...

older
[U-Boot] The way the list manages...

Fabio Estevam

31 Jul 2012 31 Jul '12

9:21 p.m.

Define CONFIG_BOARD_LATE_INIT so that the serial console messages can be redirected to the serial port.

This is needed because in mx53loco.c we have:

#ifdef CONFIG_BOARD_LATE_INIT int board_late_init(void) { setenv("stdout", "serial");

return 0; } #endif

Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- include/configs/mx53loco.h | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/configs/mx53loco.h b/include/configs/mx53loco.h index 0a25c7d..bd23387 100644 --- a/include/configs/mx53loco.h +++ b/include/configs/mx53loco.h @@ -41,6 +41,7 @@ #define CONFIG_SYS_MALLOC_LEN (10 * 1024 * 1024)

#define CONFIG_BOARD_EARLY_INIT_F +#define CONFIG_BOARD_LATE_INIT #define CONFIG_MXC_GPIO #define CONFIG_REVISION_TAG

-- 1.7.1

Show replies by date

Fabio Estevam

31 Jul 31 Jul

9:38 p.m.

On Tue, Jul 31, 2012 at 4:21 PM, Fabio Estevam fabio.estevam@freescale.com wrote:

...

Define CONFIG_BOARD_LATE_INIT so that the serial console messages can be redirected to the serial port.

This is needed because in mx53loco.c we have:

#ifdef CONFIG_BOARD_LATE_INIT int board_late_init(void) { setenv("stdout", "serial");
    return 0;
} #endif

Signed-off-by: Fabio Estevam fabio.estevam@freescale.com

include/configs/mx53loco.h | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/configs/mx53loco.h b/include/configs/mx53loco.h index 0a25c7d..bd23387 100644 --- a/include/configs/mx53loco.h +++ b/include/configs/mx53loco.h @@ -41,6 +41,7 @@ #define CONFIG_SYS_MALLOC_LEN (10 * 1024 * 1024)

#define CONFIG_BOARD_EARLY_INIT_F +#define CONFIG_BOARD_LATE_INIT #define CONFIG_MXC_GPIO #define CONFIG_REVISION_TAG

Or maybe this is a better approach?

diff --git a/board/freescale/mx53loco/mx53loco.c b/board/freescale/mx53loco/mx53loco.c index cbdcfad..4176d46 100644 --- a/board/freescale/mx53loco/mx53loco.c +++ b/board/freescale/mx53loco/mx53loco.c @@ -495,14 +495,12 @@ int print_cpuinfo(void) return 0; }

-#ifdef CONFIG_BOARD_LATE_INIT int board_late_init(void) { - setenv("stdout", "serial"); + setenv("preboot", "setenv stdout serial\0");

return 0; } -#endif

int board_init(void) {

-- 1.7.1

Wolfgang Denk

9:56 p.m.

Dear Fabio Estevam,

In message CAOMZO5CBpA4kVC+J6Yp8RwmL-EbLwK1PcYWC_RLX=SwWRnqsyg@mail.gmail.com you wrote: ...

...

-#ifdef CONFIG_BOARD_LATE_INIT int board_late_init(void) {

setenv("stdout", "serial");

setenv("preboot", "setenv stdout serial\0");

NAK. Please never, never ever mandatorily overwrite environment variables! The user who sets it to a different value and cannot find out why his settings don;t work and always get overwritten would be seriously frustrated.

Best regards,

Wolfgang Denk

-- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de If A equals success, then the formula is A = X + Y + Z. X is work. Y is play. Z is keep your mouth shut. - Albert Einstein

stefano babic

10:51 p.m.

Am 31/07/2012 21:56, schrieb Wolfgang Denk:

...

Dear Fabio Estevam,

Hi Fabio,

...

In message CAOMZO5CBpA4kVC+J6Yp8RwmL-EbLwK1PcYWC_RLX=SwWRnqsyg@mail.gmail.com you wrote: ...

...
-#ifdef CONFIG_BOARD_LATE_INIT int board_late_init(void) {

setenv("stdout", "serial");

setenv("preboot", "setenv stdout serial\0");

NAK. Please never, never ever mandatorily overwrite environment variables! The user who sets it to a different value and cannot find out why his settings don;t work and always get overwritten would be seriously frustrated.

Maybe a better way is to use CONFIG_PREBOOT as you do, but without hard-coding the variable. You can add "preboot" to the default environment (CONFIG_EXTRA_ENV_SETTINGS), and the user can still overwrite the behavior setting the variable in the u-boot shell.

But I see I did not follow the same approach for the mx51evk (blame on me !).

Stefano

-- ===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================

Wolfgang Denk

9:59 p.m.

Dear Fabio Estevam,

In message 1343762513-5574-1-git-send-email-fabio.estevam@freescale.com you wrote:

...

Define CONFIG_BOARD_LATE_INIT so that the serial console messages can be redirected to the serial port.

This is needed because in mx53loco.c we have:

#ifdef CONFIG_BOARD_LATE_INIT int board_late_init(void) { setenv("stdout", "serial");

return 0; } #endif

Why would that beneeded? And why must it be done mandatorily, without a chance for the user to configure different behaviour?

Please feel fre to define defualt settings, but please always give the user an option to select different behaviour. This is what the environment variables are made for - if we would always sent them mandatorily in the code, we would nbot need any representation in the environment.

Best regards,

Wolfgang Denk

-- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de A failure will not appear until a unit has passed final inspection.

stefano babic

10:40 p.m.

Am 31/07/2012 21:21, schrieb Fabio Estevam:

...

Define CONFIG_BOARD_LATE_INIT so that the serial console messages can be redirected to the serial port.

This is needed because in mx53loco.c we have:

#ifdef CONFIG_BOARD_LATE_INIT int board_late_init(void) { setenv("stdout", "serial");

return 0; } #endif

Signed-off-by: Fabio Estevam fabio.estevam@freescale.com

Hi Fabio,

...

include/configs/mx53loco.h | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/configs/mx53loco.h b/include/configs/mx53loco.h index 0a25c7d..bd23387 100644 --- a/include/configs/mx53loco.h +++ b/include/configs/mx53loco.h @@ -41,6 +41,7 @@ #define CONFIG_SYS_MALLOC_LEN (10 * 1024 * 1024)

#define CONFIG_BOARD_EARLY_INIT_F +#define CONFIG_BOARD_LATE_INIT

I see, commit eae08eb2b53ffb87f3342e45ab422d8625659fcd dropped it.

Acked-by: Stefano Babic sbabic@denx.de

Best regards, Stefano Babic

Wolfgang Denk

10:50 p.m.

Dear Stefano,

In message 501842D4.6060309@denx.de you wrote:

...

...
#ifdef CONFIG_BOARD_LATE_INIT int board_late_init(void) { setenv("stdout", "serial");

return 0; } #endif

Signed-off-by: Fabio Estevam fabio.estevam@freescale.com

Hi Fabio,

...
include/configs/mx53loco.h | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/configs/mx53loco.h b/include/configs/mx53loco.h index 0a25c7d..bd23387 100644 --- a/include/configs/mx53loco.h +++ b/include/configs/mx53loco.h @@ -41,6 +41,7 @@ #define CONFIG_SYS_MALLOC_LEN (10 * 1024 * 1024)

#define CONFIG_BOARD_EARLY_INIT_F +#define CONFIG_BOARD_LATE_INIT

I see, commit eae08eb2b53ffb87f3342e45ab422d8625659fcd dropped it.

Acked-by: Stefano Babic sbabic@denx.de

Please see my previous message - I dislike the first part of the patch, the unconditional "setenv stdout serial".

Best regards,

Wolfgang Denk

-- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de "If that makes any sense to you, you have a big problem." -- C. Durance, Computer Science 234

stefano babic

11:20 p.m.

Am 31/07/2012 22:50, schrieb Wolfgang Denk:

...

Dear Stefano,

In message 501842D4.6060309@denx.de you wrote:

...
...
#ifdef CONFIG_BOARD_LATE_INIT int board_late_init(void) { setenv("stdout", "serial");

return 0; } #endif

Signed-off-by: Fabio Estevam fabio.estevam@freescale.com

Hi Fabio,

...
include/configs/mx53loco.h | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/configs/mx53loco.h b/include/configs/mx53loco.h index 0a25c7d..bd23387 100644 --- a/include/configs/mx53loco.h +++ b/include/configs/mx53loco.h @@ -41,6 +41,7 @@ #define CONFIG_SYS_MALLOC_LEN (10 * 1024 * 1024)

#define CONFIG_BOARD_EARLY_INIT_F +#define CONFIG_BOARD_LATE_INIT

I see, commit eae08eb2b53ffb87f3342e45ab422d8625659fcd dropped it.

Acked-by: Stefano Babic sbabic@denx.de

Please see my previous message - I dislike the first part of the patch, the unconditional "setenv stdout serial".

Right - but this is not part of the patch, it is only in the commit message and regards code that is already mainlined. The patch sets only CONFIG_BOARD_LATE_INIT. The part you dislike should be fixed by a separate patch.

Stefano

Stefano Babic

5 Aug 5 Aug

9:34 a.m.

On 31/07/2012 21:21, Fabio Estevam wrote:

...

Define CONFIG_BOARD_LATE_INIT so that the serial console messages can be redirected to the serial port.

This is needed because in mx53loco.c we have:

#ifdef CONFIG_BOARD_LATE_INIT int board_late_init(void) { setenv("stdout", "serial");

return 0; } #endif

Signed-off-by: Fabio Estevam fabio.estevam@freescale.com

include/configs/mx53loco.h | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/configs/mx53loco.h b/include/configs/mx53loco.h index 0a25c7d..bd23387 100644 --- a/include/configs/mx53loco.h +++ b/include/configs/mx53loco.h @@ -41,6 +41,7 @@ #define CONFIG_SYS_MALLOC_LEN (10 * 1024 * 1024)

#define CONFIG_BOARD_EARLY_INIT_F +#define CONFIG_BOARD_LATE_INIT

Applied to u-boot-imx, thank.

This is a real fix - board does not run without it. According to our discussion in the thread, dropping setenv("stdout", "serial") in code should be done in a separate patch.

Best regards, Stefano Babic

-- ===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de =====================================================================

Stefano Babic

12:11 p.m.

On 05/08/2012 09:34, Stefano Babic wrote:

...

On 31/07/2012 21:21, Fabio Estevam wrote:

...
Define CONFIG_BOARD_LATE_INIT so that the serial console messages can be redirected to the serial port.

This is needed because in mx53loco.c we have:

#ifdef CONFIG_BOARD_LATE_INIT int board_late_init(void) { setenv("stdout", "serial");

return 0; } #endif

Signed-off-by: Fabio Estevam fabio.estevam@freescale.com

include/configs/mx53loco.h | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/configs/mx53loco.h b/include/configs/mx53loco.h index 0a25c7d..bd23387 100644 --- a/include/configs/mx53loco.h +++ b/include/configs/mx53loco.h @@ -41,6 +41,7 @@ #define CONFIG_SYS_MALLOC_LEN (10 * 1024 * 1024)

#define CONFIG_BOARD_EARLY_INIT_F +#define CONFIG_BOARD_LATE_INIT

Applied to u-boot-imx, thank.

This is a real fix - board does not run without it. According to our discussion in the thread, dropping setenv("stdout", "serial") in code should be done in a separate patch.

Sorry Fabio - I was too fast. I thnked about and there is a better solution. I drop your patch from u-boot-imx and I send a new on for review, fixing the same issue.

Stefano

-- ===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de =====================================================================

4659

Age (days ago)

4664

Last active (days ago)

List overview

Download

9 comments

5 participants

tags (0)

participants (5)

Fabio Estevam
Fabio Estevam
Stefano Babic
stefano babic
Wolfgang Denk