From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:41532) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RxcxS-0006tu-C2 for qemu-devel@nongnu.org; Wed, 15 Feb 2012 06:23:38 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RxcxM-0002xq-CA for qemu-devel@nongnu.org; Wed, 15 Feb 2012 06:23:34 -0500 Received: from cantor2.suse.de ([195.135.220.15]:46085 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RxcxM-0002xL-32 for qemu-devel@nongnu.org; Wed, 15 Feb 2012 06:23:28 -0500 Message-ID: <4F3B95AA.8090007@suse.de> Date: Wed, 15 Feb 2012 12:23:22 +0100 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1329241239-9327-1-git-send-email-peter.maydell@linaro.org> In-Reply-To: <1329241239-9327-1-git-send-email-peter.maydell@linaro.org> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] hw/pl031: Actually raise interrupt on timer expiry List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: daniel.forsgren@enea.com, qemu-devel@nongnu.org, patches@linaro.org Am 14.02.2012 18:40, schrieb Peter Maydell: > Fix a typo in pl031_interrupt() which meant we were setting a bit > in the interrupt mask rather than the interrupt status register > and thus not actually raising an interrupt. This fix allows the > rtctest program from the kernel's Documentation/rtc.txt to pass > rather than hanging. >=20 Reported-by: Daniel Forsgren > Signed-off-by: Peter Maydell > --- > Looks like our PL031 has always had this bug since it was added > in 2007... Daniel Forsgren reported this, suggested the fix and > pointed me at the test program. Thanks! Down here the credit for the find gets lost. > https://bugs.launchpad.net/qemu-linaro/+bug/931940 >=20 > hw/pl031.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) >=20 > diff --git a/hw/pl031.c b/hw/pl031.c > index 8416a60..f06b5ae 100644 > --- a/hw/pl031.c > +++ b/hw/pl031.c > @@ -76,7 +76,7 @@ static void pl031_interrupt(void * opaque) > { > pl031_state *s =3D (pl031_state *)opaque; > =20 > - s->im =3D 1; > + s->is =3D 1; > DPRINTF("Alarm raised\n"); > pl031_update(s); > } So on RTC_ICR write s->is =3D 0; but it was never set elsewhere, so RTC_RIS would always return 0. Acked-by: Andreas F=E4rber However, to facilitate future review of these non-telling fields I propose the following documentation patch as a follow-up: diff --git a/hw/pl031.c b/hw/pl031.c index 8416a60..a20c625 100644 --- a/hw/pl031.c +++ b/hw/pl031.c @@ -32,6 +32,11 @@ do { printf("pl031: " fmt , ## __VA_ARGS__); } while (= 0) #define RTC_MIS 0x18 /* Masked interrupt status register */ #define RTC_ICR 0x1c /* Interrupt clear register */ +/** + * pl031_state: + * @im: Interrupt mask. + * @is: Interrupt state. + */ typedef struct { SysBusDevice busdev; MemoryRegion iomem; Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg