All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Nuno Sá" <nuno.sa@analog.com>
To: Jonathan Cameron <jic23@kernel.org>
Cc: Antoniu Miclaus <antoniu.miclaus@analog.com>,
	Marcelo Schmitt <marcelo.schmitt@analog.com>,
	Michael Hennerich <Michael.Hennerich@analog.com>,
	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@analog.com,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 3/3] iio: accel: adxl367: add support for INT2 interrupt pin
Date: Mon, 17 Aug 2026 09:42:53 +0100	[thread overview]
Message-ID: <aoLI-TVgOD4PqVHj@nsa> (raw)
In-Reply-To: <20260816014959.18034ff6@jic23-huawei>

On Sun, Aug 16, 2026 at 01:49:59AM +0100, Jonathan Cameron wrote:
> On Fri, 14 Aug 2026 13:28:29 +0100
> Nuno Sá <nuno.sa@analog.com> wrote:
> 
> > On Thu, 13 Aug 2026 11:14:52 +0300, Antoniu Miclaus <antoniu.miclaus@analog.com> 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á
> > 
> 

  reply	other threads:[~2026-08-17  8:41 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-13  8:14 [PATCH v2 0/3] iio: accel: adxl367: add INT2 interrupt pin support Antoniu Miclaus
2026-08-13  8:14 ` [PATCH v2 1/3] iio: accel: adxl367: update maintainer entries Antoniu Miclaus
2026-08-14  9:23   ` Krzysztof Kozlowski
2026-08-14 12:28   ` Nuno Sá
2026-08-13  8:14 ` [PATCH v2 2/3] dt-bindings: iio: accel: adi,adxl367: add interrupt-names Antoniu Miclaus
2026-08-13  8:14 ` [PATCH v2 3/3] iio: accel: adxl367: add support for INT2 interrupt pin Antoniu Miclaus
2026-08-13  8:28   ` sashiko-bot
2026-08-14 12:28   ` Nuno Sá
2026-08-16  0:49     ` Jonathan Cameron
2026-08-17  8:42       ` Nuno Sá [this message]
2026-08-16  0:39   ` Jonathan Cameron

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aoLI-TVgOD4PqVHj@nsa \
    --to=nuno.sa@analog.com \
    --cc=Michael.Hennerich@analog.com \
    --cc=antoniu.miclaus@analog.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=jic23@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@analog.com \
    --cc=marcelo.schmitt@analog.com \
    --cc=robh@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.