diff mbox series

[v5,06/10] dt-bindings: iio: accel: add interrupt-names

Message ID 20241205171343.308963-7-l.rubusch@gmail.com (mailing list archive)
State Changes Requested
Headers show
Series iio: accel: adxl345: add FIFO operating with IRQ triggered watermark events | expand

Commit Message

Lothar Rubusch Dec. 5, 2024, 5:13 p.m. UTC
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@gmail.com>
---
 .../devicetree/bindings/iio/accel/adi,adxl345.yaml         | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Conor Dooley Dec. 5, 2024, 5:53 p.m. UTC | #1
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@gmail.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?

> +
>  required:
>    - compatible
>    - reg
> @@ -61,6 +66,7 @@ examples:
>              reg = <0x2a>;
>              interrupt-parent = <&gpio0>;
>              interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
> +            interrupt-names = "INT1";
>          };
>      };
>    - |
> @@ -79,5 +85,6 @@ examples:
>              spi-cpha;
>              interrupt-parent = <&gpio0>;
>              interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
> +            interrupt-names = "INT2";
>          };
>      };
> -- 
> 2.39.2
>
Lothar Rubusch Dec. 5, 2024, 7:41 p.m. UTC | #2
On Thu, Dec 5, 2024 at 6:54 PM Conor Dooley <conor@kernel.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@gmail.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.

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

Best,
L

> > +
> >  required:
> >    - compatible
> >    - reg
> > @@ -61,6 +66,7 @@ examples:
> >              reg = <0x2a>;
> >              interrupt-parent = <&gpio0>;
> >              interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
> > +            interrupt-names = "INT1";
> >          };
> >      };
> >    - |
> > @@ -79,5 +85,6 @@ examples:
> >              spi-cpha;
> >              interrupt-parent = <&gpio0>;
> >              interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
> > +            interrupt-names = "INT2";
> >          };
> >      };
> > --
> > 2.39.2
> >
Conor Dooley Dec. 6, 2024, 5:07 p.m. UTC | #3
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@kernel.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@gmail.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.

> Hence, I better drop the description entirely, since it rather seems
> to be confusing.
Lothar Rubusch Dec. 6, 2024, 5:29 p.m. UTC | #4
On Fri, Dec 6, 2024 at 6:08 PM Conor Dooley <conor@kernel.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@kernel.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@gmail.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.
Lothar Rubusch Dec. 7, 2024, 12:10 p.m. UTC | #5
On Fri, Dec 6, 2024 at 6:29 PM Lothar Rubusch <l.rubusch@gmail.com> wrote:
>
> On Fri, Dec 6, 2024 at 6:08 PM Conor Dooley <conor@kernel.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@kernel.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@gmail.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?
>

Sorry, I have to clarify myself, yesterday I was not focussed..

1. I agree this is kind of half way. Either, both are required or none of them.
If both were required, also the older DTS files using the ADXL345 would
need to be "fixed". If I add interrupt-names, it works with my patches for the
"newer" IIO driver, because since I implement it it's using interrupt-names.
The older input driver for that using interrupt, does not use interrupt-names.
Hence, it requires the interrupt in the DT. But it does not require
interrupt-names
(historical stuff).

2. AFAIK the sensor can operate w/o interrupts.
A) w/o INT line: measuring is possible; FIFO bypassed; no events
B) w/ INT line: measuring is possible; can use FIFO; events are possible
When setting the interrupt in DT, the interrupt line name can/could be
configured also via SW (setting up the registers of the sensor). So, it's not
impossible. This is AFAIR the approach in the legacy input driver. Now, there
is devicetree, and both should probably be better configured somewhere in
the DT

3. IMHO neither one, not the interrupt, nor the interrupt-names need to be
a required DT-binding.
If interrupt is required and interrupt-names not, it's a half-way approach,
which leaves specifying the IRQ line open to be solved partly in DT
(declaration of the interrupt) and partly in SW (configuration of the
interrupt line to use), e.g. hardcoded or configurable somewhere in the
driver via sysfs or the like. Not nice.

Pls, let me know what you think, and in case, if I need to take some
action, here.
Best,
L


> > > Hence, I better drop the description entirely, since it rather seems
> > > to be confusing.
Jonathan Cameron Dec. 8, 2024, 1:14 p.m. UTC | #6
On Fri, 6 Dec 2024 18:29:48 +0100
Lothar Rubusch <l.rubusch@gmail.com> wrote:

> On Fri, Dec 6, 2024 at 6:08 PM Conor Dooley <conor@kernel.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@kernel.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@gmail.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
Side note, but patch name must include what device it is!

dt-bindings: iio: accel: adxl345: ...

> > > > > 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?

Neither should be required.  The driver isn't currently using interrupts
and I presume it is functional?  So I'd just drop the required on interrupts.
Now a condition that says interrupt-names is needed if interrupts is supplied
would be a useful addition (IIRC there are examples of that in tree).

So interrupts being required is a bug that we should fix by just
dropping that.  

Jonathan



> 
> > > Hence, I better drop the description entirely, since it rather seems
> > > to be confusing.
Jonathan Cameron Dec. 8, 2024, 1:21 p.m. UTC | #7
On Sat, 7 Dec 2024 13:10:22 +0100
Lothar Rubusch <l.rubusch@gmail.com> wrote:

> On Fri, Dec 6, 2024 at 6:29 PM Lothar Rubusch <l.rubusch@gmail.com> wrote:
> >
> > On Fri, Dec 6, 2024 at 6:08 PM Conor Dooley <conor@kernel.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@kernel.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@gmail.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?
> >  
> 
> Sorry, I have to clarify myself, yesterday I was not focussed..
> 
> 1. I agree this is kind of half way. Either, both are required or none of them.
> If both were required, also the older DTS files using the ADXL345 would
> need to be "fixed".

Easy. If they aren't both provided, no interrupts are used.
Driver carries on working bug less functionality.  That's fine.

> If I add interrupt-names, it works with my patches for the
> "newer" IIO driver, because since I implement it it's using interrupt-names.
> The older input driver for that using interrupt, does not use interrupt-names.
> Hence, it requires the interrupt in the DT. But it does not require
> interrupt-names
> (historical stuff).

We don't care.  The required list should be about requirements for the
hardware to function in a useful fashion, not if the driver currently supports
that mode.  So it should never have been required even if the driver at the
time required it because no one had done the work to make it work without.

In theory you could provide a default for interrupt-names I guess if
do want to be nice to the legacy driver.

> 
> 2. AFAIK the sensor can operate w/o interrupts.
> A) w/o INT line: measuring is possible; FIFO bypassed; no events
> B) w/ INT line: measuring is possible; can use FIFO; events are possible
> When setting the interrupt in DT, the interrupt line name can/could be
> configured also via SW (setting up the registers of the sensor). So, it's not
> impossible. This is AFAIR the approach in the legacy input driver. Now, there
> is devicetree, and both should probably be better configured somewhere in
> the DT

Agreed no interrupts are required for device to do something useful.
(not sure I follow the rest of this entry).
> 
> 3. IMHO neither one, not the interrupt, nor the interrupt-names need to be
> a required DT-binding.
> If interrupt is required and interrupt-names not, it's a half-way approach,
> which leaves specifying the IRQ line open to be solved partly in DT
> (declaration of the interrupt) and partly in SW (configuration of the
> interrupt line to use), e.g. hardcoded or configurable somewhere in the
> driver via sysfs or the like. Not nice.

Only way I can see to be nice about this is to specify a default.
However, if someone is using the input driver and we have interrupt names
that don't match the default, all bets are off.  That setup doesn't work
today anyway, so do we care? I don't think so.

So in conclusion. Drop the required entry for interrupts, but consider
if a default can work for interrupt-names, or whether we should add
the logic to require interrupt-names if interrupts are provided.

Jonathan

> 
> Pls, let me know what you think, and in case, if I need to take some
> action, here.
> Best,
> L
> 
> 
> > > > Hence, I better drop the description entirely, since it rather seems
> > > > to be confusing.
diff mbox series

Patch

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]
+
 required:
   - compatible
   - reg
@@ -61,6 +66,7 @@  examples:
             reg = <0x2a>;
             interrupt-parent = <&gpio0>;
             interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+            interrupt-names = "INT1";
         };
     };
   - |
@@ -79,5 +85,6 @@  examples:
             spi-cpha;
             interrupt-parent = <&gpio0>;
             interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+            interrupt-names = "INT2";
         };
     };