[PATCH 0/2] iio: light: vcnl4000: shared IRQ support

[PATCH 0/2] iio: light: vcnl4000: shared IRQ support

[Tsz Shan Chan]

This series container two small changes for the vcnl4000 driver.

Patch 1 makes the iio channel count match the channel array.

Patch 2 adds support for shared interrupt lines. The driver now requests
the IRQ as shared and the handler returns IRQ_NONE when the interrupt
status register shows no pending interrupt, so shared IRQ work
correctly. It also reads the irq parent trigger type from firmware,
falling back to IRQF_TRIGGER_FALLING when one is set to keep the current
behaviour.

Signed-off-by: Tsz Shan Chan <tchan@jacques.com.au>
---
Tsz Shan Chan (2):
      iio: light: vcnl4000: use correct channel array size
      iio: light: vcnl4000: add shared IRQ support

 drivers/iio/light/vcnl4000.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)
---
base-commit: db2ddb87143519e20a95aa36c60b36107b736a58
change-id: 20260810-vcnl4000-02e19f70e706

Best regards,
-- 
Tsz Shan Chan <tchan@jacques.com.au>

[PATCH 1/2] iio: light: vcnl4000: use correct channel array size

[Tsz Shan Chan]

Use ARRAY_SIZE(vcnl4040_channels) for num_channels to match .channels.

Signed-off-by: Tsz Shan Chan <tchan@jacques.com.au>
---
 drivers/iio/light/vcnl4000.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
index 128ae3f94074..663e623da833 100644
--- a/drivers/iio/light/vcnl4000.c
+++ b/drivers/iio/light/vcnl4000.c
@@ -1874,7 +1874,7 @@ static const struct vcnl4000_chip_spec vcnl4200_spec = {
 	.measure_proximity = vcnl4200_measure_proximity,
 	.set_power_state = vcnl4200_set_power_state,
 	.channels = vcnl4040_channels,
-	.num_channels = ARRAY_SIZE(vcnl4000_channels),
+	.num_channels = ARRAY_SIZE(vcnl4040_channels),
 	.info = &vcnl4040_info,
 	.irq_thread = vcnl4040_irq_thread,
 	.int_reg = VCNL4200_INT_FLAGS,

-- 
2.55.0

[PATCH 2/2] iio: light: vcnl4000: add shared IRQ support

[Tsz Shan Chan]

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.

Request the interrupt with IRQF_SHARED, and return IRQ_NONE in the irq
handler when there is no interrupt pending.

Signed-off-by: Tsz Shan Chan <tchan@jacques.com.au>
---
 drivers/iio/light/vcnl4000.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
index 663e623da833..9fff00ae515e 100644
--- a/drivers/iio/light/vcnl4000.c
+++ b/drivers/iio/light/vcnl4000.c
@@ -1469,8 +1469,8 @@ static irqreturn_t vcnl4040_irq_thread(int irq, void *p)
 	int ret;
 
 	ret = i2c_smbus_read_word_data(data->client, data->chip_spec->int_reg);
-	if (ret < 0)
-		return IRQ_HANDLED;
+	if (ret <= 0)
+		return IRQ_NONE;
 
 	if (ret & VCNL4040_PS_IF_CLOSE) {
 		iio_push_event(indio_dev,
@@ -1525,8 +1525,8 @@ static irqreturn_t vcnl4010_irq_thread(int irq, void *p)
 	int ret;
 
 	ret = i2c_smbus_read_byte_data(data->client, VCNL4010_ISR);
-	if (ret < 0)
-		goto end;
+	if (ret <= 0)
+		return IRQ_NONE;
 
 	isr = ret;
 
@@ -1558,7 +1558,6 @@ static irqreturn_t vcnl4010_irq_thread(int irq, void *p)
 	if (isr & VCNL4010_INT_DRDY && iio_buffer_enabled(indio_dev))
 		iio_trigger_poll_nested(indio_dev->trig);
 
-end:
 	return IRQ_HANDLED;
 }
 
@@ -1979,10 +1978,24 @@ static int vcnl4000_probe(struct i2c_client *client)
 	}
 
 	if (client->irq && data->chip_spec->irq_thread) {
+		u32 irq_type = irq_get_trigger_type(client->irq);
+
+		switch (irq_type) {
+		case IRQF_TRIGGER_FALLING:
+		case IRQF_TRIGGER_LOW:
+			break;
+		case IRQF_TRIGGER_NONE:
+			irq_type = IRQF_TRIGGER_FALLING;
+			break;
+		default:
+			return dev_err_probe(dev, -EINVAL,
+					"unsupported irq trigger type %x\n",
+					irq_type);
+		}
 		ret = devm_request_threaded_irq(dev, client->irq, NULL,
 						data->chip_spec->irq_thread,
-						IRQF_TRIGGER_FALLING |
-						IRQF_ONESHOT,
+						IRQF_ONESHOT | IRQF_SHARED |
+						irq_type,
 						"vcnl4000_irq",
 						indio_dev);
 		if (ret < 0)

-- 
2.55.0

Re: [PATCH 2/2] iio: light: vcnl4000: add shared IRQ support

[Andy Shevchenko]

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

Re: [PATCH 1/2] iio: light: vcnl4000: use correct channel array size

[Andy Shevchenko]

On Tue, Aug 11, 2026 at 05:07:24PM +1000, Tsz Shan Chan wrote:
> Use ARRAY_SIZE(vcnl4040_channels) for num_channels to match .channels.

I was under impression that I have seen this patch a few months ago.
In any case sounds like a fix and hence needs a Fixes tag.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/2] iio: light: vcnl4000: use correct channel array size

[Jonathan Cameron]

On Tue, 11 Aug 2026 12:53:22 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Tue, Aug 11, 2026 at 05:07:24PM +1000, Tsz Shan Chan wrote:
> > Use ARRAY_SIZE(vcnl4040_channels) for num_channels to match .channels.  
> 
> I was under impression that I have seen this patch a few months ago.
> In any case sounds like a fix and hence needs a Fixes tag.
> 
They are both the same size == 2 I think.

So whilst nice to fix this I don't see it as needing a fixes tag
as such.  It would be helpful if the patch description called
out that the ARRAY_SIZE() of the wrong array happens to match the
one it should be.

Jonathan

Re: [PATCH 2/2] iio: light: vcnl4000: add shared IRQ support

[Jonathan Cameron]

On Tue, 11 Aug 2026 12:49:37 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> 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.  
> 
> ...
A couple of follow ups to add a few more things to what Any called out.

> 
> >  	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?

I'd definitely prefer to see the error case separately handled from the
no known interrupts.  That no interrupt check should probably also
only be the ones we have support for, so something like:

	if (ret < 0)
		return IRQ_NONE;

	if (!(ret & (VCNL4040_PS_IF_CLOSE | VCNL4040_PS_IF_AWAY |
		     VCNL4040_ALS_FALLING | VCNL4040_ALS_RISING)))
		return IRQ_NONE;

or something along those lines.
	

> 
> > +		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.

snap :)

> 
> >  	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.

Would indeed be unusual to put it lightly!

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