Re: [Qemu-devel] [PATCH 3/7] allwinner-a10-pit: avoid generation of spurious interrupts

[Li Guang <lig.fnst@cn.fujitsu.com>]

did you detect spurious?
didn't by my limited test,
code will disable any expired timer, unless be re-armed,
so it will never generate interrupt any more.

Thanks!

Beniamino Galvani wrote:
> The model was generating interrupts for all enabled timers after the
> expiration of one of them. Avoid this by passing to the timer callback
> function a structure containing the index of the expired timer.
>
> Signed-off-by: Beniamino Galvani<b.galvani@gmail.com>
> ---
>   hw/timer/allwinner-a10-pit.c |   30 +++++++++++++++++++-----------
>   1 file changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/hw/timer/allwinner-a10-pit.c b/hw/timer/allwinner-a10-pit.c
> index b27fce8..3e1c183 100644
> --- a/hw/timer/allwinner-a10-pit.c
> +++ b/hw/timer/allwinner-a10-pit.c
> @@ -19,6 +19,11 @@
>   #include "sysemu/sysemu.h"
>   #include "hw/timer/allwinner-a10-pit.h"
>
> +typedef struct TimerContext {
> +    AwA10PITState *state;
> +    int index;
> +} TimerContext;
> +
>   static uint64_t a10_pit_read(void *opaque, hwaddr offset, unsigned size)
>   {
>       AwA10PITState *s = AW_A10_PIT(opaque);
> @@ -193,18 +198,17 @@ static void a10_pit_reset(DeviceState *dev)
>
>   static void a10_pit_timer_cb(void *opaque)
>   {
> -    AwA10PITState *s = AW_A10_PIT(opaque);
> -    uint8_t i;
> +    TimerContext *tc = opaque;
> +    AwA10PITState *s = tc->state;
> +    uint8_t i = tc->index;
>
> -    for (i = 0; i<  AW_A10_PIT_TIMER_NR; i++) {
> -        if (s->control[i]&  AW_A10_PIT_TIMER_EN) {
> -            s->irq_status |= 1<<  i;
> -            if (s->control[i]&  AW_A10_PIT_TIMER_MODE) {
> -                ptimer_stop(s->timer[i]);
> -                s->control[i]&= ~AW_A10_PIT_TIMER_EN;
> -            }
> -            qemu_irq_pulse(s->irq[i]);
> +    if (s->control[i]&  AW_A10_PIT_TIMER_EN) {
> +        s->irq_status |= 1<<  i;
> +        if (s->control[i]&  AW_A10_PIT_TIMER_MODE) {
> +            ptimer_stop(s->timer[i]);
> +            s->control[i]&= ~AW_A10_PIT_TIMER_EN;
>           }
> +        qemu_irq_pulse(s->irq[i]);
>       }
>   }
>
> @@ -213,6 +217,7 @@ static void a10_pit_init(Object *obj)
>       AwA10PITState *s = AW_A10_PIT(obj);
>       SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>       QEMUBH * bh[AW_A10_PIT_TIMER_NR];
> +    TimerContext *tc;
>       uint8_t i;
>
>       for (i = 0; i<  AW_A10_PIT_TIMER_NR; i++) {
> @@ -223,7 +228,10 @@ static void a10_pit_init(Object *obj)
>       sysbus_init_mmio(sbd,&s->iomem);
>
>       for (i = 0; i<  AW_A10_PIT_TIMER_NR; i++) {
> -        bh[i] = qemu_bh_new(a10_pit_timer_cb, s);
> +        tc = g_malloc(sizeof(TimerContext));
> +        tc->state = s;
> +        tc->index = i;
> +        bh[i] = qemu_bh_new(a10_pit_timer_cb, tc);
>           s->timer[i] = ptimer_init(bh[i]);
>           ptimer_set_freq(s->timer[i], 240000);
>       }
>