[PATCH v2 00/10] iio: adc: ad7606: Improvements

[PATCH v2 00/10] iio: adc: ad7606: Improvements

[Guillaume Stols]

This series adds the following improvements over the current AD7606's
driver implementation:

- Fix wrong usage of gpio array
- Fix standby that was documented as ACTIVE_LOW but handled in the
  driver as if it was ACTIVE_HIGH
- Improve dt-bindings documentation
- Switch mutex lock to scoped guard

Signed-off-by: Guillaume Stols <gstols@baylibre.com>
---
Changes in v2:
- Change scoped guard to guard(mutex)(&st->lock). This was tested with
  Rob's bot condition, and seems not to generate warning anymore.
- Reorder the commits for avoiding bisection issues and respect commit
  priority rules.
- Add vdrive-supply to required properties.
- Separate cosmetic changes from content ones in dt-binding
  descriptions.
- Move maxItems changes (and plural in descriptions) to the commit that
  adds conditions.
- Link to v1: https://lore.kernel.org/r/20240618-cleanup-ad7606-v1-0-f1854d5c779d@baylibre.com

---
Guillaume Stols (10):
      dt-bindings: iio: adc: adi,ad7606: add missing datasheet link
      dt-bindings: iio: adc: adi,ad7606: comment and sort the compatible names
      dt-bindings: iio: adc: adi,ad7606: normalize textwidth
      dt-bindings: iio: adc: adi,ad7606: improve descriptions
      dt-bindings: iio: adc: adi,ad7606: add supply properties
      dt-bindings: iio: adc: adi,ad7606: fix example
      dt-bindings: iio: adc: adi,ad7606: add conditions
      iio: adc: ad7606: fix oversampling gpio array
      iio: adc: ad7606: fix standby gpio state to match the documentation
      iio: adc: ad7606: switch mutexes to scoped_guard

 .../devicetree/bindings/iio/adc/adi,ad7606.yaml    | 129 ++++++++++++++++-----
 drivers/iio/adc/ad7606.c                           |  68 +++++------
 drivers/iio/adc/ad7606_spi.c                       |   5 +-
 3 files changed, 131 insertions(+), 71 deletions(-)
---
base-commit: 07d4d0bb4a8ddcc463ed599b22f510d5926c2495
change-id: 20240416-cleanup-ad7606-161e2ed9818b

Best regards,
-- 
Guillaume Stols <gstols@baylibre.com>

[PATCH v2 01/10] dt-bindings: iio: adc: adi,ad7606: add missing datasheet link

[Guillaume Stols]

Add AD7606-5 datasheet link.

Signed-off-by: Guillaume Stols <gstols@baylibre.com>
---
 Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
index 7fa46df1f4fb..d55c58400df5 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
@@ -11,6 +11,7 @@ maintainers:
 
 description: |
   Analog Devices AD7606 Simultaneous Sampling ADC
+  https://www.analog.com/media/en/technical-documentation/data-sheets/AD7605-4.pdf
   https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606_7606-6_7606-4.pdf
   https://www.analog.com/media/en/technical-documentation/data-sheets/AD7606B.pdf
   https://www.analog.com/media/en/technical-documentation/data-sheets/AD7616.pdf

-- 
2.34.1

Re: [PATCH v2 01/10] dt-bindings: iio: adc: adi,ad7606: add missing datasheet link

[Jonathan Cameron]

On Fri, 28 Jun 2024 14:48:19 +0000
Guillaume Stols <gstols@baylibre.com> wrote:

> Add AD7606-5 datasheet link.
> 
> Signed-off-by: Guillaume Stols <gstols@baylibre.com>
You need to pick up all the tags given on earlier versions.
Rob acked this one for example on Thursday and you sent this update
on Friday so should have seen that to pick it up.

If there are substantial changes and it is appropriate to drop a tag,
say so below the ---
 
Applied with tag from Rob put in place.

Thanks,

Jonathan


> ---
>  Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> index 7fa46df1f4fb..d55c58400df5 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> @@ -11,6 +11,7 @@ maintainers:
>  
>  description: |
>    Analog Devices AD7606 Simultaneous Sampling ADC
> +  https://www.analog.com/media/en/technical-documentation/data-sheets/AD7605-4.pdf
>    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606_7606-6_7606-4.pdf
>    https://www.analog.com/media/en/technical-documentation/data-sheets/AD7606B.pdf
>    https://www.analog.com/media/en/technical-documentation/data-sheets/AD7616.pdf
> 

[PATCH v2 02/10] dt-bindings: iio: adc: adi,ad7606: comment and sort the compatible names

[Guillaume Stols]

AD7606-8 is referred to as AD7606 by Analog Devices. This comment aims
to avoid confusion. Also the compatible names were not sorted by
alphabetical order.

Signed-off-by: Guillaume Stols <gstols@baylibre.com>
---
 Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
index d55c58400df5..00fdaed11cbd 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
@@ -20,9 +20,9 @@ properties:
   compatible:
     enum:
       - adi,ad7605-4
-      - adi,ad7606-8
-      - adi,ad7606-6
       - adi,ad7606-4
+      - adi,ad7606-6
+      - adi,ad7606-8  # Referred to as AD7606 (without -8) in the datasheet
       - adi,ad7606b
       - adi,ad7616
 

-- 
2.34.1

Re: [PATCH v2 02/10] dt-bindings: iio: adc: adi,ad7606: comment and sort the compatible names

[Jonathan Cameron]

On Fri, 28 Jun 2024 14:48:20 +0000
Guillaume Stols <gstols@baylibre.com> wrote:

> AD7606-8 is referred to as AD7606 by Analog Devices. This comment aims
> to avoid confusion. Also the compatible names were not sorted by
> alphabetical order.
> 
> Signed-off-by: Guillaume Stols <gstols@baylibre.com>

