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

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

[Rustam Adilov]

On 2026-05-09 17:16, sashiko-bot@kernel.org 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 <adilov@disroot.org>
> 
> 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.

Best,
Rustam

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

[Sander Vanheule]

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, sashiko-bot@kernel.org 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 <adilov@disroot.org>
> > 
> > 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.

Best,
Sander

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

[Guenter Roeck]

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, sashiko-bot@kernel.org 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 <adilov@disroot.org>
>>>
>>> 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