From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45032) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VKN1g-0005SJ-Vr for qemu-devel@nongnu.org; Fri, 13 Sep 2013 02:38:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VKN1Z-0001a0-8C for qemu-devel@nongnu.org; Fri, 13 Sep 2013 02:38:44 -0400 Received: from mail-pd0-f169.google.com ([209.85.192.169]:60448) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VKN1Z-0001Zb-2l for qemu-devel@nongnu.org; Fri, 13 Sep 2013 02:38:37 -0400 Received: by mail-pd0-f169.google.com with SMTP id r10so846305pdi.14 for ; Thu, 12 Sep 2013 23:38:36 -0700 (PDT) Date: Thu, 12 Sep 2013 23:38:47 -0700 From: Christoffer Dall Message-ID: <20130913063847.GB30894@cbox> References: <1377288624-7418-1-git-send-email-christoffer.dall@linaro.org> <1377288624-7418-2-git-send-email-christoffer.dall@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Subject: Re: [Qemu-devel] [PATCH 1/5] hw: arm_gic: Fix gic_set_irq handling List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: "linaro-kernel@lists.linaro.org" , QEMU Developers , Patch Tracking , "kvmarm@lists.cs.columbia.edu" On Fri, Sep 06, 2013 at 02:59:08PM +0100, Peter Maydell wrote: > On 23 August 2013 21:10, Christoffer Dall wrote: > > For some reason only edge-triggered or enabled level-triggered > > interrupts would set the pending state of a raised IRQ. This is not in > > compliance with the specs, which indicate that the pending state is > > separate from the enabled state, which only controls if a pending > > interrupt is actually forwarded to the CPU interface. > > > > Therefore, simply always set the pending state on a rising edge, but > > only clear the pending state of falling edge if the interrupt is level > > triggered. > > > > Signed-off-by: Christoffer Dall > > --- > > hw/intc/arm_gic.c | 9 +++++---- > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c > > index d431b7a..bff3f9e 100644 > > --- a/hw/intc/arm_gic.c > > +++ b/hw/intc/arm_gic.c > > @@ -128,11 +128,12 @@ static void gic_set_irq(void *opaque, int irq, int level) > > > > if (level) { > > GIC_SET_LEVEL(irq, cm); > > - if (GIC_TEST_TRIGGER(irq) || GIC_TEST_ENABLED(irq, cm)) { > > - DPRINTF("Set %d pending mask %x\n", irq, target); > > - GIC_SET_PENDING(irq, target); > > - } > > + DPRINTF("Set %d pending mask %x\n", irq, target); > > + GIC_SET_PENDING(irq, target); > > } else { > > + if (!GIC_TEST_TRIGGER(irq)) { > > + gic_clear_pending(s, irq, target, 0); > > + } > > GIC_CLEAR_LEVEL(irq, cm); > > } > > gic_update(s); > > This doesn't compile: > > hw/intc/arm_gic.c: In function ‘gic_set_irq’: > hw/intc/arm_gic.c:135:13: error: implicit declaration of function > ‘gic_clear_pending’ [-Werror=implicit-function-declaration] > hw/intc/arm_gic.c:135:13: error: nested extern declaration of > ‘gic_clear_pending’ [-Werror=nested-externs] > > (you don't provide that function until patch 3). > whoops, should be GIC_CLEAR_PENDING at this stage. Thanks for testing the bisectability. -Christoffer