From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiko Schocher Date: Fri, 15 May 2015 07:23:42 +0200 Subject: [U-Boot] [PATCH] am33xx, spl, siemens: enable debug uart output again In-Reply-To: References: <54F409A9.7030907@denx.de> <20150302135932.GJ29891@bill-the-cat> <54F6B782.3000803@denx.de> <20150304164013.GU25373@bill-the-cat> <54F7F633.5080905@denx.de> <54F817FA.7070506@denx.de> <20150305145052.GQ25373@bill-the-cat> <54F9563F.8000000@denx.de> <55547AF4.4090503@denx.de> <20150514105132.GC5880@bill-the-cat> Message-ID: <555582DE.2070500@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hello Simon, Am 14.05.2015 14:46, schrieb Simon Glass: > Hi Heiko, > > On 14 May 2015 at 04:51, Tom Rini 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 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