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

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

[Tsz Shan Chan]

On Wed, Aug 12, 2026 at 04:39:14AM +0100, Jonathan Cameron wrote:
> 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
> 
> 

Thanks. Will update the commit message and send a v2.

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

[Tsz Shan Chan]

On Wed, Aug 12, 2026 at 04:50:02AM +0100, Jonathan Cameron wrote:
> 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?

In the vcnl4010/vcnl4020 datasheet:
- Page 5 (Application circuit Notes): The interrupt pin is an open drain
  output.

In the vcnl4040/vcnl4200 datasheet:
- Page 7 (Fig. 11 - Hardware Pin Connection Diagram) shows INT pin
  connected to V_Pull_up with an 8.2k resistor
- Page 13 (Interruption Section): the level of INT pin (pin 6/8) is pulled
  low once an interrupt event has been triggered.

This confirms that the interrupt line is an open drain active low
output, so both IRQF_TRIGGER_LOW and IRQF_TRIGGER_FALLING (on a
dedicated unshared INT line) are both valid parent trigger type.
The sensor doesn't know about the parent trigger type and simply
pulls the INT line down when an event happens.

> > 
> > > 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 :)

The (ret <= 0) checks for two cases:
1. ret < 0: I2C read error. Cannot confirm this device caused the
   interrupt, so returning IRQ_NONE is safer for a shared line
2. ret = 0: No interrupt flag was set. This means no interrupt pending
   so this device didn't cause the interrupt, so return IRQ_NONE.

Combining both into (ret <= 0) was confusing. I will split these checks,
use bitmake and update the commit message in v2.


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

I agree that edge triggers should not be shared. My goal was to support
level triggers for shared interrupt line without silently changing the
trigger type for other setups that do not share irq line.

In my understanding, IRQF_SHARED does not force sharing, only enables
the capabilities. IRQF_TRIGGER_FALLING was kept for existing setups
using dedicated irq line. IRQF_TRIGGER_LOW was added so shared lines can
work reliably without missing interrupts.

Or would it be better to completely drop the trigger type check? Simply
pass IRQF_SHARED | IRQF_ONESHOT and let the kernel use whatever trigger
type the firmware configures.


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

Will fix them in v2.