[U-Boot] [PATCH] am33xx, spl, siemens: enable debug uart output again

[U-Boot] [PATCH] am33xx, spl, siemens: enable debug uart output again

[Heiko Schocher]

a6b541b090: TI ARMv7: Don't use GD before crt0.S has set it

moves the init of the debug uart at the very end of SPL code.
Enable it for the siemens board earlier, as they print
ddr settings ... all debug output before board_init_r()
is here currently useless. Maybe we must rework this
globally?

Signed-off-by: Heiko Schocher <hs@denx.de>
---

 board/siemens/common/board.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/board/siemens/common/board.c b/board/siemens/common/board.c
index cc0ac6b..fb2de48 100644
--- a/board/siemens/common/board.c
+++ b/board/siemens/common/board.c
@@ -43,6 +43,11 @@ void set_mux_conf_regs(void)
 	/* Initalize the board header */
 	enable_i2c0_pin_mux();
 	i2c_set_bus_num(0);
+
+	/* enable early the console */
+	gd->baudrate = CONFIG_BAUDRATE;
+	serial_init();
+	gd->have_console = 1;
 	if (read_eeprom() < 0)
 		puts("Could not get board ID.\n");
 
-- 
2.1.0

[U-Boot] [PATCH] am33xx, spl, siemens: enable debug uart output again

[Simon Glass]

Hi Heiko,

On 23 February 2015 at 23:18, Heiko Schocher <hs@denx.de> wrote:
> a6b541b090: TI ARMv7: Don't use GD before crt0.S has set it
>
> moves the init of the debug uart at the very end of SPL code.
> Enable it for the siemens board earlier, as they print
> ddr settings ... all debug output before board_init_r()
> is here currently useless. Maybe we must rework this
> globally?

Assuming we are talking about U-Boot proper, the DDR init should
happen in board_init_f(), specifically dram_init(). so I think this
code should be updated.

If it is SPL, then DDR init should happen in SPL's board_init_f().

I sent a series a few weeks ago (available at u-boot-dm branch
spl-working) related to this topic:

http://patchwork.ozlabs.org/patch/438581/

>
> Signed-off-by: Heiko Schocher <hs@denx.de>
> ---
>
>  board/siemens/common/board.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/board/siemens/common/board.c b/board/siemens/common/board.c
> index cc0ac6b..fb2de48 100644
> --- a/board/siemens/common/board.c
> +++ b/board/siemens/common/board.c
> @@ -43,6 +43,11 @@ void set_mux_conf_regs(void)
>         /* Initalize the board header */
>         enable_i2c0_pin_mux();
>         i2c_set_bus_num(0);
> +
> +       /* enable early the console */
> +       gd->baudrate = CONFIG_BAUDRATE;
> +       serial_init();
> +       gd->have_console = 1;
>         if (read_eeprom() < 0)
>                 puts("Could not get board ID.\n");
>
> --
> 2.1.0
>

Regards,
Simon

[U-Boot] [PATCH] am33xx, spl, siemens: enable debug uart output again

[Heiko Schocher]

Hello Simon,

Am 24.02.2015 14:31, schrieb Simon Glass:
> Hi Heiko,
>
> On 23 February 2015 at 23:18, Heiko Schocher <hs@denx.de> wrote:
>> a6b541b090: TI ARMv7: Don't use GD before crt0.S has set it
>>
>> moves the init of the debug uart at the very end of SPL code.
>> Enable it for the siemens board earlier, as they print
>> ddr settings ... all debug output before board_init_r()
>> is here currently useless. Maybe we must rework this
>> globally?
>
> Assuming we are talking about U-Boot proper, the DDR init should
> happen in board_init_f(), specifically dram_init(). so I think this
> code should be updated.
>
> If it is SPL, then DDR init should happen in SPL's board_init_f().

It is in SPL...

sdram_init() is called from:

./arch/arm/cpu/armv7/am33xx/board.c from s_init() ...

> I sent a series a few weeks ago (available at u-boot-dm branch
> spl-working) related to this topic:
>
> http://patchwork.ozlabs.org/patch/438581/

Ah ... Hmm... so "./arch/arm/cpu/armv7/am33xx/board.c" needs
a rework, right?

Is a simple rename s_init() -> board_init_f() correct?

@Tom: Could you help here?

bye,
Heiko
>> Signed-off-by: Heiko Schocher <hs@denx.de>
>> ---
>>
>>   board/siemens/common/board.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/board/siemens/common/board.c b/board/siemens/common/board.c
>> index cc0ac6b..fb2de48 100644
>> --- a/board/siemens/common/board.c
>> +++ b/board/siemens/common/board.c
>> @@ -43,6 +43,11 @@ void set_mux_conf_regs(void)
>>          /* Initalize the board header */
>>          enable_i2c0_pin_mux();
>>          i2c_set_bus_num(0);
>> +
>> +       /* enable early the console */
>> +       gd->baudrate = CONFIG_BAUDRATE;
>> +       serial_init();
>> +       gd->have_console = 1;
>>          if (read_eeprom() < 0)
>>                  puts("Could not get board ID.\n");
>>
>> --
>> 2.1.0
>>
>
> Regards,
> Simon
>

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH] am33xx, spl, siemens: enable debug uart output again

[Tom Rini]

On Mon, Mar 02, 2015 at 07:56:41AM +0100, Heiko Schocher wrote:
> Hello Simon,
> 
> Am 24.02.2015 14:31, schrieb Simon Glass:
> >Hi Heiko,
> >
> >On 23 February 2015 at 23:18, Heiko Schocher <hs@denx.de> wrote:
> >>a6b541b090: TI ARMv7: Don't use GD before crt0.S has set it
> >>
> >>moves the init of the debug uart at the very end of SPL code.
> >>Enable it for the siemens board earlier, as they print
> >>ddr settings ... all debug output before board_init_r()
> >>is here currently useless. Maybe we must rework this
> >>globally?
> >
> >Assuming we are talking about U-Boot proper, the DDR init should
> >happen in board_init_f(), specifically dram_init(). so I think this
> >code should be updated.
> >
> >If it is SPL, then DDR init should happen in SPL's board_init_f().
> 
> It is in SPL...
> 
> sdram_init() is called from:
> 
> ./arch/arm/cpu/armv7/am33xx/board.c from s_init() ...
> 
> >I sent a series a few weeks ago (available at u-boot-dm branch
> >spl-working) related to this topic:
> >
> >http://patchwork.ozlabs.org/patch/438581/
> 
> Ah ... Hmm... so "./arch/arm/cpu/armv7/am33xx/board.c" needs
> a rework, right?
> 
> Is a simple rename s_init() -> board_init_f() correct?

Right so, no, we can't just rename s_init to board_init_f.  This is what
I was talking about in the thread about the function Hans wants to add
to enable some bits in CP15 on sunxi, iirc.

In short, armv7 has a different set of abstraction hooks than the
previous ARM cores (armv8 followed what we have for v7) and I'm not
convinced in the end that it really won us anything.  See
http://lists.denx.de/pipermail/u-boot/2015-January/202350.html

For today you need to rework the Siemens code to print out the DDR
values (when desired) in spl_board_init() as we do not, or will not
shortly, have gd prior to board_init_f running.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150302/a64f3d60/attachment.sig>

[U-Boot] [PATCH] am33xx, spl, siemens: enable debug uart output again

[Heiko Schocher]

Hello Tom,

