From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=56805 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PUK4W-0008Ls-EO for qemu-devel@nongnu.org; Sun, 19 Dec 2010 09:17:49 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PUK41-0007PV-KE for qemu-devel@nongnu.org; Sun, 19 Dec 2010 09:17:12 -0500 Received: from mx1.redhat.com ([209.132.183.28]:27657) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PUK41-0007P7-70 for qemu-devel@nongnu.org; Sun, 19 Dec 2010 09:16:41 -0500 Date: Sun, 19 Dec 2010 14:16:36 +0000 From: "Richard W.M. Jones" Subject: Re: [Qemu-devel] Re: [PATCH v3 2/7] wdt_ib700: Don't use SoftFloat int64 type Message-ID: <20101219141636.GP2601@amd.home.annexia.org> References: <1292625556-24825-1-git-send-email-andreas.faerber@web.de> <1292689531-18763-1-git-send-email-andreas.faerber@web.de> <1292689531-18763-2-git-send-email-andreas.faerber@web.de> <20101218164706.GN2601@amd.home.annexia.org> <6639DBDB-CCFA-465B-9E12-9BF0B1775B82@web.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <6639DBDB-CCFA-465B-9E12-9BF0B1775B82@web.de> Content-Transfer-Encoding: quoted-printable List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andreas =?iso-8859-1?Q?F=E4rber?= Cc: qemu-devel@nongnu.org On Sun, Dec 19, 2010 at 01:51:22PM +0100, Andreas F=E4rber wrote: > Am 18.12.2010 um 17:47 schrieb Richard W.M. Jones: >=20 > >On Sat, Dec 18, 2010 at 05:25:26PM +0100, Andreas F=E4rber wrote: > >>softfloat.h's int64 type has least-width semantics, > >>but this doesn't seem intended here, so use plain int64_t. > >> > >>v3: > >>* Split off. > >> > >>Cc: Richard W.M. Jones > >>Signed-off-by: Andreas F=E4rber > >>--- > >>hw/wdt_ib700.c | 2 +- > >>1 files changed, 1 insertions(+), 1 deletions(-) > >> > >>diff --git a/hw/wdt_ib700.c b/hw/wdt_ib700.c > >>index b6235eb..1248464 100644 > >>--- a/hw/wdt_ib700.c > >>+++ b/hw/wdt_ib700.c > >>@@ -53,7 +53,7 @@ static void ib700_write_enable_reg(void *vp, > >>uint32_t addr, uint32_t data) > >> 30, 28, 26, 24, 22, 20, 18, 16, > >> 14, 12, 10, 8, 6, 4, 2, 0 > >> }; > >>- int64 timeout; > >>+ int64_t timeout; > >> > >> ib700_debug("addr =3D %x, data =3D %x\n", addr, data); > > > >The use of int64(_t) was just so that the timeout calculation in the > >next two lines would not overflow: > > > > timeout =3D (int64_t) time_map[data & 0xF] * get_ticks_per_sec(); > > qemu_mod_timer(s->timer, qemu_get_clock (vm_clock) + timeout); > > > >and from you say it does seem like it was a mistake to use int64 > >instead of int64_t. >=20 > int64_t should be the right choice then. >=20 > >ACK. > > > >In more general terms, am I doing the timeout correctly in this code? >=20 > Being unfamiliar with both the timer code and this device, hard to > say for me. > You're taking the lower nibble of uint32_t data and indexing > time_map[] with it, which contains 16 elements, so okay, upcast it > to 64-bit and multiply it to ticks. Assuming that vm_clock works in > ticks, adding the calculated timeout for the next expiry technically > looks good. Except for the extra space. ;) >=20 > Care to provide a formal Reviewed-by or Acked-by? I'd respin it for > Blue with updated description. Is it enough just to write this: Reviewed-by: Richard W.M. Jones Acked-by: Richard W.M. Jones or do you want me to send the updated patch with this added? Rich. --=20 Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rj= ones New in Fedora 11: Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 70 libraries supprt'd http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw