On Thu, 2019-05-16 at 10:21 +0200, Stefan Roese wrote:
> On 16.05.19 08:48, Weijie Gao wrote:
> > The initr_watchdog is currently placed before initr_serial. The
> > initr_watchdog calls printf and printf finally calls ops->putc of a serial
> > driver.
> >
> > However, gd->cur_serial_dev points to a udevice allocated in board_f. The
> > gd->cur_serial_dev->driver->ops->putc points the the code region before
> > relocation.
> >
> > Some serial drivers call WATCHDOG_RESET() in ops->putc. When DM is enabled
> > for watchdog, watchdog_reset() is called. watchdog_reset() calls get_timer
> > to get current timer.
> >
> > On some platforms the timer driver is also a DM driver. initr_watchdog is
> > placed right after initr_dm, which means the timer driver hasn't been
> > initialized. So dm_timer_init() is called. To create a new udevice, calloc
> > is called.
> >
> > However start from ops->putc, u-boot execution flow is redirected into the
> > memory region before relocation (board_f). In board_f, dlmalloc hasn't
> > been initialized. The call to calloc will fail, and this will cause DM to
> > print out an error message, and it will call printf again, causing
> > recursive error outputs.
> >
> > This patch places initr_serial before initr_watchdog to solve this issue.
> >
> > Cc: Stefan Roese <s...@denx.de>
> > Reviewed-by: Ryder Lee <ryder....@mediatek.com>
> > Signed-off-by: Weijie Gao <weijie....@mediatek.com>
> > ---
> > common/board_r.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/common/board_r.c b/common/board_r.c
> > index 150e8cd424..a298146c2b 100644
> > --- a/common/board_r.c
> > +++ b/common/board_r.c
> > @@ -678,6 +678,7 @@ static init_fnc_t init_sequence_r[] = {
> > #ifdef CONFIG_DM
> > initr_dm,
> > #endif
> > + initr_serial,
> > #if defined(CONFIG_WDT)
> > initr_watchdog,
> > #endif
> > @@ -698,7 +699,6 @@ static init_fnc_t init_sequence_r[] = {
> > efi_memory_init,
> > #endif
> > stdio_init_tables,
> > - initr_serial,
> > initr_announce,
>
> I'm not 100% sure, if moving initr_serial before stdio_init_tables and
> other functions is safe. Perhaps its better to just move initr_watchdog
> down a bit, perhaps after initr_announce?
>
> What do you think?
>
> Thanks,
> Stefan
>
> BTW: Somehow your Cc'ing me did not reach me directly. I only found the
> patch on the list.
I'll change it to move initr_watchdog only.
My company's mail server reported delivery failure every time I Cc'ed
you and some others. I have no idea.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