Am 02.03.2015 14:59, schrieb Tom Rini:
> On Mon, Mar 02, 2015 at 07:56:41AM +0100, Heiko Schocher wrote:
>> Hello Simon,
>>
>> Am 24.02.2015 14:31, schrieb Simon Glass:
>>> Hi Heiko,
>>>
>>> On 23 February 2015 at 23:18, Heiko Schocher <hs@denx.de> wrote:
>>>> a6b541b090: TI ARMv7: Don't use GD before crt0.S has set it
>>>>
>>>> moves the init of the debug uart at the very end of SPL code.
>>>> Enable it for the siemens board earlier, as they print
>>>> ddr settings ... all debug output before board_init_r()
>>>> is here currently useless. Maybe we must rework this
>>>> globally?
>>>
>>> Assuming we are talking about U-Boot proper, the DDR init should
>>> happen in board_init_f(), specifically dram_init(). so I think this
>>> code should be updated.
>>>
>>> If it is SPL, then DDR init should happen in SPL's board_init_f().
>>
>> It is in SPL...
>>
>> sdram_init() is called from:
>>
>> ./arch/arm/cpu/armv7/am33xx/board.c from s_init() ...
>>
>>> I sent a series a few weeks ago (available at u-boot-dm branch
>>> spl-working) related to this topic:
>>>
>>> http://patchwork.ozlabs.org/patch/438581/
>>
>> Ah ... Hmm... so "./arch/arm/cpu/armv7/am33xx/board.c" needs
>> a rework, right?
>>
>> Is a simple rename s_init() -> board_init_f() correct?
>
> Right so, no, we can't just rename s_init to board_init_f.  This is what
> I was talking about in the thread about the function Hans wants to add
> to enable some bits in CP15 on sunxi, iirc.
>
> In short, armv7 has a different set of abstraction hooks than the
> previous ARM cores (armv8 followed what we have for v7) and I'm not
> convinced in the end that it really won us anything.  See
> http://lists.denx.de/pipermail/u-boot/2015-January/202350.html
>
> For today you need to rework the Siemens code to print out the DDR
> values (when desired) in spl_board_init() as we do not, or will not
> shortly, have gd prior to board_init_f running.

Hmm... first I thought, ok, no problem, move the output from the RAM
parameters to spl_board_init() ... but thats only the half of the
story ... They read the RAM parameters from an i2c eeprom, and if
there are errors, they print this errors ... currently this does
not work, and thats I think the more important case ... and I could
not move this error printfs to somewhere, because if RAM is not
working ... there is no later ...

So I have to enable the console early ... maybe I missed something,
but this worked fine in the past (and I think we should not break
this, as this is an essential feature).

If I look into arch/arm/cpu/armv7/am33xx/board.c s_init()
there gets the console also in the case:

#if defined(CONFIG_NOR_BOOT) || defined(CONFIG_QSPI_BOOT)

very early enabled ... is this buggy?

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH] am33xx, spl, siemens: enable debug uart output again

[Tom Rini]

On Wed, Mar 04, 2015 at 08:42:58AM +0100, Heiko Schocher wrote:
> Hello Tom,
> 
> Am 02.03.2015 14:59, schrieb Tom Rini:
> >On Mon, Mar 02, 2015 at 07:56:41AM +0100, Heiko Schocher wrote:
> >>Hello Simon,
> >>
> >>Am 24.02.2015 14:31, schrieb Simon Glass:
> >>>Hi Heiko,
> >>>
> >>>On 23 February 2015 at 23:18, Heiko Schocher <hs@denx.de> wrote:
> >>>>a6b541b090: TI ARMv7: Don't use GD before crt0.S has set it
> >>>>
> >>>>moves the init of the debug uart at the very end of SPL code.
> >>>>Enable it for the siemens board earlier, as they print
> >>>>ddr settings ... all debug output before board_init_r()
> >>>>is here currently useless. Maybe we must rework this
> >>>>globally?
> >>>
> >>>Assuming we are talking about U-Boot proper, the DDR init should
> >>>happen in board_init_f(), specifically dram_init(). so I think this
> >>>code should be updated.
> >>>
> >>>If it is SPL, then DDR init should happen in SPL's board_init_f().
> >>
> >>It is in SPL...
> >>
> >>sdram_init() is called from:
> >>
> >>./arch/arm/cpu/armv7/am33xx/board.c from s_init() ...
> >>
> >>>I sent a series a few weeks ago (available at u-boot-dm branch
> >>>spl-working) related to this topic:
> >>>
> >>>http://patchwork.ozlabs.org/patch/438581/
> >>
> >>Ah ... Hmm... so "./arch/arm/cpu/armv7/am33xx/board.c" needs
> >>a rework, right?
> >>
> >>Is a simple rename s_init() -> board_init_f() correct?
> >
> >Right so, no, we can't just rename s_init to board_init_f.  This is what
> >I was talking about in the thread about the function Hans wants to add
> >to enable some bits in CP15 on sunxi, iirc.
> >
> >In short, armv7 has a different set of abstraction hooks than the
> >previous ARM cores (armv8 followed what we have for v7) and I'm not
> >convinced in the end that it really won us anything.  See
> >http://lists.denx.de/pipermail/u-boot/2015-January/202350.html
> >
> >For today you need to rework the Siemens code to print out the DDR
> >values (when desired) in spl_board_init() as we do not, or will not
> >shortly, have gd prior to board_init_f running.
> 
> Hmm... first I thought, ok, no problem, move the output from the RAM
> parameters to spl_board_init() ... but thats only the half of the
> story ... They read the RAM parameters from an i2c eeprom, and if
> there are errors, they print this errors ... currently this does
> not work, and thats I think the more important case ... and I could
> not move this error printfs to somewhere, because if RAM is not
> working ... there is no later ...
> 
> So I have to enable the console early ... maybe I missed something,
> but this worked fine in the past (and I think we should not break
> this, as this is an essential feature).

OK, I missed something too.  I think this gets better now once I merge
Simon's SPL series as we do all of this from board_init_f() and the
siemens code should just work again.

> If I look into arch/arm/cpu/armv7/am33xx/board.c s_init()
> there gets the console also in the case:
> 
> #if defined(CONFIG_NOR_BOOT) || defined(CONFIG_QSPI_BOOT)
> 
> very early enabled ... is this buggy?

Digging out one of the QSPI bootable boards (which I only _think_ I
have) or setting up and confirming the beaglebone white + NOR cape I
have still functions at the HW level and then re-testing on mainline are
on my list as yes, I'm a bit worried these paths might be broken again.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150304/004d971a/attachment.sig>

[U-Boot] [PATCH] am33xx, spl, siemens: enable debug uart output again

[Heiko Schocher]

Hello Tom,

Am 04.03.2015 17:40, schrieb Tom Rini:
> On Wed, Mar 04, 2015 at 08:42:58AM +0100, Heiko Schocher wrote:
>> Hello Tom,
>>
>> Am 02.03.2015 14:59, schrieb Tom Rini:
>>> On Mon, Mar 02, 2015 at 07:56:41AM +0100, Heiko Schocher wrote:
>>>> Hello Simon,
>>>>
>>>> Am 24.02.2015 14:31, schrieb Simon Glass:
>>>>> Hi Heiko,
>>>>>
>>>>> On 23 February 2015 at 23:18, Heiko Schocher <hs@denx.de> wrote:
>>>>>> a6b541b090: TI ARMv7: Don't use GD before crt0.S has set it
>>>>>>
>>>>>> moves the init of the debug uart at the very end of SPL code.
>>>>>> Enable it for the siemens board earlier, as they print
>>>>>> ddr settings ... all debug output before board_init_r()
>>>>>> is here currently useless. Maybe we must rework this
>>>>>> globally?
>>>>>
>>>>> Assuming we are talking about U-Boot proper, the DDR init should
>>>>> happen in board_init_f(), specifically dram_init(). so I think this
>>>>> code should be updated.
>>>>>
>>>>> If it is SPL, then DDR init should happen in SPL's board_init_f().
>>>>
>>>> It is in SPL...
>>>>
>>>> sdram_init() is called from:
>>>>
>>>> ./arch/arm/cpu/armv7/am33xx/board.c from s_init() ...
>>>>
>>>>> I sent a series a few weeks ago (available at u-boot-dm branch
>>>>> spl-working) related to this topic:
>>>>>
>>>>> http://patchwork.ozlabs.org/patch/438581/
>>>>
>>>> Ah ... Hmm... so "./arch/arm/cpu/armv7/am33xx/board.c" needs
>>>> a rework, right?
>>>>
>>>> Is a simple rename s_init() -> board_init_f() correct?
>>>
>>> Right so, no, we can't just rename s_init to board_init_f.  This is what
>>> I was talking about in the thread about the function Hans wants to add
>>> to enable some bits in CP15 on sunxi, iirc.
>>>
>>> In short, armv7 has a different set of abstraction hooks than the
>>> previous ARM cores (armv8 followed what we have for v7) and I'm not
>>> convinced in the end that it really won us anything.  See
>>> http://lists.denx.de/pipermail/u-boot/2015-January/202350.html
>>>
>>> For today you need to rework the Siemens code to print out the DDR
>>> values (when desired) in spl_board_init() as we do not, or will not
>>> shortly, have gd prior to board_init_f running.
>>
>> Hmm... first I thought, ok, no problem, move the output from the RAM
>> parameters to spl_board_init() ... but thats only the half of the
>> story ... They read the RAM parameters from an i2c eeprom, and if
>> there are errors, they print this errors ... currently this does
>> not work, and thats I think the more important case ... and I could
>> not move this error printfs to somewhere, because if RAM is not
>> working ... there is no later ...
>>
>> So I have to enable the console early ... maybe I missed something,
>> but this worked fine in the past (and I think we should not break
>> this, as this is an essential feature).
>
> OK, I missed something too.  I think this gets better now once I merge
> Simon's SPL series as we do all of this from board_init_f() and the
> siemens code should just work again.

