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

[Andy Shevchenko <andriy.shevchenko@intel.com>]

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