diff mbox series

[v3,08/10] dt-bindings: iio: bma255: Allow multiple interrupts

Message ID 20210611080903.14384-9-stephan@gerhold.net (mailing list archive)
State Accepted
Headers show
Series iio: accel: bmc150: Add support for BMA253/BMA254 | expand

Commit Message

Stephan Gerhold June 11, 2021, 8:09 a.m. UTC
BMA253 has two interrupt pins (INT1 and INT2) that can be configured
independently. At the moment the bmc150-accel driver does not make use
of them but it might be able to in the future, so it's useful to already
specify all available interrupts in the device tree.

Set maxItems: 2 for interrupts to allow specifying a second one.
This is necessary as preparation to move the bosch,bma254 compatible
from bosch,bma180.yaml to bosch,bma255.yaml since bma180 allows two
interrupts, but BMA254 is better supported by the bmc150-accel driver.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
 .../devicetree/bindings/iio/accel/bosch,bma255.yaml        | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Jonathan Cameron June 11, 2021, 5:59 p.m. UTC | #1
On Fri, 11 Jun 2021 10:09:01 +0200
Stephan Gerhold <stephan@gerhold.net> wrote:

> BMA253 has two interrupt pins (INT1 and INT2) that can be configured
> independently. At the moment the bmc150-accel driver does not make use
> of them but it might be able to in the future, so it's useful to already
> specify all available interrupts in the device tree.
> 
> Set maxItems: 2 for interrupts to allow specifying a second one.
> This is necessary as preparation to move the bosch,bma254 compatible
> from bosch,bma180.yaml to bosch,bma255.yaml since bma180 allows two
> interrupts, but BMA254 is better supported by the bmc150-accel driver.
> 
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> ---
>  .../devicetree/bindings/iio/accel/bosch,bma255.yaml        | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml b/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
> index 8afb0fe8ef5c..65b299a5619b 100644
> --- a/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
> +++ b/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
> @@ -32,7 +32,12 @@ properties:
>    vddio-supply: true
>  
>    interrupts:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 2
> +    description: |
> +      The first interrupt listed must be the one connected to the INT1 pin,
> +      the second (optional) interrupt listed must be the one connected to the
> +      INT2 pin (if available).

As this is a direct copy from the bma180 binding and we are moving devices
from one to the other, we need to support this as the default.
Longer term, from the bma253 datasheet, it look looks the two pins are equally
capable so if we get a board where only the int2 pin is connected then we will
need to use interrupt-names to distinguish the two (as we do in other drivers).

Another thing to note is that we don't have to have separate binding docs just
because we have separate drivers. At somepoint we might want to consider just
fusing the two docs.
 
Anyhow, work for another day!

Jonathan


>  
>    mount-matrix:
>      description: an optional 3x3 mounting rotation matrix.
Stephan Gerhold June 11, 2021, 6:21 p.m. UTC | #2
On Fri, Jun 11, 2021 at 06:59:41PM +0100, Jonathan Cameron wrote:
> On Fri, 11 Jun 2021 10:09:01 +0200
> Stephan Gerhold <stephan@gerhold.net> wrote:
> 
> > BMA253 has two interrupt pins (INT1 and INT2) that can be configured
> > independently. At the moment the bmc150-accel driver does not make use
> > of them but it might be able to in the future, so it's useful to already
> > specify all available interrupts in the device tree.
> > 
> > Set maxItems: 2 for interrupts to allow specifying a second one.
> > This is necessary as preparation to move the bosch,bma254 compatible
> > from bosch,bma180.yaml to bosch,bma255.yaml since bma180 allows two
> > interrupts, but BMA254 is better supported by the bmc150-accel driver.
> > 
> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> > ---
> >  .../devicetree/bindings/iio/accel/bosch,bma255.yaml        | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml b/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
> > index 8afb0fe8ef5c..65b299a5619b 100644
> > --- a/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
> > +++ b/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
> > @@ -32,7 +32,12 @@ properties:
> >    vddio-supply: true
> >  
> >    interrupts:
> > -    maxItems: 1
> > +    minItems: 1
> > +    maxItems: 2
> > +    description: |
> > +      The first interrupt listed must be the one connected to the INT1 pin,
> > +      the second (optional) interrupt listed must be the one connected to the
> > +      INT2 pin (if available).
> 
> As this is a direct copy from the bma180 binding and we are moving devices
> from one to the other, we need to support this as the default.
> Longer term, from the bma253 datasheet, it look looks the two pins are equally
> capable so if we get a board where only the int2 pin is connected then we will
> need to use interrupt-names to distinguish the two (as we do in other drivers).
> 