Yes, just saw your patch ;-)

If they are in mainline (or do you have them somewhere in a git repo?),
I test it again on the dxr2 board, thanks!

>> If I look into arch/arm/cpu/armv7/am33xx/board.c s_init()
>> there gets the console also in the case:
>>
>> #if defined(CONFIG_NOR_BOOT) || defined(CONFIG_QSPI_BOOT)
>>
>> very early enabled ... is this buggy?
>
> Digging out one of the QSPI bootable boards (which I only _think_ I
> have) or setting up and confirming the beaglebone white + NOR cape I
> have still functions at the HW level and then re-testing on mainline are
> on my list as yes, I'm a bit worried these paths might be broken again.
>

Thanks for the info!

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH] am33xx, spl, siemens: enable debug uart output again

[Heiko Schocher]

Hello Tom,

Am 05.03.2015 07:22, schrieb Heiko Schocher:
> Hello Tom,
>
> Am 04.03.2015 17:40, schrieb Tom Rini:
>> On Wed, Mar 04, 2015 at 08:42:58AM +0100, Heiko Schocher wrote:
>>> Hello Tom,
>>>
>>> Am 02.03.2015 14:59, schrieb Tom Rini:
>>>> On Mon, Mar 02, 2015 at 07:56:41AM +0100, Heiko Schocher wrote:
>>>>> Hello Simon,
>>>>>
>>>>> Am 24.02.2015 14:31, schrieb Simon Glass:
>>>>>> Hi Heiko,
>>>>>>
>>>>>> On 23 February 2015 at 23:18, Heiko Schocher <hs@denx.de> wrote:
>>>>>>> a6b541b090: TI ARMv7: Don't use GD before crt0.S has set it
>>>>>>>
>>>>>>> moves the init of the debug uart at the very end of SPL code.
>>>>>>> Enable it for the siemens board earlier, as they print
>>>>>>> ddr settings ... all debug output before board_init_r()
>>>>>>> is here currently useless. Maybe we must rework this
>>>>>>> globally?
>>>>>>
>>>>>> Assuming we are talking about U-Boot proper, the DDR init should
>>>>>> happen in board_init_f(), specifically dram_init(). so I think this
>>>>>> code should be updated.
>>>>>>
>>>>>> If it is SPL, then DDR init should happen in SPL's board_init_f().
>>>>>
>>>>> It is in SPL...
>>>>>
>>>>> sdram_init() is called from:
>>>>>
>>>>> ./arch/arm/cpu/armv7/am33xx/board.c from s_init() ...
>>>>>
>>>>>> I sent a series a few weeks ago (available at u-boot-dm branch
>>>>>> spl-working) related to this topic:
>>>>>>
>>>>>> http://patchwork.ozlabs.org/patch/438581/
>>>>>
>>>>> Ah ... Hmm... so "./arch/arm/cpu/armv7/am33xx/board.c" needs
>>>>> a rework, right?
>>>>>
>>>>> Is a simple rename s_init() -> board_init_f() correct?
>>>>
>>>> Right so, no, we can't just rename s_init to board_init_f.  This is what
>>>> I was talking about in the thread about the function Hans wants to add
>>>> to enable some bits in CP15 on sunxi, iirc.
>>>>
>>>> In short, armv7 has a different set of abstraction hooks than the
>>>> previous ARM cores (armv8 followed what we have for v7) and I'm not
>>>> convinced in the end that it really won us anything.  See
>>>> http://lists.denx.de/pipermail/u-boot/2015-January/202350.html
>>>>
>>>> For today you need to rework the Siemens code to print out the DDR
>>>> values (when desired) in spl_board_init() as we do not, or will not
>>>> shortly, have gd prior to board_init_f running.
>>>
>>> Hmm... first I thought, ok, no problem, move the output from the RAM
>>> parameters to spl_board_init() ... but thats only the half of the
>>> story ... They read the RAM parameters from an i2c eeprom, and if
>>> there are errors, they print this errors ... currently this does
>>> not work, and thats I think the more important case ... and I could
>>> not move this error printfs to somewhere, because if RAM is not
>>> working ... there is no later ...
>>>
>>> So I have to enable the console early ... maybe I missed something,
>>> but this worked fine in the past (and I think we should not break
>>> this, as this is an essential feature).
>>
>> OK, I missed something too.  I think this gets better now once I merge
>> Simon's SPL series as we do all of this from board_init_f() and the
>> siemens code should just work again.
>
> Yes, just saw your patch ;-)
>
> If they are in mainline (or do you have them somewhere in a git repo?),
> I test it again on the dxr2 board, thanks!

If I am correct, all needed patches from you are in mainline, just
tried this on the dxr2 board ... but I still need:

diff --git a/board/siemens/common/board.c b/board/siemens/common/board.c
index a39cbd0..8724604 100644
--- a/board/siemens/common/board.c
+++ b/board/siemens/common/board.c
@@ -40,6 +40,11 @@ void set_uart_mux_conf(void)

  void set_mux_conf_regs(void)
  {
+       /* enable early the console */
+       gd->baudrate = CONFIG_BAUDRATE;
+       serial_init();
+       gd->have_console = 1;
+
         /* Initalize the board header */
         enable_i2c0_pin_mux();
         i2c_set_bus_num(0);

to see the console output ...

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH] am33xx, spl, siemens: enable debug uart output again

[Tom Rini]

