From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46511) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZMZPj-0002uP-LE for qemu-devel@nongnu.org; Tue, 04 Aug 2015 06:25:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZMZPg-0006ai-9w for qemu-devel@nongnu.org; Tue, 04 Aug 2015 06:25:43 -0400 Received: from mx1.redhat.com ([209.132.183.28]:45027) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZMZPf-0006Zo-K6 for qemu-devel@nongnu.org; Tue, 04 Aug 2015 06:25:40 -0400 References: <1438609948-3744-1-git-send-email-lvivier@redhat.com> <20150804101326.GS29283@redhat.com> From: Laurent Vivier Message-ID: <55C09321.2080500@redhat.com> Date: Tue, 4 Aug 2015 12:25:37 +0200 MIME-Version: 1.0 In-Reply-To: <20150804101326.GS29283@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] i6300esb: correctly convert watchdog clock ticks into nanoseconds List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Richard W.M. Jones" Cc: Paolo Bonzini , qemu-devel@nongnu.org, David Gibson On 04/08/2015 12:13, Richard W.M. Jones wrote: > On Mon, Aug 03, 2015 at 03:52:28PM +0200, Laurent Vivier wrote: >> + /* A 33 Mhz clock gives a 30 ns tick, >> + * convert timeout from ticks to ns >> */ >> - timeout = muldiv64(get_ticks_per_sec(), timeout, 33000000); >> + timeout *= 30; > > I see that you've just posted a v2 of this patch. However here are > the results of testing the above version. I used the attached test > script which automates things, mostly. > > All times are in seconds. > > Requested timeout Observed timeout > 60 58 > 120 120 > 250 249 > 270 271 > 500 501 [note 1] > 520 522 > 1010 1016 > 1030 1035 > 2046 2058 > 2500 ioctl: WDIOC_SETTIMEOUT: error setting timeout: Invalid argument > [note 2] Thank you Rich. > [note 1] I'm not worried about the timeout being off by a few seconds, > as that could easily be caused by inaccuracies in the test framework. The driver put 1024 in the counter for 1 second. So for 520 seconds, we have ((520 * 1024) << 15) * 30 ns = 523 seconds 2036 seconds ((2046 * 1024) << 15) * 30 ns = 2059 seconds So the emulation seems correct. > [note 2] Maximum setting for i6300esb Linux driver is 2046, see > linux.git/drivers/watchdog/i6300esb.c but note that the printed > messages in the driver relating to the range of the timeout are not > accurate. > This patch looks good to me. I will test the v2 patch shortly. I have no preference of which patch to include in QEMU. This one is nicer but request the same kind of modification in other devices, the V2 is a trivial bug fix (currently timer overflow at 256 seconds). I just want to have the bug fixed... Laurent