diff mbox

[U-Boot] mx53loco: Define CONFIG_BOARD_LATE_INIT

Message ID 1343762513-5574-1-git-send-email-fabio.estevam@freescale.com
State Awaiting Upstream
Headers show

Commit Message

Fabio Estevam July 31, 2012, 7:21 p.m. UTC
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(-)

Comments

Wolfgang Denk July 31, 2012, 7:59 p.m. UTC | #1
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
Stefano Babic July 31, 2012, 8:40 p.m. UTC | #2
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 July 31, 2012, 8:50 p.m. UTC | #3
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
Stefano Babic July 31, 2012, 9:20 p.m. UTC | #4
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 Aug. 5, 2012, 7:34 a.m. UTC | #5
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
Stefano Babic Aug. 5, 2012, 10:11 a.m. UTC | #6
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
diff mbox

Patch

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