Linux Watchdog driver development
 help / color / mirror / Atom feed
* [PATCH v3 0/1] watchdog: realtek-otto: Make use of regmap API
@ 2026-07-10  7:43 Rustam Adilov
  2026-07-10  7:43 ` [PATCH v3 1/1] watchdog: realtek-otto: Change to use " Rustam Adilov
  2026-07-10  8:13 ` [PATCH v3 0/1] watchdog: realtek-otto: Make use of " Rustam Adilov
  0 siblings, 2 replies; 4+ messages in thread
From: Rustam Adilov @ 2026-07-10  7:43 UTC (permalink / raw)
  To: Wim Van Sebroeck, Guenter Roeck, Sander Vanheule, linux-watchdog,
	linux-kernel
  Cc: Rustam Adilov

This patch series changes the driver to use regmap API for all of the
register access stuff instead of the ioread32 and iowrite32 with __iomem.
That makes the watchdog operable when SWAP_IO_SPACE config is enabled just
by adding big-endian property to the device tree node.

Also, there was a comment by sashiko which got addressed by Sander in [2]
with possibility of it being false positive so i left it at that.

---
Changelog in v3:
- reworded the commit message to make the main reasoning clear.
- rebased the branch.
- Link v2: https://lore.kernel.org/linux-watchdog/20260519182329.24472-1-adilov@disroot.org/

Changelog in v2:
- removed patches 2 and 3 that add rtl9607c device.
- included driver changes made by Sander in [1].
- added disable_locking to regmap_config and set it to true.
- Link to v1: https://lore.kernel.org/all/20260509163101.722793-1-adilov@disroot.org/

[1] https://lore.kernel.org/all/20260515212351.752054-1-sander@svanheule.net/
[2] https://lore.kernel.org/linux-watchdog/d0b159eefa6bc5abf0d1531acde568396f480500.camel@svanheule.net/

Rustam Adilov (1):
  watchdog: realtek-otto: Change to use regmap API

 drivers/watchdog/Kconfig            |  1 +
 drivers/watchdog/realtek_otto_wdt.c | 74 +++++++++++++++--------------
 2 files changed, 39 insertions(+), 36 deletions(-)

-- 
2.55.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v3 1/1] watchdog: realtek-otto: Change to use regmap API
  2026-07-10  7:43 [PATCH v3 0/1] watchdog: realtek-otto: Make use of regmap API Rustam Adilov
