On 12/2/24 13:43, Mike Crowe wrote:
sp805_wdt::load_val is of type unsigned int. When the interrupt is
inactive wdt_timeleft adds one to the value, and then adds that to the
value used to calculate the time remaining. Unfortunately it's not
unlikely that load_val contains LOAD_MAX, which is 0xFFFFFFFF and wraps
to zero when one is added to it, resulting in the time left being
understated by about 21.7s.
It would be possible to widen load_val to 64-bit, or cast the value
before adding it, but it's easy to just cap the value one tick lower at
0xFFFFFFFE so that the addition is guaranteed to succeed. Add a
static_assert to ensure this remains true.
Signed-off-by: Mike Crowe <mac@xxxxxxxxxx>
---
drivers/watchdog/sp805_wdt.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
Of course a simple cast to u64 would work just as well. Please let me
know if you'd like me to fix this in a different way.
diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
index 109e2e37e8f0..1f9a7001d0d6 100644
--- a/drivers/watchdog/sp805_wdt.c
+++ b/drivers/watchdog/sp805_wdt.c
@@ -39,7 +39,7 @@
/* watchdog register offsets and masks */
#define WDTLOAD 0x000
#define LOAD_MIN 0x00000001
- #define LOAD_MAX 0xFFFFFFFF
+ #define LOAD_MAX 0xFFFFFFFE
#define WDTVALUE 0x004
#define WDTCONTROL 0x008
/* control register masks */
@@ -127,8 +127,10 @@ static unsigned int wdt_timeleft(struct watchdog_device *wdd)
load = readl_relaxed(wdt->base + WDTVALUE);
/*If the interrupt is inactive then time left is WDTValue + WDTLoad. */
- if (!(readl_relaxed(wdt->base + WDTRIS) & INT_MASK))
+ if (!(readl_relaxed(wdt->base + WDTRIS) & INT_MASK)) {
+ static_assert((__typeof__(wdt->load_val))LOAD_MAX + 1 != 0);
"load" is already an u64. A typecast of wdt->load_val to u64 in the assignment below
would be much simpler and much less confusing.
Guenter
load += wdt->load_val + 1;
+ }
spin_unlock(&wdt->lock);
return div_u64(load, wdt->rate);
