From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56555) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WFu3X-0000p7-01 for qemu-devel@nongnu.org; Tue, 18 Feb 2014 18:26:35 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WFu3O-0005GI-33 for qemu-devel@nongnu.org; Tue, 18 Feb 2014 18:26:26 -0500 Received: from mail-ea0-x22d.google.com ([2a00:1450:4013:c01::22d]:61604) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WFu3N-0005G5-SU for qemu-devel@nongnu.org; Tue, 18 Feb 2014 18:26:18 -0500 Received: by mail-ea0-f173.google.com with SMTP id d10so8249197eaj.4 for ; Tue, 18 Feb 2014 15:26:17 -0800 (PST) Date: Wed, 19 Feb 2014 00:26:06 +0100 From: Beniamino Galvani Message-ID: <20140218232605.GC24042@gmail.com> References: <1392659003-8264-1-git-send-email-b.galvani@gmail.com> <1392659003-8264-4-git-send-email-b.galvani@gmail.com> <5302DEC0.2050104@cn.fujitsu.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5302DEC0.2050104@cn.fujitsu.com> Subject: Re: [Qemu-devel] [PATCH 3/7] allwinner-a10-pit: avoid generation of spurious interrupts List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Li Guang Cc: Peter Maydell , Peter Crosthwaite , qemu-devel@nongnu.org On Tue, Feb 18, 2014 at 12:17:04PM +0800, Li Guang wrote: > 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. > > > 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. Yes, when both timer0 and timer1 were enabled and timer0 expired, the previous implementation raised two interrupts because the callback function iterated over the array of timers. Instead it should consider only the timer that generated the event. Beniamino > >Signed-off-by: Beniamino Galvani > >--- > > 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); > > } >