So b4 interestingly picked up both acks from Rob and Conor on this
one but I can't figure out where Conor's one came from so I've dropped
it.

Applied.

> ---
>  Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> index d55c58400df5..00fdaed11cbd 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> @@ -20,9 +20,9 @@ properties:
>    compatible:
>      enum:
>        - adi,ad7605-4
> -      - adi,ad7606-8
> -      - adi,ad7606-6
>        - adi,ad7606-4
> +      - adi,ad7606-6
> +      - adi,ad7606-8  # Referred to as AD7606 (without -8) in the datasheet
>        - adi,ad7606b
>        - adi,ad7616
>  
> 

Re: [PATCH v2 02/10] dt-bindings: iio: adc: adi,ad7606: comment and sort the compatible names

[Conor Dooley]

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

On Sat, Jun 29, 2024 at 04:22:23PM +0100, Jonathan Cameron wrote:
> On Fri, 28 Jun 2024 14:48:20 +0000
> Guillaume Stols <gstols@baylibre.com> wrote:
> 
> > AD7606-8 is referred to as AD7606 by Analog Devices. This comment aims
> > to avoid confusion. Also the compatible names were not sorted by
> > alphabetical order.
> > 
> > Signed-off-by: Guillaume Stols <gstols@baylibre.com>
> 
> So b4 interestingly picked up both acks from Rob and Conor on this
> one but I can't figure out where Conor's one came from so I've dropped
> it.

My copy of b4 (0.14-dev-d4707) doesn't create one for me:
/stuff/b4/b4.sh shazam -s -S -t shazam 20240628-cleanup-ad7606-v2-2-96e02f90256d@baylibre.com
Grabbing thread from lore.kernel.org/all/20240628-cleanup-ad7606-v2-2-96e02f90256d@baylibre.com/t.mbox.gz
Checking for newer revisions
Grabbing search results from lore.kernel.org
Analyzing 20 messages in the thread
Looking for additional code-review trailers on lore.kernel.org
Checking attestation on all messages, may take a moment...
---
  ✓ [PATCH v2 1/10] dt-bindings: iio: adc: adi,ad7606: add missing datasheet link
    + Acked-by: Rob Herring (Arm) <robh@kernel.org> (✓ DKIM/kernel.org)
    + Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
  ✓ [PATCH v2 2/10] dt-bindings: iio: adc: adi,ad7606: comment and sort the compatible names
    + Acked-by: Rob Herring (Arm) <robh@kernel.org> (✓ DKIM/kernel.org)
    + Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
  ✓ [PATCH v2 3/10] dt-bindings: iio: adc: adi,ad7606: normalize textwidth
    + Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
  ✓ [PATCH v2 4/10] dt-bindings: iio: adc: adi,ad7606: improve descriptions
    + Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
  ✓ [PATCH v2 5/10] dt-bindings: iio: adc: adi,ad7606: add supply properties
    + Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
  ✓ [PATCH v2 6/10] dt-bindings: iio: adc: adi,ad7606: fix example
    + Acked-by: Conor Dooley <conor.dooley@microchip.com> (✓ DKIM/kernel.org)
    + Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
  ✓ [PATCH v2 7/10] dt-bindings: iio: adc: adi,ad7606: add conditions
    + Reviewed-by: Conor Dooley <conor.dooley@microchip.com> (✓ DKIM/kernel.org)
    + Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
  ✓ [PATCH v2 8/10] iio: adc: ad7606: fix oversampling gpio array
    + Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
  ✓ [PATCH v2 9/10] iio: adc: ad7606: fix standby gpio state to match the documentation
    + Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
  ✓ [PATCH v2 10/10] iio: adc: ad7606: switch mutexes to scoped_guard
    + Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
  ---
  ✓ Signed: DKIM/baylibre-com.20230601.gappssmtp.com (From: gstols@baylibre.com)
---
Total patches: 10
---
 Base: using specified base-commit 07d4d0bb4a8ddcc463ed599b22f510d5926c2495
Applying: dt-bindings: iio: adc: adi,ad7606: add missing datasheet link
Applying: dt-bindings: iio: adc: adi,ad7606: comment and sort the compatible names
Applying: dt-bindings: iio: adc: adi,ad7606: normalize textwidth
Applying: dt-bindings: iio: adc: adi,ad7606: improve descriptions
Applying: dt-bindings: iio: adc: adi,ad7606: add supply properties
Applying: dt-bindings: iio: adc: adi,ad7606: fix example
Applying: dt-bindings: iio: adc: adi,ad7606: add conditions
Applying: iio: adc: ad7606: fix oversampling gpio array
Applying: iio: adc: ad7606: fix standby gpio state to match the documentation
Applying: iio: adc: ad7606: switch mutexes to scoped_guard

tbh, I'm not actually sure why I didn't ack those patches on v1, they
were all pretty trivial...

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

[PATCH v2 03/10] dt-bindings: iio: adc: adi,ad7606: normalize textwidth

[Guillaume Stols]

Normalize textwidth to 80 columns on the descriptions.

Signed-off-by: Guillaume Stols <gstols@baylibre.com>
---
 .../devicetree/bindings/iio/adc/adi,ad7606.yaml    | 50 +++++++++++-----------
 1 file changed, 24 insertions(+), 26 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
index 00fdaed11cbd..32caa9fc00e4 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
@@ -40,60 +40,58 @@ properties:
 
   adi,conversion-start-gpios:
     description:
-      Must be the device tree identifier of the CONVST pin.
-      This logic input is used to initiate conversions on the analog
-      input channels. As the line is active high, it should be marked
-      GPIO_ACTIVE_HIGH.
+      Must be the device tree identifier of the CONVST pin. This logic input is
+      used to initiate conversions on the analog input channels. As the line is
+      active high, it should be marked GPIO_ACTIVE_HIGH.
     maxItems: 1
 
   reset-gpios:
     description:
-      Must be the device tree identifier of the RESET pin. If specified,
-      it will be asserted during driver probe. As the line is active high,
-      it should be marked GPIO_ACTIVE_HIGH.
+      Must be the device tree identifier of the RESET pin. If specified, it will
+      be asserted during driver probe. As the line is active high, it should be
+      marked GPIO_ACTIVE_HIGH.
     maxItems: 1
 
   standby-gpios:
     description:
-      Must be the device tree identifier of the STBY pin. This pin is used
-      to place the AD7606 into one of two power-down modes, Standby mode or
+      Must be the device tree identifier of the STBY pin. This pin is used to
+      place the AD7606 into one of two power-down modes, Standby mode or
       Shutdown mode. As the line is active low, it should be marked
       GPIO_ACTIVE_LOW.
     maxItems: 1
 
   adi,first-data-gpios:
     description:
-      Must be the device tree identifier of the FRSTDATA pin.
-      The FRSTDATA output indicates when the first channel, V1, is
-      being read back on either the parallel, byte or serial interface.
-      As the line is active high, it should be marked GPIO_ACTIVE_HIGH.
+      Must be the device tree identifier of the FRSTDATA pin.  The FRSTDATA
+      output indicates when the first channel, V1, is being read back on either
+      the parallel, byte or serial interface.  As the line is active high, it
+      should be marked GPIO_ACTIVE_HIGH.
     maxItems: 1
 
   adi,range-gpios:
     description:
-      Must be the device tree identifier of the RANGE pin. The polarity on
-      this pin determines the input range of the analog input channels. If
-      this pin is tied to a logic high, the analog input range is ±10V for
-      all channels. If this pin is tied to a logic low, the analog input range
-      is ±5V for all channels. As the line is active high, it should be marked
+      Must be the device tree identifier of the RANGE pin. The polarity on this
+      pin determines the input range of the analog input channels. If this pin
+      is tied to a logic high, the analog input range is ±10V for all channels.
+      If this pin is tied to a logic low, the analog input range is ±5V for all
+      channels. As the line is active high, it should be marked
       GPIO_ACTIVE_HIGH.
     maxItems: 1
 
   adi,oversampling-ratio-gpios:
     description:
-      Must be the device tree identifier of the over-sampling
-      mode pins. As the line is active high, it should be marked
-      GPIO_ACTIVE_HIGH.
+      Must be the device tree identifier of the over-sampling mode pins. As the
+      line is active high, it should be marked GPIO_ACTIVE_HIGH.
     maxItems: 3
 
   adi,sw-mode:
     description:
       Software mode of operation, so far available only for ad7616 and ad7606b.
-      It is enabled when all three oversampling mode pins are connected to
-      high level. The device is configured by the corresponding registers. If the
-      adi,oversampling-ratio-gpios property is defined, then the driver will set the
-      oversampling gpios to high. Otherwise, it is assumed that the pins are hardwired
-      to VDD.
+      It is enabled when all three oversampling mode pins are connected to high
+      level. The device is configured by the corresponding registers. If the
+      adi,oversampling-ratio-gpios property is defined, then the driver will set
+      the oversampling gpios to high. Otherwise, it is assumed that the pins are
+      hardwired to VDD.
     type: boolean
 
 required:

-- 
2.34.1

Re: [PATCH v2 03/10] dt-bindings: iio: adc: adi,ad7606: normalize textwidth

[Rob Herring (Arm)]

On Fri, 28 Jun 2024 14:48:21 +0000, Guillaume Stols wrote:
> Normalize textwidth to 80 columns on the descriptions.
> 
> Signed-off-by: Guillaume Stols <gstols@baylibre.com>
> ---
>  .../devicetree/bindings/iio/adc/adi,ad7606.yaml    | 50 +++++++++++-----------
>  1 file changed, 24 insertions(+), 26 deletions(-)
> 

Acked-by: Rob Herring (Arm) <robh@kernel.org>

[PATCH v2 04/10] dt-bindings: iio: adc: adi,ad7606: improve descriptions

[Guillaume Stols]

Reword a few descriptions, and normalize the text width to 80 characters.

Signed-off-by: Guillaume Stols <gstols@baylibre.com>
---
 .../devicetree/bindings/iio/adc/adi,ad7606.yaml    | 33 +++++++++++++---------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
index 32caa9fc00e4..509970922cda 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
@@ -36,6 +36,9 @@ properties:
   avcc-supply: true
 
   interrupts:
+    description:
+      The BUSY pin falling edge indicates that the conversion is over, and thus
+      new data is available.
     maxItems: 1
 
   adi,conversion-start-gpios:
@@ -48,8 +51,9 @@ properties:
   reset-gpios:
     description:
       Must be the device tree identifier of the RESET pin. If specified, it will
-      be asserted during driver probe. As the line is active high, it should be
-      marked GPIO_ACTIVE_HIGH.
+      be asserted during driver probe. On the AD7606x, as the line is active
+      high, it should be marked GPIO_ACTIVE_HIGH. On the AD7616, as the line is
+      active low, it should be marked GPIO_ACTIVE_LOW.
     maxItems: 1
 
   standby-gpios:
@@ -62,36 +66,39 @@ properties:
 
   adi,first-data-gpios:
     description:
-      Must be the device tree identifier of the FRSTDATA pin.  The FRSTDATA
+      Must be the device tree identifier of the FRSTDATA pin. The FRSTDATA
       output indicates when the first channel, V1, is being read back on either
-      the parallel, byte or serial interface.  As the line is active high, it
+      the parallel, byte or serial interface. As the line is active high, it
       should be marked GPIO_ACTIVE_HIGH.
     maxItems: 1
 
   adi,range-gpios:
     description:
-      Must be the device tree identifier of the RANGE pin. The polarity on this
+      Must be the device tree identifier of the RANGE pin. The state on this
       pin determines the input range of the analog input channels. If this pin
       is tied to a logic high, the analog input range is ±10V for all channels.
-      If this pin is tied to a logic low, the analog input range is ±5V for all
-      channels. As the line is active high, it should be marked
+      On the AD760X, if this pin is tied to a logic low, the analog input range
+      is ±5V for all channels. As the line is active high, it should be marked
       GPIO_ACTIVE_HIGH.
     maxItems: 1
 
   adi,oversampling-ratio-gpios:
     description:
       Must be the device tree identifier of the over-sampling mode pins. As the
-      line is active high, it should be marked GPIO_ACTIVE_HIGH.
+      line is active high, it should be marked GPIO_ACTIVE_HIGH. On the AD7606X
+      parts that support it, if all 3 pins are tied to a logic high, software
+      mode is enabled.
     maxItems: 3
 
   adi,sw-mode:
     description:
-      Software mode of operation, so far available only for ad7616 and ad7606b.
+      Software mode of operation, so far available only for AD7616 and AD7606b.
       It is enabled when all three oversampling mode pins are connected to high
-      level. The device is configured by the corresponding registers. If the
-      adi,oversampling-ratio-gpios property is defined, then the driver will set
-      the oversampling gpios to high. Otherwise, it is assumed that the pins are
-      hardwired to VDD.
+      level for the AD7606B, or all two range selection pins are connected to
+      high level for the AD7616. The device is configured by the corresponding
+      registers. If the adi,oversampling-ratio-gpios property is defined, then
+      the driver will set the oversampling gpios to high. Otherwise, it is
+      assumed that the pins are hardwired to VDD.
     type: boolean
 
 required:

-- 
2.34.1

Re: [PATCH v2 04/10] dt-bindings: iio: adc: adi,ad7606: improve descriptions

[Jonathan Cameron]

On Fri, 28 Jun 2024 14:48:22 +0000
Guillaume Stols <gstols@baylibre.com> wrote:

> Reword a few descriptions, and normalize the text width to 80 characters.
> 
> Signed-off-by: Guillaume Stols <gstols@baylibre.com>

One minor comment inline.  I can tidy it up whilst applying if
everything else is fine in the rest of the series.

Jonathan

> ---
>  .../devicetree/bindings/iio/adc/adi,ad7606.yaml    | 33 +++++++++++++---------
>  1 file changed, 20 insertions(+), 13 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> index 32caa9fc00e4..509970922cda 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> @@ -36,6 +36,9 @@ properties:
>    avcc-supply: true
>  
>    interrupts:
> +    description:
> +      The BUSY pin falling edge indicates that the conversion is over, and thus
> +      new data is available.
>      maxItems: 1
>  
>    adi,conversion-start-gpios:
> @@ -48,8 +51,9 @@ properties:
>    reset-gpios:
>      description:
>        Must be the device tree identifier of the RESET pin. If specified, it will
> -      be asserted during driver probe. As the line is active high, it should be
> -      marked GPIO_ACTIVE_HIGH.
> +      be asserted during driver probe. On the AD7606x, as the line is active
> +      high, it should be marked GPIO_ACTIVE_HIGH. On the AD7616, as the line is
> +      active low, it should be marked GPIO_ACTIVE_LOW.
>      maxItems: 1
>  
>    standby-gpios:
> @@ -62,36 +66,39 @@ properties:
>  
>    adi,first-data-gpios:
>      description:
> -      Must be the device tree identifier of the FRSTDATA pin.  The FRSTDATA
> +      Must be the device tree identifier of the FRSTDATA pin. The FRSTDATA
>        output indicates when the first channel, V1, is being read back on either
> -      the parallel, byte or serial interface.  As the line is active high, it
> +      the parallel, byte or serial interface. As the line is active high, it
>        should be marked GPIO_ACTIVE_HIGH.
>      maxItems: 1
>  
>    adi,range-gpios:
>      description:
> -      Must be the device tree identifier of the RANGE pin. The polarity on this
> +      Must be the device tree identifier of the RANGE pin. The state on this
>        pin determines the input range of the analog input channels. If this pin
>        is tied to a logic high, the analog input range is ±10V for all channels.
> -      If this pin is tied to a logic low, the analog input range is ±5V for all
> -      channels. As the line is active high, it should be marked
> +      On the AD760X, if this pin is tied to a logic low, the analog input range
> +      is ±5V for all channels. As the line is active high, it should be marked
>        GPIO_ACTIVE_HIGH.
>      maxItems: 1
>  
>    adi,oversampling-ratio-gpios:
>      description:
>        Must be the device tree identifier of the over-sampling mode pins. As the
> -      line is active high, it should be marked GPIO_ACTIVE_HIGH.
> +      line is active high, it should be marked GPIO_ACTIVE_HIGH. On the AD7606X
> +      parts that support it, if all 3 pins are tied to a logic high, software
> +      mode is enabled.
>      maxItems: 3
>  
>    adi,sw-mode:
>      description:
> -      Software mode of operation, so far available only for ad7616 and ad7606b.
> +      Software mode of operation, so far available only for AD7616 and AD7606b.
>        It is enabled when all three oversampling mode pins are connected to high
> -      level. The device is configured by the corresponding registers. If the
> -      adi,oversampling-ratio-gpios property is defined, then the driver will set
> -      the oversampling gpios to high. Otherwise, it is assumed that the pins are
> -      hardwired to VDD.
> +      level for the AD7606B, or all two range selection pins are connected to
If rolling again for some reason 

       level for the AD7606B, or both the range selections are connected to high
       level for the AD7616.

> +      high level for the AD7616. The device is configured by the corresponding
> +      registers. If the adi,oversampling-ratio-gpios property is defined, then
> +      the driver will set the oversampling gpios to high. Otherwise, it is
> +      assumed that the pins are hardwired to VDD.
>      type: boolean
>  
>  required:
> 

