[PATCH 0/2] watchdog: realtek-otto: various fixes

[PATCH 0/2] watchdog: realtek-otto: various fixes

[Sander Vanheule]

These patches fix some minor issues with the current code, as reported
in [1]. The individual patches can be applied in any order.

Should I add a Reported-by for the Sashiko review, or how should this be
handled?

[1] https://patch.msgid.link/20260509172748.54B33C2BCB2@smtp.kernel.org/

Sander Vanheule (2):
  watchdog: realtek-otto: prevent PHASE2 underflows
  watchdog: realtek-otto: enable clock before using I/O

 drivers/watchdog/realtek_otto_wdt.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

-- 
2.54.0

[PATCH 1/2] watchdog: realtek-otto: prevent PHASE2 underflows

[Sander Vanheule]

For small pretimeout values, ((timeout - pretimeout) / tick) might be
rounded up to the same value as (timeout / tick). As a result, the
number of PHASE2 ticks may be zero, causing an underflow when
subtracting 1 to configure the hardware. While this results in a
longer-than-expected time to system reset, the duration of PHASE1 and
minimum ping interval for the watchdog would still be correct.

As the watchdog core ensures pretimeout is strictly less than timeout,
ceil(timeout / tick) is strictly greater than floor(pretimeout / tick)
and the number of PHASE1 ticks cannot be 0. So instead of rounding up
the number of PHASE1 ticks, we can round down the number of PHASE2
ticks, maintaining the current behavior while avoiding underflows.

The original helper function is now inlined, as it doesn't save any
duplication anymore.

Signed-off-by: Sander Vanheule <sander@svanheule.net>
---
 drivers/watchdog/realtek_otto_wdt.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/watchdog/realtek_otto_wdt.c b/drivers/watchdog/realtek_otto_wdt.c
index 2c30ddd574c5..1495e8a8cbb5 100644
--- a/drivers/watchdog/realtek_otto_wdt.c
+++ b/drivers/watchdog/realtek_otto_wdt.c
@@ -114,12 +114,6 @@ static int otto_wdt_tick_ms(struct otto_wdt_ctrl *ctrl, int prescale)
  * the value stored in those fields. This means each phase will run for at least
  * one tick, so small values need to be clamped to correctly reflect the timeout.
  */
