| Message ID | 1377288624-7418-2-git-send-email-christoffer.dall@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 23 August 2013 21:10, Christoffer Dall <christoffer.dall@linaro.org> 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 <christoffer.dall@linaro.org> > --- > 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). thanks -- PMM
On Fri, Sep 06, 2013 at 02:59:08PM +0100, Peter Maydell wrote: > On 23 August 2013 21:10, Christoffer Dall <christoffer.dall@linaro.org> 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 <christoffer.dall@linaro.org> > > --- > > 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
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);
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 <christoffer.dall@linaro.org> --- hw/intc/arm_gic.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)