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

[Miclaus, Antoniu]

--
Antoniu Miclăuş

> -----Original Message-----
> From: Jonathan Cameron <jic23@xxxxxxxxxx>
> Sent: Tuesday, September 1, 2026 7:41 PM
> To: Miclaus, Antoniu <Antoniu.Miclaus@xxxxxxxxxx>
> Cc: Schmitt, Marcelo <Marcelo.Schmitt@xxxxxxxxxx>; Sa, Nuno
> <Nuno.Sa@xxxxxxxxxx>; Hennerich, Michael
> <Michael.Hennerich@xxxxxxxxxx>; Jonathan Cameron <jic23@xxxxxxxxxx>;
> David Lechner <dlechner@xxxxxxxxxxxx>; Rob Herring <robh@xxxxxxxxxx>;
> Krzysztof Kozlowski <krzk+dt@xxxxxxxxxx>; Conor Dooley
> <conor+dt@xxxxxxxxxx>; linux-iio@xxxxxxxxxxxxxxx; linux <linux@xxxxxxxxxx>;
> devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> 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@xxxxxxxxxx>
> >
> 
> 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.

> 
> > 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@xxxxxxxxxxxxxxxx>