| 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 |
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
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
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 >
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 >
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 > >
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 > > >
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 --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
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(-)