[PATCH v2 00/10] changes to mprls0025pa

[PATCH v2 00/10] changes to mprls0025pa

[Petre Rodan]

A number of fixes to the mprls0025pa driver:
 - an off-by-one initially caused by a typo in the bindings file
 - two error fields are never checked during sensor interaction
 - unsafe initialization if the driver is instantiated via sysfs
   and the bindings are missing

Quality of life changes:
 - a refactor that adds a pressure-triplet property which initializes
pmin-pascal and pmax-pascal just like in the hsc030pa driver.
The user only needs to extract a short string from the chip name
instead of looking up the chip in the datasheet, understand the
nomenclature, extract the measurement range and then convert all units
to pascals.

New feature:
 - SPI compatibility for Honeywell MPR sensors that require it.

Both binding and driver are backwards compatible.
Tested in I2C and SPI modes with two different sensors.
The refactor requires property function present in the togreg branch.

Datasheet: 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
Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
Signed-off-by: Andreas Klinger <ak@it-klinger.de>
---
Petre Rodan (10):
  dt-bindings: iio: pressure: honeywell,mprls0025pa.yaml fix
  dt-bindings: iio: pressure: honeywell,mprls0025pa.yaml add
    pressure-triplet
  dt-bindings: iio: pressure: honeywell,mprls0025pa.yaml add spi bus
  iio: pressure: mprls0025pa.c fix off-by-one enum
  iio: pressure: mprls0025pa.c fix error flag check
  iio: pressure: mprls0025pa.c remove dangerous defaults
  iio: pressure: mprls0025pa.c whitespace cleanup
  iio: pressure: mprls0025pa.c refactor
  iio: pressure: mprls0025pa.c add triplet property
  iio: pressure: mprls0025pa.c add SPI driver

 .../iio/pressure/honeywell,mprls0025pa.yaml   |  97 ++++--
 MAINTAINERS                                   |   3 +-
 drivers/iio/pressure/Kconfig                  |  14 +-
 drivers/iio/pressure/Makefile                 |   2 +
 drivers/iio/pressure/mprls0025pa.c            | 308 +++++++++---------
 drivers/iio/pressure/mprls0025pa.h            | 100 ++++++
 drivers/iio/pressure/mprls0025pa_i2c.c        |  98 ++++++
 drivers/iio/pressure/mprls0025pa_spi.c        |  91 ++++++
 8 files changed, 539 insertions(+), 174 deletions(-)
 create mode 100644 drivers/iio/pressure/mprls0025pa.h
 create mode 100644 drivers/iio/pressure/mprls0025pa_i2c.c
 create mode 100644 drivers/iio/pressure/mprls0025pa_spi.c

--
2.41.0

[PATCH v2 01/10] dt-bindings: iio: pressure: honeywell,mprls0025pa.yaml fix

[Petre Rodan]

Define enum inside the honeywell,transfer-function property block.

Set the correct irq edge in the example block.
Based on the datasheet, in table 13 on page 11:
"End-of-conversion indicator: This pin is set high when a measurement
and calculation have been completed and the data is ready to be
clocked out"

Add description on End-of-conversion interrupt.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
Signed-off-by: Andreas Klinger <ak@it-klinger.de>
---
 .../bindings/iio/pressure/honeywell,mprls0025pa.yaml       | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml b/Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml
index d9e903fbfd99..84ced4e5a7da 100644
--- a/Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml
+++ b/Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml
@@ -42,6 +42,10 @@ properties:
     maxItems: 1

   interrupts:
+    description:
+      Optional interrupt for indicating End-of-conversion.
+      If not present, the driver loops for a while until the received status
+      byte indicates correct measurement.
     maxItems: 1

   reset-gpios:
@@ -65,6 +69,7 @@ properties:
       1 - A, 10% to 90% of 2^24 (1677722 .. 15099494)
       2 - B, 2.5% to 22.5% of 2^24 (419430 .. 3774874)
       3 - C, 20% to 80% of 2^24 (3355443 .. 13421773)
+    enum: [1, 2, 3]
     $ref: /schemas/types.yaml#/definitions/uint32

   vdd-supply:
@@ -93,7 +98,7 @@ examples:
             reg = <0x18>;
             reset-gpios = <&gpio3 19 GPIO_ACTIVE_HIGH>;
             interrupt-parent = <&gpio3>;
