Re: [PATCH v2 1/1] watchdog: realtek-otto: Change to use regmap API

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Calling `disable_irq()` in the watchdog restart handler can cause a system deadlock and sleeps in an atomic context during reboot.
--

commit 31e0a644ed16d3bca39871dd569420fedb50663e
Author: Rustam Adilov <adilov@disroot.org>

watchdog: realtek-otto: Change to use regmap API

This commit converts the Realtek Otto watchdog driver to use the regmap API
for register access instead of direct MMIO. It simplifies the code by
replacing read/modify/write sequences with regmap-specific functions like
regmap_update_bits.

> diff --git a/drivers/watchdog/realtek_otto_wdt.c b/drivers/watchdog/realtek_otto_wdt.c
> index 01b3ef89bacfc..504a7d379f4ae 100644
> --- a/drivers/watchdog/realtek_otto_wdt.c
> +++ b/drivers/watchdog/realtek_otto_wdt.c

[ ... ]

> @@ -193,7 +183,7 @@ static int otto_wdt_restart(struct watchdog_device *wdev, unsigned long reboot_m
>  
>  	/* Configure for shortest timeout and wait for reset to occur */
>  	v = FIELD_PREP(OTTO_WDT_CTRL_RST_MODE, reset_mode) | OTTO_WDT_CTRL_ENABLE;
> -	iowrite32(v, ctrl->base + OTTO_WDT_REG_CTRL);
> +	regmap_write(ctrl->regmap, OTTO_WDT_REG_CTRL, v);
>  
>  	mdelay(3 * otto_wdt_tick_ms(ctrl, 0));
>  

[Severity: High]
This isn't a bug introduced by this patch, but since you are modifying this
function, could the disable_irq() call earlier in otto_wdt_restart() cause a
deadlock during reboot?

Looking at the full otto_wdt_restart() function, it disables the interrupt
before triggering the hardware reset:

static int otto_wdt_restart(...)
{
    ...
    disable_irq(ctrl->irq_phase1);
    ...

Because otto_wdt_restart() acts as a system restart handler, it is executed
via machine_restart() in an atomic context with local interrupts disabled and
other CPUs halted.

Calling disable_irq() invokes synchronize_irq(), which waits for any
in-progress interrupt handler to complete. If the otto_wdt_phase1_isr handler
was running on another CPU that was just halted via smp_send_stop(),
synchronize_irq() will spin indefinitely, deadlocking the restart process.

Would it be safer to use disable_irq_nosync() here instead to avoid waiting?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519182329.24472-2-adilov@disroot.org?part=1