Re: [PATCH] iio: proximity: vcnl3020: fix ISR bitmask check in IRQ handler
Jonathan Cameron <[email protected]>
| Newsgroups | org.kernel.vger.linux-iio,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <20260823225013.4e6fea11@jic23-huawei> |
On Wed, 19 Aug 2026 21:37:33 +0100 Salah Triki <[email protected]> wrote: > In vcnl3020_handle_irq_thread(), the status register (VCNL_ISR) was > incorrectly checked against VCNL_ICR_THRES_EN (BIT(1)), which is a bit > mask intended for the Interrupt Control Register (VCNL_PS_ICR). > > Because BIT(1) in the ISR register corresponds only to VCNL_INT_TH_LOW, > any high-threshold interrupt (VCNL_INT_TH_HI, BIT(0)) occurring on its own > resulted in the handler returning IRQ_NONE and ignoring the event. > > Fix this by checking the ISR value against both valid status bits: > VCNL_INT_TH_HI and VCNL_INT_TH_LOW. > > Fixes: 3363fbbe19e5 ("iio: proximity: vcnl3020: add periodic mode") > Signed-off-by: Salah Triki <[email protected]> > --- > drivers/iio/proximity/vcnl3020.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/iio/proximity/vcnl3020.c b/drivers/iio/proximity/vcnl3020.c > index 7f417372566a..d4ad06f8f366 100644 > --- a/drivers/iio/proximity/vcnl3020.c > +++ b/drivers/iio/proximity/vcnl3020.c > @@ -584,7 +584,7 @@ static irqreturn_t vcnl3020_handle_irq_thread(int irq, void *p) > return IRQ_HANDLED; > } > > - if (!(isr & VCNL_ICR_THRES_EN)) > + if (!(isr & (VCNL_INT_TH_HI | VCNL_INT_TH_LOW))) Whilst I fully agree this looks suspect it needs to be looked at with some more context. The event pushed is rising, which will be incorrect if we see an interrupt indicating the other direction. Also, definitions of high and low get vague for proximity so check the ABI docs. What: /sys/.../iio:deviceX/in_proximity_raw What: /sys/.../iio:deviceX/in_proximity_input What: /sys/.../iio:deviceX/in_proximityY_raw KernelVersion: 3.4 Contact: [email protected] Description: Proximity measurement indicating that some object is near the sensor, usually by observing reflectivity of infrared or ultrasound emitted. Often these sensors are unit less and as such conversion to SI units is not possible. Higher proximity measurements indicate closer objects, and vice versa. Units after application of scale and offset are meters. So rising proximity means getting nearer... Now what does it mean for this sensor - complicated. Figure 3 shows that the when nearer than 1mm the value measured goes up. After that it goes down with distance. So I think that means that for most distances of interest this is the same way around as the IIO definition. So you are right about the missmatch between the rising interrupt and bit checked. So I think to fix this we need - What you have here - To chose which event to emit based which status bit is set (potentially both in some corner cases - Clear up the ISR clear that follows which suffers from same problem you have here. Jonathan > return IRQ_NONE; > > iio_push_event(indio_dev,