-            interrupts = <21 IRQ_TYPE_EDGE_FALLING>;
+            interrupts = <21 IRQ_TYPE_EDGE_RISING>;
             honeywell,pmin-pascal = <0>;
             honeywell,pmax-pascal = <172369>;
             honeywell,transfer-function = <1>;
--
2.41.0

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

[Petre Rodan]

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:
+    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>;
         };
--
2.41.0

[PATCH v2 03/10] dt-bindings: iio: pressure: honeywell,mprls0025pa.yaml add spi bus

[Petre Rodan]

Add spi based example.

Add spi-max-frequency property required by chip specifications.

Add additional maintainer.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
Signed-off-by: Andreas Klinger <ak@it-klinger.de>
---
 .../iio/pressure/honeywell,mprls0025pa.yaml   | 26 +++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml b/Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml
index e4021306d187..430496b047c7 100644
--- a/Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml
+++ b/Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml
@@ -8,12 +8,12 @@ title: Honeywell mprls0025pa pressure sensor

 maintainers:
   - Andreas Klinger <ak@it-klinger.de>
+  - Petre Rodan <petre.rodan@subdimension.ro>

 description: |
   Honeywell pressure sensor of model mprls0025pa.

-  This sensor has an I2C and SPI interface. Only the I2C interface is
-  implemented.
+  This sensor has an I2C and SPI interface. Both are supported.

   There are many models with different pressure ranges available. The vendor
   calls them "mpr series". All of them have the identical programming model and
@@ -88,6 +88,9 @@ properties:
       Maximum pressure value the sensor can measure in pascal.
       To be specified only if honeywell,pressure-triplet is not set.

+  spi-max-frequency:
+    maximum: 800000
+
   vdd-supply:
     description: provide VDD power to the sensor.

@@ -135,3 +138,22 @@ examples:
             vdd-supply = <&vcc_3v3>;
         };
     };
+  - |
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        pressure@0 {
+            compatible = "honeywell,mprls0025pa";
+            reg = <0>;
+            spi-max-frequency = <800000>;
+            reset-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
+            interrupt-parent = <&gpio0>;
+            interrupts = <30 IRQ_TYPE_EDGE_RISING>;
+
+            honeywell,pressure-triplet = "0015PA";
+            honeywell,transfer-function = <1>;
+            vdd-supply = <&vcc_3v3>;
+        };
+    };
+...
--
2.41.0

Re: [PATCH v2 01/10] dt-bindings: iio: pressure: honeywell,mprls0025pa.yaml fix

[Krzysztof Kozlowski]

On 24/12/2023 15:34, Petre Rodan wrote:
> Define enum inside the honeywell,transfer-function property block.
> 
> Set the correct irq edge in the example block.
> Based on the datasheet, in table 13 on page 11:
> "End-of-conversion indicator: This pin is set high when a measurement
> and calculation have been completed and the data is ready to be
> clocked out"
> 

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

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

Re: [PATCH v2 03/10] dt-bindings: iio: pressure: honeywell,mprls0025pa.yaml add spi bus

[Krzysztof Kozlowski]

On 24/12/2023 15:34, Petre Rodan wrote:
> Add spi based example.
> 
> Add spi-max-frequency property required by chip specifications.
> 
> Add additional maintainer.
> 
> Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
> Signed-off-by: Andreas Klinger <ak@it-klinger.de>
> ---
>  .../iio/pressure/honeywell,mprls0025pa.yaml   | 26 +++++++++++++++++--
>  1 file changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml b/Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml
> index e4021306d187..430496b047c7 100644
> --- a/Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml
> +++ b/Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml
> @@ -8,12 +8,12 @@ title: Honeywell mprls0025pa pressure sensor
> 
>  maintainers:
>    - Andreas Klinger <ak@it-klinger.de>
> +  - Petre Rodan <petre.rodan@subdimension.ro>
> 
>  description: |
>    Honeywell pressure sensor of model mprls0025pa.
> 
> -  This sensor has an I2C and SPI interface. Only the I2C interface is
> -  implemented.
> +  This sensor has an I2C and SPI interface. Both are supported.

Instead drop that sentence. Current driver support should not matter for
the bindings.

> 
>    There are many models with different pressure ranges available. The vendor
>    calls them "mpr series". All of them have the identical programming model and
> @@ -88,6 +88,9 @@ properties:
>        Maximum pressure value the sensor can measure in pascal.
>        To be specified only if honeywell,pressure-triplet is not set.
> 
> +  spi-max-frequency:
> +    maximum: 800000

So you miss allOf: with $ref to spi props.

Best regards,
Krzysztof

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

