linux-watchdog.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Doug Anderson <dianders@chromium.org>
To: Wim Van Sebroeck <wim@iguana.be>, Guenter Roeck <linux@roeck-us.net>
Cc: Leela Krishna Amudala <l.krishna@samsung.com>,
	Olof Johansson <olof@lixom.net>,
	Tomasz Figa <tomasz.figa@gmail.com>,
	Kukjin Kim <kgene.kim@samsung.com>,
	Doug Anderson <dianders@chromium.org>,
	Ben Dooks <ben-linux@fluff.org>,
	linux-arm-kernel@lists.infradead.org,
	linux-samsung-soc@vger.kernel.org,
	linux-watchdog@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH] watchdog: s3c2410_wdt: Handle rounding a little better for timeout
Date: Tue, 26 Nov 2013 10:30:37 -0800	[thread overview]
Message-ID: <1385490637-10306-1-git-send-email-dianders@chromium.org> (raw)

The existing watchdog timeout worked OK but didn't deal with
rounding in an ideal way when dividing out all of its clocks.

Specifically if you had a timeout of 32 seconds and an input clock of
66666666, you'd end up setting a timeout of 31.9998 seconds and
reporting a timeout of 31 seconds.

Specifically DBG printouts showed:
  s3c2410wdt_set_heartbeat: count=16666656, timeout=32, freq=520833
  s3c2410wdt_set_heartbeat: timeout=32, divisor=255, count=16666656 (0000ff4f)
and the final timeout reported to the user was:
  ((count / divisor) * divisor) / freq
  (0xff4f * 255) / 520833 = 31 (truncated from 31.9998)
the technically "correct" value is:
  (0xff4f * 255) / (66666666.0 / 128) = 31.9998

By using "DIV_ROUND_UP" we can be a little more correct.
  s3c2410wdt_set_heartbeat: count=16666688, timeout=32, freq=520834
  s3c2410wdt_set_heartbeat: timeout=32, divisor=255, count=16666688 (0000ff50)
and the final timeout reported to the user:
  (0xff50 * 255) / 520834 = 32
the technically "correct" value is:
  (0xff50 * 255) / (66666666.0 / 128) = 32.0003

We'll use a DIV_ROUND_UP to solve this, generally erroring on the side
of reporting shorter values to the user and setting the watchdog to
slightly longer than requested:
* Round input frequency up to assume watchdog is counting faster.
* Round divisions by divisor up to give us extra time.

Signed-off-by: Doug Anderson <dianders@chromium.org>
---
 drivers/watchdog/s3c2410_wdt.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
index 7d8fd04..fe2322b 100644
--- a/drivers/watchdog/s3c2410_wdt.c
+++ b/drivers/watchdog/s3c2410_wdt.c
@@ -188,7 +188,7 @@ static int s3c2410wdt_set_heartbeat(struct watchdog_device *wdd, unsigned timeou
 	if (timeout < 1)
 		return -EINVAL;
 
-	freq /= 128;
+	freq = DIV_ROUND_UP(freq, 128);
 	count = timeout * freq;
 
 	DBG("%s: count=%d, timeout=%d, freq=%lu\n",
@@ -201,20 +201,20 @@ static int s3c2410wdt_set_heartbeat(struct watchdog_device *wdd, unsigned timeou
 
 	if (count >= 0x10000) {
 		for (divisor = 1; divisor <= 0x100; divisor++) {
-			if ((count / divisor) < 0x10000)
+			if (DIV_ROUND_UP(count, divisor) < 0x10000)
 				break;
 		}
 
-		if ((count / divisor) >= 0x10000) {
+		if (divisor > 0x100) {
 			dev_err(wdt->dev, "timeout %d too big\n", timeout);
 			return -EINVAL;
 		}
 	}
 
 	DBG("%s: timeout=%d, divisor=%d, count=%d (%08x)\n",
-	    __func__, timeout, divisor, count, count/divisor);
+	    __func__, timeout, divisor, count, DIV_ROUND_UP(count, divisor));
 
-	count /= divisor;
+	count = DIV_ROUND_UP(count, divisor);
 	wdt->count = count;
 
 	/* update the pre-scaler */
-- 
1.8.4.1

             reply	other threads:[~2013-11-26 18:30 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-26 18:30 Doug Anderson [this message]
2013-11-26 18:48 ` [PATCH] watchdog: s3c2410_wdt: Handle rounding a little better for timeout Guenter Roeck
2013-11-26 21:34   ` Doug Anderson
2013-11-27  0:10     ` Guenter Roeck
2013-11-27  0:57       ` Doug Anderson
2013-11-27  0:57 ` [PATCH v2] " Doug Anderson
2013-11-27  1:04   ` Guenter Roeck
2014-01-08 20:03   ` Wim Van Sebroeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1385490637-10306-1-git-send-email-dianders@chromium.org \
    --to=dianders@chromium.org \
    --cc=ben-linux@fluff.org \
    --cc=kgene.kim@samsung.com \
    --cc=l.krishna@samsung.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=olof@lixom.net \
    --cc=tomasz.figa@gmail.com \
    --cc=wim@iguana.be \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).