Re: [Qemu-devel] [PATCH] hw/pl031: Actually raise interrupt on timer expiry

[Andreas Färber]

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 <address@hidden>

> Signed-off-by: Peter Maydell <address@hidden>
> ---
> 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 <address@hidden>

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

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg