[PATCH v2] iio: proximity: vcnl3020: fix ISR bitmask check in IRQ handler

[PATCH v2] iio: proximity: vcnl3020: fix ISR bitmask check in IRQ handler

[Salah Triki]

The threaded IRQ handler contained multiple issues in handling interrupt
events and clearing status flags:

1. ISR bit check: The handler incorrectly checked the Interrupt Status
   Register (VCNL_ISR) against VCNL_ICR_THRES_EN (BIT(1)), which is a
   bitmask meant for the Control Register (VCNL_PS_ICR). In VCNL_ISR,
   BIT(1) corresponds only to low-threshold interrupts. A high-threshold
   interrupt (VCNL_INT_TH_HI, BIT(0)) on its own was completely ignored and
   returned IRQ_NONE.

2. Event direction & channel index: The handler unconditionally pushed a
   RISING event code on channel index 1. The driver only registers a single
   proximity channel (index 0), and low-threshold interrupts should be
   reported with IIO_EV_DIR_FALLING.

3. ISR clearing: The write-back to acknowledge the interrupt only preserved
   BIT(1) instead of masking against both valid status bits.

Fix this by checking both VCNL_INT_TH_HI and VCNL_INT_TH_LOW bits in
VCNL_ISR, pushing separate IIO events with the correct direction and
channel index (0), and properly clearing handled status bits.

Fixes: 3363fbbe19e5 ("iio: proximity: vcnl3020: add periodic mode")
Signed-off-by: Salah Triki <salah.triki@gmail.com>
---
Changes since v1:
  - Extended the patch to fix additional bugs found in vcnl3020_handle_irq_thread():
    - Corrected event directions (RISING for high threshold, FALLING for low threshold).
    - Fixed channel index from 1 to 0 (matching single proximity channel).
    - Corrected ISR W1C clear logic for both HI and LOW bits.
  - Updated commit message to detail all IRQ handler fixes.

 drivers/iio/proximity/vcnl3020.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/proximity/vcnl3020.c b/drivers/iio/proximity/vcnl3020.c
index 7f417372566a..d0b8f429999f 100644
--- a/drivers/iio/proximity/vcnl3020.c
+++ b/drivers/iio/proximity/vcnl3020.c
@@ -584,16 +584,26 @@ 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)))
 		return IRQ_NONE;
 
-	iio_push_event(indio_dev,
-		       IIO_UNMOD_EVENT_CODE(IIO_PROXIMITY, 1,
-				            IIO_EV_TYPE_THRESH,
-				            IIO_EV_DIR_RISING),
-		       iio_get_time_ns(indio_dev));
+	if (isr & VCNL_INT_TH_HI) {
+		iio_push_event(indio_dev,
+			       IIO_UNMOD_EVENT_CODE(IIO_PROXIMITY, 0,
+						    IIO_EV_TYPE_THRESH,
+						    IIO_EV_DIR_RISING),
+			       iio_get_time_ns(indio_dev));
+	}
+
+	if (isr & VCNL_INT_TH_LOW) {
+		iio_push_event(indio_dev,
+			       IIO_UNMOD_EVENT_CODE(IIO_PROXIMITY, 0,
+						    IIO_EV_TYPE_THRESH,
+						    IIO_EV_DIR_FALLING),
+			       iio_get_time_ns(indio_dev));
+	}
 
-	rc = regmap_write(data->regmap, VCNL_ISR, isr & VCNL_ICR_THRES_EN);
+	rc = regmap_write(data->regmap, VCNL_ISR, isr & (VCNL_INT_TH_HI | VCNL_INT_TH_LOW));
 	if (rc)
 		dev_err(data->dev, "Error (%d) writing in reg (0x%x)\n",
 			rc, VCNL_ISR);
-- 
2.43.0

Re: [PATCH v2] iio: proximity: vcnl3020: fix ISR bitmask check in IRQ handler

[Jonathan Cameron]

On Mon, 24 Aug 2026 05:29:07 +0100
Salah Triki <salah.triki@gmail.com> wrote:

> The threaded IRQ handler contained multiple issues in handling interrupt
> events and clearing status flags:
> 
> 1. ISR bit check: The handler incorrectly checked the Interrupt Status
>    Register (VCNL_ISR) against VCNL_ICR_THRES_EN (BIT(1)), which is a
>    bitmask meant for the Control Register (VCNL_PS_ICR). In VCNL_ISR,
>    BIT(1) corresponds only to low-threshold interrupts. A high-threshold
>    interrupt (VCNL_INT_TH_HI, BIT(0)) on its own was completely ignored and
>    returned IRQ_NONE.
> 
> 2. Event direction & channel index: The handler unconditionally pushed a
>    RISING event code on channel index 1. The driver only registers a single
>    proximity channel (index 0), and low-threshold interrupts should be
>    reported with IIO_EV_DIR_FALLING.
> 
> 3. ISR clearing: The write-back to acknowledge the interrupt only preserved
>    BIT(1) instead of masking against both valid status bits.
> 
> Fix this by checking both VCNL_INT_TH_HI and VCNL_INT_TH_LOW bits in
> VCNL_ISR, pushing separate IIO events with the correct direction and
> channel index (0), and properly clearing handled status bits.
> 
> Fixes: 3363fbbe19e5 ("iio: proximity: vcnl3020: add periodic mode")
> Signed-off-by: Salah Triki <salah.triki@gmail.com>

Trying an active address that is hopefully the same Ivan!

This looks fine to me, but I'd ideally like input from Ivan.

Jonathan

> ---
> Changes since v1:
>   - Extended the patch to fix additional bugs found in vcnl3020_handle_irq_thread():
>     - Corrected event directions (RISING for high threshold, FALLING for low threshold).
>     - Fixed channel index from 1 to 0 (matching single proximity channel).
>     - Corrected ISR W1C clear logic for both HI and LOW bits.
>   - Updated commit message to detail all IRQ handler fixes.
> 
>  drivers/iio/proximity/vcnl3020.c | 24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/proximity/vcnl3020.c b/drivers/iio/proximity/vcnl3020.c
> index 7f417372566a..d0b8f429999f 100644
> --- a/drivers/iio/proximity/vcnl3020.c
> +++ b/drivers/iio/proximity/vcnl3020.c
> @@ -584,16 +584,26 @@ 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)))
>  		return IRQ_NONE;
>  
> -	iio_push_event(indio_dev,
> -		       IIO_UNMOD_EVENT_CODE(IIO_PROXIMITY, 1,
> -				            IIO_EV_TYPE_THRESH,
> -				            IIO_EV_DIR_RISING),
> -		       iio_get_time_ns(indio_dev));
> +	if (isr & VCNL_INT_TH_HI) {
> +		iio_push_event(indio_dev,
> +			       IIO_UNMOD_EVENT_CODE(IIO_PROXIMITY, 0,
> +						    IIO_EV_TYPE_THRESH,
> +						    IIO_EV_DIR_RISING),
> +			       iio_get_time_ns(indio_dev));
> +	}
> +
> +	if (isr & VCNL_INT_TH_LOW) {
> +		iio_push_event(indio_dev,
> +			       IIO_UNMOD_EVENT_CODE(IIO_PROXIMITY, 0,
> +						    IIO_EV_TYPE_THRESH,
> +						    IIO_EV_DIR_FALLING),
> +			       iio_get_time_ns(indio_dev));
> +	}
>  
> -	rc = regmap_write(data->regmap, VCNL_ISR, isr & VCNL_ICR_THRES_EN);
> +	rc = regmap_write(data->regmap, VCNL_ISR, isr & (VCNL_INT_TH_HI | VCNL_INT_TH_LOW));
>  	if (rc)
>  		dev_err(data->dev, "Error (%d) writing in reg (0x%x)\n",
>  			rc, VCNL_ISR);

Re: [PATCH v2] iio: proximity: vcnl3020: fix ISR bitmask check in IRQ handler

[Ivan Mikhaylov]