[Petre Rodan]

[-- Attachment #1: Type: text/plain, Size: 2918 bytes --]


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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Krzysztof Kozlowski]

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

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

[Krzysztof Kozlowski]

On 25/12/2023 14:34, Krzysztof Kozlowski wrote:
>>>> +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?

Hm, indeed, on pmin/pmax would not trigger first required case. OK, then
this part make sense.

Best regards,
Krzysztof

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

[Petre Rodan]

[-- Attachment #1: Type: text/plain, Size: 4378 bytes --]


hello,

On Mon, Dec 25, 2023 at 02:34:04PM +0100, Krzysztof Kozlowski wrote:
> On 25/12/2023 14:23, Petre Rodan wrote:
> > 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.

as I also said in the comments and in my last reply I want the user to not feel
in any way obliged to fill in pmin-pascal, pmax-pascal.
and since a user reads this file from top to bottom, the order in which these
properties are shown to him is important, and it is the one above.

> The order is often alphabetical.

can we please make an exception?

> >>> +  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?

for more hours than I would have liked. the allOf was provided with kindness by
Conor in my first revision.

testing it:
 1. invalid yaml with both honeywell,pmax-pascal and honeywell,pressure-triplet
 defined passes the schema check if the allOf is removed:

 # make DT_SCHEMA_FILES=Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml  DT_CHECKER_FLAGS=-m dt_binding_check
 # echo $?
0

 2. the same invalid yaml but with the allOf not removed issues this output:

 [..]/Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.example.dtb: pressure@18: honeywell,pmax-pascal: False schema does not allow [[84000]]
         from schema $id: http://devicetree.org/schemas/iio/pressure/honeywell,mprls0025pa.yaml#

which is the expected behaviour. so AFAICT the allOf block is required, as well
as the redundant text for the humans that read the human-readable parts of the
bindings file.

invalid yaml example used above:

    #include <dt-bindings/gpio/gpio.h>
    #include <dt-bindings/interrupt-controller/irq.h>
    i2c {
        #address-cells = <1>;
        #size-cells = <0>;

        pressure@18 {
            compatible = "honeywell,mprls0025pa";
            reg = <0x18>;
            reset-gpios = <&gpio3 19 GPIO_ACTIVE_HIGH>;
            interrupt-parent = <&gpio3>;
            interrupts = <21 IRQ_TYPE_EDGE_RISING>;

            honeywell,pmax-pascal = <84000>;
            honeywell,pressure-triplet = "0025PA";
            honeywell,transfer-function = <1>;
            vdd-supply = <&vcc_3v3>;
        };
    };

best regards,
peter

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 03/10] dt-bindings: iio: pressure: honeywell,mprls0025pa.yaml add spi bus

[Petre Rodan]

[-- Attachment #1: Type: text/plain, Size: 1041 bytes --]


hello,

On Mon, Dec 25, 2023 at 01:59:43PM +0100, Krzysztof Kozlowski wrote:
> On 24/12/2023 15:34, Petre Rodan wrote:
> > Add spi based example.
> >    There are many models with different pressure ranges available. The vendor
> >    calls them "mpr series". All of them have the identical programming model and
> > @@ -88,6 +88,9 @@ properties:
> >        Maximum pressure value the sensor can measure in pascal.
> >        To be specified only if honeywell,pressure-triplet is not set.
> > 
> > +  spi-max-frequency:
> > +    maximum: 800000
> 
> So you miss allOf: with $ref to spi props.

for simplicity's sake and for compatibility with the i2c devices already in use,
this driver does not have distinct 'compatible' properties for the i2c and spi
implementation.
this is why I just defined spi-max-frequency, used it in the spi example, but
not required it. just like in hsc030pa.yaml .

without a differentiation in the 'compatible' string I don't see how your request
can be implemented.

cheers,
peter

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 03/10] dt-bindings: iio: pressure: honeywell,mprls0025pa.yaml add spi bus

[Krzysztof Kozlowski]

On 25/12/2023 16:13, Petre Rodan wrote:
> 
> hello,
> 
> On Mon, Dec 25, 2023 at 01:59:43PM +0100, Krzysztof Kozlowski wrote:
>> On 24/12/2023 15:34, Petre Rodan wrote:
>>> Add spi based example.
>>>    There are many models with different pressure ranges available. The vendor
>>>    calls them "mpr series". All of them have the identical programming model and
>>> @@ -88,6 +88,9 @@ properties:
>>>        Maximum pressure value the sensor can measure in pascal.
>>>        To be specified only if honeywell,pressure-triplet is not set.
>>>
>>> +  spi-max-frequency:
>>> +    maximum: 800000
>>
>> So you miss allOf: with $ref to spi props.
> 
> for simplicity's sake and for compatibility with the i2c devices already in use,
> this driver does not have distinct 'compatible' properties for the i2c and spi
> implementation.
> this is why I just defined spi-max-frequency, used it in the spi example, but
> not required it. just like in hsc030pa.yaml .
> 
> without a differentiation in the 'compatible' string I don't see how your request
> can be implemented.

You cannot have different compatibles. I did not propose it. I wrote
nothing about compatible. I wrote about missing $ref in top-level for
spi-peripheral-props. Where do you see anything about compatible?

Best regards,
Krzysztof

Re: [PATCH v2 03/10] dt-bindings: iio: pressure: honeywell,mprls0025pa.yaml add spi bus

[Petre Rodan]

[-- Attachment #1: Type: text/plain, Size: 1568 bytes --]

On Mon, Dec 25, 2023 at 07:56:24PM +0100, Krzysztof Kozlowski wrote:
> On 25/12/2023 16:13, Petre Rodan wrote:
> >>> @@ -88,6 +88,9 @@ properties:
> >>>        Maximum pressure value the sensor can measure in pascal.
> >>>        To be specified only if honeywell,pressure-triplet is not set.
> >>>
> >>> +  spi-max-frequency:
> >>> +    maximum: 800000
> >>
> >> So you miss allOf: with $ref to spi props.
> > 
> > for simplicity's sake and for compatibility with the i2c devices already in use,
> > this driver does not have distinct 'compatible' properties for the i2c and spi
> > implementation.
> > this is why I just defined spi-max-frequency, used it in the spi example, but
> > not required it. just like in hsc030pa.yaml .
> > 
> > without a differentiation in the 'compatible' string I don't see how your request
> > can be implemented.
> 
> You cannot have different compatibles. I did not propose it. I wrote
> nothing about compatible. I wrote about missing $ref in top-level for
> spi-peripheral-props. Where do you see anything about compatible?

sorry, for one hot second I thought you want that property to be conditionally
defined, like

allOf:
  - $ref: /schemas/spi/spi-peripheral-props.yaml

  - if:
      properties:
        compatible:
          contains:
            const: honeywell,foo-spi
    then:
      properties:
        spi-max-frequency:
          maximum: 800000
      required:
        - spi-max-frequency

but I guess you only want the first two lines from here.

happy holidays,
peter

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 03/10] dt-bindings: iio: pressure: honeywell,mprls0025pa.yaml add spi bus

[Krzysztof Kozlowski]

On 25/12/2023 21:29, Petre Rodan wrote:
>>> without a differentiation in the 'compatible' string I don't see how your request
>>> can be implemented.
>>
>> You cannot have different compatibles. I did not propose it. I wrote
>> nothing about compatible. I wrote about missing $ref in top-level for
>> spi-peripheral-props. Where do you see anything about compatible?
> 
> sorry, for one hot second I thought you want that property to be conditionally
> defined, like
> 
> allOf:
>   - $ref: /schemas/spi/spi-peripheral-props.yaml
> 

Yes, this one.

Best regards,
Krzysztof

Re: [PATCH v2 01/10] dt-bindings: iio: pressure: honeywell,mprls0025pa.yaml fix

[Jonathan Cameron]

On Sun, 24 Dec 2023 16:34:46 +0200
Petre Rodan <petre.rodan@subdimension.ro> wrote:

> Define enum inside the honeywell,transfer-function property block.
> 
> Set the correct irq edge in the example block.
> Based on the datasheet, in table 13 on page 11:
> "End-of-conversion indicator: This pin is set high when a measurement
> and calculation have been completed and the data is ready to be
> clocked out"
> 
> Add description on End-of-conversion interrupt.
> 
> Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
> Signed-off-by: Andreas Klinger <ak@it-klinger.de>
What's the relationship between Andreas and this patch?

Petre seems to have sent it so either Andreas should have a Co-authored-by or
should be the author... Or not there at all

Thanks,

Jonathan

> ---
>  .../bindings/iio/pressure/honeywell,mprls0025pa.yaml       | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml b/Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml
> index d9e903fbfd99..84ced4e5a7da 100644
> --- a/Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml
> +++ b/Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml
> @@ -42,6 +42,10 @@ properties:
>      maxItems: 1
> 
>    interrupts:
> +    description:
> +      Optional interrupt for indicating End-of-conversion.
> +      If not present, the driver loops for a while until the received status
> +      byte indicates correct measurement.
>      maxItems: 1
> 
>    reset-gpios:
> @@ -65,6 +69,7 @@ properties:
>        1 - A, 10% to 90% of 2^24 (1677722 .. 15099494)
>        2 - B, 2.5% to 22.5% of 2^24 (419430 .. 3774874)
>        3 - C, 20% to 80% of 2^24 (3355443 .. 13421773)
> +    enum: [1, 2, 3]
>      $ref: /schemas/types.yaml#/definitions/uint32
> 
>    vdd-supply:
> @@ -93,7 +98,7 @@ examples:
>              reg = <0x18>;
>              reset-gpios = <&gpio3 19 GPIO_ACTIVE_HIGH>;
>              interrupt-parent = <&gpio3>;
> -            interrupts = <21 IRQ_TYPE_EDGE_FALLING>;
> +            interrupts = <21 IRQ_TYPE_EDGE_RISING>;
>              honeywell,pmin-pascal = <0>;
>              honeywell,pmax-pascal = <172369>;
>              honeywell,transfer-function = <1>;
> --
> 2.41.0
> 

Re: [PATCH v2 01/10] dt-bindings: iio: pressure: honeywell,mprls0025pa.yaml fix

[Jonathan Cameron]

On Tue, 26 Dec 2023 16:28:39 +0000
Jonathan Cameron <jic23@kernel.org> wrote:

> On Sun, 24 Dec 2023 16:34:46 +0200
> Petre Rodan <petre.rodan@subdimension.ro> wrote:
> 
> > Define enum inside the honeywell,transfer-function property block.
> > 
> > Set the correct irq edge in the example block.
> > Based on the datasheet, in table 13 on page 11:
> > "End-of-conversion indicator: This pin is set high when a measurement
> > and calculation have been completed and the data is ready to be
> > clocked out"
> > 
> > Add description on End-of-conversion interrupt.
> > 
> > Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
> > Signed-off-by: Andreas Klinger <ak@it-klinger.de>  
> What's the relationship between Andreas and this patch?
> 
> Petre seems to have sent it so either Andreas should have a Co-authored-by or
Co-developed-by: that is.


> should be the author... Or not there at all
> 
> Thanks,
> 
> Jonathan
> 
> > ---
> >  .../bindings/iio/pressure/honeywell,mprls0025pa.yaml       | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml b/Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml
> > index d9e903fbfd99..84ced4e5a7da 100644
> > --- a/Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml
> > +++ b/Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml
> > @@ -42,6 +42,10 @@ properties:
> >      maxItems: 1
> > 
> >    interrupts:
> > +    description:
> > +      Optional interrupt for indicating End-of-conversion.
> > +      If not present, the driver loops for a while until the received status
> > +      byte indicates correct measurement.
> >      maxItems: 1
> > 
> >    reset-gpios:
> > @@ -65,6 +69,7 @@ properties:
> >        1 - A, 10% to 90% of 2^24 (1677722 .. 15099494)
> >        2 - B, 2.5% to 22.5% of 2^24 (419430 .. 3774874)
> >        3 - C, 20% to 80% of 2^24 (3355443 .. 13421773)
> > +    enum: [1, 2, 3]
> >      $ref: /schemas/types.yaml#/definitions/uint32
> > 
> >    vdd-supply:
> > @@ -93,7 +98,7 @@ examples:
> >              reg = <0x18>;
> >              reset-gpios = <&gpio3 19 GPIO_ACTIVE_HIGH>;
> >              interrupt-parent = <&gpio3>;
> > -            interrupts = <21 IRQ_TYPE_EDGE_FALLING>;
> > +            interrupts = <21 IRQ_TYPE_EDGE_RISING>;
> >              honeywell,pmin-pascal = <0>;
> >              honeywell,pmax-pascal = <172369>;
> >              honeywell,transfer-function = <1>;
> > --
> > 2.41.0
> >   
> 
> 

Re: [PATCH v2 01/10] dt-bindings: iio: pressure: honeywell,mprls0025pa.yaml fix

[Petre Rodan]

