diff mbox series

[v3,5/7] dt-bindings: iio: pressure: bmp085: Add interrupts for BMP3xx and BMP5xx devices

Message ID 20240823181714.64545-6-vassilisamir@gmail.com (mailing list archive)
State Changes Requested
Headers show
Series pressure: bmp280: Minor cleanup and interrupt support | expand

Commit Message

Vasileios Amoiridis Aug. 23, 2024, 6:17 p.m. UTC
Add interrupt options for BMP3xx and BMP5xx devices as well.

Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
 Documentation/devicetree/bindings/iio/pressure/bmp085.yaml | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Biju Das Aug. 23, 2024, 6:51 p.m. UTC | #1
Hi Vasileios Amoiridis,

Thanks for the patch.

> -----Original Message-----
> From: Vasileios Amoiridis <vassilisamir@gmail.com>
> Sent: Friday, August 23, 2024 7:17 PM
> Subject: [PATCH v3 5/7] dt-bindings: iio: pressure: bmp085: Add interrupts for BMP3xx and BMP5xx
> devices
> 
> Add interrupt options for BMP3xx and BMP5xx devices as well.
> 
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> ---
>  Documentation/devicetree/bindings/iio/pressure/bmp085.yaml | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
> b/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
> index 6fda887ee9d4..eb1e1ab3dd18 100644
> --- a/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
> +++ b/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
> @@ -48,9 +48,14 @@ properties:
> 
>    interrupts:
>      description:
> -      interrupt mapping for IRQ (BMP085 only)
> +      interrupt mapping for IRQ. Supported in BMP085, BMP3xx, BMP5xx

Since you have updated the description. It is better to enforce the same in
conditional schema?? So that dt binding check throws error if interrupt
used in bmp{180,280 and bme280}.

Cheers,
Biju

>      maxItems: 1
> 
> +  drive-open-drain:
> +    description:
> +      set if the interrupt pin should be configured as open drain.
> +      If not set, defaults to push-pull configuration.
> +
>  required:
>    - compatible
>    - vddd-supply
> --
> 2.25.1
Krzysztof Kozlowski Aug. 24, 2024, 7:45 a.m. UTC | #2
On Fri, Aug 23, 2024 at 08:17:12PM +0200, Vasileios Amoiridis wrote:
> Add interrupt options for BMP3xx and BMP5xx devices as well.
> 
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> ---
>  Documentation/devicetree/bindings/iio/pressure/bmp085.yaml | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml b/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
> index 6fda887ee9d4..eb1e1ab3dd18 100644
> --- a/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
> +++ b/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
> @@ -48,9 +48,14 @@ properties:
>  
>    interrupts:
>      description:
> -      interrupt mapping for IRQ (BMP085 only)
> +      interrupt mapping for IRQ. Supported in BMP085, BMP3xx, BMP5xx

Supported by driver or device?
If the latter, this should be constrained per device variant in
allOf:if:then:.


>      maxItems: 1
>  
> +  drive-open-drain:

Missing type, unless some other core schema defined it? But then I
actually wonder if we need it.  Maybe this should be interrupt flag?
Just like GPIO has such.

Best regards,
Krzysztof
Vasileios Amoiridis Aug. 24, 2024, 11:31 a.m. UTC | #3
On Fri, Aug 23, 2024 at 06:51:55PM +0000, Biju Das wrote:
> Hi Vasileios Amoiridis,
> 
> Thanks for the patch.
> 
> > -----Original Message-----
> > From: Vasileios Amoiridis <vassilisamir@gmail.com>
> > Sent: Friday, August 23, 2024 7:17 PM
> > Subject: [PATCH v3 5/7] dt-bindings: iio: pressure: bmp085: Add interrupts for BMP3xx and BMP5xx
> > devices
> > 
> > Add interrupt options for BMP3xx and BMP5xx devices as well.
> > 
> > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> > ---
> >  Documentation/devicetree/bindings/iio/pressure/bmp085.yaml | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
> > b/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
> > index 6fda887ee9d4..eb1e1ab3dd18 100644
> > --- a/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
> > +++ b/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
> > @@ -48,9 +48,14 @@ properties:
> > 
> >    interrupts:
> >      description:
> > -      interrupt mapping for IRQ (BMP085 only)
> > +      interrupt mapping for IRQ. Supported in BMP085, BMP3xx, BMP5xx
> 
> Since you have updated the description. It is better to enforce the same in
> conditional schema?? So that dt binding check throws error if interrupt
> used in bmp{180,280 and bme280}.
> 
> Cheers,
> Biju
> 

Hi Biju,

Thanks for the feedback! It is true that it would be good to throw an
error in case the IRQ is used in a not supported sensor. If you could
point me to an example of another sensor implementing it, it would be
even more helpful, but I am sure I will find something :)

Cheers,
Vasilis

> >      maxItems: 1
> > 
> > +  drive-open-drain:
> > +    description:
> > +      set if the interrupt pin should be configured as open drain.
> > +      If not set, defaults to push-pull configuration.
> > +
> >  required:
> >    - compatible
> >    - vddd-supply
> > --
> > 2.25.1
>
Vasileios Amoiridis Aug. 24, 2024, 11:35 a.m. UTC | #4
On Sat, Aug 24, 2024 at 09:45:43AM +0200, Krzysztof Kozlowski wrote:
> On Fri, Aug 23, 2024 at 08:17:12PM +0200, Vasileios Amoiridis wrote:
> > Add interrupt options for BMP3xx and BMP5xx devices as well.
> > 
> > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> > ---
> >  Documentation/devicetree/bindings/iio/pressure/bmp085.yaml | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml b/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
> > index 6fda887ee9d4..eb1e1ab3dd18 100644
> > --- a/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
> > +++ b/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
> > @@ -48,9 +48,14 @@ properties:
> >  
> >    interrupts:
> >      description:
> > -      interrupt mapping for IRQ (BMP085 only)
> > +      interrupt mapping for IRQ. Supported in BMP085, BMP3xx, BMP5xx
> 
> Supported by driver or device?
> If the latter, this should be constrained per device variant in
> allOf:if:then:.
> 

Hi Krzysztof,

Supported by some devices controlled by the same (just 1) driver.
Thanks for the hint, I will take a look how other drivers do it :)

> 
> >      maxItems: 1
> >  
> > +  drive-open-drain:
> 
> Missing type, unless some other core schema defined it? But then I
> actually wonder if we need it.  Maybe this should be interrupt flag?
> Just like GPIO has such.

I took it from the bindings/iio/imu/bosch,bmi323.yaml example which is
the same. You think something needs to change?

Cheers,
Vasilis

> 
> Best regards,
> Krzysztof
>
Biju Das Aug. 24, 2024, 11:41 a.m. UTC | #5
Hi Vasileios Amoiridis,

> -----Original Message-----
> From: Vasileios Amoiridis <vassilisamir@gmail.com>
> Sent: Saturday, August 24, 2024 12:31 PM
> Subject: Re: [PATCH v3 5/7] dt-bindings: iio: pressure: bmp085: Add interrupts for BMP3xx and BMP5xx
> devices
> 
> On Fri, Aug 23, 2024 at 06:51:55PM +0000, Biju Das wrote:
> > Hi Vasileios Amoiridis,
> >
> > Thanks for the patch.
> >
> > > -----Original Message-----
> > > From: Vasileios Amoiridis <vassilisamir@gmail.com>
> > > Sent: Friday, August 23, 2024 7:17 PM
> > > Subject: [PATCH v3 5/7] dt-bindings: iio: pressure: bmp085: Add
> > > interrupts for BMP3xx and BMP5xx devices
> > >
> > > Add interrupt options for BMP3xx and BMP5xx devices as well.
> > >
> > > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> > > ---
> > >  Documentation/devicetree/bindings/iio/pressure/bmp085.yaml | 7
> > > ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
> > > b/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
> > > index 6fda887ee9d4..eb1e1ab3dd18 100644
> > > --- a/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
> > > +++ b/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
> > > @@ -48,9 +48,14 @@ properties:
> > >
> > >    interrupts:
> > >      description:
> > > -      interrupt mapping for IRQ (BMP085 only)
> > > +      interrupt mapping for IRQ. Supported in BMP085, BMP3xx,
> > > + BMP5xx
> >
> > Since you have updated the description. It is better to enforce the
> > same in conditional schema?? So that dt binding check throws error if
> > interrupt used in bmp{180,280 and bme280}.
> >
> > Cheers,
> > Biju
> >
> 
> Hi Biju,
> 
> Thanks for the feedback! It is true that it would be good to throw an error in case the IRQ is used in
> a not supported sensor. If you could point me to an example of another sensor implementing it, it
> would be even more helpful, but I am sure I will find something :)

As Krzysztof Kozlowski mentioned it depends upon driver(s/w) or device(H/W).

if it is driver(s/w), then you don't need conditional check as bindings describes
hardware.

There are plenty of examples for allOf:if:then: 

Cheers,
Biju

> 
> Cheers,
> Vasilis
> 
> > >      maxItems: 1
> > >
> > > +  drive-open-drain:
> > > +    description:
> > > +      set if the interrupt pin should be configured as open drain.
> > > +      If not set, defaults to push-pull configuration.
> > > +
> > >  required:
> > >    - compatible
> > >    - vddd-supply
> > > --
> > > 2.25.1
> >
Vasileios Amoiridis Aug. 24, 2024, 12:09 p.m. UTC | #6
On Sat, Aug 24, 2024 at 11:41:26AM +0000, Biju Das wrote:
> Hi Vasileios Amoiridis,
> 
> > -----Original Message-----
> > From: Vasileios Amoiridis <vassilisamir@gmail.com>
> > Sent: Saturday, August 24, 2024 12:31 PM
> > Subject: Re: [PATCH v3 5/7] dt-bindings: iio: pressure: bmp085: Add interrupts for BMP3xx and BMP5xx
> > devices
> > 
> > On Fri, Aug 23, 2024 at 06:51:55PM +0000, Biju Das wrote:
> > > Hi Vasileios Amoiridis,
> > >
> > > Thanks for the patch.
> > >
> > > > -----Original Message-----
> > > > From: Vasileios Amoiridis <vassilisamir@gmail.com>
> > > > Sent: Friday, August 23, 2024 7:17 PM
> > > > Subject: [PATCH v3 5/7] dt-bindings: iio: pressure: bmp085: Add
> > > > interrupts for BMP3xx and BMP5xx devices
> > > >
> > > > Add interrupt options for BMP3xx and BMP5xx devices as well.
> > > >
> > > > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> > > > ---
> > > >  Documentation/devicetree/bindings/iio/pressure/bmp085.yaml | 7
> > > > ++++++-
> > > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
> > > > b/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
> > > > index 6fda887ee9d4..eb1e1ab3dd18 100644
> > > > --- a/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
> > > > +++ b/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
> > > > @@ -48,9 +48,14 @@ properties:
> > > >
> > > >    interrupts:
> > > >      description:
> > > > -      interrupt mapping for IRQ (BMP085 only)
> > > > +      interrupt mapping for IRQ. Supported in BMP085, BMP3xx,
> > > > + BMP5xx
> > >
> > > Since you have updated the description. It is better to enforce the
> > > same in conditional schema?? So that dt binding check throws error if
> > > interrupt used in bmp{180,280 and bme280}.
> > >
> > > Cheers,
> > > Biju
> > >
> > 
> > Hi Biju,
> > 
> > Thanks for the feedback! It is true that it would be good to throw an error in case the IRQ is used in
> > a not supported sensor. If you could point me to an example of another sensor implementing it, it
> > would be even more helpful, but I am sure I will find something :)
> 
> As Krzysztof Kozlowski mentioned it depends upon driver(s/w) or device(H/W).
> 
> if it is driver(s/w), then you don't need conditional check as bindings describes
> hardware.
> 
> There are plenty of examples for allOf:if:then: 
> 
> Cheers,
> Biju
> 

Hi Biju,

Indeed, I checked Krzysztof's mail later. I will implement it like this
since it is a device(H/W) issue.

Cheers,
Vasilis

> > 
> > Cheers,
> > Vasilis
> > 
> > > >      maxItems: 1
> > > >
> > > > +  drive-open-drain:
> > > > +    description:
> > > > +      set if the interrupt pin should be configured as open drain.
> > > > +      If not set, defaults to push-pull configuration.
> > > > +
> > > >  required:
> > > >    - compatible
> > > >    - vddd-supply
> > > > --
> > > > 2.25.1
> > >
Krzysztof Kozlowski Aug. 25, 2024, 6:57 a.m. UTC | #7
On 24/08/2024 13:35, Vasileios Amoiridis wrote:
> On Sat, Aug 24, 2024 at 09:45:43AM +0200, Krzysztof Kozlowski wrote:
>> On Fri, Aug 23, 2024 at 08:17:12PM +0200, Vasileios Amoiridis wrote:
>>> Add interrupt options for BMP3xx and BMP5xx devices as well.
>>>
>>> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
>>> ---
>>>  Documentation/devicetree/bindings/iio/pressure/bmp085.yaml | 7 ++++++-
>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml b/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
>>> index 6fda887ee9d4..eb1e1ab3dd18 100644
>>> --- a/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
>>> +++ b/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
>>> @@ -48,9 +48,14 @@ properties:
>>>  
>>>    interrupts:
>>>      description:
>>> -      interrupt mapping for IRQ (BMP085 only)
>>> +      interrupt mapping for IRQ. Supported in BMP085, BMP3xx, BMP5xx
>>
>> Supported by driver or device?
>> If the latter, this should be constrained per device variant in
>> allOf:if:then:.
>>
> 
> Hi Krzysztof,
> 
> Supported by some devices controlled by the same (just 1) driver.
> Thanks for the hint, I will take a look how other drivers do it :)
> 
>>
>>>      maxItems: 1
>>>  
>>> +  drive-open-drain:
>>
>> Missing type, unless some other core schema defined it? But then I
>> actually wonder if we need it.  Maybe this should be interrupt flag?
>> Just like GPIO has such.
> 
> I took it from the bindings/iio/imu/bosch,bmi323.yaml example which is
> the same. You think something needs to change?
> 

You need type: boolean.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml b/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
index 6fda887ee9d4..eb1e1ab3dd18 100644
--- a/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
+++ b/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
@@ -48,9 +48,14 @@  properties:
 
   interrupts:
     description:
-      interrupt mapping for IRQ (BMP085 only)
+      interrupt mapping for IRQ. Supported in BMP085, BMP3xx, BMP5xx
     maxItems: 1
 
+  drive-open-drain:
+    description:
+      set if the interrupt pin should be configured as open drain.
+      If not set, defaults to push-pull configuration.
+
 required:
   - compatible
   - vddd-supply