| Message ID | 20231224143500.10940-3-petre.rodan@subdimension.ro |
|---|---|
| State | New |
| Headers | show |
| Series | changes to mprls0025pa | expand |
On 24/12/2023 15:34, Petre Rodan wrote: > Change order of properties in order for the end user to de-prioritize > pmin-pascal and pmax-pascal which are superseded by pressure-triplet. > > Add pressure-triplet property which automatically initializes > pmin-pascal and pmax-pascal inside the driver > > Rework honeywell,pmXX-pascal requirements based on feedback from > Jonathan and Conor. > > Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro> > Signed-off-by: Andreas Klinger <ak@it-klinger.de> > --- > .../iio/pressure/honeywell,mprls0025pa.yaml | 64 ++++++++++++++----- > 1 file changed, 47 insertions(+), 17 deletions(-) > > diff --git a/Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml b/Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml > index 84ced4e5a7da..e4021306d187 100644 > --- a/Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml > +++ b/Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml > @@ -19,14 +19,17 @@ description: | > calls them "mpr series". All of them have the identical programming model and > differ in the pressure range, unit and transfer function. > > - To support different models one need to specify the pressure range as well as > - the transfer function. Pressure range needs to be converted from its unit to > + To support different models one need to specify its pressure triplet as well > + as the transfer function. > + > + For custom models the pressure values can alternatively be specified manually. > + The minimal range value stands for the minimum pressure and the maximum value > + also for the maximum pressure with linear relation inside the range. > + Pressure range needs to be converted from the datasheet specified unit to > pascal. > > The transfer function defines the ranges of numerical values delivered by the > - sensor. The minimal range value stands for the minimum pressure and the > - maximum value also for the maximum pressure with linear relation inside the > - range. > + sensor. > > Specifications about the devices can be found at: > https://prod-edam.honeywell.com/content/dam/honeywell-edam/sps/siot/en-us/ > @@ -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? > + description: | > + Case-sensitive five character string that defines pressure range, unit > + and type as part of the device nomenclature. In the unlikely case of a > + custom chip, unset and provide pmin-pascal and pmax-pascal instead. > + enum: [0001BA, 01.6BA, 02.5BA, 0060MG, 0100MG, 0160MG, 0250MG, 0400MG, > + 0600MG, 0001BG, 01.6BG, 02.5BG, 0100KA, 0160KA, 0250KA, 0006KG, > + 0010KG, 0016KG, 0025KG, 0040KG, 0060KG, 0100KG, 0160KG, 0250KG, > + 0015PA, 0025PA, 0030PA, 0001PG, 0005PG, 0015PG, 0030PG, 0300YG] > + $ref: /schemas/types.yaml#/definitions/string > + > + 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. > + > + honeywell,pmax-pascal: > + description: > + Maximum pressure value the sensor can measure in pascal. > + To be specified only if honeywell,pressure-triplet is not set. > + > vdd-supply: > description: provide VDD power to the sensor. > > required: > - compatible > - reg > - - honeywell,pmin-pascal > - - honeywell,pmax-pascal > - honeywell,transfer-function > - vdd-supply > > +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. Best regards, Krzysztof
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
On 25/12/2023 14:23, Petre Rodan wrote: > > 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? I just don't get why moving the code instead of adding new property next to them. The order is often alphabetical. > >>> + 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" And how this command matters? DT overlays are checked, so error is printed. > > the schema is not checked in any way. When I run `make` the schema is also not checked, so is it an argument to add anything to the binding? No. Drop redundant text. > 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. Really? Did you test it? Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml b/Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml index 84ced4e5a7da..e4021306d187 100644 --- a/Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml +++ b/Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml @@ -19,14 +19,17 @@ description: | calls them "mpr series". All of them have the identical programming model and differ in the pressure range, unit and transfer function. - To support different models one need to specify the pressure range as well as - the transfer function. Pressure range needs to be converted from its unit to + To support different models one need to specify its pressure triplet as well + as the transfer function. + + For custom models the pressure values can alternatively be specified manually. + The minimal range value stands for the minimum pressure and the maximum value + also for the maximum pressure with linear relation inside the range. + Pressure range needs to be converted from the datasheet specified unit to pascal. The transfer function defines the ranges of numerical values delivered by the - sensor. The minimal range value stands for the minimum pressure and the - maximum value also for the maximum pressure with linear relation inside the - range. + sensor. Specifications about the devices can be found at: https://prod-edam.honeywell.com/content/dam/honeywell-edam/sps/siot/en-us/ @@ -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: + description: | + Case-sensitive five character string that defines pressure range, unit + and type as part of the device nomenclature. In the unlikely case of a + custom chip, unset and provide pmin-pascal and pmax-pascal instead. + enum: [0001BA, 01.6BA, 02.5BA, 0060MG, 0100MG, 0160MG, 0250MG, 0400MG, + 0600MG, 0001BG, 01.6BG, 02.5BG, 0100KA, 0160KA, 0250KA, 0006KG, + 0010KG, 0016KG, 0025KG, 0040KG, 0060KG, 0100KG, 0160KG, 0250KG, + 0015PA, 0025PA, 0030PA, 0001PG, 0005PG, 0015PG, 0030PG, 0300YG] + $ref: /schemas/types.yaml#/definitions/string + + honeywell,pmin-pascal: + description: + Minimum pressure value the sensor can measure in pascal. + To be specified only if honeywell,pressure-triplet is not set. + + honeywell,pmax-pascal: + description: + Maximum pressure value the sensor can measure in pascal. + To be specified only if honeywell,pressure-triplet is not set. + vdd-supply: description: provide VDD power to the sensor. required: - compatible - reg - - honeywell,pmin-pascal - - honeywell,pmax-pascal - honeywell,transfer-function - vdd-supply +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 + additionalProperties: false examples: @@ -99,8 +129,8 @@ examples: reset-gpios = <&gpio3 19 GPIO_ACTIVE_HIGH>; interrupt-parent = <&gpio3>; interrupts = <21 IRQ_TYPE_EDGE_RISING>; - honeywell,pmin-pascal = <0>; - honeywell,pmax-pascal = <172369>; + + honeywell,pressure-triplet = "0025PA"; honeywell,transfer-function = <1>; vdd-supply = <&vcc_3v3>; };