linux-kernel - Re: [PATCH v5 06/10] dt-bindings: iio: accel: add interrupt-names

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAFXKEHb1NbV-Us3kaNyG+P90SMXsV7233dXd64_gbtCKst6gmQ@mail.gmail.com>
Date: Fri, 6 Dec 2024 18:29:48 +0100
From: Lothar Rubusch <l.rubusch@...il.com>
To: Conor Dooley <conor@...nel.org>
Cc: lars@...afoo.de, Michael.Hennerich@...log.com, jic23@...nel.org, 
	robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org, 
	devicetree@...r.kernel.org, linux-iio@...r.kernel.org, 
	linux-kernel@...r.kernel.org, eraretuya@...il.com
Subject: Re: [PATCH v5 06/10] dt-bindings: iio: accel: add interrupt-names

On Fri, Dec 6, 2024 at 6:08 PM Conor Dooley <conor@...nel.org> wrote:
>
> On Thu, Dec 05, 2024 at 08:41:52PM +0100, Lothar Rubusch wrote:
> > On Thu, Dec 5, 2024 at 6:54 PM Conor Dooley <conor@...nel.org> wrote:
> > >
> > > On Thu, Dec 05, 2024 at 05:13:39PM +0000, Lothar Rubusch wrote:
> > > > Add interrupt-names INT1 and INT2 for the two interrupt lines of the
> > > > sensor. Only one line will be connected for incoming events. The driver
> > > > needs to be configured accordingly. If no interrupt line is set up, the
> > > > sensor will still measure, but no events are possible.
> > > >
> > > > Signed-off-by: Lothar Rubusch <l.rubusch@...il.com>
> > > > ---
> > > >  .../devicetree/bindings/iio/accel/adi,adxl345.yaml         | 7 +++++++
> > > >  1 file changed, 7 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/iio/accel/adi,adxl345.yaml b/Documentation/devicetree/bindings/iio/accel/adi,adxl345.yaml
> > > > index 280ed479ef5..67e2c029a6c 100644
> > > > --- a/Documentation/devicetree/bindings/iio/accel/adi,adxl345.yaml
> > > > +++ b/Documentation/devicetree/bindings/iio/accel/adi,adxl345.yaml
> > > > @@ -37,6 +37,11 @@ properties:
> > > >    interrupts:
> > > >      maxItems: 1
> > > >
> > > > +  interrupt-names:
> > > > +    description: Use either INT1 or INT2 for events, or ignore events.
> > > > +    items:
> > > > +      - enum: [INT1, INT2]
> > >
> > > The description for this ", or ignore events" does not make sense. Just
> > > drop it, it's clear what happens if you don't provide interrupts.
> > >
> > > However, interrupts is a required property but interrupt-names is not.
> > > Seems rather pointless not making interrupt-names a required property
> > > (in the binding!) since if you only add interrupts and not
> > > interrupt-names you can't even use the interrupt as you do not know
> > > whether or not it is INT1 or INT2?
> >
> > What I meant is, yes, the sensor needs an interrupt line.
> > Interrupt-names is optional. The sensor always can measure. When
> > interrupt-names is specified, though, the sensor will setup a FIFO and
> > can use events, such as data ready, watermark, single tap, freefall,
> > etc. Without the interrupt-names, the sensor goes into a "FIFO bypass
> > mode" without its specific events.
>
> What I'm talking about here is how it is ultimately pointless for
> interrupts to be a required property if it can never be used without
> interrupt-names as you cannot know which interrupt is in use. I think
> both should be made mandatory or neither.
>

Ah, now I can see your point. I agree that it should be equally
mandatory as the interrupt. Legacy implementations used simply always
just INT1. I'd like to make it configurable in the IIO driver but
tried to avoid the DT topic for now (which was not a smart decision
either). Hence, I added the interrupt-names.
I'm unsure should I make "interrupt-names" a required property now?
What about the existing DTS files using this sensor? There are no
interrupt-names specified, so if made required, the missing
interrupt-names there would break binding check, or not?

> > Hence, I better drop the description entirely, since it rather seems
> > to be confusing.