-static inline unsigned int div_round_ticks(unsigned int val, unsigned int tick_duration,
-		unsigned int min_ticks)
-{
-	return max(min_ticks, DIV_ROUND_UP(val, tick_duration));
-}
-
 static int otto_wdt_determine_timeouts(struct watchdog_device *wdev, unsigned int timeout,
 		unsigned int pretimeout)
 {
@@ -140,9 +134,9 @@ static int otto_wdt_determine_timeouts(struct watchdog_device *wdev, unsigned in
 			return -EINVAL;
 
 		tick_ms = otto_wdt_tick_ms(ctrl, prescale);
-		total_ticks = div_round_ticks(timeout_ms, tick_ms, 2);
-		phase1_ticks = div_round_ticks(timeout_ms - pretimeout_ms, tick_ms, 1);
-		phase2_ticks = total_ticks - phase1_ticks;
+		total_ticks = max(2, DIV_ROUND_UP(timeout_ms, tick_ms));
+		phase2_ticks = max(1, pretimeout_ms / tick_ms);
+		phase1_ticks = total_ticks - phase2_ticks;
 
 		prescale_next++;
 	} while (phase1_ticks > OTTO_WDT_PHASE_TICKS_MAX
-- 
2.54.0

Re: [PATCH 1/2] watchdog: realtek-otto: prevent PHASE2 underflows

[Guenter Roeck]

On Fri, May 15, 2026 at 11:23:50PM +0200, Sander Vanheule wrote:
> For small pretimeout values, ((timeout - pretimeout) / tick) might be
> rounded up to the same value as (timeout / tick). As a result, the
> number of PHASE2 ticks may be zero, causing an underflow when
> subtracting 1 to configure the hardware. While this results in a
> longer-than-expected time to system reset, the duration of PHASE1 and
> minimum ping interval for the watchdog would still be correct.
> 
> As the watchdog core ensures pretimeout is strictly less than timeout,
> ceil(timeout / tick) is strictly greater than floor(pretimeout / tick)
> and the number of PHASE1 ticks cannot be 0. So instead of rounding up
> the number of PHASE1 ticks, we can round down the number of PHASE2
> ticks, maintaining the current behavior while avoiding underflows.
> 
> The original helper function is now inlined, as it doesn't save any
> duplication anymore.
> 
> Signed-off-by: Sander Vanheule <sander@svanheule.net>

Applied.

Thanks,
Guenter

> ---
>  drivers/watchdog/realtek_otto_wdt.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/watchdog/realtek_otto_wdt.c b/drivers/watchdog/realtek_otto_wdt.c
> index 2c30ddd574c5..1495e8a8cbb5 100644
> --- a/drivers/watchdog/realtek_otto_wdt.c
> +++ b/drivers/watchdog/realtek_otto_wdt.c
> @@ -114,12 +114,6 @@ static int otto_wdt_tick_ms(struct otto_wdt_ctrl *ctrl, int prescale)
>   * the value stored in those fields. This means each phase will run for at least
>   * one tick, so small values need to be clamped to correctly reflect the timeout.
>   */
> -static inline unsigned int div_round_ticks(unsigned int val, unsigned int tick_duration,
> -		unsigned int min_ticks)
> -{
> -	return max(min_ticks, DIV_ROUND_UP(val, tick_duration));
> -}
> -
>  static int otto_wdt_determine_timeouts(struct watchdog_device *wdev, unsigned int timeout,
>  		unsigned int pretimeout)
>  {
> @@ -140,9 +134,9 @@ static int otto_wdt_determine_timeouts(struct watchdog_device *wdev, unsigned in
>  			return -EINVAL;
>  
>  		tick_ms = otto_wdt_tick_ms(ctrl, prescale);
> -		total_ticks = div_round_ticks(timeout_ms, tick_ms, 2);
> -		phase1_ticks = div_round_ticks(timeout_ms - pretimeout_ms, tick_ms, 1);
> -		phase2_ticks = total_ticks - phase1_ticks;
> +		total_ticks = max(2, DIV_ROUND_UP(timeout_ms, tick_ms));
> +		phase2_ticks = max(1, pretimeout_ms / tick_ms);
> +		phase1_ticks = total_ticks - phase2_ticks;
>  
>  		prescale_next++;
>  	} while (phase1_ticks > OTTO_WDT_PHASE_TICKS_MAX

[PATCH 2/2] watchdog: realtek-otto: enable clock before using I/O

[Sander Vanheule]

As the watchdog is normally on the same bus as the UART peripheral, the
bootloader will have ensured the bus' clock is up and running before the
watchdog driver is probed. Nevertheless, let's do things the right way
and enable the watchdog's clock before performing I/O accesses.

Signed-off-by: Sander Vanheule <sander@svanheule.net>
---
 drivers/watchdog/realtek_otto_wdt.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/watchdog/realtek_otto_wdt.c b/drivers/watchdog/realtek_otto_wdt.c
index 1495e8a8cbb5..01b3ef89bacf 100644
--- a/drivers/watchdog/realtek_otto_wdt.c
+++ b/drivers/watchdog/realtek_otto_wdt.c
@@ -296,15 +296,15 @@ static int otto_wdt_probe(struct platform_device *pdev)
 	if (IS_ERR(ctrl->base))
 		return PTR_ERR(ctrl->base);
 
+	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);
 
-	ret = otto_wdt_probe_clk(ctrl);
-	if (ret)
-		return ret;
-
 	ctrl->irq_phase1 = platform_get_irq_byname(pdev, "phase1");
 	if (ctrl->irq_phase1 < 0)
 		return ctrl->irq_phase1;
-- 
2.54.0

Re: [PATCH 2/2] watchdog: realtek-otto: enable clock before using I/O

[Guenter Roeck]

On Fri, May 15, 2026 at 11:23:51PM +0200, Sander Vanheule wrote:
> As the watchdog is normally on the same bus as the UART peripheral, the
> bootloader will have ensured the bus' clock is up and running before the
> watchdog driver is probed. Nevertheless, let's do things the right way
> and enable the watchdog's clock before performing I/O accesses.
> 
> Signed-off-by: Sander Vanheule <sander@svanheule.net>

Applied.

Thanks,
Guenter