| Message ID | a0557701e77d1dcb7332b0885cab8f6b719c5139.1491865973.git.alistair.francis@xilinx.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Apr 10, 2017 at 4:43 PM, Alistair Francis <alistair.francis@xilinx.com> wrote: > This patch fixes two mistakes in the interrupt logic. > > First we only trigger single-queue or multi-queue interrupts if the status > register is set. This logic was already used for non multi-queue interrupts > but it also applies to multi-queue interrupts. > > Secondly we need to lower the interrupts if the ISR isn't set. As part > of this we can remove the other interrupt lowering logic and consolidate > it inside gem_update_int_status(). > > Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> This patch can be cleaned up more, I'll send a V3. Thanks, Alistair > --- > > hw/net/cadence_gem.c | 18 +++++++++++++----- > 1 file changed, 13 insertions(+), 5 deletions(-) > > diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c > index a66a9cc..fc3a184 100644 > --- a/hw/net/cadence_gem.c > +++ b/hw/net/cadence_gem.c > @@ -509,7 +509,18 @@ static void gem_update_int_status(CadenceGEMState *s) > { > int i; > > - if ((s->num_priority_queues == 1) && s->regs[GEM_ISR]) { > + if (!s->regs[GEM_ISR]) { > + /* ISR isn't set, clear all the interrupts */ > + for (i = 0; i < s->num_priority_queues; ++i) { > + qemu_set_irq(s->irq[i], 0); > + } > + return; > + } > + > + /* If we get here we know s->regs[GEM_ISR] is set, so we don't need to > + * check it again. > + */ > + if (s->num_priority_queues == 1) { > /* No priority queues, just trigger the interrupt */ > DB_PRINT("asserting int.\n"); > qemu_set_irq(s->irq[0], 1); > @@ -1274,7 +1285,6 @@ static uint64_t gem_read(void *opaque, hwaddr offset, unsigned size) > { > CadenceGEMState *s; > uint32_t retval; > - int i; > s = (CadenceGEMState *)opaque; > > offset >>= 2; > @@ -1285,9 +1295,7 @@ static uint64_t gem_read(void *opaque, hwaddr offset, unsigned size) > switch (offset) { > case GEM_ISR: > DB_PRINT("lowering irqs on ISR read\n"); > - for (i = 0; i < s->num_priority_queues; ++i) { > - qemu_set_irq(s->irq[i], 0); > - } > + gem_update_int_status(s); > break; > case GEM_PHYMNTNC: > if (retval & GEM_PHYMNTNC_OP_R) { > -- > 2.9.3 >
diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c index a66a9cc..fc3a184 100644 --- a/hw/net/cadence_gem.c +++ b/hw/net/cadence_gem.c @@ -509,7 +509,18 @@ static void gem_update_int_status(CadenceGEMState *s) { int i; - if ((s->num_priority_queues == 1) && s->regs[GEM_ISR]) { + if (!s->regs[GEM_ISR]) { + /* ISR isn't set, clear all the interrupts */ + for (i = 0; i < s->num_priority_queues; ++i) { + qemu_set_irq(s->irq[i], 0); + } + return; + } + + /* If we get here we know s->regs[GEM_ISR] is set, so we don't need to + * check it again. + */ + if (s->num_priority_queues == 1) { /* No priority queues, just trigger the interrupt */ DB_PRINT("asserting int.\n"); qemu_set_irq(s->irq[0], 1); @@ -1274,7 +1285,6 @@ static uint64_t gem_read(void *opaque, hwaddr offset, unsigned size) { CadenceGEMState *s; uint32_t retval; - int i; s = (CadenceGEMState *)opaque; offset >>= 2; @@ -1285,9 +1295,7 @@ static uint64_t gem_read(void *opaque, hwaddr offset, unsigned size) switch (offset) { case GEM_ISR: DB_PRINT("lowering irqs on ISR read\n"); - for (i = 0; i < s->num_priority_queues; ++i) { - qemu_set_irq(s->irq[i], 0); - } + gem_update_int_status(s); break; case GEM_PHYMNTNC: if (retval & GEM_PHYMNTNC_OP_R) {
This patch fixes two mistakes in the interrupt logic. First we only trigger single-queue or multi-queue interrupts if the status register is set. This logic was already used for non multi-queue interrupts but it also applies to multi-queue interrupts. Secondly we need to lower the interrupts if the ISR isn't set. As part of this we can remove the other interrupt lowering logic and consolidate it inside gem_update_int_status(). Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> --- hw/net/cadence_gem.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)