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

[David Lechner]

On 8/21/26 1:07 PM, Antoniu Miclaus wrote:
> 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>
> ---
> Changes in v3:
> - Use the interrupt number returned by fwnode_irq_get_byname() for
>   devm_request_threaded_irq() so the requested line always matches the
>   routed INTMAP register, regardless of the interrupt-names order.
> - Access the interrupt map register via regmap_assign_bits() following
>   the new precursor patch.
> 
>  drivers/iio/accel/adxl367.c | 40 ++++++++++++++++++++++++++++++++++---
>  1 file changed, 37 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/accel/adxl367.c b/drivers/iio/accel/adxl367.c
> index 54c4d1226790..f5808980b639 100644
> --- a/drivers/iio/accel/adxl367.c
> +++ b/drivers/iio/accel/adxl367.c
> @@ -13,6 +13,7 @@
>  #include <linux/iio/sysfs.h>
>  #include <linux/interrupt.h>
>  #include <linux/irq.h>
> +#include <linux/property.h>
>  #include <linux/regmap.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/unaligned.h>
> @@ -81,6 +82,7 @@
>  #define ADXL367_SAMPLES_L_MASK		GENMASK(7, 0)
>  
>  #define ADXL367_REG_INT1_MAP		0x2A
> +#define ADXL367_REG_INT2_MAP		0x2B
>  #define ADXL367_INT_INACT_MASK		BIT(5)
>  #define ADXL367_INT_ACT_MASK		BIT(4)
>  #define ADXL367_INT_FIFO_WATERMARK_MASK	BIT(2)
> @@ -168,6 +170,8 @@ struct adxl367_state {
>  	enum adxl367_odr	odr;
>  	enum adxl367_range	range;
>  
> +	u8		int_map_reg;
> +
>  	unsigned int	act_threshold;
>  	unsigned int	act_time_ms;
>  	unsigned int	inact_threshold;
> @@ -366,7 +370,7 @@ static int adxl367_set_act_interrupt_en(struct adxl367_state *st,
>  {
>  	unsigned int mask = adxl367_act_int_mask_tbl[act];
>  
> -	return regmap_assign_bits(st->regmap, ADXL367_REG_INT1_MAP, mask, en);
> +	return regmap_assign_bits(st->regmap, st->int_map_reg, mask, en);
>  }
>  
>  static int adxl367_get_act_interrupt_en(struct adxl367_state *st,
> @@ -377,7 +381,7 @@ static int adxl367_get_act_interrupt_en(struct adxl367_state *st,
>  	unsigned int val;
>  	int ret;
>  
> -	ret = regmap_read(st->regmap, ADXL367_REG_INT1_MAP, &val);
> +	ret = regmap_read(st->regmap, st->int_map_reg, &val);
>  	if (ret)
>  		return ret;
>  
> @@ -400,7 +404,7 @@ static int adxl367_set_act_en(struct adxl367_state *st,
>  static int adxl367_set_fifo_watermark_interrupt_en(struct adxl367_state *st,
>  						   bool en)
>  {
> -	return regmap_assign_bits(st->regmap, ADXL367_REG_INT1_MAP,
> +	return regmap_assign_bits(st->regmap, st->int_map_reg,
>  				  ADXL367_INT_FIFO_WATERMARK_MASK, en);
>  }
>  
> @@ -1423,6 +1427,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)
> +{
> +	int ret;
> +
> +	ret = fwnode_irq_get_byname(dev_fwnode(st->dev), "INT1");
> +	if (ret == -EPROBE_DEFER)
> +		return ret;
> +	if (ret > 0) {
> +		st->int_map_reg = ADXL367_REG_INT1_MAP;
> +		return ret;
> +	}
> +
> +	ret = fwnode_irq_get_byname(dev_fwnode(st->dev), "INT2");
> +	if (ret == -EPROBE_DEFER)
> +		return ret;
> +	if (ret > 0) {
> +		st->int_map_reg = ADXL367_REG_INT2_MAP;
> +		return ret;
> +	}
> +
> +	/* No interrupt-names: default to INT1 for backwards compatibility. */
> +	st->int_map_reg = ADXL367_REG_INT1_MAP;
> +	return irq;

I guess this works, but it seems like trial and error rather than being
purposeful. 

Could probably do something like this too:

	if (device_property_present("interrupt-names")) {
		ret = device_property_match_string(st->dev, "interrupt-names",
						   "INT2");
		if (ret < 0)
			return ret;

		/* If INT2 is at index 0, use it, otherwise use INT1. */
		st->int_map_reg = ret ? ADXL367_REG_INT2_MAP : ADXL367_REG_INT1_MAP;
	} else {
		/* No interrupt-names: default to INT1 for backwards compatibility. */
		st->int_map_reg = ADXL367_REG_INT1_MAP;
	}


	ret = fwnode_irq_get_byname(dev_fwnode(st->dev),
				    st->int_map_reg == ADXL367_REG_INT1_MAP ?
				    "INT1" : "INT2");


(Could use device_property_match_property_string() instead if we want to
return error on invalid values instead of assuming INT1.)

> +}
> +
>  int adxl367_probe(struct device *dev, const struct adxl367_ops *ops,
>  		  void *context, struct regmap *regmap, int irq)
>  {
> @@ -1479,6 +1508,11 @@ int adxl367_probe(struct device *dev, const struct adxl367_ops *ops,
>  	if (ret)
>  		return ret;
>  
> +	ret = adxl367_set_int_map_reg(st, irq);
> +	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);