Re: [PATCH v5 5/5] counter: 104-quad-8: Add IRQ support for the ACCES 104-QUAD-8

David Lechner
2020-10-14T00:15:11Z

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;
> +}
> +