This kind of sounds like a strange board layout in general. But what's
worse is that for some reason even Bosch thought this is a "good" idea
so they released the BMC156 [1]. It works just like the BMC150 but has
only a single interrupt pin. One would expect that would be INT1,
but nope, it's INT2 of course. :-)

I have a device with BMC156 where this is the case, so I guess I need to
make bmc150-accel use INT2 somehow (without specifying INT1). It might
be easiest if we treat this the same way as the case that you mentioned,
i.e. everyone with BMC156 would specify the interrupt-names explicitly
to have a consistent meaning of the device tree.

[1]: https://www.mouser.de/datasheet/2/783/BST-BMC156-DS000-1509546.pdf

> Another thing to note is that we don't have to have separate binding docs just
> because we have separate drivers. At somepoint we might want to consider just
> fusing the two docs.
>  

Good point! I might do that together with the BMC156 changes. :)

Thanks!
Stephan
Jonathan Cameron June 11, 2021, 6:26 p.m. UTC | #3
On Fri, 11 Jun 2021 20:21:05 +0200
Stephan Gerhold <stephan@gerhold.net> wrote:

> On Fri, Jun 11, 2021 at 06:59:41PM +0100, Jonathan Cameron wrote:
> > On Fri, 11 Jun 2021 10:09:01 +0200
> > Stephan Gerhold <stephan@gerhold.net> wrote:
> >   
> > > BMA253 has two interrupt pins (INT1 and INT2) that can be configured
> > > independently. At the moment the bmc150-accel driver does not make use
> > > of them but it might be able to in the future, so it's useful to already
> > > specify all available interrupts in the device tree.
> > > 
> > > Set maxItems: 2 for interrupts to allow specifying a second one.
> > > This is necessary as preparation to move the bosch,bma254 compatible
> > > from bosch,bma180.yaml to bosch,bma255.yaml since bma180 allows two
> > > interrupts, but BMA254 is better supported by the bmc150-accel driver.
> > > 
> > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > > Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > > Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> > > ---
> > >  .../devicetree/bindings/iio/accel/bosch,bma255.yaml        | 7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml b/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
> > > index 8afb0fe8ef5c..65b299a5619b 100644
> > > --- a/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
> > > +++ b/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
> > > @@ -32,7 +32,12 @@ properties:
> > >    vddio-supply: true
> > >  
> > >    interrupts:
> > > -    maxItems: 1
> > > +    minItems: 1
> > > +    maxItems: 2
> > > +    description: |
> > > +      The first interrupt listed must be the one connected to the INT1 pin,
> > > +      the second (optional) interrupt listed must be the one connected to the
> > > +      INT2 pin (if available).  
> > 
> > As this is a direct copy from the bma180 binding and we are moving devices
> > from one to the other, we need to support this as the default.
> > Longer term, from the bma253 datasheet, it look looks the two pins are equally
> > capable so if we get a board where only the int2 pin is connected then we will
> > need to use interrupt-names to distinguish the two (as we do in other drivers).
> >   
> 
> This kind of sounds like a strange board layout in general. But what's
> worse is that for some reason even Bosch thought this is a "good" idea
> so they released the BMC156 [1]. It works just like the BMC150 but has
> only a single interrupt pin. One would expect that would be INT1,
> but nope, it's INT2 of course. :-)
> 
> I have a device with BMC156 where this is the case, so I guess I need to
> make bmc150-accel use INT2 somehow (without specifying INT1). It might
> be easiest if we treat this the same way as the case that you mentioned,
> i.e. everyone with BMC156 would specify the interrupt-names explicitly
> to have a consistent meaning of the device tree.

That will really confuse people who think they just have one interrupt so
why do they need the name!  You'll have to special case that delight in
the driver.