On Thu, Mar 05, 2015 at 09:46:50AM +0100, Heiko Schocher wrote:
> Hello Tom,
> 
> Am 05.03.2015 07:22, schrieb Heiko Schocher:
> >Hello Tom,
> >
> >Am 04.03.2015 17:40, schrieb Tom Rini:
> >>On Wed, Mar 04, 2015 at 08:42:58AM +0100, Heiko Schocher wrote:
> >>>Hello Tom,
> >>>
> >>>Am 02.03.2015 14:59, schrieb Tom Rini:
> >>>>On Mon, Mar 02, 2015 at 07:56:41AM +0100, Heiko Schocher wrote:
> >>>>>Hello Simon,
> >>>>>
> >>>>>Am 24.02.2015 14:31, schrieb Simon Glass:
> >>>>>>Hi Heiko,
> >>>>>>
> >>>>>>On 23 February 2015 at 23:18, Heiko Schocher <hs@denx.de> wrote:
> >>>>>>>a6b541b090: TI ARMv7: Don't use GD before crt0.S has set it
> >>>>>>>
> >>>>>>>moves the init of the debug uart at the very end of SPL code.
> >>>>>>>Enable it for the siemens board earlier, as they print
> >>>>>>>ddr settings ... all debug output before board_init_r()
> >>>>>>>is here currently useless. Maybe we must rework this
> >>>>>>>globally?
> >>>>>>
> >>>>>>Assuming we are talking about U-Boot proper, the DDR init should
> >>>>>>happen in board_init_f(), specifically dram_init(). so I think this
> >>>>>>code should be updated.
> >>>>>>
> >>>>>>If it is SPL, then DDR init should happen in SPL's board_init_f().
> >>>>>
> >>>>>It is in SPL...
> >>>>>
> >>>>>sdram_init() is called from:
> >>>>>
> >>>>>./arch/arm/cpu/armv7/am33xx/board.c from s_init() ...
> >>>>>
> >>>>>>I sent a series a few weeks ago (available at u-boot-dm branch
> >>>>>>spl-working) related to this topic:
> >>>>>>
> >>>>>>http://patchwork.ozlabs.org/patch/438581/
> >>>>>
> >>>>>Ah ... Hmm... so "./arch/arm/cpu/armv7/am33xx/board.c" needs
> >>>>>a rework, right?
> >>>>>
> >>>>>Is a simple rename s_init() -> board_init_f() correct?
> >>>>
> >>>>Right so, no, we can't just rename s_init to board_init_f.  This is what
> >>>>I was talking about in the thread about the function Hans wants to add
> >>>>to enable some bits in CP15 on sunxi, iirc.
> >>>>
> >>>>In short, armv7 has a different set of abstraction hooks than the
> >>>>previous ARM cores (armv8 followed what we have for v7) and I'm not
> >>>>convinced in the end that it really won us anything.  See
> >>>>http://lists.denx.de/pipermail/u-boot/2015-January/202350.html
> >>>>
> >>>>For today you need to rework the Siemens code to print out the DDR
> >>>>values (when desired) in spl_board_init() as we do not, or will not
> >>>>shortly, have gd prior to board_init_f running.
> >>>
> >>>Hmm... first I thought, ok, no problem, move the output from the RAM
> >>>parameters to spl_board_init() ... but thats only the half of the
> >>>story ... They read the RAM parameters from an i2c eeprom, and if
> >>>there are errors, they print this errors ... currently this does
> >>>not work, and thats I think the more important case ... and I could
> >>>not move this error printfs to somewhere, because if RAM is not
> >>>working ... there is no later ...
> >>>
> >>>So I have to enable the console early ... maybe I missed something,
> >>>but this worked fine in the past (and I think we should not break
> >>>this, as this is an essential feature).
> >>
> >>OK, I missed something too.  I think this gets better now once I merge
> >>Simon's SPL series as we do all of this from board_init_f() and the
> >>siemens code should just work again.
> >
> >Yes, just saw your patch ;-)
> >
> >If they are in mainline (or do you have them somewhere in a git repo?),
> >I test it again on the dxr2 board, thanks!
> 
> If I am correct, all needed patches from you are in mainline, just
> tried this on the dxr2 board ... but I still need:
> 
> diff --git a/board/siemens/common/board.c b/board/siemens/common/board.c
> index a39cbd0..8724604 100644
> --- a/board/siemens/common/board.c
> +++ b/board/siemens/common/board.c
> @@ -40,6 +40,11 @@ void set_uart_mux_conf(void)
> 
>  void set_mux_conf_regs(void)
>  {
> +       /* enable early the console */
> +       gd->baudrate = CONFIG_BAUDRATE;
> +       serial_init();
> +       gd->have_console = 1;
> +
>         /* Initalize the board header */
>         enable_i2c0_pin_mux();
>         i2c_set_bus_num(0);
> 
> to see the console output ...

Yes, if you want console prior to spl_board_init() then you need to do
that where you are.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150305/dc275e29/attachment.sig>

[U-Boot] [PATCH] am33xx, spl, siemens: enable debug uart output again

[Heiko Schocher]

Hello Tom,

Am 05.03.2015 15:50, schrieb Tom Rini:
> On Thu, Mar 05, 2015 at 09:46:50AM +0100, Heiko Schocher wrote:
>> Hello Tom,
>>
>> Am 05.03.2015 07:22, schrieb Heiko Schocher:
>>> Hello Tom,
>>>
>>> Am 04.03.2015 17:40, schrieb Tom Rini:
>>>> On Wed, Mar 04, 2015 at 08:42:58AM +0100, Heiko Schocher wrote:
>>>>> Hello Tom,
>>>>>
>>>>> Am 02.03.2015 14:59, schrieb Tom Rini:
>>>>>> On Mon, Mar 02, 2015 at 07:56:41AM +0100, Heiko Schocher wrote:
>>>>>>> Hello Simon,
>>>>>>>
>>>>>>> Am 24.02.2015 14:31, schrieb Simon Glass:
>>>>>>>> Hi Heiko,
>>>>>>>>
>>>>>>>> On 23 February 2015 at 23:18, Heiko Schocher <hs@denx.de> wrote:
>>>>>>>>> a6b541b090: TI ARMv7: Don't use GD before crt0.S has set it
>>>>>>>>>
>>>>>>>>> moves the init of the debug uart at the very end of SPL code.
>>>>>>>>> Enable it for the siemens board earlier, as they print
>>>>>>>>> ddr settings ... all debug output before board_init_r()
>>>>>>>>> is here currently useless. Maybe we must rework this
>>>>>>>>> globally?
>>>>>>>>
>>>>>>>> Assuming we are talking about U-Boot proper, the DDR init should
>>>>>>>> happen in board_init_f(), specifically dram_init(). so I think this
>>>>>>>> code should be updated.
>>>>>>>>
>>>>>>>> If it is SPL, then DDR init should happen in SPL's board_init_f().
>>>>>>>
>>>>>>> It is in SPL...
>>>>>>>
>>>>>>> sdram_init() is called from:
>>>>>>>
>>>>>>> ./arch/arm/cpu/armv7/am33xx/board.c from s_init() ...
>>>>>>>
>>>>>>>> I sent a series a few weeks ago (available at u-boot-dm branch
>>>>>>>> spl-working) related to this topic:
>>>>>>>>
>>>>>>>> http://patchwork.ozlabs.org/patch/438581/
>>>>>>>
>>>>>>> Ah ... Hmm... so "./arch/arm/cpu/armv7/am33xx/board.c" needs
>>>>>>> a rework, right?
>>>>>>>
>>>>>>> Is a simple rename s_init() -> board_init_f() correct?
>>>>>>
>>>>>> Right so, no, we can't just rename s_init to board_init_f.  This is what
>>>>>> I was talking about in the thread about the function Hans wants to add
>>>>>> to enable some bits in CP15 on sunxi, iirc.
>>>>>>
>>>>>> In short, armv7 has a different set of abstraction hooks than the
>>>>>> previous ARM cores (armv8 followed what we have for v7) and I'm not
>>>>>> convinced in the end that it really won us anything.  See
>>>>>> http://lists.denx.de/pipermail/u-boot/2015-January/202350.html
>>>>>>
>>>>>> For today you need to rework the Siemens code to print out the DDR
>>>>>> values (when desired) in spl_board_init() as we do not, or will not
>>>>>> shortly, have gd prior to board_init_f running.
>>>>>
>>>>> Hmm... first I thought, ok, no problem, move the output from the RAM
>>>>> parameters to spl_board_init() ... but thats only the half of the
>>>>> story ... They read the RAM parameters from an i2c eeprom, and if
>>>>> there are errors, they print this errors ... currently this does
>>>>> not work, and thats I think the more important case ... and I could
>>>>> not move this error printfs to somewhere, because if RAM is not
>>>>> working ... there is no later ...
>>>>>
>>>>> So I have to enable the console early ... maybe I missed something,
>>>>> but this worked fine in the past (and I think we should not break
>>>>> this, as this is an essential feature).
>>>>
>>>> OK, I missed something too.  I think this gets better now once I merge
>>>> Simon's SPL series as we do all of this from board_init_f() and the
>>>> siemens code should just work again.
>>>
>>> Yes, just saw your patch ;-)
>>>
>>> If they are in mainline (or do you have them somewhere in a git repo?),
>>> I test it again on the dxr2 board, thanks!
>>
>> If I am correct, all needed patches from you are in mainline, just
>> tried this on the dxr2 board ... but I still need:
>>
>> diff --git a/board/siemens/common/board.c b/board/siemens/common/board.c
>> index a39cbd0..8724604 100644
>> --- a/board/siemens/common/board.c
>> +++ b/board/siemens/common/board.c
>> @@ -40,6 +40,11 @@ void set_uart_mux_conf(void)
>>
>>   void set_mux_conf_regs(void)
>>   {
>> +       /* enable early the console */
>> +       gd->baudrate = CONFIG_BAUDRATE;
>> +       serial_init();
>> +       gd->have_console = 1;
>> +
>>          /* Initalize the board header */
>>          enable_i2c0_pin_mux();
>>          i2c_set_bus_num(0);
>>
>> to see the console output ...
>
> Yes, if you want console prior to spl_board_init() then you need to do
> that where you are.
>

Ah, ok... so this patch is OK?

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH] am33xx, spl, siemens: enable debug uart output again

[Tom Rini]

