Messages in this thread | | | Subject | Re: [PATCH v5 5/5] counter: 104-quad-8: Add IRQ support for the ACCES 104-QUAD-8 | From | David Lechner <> | Date | Tue, 13 Oct 2020 19:13:32 -0500 |
| |
On 9/26/20 9:18 PM, William Breathitt Gray wrote: > +static irqreturn_t quad8_irq_handler(int irq, void *quad8iio) > +{ > + struct quad8_iio *const priv = quad8iio; > + const unsigned long base = priv->base; > + unsigned long irq_status; > + unsigned long channel; > + u8 event; > + int err; > + > + irq_status = inb(base + QUAD8_REG_INTERRUPT_STATUS); > + if (!irq_status) > + return IRQ_NONE; > + > + for_each_set_bit(channel, &irq_status, QUAD8_NUM_COUNTERS) { > + switch (priv->irq_trigger[channel]) { > + case 0: > + event = COUNTER_EVENT_OVERFLOW; > + break; > + case 1: > + event = COUNTER_EVENT_THRESHOLD; > + break; > + case 2: > + event = COUNTER_EVENT_OVERFLOW_UNDERFLOW; > + break; > + case 3: > + event = COUNTER_EVENT_INDEX; > + break; > + default: > + /* We should never reach here */ > + return -EINVAL;
This is not a valid return value for an IRQ handler. Maybe WARN_ONCE instead?
> + } > + err = counter_push_event(&priv->counter, event, channel); > + if (err) > + return err;
Same here. Otherwise, I think we could end up with interrupts in an endless loop since the interrupt would never be cleared.
> + } > + > + /* Clear pending interrupts on device */ > + outb(QUAD8_CHAN_OP_ENABLE_INTERRUPT_FUNC, base + QUAD8_REG_CHAN_OP); > + > + return IRQ_HANDLED; > +} > +
| |