@ 2026-07-10  7:43 ` Rustam Adilov
  2026-07-10  7:58   ` sashiko-bot
  2026-07-10  8:13 ` [PATCH v3 0/1] watchdog: realtek-otto: Make use of " Rustam Adilov
  1 sibling, 1 reply; 4+ messages in thread
From: Rustam Adilov @ 2026-07-10  7:43 UTC (permalink / raw)
  To: Wim Van Sebroeck, Guenter Roeck, Sander Vanheule, linux-watchdog,
	linux-kernel
  Cc: Rustam Adilov

To make the realtek watchdog driver functional when SWAP_IO_SPACE
config is enabled, change all of the register access to be done
by regmap API which helps us to tweak endianness with big-endian
or little-endian property from within the device tree node.

Add the REGMAP_MMIO as a select to REALTEK_OTTO_WDT now that the
regmap is used.

Signed-off-by: Rustam Adilov <adilov@disroot.org>
---
 drivers/watchdog/Kconfig            |  1 +
 drivers/watchdog/realtek_otto_wdt.c | 74 +++++++++++++++--------------
 2 files changed, 39 insertions(+), 36 deletions(-)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 08cb8612d41f..e0ef0b339abd 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1076,6 +1076,7 @@ config REALTEK_OTTO_WDT
 	depends on MACH_REALTEK_RTL || COMPILE_TEST
 	depends on COMMON_CLK
 	select WATCHDOG_CORE
+	select REGMAP_MMIO
 	default MACH_REALTEK_RTL
 	help
 	  Say Y here to include support for the watchdog timer on Realtek
diff --git a/drivers/watchdog/realtek_otto_wdt.c b/drivers/watchdog/realtek_otto_wdt.c
index 9094f2189f55..5c48ffba70a2 100644
--- a/drivers/watchdog/realtek_otto_wdt.c
+++ b/drivers/watchdog/realtek_otto_wdt.c
@@ -27,6 +27,7 @@
 #include <linux/platform_device.h>
 #include <linux/property.h>
 #include <linux/reboot.h>
+#include <linux/regmap.h>
 #include <linux/watchdog.h>
 
 #define OTTO_WDT_REG_CNTR		0x0
@@ -65,7 +66,7 @@
 struct otto_wdt_ctrl {
 	struct watchdog_device wdev;
 	struct device *dev;
-	void __iomem *base;
+	struct regmap *regmap;
 	unsigned int clk_rate_khz;
 	int irq_phase1;
 };
@@ -73,24 +74,17 @@ struct otto_wdt_ctrl {
 static int otto_wdt_start(struct watchdog_device *wdev)
 {
 	struct otto_wdt_ctrl *ctrl = watchdog_get_drvdata(wdev);
-	u32 v;
-
-	v = ioread32(ctrl->base + OTTO_WDT_REG_CTRL);
-	v |= OTTO_WDT_CTRL_ENABLE;
-	iowrite32(v, ctrl->base + OTTO_WDT_REG_CTRL);
 
+	regmap_set_bits(ctrl->regmap, OTTO_WDT_REG_CTRL, OTTO_WDT_CTRL_ENABLE);
 	return 0;
 }
 
 static int otto_wdt_stop(struct watchdog_device *wdev)
 {
 	struct otto_wdt_ctrl *ctrl = watchdog_get_drvdata(wdev);
-	u32 v;
-
-	v = ioread32(ctrl->base + OTTO_WDT_REG_CTRL);
-	v &= ~OTTO_WDT_CTRL_ENABLE;
-	iowrite32(v, ctrl->base + OTTO_WDT_REG_CTRL);
 
+	regmap_clear_bits(ctrl->regmap, OTTO_WDT_REG_CTRL,
+			  OTTO_WDT_CTRL_ENABLE);
 	return 0;
 }
 
@@ -98,8 +92,7 @@ static int otto_wdt_ping(struct watchdog_device *wdev)
 {
 	struct otto_wdt_ctrl *ctrl = watchdog_get_drvdata(wdev);
 
-	iowrite32(OTTO_WDT_CNTR_PING, ctrl->base + OTTO_WDT_REG_CNTR);
-
+	regmap_write(ctrl->regmap, OTTO_WDT_REG_CNTR, OTTO_WDT_CNTR_PING);
 	return 0;
 }
 
@@ -125,7 +118,7 @@ static int otto_wdt_determine_timeouts(struct watchdog_device *wdev, unsigned in
 	unsigned int total_ticks;
 	unsigned int prescale;
 	unsigned int tick_ms;
-	u32 v;
+	u32 mask, val;
 
 	do {
 		prescale = prescale_next;
@@ -141,14 +134,11 @@ static int otto_wdt_determine_timeouts(struct watchdog_device *wdev, unsigned in
 	} while (phase1_ticks > OTTO_WDT_PHASE_TICKS_MAX
 		|| phase2_ticks > OTTO_WDT_PHASE_TICKS_MAX);
 
-	v = ioread32(ctrl->base + OTTO_WDT_REG_CTRL);
-
-	v &= ~(OTTO_WDT_CTRL_PRESCALE | OTTO_WDT_CTRL_PHASE1 | OTTO_WDT_CTRL_PHASE2);
-	v |= FIELD_PREP(OTTO_WDT_CTRL_PHASE1, phase1_ticks - 1);
-	v |= FIELD_PREP(OTTO_WDT_CTRL_PHASE2, phase2_ticks - 1);
-	v |= FIELD_PREP(OTTO_WDT_CTRL_PRESCALE, prescale);
-
-	iowrite32(v, ctrl->base + OTTO_WDT_REG_CTRL);
+	mask = OTTO_WDT_CTRL_PRESCALE | OTTO_WDT_CTRL_PHASE1 | OTTO_WDT_CTRL_PHASE2;
+	val = FIELD_PREP(OTTO_WDT_CTRL_PHASE1, phase1_ticks - 1);
+	val |= FIELD_PREP(OTTO_WDT_CTRL_PHASE2, phase2_ticks - 1);
+	val |= FIELD_PREP(OTTO_WDT_CTRL_PRESCALE, prescale);
+	regmap_update_bits(ctrl->regmap, OTTO_WDT_REG_CTRL, mask, val);
 
 	timeout_ms = total_ticks * tick_ms;
 	ctrl->wdev.timeout = timeout_ms / 1000;
@@ -192,7 +182,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));
 
@@ -203,7 +193,7 @@ static irqreturn_t otto_wdt_phase1_isr(int irq, void *dev_id)
 {
 	struct otto_wdt_ctrl *ctrl = dev_id;
 
-	iowrite32(OTTO_WDT_INTR_PHASE_1, ctrl->base + OTTO_WDT_REG_INTR);
+	regmap_write(ctrl->regmap, OTTO_WDT_REG_INTR, OTTO_WDT_INTR_PHASE_1);
 	dev_crit(ctrl->dev, "phase 1 timeout\n");
 	watchdog_notify_pretimeout(&ctrl->wdev);
 
@@ -249,7 +239,6 @@ static int otto_wdt_probe_reset_mode(struct otto_wdt_ctrl *ctrl)
 	const struct fwnode_handle *node = ctrl->dev->fwnode;
 	int mode_count;
 	u32 mode;
-	u32 v;
 
 	if (!node)
 		return -ENXIO;
@@ -271,19 +260,25 @@ static int otto_wdt_probe_reset_mode(struct otto_wdt_ctrl *ctrl)
 	else
 		return -EINVAL;
 
-	v = ioread32(ctrl->base + OTTO_WDT_REG_CTRL);
-	v &= ~OTTO_WDT_CTRL_RST_MODE;
-	v |= FIELD_PREP(OTTO_WDT_CTRL_RST_MODE, mode);
-	iowrite32(v, ctrl->base + OTTO_WDT_REG_CTRL);
-
+	regmap_update_bits(ctrl->regmap, OTTO_WDT_REG_CTRL,
+			   OTTO_WDT_CTRL_RST_MODE,
+			   FIELD_PREP(OTTO_WDT_CTRL_RST_MODE, mode));
 	return 0;
 }
 
+static const struct regmap_config realtek_otto_wdt_regmap_config = {
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.disable_locking = true,
+};
+
 static int otto_wdt_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct otto_wdt_ctrl *ctrl;
 	unsigned int max_tick_ms;
+	void __iomem *base;
 	int ret;
 
 	ctrl = devm_kzalloc(dev, sizeof(*ctrl), GFP_KERNEL);
@@ -291,18 +286,25 @@ static int otto_wdt_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	ctrl->dev = dev;
-	ctrl->base = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(ctrl->base))
-		return PTR_ERR(ctrl->base);
+	base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	ctrl->regmap = devm_regmap_init_mmio(dev, base,
+					     &realtek_otto_wdt_regmap_config);
+	if (IS_ERR(ctrl->regmap)) {
+		dev_err(dev, "regmap init failed\n");
+		return PTR_ERR(ctrl->regmap);
+	}
 
 	ret = otto_wdt_probe_clk(ctrl);
 	if (ret)
 		return ret;
 
 	/* Clear any old interrupts and reset initial state */
-	iowrite32(OTTO_WDT_INTR_PHASE_1 | OTTO_WDT_INTR_PHASE_2,
-			ctrl->base + OTTO_WDT_REG_INTR);
-	iowrite32(OTTO_WDT_CTRL_DEFAULT, ctrl->base + OTTO_WDT_REG_CTRL);
+	regmap_write(ctrl->regmap, OTTO_WDT_REG_INTR,
+		     OTTO_WDT_INTR_PHASE_1 | OTTO_WDT_INTR_PHASE_2);
+	regmap_write(ctrl->regmap, OTTO_WDT_REG_CTRL, OTTO_WDT_CTRL_DEFAULT);
 
 	ctrl->irq_phase1 = platform_get_irq_byname(pdev, "phase1");
 	if (ctrl->irq_phase1 < 0)
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v3 1/1] watchdog: realtek-otto: Change to use regmap API
  2026-07-10  7:43 ` [PATCH v3 1/1] watchdog: realtek-otto: Change to use " Rustam Adilov