[-- Attachment #1: Type: text/plain, Size: 2005 bytes --]


Hello Cameron,

On Tue, Dec 26, 2023 at 04:28:39PM +0000, Jonathan Cameron wrote:
> On Sun, 24 Dec 2023 16:34:46 +0200
> Petre Rodan <petre.rodan@subdimension.ro> wrote:
> 
> > Define enum inside the honeywell,transfer-function property block.
> > 
> > Set the correct irq edge in the example block.
> > Based on the datasheet, in table 13 on page 11:
> > "End-of-conversion indicator: This pin is set high when a measurement
> > and calculation have been completed and the data is ready to be
> > clocked out"
> > 
> > Add description on End-of-conversion interrupt.
> > 
> > Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
> > Signed-off-by: Andreas Klinger <ak@it-klinger.de>
> What's the relationship between Andreas and this patch?
> 
> Petre seems to have sent it so either Andreas should have a Co-authored-by or
> should be the author... Or not there at all

Andreas has written this driver as it is in the mainline tree right now and he
is marked as a maintainer for it.
A month back I told him about the enum off-by-one problem and also about my plan
of adding more features to the driver.
He was happy to accept my code and once I sent v1 of this patch to the list has
asked to work together for the v2 you see here. This has helped with cleaning up
the code. He requested the additional 'Signed-off-by' tag, but if you have a more
explicit one I will happily use it. 'Co-developed-by' it is.

He also owns an i2c version of the sensor so he was able to make sure that the
original half of the driver still works after my refactor, hence the 'Tested-by'
tag in the last patch.

please tell me how do the 'fixes'/feature/improvement tags/keywords look like?
are these to be added on the subject line, or should they reside near my
'Signed-off-by' inside the email body? I probably missed the documentation where
these are covered :)

also, should I add a 'Reviewed-by:' you for 09/10 and 10/10 (the last two patches)?

best regards,
peter

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 01/10] dt-bindings: iio: pressure: honeywell,mprls0025pa.yaml fix

[Jonathan Cameron]

On Wed, 27 Dec 2023 09:11:35 +0200
Petre Rodan <petre.rodan@subdimension.ro> wrote:

> Hello Cameron,
> 
> On Tue, Dec 26, 2023 at 04:28:39PM +0000, Jonathan Cameron wrote:
> > On Sun, 24 Dec 2023 16:34:46 +0200
> > Petre Rodan <petre.rodan@subdimension.ro> wrote:
> >   
> > > Define enum inside the honeywell,transfer-function property block.
> > > 
> > > Set the correct irq edge in the example block.
> > > Based on the datasheet, in table 13 on page 11:
> > > "End-of-conversion indicator: This pin is set high when a measurement
> > > and calculation have been completed and the data is ready to be
> > > clocked out"
> > > 
> > > Add description on End-of-conversion interrupt.
> > > 
> > > Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
> > > Signed-off-by: Andreas Klinger <ak@it-klinger.de>  
> > What's the relationship between Andreas and this patch?
> > 
> > Petre seems to have sent it so either Andreas should have a Co-authored-by or
> > should be the author... Or not there at all  
> 
> Andreas has written this driver as it is in the mainline tree right now and he
> is marked as a maintainer for it.
> A month back I told him about the enum off-by-one problem and also about my plan
> of adding more features to the driver.
> He was happy to accept my code and once I sent v1 of this patch to the list has
> asked to work together for the v2 you see here. This has helped with cleaning up
> the code. He requested the additional 'Signed-off-by' tag, but if you have a more
> explicit one I will happily use it. 'Co-developed-by' it is.

Yup, needs combination of Co-developed-by and a Sign off for this case as in
effect both of you wrote the code.

> 
> He also owns an i2c version of the sensor so he was able to make sure that the
> original half of the driver still works after my refactor, hence the 'Tested-by'
> tag in the last patch.

That's fine.

> 
> please tell me how do the 'fixes'/feature/improvement tags/keywords look like?
> are these to be added on the subject line, or should they reside near my
> 'Signed-off-by' inside the email body? I probably missed the documentation where
> these are covered :)
See https://elixir.bootlin.com/linux/latest/source/Documentation/process/submitting-patches.rst
for lots of detail.  The fixes tag has a particular format and goes in the main
tag block. Lots of examples in tree so can just take a look at the git history for
how to apply these rules in practice.

> 
> also, should I add a 'Reviewed-by:' you for 09/10 and 10/10 (the last two patches)?
No. Tags should be explicitly given.  I tend not to give RB for stuff I'll pick up
because they will have my SoB anyway as the person who applies the patches
and that includes reviewing.

In this case, Andreas asked for a tag to be included whereas I have not.

Jonathan


> 
> best regards,
> peter