Message ID | 54F817FA.7070506@denx.de |
---|---|
State | Superseded |
Delegated to: | Tom Rini |
Headers | show |
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.
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
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.
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 ... ?
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.
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
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
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);