Re: [PATCH 2/2] iio: light: vcnl4000: add shared IRQ support
Andy Shevchenko <[email protected]>
| Newsgroups | org.kernel.vger.linux-iio,org.kernel.vger.linux-kernel |
|---|---|
| Organization | Intel Finland Oy - BIC 0357606-4 - c/o Alberga Business Park, 6 krs, Bertel Jungin Aukio 5, 02600 Espoo |
| Message-ID | <[email protected]> |
On Tue, Aug 11, 2026 at 05:07:25PM +1000, Tsz Shan Chan wrote:
> Use the IRQ trigger type set by firmware instead, and fall back to
> IRQF_TRIGGER_FALLING if no trigger type is specified to maintain current
> behaviour.
>
> Support IRQF_TRIGGER_FALLING and IRQF_TRIGGER_LOW, which match the open
> drain active low interrupt output. Reject unsupported trigger types.
Can you elaborate with the reference to datasheet if the HW support this
type of IRQ? In such a case, how does HW know which type to trigger?
> Request the interrupt with IRQF_SHARED, and return IRQ_NONE in the irq
> handler when there is no interrupt pending.
...
> ret = i2c_smbus_read_word_data(data->client, data->chip_spec->int_reg);
> - if (ret < 0)
> - return IRQ_HANDLED;
> + if (ret <= 0)
I haven't seen mention of this change in the commit message. Is it related
somehow to the trigger type? How?
> + return IRQ_NONE;
...
> ret = i2c_smbus_read_byte_data(data->client, VCNL4010_ISR);
> - if (ret < 0)
> - goto end;
> + if (ret <= 0)
> + return IRQ_NONE;
Ditto.
> isr = ret;
...
> if (client->irq && data->chip_spec->irq_thread) {
> + u32 irq_type = irq_get_trigger_type(client->irq);
> +
> + switch (irq_type) {
> + case IRQF_TRIGGER_FALLING:
Hmm... Do you have a case with edge sharing interrupts IRL? I think it's
a brain damage setup if it exists.
> + case IRQF_TRIGGER_LOW:
> + break;
> + case IRQF_TRIGGER_NONE:
> + irq_type = IRQF_TRIGGER_FALLING;
Ditto.
> + break;
> + default:
> + return dev_err_probe(dev, -EINVAL,
> + "unsupported irq trigger type %x\n",
> + irq_type);
Broken indentation.
> + }
> ret = devm_request_threaded_irq(dev, client->irq, NULL,
> data->chip_spec->irq_thread,
> - IRQF_TRIGGER_FALLING |
> - IRQF_ONESHOT,
> + IRQF_ONESHOT | IRQF_SHARED |
Also assign these above in a separate line, so this will be just irq_flags (and
name it irq_flags as IRQF_ stands for).
> + irq_type,
> "vcnl4000_irq",
> indio_dev);
--
With Best Regards,
Andy Shevchenko