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

Fri May 15 07:23:42 CEST 2015

Hello Simon,

Am 14.05.2015 14:46, schrieb Simon Glass:
> Hi Heiko,
>
> On 14 May 2015 at 04:51, Tom Rini <trini at 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 at 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