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

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The driver configures the interrupt map register based on a hardcoded name priority, but ignores the corresponding IRQ line and registers the default `irq` parameter, risking an IRQ routing mismatch.
--

--- Patch [3]: [PATCH v2 3/3] iio: accel: adxl367: add support for INT2 interrupt pin ---
commit 89ea1f01faa6ff074e5199721996b317c618f006
Author: Antoniu Miclaus <[email protected]>

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. This commit updates the
driver to determine the connected pin from the interrupt-names device tree
property and routes the interrupt mappings to the matching register.

> diff --git a/drivers/iio/accel/adxl367.c b/drivers/iio/accel/adxl367.c
> index 8c3de11a10a37..df385740b17b5 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;
> +	}
> +
> +	irq = fwnode_irq_get_byname(dev_fwnode(st->dev), "INT2");
> +	if (irq == -EPROBE_DEFER)
> +		return irq;
> +	if (irq > 0) {
> +		st->int_map_reg = ADXL367_REG_INT2_MAP;
> +		return 0;
> +	}
> +
> +	/* No interrupt-names: default to INT1 for backwards compatibility. */
> +	st->int_map_reg = ADXL367_REG_INT1_MAP;
> +	return 0;
> +}
> +

[ ... ]

> @@ -1482,6 +1511,10 @@ int adxl367_probe(struct device *dev, const struct adxl367_ops *ops,
>  	if (ret)
>  		return ret;
>  
> +	ret = adxl367_set_int_map_reg(st);
> +	if (ret)
> +		return dev_err_probe(st->dev, ret, "Failed to get interrupt\n");
> +
>  	ret = devm_request_threaded_irq(st->dev, irq, NULL,
>  					adxl367_irq_handler, IRQF_ONESHOT,
>  					indio_dev->name, indio_dev);

[Severity: High]
Could this cause a mismatch between the interrupt map register and the
requested irq line?

If a device tree specifies both interrupts but INT2 is listed first
(index 0), the SPI/I2C core passes the INT2 irq as the irq parameter to
adxl367_probe().

Meanwhile, adxl367_set_int_map_reg() checks INT1 first, configures the
sensor to route interrupts to the INT1 pin, and discards the retrieved
irq number.

Then devm_request_threaded_irq() registers the handler on the original irq
parameter (INT2). Does this result in the device signaling on INT1 while
the OS listens on INT2?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3