mx53loco: Define CONFIG_BOARD_LATE_INIT

PATCH New Fabio Estevam 4 participants 10 messages Jul 31, 2012 – Aug 5, 2012

lore

Share & tools

lore

Fabio Estevam

author

2012-07-31 19:21 UTC #0 lore raw

PATCH 1/1 mx53loco: Define CONFIG_BOARD_LATE_INIT

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 <[email protected]>

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

include/configs/mx53loco.h +1 -0

index 0a25c7d..bd23387 100644
@@ -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

Signature · 1 line

1.7.1

Fabio Estevam

2012-07-31 19:38 UTC #1 lore raw

PATCH 1/1 mx53loco: Define CONFIG_BOARD_LATE_INIT

Quoted 23 lines · 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 <[email protected]>
---
 include/configs/mx53loco.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
include/configs/mx53loco.h +0 -0
index 0a25c7d..bd23387 100644

include/configs/mx53loco.h line 46

Quoted 6 lines

include/configs/mx53loco.h +1 -0

@@ -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?

(quoted diff) +0 -0

b/board/freescale/mx53loco/mx53loco.c
index cbdcfad..4176d46 100644

board/freescale/mx53loco/mx53loco.c +1 -3

@@ -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)
 {

Signature · 1 line

1.7.1

Wolfgang Denk

2012-07-31 19:56 UTC #2 lore raw

PATCH 1/1 mx53loco: Define CONFIG_BOARD_LATE_INIT

Dear Fabio Estevam,

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

board/freescale/mx53loco/mx53loco.c int print_cpuinfo(void) · line 500

Quoted 6 lines

board/freescale/mx53loco/mx53loco.c +1 -2

-#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

Signature · 5 lines

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 at 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

2012-07-31 20:51 UTC #3 lore raw

PATCH 1/1 mx53loco: Define CONFIG_BOARD_LATE_INIT

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

Quoted 2 lines

Dear Fabio Estevam,

Hi Fabio,

Quoted 13 lines

In message <CAOMZO5CBpA4kVC+J6Yp8RwmL-EbLwK1PcYWC_RLX=SwWRnqsyg@mail.gmail.com> you wrote:
...
board/freescale/mx53loco/mx53loco.c int print_cpuinfo(void) · line 500
Quoted 6 lines
board/freescale/mx53loco/mx53loco.c +1 -2
-#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

Signature · 5 lines

=====================================================================
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 at denx.de
=====================================================================

Wolfgang Denk

2012-07-31 19:59 UTC #4 lore raw

PATCH 1/1 mx53loco: Define CONFIG_BOARD_LATE_INIT

In reply to Fabio Estevam

Dear Fabio Estevam,

Quoted 4 lines · In message <[email protected]> you…

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:

Quoted 8 lines

#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

Signature · 4 lines

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 at denx.de
A failure will not appear until a unit has passed final inspection.

stefano babic

2012-07-31 20:40 UTC #5 lore raw

PATCH 1/1 mx53loco: Define CONFIG_BOARD_LATE_INIT

In reply to Fabio Estevam

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

Quoted 11 lines

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;
}

Quoted 4 lines

#endif
Signed-off-by Fabio Estevam <[email protected]>
---

Hi Fabio,

include/configs/mx53loco.h line 44

Quoted 12 lines

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

include/configs/mx53loco.h +1 -0

index 0a25c7d..bd23387 100644
@@ -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 <[email protected]>

Best regards,
Stefano Babic

Signature · 5 lines

=====================================================================
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 at denx.de
=====================================================================

Wolfgang Denk

2012-07-31 20:50 UTC #6 lore raw

PATCH 1/1 mx53loco: Define CONFIG_BOARD_LATE_INIT

Dear Stefano,

Quoted 25 lines · In message <[email protected]> you wrote:

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

	return 0;
}
Quoted 4 lines
#endif
Signed-off-by Fabio Estevam <[email protected]>
---
Hi Fabio,
Quoted 10 lines
 include/configs/mx53loco.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
include/configs/mx53loco.h +0 -0
index 0a25c7d..bd23387 100644
@@ -41,6 +41,7 @@
 #define CONFIG_SYS_MALLOC_LEN		(10 * 1024 * 1024)
 

Quoted 6 lines

include/configs/mx53loco.h line 44
Quoted 2 lines
include/configs/mx53loco.h +1 -0
 #define CONFIG_BOARD_EARLY_INIT_F
+#define CONFIG_BOARD_LATE_INIT
I see, commit eae08eb2b53ffb87f3342e45ab422d8625659fcd dropped it.
Acked-by Stefano Babic <[email protected]>

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

Best regards,

Wolfgang Denk

Signature · 5 lines

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 at denx.de
"If that makes any sense to you, you have a big problem."
                                  -- C. Durance, Computer Science 234

stefano babic

2012-07-31 21:20 UTC #7 lore raw

PATCH 1/1 mx53loco: Define CONFIG_BOARD_LATE_INIT

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

Quoted 33 lines

Dear Stefano,
Quoted 30 lines · In message <[email protected]> you wrote:
Quoted 8 lines
#ifdef CONFIG_BOARD_LATE_INIT
int board_late_init(void)
{
	setenv("stdout", "serial");

	return 0;
}
Quoted 4 lines
#endif
Signed-off-by Fabio Estevam <[email protected]>
---
Hi Fabio,
include/configs/mx53loco.h line 44
Quoted 12 lines
 include/configs/mx53loco.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
include/configs/mx53loco.h +1 -0
index 0a25c7d..bd23387 100644
@@ -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.

Quoted 4 lines

Quoted 1 line
Acked-by Stefano Babic <[email protected]>
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

Signature · 5 lines

=====================================================================
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 at denx.de
=====================================================================

Stefano Babic

2012-08-05 07:34 UTC #8 lore raw

PATCH 1/1 mx53loco: Define CONFIG_BOARD_LATE_INIT

In reply to Fabio Estevam

Quoted 23 lines · 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 <[email protected]>
---
 include/configs/mx53loco.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
include/configs/mx53loco.h +0 -0
index 0a25c7d..bd23387 100644

include/configs/mx53loco.h line 44

Quoted 4 lines

include/configs/mx53loco.h +1 -0

@@ -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

Signature · 5 lines

=====================================================================
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 at denx.de
=====================================================================

Stefano Babic

2012-08-05 10:11 UTC #9 lore raw

PATCH 1/1 mx53loco: Define CONFIG_BOARD_LATE_INIT

Quoted 32 lines · Stefano Babic

Quoted 23 lines · 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 <[email protected]>
---
 include/configs/mx53loco.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
include/configs/mx53loco.h +0 -0
index 0a25c7d..bd23387 100644
include/configs/mx53loco.h line 44
Quoted 4 lines
include/configs/mx53loco.h +1 -0
@@ -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.

Quoted 3 lines

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

Signature · 5 lines

=====================================================================
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 at denx.de
=====================================================================