* [U-Boot] [PATCH v2 1/3] board_r: move initr_watchdog to be called after initr_serial
@ 2019-05-16 9:19 Weijie Gao
2019-06-29 6:09 ` [U-Boot] [PATCH v2 1/3] board_r: move initr_watchdog to be called, " Suniel Mahesh
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Weijie Gao @ 2019-05-16 9:19 UTC (permalink / raw)
To: u-boot
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_watchdog after initr_serial to solve this issue.
Cc: Stefan Roese <sr@denx.de>
Reviewed-by: Ryder Lee <ryder.lee@mediatek.com>
Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>
---
Changes since v1: move initr_watchdog instead of initr_serial
---
common/board_r.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/common/board_r.c b/common/board_r.c
index 150e8cd424..df24021f2c 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -678,9 +678,6 @@ static init_fnc_t init_sequence_r[] = {
#ifdef CONFIG_DM
initr_dm,
#endif
-#if defined(CONFIG_WDT)
- initr_watchdog,
-#endif
#if defined(CONFIG_ARM) || defined(CONFIG_NDS32) || defined(CONFIG_RISCV) || \
defined(CONFIG_SANDBOX)
board_init, /* Setup chipselects */
@@ -700,6 +697,9 @@ static init_fnc_t init_sequence_r[] = {
stdio_init_tables,
initr_serial,
initr_announce,
+#if defined(CONFIG_WDT)
+ initr_watchdog,
+#endif
INIT_FUNC_WATCHDOG_RESET
#ifdef CONFIG_NEEDS_MANUAL_RELOC
initr_manual_reloc_cmdtable,
--
2.18.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH v2 1/3] board_r: move initr_watchdog to be called, after initr_serial
2019-05-16 9:19 [U-Boot] [PATCH v2 1/3] board_r: move initr_watchdog to be called after initr_serial Weijie Gao
@ 2019-06-29 6:09 ` Suniel Mahesh
2019-06-29 8:34 ` [U-Boot] [PATCH v2 1/3] board_r: move initr_watchdog to be called " Stefan Roese
2019-07-08 1:15 ` Tom Rini
2 siblings, 0 replies; 9+ messages in thread
From: Suniel Mahesh @ 2019-06-29 6:09 UTC (permalink / raw)
To: u-boot
Hi,
May I know if this patch is still under review ?
I have converted OMAP3 watchdog driver into driver model for TI AM33XX based SOC and tested corresponding
patches on BeagleBone Black board.
Build was successful but board goes into a loop when turned on/runtime, problem identified and is in sync
with Weijie Gao's explanation and his patch for such behaviour (please follow this thread for explanation).
When initr_watchdog is moved/placed just after initr_serial, board is behaving normal. Tested on BeagleBone Black.
Please suggest if there are any workarounds for this problem.
Regards
--
Suniel Mahesh
Embedded Linux, Kernel & U-Boot engineer
https://github.com/sunielmahesh
www.tuxtrons.com
https://github.com/techveda
Hyderabad, India
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH v2 1/3] board_r: move initr_watchdog to be called after initr_serial
2019-05-16 9:19 [U-Boot] [PATCH v2 1/3] board_r: move initr_watchdog to be called after initr_serial Weijie Gao
2019-06-29 6:09 ` [U-Boot] [PATCH v2 1/3] board_r: move initr_watchdog to be called, " Suniel Mahesh
@ 2019-06-29 8:34 ` Stefan Roese
2019-06-29 8:44 ` Frank Wunderlich
2019-07-08 1:15 ` Tom Rini
2 siblings, 1 reply; 9+ messages in thread
From: Stefan Roese @ 2019-06-29 8:34 UTC (permalink / raw)
To: u-boot
Cc'ing a few people
On 16.05.19 11:19, 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_watchdog after initr_serial to solve this issue.
>
> Cc: Stefan Roese <sr@denx.de>
> Reviewed-by: Ryder Lee <ryder.lee@mediatek.com>
> Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>
> ---
> Changes since v1: move initr_watchdog instead of initr_serial
> ---
> common/board_r.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/common/board_r.c b/common/board_r.c
> index 150e8cd424..df24021f2c 100644
> --- a/common/board_r.c
> +++ b/common/board_r.c
> @@ -678,9 +678,6 @@ static init_fnc_t init_sequence_r[] = {
> #ifdef CONFIG_DM
> initr_dm,
> #endif
> -#if defined(CONFIG_WDT)
> - initr_watchdog,
> -#endif
> #if defined(CONFIG_ARM) || defined(CONFIG_NDS32) || defined(CONFIG_RISCV) || \
> defined(CONFIG_SANDBOX)
> board_init, /* Setup chipselects */
> @@ -700,6 +697,9 @@ static init_fnc_t init_sequence_r[] = {
> stdio_init_tables,
> initr_serial,
> initr_announce,
> +#if defined(CONFIG_WDT)
> + initr_watchdog,
> +#endif
> INIT_FUNC_WATCHDOG_RESET
> #ifdef CONFIG_NEEDS_MANUAL_RELOC
> initr_manual_reloc_cmdtable,
Reviewed-by: Stefan Roese <sr@denx.de>
Frank, does this patch fix the watchdog issue on your board as well?
Tom, could you please apply this patch to the upcoming release?
Thanks,
Stefan
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH v2 1/3] board_r: move initr_watchdog to be called after initr_serial
2019-06-29 8:34 ` [U-Boot] [PATCH v2 1/3] board_r: move initr_watchdog to be called " Stefan Roese
@ 2019-06-29 8:44 ` Frank Wunderlich
2019-06-29 8:49 ` Stefan Roese
0 siblings, 1 reply; 9+ messages in thread
From: Frank Wunderlich @ 2019-06-29 8:44 UTC (permalink / raw)
To: u-boot
Hi Stefan/Tom
this Patch in combination with https://patchwork.ozlabs.org/patch/1113120/ seem to fix issues on bananapi-r2 /mt7623 (bootloop/hang on WDT servicing) if CONFIG_WATCHDOG is not set to "n" (else it will be set to y because of new "imply WATCHDOG" inside of "config WDT").
@tom can you please include this 2 Patches in master?
regards Frank
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH v2 1/3] board_r: move initr_watchdog to be called after initr_serial
2019-06-29 8:44 ` Frank Wunderlich
@ 2019-06-29 8:49 ` Stefan Roese
2019-07-07 6:54 ` Frank Wunderlich
2019-07-07 7:46 ` Frank Wunderlich
0 siblings, 2 replies; 9+ messages in thread
From: Stefan Roese @ 2019-06-29 8:49 UTC (permalink / raw)
To: u-boot
Hi Frank,
On 29.06.19 10:44, Frank Wunderlich wrote:
> Hi Stefan/Tom
>
> this Patch in combination with https://patchwork.ozlabs.org/patch/1113120/
This one already is applied in mainline:
https://gitlab.denx.de/u-boot/u-boot/commit/2bc1821e8662f9ed67cc6eeb680148bb5c148379
Even though the coding style is pretty messed up. We should clean this
file up a bit.
Thanks,
Stefan
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH v2 1/3] board_r: move initr_watchdog to be called after initr_serial
2019-06-29 8:49 ` Stefan Roese
@ 2019-07-07 6:54 ` Frank Wunderlich
2019-07-07 7:46 ` Frank Wunderlich
1 sibling, 0 replies; 9+ messages in thread
From: Frank Wunderlich @ 2019-07-07 6:54 UTC (permalink / raw)
To: u-boot
Hi
It looks this fix is not yet merged. Tom can you add it before release? Without it bpi-r2 and maybe other boards are broken.
Regards Frank
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH v2 1/3] board_r: move initr_watchdog to be called after initr_serial
2019-06-29 8:49 ` Stefan Roese
2019-07-07 6:54 ` Frank Wunderlich
@ 2019-07-07 7:46 ` Frank Wunderlich
2019-07-07 8:32 ` Suniel Mahesh
1 sibling, 1 reply; 9+ messages in thread
From: Frank Wunderlich @ 2019-07-07 7:46 UTC (permalink / raw)
To: u-boot
Tested-by: Frank Wunderlich <frank-w@public-files.de>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH v2 1/3] board_r: move initr_watchdog to be called after initr_serial
2019-07-07 7:46 ` Frank Wunderlich
@ 2019-07-07 8:32 ` Suniel Mahesh
0 siblings, 0 replies; 9+ messages in thread
From: Suniel Mahesh @ 2019-07-07 8:32 UTC (permalink / raw)
To: u-boot
Agreed with frank. My patches for watchdog DM conversion depends on this change set.
tested on AM335x based Beaglebone Black
Tested-by: Suniel Mahesh <sunil.m@techveda.org>
Regards
--
Suniel Mahesh
Embedded Linux, Kernel & U-Boot engineer
https://github.com/sunielmahesh
www.tuxtrons.com
https://github.com/techveda
Hyderabad, India
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH v2 1/3] board_r: move initr_watchdog to be called after initr_serial
2019-05-16 9:19 [U-Boot] [PATCH v2 1/3] board_r: move initr_watchdog to be called after initr_serial Weijie Gao
2019-06-29 6:09 ` [U-Boot] [PATCH v2 1/3] board_r: move initr_watchdog to be called, " Suniel Mahesh
2019-06-29 8:34 ` [U-Boot] [PATCH v2 1/3] board_r: move initr_watchdog to be called " Stefan Roese
@ 2019-07-08 1:15 ` Tom Rini
2 siblings, 0 replies; 9+ messages in thread
From: Tom Rini @ 2019-07-08 1:15 UTC (permalink / raw)
To: u-boot
On Thu, May 16, 2019 at 05:19:13PM +0800, 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_watchdog after initr_serial to solve this issue.
>
> Cc: Stefan Roese <sr@denx.de>
> Reviewed-by: Ryder Lee <ryder.lee@mediatek.com>
> Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>
> Reviewed-by: Stefan Roese <sr@denx.de>
> Tested-by: Frank Wunderlich <frank-w@public-files.de>
> Tested-by: Suniel Mahesh <sunil.m@techveda.org>
Applied to u-boot/master, thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190707/e9beaa2b/attachment.sig>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-07-08 1:15 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-16 9:19 [U-Boot] [PATCH v2 1/3] board_r: move initr_watchdog to be called after initr_serial Weijie Gao
2019-06-29 6:09 ` [U-Boot] [PATCH v2 1/3] board_r: move initr_watchdog to be called, " Suniel Mahesh
2019-06-29 8:34 ` [U-Boot] [PATCH v2 1/3] board_r: move initr_watchdog to be called " Stefan Roese
2019-06-29 8:44 ` Frank Wunderlich
2019-06-29 8:49 ` Stefan Roese
2019-07-07 6:54 ` Frank Wunderlich
2019-07-07 7:46 ` Frank Wunderlich
2019-07-07 8:32 ` Suniel Mahesh
2019-07-08 1:15 ` Tom Rini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.