From patchwork Wed Feb 15 11:23:22 2012
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Subject: hw/pl031: Actually raise interrupt on timer expiry
Date: Wed, 15 Feb 2012 01:23:22 -0000
From: =?utf-8?q?Andreas_F=C3=A4rber?= <afaerber@suse.de>
X-Patchwork-Id: 141312
Message-Id: <4F3B95AA.8090007@suse.de>
To: Peter Maydell <peter.maydell@linaro.org>
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.
> 

Reported-by: Daniel Forsgren <daniel.forsgren@enea.com>

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> 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
> 
>  hw/pl031.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> 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 = (pl031_state *)opaque;
>  
> -    s->im = 1;
> +    s->is = 1;
>      DPRINTF("Alarm raised\n");
>      pl031_update(s);
>  }

So on RTC_ICR write s->is = 0; but it was never set elsewhere, so
RTC_RIS would always return 0.

Acked-by: Andreas Färber <afaerber@suse.de>

However, to facilitate future review of these non-telling fields I
propose the following documentation patch as a follow-up:


Andreas


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;