@ 2026-07-10  7:58   ` sashiko-bot
  0 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-07-10  7:58 UTC (permalink / raw)
  To: Rustam Adilov; +Cc: linux-watchdog

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

Pre-existing issues:
- [High] Calling `disable_irq()` in the restart handler causes a scheduling while atomic bug and a deadlock when triggered by a pretimeout panic.
- [High] The watchdog timeout setup logic does not clamp user timeouts to hardware limits, breaking the watchdog core's software-extended timeout feature.
--

--- Patch [1]: [PATCH v3 1/1] watchdog: realtek-otto: Change to use regmap API ---
commit df79c826fd293fb9a9e926fad614d84ecc05598a
Author: Rustam Adilov <adilov@disroot.org>

watchdog: realtek-otto: Change to use regmap API

To make the realtek watchdog driver functional when SWAP_IO_SPACE
config is enabled, change all of the register access to be done
by regmap API which helps us to tweak endianness with big-endian
or little-endian property from within the device tree node.

Add the REGMAP_MMIO as a select to REALTEK_OTTO_WDT now that the
regmap is used.

> diff --git a/drivers/watchdog/realtek_otto_wdt.c b/drivers/watchdog/realtek_otto_wdt.c
> index 9094f2189f555..5c48ffba70a20 100644
> --- a/drivers/watchdog/realtek_otto_wdt.c
> +++ b/drivers/watchdog/realtek_otto_wdt.c

[ ... ]

> @@ -141,14 +134,11 @@ static int otto_wdt_determine_timeouts(struct watchdog_device *wdev, unsigned in
>  	} while (phase1_ticks > OTTO_WDT_PHASE_TICKS_MAX
>  		|| phase2_ticks > OTTO_WDT_PHASE_TICKS_MAX);
>  
> -	v = ioread32(ctrl->base + OTTO_WDT_REG_CTRL);
> -
> -	v &= ~(OTTO_WDT_CTRL_PRESCALE | OTTO_WDT_CTRL_PHASE1 | OTTO_WDT_CTRL_PHASE2);
> -	v |= FIELD_PREP(OTTO_WDT_CTRL_PHASE1, phase1_ticks - 1);
> -	v |= FIELD_PREP(OTTO_WDT_CTRL_PHASE2, phase2_ticks - 1);
> -	v |= FIELD_PREP(OTTO_WDT_CTRL_PRESCALE, prescale);
> -
> -	iowrite32(v, ctrl->base + OTTO_WDT_REG_CTRL);
> +	mask = OTTO_WDT_CTRL_PRESCALE | OTTO_WDT_CTRL_PHASE1 | OTTO_WDT_CTRL_PHASE2;
> +	val = FIELD_PREP(OTTO_WDT_CTRL_PHASE1, phase1_ticks - 1);
> +	val |= FIELD_PREP(OTTO_WDT_CTRL_PHASE2, phase2_ticks - 1);
> +	val |= FIELD_PREP(OTTO_WDT_CTRL_PRESCALE, prescale);
> +	regmap_update_bits(ctrl->regmap, OTTO_WDT_REG_CTRL, mask, val);
>  
>  	timeout_ms = total_ticks * tick_ms;
>  	ctrl->wdev.timeout = timeout_ms / 1000;

[Severity: High]
This is a pre-existing issue, but does otto_wdt_determine_timeouts() break
the watchdog core's software heartbeat extension?

When a user requests a timeout larger than the hardware's maximum capability,
the prescale calculation loop earlier in this function exceeds
OTTO_WDT_PRESCALE_MAX and returns -EINVAL.

It seems the driver fails to clamp the requested timeout to its hardware
capability before programming the registers, and overwrites wdev.timeout with
the calculated hardware ticks. This prevents the core from maintaining the
full user-requested timeout via periodic pings.

[ ... ]

> @@ -192,7 +182,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);

[Severity: High]
This isn't a bug introduced by this patch, but can the call to disable_irq()
earlier in otto_wdt_restart() cause a deadlock during a pretimeout panic?

If the pretimeout governor is set to panic, otto_wdt_phase1_isr() calls
watchdog_notify_pretimeout() which triggers a kernel panic. The panic routine
disables local interrupts and calls machine_restart(), which invokes
otto_wdt_restart().

Because otto_wdt_restart() calls disable_irq(), it internally calls
synchronize_irq() which will spin waiting for the currently active interrupt
handler (otto_wdt_phase1_isr) to complete.

Since we are already inside that handler, this will hang indefinitely during
reboot. Additionally, calling disable_irq() in an atomic context with
interrupts disabled could trigger a scheduling while atomic bug.

>  
>  	mdelay(3 * otto_wdt_tick_ms(ctrl, 0));

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3 0/1] watchdog: realtek-otto: Make use of regmap API
  2026-07-10  7:43 [PATCH v3 0/1] watchdog: realtek-otto: Make use of regmap API Rustam Adilov
  2026-07-10  7:43 ` [PATCH v3 1/1] watchdog: realtek-otto: Change to use " Rustam Adilov