On Tue, 2026-09-01 at 02:27 +0100, Jonathan Cameron wrote:
> On Mon, 24 Aug 2026 05:29:07 +0100
> Salah Triki <salah.triki@gmail.com> wrote:
> 
> > The threaded IRQ handler contained multiple issues in handling
> > interrupt
> > events and clearing status flags:
> > 
> > 1. ISR bit check: The handler incorrectly checked the Interrupt
> > Status
> >    Register (VCNL_ISR) against VCNL_ICR_THRES_EN (BIT(1)), which is
> > a
> >    bitmask meant for the Control Register (VCNL_PS_ICR). In
> > VCNL_ISR,
> >    BIT(1) corresponds only to low-threshold interrupts. A high-
> > threshold
> >    interrupt (VCNL_INT_TH_HI, BIT(0)) on its own was completely
> > ignored and
> >    returned IRQ_NONE.
> > 
> > 2. Event direction & channel index: The handler unconditionally
> > pushed a
> >    RISING event code on channel index 1. The driver only registers
> > a single
> >    proximity channel (index 0), and low-threshold interrupts should
> > be
> >    reported with IIO_EV_DIR_FALLING.
> > 
> > 3. ISR clearing: The write-back to acknowledge the interrupt only
> > preserved
> >    BIT(1) instead of masking against both valid status bits.
> > 
> > Fix this by checking both VCNL_INT_TH_HI and VCNL_INT_TH_LOW bits
> > in
> > VCNL_ISR, pushing separate IIO events with the correct direction
> > and
> > channel index (0), and properly clearing handled status bits.
> > 
> > Fixes: 3363fbbe19e5 ("iio: proximity: vcnl3020: add periodic mode")
> > Signed-off-by: Salah Triki <salah.triki@gmail.com>
> 
> Trying an active address that is hopefully the same Ivan!
> 
> This looks fine to me, but I'd ideally like input from Ivan.
> 
> Jonathan
> 
> > ---
> > Changes since v1:
> >   - Extended the patch to fix additional bugs found in
> > vcnl3020_handle_irq_thread():
> >     - Corrected event directions (RISING for high threshold,
> > FALLING for low threshold).
> >     - Fixed channel index from 1 to 0 (matching single proximity
> > channel).
> >     - Corrected ISR W1C clear logic for both HI and LOW bits.
> >   - Updated commit message to detail all IRQ handler fixes.
> > 
> >  drivers/iio/proximity/vcnl3020.c | 24 +++++++++++++++++-------
> >  1 file changed, 17 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/iio/proximity/vcnl3020.c
> > b/drivers/iio/proximity/vcnl3020.c
> > index 7f417372566a..d0b8f429999f 100644
> > --- a/drivers/iio/proximity/vcnl3020.c
> > +++ b/drivers/iio/proximity/vcnl3020.c
> > @@ -584,16 +584,26 @@ 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)))
> >  		return IRQ_NONE;
> >  
> > -	iio_push_event(indio_dev,
> > -		       IIO_UNMOD_EVENT_CODE(IIO_PROXIMITY, 1,
> > -				            IIO_EV_TYPE_THRESH,
> > -				            IIO_EV_DIR_RISING),
> > -		       iio_get_time_ns(indio_dev));
> > +	if (isr & VCNL_INT_TH_HI) {
> > +		iio_push_event(indio_dev,
> > +			       IIO_UNMOD_EVENT_CODE(IIO_PROXIMITY,
> > 0,
> > +						   
> > IIO_EV_TYPE_THRESH,
> > +						   
> > IIO_EV_DIR_RISING),
> > +			       iio_get_time_ns(indio_dev));
> > +	}
> > +
> > +	if (isr & VCNL_INT_TH_LOW) {
> > +		iio_push_event(indio_dev,
> > +			       IIO_UNMOD_EVENT_CODE(IIO_PROXIMITY,
> > 0,
> > +						   
> > IIO_EV_TYPE_THRESH,
> > +						   
> > IIO_EV_DIR_FALLING),
> > +			       iio_get_time_ns(indio_dev));
> > +	}
> >  
> > -	rc = regmap_write(data->regmap, VCNL_ISR, isr &
> > VCNL_ICR_THRES_EN);
> > +	rc = regmap_write(data->regmap, VCNL_ISR, isr &
> > (VCNL_INT_TH_HI | VCNL_INT_TH_LOW));
> >  	if (rc)
> >  		dev_err(data->dev, "Error (%d) writing in reg
> > (0x%x)\n",
> >  			rc, VCNL_ISR);

Jonathan, thanks for pinging me, yes, address still same, I'll check
patch until end of the week.

Thanks.