> 
> [1]: https://www.mouser.de/datasheet/2/783/BST-BMC156-DS000-1509546.pdf
> 
> > Another thing to note is that we don't have to have separate binding docs just
> > because we have separate drivers. At somepoint we might want to consider just
> > fusing the two docs.
> >    
> 
> Good point! I might do that together with the BMC156 changes. :)

Great.  Best of all you can drop me as maintainer the combined binding ;)

Jonathan

> 
> Thanks!
> Stephan
Stephan Gerhold June 11, 2021, 7:21 p.m. UTC | #4
On Fri, Jun 11, 2021 at 07:26:39PM +0100, Jonathan Cameron wrote:
> On Fri, 11 Jun 2021 20:21:05 +0200
> Stephan Gerhold <stephan@gerhold.net> wrote:
> 
> > On Fri, Jun 11, 2021 at 06:59:41PM +0100, Jonathan Cameron wrote:
> > > On Fri, 11 Jun 2021 10:09:01 +0200
> > > Stephan Gerhold <stephan@gerhold.net> wrote:
> > >   
> > > > BMA253 has two interrupt pins (INT1 and INT2) that can be configured
> > > > independently. At the moment the bmc150-accel driver does not make use
> > > > of them but it might be able to in the future, so it's useful to already
> > > > specify all available interrupts in the device tree.
> > > > 
> > > > Set maxItems: 2 for interrupts to allow specifying a second one.
> > > > This is necessary as preparation to move the bosch,bma254 compatible
> > > > from bosch,bma180.yaml to bosch,bma255.yaml since bma180 allows two
> > > > interrupts, but BMA254 is better supported by the bmc150-accel driver.
> > > > 
> > > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > > > Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> > > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > > > Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> > > > ---
> > > >  .../devicetree/bindings/iio/accel/bosch,bma255.yaml        | 7 ++++++-
> > > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml b/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
> > > > index 8afb0fe8ef5c..65b299a5619b 100644
> > > > --- a/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
> > > > +++ b/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
> > > > @@ -32,7 +32,12 @@ properties:
> > > >    vddio-supply: true
> > > >  
> > > >    interrupts:
> > > > -    maxItems: 1
> > > > +    minItems: 1
> > > > +    maxItems: 2
> > > > +    description: |
> > > > +      The first interrupt listed must be the one connected to the INT1 pin,
> > > > +      the second (optional) interrupt listed must be the one connected to the
> > > > +      INT2 pin (if available).  
> > > 
> > > As this is a direct copy from the bma180 binding and we are moving devices
> > > from one to the other, we need to support this as the default.
> > > Longer term, from the bma253 datasheet, it look looks the two pins are equally
> > > capable so if we get a board where only the int2 pin is connected then we will
> > > need to use interrupt-names to distinguish the two (as we do in other drivers).
> > >   
> > 
> > This kind of sounds like a strange board layout in general. But what's
> > worse is that for some reason even Bosch thought this is a "good" idea
> > so they released the BMC156 [1]. It works just like the BMC150 but has
> > only a single interrupt pin. One would expect that would be INT1,
> > but nope, it's INT2 of course. :-)
> > 
> > I have a device with BMC156 where this is the case, so I guess I need to
> > make bmc150-accel use INT2 somehow (without specifying INT1). It might
> > be easiest if we treat this the same way as the case that you mentioned,
> > i.e. everyone with BMC156 would specify the interrupt-names explicitly
> > to have a consistent meaning of the device tree.
> 
> That will really confuse people who think they just have one interrupt so
> why do they need the name!  You'll have to special case that delight in
> the driver.
> 

Okay, good that we discussed this before then. :)
I was unsure if it would be better to special case it for BMC156 *or*
add support for the interrupt-names, but perhaps I'm just going to do
*both* then. (Special case it for BMC156, look at interrupt-names for
all others.)

Thanks!
Stephan
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml b/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
index 8afb0fe8ef5c..65b299a5619b 100644
--- a/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
+++ b/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
@@ -32,7 +32,12 @@  properties:
   vddio-supply: true
 
   interrupts:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
+    description: |
+      The first interrupt listed must be the one connected to the INT1 pin,
+      the second (optional) interrupt listed must be the one connected to the
+      INT2 pin (if available).
 
   mount-matrix:
     description: an optional 3x3 mounting rotation matrix.