linux-kernel - Re: [PATCH v2 02/10] dt-bindings: iio: pressure: honeywell,mprls0025pa.yaml add pressure-triplet

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Mon, 25 Dec 2023 15:23:05 +0200
From: Petre Rodan <petre.rodan@...dimension.ro>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc: linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, Andreas Klinger <ak@...klinger.de>,
	Jonathan Cameron <jic23@...nel.org>,
	Lars-Peter Clausen <lars@...afoo.de>,
	Rob Herring <robh+dt@...nel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
	Conor Dooley <conor+dt@...nel.org>
Subject: Re: [PATCH v2 02/10] dt-bindings: iio: pressure:
 honeywell,mprls0025pa.yaml add pressure-triplet


hello Krzysztof,

On Mon, Dec 25, 2023 at 01:57:39PM +0100, Krzysztof Kozlowski wrote:
> On 24/12/2023 15:34, Petre Rodan wrote:
> > @@ -54,14 +57,6 @@ properties:
> >        If not present the device is not reset during the probe.
> >      maxItems: 1
> > 
> > -  honeywell,pmin-pascal:
> > -    description:
> > -      Minimum pressure value the sensor can measure in pascal.
> > -
> > -  honeywell,pmax-pascal:
> > -    description:
> > -      Maximum pressure value the sensor can measure in pascal.
> > -
> >    honeywell,transfer-function:
> >      description: |
> >        Transfer function which defines the range of valid values delivered by the
> > @@ -72,17 +67,52 @@ properties:
> >      enum: [1, 2, 3]
> >      $ref: /schemas/types.yaml#/definitions/uint32
> > 
> > +  honeywell,pressure-triplet:
> 
> Why not putting it just before existing properties?

I'd like to have pmin-pascal, pmax-pascal as the last two honeywell specific
properties, since they are not to be used unless someone has custom silicon.
so we will still have a block moved just like above.
the most logic order is the one I proposed above:

honeywell,transfer-function:
[..]
honeywell,pressure-triplet:
[..]
honeywell,pmin-pascal:
[..]
honeywell,pmax-pascal:
[..]

since the last 3 are tied together as we will see below.
is there any reason you want this order to change?

> > +  honeywell,pmin-pascal:
> > +    description:
> > +      Minimum pressure value the sensor can measure in pascal.
> > +      To be specified only if honeywell,pressure-triplet is not set.
> 
> The last sentence is redundant - schema should enforce that.

when someone generates the dtbo files via

cpp -nostdinc -I include -I ${LINUX_SRC}/include/ -I arch -undef -x assembler-with-cpp ${file}.dts "${BUILD_DIR}/${file}.dts.preprocessed"
dtc -@ -I dts -O dtb -o "${BUILD_DIR}/${file}.dtbo" "${BUILD_DIR}/${file}.dts.preprocessed"

the schema is not checked in any way.
so unless people can be bothered to understand the yaml intricacies in the
bindings file, I feel they need to see that redundant information there, see below.

> > +oneOf:
> > +  - required:
> > +      - honeywell,pmin-pascal
> > +      - honeywell,pmax-pascal
> > +  - required:
> > +      - honeywell,pressure-triplet
> > +
> > +allOf:
> > +  - if:
> > +      required:
> > +        - honeywell,pressure-triplet
> > +    then:
> > +      properties:
> > +        honeywell,pmin-pascal: false
> > +        honeywell,pmax-pascal: false
> 
> This allOf is not needed.

speaking for intricacies, if the allOf is removed, then a binding containing

honeywell,pmax-pascal = <840000>;
honeywell,pressure-triplet = "0015PA";

would be considered to be correct by the schema, but that would be the incorrect
result. so afaict allOf needs to stay, and so does the redundant text.

best regards,
peter

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)