[U-Boot] [PATCH 1/3] board_r: move initr_serial to be called before initr_watchdog

[Stefan Roese]

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 <sr at denx.de>
> Reviewed-by: Ryder Lee <ryder.lee at mediatek.com>
> Signed-off-by: Weijie Gao <weijie.gao at 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.