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

On 5/15/26 14:10, Sander Vanheule wrote:
> Adding back Guenter (and Wim) in Cc
> 
> On Fri, 2026-05-15 at 20:48 +0000, Rustam Adilov wrote:
>> On 2026-05-09 17:16, [email protected] wrote:
>>> Thank you for your contribution! Sashiko AI review found 1 potential
>>> issue(s) to consider:
>>> - [High] Deadlock risk in the atomic restart handler due to regmap's
>>> internal spinlock.
>>> --
>>>
>>> commit a520c8e2203154ba331a6ae28f00daec074e62ee
>>> Author: Rustam Adilov <[email protected]>
>>>
>>> watchdog: realtek-otto: Change to use regmap API
>>>
>>> This commit converts the realtek-otto watchdog driver to use the regmap API
>>> instead of direct mmio accesses. It replaces direct read/modify/write
>>> sequences with regmap functions and selects REGMAP_MMIO.
>>>
>>>> diff --git a/drivers/watchdog/realtek_otto_wdt.c
>>>> b/drivers/watchdog/realtek_otto_wdt.c
>>>> index 2c30ddd574c59..e5e9cb480f4f9 100644
>>>> --- a/drivers/watchdog/realtek_otto_wdt.c
>>>> +++ b/drivers/watchdog/realtek_otto_wdt.c
>>> [ ... ]
>>>> @@ -199,7 +189,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);
>>>
>>> Could this regmap_write() cause a deadlock during system restart?
>>>
>>> The restart handler is often called during system shutdown or kernel panic
>>> in an atomic context with local interrupts disabled. Since
>>> devm_regmap_init_mmio() configures the regmap to use a spinlock by default,
>>> if a panic occurs while another cpu holds the regmap lock (e.g., during
>>> a ping or timeout update) and that cpu is halted by smp_send_stop(), this
>>> regmap_write() could spin endlessly and prevent the system from rebooting.
>>>
>>> Additionally, on preempt-rt kernels, the regmap spinlock becomes a sleepable
>>> rt_mutex, which could cause a sleeping in atomic context bug when acquired
>>> with interrupts disabled.
>>>
>>>>   	mdelay(3 * otto_wdt_tick_ms(ctrl, 0));
>>>>
>>
>> Hi Sander,
>>
>> I hope it is alright if i CCd you for this as i am not an expert on watchdog
>> stuff.
>> Wonna make sure this issue applies here before i proceed with the suggested
>> solution
>> to add .disable_locking to regmap_config without understanding the
>> consequences of it.
> 
> I'm no expert either, but AFAICT using .disable_locking would essentially mean
> all accesses are only protected by the watchdog subsystem. Which is also the
> case now, so that would mean feature parity.
> 
> I'll leave it up to Guenter to decide if this refactor is worthwhile or not.
> 

I don't mind the refactor, but it must not introduce a regression.
As Sashiko points out, either regmap needs to be configured with
.disable_locking set, or regmap can not be used in the restart handler.

Guenter