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

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