On Fri, Mar 06, 2015 at 08:24:47AM +0100, Heiko Schocher wrote:
> Hello Tom,
> 
> Am 05.03.2015 15:50, schrieb Tom Rini:
> >On Thu, Mar 05, 2015 at 09:46:50AM +0100, Heiko Schocher wrote:
> >>Hello Tom,
> >>
> >>Am 05.03.2015 07:22, schrieb Heiko Schocher:
> >>>Hello Tom,
> >>>
> >>>Am 04.03.2015 17:40, schrieb Tom Rini:
> >>>>On Wed, Mar 04, 2015 at 08:42:58AM +0100, Heiko Schocher wrote:
> >>>>>Hello Tom,
> >>>>>
> >>>>>Am 02.03.2015 14:59, schrieb Tom Rini:
> >>>>>>On Mon, Mar 02, 2015 at 07:56:41AM +0100, Heiko Schocher wrote:
> >>>>>>>Hello Simon,
> >>>>>>>
> >>>>>>>Am 24.02.2015 14:31, schrieb Simon Glass:
> >>>>>>>>Hi Heiko,
> >>>>>>>>
> >>>>>>>>On 23 February 2015 at 23:18, Heiko Schocher <hs@denx.de> wrote:
> >>>>>>>>>a6b541b090: TI ARMv7: Don't use GD before crt0.S has set it
> >>>>>>>>>
> >>>>>>>>>moves the init of the debug uart at the very end of SPL code.
> >>>>>>>>>Enable it for the siemens board earlier, as they print
> >>>>>>>>>ddr settings ... all debug output before board_init_r()
> >>>>>>>>>is here currently useless. Maybe we must rework this
> >>>>>>>>>globally?
> >>>>>>>>
> >>>>>>>>Assuming we are talking about U-Boot proper, the DDR init should
> >>>>>>>>happen in board_init_f(), specifically dram_init(). so I think this
> >>>>>>>>code should be updated.
> >>>>>>>>
> >>>>>>>>If it is SPL, then DDR init should happen in SPL's board_init_f().
> >>>>>>>
> >>>>>>>It is in SPL...
> >>>>>>>
> >>>>>>>sdram_init() is called from:
> >>>>>>>
> >>>>>>>./arch/arm/cpu/armv7/am33xx/board.c from s_init() ...
> >>>>>>>
> >>>>>>>>I sent a series a few weeks ago (available at u-boot-dm branch
> >>>>>>>>spl-working) related to this topic:
> >>>>>>>>
> >>>>>>>>http://patchwork.ozlabs.org/patch/438581/
> >>>>>>>
> >>>>>>>Ah ... Hmm... so "./arch/arm/cpu/armv7/am33xx/board.c" needs
> >>>>>>>a rework, right?
> >>>>>>>
> >>>>>>>Is a simple rename s_init() -> board_init_f() correct?
> >>>>>>
> >>>>>>Right so, no, we can't just rename s_init to board_init_f.  This is what
> >>>>>>I was talking about in the thread about the function Hans wants to add
> >>>>>>to enable some bits in CP15 on sunxi, iirc.
> >>>>>>
> >>>>>>In short, armv7 has a different set of abstraction hooks than the
> >>>>>>previous ARM cores (armv8 followed what we have for v7) and I'm not
> >>>>>>convinced in the end that it really won us anything.  See
> >>>>>>http://lists.denx.de/pipermail/u-boot/2015-January/202350.html
> >>>>>>
> >>>>>>For today you need to rework the Siemens code to print out the DDR
> >>>>>>values (when desired) in spl_board_init() as we do not, or will not
> >>>>>>shortly, have gd prior to board_init_f running.
> >>>>>
> >>>>>Hmm... first I thought, ok, no problem, move the output from the RAM
> >>>>>parameters to spl_board_init() ... but thats only the half of the
> >>>>>story ... They read the RAM parameters from an i2c eeprom, and if
> >>>>>there are errors, they print this errors ... currently this does
> >>>>>not work, and thats I think the more important case ... and I could
> >>>>>not move this error printfs to somewhere, because if RAM is not
> >>>>>working ... there is no later ...
> >>>>>
> >>>>>So I have to enable the console early ... maybe I missed something,
> >>>>>but this worked fine in the past (and I think we should not break
> >>>>>this, as this is an essential feature).
> >>>>
> >>>>OK, I missed something too.  I think this gets better now once I merge
> >>>>Simon's SPL series as we do all of this from board_init_f() and the
> >>>>siemens code should just work again.
> >>>
> >>>Yes, just saw your patch ;-)
> >>>
> >>>If they are in mainline (or do you have them somewhere in a git repo?),
> >>>I test it again on the dxr2 board, thanks!
> >>
> >>If I am correct, all needed patches from you are in mainline, just
> >>tried this on the dxr2 board ... but I still need:
> >>
> >>diff --git a/board/siemens/common/board.c b/board/siemens/common/board.c
> >>index a39cbd0..8724604 100644
> >>--- a/board/siemens/common/board.c
> >>+++ b/board/siemens/common/board.c
> >>@@ -40,6 +40,11 @@ void set_uart_mux_conf(void)
> >>
> >>  void set_mux_conf_regs(void)
> >>  {
> >>+       /* enable early the console */
> >>+       gd->baudrate = CONFIG_BAUDRATE;
> >>+       serial_init();
> >>+       gd->have_console = 1;
> >>+
> >>         /* Initalize the board header */
> >>         enable_i2c0_pin_mux();
> >>         i2c_set_bus_num(0);
> >>
> >>to see the console output ...
> >
> >Yes, if you want console prior to spl_board_init() then you need to do
> >that where you are.
> >
> 
> Ah, ok... so this patch is OK?

If you must print out the DDR timings, yes, this is normal.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150306/0e38272d/attachment.sig>

[U-Boot] [PATCH] am33xx, spl, siemens: enable debug uart output again

[Heiko Schocher]

Hello Tom,

Am 06.03.2015 08:24, schrieb Heiko Schocher:
> Hello Tom,
>
> Am 05.03.2015 15:50, schrieb Tom Rini:
>> On Thu, Mar 05, 2015 at 09:46:50AM +0100, Heiko Schocher wrote:
>>> Hello Tom,
>>>
>>> Am 05.03.2015 07:22, schrieb Heiko Schocher:
>>>> Hello Tom,
>>>>
>>>> Am 04.03.2015 17:40, schrieb Tom Rini:
>>>>> On Wed, Mar 04, 2015 at 08:42:58AM +0100, Heiko Schocher wrote:
>>>>>> Hello Tom,
>>>>>>
>>>>>> Am 02.03.2015 14:59, schrieb Tom Rini:
>>>>>>> On Mon, Mar 02, 2015 at 07:56:41AM +0100, Heiko Schocher wrote:
>>>>>>>> Hello Simon,
>>>>>>>>
>>>>>>>> Am 24.02.2015 14:31, schrieb Simon Glass:
>>>>>>>>> Hi Heiko,
>>>>>>>>>
>>>>>>>>> On 23 February 2015 at 23:18, Heiko Schocher <hs@denx.de> wrote:
>>>>>>>>>> a6b541b090: TI ARMv7: Don't use GD before crt0.S has set it
>>>>>>>>>>
>>>>>>>>>> moves the init of the debug uart at the very end of SPL code.
>>>>>>>>>> Enable it for the siemens board earlier, as they print
>>>>>>>>>> ddr settings ... all debug output before board_init_r()
>>>>>>>>>> is here currently useless. Maybe we must rework this
>>>>>>>>>> globally?
>>>>>>>>>
>>>>>>>>> Assuming we are talking about U-Boot proper, the DDR init should
>>>>>>>>> happen in board_init_f(), specifically dram_init(). so I think this
>>>>>>>>> code should be updated.
>>>>>>>>>
>>>>>>>>> If it is SPL, then DDR init should happen in SPL's board_init_f().
>>>>>>>>
>>>>>>>> It is in SPL...
>>>>>>>>
>>>>>>>> sdram_init() is called from:
>>>>>>>>
>>>>>>>> ./arch/arm/cpu/armv7/am33xx/board.c from s_init() ...
>>>>>>>>
>>>>>>>>> I sent a series a few weeks ago (available at u-boot-dm branch
>>>>>>>>> spl-working) related to this topic:
>>>>>>>>>
>>>>>>>>> http://patchwork.ozlabs.org/patch/438581/
>>>>>>>>
>>>>>>>> Ah ... Hmm... so "./arch/arm/cpu/armv7/am33xx/board.c" needs
>>>>>>>> a rework, right?
>>>>>>>>
>>>>>>>> Is a simple rename s_init() -> board_init_f() correct?
>>>>>>>
>>>>>>> Right so, no, we can't just rename s_init to board_init_f.  This is what
>>>>>>> I was talking about in the thread about the function Hans wants to add
>>>>>>> to enable some bits in CP15 on sunxi, iirc.
>>>>>>>
>>>>>>> In short, armv7 has a different set of abstraction hooks than the
>>>>>>> previous ARM cores (armv8 followed what we have for v7) and I'm not
>>>>>>> convinced in the end that it really won us anything.  See
>>>>>>> http://lists.denx.de/pipermail/u-boot/2015-January/202350.html
>>>>>>>
>>>>>>> For today you need to rework the Siemens code to print out the DDR
>>>>>>> values (when desired) in spl_board_init() as we do not, or will not
>>>>>>> shortly, have gd prior to board_init_f running.
>>>>>>
>>>>>> Hmm... first I thought, ok, no problem, move the output from the RAM
>>>>>> parameters to spl_board_init() ... but thats only the half of the
>>>>>> story ... They read the RAM parameters from an i2c eeprom, and if
>>>>>> there are errors, they print this errors ... currently this does
>>>>>> not work, and thats I think the more important case ... and I could
>>>>>> not move this error printfs to somewhere, because if RAM is not
>>>>>> working ... there is no later ...
>>>>>>
>>>>>> So I have to enable the console early ... maybe I missed something,
>>>>>> but this worked fine in the past (and I think we should not break
>>>>>> this, as this is an essential feature).
>>>>>
>>>>> OK, I missed something too.  I think this gets better now once I merge
>>>>> Simon's SPL series as we do all of this from board_init_f() and the
>>>>> siemens code should just work again.
>>>>
>>>> Yes, just saw your patch ;-)
>>>>
>>>> If they are in mainline (or do you have them somewhere in a git repo?),
>>>> I test it again on the dxr2 board, thanks!
>>>
>>> If I am correct, all needed patches from you are in mainline, just
>>> tried this on the dxr2 board ... but I still need:
>>>
>>> diff --git a/board/siemens/common/board.c b/board/siemens/common/board.c
>>> index a39cbd0..8724604 100644
>>> --- a/board/siemens/common/board.c
>>> +++ b/board/siemens/common/board.c
>>> @@ -40,6 +40,11 @@ void set_uart_mux_conf(void)
>>>
>>>   void set_mux_conf_regs(void)
>>>   {
>>> +       /* enable early the console */
>>> +       gd->baudrate = CONFIG_BAUDRATE;
>>> +       serial_init();
>>> +       gd->have_console = 1;
>>> +
>>>          /* Initalize the board header */
>>>          enable_i2c0_pin_mux();
>>>          i2c_set_bus_num(0);
>>>
>>> to see the console output ...
>>
>> Yes, if you want console prior to spl_board_init() then you need to do
>> that where you are.
>>
>
> Ah, ok... so this patch is OK?

Just found this thread again, as I want to post updates for the am335x
based siemens boards ... Is this patch [1] Ok, and can get applied?

Thanks!

bye,
Heiko
[1] Patchwork [U-Boot] am33xx, spl, siemens: enable debug uart output again
https://patchwork.ozlabs.org/patch/446644/

Hups... found this patch in patchwork marked as RFC and Delegated
to me ... ?
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH] am33xx, spl, siemens: enable debug uart output again