@ 2026-07-10  8:13 ` Rustam Adilov
  1 sibling, 0 replies; 4+ messages in thread
From: Rustam Adilov @ 2026-07-10  8:13 UTC (permalink / raw)
  To: Wim Van Sebroeck, Guenter Roeck, Sander Vanheule, linux-watchdog,
	linux-kernel

On 2026-07-10 07:43, Rustam Adilov wrote:
> This patch series changes the driver to use regmap API for all of the
> register access stuff instead of the ioread32 and iowrite32 with __iomem.
> That makes the watchdog operable when SWAP_IO_SPACE config is enabled just
> by adding big-endian property to the device tree node.
> 
> Also, there was a comment by sashiko which got addressed by Sander in [2]
> with possibility of it being false positive so i left it at that.
> 
> ---
> Changelog in v3:
> - reworded the commit message to make the main reasoning clear.
> - rebased the branch.
> - Link v2: https://lore.kernel.org/linux-watchdog/20260519182329.24472-1-adilov@disroot.org/
> 
> Changelog in v2:
> - removed patches 2 and 3 that add rtl9607c device.
> - included driver changes made by Sander in [1].
> - added disable_locking to regmap_config and set it to true.
> - Link to v1: https://lore.kernel.org/all/20260509163101.722793-1-adilov@disroot.org/
> 
> [1] https://lore.kernel.org/all/20260515212351.752054-1-sander@svanheule.net/
> [2] https://lore.kernel.org/linux-watchdog/d0b159eefa6bc5abf0d1531acde568396f480500.camel@svanheule.net/
> 
> Rustam Adilov (1):
>   watchdog: realtek-otto: Change to use regmap API
> 
>  drivers/watchdog/Kconfig            |  1 +
>  drivers/watchdog/realtek_otto_wdt.c | 74 +++++++++++++++--------------
>  2 files changed, 39 insertions(+), 36 deletions(-)

Hmm, there is a real possibility that Guenter Roeck didn't receive
my patches because I've received a message "Undelivered Mail
Returned to Sender" indicating mail host roeck-us.net has rejected them.

I don't know if that's gonna be a problem but i hope not. At the very
least it is visible on linux-watchdog mailing list.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-07-10  8:13 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-10  7:43 [PATCH v3 0/1] watchdog: realtek-otto: Make use of regmap API Rustam Adilov
2026-07-10  7:43 ` [PATCH v3 1/1] watchdog: realtek-otto: Change to use " Rustam Adilov
2026-07-10  7:58   ` sashiko-bot
2026-07-10  8:13 ` [PATCH v3 0/1] watchdog: realtek-otto: Make use of " Rustam Adilov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox