Re: [PATCH v3 1/3] dt-bindings: iio: pressure: Add Sensirion SDP500

Petar Stoykov
2024-07-04T08:35:57Z

On Tue, Jul 2, 2024 at 10:18 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Tue, 02 Jul 2024 16:59:08 +0200
> Petar Stoykov via B4 Relay <devnull+pd.pstoykov.gmail.com@kernel.org> wrote:
>
> > From: Petar Stoykov <pd.pstoykov@gmail.com>
> >
> > Sensirion SDP500 is a digital differential pressure sensor. It provides
> > a digital I2C output. Add devicetree bindings requiring the compatible
> > string and I2C slave address (reg).
> >
> > Signed-off-by: Petar Stoykov <pd.pstoykov@gmail.com>
> > ---
> > .../bindings/iio/pressure/sensirion,sdp500.yaml | 41 ++++++++++++++++++++++
> > 1 file changed, 41 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/iio/pressure/sensirion,sdp500.yaml b/Documentation/devicetree/bindings/iio/pressure/sensirion,sdp500.yaml
> > new file mode 100644
> > index 000000000000..6b3e54def367
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/pressure/sensirion,sdp500.yaml
> > @@ -0,0 +1,41 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/pressure/sdp500.yaml#
>
> Naming mismatch (as already reporteD).

Same mistake as in the MAINTAINERS file. Sigh.

>
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: sdp500/sdp510 pressure sensor with I2C bus interface
> > +
> > +maintainers:
> > + - Petar Stoykov <petar.stoykov@prodrive-technologies.com>
> > +
> > +description: |
> > + Pressure sensor from Sensirion with I2C bus interface.
> > + There is no software difference between sdp500 and sdp510.
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - sensirion,sdp500
> > + - sensirion,sdp510
>
> Include the vdd-supply both as a property and in requried.
>
> vdd-supply: true;
>
> is enough for the property.

Thank you for the example!
But I don't think it's a required property.
Our system is an example where it's not needed.
The device is always powered on and Linux has no control over it's supply.
One can argue that it's nice to write down what is that power supply but
to me it's just noise, because I can write anything there and it would still
work. So I prefer to not add anything instead of putting a value I can't trust.

> > +
> > + reg:
> > + maxItems: 1
> > +
> > +required:
> > + - compatible
> > + - reg
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + i2c {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + pressure@40 {
> > + compatible = "sensirion,sdp500";
> > + reg = <0x40>;
> > + vdd-supply = <&foo>;
> > + };
> > + };
> >
>