[Tom Rini]

On Thu, May 14, 2015 at 12:37:40PM +0200, Heiko Schocher wrote:
> Hello Tom,
> 
> Am 06.03.2015 08:24, schrieb Heiko Schocher:
> >Hello Tom,
> >
> >Am 05.03.2015 15:50, schrieb Tom Rini:
> >>On Thu, Mar 05, 2015 at 09:46:50AM +0100, Heiko Schocher wrote:
> >>>Hello Tom,
> >>>
> >>>Am 05.03.2015 07:22, schrieb Heiko Schocher:
> >>>>Hello Tom,
> >>>>
> >>>>Am 04.03.2015 17:40, schrieb Tom Rini:
> >>>>>On Wed, Mar 04, 2015 at 08:42:58AM +0100, Heiko Schocher wrote:
> >>>>>>Hello Tom,
> >>>>>>
> >>>>>>Am 02.03.2015 14:59, schrieb Tom Rini:
> >>>>>>>On Mon, Mar 02, 2015 at 07:56:41AM +0100, Heiko Schocher wrote:
> >>>>>>>>Hello Simon,
> >>>>>>>>
> >>>>>>>>Am 24.02.2015 14:31, schrieb Simon Glass:
> >>>>>>>>>Hi Heiko,
> >>>>>>>>>
> >>>>>>>>>On 23 February 2015 at 23:18, Heiko Schocher <hs@denx.de> wrote:
> >>>>>>>>>>a6b541b090: TI ARMv7: Don't use GD before crt0.S has set it
> >>>>>>>>>>
> >>>>>>>>>>moves the init of the debug uart at the very end of SPL code.
> >>>>>>>>>>Enable it for the siemens board earlier, as they print
> >>>>>>>>>>ddr settings ... all debug output before board_init_r()
> >>>>>>>>>>is here currently useless. Maybe we must rework this
> >>>>>>>>>>globally?
> >>>>>>>>>
> >>>>>>>>>Assuming we are talking about U-Boot proper, the DDR init should
> >>>>>>>>>happen in board_init_f(), specifically dram_init(). so I think this
> >>>>>>>>>code should be updated.
> >>>>>>>>>
> >>>>>>>>>If it is SPL, then DDR init should happen in SPL's board_init_f().
> >>>>>>>>
> >>>>>>>>It is in SPL...
> >>>>>>>>
> >>>>>>>>sdram_init() is called from:
> >>>>>>>>
> >>>>>>>>./arch/arm/cpu/armv7/am33xx/board.c from s_init() ...
> >>>>>>>>
> >>>>>>>>>I sent a series a few weeks ago (available at u-boot-dm branch
> >>>>>>>>>spl-working) related to this topic:
> >>>>>>>>>
> >>>>>>>>>http://patchwork.ozlabs.org/patch/438581/
> >>>>>>>>
> >>>>>>>>Ah ... Hmm... so "./arch/arm/cpu/armv7/am33xx/board.c" needs
> >>>>>>>>a rework, right?
> >>>>>>>>
> >>>>>>>>Is a simple rename s_init() -> board_init_f() correct?
> >>>>>>>
> >>>>>>>Right so, no, we can't just rename s_init to board_init_f.  This is what
> >>>>>>>I was talking about in the thread about the function Hans wants to add
> >>>>>>>to enable some bits in CP15 on sunxi, iirc.
> >>>>>>>
> >>>>>>>In short, armv7 has a different set of abstraction hooks than the
> >>>>>>>previous ARM cores (armv8 followed what we have for v7) and I'm not
> >>>>>>>convinced in the end that it really won us anything.  See
> >>>>>>>http://lists.denx.de/pipermail/u-boot/2015-January/202350.html
> >>>>>>>
> >>>>>>>For today you need to rework the Siemens code to print out the DDR
> >>>>>>>values (when desired) in spl_board_init() as we do not, or will not
> >>>>>>>shortly, have gd prior to board_init_f running.
> >>>>>>
> >>>>>>Hmm... first I thought, ok, no problem, move the output from the RAM
> >>>>>>parameters to spl_board_init() ... but thats only the half of the
> >>>>>>story ... They read the RAM parameters from an i2c eeprom, and if
> >>>>>>there are errors, they print this errors ... currently this does
> >>>>>>not work, and thats I think the more important case ... and I could
> >>>>>>not move this error printfs to somewhere, because if RAM is not
> >>>>>>working ... there is no later ...
> >>>>>>
> >>>>>>So I have to enable the console early ... maybe I missed something,
> >>>>>>but this worked fine in the past (and I think we should not break
> >>>>>>this, as this is an essential feature).
> >>>>>
> >>>>>OK, I missed something too.  I think this gets better now once I merge
> >>>>>Simon's SPL series as we do all of this from board_init_f() and the
> >>>>>siemens code should just work again.
> >>>>
> >>>>Yes, just saw your patch ;-)
> >>>>
> >>>>If they are in mainline (or do you have them somewhere in a git repo?),
> >>>>I test it again on the dxr2 board, thanks!
> >>>
> >>>If I am correct, all needed patches from you are in mainline, just
> >>>tried this on the dxr2 board ... but I still need:
> >>>
> >>>diff --git a/board/siemens/common/board.c b/board/siemens/common/board.c
> >>>index a39cbd0..8724604 100644
> >>>--- a/board/siemens/common/board.c
> >>>+++ b/board/siemens/common/board.c
> >>>@@ -40,6 +40,11 @@ void set_uart_mux_conf(void)
> >>>
> >>>  void set_mux_conf_regs(void)
> >>>  {
> >>>+       /* enable early the console */
> >>>+       gd->baudrate = CONFIG_BAUDRATE;
> >>>+       serial_init();
> >>>+       gd->have_console = 1;
> >>>+
> >>>         /* Initalize the board header */
> >>>         enable_i2c0_pin_mux();
> >>>         i2c_set_bus_num(0);
> >>>
> >>>to see the console output ...
> >>
> >>Yes, if you want console prior to spl_board_init() then you need to do
> >>that where you are.
> >>
> >
> >Ah, ok... so this patch is OK?
> 
> Just found this thread again, as I want to post updates for the am335x
> based siemens boards ... Is this patch [1] Ok, and can get applied?
> 
> Thanks!
> 
> bye,
> Heiko
> [1] Patchwork [U-Boot] am33xx, spl, siemens: enable debug uart output again
> https://patchwork.ozlabs.org/patch/446644/
> 
> Hups... found this patch in patchwork marked as RFC and Delegated
> to me ... ?

I'll look it over again then, thanks.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150514/9191b5ad/attachment.sig>

[U-Boot] [PATCH] am33xx, spl, siemens: enable debug uart output again

[Simon Glass]

Hi Heiko,

On 14 May 2015 at 04:51, Tom Rini <trini@konsulko.com> wrote:
> On Thu, May 14, 2015 at 12:37:40PM +0200, Heiko Schocher wrote:
>> Hello Tom,
>>
>> Am 06.03.2015 08:24, schrieb Heiko Schocher:
>> >Hello Tom,
>> >
>> >Am 05.03.2015 15:50, schrieb Tom Rini:
>> >>On Thu, Mar 05, 2015 at 09:46:50AM +0100, Heiko Schocher wrote:
>> >>>Hello Tom,
>> >>>
>> >>>Am 05.03.2015 07:22, schrieb Heiko Schocher:
>> >>>>Hello Tom,
>> >>>>
>> >>>>Am 04.03.2015 17:40, schrieb Tom Rini:
>> >>>>>On Wed, Mar 04, 2015 at 08:42:58AM +0100, Heiko Schocher wrote:
>> >>>>>>Hello Tom,
>> >>>>>>
>> >>>>>>Am 02.03.2015 14:59, schrieb Tom Rini:
>> >>>>>>>On Mon, Mar 02, 2015 at 07:56:41AM +0100, Heiko Schocher wrote:
>> >>>>>>>>Hello Simon,
>> >>>>>>>>
>> >>>>>>>>Am 24.02.2015 14:31, schrieb Simon Glass:
>> >>>>>>>>>Hi Heiko,
>> >>>>>>>>>
>> >>>>>>>>>On 23 February 2015 at 23:18, Heiko Schocher <hs@denx.de> wrote:
>> >>>>>>>>>>a6b541b090: TI ARMv7: Don't use GD before crt0.S has set it
>> >>>>>>>>>>
>> >>>>>>>>>>moves the init of the debug uart at the very end of SPL code.
>> >>>>>>>>>>Enable it for the siemens board earlier, as they print
>> >>>>>>>>>>ddr settings ... all debug output before board_init_r()
>> >>>>>>>>>>is here currently useless. Maybe we must rework this
>> >>>>>>>>>>globally?
>> >>>>>>>>>
>> >>>>>>>>>Assuming we are talking about U-Boot proper, the DDR init should
>> >>>>>>>>>happen in board_init_f(), specifically dram_init(). so I think this
>> >>>>>>>>>code should be updated.
>> >>>>>>>>>
>> >>>>>>>>>If it is SPL, then DDR init should happen in SPL's board_init_f().
>> >>>>>>>>
>> >>>>>>>>It is in SPL...
>> >>>>>>>>
>> >>>>>>>>sdram_init() is called from:
>> >>>>>>>>
>> >>>>>>>>./arch/arm/cpu/armv7/am33xx/board.c from s_init() ...
>> >>>>>>>>
>> >>>>>>>>>I sent a series a few weeks ago (available at u-boot-dm branch
>> >>>>>>>>>spl-working) related to this topic:
>> >>>>>>>>>
>> >>>>>>>>>http://patchwork.ozlabs.org/patch/438581/
>> >>>>>>>>
>> >>>>>>>>Ah ... Hmm... so "./arch/arm/cpu/armv7/am33xx/board.c" needs
>> >>>>>>>>a rework, right?
>> >>>>>>>>
>> >>>>>>>>Is a simple rename s_init() -> board_init_f() correct?
>> >>>>>>>
>> >>>>>>>Right so, no, we can't just rename s_init to board_init_f.  This is what
>> >>>>>>>I was talking about in the thread about the function Hans wants to add
>> >>>>>>>to enable some bits in CP15 on sunxi, iirc.
>> >>>>>>>
>> >>>>>>>In short, armv7 has a different set of abstraction hooks than the
>> >>>>>>>previous ARM cores (armv8 followed what we have for v7) and I'm not
>> >>>>>>>convinced in the end that it really won us anything.  See
>> >>>>>>>http://lists.denx.de/pipermail/u-boot/2015-January/202350.html
>> >>>>>>>
>> >>>>>>>For today you need to rework the Siemens code to print out the DDR
>> >>>>>>>values (when desired) in spl_board_init() as we do not, or will not
>> >>>>>>>shortly, have gd prior to board_init_f running.
>> >>>>>>
>> >>>>>>Hmm... first I thought, ok, no problem, move the output from the RAM
>> >>>>>>parameters to spl_board_init() ... but thats only the half of the
>> >>>>>>story ... They read the RAM parameters from an i2c eeprom, and if
>> >>>>>>there are errors, they print this errors ... currently this does
>> >>>>>>not work, and thats I think the more important case ... and I could
>> >>>>>>not move this error printfs to somewhere, because if RAM is not
>> >>>>>>working ... there is no later ...
>> >>>>>>
>> >>>>>>So I have to enable the console early ... maybe I missed something,
>> >>>>>>but this worked fine in the past (and I think we should not break
>> >>>>>>this, as this is an essential feature).
>> >>>>>
>> >>>>>OK, I missed something too.  I think this gets better now once I merge
>> >>>>>Simon's SPL series as we do all of this from board_init_f() and the
>> >>>>>siemens code should just work again.
>> >>>>
>> >>>>Yes, just saw your patch ;-)
>> >>>>
>> >>>>If they are in mainline (or do you have them somewhere in a git repo?),
>> >>>>I test it again on the dxr2 board, thanks!
>> >>>
>> >>>If I am correct, all needed patches from you are in mainline, just
>> >>>tried this on the dxr2 board ... but I still need:
>> >>>
>> >>>diff --git a/board/siemens/common/board.c b/board/siemens/common/board.c
>> >>>index a39cbd0..8724604 100644
>> >>>--- a/board/siemens/common/board.c
>> >>>+++ b/board/siemens/common/board.c
>> >>>@@ -40,6 +40,11 @@ void set_uart_mux_conf(void)
>> >>>
>> >>>  void set_mux_conf_regs(void)
>> >>>  {
>> >>>+       /* enable early the console */
>> >>>+       gd->baudrate = CONFIG_BAUDRATE;
>> >>>+       serial_init();
>> >>>+       gd->have_console = 1;
>> >>>+
>> >>>         /* Initalize the board header */
>> >>>         enable_i2c0_pin_mux();
>> >>>         i2c_set_bus_num(0);
>> >>>
>> >>>to see the console output ...
>> >>
>> >>Yes, if you want console prior to spl_board_init() then you need to do
>> >>that where you are.
>> >>
>> >
>> >Ah, ok... so this patch is OK?
>>
>> Just found this thread again, as I want to post updates for the am335x
>> based siemens boards ... Is this patch [1] Ok, and can get applied?
>>
>> Thanks!
>>
>> bye,
>> Heiko
>> [1] Patchwork [U-Boot] am33xx, spl, siemens: enable debug uart output again
>> https://patchwork.ozlabs.org/patch/446644/
>>
>> Hups... found this patch in patchwork marked as RFC and Delegated
>> to me ... ?
>
> I'll look it over again then, thanks.

It's a bit confusing - am I right in thinking that this is called from
board_early_init_f() in arch/arm/cpu/armv7/am33xx/board.c? That in
turn is called from board_init_f() in the same file?

If so it seems OK, although I wonder if the SPL console setup could
happen in board_early_init_f() or even board_init_f() rather than
being buried so deep? It seems odd that setting up the mux registers
should enable the console, and I doubt people will find it easily.

Regards,
Simon

[U-Boot] [PATCH] am33xx, spl, siemens: enable debug uart output again

[Heiko Schocher]

Hello Simon,

Am 14.05.2015 14:46, schrieb Simon Glass:
> Hi Heiko,
>
> On 14 May 2015 at 04:51, Tom Rini <trini@konsulko.com> wrote:
>> On Thu, May 14, 2015 at 12:37:40PM +0200, Heiko Schocher wrote:
>>> Hello Tom,
>>>
>>> Am 06.03.2015 08:24, schrieb Heiko Schocher:
>>>> Hello Tom,
>>>>
>>>> Am 05.03.2015 15:50, schrieb Tom Rini:
>>>>> On Thu, Mar 05, 2015 at 09:46:50AM +0100, Heiko Schocher wrote:
>>>>>> Hello Tom,
>>>>>>
>>>>>> Am 05.03.2015 07:22, schrieb Heiko Schocher:
>>>>>>> Hello Tom,
>>>>>>>
>>>>>>> Am 04.03.2015 17:40, schrieb Tom Rini:
>>>>>>>> On Wed, Mar 04, 2015 at 08:42:58AM +0100, Heiko Schocher wrote:
>>>>>>>>> Hello Tom,
>>>>>>>>>
>>>>>>>>> Am 02.03.2015 14:59, schrieb Tom Rini:
>>>>>>>>>> On Mon, Mar 02, 2015 at 07:56:41AM +0100, Heiko Schocher wrote:
>>>>>>>>>>> Hello Simon,
>>>>>>>>>>>
>>>>>>>>>>> Am 24.02.2015 14:31, schrieb Simon Glass:
>>>>>>>>>>>> Hi Heiko,
>>>>>>>>>>>>
>>>>>>>>>>>> On 23 February 2015 at 23:18, Heiko Schocher <hs@denx.de> wrote:
>>>>>>>>>>>>> a6b541b090: TI ARMv7: Don't use GD before crt0.S has set it
>>>>>>>>>>>>>
>>>>>>>>>>>>> moves the init of the debug uart at the very end of SPL code.
>>>>>>>>>>>>> Enable it for the siemens board earlier, as they print
>>>>>>>>>>>>> ddr settings ... all debug output before board_init_r()
>>>>>>>>>>>>> is here currently useless. Maybe we must rework this
>>>>>>>>>>>>> globally?
>>>>>>>>>>>>
>>>>>>>>>>>> Assuming we are talking about U-Boot proper, the DDR init should
>>>>>>>>>>>> happen in board_init_f(), specifically dram_init(). so I think this
>>>>>>>>>>>> code should be updated.
>>>>>>>>>>>>
>>>>>>>>>>>> If it is SPL, then DDR init should happen in SPL's board_init_f().
>>>>>>>>>>>
>>>>>>>>>>> It is in SPL...
>>>>>>>>>>>
>>>>>>>>>>> sdram_init() is called from:
>>>>>>>>>>>
>>>>>>>>>>> ./arch/arm/cpu/armv7/am33xx/board.c from s_init() ...
>>>>>>>>>>>
>>>>>>>>>>>> I sent a series a few weeks ago (available at u-boot-dm branch
>>>>>>>>>>>> spl-working) related to this topic:
>>>>>>>>>>>>
>>>>>>>>>>>> http://patchwork.ozlabs.org/patch/438581/
>>>>>>>>>>>
>>>>>>>>>>> Ah ... Hmm... so "./arch/arm/cpu/armv7/am33xx/board.c" needs
>>>>>>>>>>> a rework, right?
>>>>>>>>>>>
>>>>>>>>>>> Is a simple rename s_init() -> board_init_f() correct?
>>>>>>>>>>
>>>>>>>>>> Right so, no, we can't just rename s_init to board_init_f.  This is what
>>>>>>>>>> I was talking about in the thread about the function Hans wants to add
>>>>>>>>>> to enable some bits in CP15 on sunxi, iirc.
>>>>>>>>>>
>>>>>>>>>> In short, armv7 has a different set of abstraction hooks than the
>>>>>>>>>> previous ARM cores (armv8 followed what we have for v7) and I'm not
>>>>>>>>>> convinced in the end that it really won us anything.  See
>>>>>>>>>> http://lists.denx.de/pipermail/u-boot/2015-January/202350.html
>>>>>>>>>>
>>>>>>>>>> For today you need to rework the Siemens code to print out the DDR
>>>>>>>>>> values (when desired) in spl_board_init() as we do not, or will not
>>>>>>>>>> shortly, have gd prior to board_init_f running.
>>>>>>>>>
>>>>>>>>> Hmm... first I thought, ok, no problem, move the output from the RAM
>>>>>>>>> parameters to spl_board_init() ... but thats only the half of the
>>>>>>>>> story ... They read the RAM parameters from an i2c eeprom, and if
>>>>>>>>> there are errors, they print this errors ... currently this does
>>>>>>>>> not work, and thats I think the more important case ... and I could
>>>>>>>>> not move this error printfs to somewhere, because if RAM is not
>>>>>>>>> working ... there is no later ...
>>>>>>>>>
>>>>>>>>> So I have to enable the console early ... maybe I missed something,
>>>>>>>>> but this worked fine in the past (and I think we should not break
>>>>>>>>> this, as this is an essential feature).
>>>>>>>>
>>>>>>>> OK, I missed something too.  I think this gets better now once I merge
>>>>>>>> Simon's SPL series as we do all of this from board_init_f() and the
>>>>>>>> siemens code should just work again.
>>>>>>>
>>>>>>> Yes, just saw your patch ;-)
>>>>>>>
>>>>>>> If they are in mainline (or do you have them somewhere in a git repo?),
>>>>>>> I test it again on the dxr2 board, thanks!
>>>>>>
>>>>>> If I am correct, all needed patches from you are in mainline, just
>>>>>> tried this on the dxr2 board ... but I still need:
>>>>>>
>>>>>> diff --git a/board/siemens/common/board.c b/board/siemens/common/board.c
>>>>>> index a39cbd0..8724604 100644
>>>>>> --- a/board/siemens/common/board.c
>>>>>> +++ b/board/siemens/common/board.c
>>>>>> @@ -40,6 +40,11 @@ void set_uart_mux_conf(void)
>>>>>>
>>>>>>   void set_mux_conf_regs(void)
>>>>>>   {
>>>>>> +       /* enable early the console */
>>>>>> +       gd->baudrate = CONFIG_BAUDRATE;
>>>>>> +       serial_init();
>>>>>> +       gd->have_console = 1;
>>>>>> +
>>>>>>          /* Initalize the board header */
>>>>>>          enable_i2c0_pin_mux();
>>>>>>          i2c_set_bus_num(0);
>>>>>>
>>>>>> to see the console output ...
>>>>>
>>>>> Yes, if you want console prior to spl_board_init() then you need to do
>>>>> that where you are.
>>>>>
>>>>
>>>> Ah, ok... so this patch is OK?
>>>
>>> Just found this thread again, as I want to post updates for the am335x
>>> based siemens boards ... Is this patch [1] Ok, and can get applied?
>>>
>>> Thanks!
>>>
>>> bye,
>>> Heiko
>>> [1] Patchwork [U-Boot] am33xx, spl, siemens: enable debug uart output again
>>> https://patchwork.ozlabs.org/patch/446644/
>>>
>>> Hups... found this patch in patchwork marked as RFC and Delegated
>>> to me ... ?
>>
>> I'll look it over again then, thanks.
>
> It's a bit confusing - am I right in thinking that this is called from
> board_early_init_f() in arch/arm/cpu/armv7/am33xx/board.c? That in
> turn is called from board_init_f() in the same file?

I think so, yes (Could not currently power on the board, so I look
in it, if I have it again accessable)

> If so it seems OK, although I wonder if the SPL console setup could
> happen in board_early_init_f() or even board_init_f() rather than
> being buried so deep? It seems odd that setting up the mux registers
> should enable the console, and I doubt people will find it easily.

Yes, board_early_init_f() seems a better place for me too, but this
affects then all am335x boards (or we add a weak function, which
boards can define)?

since commit:

a6b541b09022: TI ARMv7: Don't use GD before crt0.S has set it

early debug output is not longer shown on the siemens am335x boards,
and this patch fixes this issue ... I cannot say, if we need this
for all am335x boards ...

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany