Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sat, 1 Apr 2023 16:27:01 +0100
From:   Jonathan Cameron <jic23@kernel.org>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc:     Andreas Klinger <ak@it-klinger.de>, linux-iio@vger.kernel.org,
        devicetree@vger.kernel.org, Lars-Peter Clausen <lars@metafoo.de>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Angel Iglesias <ang.iglesiasg@gmail.com>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/3] dt-bindings: iio: pressure: Support Honeywell mpr
 sensors
Message-ID: <20230401162701.1d3caebc@jic23-huawei>
In-Reply-To: <27a1d0f4-3a02-c7fd-36a0-07729a136e20@linaro.org>
References: <ZCf01btsJRXcIOce@arbad>
        <27a1d0f4-3a02-c7fd-36a0-07729a136e20@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Sat, 1 Apr 2023 11:42:15 +0200
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> On 01/04/2023 11:09, Andreas Klinger wrote:
> > Honeywell mpr is a pressure sensor family. There are many different
> > types with different pressure ranges. The range needs to be set up in
> > the dt. Therefore new properties honeywell,pmin and honeywell,pmax are
> > introduced.
> > 
> > Add dt-bindings.
> > 
> > Signed-off-by: Andreas Klinger <ak@it-klinger.de>
> > ---
> >  .../bindings/iio/pressure/honeywell,mpr.yaml  | 74 +++++++++++++++++++
> >  1 file changed, 74 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/iio/pressure/honeywell,mpr.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/pressure/honeywell,mpr.yaml b/Documentation/devicetree/bindings/iio/pressure/honeywell,mpr.yaml
> > new file mode 100644
> > index 000000000000..d6fad6f841cf
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/pressure/honeywell,mpr.yaml
> > @@ -0,0 +1,74 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/pressure/honeywell,mpr.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Honeywell mpr pressure sensor
> > +
> > +maintainers:
> > +  - Andreas Klinger <ak@it-klinger.de>
> > +
> > +description: |
> > +  Honeywell pressure sensor of type mpr. This sensor has an I2C and SPI interface. Only the I2C  
> 
> Doesn't look wrapped according to Linux coding style (see Coding style).
> 
> > +  interface is implemented.
> > +
> > +  There are many subtypes with different pressure ranges available. Therefore the minimum and
> > +  maximum pressure values of the specific sensor needs to be specified in Pascal.
> > +
> > +  Specifications about the devices can be found at:
> > +    https://prod-edam.honeywell.com/content/dam/honeywell-edam/sps/siot/en-us/products/sensors/  \
> > +      pressure-sensors/board-mount-pressure-sensors/micropressure-mpr-series/documents/          \
> > +      sps-siot-mpr-series-datasheet-32332628-ciid-172626.pdf  
> 
> Lines are not continued, so drop \
> 
> > +
> > +properties:
> > +  compatible:
> > +    const: honeywell,mpr  
> 
> You need device specific compatible, not some generic one. Rename also
> then the filename (should match the compatible).
> 
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  reset-gpios:
> > +    description:
> > +      Optional GPIO for resetting the device. If not present the device is not resetted.  
> 
> Are you sure it is wrapped properly?
> 
> > +    maxItems: 1
> > +
> > +  honeywell,pmin:
> > +    description:
> > +      Minimum pressure value the sensor can measure in pascal.  
> 
> Use standard unit suffix:
> https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml
> 
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +
> > +  honeywell,pmax:
> > +    description:
> > +      Maximum pressure value the sensor can measure in pascal.
> > +    $ref: /schemas/types.yaml#/definitions/uint32  
> 
> Same.
> 
> Why these values are suitable for DT? Does it depend on type of sensor
> (thus it is implied from compatible) or on system setup?

I think we'll end up with a lot of compatibles, but that's still better
than free form description.  May still need these as well though given
the datasheet helpfully adds a foot note.

1. Custom pressure ranges are available.

Might not be worth including all the details though but unhelpfully the
bits we care about are after details like is the gel food grade or the port long.
Definitely can ignore the encoding of i2c address / spi in the last few bits but
may need the transfer function.


mpr-0025GA-A maybe as a form?

> 
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - honeywell,pmin
> > +  - honeywell,pmax
> > +  
> 
> 
> Best regards,
> Krzysztof
>