[PATCH] common: board_r: Initialize interrupts before watchdog

[Simon Glass]

Hi J,

On Tue, 18 Feb 2025 at 08:55, J. Neuschäfer via B4 Relay
<devnull+j.ne.posteo.net at kernel.org> wrote:
>
> From: "J. Neuschäfer" <j.ne at posteo.net>
>
> On some platforms, initializing the watchdog driver enables a timer
> interrupt. This of course requires the interrupt handlers to be
> properly initialized, otherwise U-Boot may crash or run the timer
> interrupt handler of a previous bootloader stage.
>
> To account for such systems, always initialize interrupts
> (arch_initr_trap) before the watchdog (initr_watchdog).
>
> This problem was observed on a PowerPC MPC83xx board.
>
> Signed-off-by: J. Neuschäfer <j.ne at posteo.net>
> ---
> NOTE: This approach seems safe and fine to me, but an argument could be
>       made that this should be fixed in the platform-specific drivers
>       instead.  Please let me know what you think.
>
>
> Rough stack trace (not sure if it should be part of the commit message):
>
>   initr_watchdog                (drivers/watchdog/wdt-uclass.c)
>     device_probe(wdt at 200)
>       device_probe(timer)
>         mpc8xxx_wdt_start       (drivers/watchdog/mpc8xxx_wdt.c)
>           set_msr(get_msr() | MSR_EE);
>
>   arch_initr_trap               (arch/powerpc/lib/traps.c)
>     trap_init                   (arch/powerpc/cpu/mpc83xx/start.S)
> ---
>  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 179259b00de81f7ba9802fc5288e7c2b6e6f381a..f711cd237ae76d80ca2413017bfe131656f44180 100644
> --- a/common/board_r.c
> +++ b/common/board_r.c
> @@ -652,11 +652,11 @@ static init_fnc_t init_sequence_r[] = {
>         serial_initialize,
>         initr_announce,
>         dm_announce,
> +       arch_initr_trap,
>  #if CONFIG_IS_ENABLED(WDT)
>         initr_watchdog,
>  #endif
>         INIT_FUNC_WATCHDOG_RESET
> -       arch_initr_trap,
>  #if defined(CONFIG_BOARD_EARLY_INIT_R)
>         board_early_init_r,
>  #endif
>
> ---
> base-commit: 064556910e61044f1295162ceaad600582b66cda
> change-id: 20250218-init-dab1fc72abd2
>
> Best regards,
> --
> J. Neuschäfer <j.ne at posteo.net>
>
>

The driver model way of doing this would be that your UCLASS_WDT
driver calls uclass_first_device(UCLASS_IRQ) to make sure interrupts
are ready.

The arch_initr_trap thing should probably not be used in new code.
Also, we have interrupt_init() which sounds like it is similar?

Regards,
Simon