Re: [PATCH v4 4/4] iio: accel: adxl367: add support for INT2 interrupt pin

[Jonathan Cameron <jic23@kernel.org>]

On Fri, 4 Sep 2026 09:14:53 +0000
"Miclaus, Antoniu" <Antoniu.Miclaus@analog.com> wrote:

> --
> Antoniu Miclăuş
> 
> > -----Original Message-----
> > From: Jonathan Cameron <jic23@kernel.org>
> > Sent: Tuesday, September 1, 2026 7:41 PM
> > To: Miclaus, Antoniu <Antoniu.Miclaus@analog.com>
> > Cc: Schmitt, Marcelo <Marcelo.Schmitt@analog.com>; Sa, Nuno
> > <Nuno.Sa@analog.com>; Hennerich, Michael
> > <Michael.Hennerich@analog.com>; Jonathan Cameron <jic23@kernel.org>;
> > David Lechner <dlechner@baylibre.com>; Rob Herring <robh@kernel.org>;
> > Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> > <conor+dt@kernel.org>; linux-iio@vger.kernel.org; linux <linux@analog.com>;
> > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH v4 4/4] iio: accel: adxl367: add support for INT2 interrupt
> > pin
> >   
> > > The ADXL367 provides two independent interrupt output pins, INT1 and
> > > INT2, each with its own event mapping register (INTMAP1_LOWER at 0x2A
> > > and INTMAP2_LOWER at 0x2B) sharing an identical bit layout. Until now
> > > the driver hardcoded INT1 for all interrupt mappings, so a board that
> > > routes only INT2 to the host could never receive activity, inactivity
> > > or FIFO watermark interrupts.
> > >
> > > Determine the connected pin from the interrupt-names device tree
> > > property using fwnode_irq_get_byname(), and route the interrupt
> > > mappings to the matching register. Use the interrupt number returned by
> > > the lookup for devm_request_threaded_irq() so the requested line always
> > > matches the routed INTMAP register, regardless of the order in which the
> > > interrupts are listed. When no interrupt-names are present, default to
> > > INT1 and the bus-provided interrupt to preserve the existing behaviour.
> > >
> > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> > >  
> > 
> > One follow on thing to look at via sashiko.
> > 
> > Otherwise, this looks good to me, but given the active discussion on previous
> > version I'm not going to pick this up until plenty of time has passed
> > or tags have been given.  
> 
> Ok, I will send a new version addressing also the sashiko issue in a follow up patch.

No need for a new version of these - I'll pick them up now.  Just send a new
patch on top.  That might break sashiko though, so you could send these + the
new patch as new version - just stick 'applied' or something like that in the patch
titles so no one spends time reviewing them.

Applied to the testing branch of iio.git

Thanks,

Jonathan

> 
> >   
> > > diff --git a/drivers/iio/accel/adxl367.c b/drivers/iio/accel/adxl367.c
> > > index b4e8dc6d6a5c..9b47e66c49ea 100644
> > > --- a/drivers/iio/accel/adxl367.c
> > > +++ b/drivers/iio/accel/adxl367.c  
> >   
> > > @@ -1430,6 +1434,36 @@ 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)
> > > +{
> > > +	int ret;
> > > +
> > > +	/*
> > > +	 * Route the mappings to whichever of INT1/INT2 is named in the
> > > +	 * interrupt-names property. -EINVAL (property absent) and -  
> > ENODATA  
> > > +	 * (name not listed) are not errors; anything else is.
> > > +	 */
> > > +	ret = fwnode_irq_get_byname(dev_fwnode(st->dev), "INT1");
> > > +	if (ret > 0) {
> > > +		st->int_map_reg = ADXL367_REG_INT1_MAP;
> > > +		return ret;
> > > +	}
> > > +	if (ret != -ENODATA && ret != -EINVAL)
> > > +		return ret;
> > > +
> > > +	ret = fwnode_irq_get_byname(dev_fwnode(st->dev), "INT2");
> > > +	if (ret > 0) {
> > > +		st->int_map_reg = ADXL367_REG_INT2_MAP;
> > > +		return ret;
> > > +	}
> > > +	if (ret != -ENODATA && ret != -EINVAL)
> > > +		return ret;
> > > +
> > > +	/* No INT1/INT2 named: default to INT1 and the bus interrupt. */
> > > +	st->int_map_reg = ADXL367_REG_INT1_MAP;
> > > +	return irq;  
> > 
> > I'd like to explore the helper idea for this, but it can be a separate
> > series when someone has the time.
> >   
> > > +}
> > > +
> > >  int adxl367_probe(struct device *dev, const struct adxl367_ops *ops,
> > >  		  void *context, struct regmap *regmap, int irq)
> > >  {
> > > @@ -1486,6 +1520,11 @@ int adxl367_probe(struct device *dev, const  
> > struct adxl367_ops *ops,  
> > >  	if (ret)
> > >  		return ret;
> > >  
> > 
> > [Severity: Medium]
> > Since fwnode_irq_get_byname() inside adxl367_set_int_map_reg() can return
> > -EPROBE_DEFER if the interrupt controller is not yet ready, will placing this
> > lookup after adxl367_setup() cause repeated boot delays and leave the device
> > powered up on deferral?
> > 
> > Looking at adxl367_probe(), it resets the hardware, sleeps for 15ms, and calls
> > adxl367_setup() which powers up the device into measurement mode.
> > 
> > If adxl367_set_int_map_reg() then returns -EPROBE_DEFER, the probe aborts
> > without an error cleanup path to power down the device.
> > 
> > Could this software resource acquisition be performed before the hardware
> > interaction to avoid unnecessary boot stalls and power leaks during probe
> > deferral?
> > 
> > -
> > 
> > This obviously isn't new, but it might make sense to turn the device
> > off at least.
> > 
> > Can be a follow up patch.
> > 
> > --
> > Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>