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

On Thu, Aug 13, 2026 at 12:37:57PM +1000, Tsz Shan Chan wrote:
> On Wed, Aug 12, 2026 at 04:50:02AM +0100, Jonathan Cameron wrote:
> > On Tue, 11 Aug 2026 12:49:37 +0300
> > Andy Shevchenko <[email protected]> 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.

I see, so why do we even bother with the _FALLING then? Make it trigger LOW
when it's not defined and always enable sharing.

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

...

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

You can't avoid possible regressions for the cases when firmware neglected to
set the trigger type completely.

-- 
With Best Regards,
Andy Shevchenko