Re: [PATCH v2 04/10] dt-bindings: iio: adc: adi,ad7606: improve descriptions

[Rob Herring (Arm)]

On Fri, 28 Jun 2024 14:48:22 +0000, Guillaume Stols wrote:
> Reword a few descriptions, and normalize the text width to 80 characters.
> 
> Signed-off-by: Guillaume Stols <gstols@baylibre.com>
> ---
>  .../devicetree/bindings/iio/adc/adi,ad7606.yaml    | 33 +++++++++++++---------
>  1 file changed, 20 insertions(+), 13 deletions(-)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

[PATCH v2 05/10] dt-bindings: iio: adc: adi,ad7606: add supply properties

[Guillaume Stols]

Add voltage supplies

Signed-off-by: Guillaume Stols <gstols@baylibre.com>
---
 Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
index 509970922cda..a1c38a5812f7 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
@@ -35,6 +35,15 @@ properties:
 
   avcc-supply: true
 
+  vdrive-supply:
+    description:
+      Determines the voltage level at which the interface logic pins will
+      operate.
+
+  refin-supply:
+    description:
+      The voltage supply for optional external reference voltage.
+
   interrupts:
     description:
       The BUSY pin falling edge indicates that the conversion is over, and thus
@@ -106,9 +115,11 @@ required:
   - reg
   - spi-cpha
   - avcc-supply
+  - vdrive-supply
   - interrupts
   - adi,conversion-start-gpios
 
+
 allOf:
   - $ref: /schemas/spi/spi-peripheral-props.yaml#
 
@@ -130,6 +141,7 @@ examples:
             spi-cpha;
 
             avcc-supply = <&adc_vref>;
+            vdrive-supply = <&vdd_supply>;
 
             interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
             interrupt-parent = <&gpio>;

-- 
2.34.1

Re: [PATCH v2 05/10] dt-bindings: iio: adc: adi,ad7606: add supply properties

[Jonathan Cameron]

On Fri, 28 Jun 2024 14:48:23 +0000
Guillaume Stols <gstols@baylibre.com> wrote:

> Add voltage supplies
> 
> Signed-off-by: Guillaume Stols <gstols@baylibre.com>
Trivial comment inline. No need to reroll for that though.

> ---
>  Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> index 509970922cda..a1c38a5812f7 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> @@ -35,6 +35,15 @@ properties:
>  
>    avcc-supply: true
>  
> +  vdrive-supply:
> +    description:
> +      Determines the voltage level at which the interface logic pins will
> +      operate.
> +
> +  refin-supply:
> +    description:
> +      The voltage supply for optional external reference voltage.
> +
>    interrupts:
>      description:
>        The BUSY pin falling edge indicates that the conversion is over, and thus
> @@ -106,9 +115,11 @@ required:
>    - reg
>    - spi-cpha
>    - avcc-supply
> +  - vdrive-supply
>    - interrupts
>    - adi,conversion-start-gpios
>  
> +
Unrelated white space change.  Remember to check patches for these.
If whitespace changes are needed have an additional patch that just does
that.

>  allOf:
>    - $ref: /schemas/spi/spi-peripheral-props.yaml#
>  
> @@ -130,6 +141,7 @@ examples:
>              spi-cpha;
>  
>              avcc-supply = <&adc_vref>;
> +            vdrive-supply = <&vdd_supply>;
>  
>              interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
>              interrupt-parent = <&gpio>;
> 

[PATCH v2 06/10] dt-bindings: iio: adc: adi,ad7606: fix example

[Guillaume Stols]

Example uses adi,ad7606-8 as compatible, but adi,sw-mode is not
available for it. So remove this property from example.

Signed-off-by: Guillaume Stols <gstols@baylibre.com>
---
 Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml | 1 -
 1 file changed, 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
index a1c38a5812f7..626782ea00b3 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
@@ -153,7 +153,6 @@ examples:
                                            <&gpio 23 GPIO_ACTIVE_HIGH>,
                                            <&gpio 26 GPIO_ACTIVE_HIGH>;
             standby-gpios = <&gpio 24 GPIO_ACTIVE_LOW>;
-            adi,sw-mode;
         };
     };
 ...

-- 
2.34.1

[PATCH v2 07/10] dt-bindings: iio: adc: adi,ad7606: add conditions

[Guillaume Stols]

Since the driver supports several parts that present differences in
their layout and behaviour, it is necessary to describe the differences
from one chip to another.

Signed-off-by: Guillaume Stols <gstols@baylibre.com>
---
 .../devicetree/bindings/iio/adc/adi,ad7606.yaml    | 60 +++++++++++++++++++---
 1 file changed, 54 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
index 626782ea00b3..f825a2c19a06 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
@@ -52,10 +52,11 @@ properties:
 
   adi,conversion-start-gpios:
     description:
-      Must be the device tree identifier of the CONVST pin. This logic input is
-      used to initiate conversions on the analog input channels. As the line is
-      active high, it should be marked GPIO_ACTIVE_HIGH.
-    maxItems: 1
+      Must be the device tree identifier of the CONVST pin(s). This logic input
+      is used to initiate conversions on the analog input channels. As the line
+      is active high, it should be marked GPIO_ACTIVE_HIGH.
+    minItems: 1
+    maxItems: 2
 
   reset-gpios:
     description:
@@ -88,8 +89,11 @@ properties:
       is tied to a logic high, the analog input range is ±10V for all channels.
       On the AD760X, if this pin is tied to a logic low, the analog input range
       is ±5V for all channels. As the line is active high, it should be marked
-      GPIO_ACTIVE_HIGH.
-    maxItems: 1
+      GPIO_ACTIVE_HIGH. On the AD7616, there are 2 pins, and if the 2 pins are
+      tied to a logic high, software mode is enabled, otherwise one of the 3
+      possible range values is selected.
+    minItems: 1
+    maxItems: 2
 
   adi,oversampling-ratio-gpios:
     description:
@@ -123,6 +127,50 @@ required:
 allOf:
   - $ref: /schemas/spi/spi-peripheral-props.yaml#
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: adi,ad7616
+    then:
+      properties:
+        adi,first-data-gpios: false
+        standby-gpios: false
+        adi,range-gpios:
+          maxItems: 2
+    else:
+      properties:
+        adi,range-gpios:
+          maxItems: 1
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - adi,ad7605-4
+              - adi,ad7616
+    then:
+      properties:
+        adi,oversampling-ratio-gpios: false
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - adi,ad7605-4
+              - adi,ad7606-4
+              - adi,ad7606-6
+              - adi,ad7606-8
+    then:
+      properties:
+        adi,sw-mode: false
+    else:
+      properties:
+        adi,conversion-start-gpios:
+          maxItems: 1
+
 unevaluatedProperties: false
 
 examples:

-- 
2.34.1

Re: [PATCH v2 07/10] dt-bindings: iio: adc: adi,ad7606: add conditions

[Conor Dooley]

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

On Fri, Jun 28, 2024 at 02:48:25PM +0000, Guillaume Stols wrote:
> Since the driver supports several parts that present differences in
> their layout and behaviour, it is necessary to describe the differences
> from one chip to another.
> 
> Signed-off-by: Guillaume Stols <gstols@baylibre.com>

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

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

[PATCH v2 08/10] iio: adc: ad7606: fix oversampling gpio array

[Guillaume Stols]

gpiod_set_array_value was misused here: the implementation relied on the
assumption that an unsigned long was required for each gpio, while the
function expects a bit array stored in "as much unsigned long as needed
for storing one bit per GPIO", i.e it is using a bit field.

Fixes: d2a415c86c6b ("iio: adc: ad7606: Add support for AD7606B ADC")
Signed-off-by: Guillaume Stols <gstols@baylibre.com>
---
 drivers/iio/adc/ad7606.c     | 4 ++--
 drivers/iio/adc/ad7606_spi.c | 5 +++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
index 3a417595294f..8cce1fad9763 100644
--- a/drivers/iio/adc/ad7606.c
+++ b/drivers/iio/adc/ad7606.c
@@ -236,9 +236,9 @@ static int ad7606_write_os_hw(struct iio_dev *indio_dev, int val)
 	struct ad7606_state *st = iio_priv(indio_dev);
 	DECLARE_BITMAP(values, 3);
 
-	values[0] = val;
+	values[0] = val & GENMASK(2, 0);
 
