Re: [PATCH v2 3/3] iio: accel: adxl367: add support for INT2 interrupt pin

On Sun, Aug 16, 2026 at 01:49:59AM +0100, Jonathan Cameron wrote:
> On Fri, 14 Aug 2026 13:28:29 +0100
> Nuno Sá <[email protected]> wrote:
> 
> > On Thu, 13 Aug 2026 11:14:52 +0300, Antoniu Miclaus <[email protected]> wrote:
> > > diff --git a/drivers/iio/accel/adxl367.c b/drivers/iio/accel/adxl367.c
> > > index 8c3de11a10a3..df385740b17b 100644
> > > --- a/drivers/iio/accel/adxl367.c
> > > +++ b/drivers/iio/accel/adxl367.c
> > > @@ -1426,6 +1430,31 @@ static int adxl367_setup(struct adxl367_state *st)
> > >  	return adxl367_set_measure_en(st, true);
> > >  }
> > >  
> > > +static int adxl367_set_int_map_reg(struct adxl367_state *st)
> > > +{
> > > +	int irq;
> > > +
> > > +	irq = fwnode_irq_get_byname(dev_fwnode(st->dev), "INT1");
> > > +	if (irq == -EPROBE_DEFER)
> > > +		return irq;
> > > +	if (irq > 0) {
> > > +		st->int_map_reg = ADXL367_REG_INT1_MAP;
> > > +		return 0;  
> > 
> > sashiko point is sensible but I guess both INT ouputs are pretty much
> > the same? So even even if there's a mismatch against spi/i2c I guess
> > it is not problematic?
> 
> Sashiko has spotted a real problem here, be it one that won't
> pass the binding restrictions which require INT1 to be first
> if it is present at all.  Anyhow we should be checking for a dts
> that provides bother interrupts but ordered as INT2 INT1 then
> the i2c and spi layers will give us the irq associated with
> the first element (so INT2) but the code here will see that
> there is an INT1 and route the interrupts there.  Hence the miss.
> 

Yes, using the real irq was more or like "implicit" when I said
"even if there's a mismatch against spi/i2c" (which I think it will be
the case). I'm truncating my replies too much :)

- Nuno Sá

> If you are looking up interrupts by name you should use the
> interrupt number retrieved. 
> 
> return irq from adxl367_set_int_map_reg() on success then
> 
> 
> 	ret = adxl367_set_int_map_reg(st);
> 	if (ret < 0)
> 		return dev_err_probe(st->dev, ret, "Failed to get interrupt\n");
> 
> 	irq = ret;
> 	ret = devm_request_threaded_irq(st->dev, irq, NULL,
> 					adxl367_irq_handler, IRQF_ONESHOT,
> 					indio_dev->name, indio_dev);
> 	if (ret)
> 		return dev_err_probe(st->dev, ret, "Failed to request irq\n");
> 
> Or pass it as a pointer so that adxl367_set_int_map_reg() can modify it
> on matching by name. 
> 
> Could argue that we should never see this but I think it's easy to harden
> against, so why not.
> 
> Hmm. On a wide awake day I think I'd have spotted this, but not sure I would have
> today!
> 
> Jonathan
> 
> > 
> > - Nuno Sá
> > 
>