-	gpiod_set_array_value(ARRAY_SIZE(values), st->gpio_os->desc,
+	gpiod_set_array_value(st->gpio_os->ndescs, st->gpio_os->desc,
 			      st->gpio_os->info, values);
 
 	/* AD7616 requires a reset to update value */
diff --git a/drivers/iio/adc/ad7606_spi.c b/drivers/iio/adc/ad7606_spi.c
index 263a778bcf25..287a0591533b 100644
--- a/drivers/iio/adc/ad7606_spi.c
+++ b/drivers/iio/adc/ad7606_spi.c
@@ -249,8 +249,9 @@ static int ad7616_sw_mode_config(struct iio_dev *indio_dev)
 static int ad7606B_sw_mode_config(struct iio_dev *indio_dev)
 {
 	struct ad7606_state *st = iio_priv(indio_dev);
-	unsigned long os[3] = {1};
+	DECLARE_BITMAP(os, 3);
 
+	bitmap_fill(os, 3);
 	/*
 	 * Software mode is enabled when all three oversampling
 	 * pins are set to high. If oversampling gpios are defined
@@ -258,7 +259,7 @@ static int ad7606B_sw_mode_config(struct iio_dev *indio_dev)
 	 * otherwise, they must be hardwired to VDD
 	 */
 	if (st->gpio_os) {
-		gpiod_set_array_value(ARRAY_SIZE(os),
+		gpiod_set_array_value(st->gpio_os->ndescs,
 				      st->gpio_os->desc, st->gpio_os->info, os);
 	}
 	/* OS of 128 and 256 are available only in software mode */

-- 
2.34.1

[PATCH v2 09/10] iio: adc: ad7606: fix standby gpio state to match the documentation

[Guillaume Stols]

The binding's documentation specifies that "As the line is active low, it
should be marked GPIO_ACTIVE_LOW". However, in the driver, it was handled
the opposite way. This commit sets the driver's behaviour in sync with the
documentation

Fixes: 722407a4e8c0 ("staging:iio:ad7606: Use GPIO descriptor API")
Signed-off-by: Guillaume Stols <gstols@baylibre.com>
---
 drivers/iio/adc/ad7606.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
index 8cce1fad9763..50ccc245e314 100644
--- a/drivers/iio/adc/ad7606.c
+++ b/drivers/iio/adc/ad7606.c
@@ -443,7 +443,7 @@ static int ad7606_request_gpios(struct ad7606_state *st)
 		return PTR_ERR(st->gpio_range);
 
 	st->gpio_standby = devm_gpiod_get_optional(dev, "standby",
-						   GPIOD_OUT_HIGH);
+						   GPIOD_OUT_LOW);
 	if (IS_ERR(st->gpio_standby))
 		return PTR_ERR(st->gpio_standby);
 
@@ -686,7 +686,7 @@ static int ad7606_suspend(struct device *dev)
 
 	if (st->gpio_standby) {
 		gpiod_set_value(st->gpio_range, 1);
-		gpiod_set_value(st->gpio_standby, 0);
+		gpiod_set_value(st->gpio_standby, 1);
 	}
 
 	return 0;

-- 
2.34.1

[PATCH v2 10/10] iio: adc: ad7606: switch mutexes to scoped_guard

[Guillaume Stols]

Switching to scoped_guard simplifies the code and avoids to take care to
unlock the mutex in case of premature return.

Signed-off-by: Guillaume Stols <gstols@baylibre.com>
---
 drivers/iio/adc/ad7606.c | 60 ++++++++++++++++++++++--------------------------
 1 file changed, 27 insertions(+), 33 deletions(-)

diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
index 50ccc245e314..3c439787d130 100644
--- a/drivers/iio/adc/ad7606.c
+++ b/drivers/iio/adc/ad7606.c
@@ -69,19 +69,17 @@ static int ad7606_reg_access(struct iio_dev *indio_dev,
 	struct ad7606_state *st = iio_priv(indio_dev);
 	int ret;
 
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
+
 	if (readval) {
 		ret = st->bops->reg_read(st, reg);
 		if (ret < 0)
-			goto err_unlock;
+			return ret;
 		*readval = ret;
-		ret = 0;
+		return 0;
 	} else {
-		ret = st->bops->reg_write(st, reg, writeval);
+		return st->bops->reg_write(st, reg, writeval);
 	}
-err_unlock:
-	mutex_unlock(&st->lock);
-	return ret;
 }
 
 static int ad7606_read_samples(struct ad7606_state *st)
@@ -124,18 +122,18 @@ static irqreturn_t ad7606_trigger_handler(int irq, void *p)
 	struct ad7606_state *st = iio_priv(indio_dev);
 	int ret;
 
-	mutex_lock(&st->lock);
+	scoped_guard(mutex, &st->lock) {
+		ret = ad7606_read_samples(st);
+		if (ret)
+			goto error_ret;
 
-	ret = ad7606_read_samples(st);
-	if (ret == 0)
 		iio_push_to_buffers_with_timestamp(indio_dev, st->data,
 						   iio_get_time_ns(indio_dev));
-
-	iio_trigger_notify_done(indio_dev->trig);
-	/* The rising edge of the CONVST signal starts a new conversion. */
-	gpiod_set_value(st->gpio_convst, 1);
-
-	mutex_unlock(&st->lock);
+error_ret:
+		iio_trigger_notify_done(indio_dev->trig);
+		/* The rising edge of the CONVST signal starts a new conversion. */
+		gpiod_set_value(st->gpio_convst, 1);
+	}
 
 	return IRQ_HANDLED;
 }
@@ -259,17 +257,15 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
 
 	switch (mask) {
 	case IIO_CHAN_INFO_SCALE:
-		mutex_lock(&st->lock);
-		i = find_closest(val2, st->scale_avail, st->num_scales);
-		if (st->sw_mode_en)
-			ch = chan->address;
-		ret = st->write_scale(indio_dev, ch, i);
-		if (ret < 0) {
-			mutex_unlock(&st->lock);
-			return ret;
+		scoped_guard(mutex, &st->lock) {
+			i = find_closest(val2, st->scale_avail, st->num_scales);
+			if (st->sw_mode_en)
+				ch = chan->address;
+			ret = st->write_scale(indio_dev, ch, i);
+			if (ret < 0)
+				return ret;
+			st->range[ch] = i;
 		}
-		st->range[ch] = i;
-		mutex_unlock(&st->lock);
 
 		return 0;
 	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
@@ -277,14 +273,12 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
 			return -EINVAL;
 		i = find_closest(val, st->oversampling_avail,
 				 st->num_os_ratios);
-		mutex_lock(&st->lock);
-		ret = st->write_os(indio_dev, i);
-		if (ret < 0) {
-			mutex_unlock(&st->lock);
-			return ret;
+		scoped_guard(mutex, &st->lock) {
+			ret = st->write_os(indio_dev, i);
+			if (ret < 0)
+				return ret;
+			st->oversampling = st->oversampling_avail[i];
 		}
-		st->oversampling = st->oversampling_avail[i];
-		mutex_unlock(&st->lock);
 
 		return 0;
 	default:

-- 
2.34.1

Re: [PATCH v2 10/10] iio: adc: ad7606: switch mutexes to scoped_guard

[Jonathan Cameron]

On Fri, 28 Jun 2024 14:48:28 +0000
Guillaume Stols <gstols@baylibre.com> wrote:

> Switching to scoped_guard simplifies the code and avoids to take care to
> unlock the mutex in case of premature return.
> 
> Signed-off-by: Guillaume Stols <gstols@baylibre.com>
> ---
>  drivers/iio/adc/ad7606.c | 60 ++++++++++++++++++++++--------------------------
>  1 file changed, 27 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
> index 50ccc245e314..3c439787d130 100644
> --- a/drivers/iio/adc/ad7606.c
> +++ b/drivers/iio/adc/ad7606.c
> @@ -69,19 +69,17 @@ static int ad7606_reg_access(struct iio_dev *indio_dev,
>  	struct ad7606_state *st = iio_priv(indio_dev);
>  	int ret;
>  
> -	mutex_lock(&st->lock);
> +	guard(mutex)(&st->lock);
> +
>  	if (readval) {
>  		ret = st->bops->reg_read(st, reg);
>  		if (ret < 0)
> -			goto err_unlock;
> +			return ret;
>  		*readval = ret;
> -		ret = 0;
> +		return 0;
>  	} else {
> -		ret = st->bops->reg_write(st, reg, writeval);
> +		return st->bops->reg_write(st, reg, writeval);
>  	}
> -err_unlock:
> -	mutex_unlock(&st->lock);
> -	return ret;
>  }
>  
>  static int ad7606_read_samples(struct ad7606_state *st)
> @@ -124,18 +122,18 @@ static irqreturn_t ad7606_trigger_handler(int irq, void *p)
>  	struct ad7606_state *st = iio_priv(indio_dev);
>  	int ret;
>  
> -	mutex_lock(&st->lock);

Why not simply a guard(mutex)(&st->lock) ?

Then we avoid the somewhat nasty label in an nested block of code.

> +	scoped_guard(mutex, &st->lock) {
> +		ret = ad7606_read_samples(st);
> +		if (ret)
> +			goto error_ret;
>  
> -	ret = ad7606_read_samples(st);
> -	if (ret == 0)
>  		iio_push_to_buffers_with_timestamp(indio_dev, st->data,
>  						   iio_get_time_ns(indio_dev));
> -
> -	iio_trigger_notify_done(indio_dev->trig);
> -	/* The rising edge of the CONVST signal starts a new conversion. */
> -	gpiod_set_value(st->gpio_convst, 1);
> -
> -	mutex_unlock(&st->lock);
> +error_ret:
> +		iio_trigger_notify_done(indio_dev->trig);
> +		/* The rising edge of the CONVST signal starts a new conversion. */
> +		gpiod_set_value(st->gpio_convst, 1);
> +	}
>  
>  	return IRQ_HANDLED;
>  }
> @@ -259,17 +257,15 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_SCALE:
> -		mutex_lock(&st->lock);
> -		i = find_closest(val2, st->scale_avail, st->num_scales);
> -		if (st->sw_mode_en)
> -			ch = chan->address;
> -		ret = st->write_scale(indio_dev, ch, i);
> -		if (ret < 0) {
> -			mutex_unlock(&st->lock);
> -			return ret;
> +		scoped_guard(mutex, &st->lock) {

The mutex is grabbed in all paths that actually do anything.
Pull it out of the switch and use
guard(mutex)(&st->lock);

That will reduce the changes needed and give the same cleanups.
I doubt we care about potentially slowing down the path that returns an error
as we are writing something unwriteable.

> +			i = find_closest(val2, st->scale_avail, st->num_scales);
> +			if (st->sw_mode_en)
> +				ch = chan->address;
> +			ret = st->write_scale(indio_dev, ch, i);
> +			if (ret < 0)
> +				return ret;
> +			st->range[ch] = i;
>  		}
> -		st->range[ch] = i;
> -		mutex_unlock(&st->lock);
>  
>  		return 0;
>  	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> @@ -277,14 +273,12 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
>  			return -EINVAL;
>  		i = find_closest(val, st->oversampling_avail,
>  				 st->num_os_ratios);
> -		mutex_lock(&st->lock);
> -		ret = st->write_os(indio_dev, i);
> -		if (ret < 0) {
> -			mutex_unlock(&st->lock);
> -			return ret;
> +		scoped_guard(mutex, &st->lock) {
> +			ret = st->write_os(indio_dev, i);
> +			if (ret < 0)
> +				return ret;
> +			st->oversampling = st->oversampling_avail[i];
>  		}
> -		st->oversampling = st->oversampling_avail[i];
> -		mutex_unlock(&st->lock);
>  
>  		return 0;
>  	default:
> 

Re: [PATCH v2 00/10] iio: adc: ad7606: Improvements

[Conor Dooley]

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

On Fri, Jun 28, 2024 at 02:48:18PM +0000, Guillaume Stols wrote:
> This series adds the following improvements over the current AD7606's
> driver implementation:
> 
> - Fix wrong usage of gpio array
> - Fix standby that was documented as ACTIVE_LOW but handled in the
>   driver as if it was ACTIVE_HIGH
> - Improve dt-bindings documentation
> - Switch mutex lock to scoped guard
> 
> Signed-off-by: Guillaume Stols <gstols@baylibre.com>

You missed Acks from Rob on several patches that he gave yesterday:
https://lore.kernel.org/all/171952025424.477297.14698127361119381011.robh@kernel.org/


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

Re: [PATCH v2 00/10] iio: adc: ad7606: Improvements

[Conor Dooley]

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

On Fri, Jun 28, 2024 at 04:53:50PM +0100, Conor Dooley wrote:
> On Fri, Jun 28, 2024 at 02:48:18PM +0000, Guillaume Stols wrote:
> > This series adds the following improvements over the current AD7606's
> > driver implementation:
> > 
> > - Fix wrong usage of gpio array
> > - Fix standby that was documented as ACTIVE_LOW but handled in the
> >   driver as if it was ACTIVE_HIGH
> > - Improve dt-bindings documentation
> > - Switch mutex lock to scoped guard
> > 
> > Signed-off-by: Guillaume Stols <gstols@baylibre.com>
> 
> You missed Acks from Rob on several patches that he gave yesterday:
> https://lore.kernel.org/all/171952025424.477297.14698127361119381011.robh@kernel.org/

You also seem to be missing acks from me..


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

Re: [PATCH v2 00/10] iio: adc: ad7606: Improvements

[Jonathan Cameron]

On Fri, 28 Jun 2024 16:55:37 +0100
Conor Dooley <conor@kernel.org> wrote:

> On Fri, Jun 28, 2024 at 04:53:50PM +0100, Conor Dooley wrote:
> > On Fri, Jun 28, 2024 at 02:48:18PM +0000, Guillaume Stols wrote:  
> > > This series adds the following improvements over the current AD7606's
> > > driver implementation:
> > > 
> > > - Fix wrong usage of gpio array
> > > - Fix standby that was documented as ACTIVE_LOW but handled in the
> > >   driver as if it was ACTIVE_HIGH
> > > - Improve dt-bindings documentation
> > > - Switch mutex lock to scoped guard
> > > 
> > > Signed-off-by: Guillaume Stols <gstols@baylibre.com>  
> > 
> > You missed Acks from Rob on several patches that he gave yesterday:
> > https://lore.kernel.org/all/171952025424.477297.14698127361119381011.robh@kernel.org/  
> 
> You also seem to be missing acks from me..
> 

I picked up the first 2 with the acks scraped from v1.
There are enough minor changes that I've requested in the other patches
that I'd like a v3 fixing those.  Obviously make sure to gather up appropriate
acks.  You may want to wait a few days first though as there are a couple
of DT patches in here that need tags.

Jonathan