[PATCH v3 00/12] New features for the AD5686 IIO driver

[PATCH v3 00/12] New features for the AD5686 IIO driver

[Rodrigo Alencar via B4 Relay]

This is the second series of three on updating the AD5686 driver.

Initially, a big patch series was sent:
https://lore.kernel.org/r/20260422-ad5313r-iio-support-v1-0-ed7dca001d1b@analog.com

Then, the first patch series added fixes and cleanups:
https://lore.kernel.org/linux-iio/20260524-ad5686-fixes-v7-0-b6bf395d08bd@analog.com/

This one is introducing new features:
- Consume optional reset and correct power supplies;
- LDAC GPIO handling (active-low, held low when unused);
- SPI bus sync() implementation for batching multiple transfers;
- Triggered buffer support, leveraging LDAC and sync() to flush
  all channel writes atomically;
- Gain control support through the scale property.

Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
Changes in v3:
- Add range-double property.
- Acquire reset control after power-up delay.
- Include cleanup.h and use guard(mutex)() in read_raw().
- Link to v2: https://lore.kernel.org/r/20260609-ad5686-new-features-v2-0-70b423f5c76d@analog.com

Changes in v2:
- Get reset control deasserted.
- Update entire spi_transfer struct rather than individual fields.
- Replace udelay() for fsleep() in probe().
- Minor changes addressing further feedback.
- Link to v1: https://lore.kernel.org/r/20260602-ad5686-new-features-v1-0-691e01883d27@analog.com

---
Rodrigo Alencar (12):
      dt-bindings: iio: dac: ad5696: add reset/ldac/gain support
      dt-bindings: iio: dac: ad5696: rework on power supplies
      dt-bindings: iio: dac: ad5686: add reset/ldac/gain support
      dt-bindings: iio: dac: ad5686: rework on power supplies
      iio: dac: ad5686: add support for missing power supplies
      iio: dac: ad5686: consume optional reset signal
      iio: dac: ad5686: add ldac gpio
      iio: dac: ad5686: introduce sync operation
      iio: dac: ad5686: implement new sync() op for the spi bus
      iio: dac: ad5686: add triggered buffer support
      iio: dac: ad5686: read_raw/write_raw: use guard(mutex)()
      iio: dac: ad5686: add gain control support

 .../devicetree/bindings/iio/dac/adi,ad5686.yaml    |  72 +++++-
 .../devicetree/bindings/iio/dac/adi,ad5696.yaml    |  75 ++++++-
 drivers/iio/dac/Kconfig                            |   2 +
 drivers/iio/dac/ad5686-spi.c                       | 121 +++++++---
 drivers/iio/dac/ad5686.c                           | 249 +++++++++++++++++++--
 drivers/iio/dac/ad5686.h                           |  33 ++-
 drivers/iio/dac/ad5696-i2c.c                       |   2 +-
 7 files changed, 492 insertions(+), 62 deletions(-)
---
base-commit: ae696dfa47c30016cd429b9db5e70b259b8f509e
change-id: 20260602-ad5686-new-features-e116c04bddb9

Best regards,
-- 
Rodrigo Alencar <rodrigo.alencar@analog.com>

[PATCH v3 01/12] dt-bindings: iio: dac: ad5696: add reset/ldac/gain support

[Rodrigo Alencar via B4 Relay]

From: Rodrigo Alencar <rodrigo.alencar@analog.com>

Add GPIO property for RESET, LDAC and GAIN pin. RESET is active-low, LDAC
is used to load DAC channels with values from input registers and GAIN
can double the voltage in output channels. The gain-gpios property is
not available to all supported parts. The adi,range-double property
indicates that GAIN pin is hardwired to high in case gain-gpios is not
set, otherwise it sets the initial value for the gain setting.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
 .../devicetree/bindings/iio/dac/adi,ad5696.yaml    | 41 +++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5696.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5696.yaml
index b5a88b03dc2f..c55158c464fd 100644
--- a/Documentation/devicetree/bindings/iio/dac/adi,ad5696.yaml
+++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5696.yaml
@@ -37,14 +37,52 @@ properties:
     description: |
       The regulator supply for DAC reference voltage.
 
+  reset-gpios:
+    description: Active-low RESET pin to reset the device.
+    maxItems: 1
+
+  ldac-gpios:
+    description:
+      Active-low LDAC pin used to asynchronously update the DAC channels.
+    maxItems: 1
+
+  gain-gpios:
+    description:
+      GAIN pin that sets a multiplier for the DAC output voltage. When high,
+      the DAC output voltage is multiplied by 2, otherwise it is unchanged.
+    maxItems: 1
+
+  adi,range-double:
+    description:
+      Sets the initial voltage output range from 0 to 2xVREF. On devices that
+      have a GAIN pin and no gain-gpios property is set, this indicates the pin
+      is hardwired high.
+    type: boolean
+
 required:
   - compatible
   - reg
 
-additionalProperties: false
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            anyOf:
+              - const: adi,ad5311r
+              - const: adi,ad5691r
+              - const: adi,ad5692r
+              - const: adi,ad5693
+              - const: adi,ad5693r
+    then:
+      properties:
+        gain-gpios: false
+
+unevaluatedProperties: false
 
 examples:
   - |
+    #include <dt-bindings/gpio/gpio.h>
     i2c {
       #address-cells = <1>;
       #size-cells = <0>;
@@ -53,6 +91,7 @@ examples:
         compatible = "adi,ad5696";
         reg = <0>;
         vcc-supply = <&dac_vref>;
+        ldac-gpios = <&gpio0 1 GPIO_ACTIVE_LOW>;
       };
     };
 ...

-- 
2.43.0

[PATCH v3 02/12] dt-bindings: iio: dac: ad5696: rework on power supplies

[Rodrigo Alencar via B4 Relay]

From: Rodrigo Alencar <rodrigo.alencar@analog.com>

Add supplies for VDD, VLOGIC and VREF input voltage pins. The vcc-supply
property is deprecated, once it does not really exist as none of the
devices describe any power input with that name. VCC is also misleading as
it sounds like the input power supply, but it is being used as an external
voltage reference, which should be called VREF. Certain devices require
vref-supply to be available once an internal reference voltage is absent.
For correct operation vdd and vlogic supplies are required.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
 .../devicetree/bindings/iio/dac/adi,ad5696.yaml    | 34 ++++++++++++++++++++--
 1 file changed, 31 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5696.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5696.yaml
index c55158c464fd..7b936824917e 100644
--- a/Documentation/devicetree/bindings/iio/dac/adi,ad5696.yaml
+++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5696.yaml
@@ -33,9 +33,22 @@ properties:
   reg:
     maxItems: 1
 
+  vdd-supply:
+    description: Input power supply.
+
+  vlogic-supply:
+    description:
+      Digital power supply. On some tiny package variants for single-channel
+      devices, this supply is internally connected to vdd; in that case, specify
+      this property with the same regulator as vdd.
+
+  vref-supply:
+    description:
+      Reference voltage supply. If not supplied the internal reference is used.
+
   vcc-supply:
-    description: |
-      The regulator supply for DAC reference voltage.
+    deprecated: true
+    description: Use vref-supply instead.
 
   reset-gpios:
     description: Active-low RESET pin to reset the device.
@@ -62,8 +75,21 @@ properties:
 required:
   - compatible
   - reg
+  - vdd-supply
+  - vlogic-supply
 
 allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            anyOf:
+              - const: adi,ad5693
+              - const: adi,ad5694
+              - const: adi,ad5696
+    then:
+      required:
+        - vref-supply
   - if:
       properties:
         compatible:
@@ -90,7 +116,9 @@ examples:
       ad5696: dac@0 {
         compatible = "adi,ad5696";
         reg = <0>;
-        vcc-supply = <&dac_vref>;
+        vdd-supply = <&dac_vdd>;
+        vlogic-supply = <&dac_vlogic>;
+        vref-supply = <&dac_vref>;
         ldac-gpios = <&gpio0 1 GPIO_ACTIVE_LOW>;
       };
     };

-- 
2.43.0

[PATCH v3 03/12] dt-bindings: iio: dac: ad5686: add reset/ldac/gain support

[Rodrigo Alencar via B4 Relay]

From: Rodrigo Alencar <rodrigo.alencar@analog.com>

Add GPIO property for RESET, LDAC and GAIN pin. RESET is active-low, LDAC
is used to load DAC channels with values from input registers and GAIN
can double the voltage in output channels. The gain-gpios property is
not available to all supported parts. The adi,range-double property
indicates that GAIN pin is hardwired to high in case gain-gpios is not
set, otherwise it sets the initial value for the gain setting.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
 .../devicetree/bindings/iio/dac/adi,ad5686.yaml    | 38 ++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5686.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5686.yaml
index 713f535bb33a..7567de0ead4a 100644
--- a/Documentation/devicetree/bindings/iio/dac/adi,ad5686.yaml
+++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5686.yaml
@@ -35,17 +35,53 @@ properties:
   vcc-supply:
     description: If not supplied the internal reference is used.
 
+  reset-gpios:
+    description: Active-low RESET pin to reset the device.
+    maxItems: 1
+
+  ldac-gpios:
+    description:
+      Active-low LDAC pin used to asynchronously update the DAC channels.
+    maxItems: 1
+
+  gain-gpios:
+    description:
+      GAIN pin that sets a multiplier for the DAC output voltage. When high,
+      the DAC output voltage is multiplied by 2, otherwise it is unchanged.
+    maxItems: 1
+
+  adi,range-double:
+    description:
+      Sets the initial voltage output range from 0 to 2xVREF. On devices that
+      have a GAIN pin and no gain-gpios property is set, this indicates the pin
+      is hardwired high.
+    type: boolean
+
 required:
   - compatible
   - reg
 
 allOf:
   - $ref: /schemas/spi/spi-peripheral-props.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            anyOf:
+              - const: adi,ad5310r
+              - const: adi,ad5681r
+              - const: adi,ad5682r
+              - const: adi,ad5683
+              - const: adi,ad5683r
+    then:
+      properties:
+        gain-gpios: false
 
 unevaluatedProperties: false
 
 examples:
   - |
+    #include <dt-bindings/gpio/gpio.h>
     spi {
         #address-cells = <1>;
         #size-cells = <0>;
@@ -53,6 +89,8 @@ examples:
             reg = <0>;
             compatible = "adi,ad5310r";
             vcc-supply = <&dac_vref0>;
+            reset-gpios = <&gpio0 0 GPIO_ACTIVE_LOW>;
+            ldac-gpios = <&gpio0 1 GPIO_ACTIVE_LOW>;
         };
     };
 ...

-- 
2.43.0

[PATCH v3 04/12] dt-bindings: iio: dac: ad5686: rework on power supplies

[Rodrigo Alencar via B4 Relay]

From: Rodrigo Alencar <rodrigo.alencar@analog.com>

Add supplies for VDD, VLOGIC and VREF input voltage pins. The vcc-supply
property is deprecated, once it does not really exist as none of the
devices describe any power input with that name. VCC is also misleading as
it sounds like the input power supply, but it is being used as an external
voltage reference, which should be called VREF. Certain devices require
vref-supply to be available once an internal reference voltage is absent.
For correct operation vdd and vlogic supplies are required.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
 .../devicetree/bindings/iio/dac/adi,ad5686.yaml    | 34 ++++++++++++++++++++--
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5686.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5686.yaml
index 7567de0ead4a..bdc6bc425b48 100644
--- a/Documentation/devicetree/bindings/iio/dac/adi,ad5686.yaml
+++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5686.yaml
@@ -32,8 +32,22 @@ properties:
   reg:
     maxItems: 1
 
+  vdd-supply:
+    description: Input power supply.
+
+  vlogic-supply:
+    description:
+      Digital power supply. On some tiny package variants for single-channel
+      devices, this supply is internally connected to vdd; in that case, specify
+      this property with the same regulator as vdd.
+
+  vref-supply:
+    description:
+      Reference voltage supply. If not supplied the internal reference is used.
+
   vcc-supply:
-    description: If not supplied the internal reference is used.
+    deprecated: true
+    description: Use vref-supply instead.
 
   reset-gpios:
     description: Active-low RESET pin to reset the device.
@@ -60,9 +74,23 @@ properties:
 required:
   - compatible
   - reg
+  - vdd-supply
+  - vlogic-supply
 
 allOf:
   - $ref: /schemas/spi/spi-peripheral-props.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            anyOf:
+              - const: adi,ad5676
+              - const: adi,ad5683
+              - const: adi,ad5684
+              - const: adi,ad5686
+    then:
+      required:
+        - vref-supply
   - if:
       properties:
         compatible:
@@ -88,7 +116,9 @@ examples:
         dac@0 {
             reg = <0>;
             compatible = "adi,ad5310r";
-            vcc-supply = <&dac_vref0>;
+            vdd-supply = <&dac_vdd>;
+            vlogic-supply = <&dac_vlogic>;
+            vref-supply = <&dac_vref>;
             reset-gpios = <&gpio0 0 GPIO_ACTIVE_LOW>;
             ldac-gpios = <&gpio0 1 GPIO_ACTIVE_LOW>;
         };

-- 
2.43.0

[PATCH v3 05/12] iio: dac: ad5686: add support for missing power supplies

[Rodrigo Alencar via B4 Relay]

From: Rodrigo Alencar <rodrigo.alencar@analog.com>

Get and enable regulators for vdd, vlogic and vref input power pins. Vdd
is the input power supply, while vlogic powers the digital side. vref is
replacing vcc, which is being deprecated, but still supported. The value
of vref_mv is checked so that a device without internal voltage reference
cannot proceed without an explicit supply. For correct operation, vdd and
vlogic are required, then devm_regulator_get_enable() is used so the
driver can still work without them by using the stub/dummy regulators.
Error report uses dev_err_probe(), which helps debugging an init issue.
A small delay is added after the regulators are enabled to consider for
the power-up time (4.5 us).

Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
 drivers/iio/dac/ad5686.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
index 5840fda4b011..fc3863274b29 100644
--- a/drivers/iio/dac/ad5686.c
+++ b/drivers/iio/dac/ad5686.c
@@ -8,6 +8,8 @@
 #include <linux/array_size.h>
 #include <linux/bitfield.h>
 #include <linux/bitops.h>
+#include <linux/delay.h>
+#include <linux/dev_printk.h>
 #include <linux/errno.h>
 #include <linux/export.h>
 #include <linux/kstrtox.h>
@@ -484,12 +486,27 @@ int ad5686_probe(struct device *dev,
 	st->ops = ops;
 	st->chip_info = chip_info;
 
-	ret = devm_regulator_get_enable_read_voltage(dev, "vcc");
+	ret = devm_regulator_get_enable(dev, "vdd");
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to enable vdd supply\n");
+
+	ret = devm_regulator_get_enable(dev, "vlogic");
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to enable vlogic supply\n");
+
+	ret = devm_regulator_get_enable_read_voltage(dev, "vref");
+	if (ret == -ENODEV) /* vcc-supply is deprecated, but supported still */
+		ret = devm_regulator_get_enable_read_voltage(dev, "vcc");
 	if (ret < 0 && ret != -ENODEV)
-		return ret;
+		return dev_err_probe(dev, ret, "failed to read vref voltage\n");
 
 	st->use_internal_vref = ret == -ENODEV;
 	st->vref_mv = st->use_internal_vref ? st->chip_info->int_vref_mv : ret / 1000;
+	if (!st->vref_mv)
+		return dev_err_probe(dev, -EINVAL,
+				     "invalid or not provided vref voltage\n");
+
+	fsleep(5); /* power-up time */
 
 	/* Initialize masks to all ones */
 	st->pwr_down_mask = ~0;

-- 
2.43.0

[PATCH v3 06/12] iio: dac: ad5686: consume optional reset signal

[Rodrigo Alencar via B4 Relay]

From: Rodrigo Alencar <rodrigo.alencar@analog.com>

Add RESET pin GPIO support through an optional reset control, which is
local to the probe function. A reset pulse is manually generated after
the device is powered up.

Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
 drivers/iio/dac/ad5686.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
index fc3863274b29..a48acedff51c 100644
--- a/drivers/iio/dac/ad5686.c
+++ b/drivers/iio/dac/ad5686.c
@@ -15,6 +15,7 @@
 #include <linux/kstrtox.h>
 #include <linux/module.h>
 #include <linux/regulator/consumer.h>
+#include <linux/reset.h>
 #include <linux/sysfs.h>
 #include <linux/wordpart.h>
 
@@ -472,6 +473,7 @@ int ad5686_probe(struct device *dev,
 		 const struct ad5686_chip_info *chip_info,
 		 const char *name, const struct ad5686_bus_ops *ops)
 {
+	struct reset_control *rstc;
 	struct ad5686_state *st;
 	struct iio_dev *indio_dev;
 	int ret, i;
@@ -508,6 +510,15 @@ int ad5686_probe(struct device *dev,
 
 	fsleep(5); /* power-up time */
 
+	rstc = devm_reset_control_get_optional_exclusive(dev, NULL);
+	if (IS_ERR(rstc))
+		return dev_err_probe(dev, PTR_ERR(rstc),
+				     "Failed to get reset control\n");
+
+	reset_control_assert(rstc);
+	fsleep(1); /* reset pulse: comfortably bigger than the spec */
+	reset_control_deassert(rstc);
+
 	/* Initialize masks to all ones */
 	st->pwr_down_mask = ~0;
 	st->pwr_down_mode = ~0;

-- 
2.43.0

[PATCH v3 07/12] iio: dac: ad5686: add ldac gpio

[Rodrigo Alencar via B4 Relay]

From: Rodrigo Alencar <rodrigo.alencar@analog.com>

If wired LDAC, should be held low when unused (pin is active-low), which
allows for synchronous DAC updates. This will be used to update all the
channels at the same time when adding buffer support.

Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
 drivers/iio/dac/ad5686.c | 6 ++++++
 drivers/iio/dac/ad5686.h | 3 +++
 2 files changed, 9 insertions(+)

diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
index a48acedff51c..73105c197964 100644
--- a/drivers/iio/dac/ad5686.c
+++ b/drivers/iio/dac/ad5686.c
@@ -12,6 +12,7 @@
 #include <linux/dev_printk.h>
 #include <linux/errno.h>
 #include <linux/export.h>
+#include <linux/gpio/consumer.h>
 #include <linux/kstrtox.h>
 #include <linux/module.h>
 #include <linux/regulator/consumer.h>
@@ -515,6 +516,11 @@ int ad5686_probe(struct device *dev,
 		return dev_err_probe(dev, PTR_ERR(rstc),
 				     "Failed to get reset control\n");
 
+	st->ldac_gpio = devm_gpiod_get_optional(dev, "ldac", GPIOD_OUT_HIGH);
+	if (IS_ERR(st->ldac_gpio))
+		return dev_err_probe(dev, PTR_ERR(st->ldac_gpio),
+				     "Failed to get LDAC GPIO\n");
+
 	reset_control_assert(rstc);
 	fsleep(1); /* reset pulse: comfortably bigger than the spec */
 	reset_control_deassert(rstc);
diff --git a/drivers/iio/dac/ad5686.h b/drivers/iio/dac/ad5686.h
index a06fe7d89305..f3110410d89b 100644
--- a/drivers/iio/dac/ad5686.h
+++ b/drivers/iio/dac/ad5686.h
@@ -60,6 +60,7 @@ enum ad5686_regmap_type {
 	AD5686_REGMAP,
 };
 
+struct gpio_desc;
 struct ad5686_state;
 
 /**
@@ -119,6 +120,7 @@ extern const struct ad5686_chip_info ad5679r_chip_info;
  * @dev:		device instance
  * @chip_info:		chip model specific constants, available modes etc
  * @ops:		bus specific operations
+ * @ldac_gpio:		LDAC pin GPIO descriptor
  * @vref_mv:		actual reference voltage used
  * @pwr_down_mask:	power down mask
  * @pwr_down_mode:	current power down mode
@@ -131,6 +133,7 @@ struct ad5686_state {
 	struct device			*dev;
 	const struct ad5686_chip_info	*chip_info;
 	const struct ad5686_bus_ops	*ops;
+	struct gpio_desc		*ldac_gpio;
 	unsigned short			vref_mv;
 	unsigned int			pwr_down_mask;
 	unsigned int			pwr_down_mode;

-- 
2.43.0

[PATCH v3 08/12] iio: dac: ad5686: introduce sync operation

[Rodrigo Alencar via B4 Relay]

From: Rodrigo Alencar <rodrigo.alencar@analog.com>

Add sync() to operation to ad5686_bus_ops, which can be used to flush
multiple pending data transfers at once. This is going to be used when
implementing triggered buffer support.

Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
 drivers/iio/dac/ad5686.h | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/dac/ad5686.h b/drivers/iio/dac/ad5686.h
index f3110410d89b..1da5f1b4d52a 100644
--- a/drivers/iio/dac/ad5686.h
+++ b/drivers/iio/dac/ad5686.h
@@ -67,10 +67,12 @@ struct ad5686_state;
  * struct ad5686_bus_ops - bus specific read/write operations
  * @read: read a register value at the given address
  * @write: write a command, address and value to the device
+ * @sync: ensure the completion of the write operation (optional)
  */
 struct ad5686_bus_ops {
 	int (*read)(struct ad5686_state *st, u8 addr);
 	int (*write)(struct ad5686_state *st, u8 cmd, u8 addr, u16 val);
+	int (*sync)(struct ad5686_state *st);
 };
 
 /**
@@ -159,7 +161,13 @@ int ad5686_probe(struct device *dev,
 
 static inline int ad5686_write(struct ad5686_state *st, u8 cmd, u8 addr, u16 val)
 {
-	return st->ops->write(st, cmd, addr, val);
+	int ret;
+
+	ret = st->ops->write(st, cmd, addr, val);
+	if (ret)
+		return ret;
+
+	return st->ops->sync ? st->ops->sync(st) : 0;
 }
 
 static inline int ad5686_read(struct ad5686_state *st, u8 addr)

-- 
2.43.0

[PATCH v3 09/12] iio: dac: ad5686: implement new sync() op for the spi bus

[Rodrigo Alencar via B4 Relay]

From: Rodrigo Alencar <rodrigo.alencar@analog.com>

Use of local SPI bus data to manage a collection of SPI transfers and
flush them to the SPI platform driver with the sync() operation. This
allows for faster handling of multiple channel DAC writes, avoiding kernel
overhead per spi_sync() call, which will be helpful when enabling
triggered buffer support.

Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
 drivers/iio/dac/ad5686-spi.c | 121 ++++++++++++++++++++++++++++++++-----------
 drivers/iio/dac/ad5686.c     |   4 +-
 drivers/iio/dac/ad5686.h     |   8 ++-
 drivers/iio/dac/ad5696-i2c.c |   2 +-
 4 files changed, 100 insertions(+), 35 deletions(-)

diff --git a/drivers/iio/dac/ad5686-spi.c b/drivers/iio/dac/ad5686-spi.c
index 6b6ef1d7071f..77c86674d178 100644
--- a/drivers/iio/dac/ad5686-spi.c
+++ b/drivers/iio/dac/ad5686-spi.c
@@ -12,59 +12,91 @@
 #include <linux/errno.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
+#include <linux/overflow.h>
 #include <linux/spi/spi.h>
 
 #include <asm/byteorder.h>
 
 #include "ad5686.h"
 
+/**
+ * struct ad5686_spi_data - SPI bus specific data
+ * @msg: SPI message used for transfers
+ * @size: number of transfers currently in the message
+ * @capacity: maximum number of transfers that can be added to the message
+ * @xfers: array of SPI transfers, allocated with the provided capacity
+ */
+struct ad5686_spi_data {
+	struct spi_message msg;
+	unsigned int size;
+	unsigned int capacity;
+	struct spi_transfer xfers[] __counted_by(capacity);
+};
+
 static int ad5686_spi_write(struct ad5686_state *st,
 			    u8 cmd, u8 addr, u16 val)
 {
-	struct spi_device *spi = to_spi_device(st->dev);
-	u8 tx_len, *buf;
+	struct ad5686_spi_data *bus_data = st->bus_data;
+	struct spi_transfer *xfer;
 
+	if (bus_data->size >= bus_data->capacity)
+		return -E2BIG;
+
+	if (bus_data->size)
+		bus_data->xfers[bus_data->size - 1].cs_change = 1;
+	else
+		spi_message_init(&bus_data->msg);
+
+	xfer = &bus_data->xfers[bus_data->size];
 	switch (st->chip_info->regmap_type) {
 	case AD5310_REGMAP:
-		st->data[0].d16 = cpu_to_be16(AD5310_CMD(cmd) |
-					      val);
-		buf = &st->data[0].d8[0];
-		tx_len = 2;
+		st->data[bus_data->size].d16 =
+			cpu_to_be16(AD5310_CMD(cmd) | val);
+		*xfer = (struct spi_transfer) {
+			.tx_buf = &st->data[bus_data->size].d16,
+			.len = sizeof(st->data[bus_data->size].d16),
+		};
 		break;
 	case AD5683_REGMAP:
-		st->data[0].d32 = cpu_to_be32(AD5686_CMD(cmd) |
-					      AD5683_DATA(val));
-		buf = &st->data[0].d8[1];
-		tx_len = 3;
+		st->data[bus_data->size].d32 =
+			cpu_to_be32(AD5686_CMD(cmd) | AD5683_DATA(val));
+		*xfer = (struct spi_transfer) {
+			.tx_buf = &st->data[bus_data->size].d8[1],
+			.len = sizeof(st->data[bus_data->size].d32) - 1,
+		};
 		break;
 	case AD5686_REGMAP:
-		st->data[0].d32 = cpu_to_be32(AD5686_CMD(cmd) |
-					      AD5686_ADDR(addr) |
-					      val);
-		buf = &st->data[0].d8[1];
-		tx_len = 3;
+		st->data[bus_data->size].d32 =
+			cpu_to_be32(AD5686_CMD(cmd) | AD5686_ADDR(addr) | val);
+		*xfer = (struct spi_transfer) {
+			.tx_buf = &st->data[bus_data->size].d8[1],
+			.len = sizeof(st->data[bus_data->size].d32) - 1,
+		};
 		break;
 	default:
 		return -EINVAL;
 	}
 
-	return spi_write(spi, buf, tx_len);
+	spi_message_add_tail(xfer, &bus_data->msg);
+	bus_data->size++;
+
+	return 0;
+}
+
+static int ad5686_spi_sync(struct ad5686_state *st)
+{
+	struct spi_device *spi = to_spi_device(st->dev);
+	struct ad5686_spi_data *bus_data = st->bus_data;
+
+	bus_data->size = 0; /* always reset, even on sync failure */
+	return spi_sync(spi, &bus_data->msg);
 }
 
 static int ad5686_spi_read(struct ad5686_state *st, u8 addr)
 {
-	struct spi_transfer t[] = {
-		{
-			.tx_buf = &st->data[0].d8[1],
-			.len = 3,
-			.cs_change = 1,
-		}, {
-			.tx_buf = &st->data[1].d8[1],
-			.rx_buf = &st->data[2].d8[1],
-			.len = 3,
-		},
-	};
 	struct spi_device *spi = to_spi_device(st->dev);
+	struct ad5686_spi_data *bus_data = st->bus_data;
+	struct spi_transfer *xfer = &bus_data->xfers[0];
 	u8 cmd = 0;
 	int ret;
 
@@ -85,8 +117,21 @@ static int ad5686_spi_read(struct ad5686_state *st, u8 addr)
 				      AD5686_ADDR(addr));
 	st->data[1].d32 = cpu_to_be32(AD5686_CMD(AD5686_CMD_NOOP));
 
-	ret = spi_sync_transfer(spi, t, ARRAY_SIZE(t));
-	if (ret < 0)
+	xfer[0] = (struct spi_transfer) {
+		.tx_buf = &st->data[0].d8[1],
+		.len = sizeof(st->data[0].d32) - 1,
+		.cs_change = 1,
+	};
+	xfer[1] = (struct spi_transfer) {
+		.tx_buf = &st->data[1].d8[1],
+		.rx_buf = &st->data[2].d8[1],
+		.len = sizeof(st->data[1].d32) - 1,
+	};
+
+	spi_message_init_with_transfers(&bus_data->msg, xfer, 2);
+
+	ret = spi_sync(spi, &bus_data->msg);
+	if (ret)
 		return ret;
 
 	return be32_to_cpu(st->data[2].d32);
@@ -95,12 +140,26 @@ static int ad5686_spi_read(struct ad5686_state *st, u8 addr)
 static const struct ad5686_bus_ops ad5686_spi_ops = {
 	.write = ad5686_spi_write,
 	.read = ad5686_spi_read,
+	.sync = ad5686_spi_sync,
 };
 
 static int ad5686_spi_probe(struct spi_device *spi)
 {
-	return ad5686_probe(&spi->dev, spi_get_device_match_data(spi),
-			    spi->modalias, &ad5686_spi_ops);
+	const struct ad5686_chip_info *info = spi_get_device_match_data(spi);
+	struct ad5686_spi_data *bus_data;
+	struct device *dev = &spi->dev;
+	unsigned int capacity;
+
+	/* read operation requires at least 2 transfers */
+	capacity = max(info->num_channels, 2);
+	bus_data = devm_kzalloc(dev, struct_size(bus_data, xfers, capacity),
+				GFP_KERNEL);
+	if (!bus_data)
+		return -ENOMEM;
+
+	bus_data->capacity = capacity;
+
+	return ad5686_probe(dev, info, spi->modalias, &ad5686_spi_ops, bus_data);
 }
 
 static const struct spi_device_id ad5686_spi_id[] = {
diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
index 73105c197964..34011cbef903 100644
--- a/drivers/iio/dac/ad5686.c
+++ b/drivers/iio/dac/ad5686.c
@@ -472,7 +472,8 @@ EXPORT_SYMBOL_NS_GPL(ad5679r_chip_info, "IIO_AD5686");
 
 int ad5686_probe(struct device *dev,
 		 const struct ad5686_chip_info *chip_info,
-		 const char *name, const struct ad5686_bus_ops *ops)
+		 const char *name, const struct ad5686_bus_ops *ops,
+		 void *bus_data)
 {
 	struct reset_control *rstc;
 	struct ad5686_state *st;
@@ -487,6 +488,7 @@ int ad5686_probe(struct device *dev,
 
 	st->dev = dev;
 	st->ops = ops;
+	st->bus_data = bus_data;
 	st->chip_info = chip_info;
 
 	ret = devm_regulator_get_enable(dev, "vdd");
diff --git a/drivers/iio/dac/ad5686.h b/drivers/iio/dac/ad5686.h
index 1da5f1b4d52a..6f47493906d4 100644
--- a/drivers/iio/dac/ad5686.h
+++ b/drivers/iio/dac/ad5686.h
@@ -23,6 +23,7 @@
 
 #define AD5686_ADDR_DAC(chan)			(0x1 << (chan))
 #define AD5686_ADDR_ALL_DAC			0xF
+#define AD5686_MAX_CHANNELS			16
 
 #define AD5686_CMD_NOOP				0x0
 #define AD5686_CMD_WRITE_INPUT_N		0x1
@@ -129,6 +130,7 @@ extern const struct ad5686_chip_info ad5679r_chip_info;
  * @use_internal_vref:	set to true if the internal reference voltage is used
  * @lock:		lock to protect access to state fields, which includes
  *			the data buffer during regmap ops
+ * @bus_data:		bus specific data
  * @data:		transfer buffers
  */
 struct ad5686_state {
@@ -141,6 +143,7 @@ struct ad5686_state {
 	unsigned int			pwr_down_mode;
 	bool				use_internal_vref;
 	struct mutex			lock;
+	void				*bus_data;
 
 	/*
 	 * DMA (thus cache coherency maintenance) may require the
@@ -151,13 +154,14 @@ struct ad5686_state {
 		__be32 d32;
 		__be16 d16;
 		u8 d8[4];
-	} data[3] __aligned(IIO_DMA_MINALIGN);
+	} data[AD5686_MAX_CHANNELS] __aligned(IIO_DMA_MINALIGN);
 };
 
 
 int ad5686_probe(struct device *dev,
 		 const struct ad5686_chip_info *chip_info,
-		 const char *name, const struct ad5686_bus_ops *ops);
+		 const char *name, const struct ad5686_bus_ops *ops,
+		 void *bus_data);
 
 static inline int ad5686_write(struct ad5686_state *st, u8 cmd, u8 addr, u16 val)
 {
diff --git a/drivers/iio/dac/ad5696-i2c.c b/drivers/iio/dac/ad5696-i2c.c
index 279309329b64..28c97ded43ce 100644
--- a/drivers/iio/dac/ad5696-i2c.c
+++ b/drivers/iio/dac/ad5696-i2c.c
@@ -70,7 +70,7 @@ static const struct ad5686_bus_ops ad5686_i2c_ops = {
 static int ad5686_i2c_probe(struct i2c_client *i2c)
 {
 	return ad5686_probe(&i2c->dev, i2c_get_match_data(i2c),
-			    i2c->name, &ad5686_i2c_ops);
+			    i2c->name, &ad5686_i2c_ops, NULL);
 }
 
 static const struct i2c_device_id ad5686_i2c_id[] = {

-- 
2.43.0

[PATCH v3 10/12] iio: dac: ad5686: add triggered buffer support

[Rodrigo Alencar via B4 Relay]

From: Rodrigo Alencar <rodrigo.alencar@analog.com>

Implement trigger handler by leveraging the LDAC gpio to update all DAC
channels at once when it is available. Also, the multiple channel writes
can be flushed at once with the sync() operation.

Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
 drivers/iio/dac/Kconfig  |  2 ++
 drivers/iio/dac/ad5686.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 68 insertions(+)

diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index 657c68e75542..5f14fcd780e2 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -240,6 +240,8 @@ config LTC2688
 
 config AD5686
 	tristate
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
 
 config AD5686_SPI
 	tristate "Analog Devices AD5686 and similar multi-channel DACs (SPI)"
diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
index 34011cbef903..9e8a014e5ac3 100644
--- a/drivers/iio/dac/ad5686.c
+++ b/drivers/iio/dac/ad5686.c
@@ -20,7 +20,11 @@
 #include <linux/sysfs.h>
 #include <linux/wordpart.h>
 
+#include <linux/iio/buffer.h>
 #include <linux/iio/iio.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
 
 #include "ad5686.h"
 
@@ -246,6 +250,7 @@ static const struct iio_chan_spec_ext_info ad5686_ext_info[] = {
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),	\
 		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),\
 		.address = addr,				\
+		.scan_index = chan,				\
 		.scan_type = {					\
 			.sign = 'u',				\
 			.realbits = (bits),			\
@@ -470,6 +475,60 @@ const struct ad5686_chip_info ad5679r_chip_info = {
 };
 EXPORT_SYMBOL_NS_GPL(ad5679r_chip_info, "IIO_AD5686");
 
+static irqreturn_t ad5686_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct iio_buffer *buffer = indio_dev->buffer;
+	struct ad5686_state *st = iio_priv(indio_dev);
+	u16 val[AD5686_MAX_CHANNELS] = { };
+	unsigned int scan_count;
+	int ret, ch, i = 0;
+	bool async_update;
+	u8 cmd;
+
+	ret = iio_pop_from_buffer(buffer, val);
+	if (ret)
+		goto out_notify_done;
+
+	mutex_lock(&st->lock);
+
+	scan_count = bitmap_weight(indio_dev->active_scan_mask,
+				   iio_get_masklength(indio_dev));
+	async_update = st->ldac_gpio && scan_count > 1;
+	if (async_update) {
+		/* use LDAC to update all channels simultaneously */
+		cmd = AD5686_CMD_WRITE_INPUT_N;
+		gpiod_set_value_cansleep(st->ldac_gpio, 0);
+	} else {
+		cmd = AD5686_CMD_WRITE_INPUT_N_UPDATE_N;
+	}
+
+	iio_for_each_active_channel(indio_dev, ch) {
+		ret = st->ops->write(st, cmd, indio_dev->channels[ch].address, val[i++]);
+		if (ret)
+			break;
+	}
+
+	/*
+	 * If sync() is available, it is called here regardless of write
+	 * failure to allow bus implementation to reset. In that case, partial
+	 * writes are unlikely as the write operations would just queue up
+	 * the transfers.
+	 */
+	if (st->ops->sync)
+		ret = st->ops->sync(st); /* flush all pending transfers */
+
+	if (async_update)
+		gpiod_set_value_cansleep(st->ldac_gpio, 1);
+
+	mutex_unlock(&st->lock);
+out_notify_done:
+	iio_trigger_notify_done(indio_dev->trig);
+
+	return IRQ_HANDLED;
+}
+
 int ad5686_probe(struct device *dev,
 		 const struct ad5686_chip_info *chip_info,
 		 const char *name, const struct ad5686_bus_ops *ops,
@@ -570,6 +629,13 @@ int ad5686_probe(struct device *dev,
 		return -EINVAL;
 	}
 
+	ret = devm_iio_triggered_buffer_setup_ext(dev, indio_dev, NULL,
+						  &ad5686_trigger_handler,
+						  IIO_BUFFER_DIRECTION_OUT,
+						  NULL, NULL);
+	if (ret)
+		return ret;
+
 	return devm_iio_device_register(dev, indio_dev);
 }
 EXPORT_SYMBOL_NS_GPL(ad5686_probe, "IIO_AD5686");

-- 
2.43.0

[PATCH v3 11/12] iio: dac: ad5686: read_raw/write_raw: use guard(mutex)()

[Rodrigo Alencar via B4 Relay]

From: Rodrigo Alencar <rodrigo.alencar@analog.com>

Use guarded mutex lock to facilitate code review when adding new
attributes. This will allow for early returns, avoiding error-prone
locking and unlocking in error paths. This also adds missing include
linux/cleanup.h. Gain-control support will allow the scale attribute
to be configurable.

Reviewed-by: Maxwell Doose <m32285159@gmail.com>
Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
 drivers/iio/dac/ad5686.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
index 9e8a014e5ac3..6ae788f665b4 100644
--- a/drivers/iio/dac/ad5686.c
+++ b/drivers/iio/dac/ad5686.c
@@ -8,6 +8,7 @@
 #include <linux/array_size.h>
 #include <linux/bitfield.h>
 #include <linux/bitops.h>
+#include <linux/cleanup.h>
 #include <linux/delay.h>
 #include <linux/dev_printk.h>
 #include <linux/errno.h>
@@ -181,11 +182,11 @@ static int ad5686_read_raw(struct iio_dev *indio_dev,
 	struct ad5686_state *st = iio_priv(indio_dev);
 	int ret;
 
+	guard(mutex)(&st->lock);
+
 	switch (m) {
 	case IIO_CHAN_INFO_RAW:
-		mutex_lock(&st->lock);
 		ret = ad5686_read(st, chan->address);
-		mutex_unlock(&st->lock);
 		if (ret < 0)
 			return ret;
 		*val = (ret >> chan->scan_type.shift) &
@@ -206,23 +207,19 @@ static int ad5686_write_raw(struct iio_dev *indio_dev,
 			    long mask)
 {
 	struct ad5686_state *st = iio_priv(indio_dev);
-	int ret;
+
+	guard(mutex)(&st->lock);
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
 		if (val >= (1 << chan->scan_type.realbits) || val < 0)
 			return -EINVAL;
 
-		mutex_lock(&st->lock);
-		ret = ad5686_write(st, AD5686_CMD_WRITE_INPUT_N_UPDATE_N,
-				   chan->address, val << chan->scan_type.shift);
-		mutex_unlock(&st->lock);
-		break;
+		return ad5686_write(st, AD5686_CMD_WRITE_INPUT_N_UPDATE_N,
+				    chan->address, val << chan->scan_type.shift);
 	default:
-		ret = -EINVAL;
+		return -EINVAL;
 	}
-
-	return ret;
 }
 
 static const struct iio_info ad5686_info = {

-- 
2.43.0

[PATCH v3 12/12] iio: dac: ad5686: add gain control support

[Rodrigo Alencar via B4 Relay]

From: Rodrigo Alencar <rodrigo.alencar@analog.com>

Most of the supported devices rely on a GAIN pin to control a 2x
multiplier applied to the output voltage. Other devices, e.g. the
single-channel ones, provides a gain control through a bit field in
the control register. Some designs might have the GAIN pin hardwired
to VDD/VLOGIC or GND, which would have no "gain-gpios" device property,
being able to set "adi,range-double" if it is hardwired to VDD. The
vref_mv field is moved down in the struct ad5686_state, so that the
overall size increase is reduced.

Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
 drivers/iio/dac/ad5686.c | 122 +++++++++++++++++++++++++++++++++++++++++++++--
 drivers/iio/dac/ad5686.h |  12 ++++-
 2 files changed, 127 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
index 6ae788f665b4..c186213a46f6 100644
--- a/drivers/iio/dac/ad5686.c
+++ b/drivers/iio/dac/ad5686.c
@@ -15,10 +15,13 @@
 #include <linux/export.h>
 #include <linux/gpio/consumer.h>
 #include <linux/kstrtox.h>
+#include <linux/math64.h>
 #include <linux/module.h>
+#include <linux/property.h>
 #include <linux/regulator/consumer.h>
 #include <linux/reset.h>
 #include <linux/sysfs.h>
+#include <linux/units.h>
 #include <linux/wordpart.h>
 
 #include <linux/iio/buffer.h>
@@ -41,7 +44,8 @@ static int ad5310_control_sync(struct ad5686_state *st)
 
 	return ad5686_write(st, AD5686_CMD_CONTROL_REG, 0,
 			    FIELD_PREP(AD5310_PD_MSK, pd_val & AD5686_PD_MSK) |
-			    FIELD_PREP(AD5310_REF_BIT_MSK, st->use_internal_vref ? 0 : 1));
+			    FIELD_PREP(AD5310_REF_BIT_MSK, st->use_internal_vref ? 0 : 1) |
+			    FIELD_PREP(AD5310_GAIN_BIT_MSK, st->double_scale ? 1 : 0));
 }
 
 static int ad5683_control_sync(struct ad5686_state *st)
@@ -50,7 +54,8 @@ static int ad5683_control_sync(struct ad5686_state *st)
 
 	return ad5686_write(st, AD5686_CMD_CONTROL_REG, 0,
 			    FIELD_PREP(AD5683_PD_MSK, pd_val & AD5686_PD_MSK) |
-			    FIELD_PREP(AD5683_REF_BIT_MSK, st->use_internal_vref ? 0 : 1));
+			    FIELD_PREP(AD5683_REF_BIT_MSK, st->use_internal_vref ? 0 : 1) |
+			    FIELD_PREP(AD5683_GAIN_BIT_MSK, st->double_scale ? 1 : 0));
 }
 
 static inline unsigned int ad5686_pd_mask_shift(const struct iio_chan_spec *chan)
@@ -193,9 +198,14 @@ static int ad5686_read_raw(struct iio_dev *indio_dev,
 			GENMASK(chan->scan_type.realbits - 1, 0);
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
-		*val = st->vref_mv;
-		*val2 = chan->scan_type.realbits;
-		return IIO_VAL_FRACTIONAL_LOG2;
+		if (st->double_scale) {
+			*val = st->scale_avail[2];
+			*val2 = st->scale_avail[3];
+		} else {
+			*val = st->scale_avail[0];
+			*val2 = st->scale_avail[1];
+		}
+		return IIO_VAL_INT_PLUS_NANO;
 	}
 	return -EINVAL;
 }
@@ -207,6 +217,8 @@ static int ad5686_write_raw(struct iio_dev *indio_dev,
 			    long mask)
 {
 	struct ad5686_state *st = iio_priv(indio_dev);
+	bool double_scale;
+	int ret;
 
 	guard(mutex)(&st->lock);
 
@@ -217,6 +229,84 @@ static int ad5686_write_raw(struct iio_dev *indio_dev,
 
 		return ad5686_write(st, AD5686_CMD_WRITE_INPUT_N_UPDATE_N,
 				    chan->address, val << chan->scan_type.shift);
+	case IIO_CHAN_INFO_SCALE:
+		if (val == st->scale_avail[0] && val2 == st->scale_avail[1])
+			double_scale = false;
+		else if (val == st->scale_avail[2] && val2 == st->scale_avail[3])
+			double_scale = true;
+		else
+			return -EINVAL;
+
+		if (st->double_scale == double_scale)
+			return 0; /* no change */
+
+		st->double_scale = double_scale;
+		switch (st->chip_info->regmap_type) {
+		case AD5310_REGMAP:
+			ret = ad5310_control_sync(st);
+			break;
+		case AD5683_REGMAP:
+			ret = ad5683_control_sync(st);
+			break;
+		case AD5686_REGMAP:
+			if (!st->gain_gpio) {
+				ret = -EINVAL;
+				break;
+			}
+
+			ret = gpiod_set_value_cansleep(st->gain_gpio,
+						       st->double_scale ? 1 : 0);
+			break;
+		default:
+			ret = -EINVAL;
+		}
+		if (ret)
+			st->double_scale = !double_scale; /* revert on failure */
+		return ret;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad5686_write_raw_get_fmt(struct iio_dev *indio_dev,
+				    struct iio_chan_spec const *chan,
+				    long mask)
+{
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		return IIO_VAL_INT_PLUS_NANO;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad5686_read_avail(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     const int **vals, int *type, int *length,
+			     long mask)
+{
+	struct ad5686_state *st = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		*type = IIO_VAL_INT_PLUS_NANO;
+
+		if (st->chip_info->regmap_type == AD5686_REGMAP && !st->gain_gpio) {
+			/*
+			 * GAIN pin is board-strapped, so only the current
+			 * scale is available.
+			 */
+			*vals = st->double_scale ? &st->scale_avail[2] :
+						   &st->scale_avail[0];
+			*length = 2;
+			return IIO_AVAIL_LIST;
+		}
+
+		*vals = st->scale_avail;
+		*length = ARRAY_SIZE(st->scale_avail);
+		return IIO_AVAIL_LIST;
 	default:
 		return -EINVAL;
 	}
@@ -225,6 +315,8 @@ static int ad5686_write_raw(struct iio_dev *indio_dev,
 static const struct iio_info ad5686_info = {
 	.read_raw = ad5686_read_raw,
 	.write_raw = ad5686_write_raw,
+	.write_raw_get_fmt = ad5686_write_raw_get_fmt,
+	.read_avail = ad5686_read_avail,
 };
 
 static const struct iio_chan_spec_ext_info ad5686_ext_info[] = {
@@ -246,6 +338,7 @@ static const struct iio_chan_spec_ext_info ad5686_ext_info[] = {
 		.channel = chan,				\
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),	\
 		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),\
+		.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SCALE),\
 		.address = addr,				\
 		.scan_index = chan,				\
 		.scan_type = {					\
@@ -472,6 +565,15 @@ const struct ad5686_chip_info ad5679r_chip_info = {
 };
 EXPORT_SYMBOL_NS_GPL(ad5679r_chip_info, "IIO_AD5686");
 
+static void ad5686_init_scale_avail(struct ad5686_state *st)
+{
+	int realbits = st->chip_info->channels[0].scan_type.realbits;
+	s64 tmp = 2ULL * st->vref_mv * NANO >> realbits;
+
+	st->scale_avail[2] = div_s64_rem(tmp, NANO, &st->scale_avail[3]);
+	st->scale_avail[0] = div_s64_rem(tmp >> 1, NANO, &st->scale_avail[1]);
+}
+
 static irqreturn_t ad5686_trigger_handler(int irq, void *p)
 {
 	struct iio_poll_func *pf = p;
@@ -579,6 +681,14 @@ int ad5686_probe(struct device *dev,
 		return dev_err_probe(dev, PTR_ERR(st->ldac_gpio),
 				     "Failed to get LDAC GPIO\n");
 
+	st->gain_gpio = devm_gpiod_get_optional(dev, "gain", GPIOD_OUT_LOW);
+	if (IS_ERR(st->gain_gpio))
+		return dev_err_probe(dev, PTR_ERR(st->gain_gpio),
+				     "Failed to get GAIN GPIO\n");
+
+	st->double_scale = device_property_read_bool(dev, "adi,range-double");
+	ad5686_init_scale_avail(st);
+
 	reset_control_assert(rstc);
 	fsleep(1); /* reset pulse: comfortably bigger than the spec */
 	reset_control_deassert(rstc);
@@ -621,6 +731,8 @@ int ad5686_probe(struct device *dev,
 				   st->use_internal_vref ? 0 : AD5686_REF_BIT_MSK);
 		if (ret)
 			return ret;
+
+		gpiod_set_value_cansleep(st->gain_gpio, st->double_scale ? 1 : 0);
 		break;
 	default:
 		return -EINVAL;
diff --git a/drivers/iio/dac/ad5686.h b/drivers/iio/dac/ad5686.h
index 6f47493906d4..b74a641d8fa4 100644
--- a/drivers/iio/dac/ad5686.h
+++ b/drivers/iio/dac/ad5686.h
@@ -39,9 +39,11 @@
 #define AD5686_CMD_CONTROL_REG			0x4
 #define AD5686_CMD_READBACK_ENABLE_V2		0x5
 
+#define AD5310_GAIN_BIT_MSK			BIT(7)
 #define AD5310_REF_BIT_MSK			BIT(8)
 #define AD5310_PD_MSK				GENMASK(10, 9)
 
+#define AD5683_GAIN_BIT_MSK			BIT(11)
 #define AD5683_REF_BIT_MSK			BIT(12)
 #define AD5683_PD_MSK				GENMASK(14, 13)
 
@@ -124,9 +126,12 @@ extern const struct ad5686_chip_info ad5679r_chip_info;
  * @chip_info:		chip model specific constants, available modes etc
  * @ops:		bus specific operations
  * @ldac_gpio:		LDAC pin GPIO descriptor
- * @vref_mv:		actual reference voltage used
+ * @gain_gpio:		GAIN pin GPIO descriptor
  * @pwr_down_mask:	power down mask
  * @pwr_down_mode:	current power down mode
+ * @scale_avail:	pre-calculated available scale values
+ * @vref_mv:		actual reference voltage used
+ * @double_scale:	flag to indicate the gain multiplier is applied
  * @use_internal_vref:	set to true if the internal reference voltage is used
  * @lock:		lock to protect access to state fields, which includes
  *			the data buffer during regmap ops
@@ -138,9 +143,12 @@ struct ad5686_state {
 	const struct ad5686_chip_info	*chip_info;
 	const struct ad5686_bus_ops	*ops;
 	struct gpio_desc		*ldac_gpio;
-	unsigned short			vref_mv;
+	struct gpio_desc		*gain_gpio;
 	unsigned int			pwr_down_mask;
 	unsigned int			pwr_down_mode;
+	int				scale_avail[4];
+	unsigned short			vref_mv;
+	bool				double_scale;
 	bool				use_internal_vref;
 	struct mutex			lock;
 	void				*bus_data;

-- 
2.43.0

Re: [PATCH v3 06/12] iio: dac: ad5686: consume optional reset signal

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Missing return value checks for `reset_control_assert()` and `reset_control_deassert()`.
- [High] The reset pin is left in a deasserted (driven) state on probe failure or device unbind, leading to potential back-powering of the DAC.
- [Medium] Acquiring the reset control after enabling regulators causes unnecessary power toggling on `-EPROBE_DEFER` and leaves the reset pin floating during power-up.
- [Medium] Missing recovery delay after deasserting the hardware reset may cause subsequent device communications to fail on fast CPUs.
--

commit c3eb5fda0704f698d1f8b2f76769857300b80132
Author: Rodrigo Alencar <rodrigo.alencar@analog.com>

iio: dac: ad5686: consume optional reset signal
    
This commit adds GPIO support for the RESET pin using an optional
reset control local to the probe function. It manually generates
a reset pulse after the device is powered up.

> diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
> index fc3863274b291..a48acedff51c1 100644
> --- a/drivers/iio/dac/ad5686.c
> +++ b/drivers/iio/dac/ad5686.c
[ ... ]
> @@ -508,6 +510,15 @@ int ad5686_probe(struct device *dev,
>  
>  	fsleep(5); /* power-up time */
>  
> +	rstc = devm_reset_control_get_optional_exclusive(dev, NULL);

[Severity: Medium]
Is it intentional to fetch the reset control after the regulators are
enabled and the 5µs power-up delay has occurred?

If devm_reset_control_get_optional_exclusive() returns -EPROBE_DEFER,
the driver will disable the regulators and have to retry the sequence
later. Fetching the reset control before enabling regulators might also
prevent the reset pin from floating during power-up.

> +	if (IS_ERR(rstc))
> +		return dev_err_probe(dev, PTR_ERR(rstc),
> +				     "Failed to get reset control\n");
> +
> +	reset_control_assert(rstc);

[Severity: High]
Do we need to check the return value of reset_control_assert() and
reset_control_deassert()?

These functions can fail and return error codes, particularly if the
reset controller is behind a slow bus. Proceeding without checking
could cause the driver to communicate with a device that is still held
in reset.

> +	fsleep(1); /* reset pulse: comfortably bigger than the spec */
> +	reset_control_deassert(rstc);

[Severity: Medium]
Does the device need a guaranteed recovery delay after the reset is
deasserted?

Exiting the hardware reset initiates an internal power-on reset
sequence. On faster CPUs, the execution time before the first SPI/I2C
command might be shorter than the device's required reset recovery time.

[Severity: High]
Should there be a registered devm action to assert the reset pin on
driver removal or probe failure?

Currently, the reset pin is deasserted here but never asserted again on
cleanup. Since devm_regulator_get_enable() automatically disables the
VDD and VLOGIC regulators during cleanup, leaving this pin driven HIGH
while the device is unpowered could forward-bias ESD protection diodes
and back-power or damage the DAC.

> +
>  	/* Initialize masks to all ones */
>  	st->pwr_down_mask = ~0;
>  	st->pwr_down_mode = ~0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-ad5686-new-features-v3-0-f829fb7e9262@analog.com?part=6

Re: [PATCH v3 01/12] dt-bindings: iio: dac: ad5696: add reset/ldac/gain support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Incorrect use of `unevaluatedProperties: false` without an external schema `$ref`.
--

commit cd7d8addb3fad4751f6c895cad5073c1197387e2
Author: Rodrigo Alencar <rodrigo.alencar@analog.com>

dt-bindings: iio: dac: ad5696: add reset/ldac/gain support

This commit adds GPIO properties for RESET, LDAC, and GAIN pins to the 
adi,ad5696 DAC device tree binding. It introduces conditionally restricted 
support for gain-gpios and adi,range-double based on the compatible string.

> diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5696.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5696.yaml
> index b5a88b03dc2f0..c55158c464fde 100644
> --- a/Documentation/devicetree/bindings/iio/dac/adi,ad5696.yaml
> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5696.yaml

[ ... ]

>  required:
>    - compatible
>    - reg
>  
> -additionalProperties: false
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            anyOf:
> +              - const: adi,ad5311r
> +              - const: adi,ad5691r
> +              - const: adi,ad5692r
> +              - const: adi,ad5693
> +              - const: adi,ad5693r
> +    then:
> +      properties:
> +        gain-gpios: false
> +
> +unevaluatedProperties: false

[Severity: Low]
This isn't a bug, but is it necessary to change additionalProperties to
unevaluatedProperties here? Because this schema does not reference any
external schemas via a $ref, and the conditionally restricted gain-gpios
property is already explicitly declared in the top-level properties block,
additionalProperties: false is sufficient and usually preferred when no
external schema references are present.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-ad5686-new-features-v3-0-f829fb7e9262@analog.com?part=1

Re: [PATCH v3 04/12] dt-bindings: iio: dac: ad5686: rework on power supplies

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Making new power supply properties strictly required breaks backward compatibility for existing devicetrees.
--

commit 37c22112eb8071cacd8ef295c9d3096b1ae584af
Author: Rodrigo Alencar <rodrigo.alencar@analog.com>

dt-bindings: iio: dac: ad5686: rework on power supplies

This commit introduces vdd-supply, vlogic-supply, and vref-supply properties
to the ad5686 binding while deprecating the misleading vcc-supply property. 
The new power supply properties are added to the required lists to better
reflect the physical hardware requirements.

> diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5686.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5686.yaml
> index 7567de0ead4a3..bdc6bc425b489 100644
> --- a/Documentation/devicetree/bindings/iio/dac/adi,ad5686.yaml
> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5686.yaml

[ ... ]

> @@ -60,9 +74,23 @@ properties:
>  required:
>    - compatible
>    - reg
> +  - vdd-supply
> +  - vlogic-supply

[Severity: High]
Does making these new properties unconditionally required break backward 
compatibility for existing device trees? Older device trees that were valid 
under the previous schema will now fail dtbs_check because they lack 
vdd-supply and vlogic-supply.

>  allOf:
>    - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            anyOf:
> +              - const: adi,ad5676
> +              - const: adi,ad5683
> +              - const: adi,ad5684
> +              - const: adi,ad5686
> +    then:
> +      required:
> +        - vref-supply

[Severity: High]
Will this strict requirement cause older device trees that rely on the now 
deprecated vcc-supply to fail validation? Is there a way to allow either 
vref-supply or the deprecated vcc-supply in an anyOf block so older dtb 
files still pass validation while enforcing the requirement moving forward?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-ad5686-new-features-v3-0-f829fb7e9262@analog.com?part=4

Re: [PATCH v3 12/12] iio: dac: ad5686: add gain control support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The GAIN GPIO is unconditionally requested as driven low, causing a potential DAC output glitch during probe.
--

commit 2f56ef5391a1b46c707191ed6f4562758e4e5acc
Author: Rodrigo Alencar <rodrigo.alencar@analog.com>

iio: dac: ad5686: add gain control support

This commit adds gain control support to the AD5686 DAC driver. It handles
gain multiplication either via an external hardware GAIN pin or through
internal control register bits, initializing the default scale based on
device properties.

> diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
> index 6ae788f665b4d..c186213a46f6d 100644
> --- a/drivers/iio/dac/ad5686.c
> +++ b/drivers/iio/dac/ad5686.c
[ ... ]
> @@ -579,6 +681,14 @@ int ad5686_probe(struct device *dev,
>  		return dev_err_probe(dev, PTR_ERR(st->ldac_gpio),
>  				     "Failed to get LDAC GPIO\n");
>  
> +	st->gain_gpio = devm_gpiod_get_optional(dev, "gain", GPIOD_OUT_LOW);
> +	if (IS_ERR(st->gain_gpio))
> +		return dev_err_probe(dev, PTR_ERR(st->gain_gpio),
> +				     "Failed to get GAIN GPIO\n");
> +
> +	st->double_scale = device_property_read_bool(dev, "adi,range-double");

[Severity: Medium]
Could this cause an unwanted glitch on the analog output during probe?

If the system is warm-booted and the DAC is already outputting voltage with
a 2x gain, unconditionally forcing the gain pin low here will abruptly halve
the output voltage before the device is reset or the pin is corrected later in
the probe sequence.

Would it be safer to read the adi,range-double property before requesting the
GPIO, so that the pin can be initialized to its correct target state without
the temporary drop to 1x gain?

> +	ad5686_init_scale_avail(st);
> +
>  	reset_control_assert(rstc);
>  	fsleep(1); /* reset pulse: comfortably bigger than the spec */
>  	reset_control_deassert(rstc);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-ad5686-new-features-v3-0-f829fb7e9262@analog.com?part=12

Re: [PATCH v3 03/12] dt-bindings: iio: dac: ad5686: add reset/ldac/gain support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The conditional block restricts `gain-gpios` for single-channel variants but omits several multi-channel variants (e.g., `adi,ad5672r`, `adi,ad5676`, `adi,ad5674r`, `adi,ad5679r`) that lack a physical GAIN pin and internal multiplier. It also fails to restrict the `adi,range-double` property for these multi-channel variants.
--

commit aa16b66ecffaca7347b3cea2e8c47bd2390db0b1
Author: Rodrigo Alencar <rodrigo.alencar@analog.com>

dt-bindings: iio: dac: ad5686: add reset/ldac/gain support

This commit adds GPIO properties for RESET, LDAC, and GAIN pins to the
ad5686 device tree binding. It also adds an adi,range-double property to
indicate when the GAIN pin is hardwired high.

> diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5686.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5686.yaml
> --- a/Documentation/devicetree/bindings/iio/dac/adi,ad5686.yaml
> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5686.yaml

[ ... ]

>  allOf:
>    - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            anyOf:
> +              - const: adi,ad5310r
> +              - const: adi,ad5681r
> +              - const: adi,ad5682r
> +              - const: adi,ad5683
> +              - const: adi,ad5683r
> +    then:
> +      properties:
> +        gain-gpios: false
>  
>  unevaluatedProperties: false

[Severity: Medium]
Does this conditional block also need to restrict gain-gpios and
adi,range-double for multi-channel variants that lack a physical GAIN pin?

Variants like adi,ad5672r, adi,ad5676, adi,ad5674r, and adi,ad5679r also
seem to lack this hardware pin and an internal multiplier. If omitted from
this list, the binding allows these properties to pass validation for
incompatible hardware.

Furthermore, if adi,range-double is provided for these multi-channel
variants, the driver might initialize with double_scale = true and
permanently report an incorrect 2x output voltage scale to userspace.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-ad5686-new-features-v3-0-f829fb7e9262@analog.com?part=3

Re: [PATCH v3 05/12] iio: dac: ad5686: add support for missing power supplies

[Joshua Crofts]

On Tue, 16 Jun 2026 09:21:11 +0100
Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote:
>  	st->use_internal_vref = ret == -ENODEV;
>  	st->vref_mv = st->use_internal_vref ? st->chip_info->int_vref_mv : ret / 1000;

I'd use a macro from <linux/units.h> instead of just doing / 1000.

> +	if (!st->vref_mv)
> +		return dev_err_probe(dev, -EINVAL,
> +				     "invalid or not provided vref voltage\n");
> +
> +	fsleep(5); /* power-up time */

I'd recommend using a macro to define the sleep time, removing
the need for a comment.

>  	/* Initialize masks to all ones */
>  	st->pwr_down_mask = ~0;
> 

Otherwise LGTM!

-- 
Kind regards

CJD

Re: [PATCH v3 05/12] iio: dac: ad5686: add support for missing power supplies

[Andy Shevchenko]

On Tue, Jun 16, 2026 at 11:33:22AM +0200, Joshua Crofts wrote:
> On Tue, 16 Jun 2026 09:21:11 +0100
> Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote:

...

> >  	st->use_internal_vref = ret == -ENODEV;
> >  	st->vref_mv = st->use_internal_vref ? st->chip_info->int_vref_mv : ret / 1000;

> I'd use a macro from <linux/units.h> instead of just doing / 1000.

In this case we use (MICRO / MILLI) as we don't have (yet?) the
MILLIVOLTS_PER_VOLT and so on. Feeling like you want to add those?

...

> > +	fsleep(5); /* power-up time */
> 
> I'd recommend using a macro to define the sleep time, removing
> the need for a comment.

OTOH macro might hide the actual value. The comment is fine, but I would extend
it with a reference to a data sheet page, table, section, et cetera.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 06/12] iio: dac: ad5686: consume optional reset signal

[Andy Shevchenko]

On Tue, Jun 16, 2026 at 09:21:12AM +0100, Rodrigo Alencar via B4 Relay wrote:

> Add RESET pin GPIO support through an optional reset control, which is
> local to the probe function. A reset pulse is manually generated after
> the device is powered up.

...

> +	fsleep(1); /* reset pulse: comfortably bigger than the spec */

What spec? Same comment basically, extend with the reference to the datasheet.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 07/12] iio: dac: ad5686: add ldac gpio

[Andy Shevchenko]

On Tue, Jun 16, 2026 at 09:21:13AM +0100, Rodrigo Alencar via B4 Relay wrote:

> If wired LDAC, should be held low when unused (pin is active-low), which
> allows for synchronous DAC updates. This will be used to update all the
> channels at the same time when adding buffer support.

...

> +struct gpio_desc;

+ blank line to decouple generic declarations from local ones?

>  struct ad5686_state;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 09/12] iio: dac: ad5686: implement new sync() op for the spi bus

[Andy Shevchenko]

On Tue, Jun 16, 2026 at 09:21:15AM +0100, Rodrigo Alencar via B4 Relay wrote:

> Use of local SPI bus data to manage a collection of SPI transfers and
> flush them to the SPI platform driver with the sync() operation. This
> allows for faster handling of multiple channel DAC writes, avoiding kernel
> overhead per spi_sync() call, which will be helpful when enabling
> triggered buffer support.

...

>  static int ad5686_spi_probe(struct spi_device *spi)
>  {
> -	return ad5686_probe(&spi->dev, spi_get_device_match_data(spi),
> -			    spi->modalias, &ad5686_spi_ops);
> +	const struct ad5686_chip_info *info = spi_get_device_match_data(spi);

Sashiko might still complain on driver_override case, which means we need to
add a NULL check (or is NULL fine?).

> +	struct ad5686_spi_data *bus_data;
> +	struct device *dev = &spi->dev;
> +	unsigned int capacity;
> +
> +	/* read operation requires at least 2 transfers */
> +	capacity = max(info->num_channels, 2);
> +	bus_data = devm_kzalloc(dev, struct_size(bus_data, xfers, capacity),
> +				GFP_KERNEL);
> +	if (!bus_data)
> +		return -ENOMEM;
> +
> +	bus_data->capacity = capacity;
> +
> +	return ad5686_probe(dev, info, spi->modalias, &ad5686_spi_ops, bus_data);
>  }

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 10/12] iio: dac: ad5686: add triggered buffer support

[Andy Shevchenko]

On Tue, Jun 16, 2026 at 09:21:16AM +0100, Rodrigo Alencar via B4 Relay wrote:

> Implement trigger handler by leveraging the LDAC gpio to update all DAC
> channels at once when it is available. Also, the multiple channel writes
> can be flushed at once with the sync() operation.

...

> +static irqreturn_t ad5686_trigger_handler(int irq, void *p)
> +{
> +	struct iio_poll_func *pf = p;
> +	struct iio_dev *indio_dev = pf->indio_dev;
> +	struct iio_buffer *buffer = indio_dev->buffer;
> +	struct ad5686_state *st = iio_priv(indio_dev);
> +	u16 val[AD5686_MAX_CHANNELS] = { };
> +	unsigned int scan_count;

> +	int ret, ch, i = 0;

Decouple assignment and definition. Also do 'i' and 'ch' need to be signed?

> +	bool async_update;
> +	u8 cmd;
> +
> +	ret = iio_pop_from_buffer(buffer, val);
> +	if (ret)
> +		goto out_notify_done;
> +
> +	mutex_lock(&st->lock);
> +
> +	scan_count = bitmap_weight(indio_dev->active_scan_mask,
> +				   iio_get_masklength(indio_dev));
> +	async_update = st->ldac_gpio && scan_count > 1;
> +	if (async_update) {
> +		/* use LDAC to update all channels simultaneously */
> +		cmd = AD5686_CMD_WRITE_INPUT_N;
> +		gpiod_set_value_cansleep(st->ldac_gpio, 0);
> +	} else {
> +		cmd = AD5686_CMD_WRITE_INPUT_N_UPDATE_N;
> +	}
> +
> +	iio_for_each_active_channel(indio_dev, ch) {
> +		ret = st->ops->write(st, cmd, indio_dev->channels[ch].address, val[i++]);
> +		if (ret)
> +			break;
> +	}
> +
> +	/*
> +	 * If sync() is available, it is called here regardless of write
> +	 * failure to allow bus implementation to reset. In that case, partial
> +	 * writes are unlikely as the write operations would just queue up
> +	 * the transfers.
> +	 */
> +	if (st->ops->sync)
> +		ret = st->ops->sync(st); /* flush all pending transfers */

'ret' is set but not used. Always compile your code with `make W=1` and both
GCC and clang.

> +	if (async_update)
> +		gpiod_set_value_cansleep(st->ldac_gpio, 1);
> +
> +	mutex_unlock(&st->lock);
> +out_notify_done:
> +	iio_trigger_notify_done(indio_dev->trig);
> +
> +	return IRQ_HANDLED;
> +}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 11/12] iio: dac: ad5686: read_raw/write_raw: use guard(mutex)()

[Andy Shevchenko]

On Tue, Jun 16, 2026 at 09:21:17AM +0100, Rodrigo Alencar via B4 Relay wrote:

> Use guarded mutex lock to facilitate code review when adding new
> attributes. This will allow for early returns, avoiding error-prone
> locking and unlocking in error paths. This also adds missing include
> linux/cleanup.h. Gain-control support will allow the scale attribute
> to be configurable.

What about interrupt handler? You just added yet another mutex lock/unlock
there. Perhaps this patch should be done before the previous one?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 12/12] iio: dac: ad5686: add gain control support

[Andy Shevchenko]

On Tue, Jun 16, 2026 at 09:21:18AM +0100, Rodrigo Alencar via B4 Relay wrote:

> Most of the supported devices rely on a GAIN pin to control a 2x
> multiplier applied to the output voltage. Other devices, e.g. the
> single-channel ones, provides a gain control through a bit field in
> the control register. Some designs might have the GAIN pin hardwired
> to VDD/VLOGIC or GND, which would have no "gain-gpios" device property,
> being able to set "adi,range-double" if it is hardwired to VDD. The
> vref_mv field is moved down in the struct ad5686_state, so that the
> overall size increase is reduced.

...

> +static void ad5686_init_scale_avail(struct ad5686_state *st)
> +{
> +	int realbits = st->chip_info->channels[0].scan_type.realbits;
> +	s64 tmp = 2ULL * st->vref_mv * NANO >> realbits;

As Nuno noticed in other patch review, this can probably use mul_u64_u32_shr().

> +	st->scale_avail[2] = div_s64_rem(tmp, NANO, &st->scale_avail[3]);
> +	st->scale_avail[0] = div_s64_rem(tmp >> 1, NANO, &st->scale_avail[1]);
> +}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 11/12] iio: dac: ad5686: read_raw/write_raw: use guard(mutex)()

[Rodrigo Alencar]

On 16/06/26 13:43, Andy Shevchenko wrote:
> On Tue, Jun 16, 2026 at 09:21:17AM +0100, Rodrigo Alencar via B4 Relay wrote:
> 
> > Use guarded mutex lock to facilitate code review when adding new
> > attributes. This will allow for early returns, avoiding error-prone
> > locking and unlocking in error paths. This also adds missing include
> > linux/cleanup.h. Gain-control support will allow the scale attribute
> > to be configurable.
> 
> What about interrupt handler? You just added yet another mutex lock/unlock
> there. Perhaps this patch should be done before the previous one?

Can't add the guard(mutex)() in the trigger handler because of the goto.

-- 
Kind regards,

Rodrigo Alencar

Re: [PATCH v3 11/12] iio: dac: ad5686: read_raw/write_raw: use guard(mutex)()

[Andy Shevchenko]

On Tue, Jun 16, 2026 at 1:50 PM Rodrigo Alencar
<455.rodrigo.alencar@gmail.com> wrote:
> On 16/06/26 13:43, Andy Shevchenko wrote:
> > On Tue, Jun 16, 2026 at 09:21:17AM +0100, Rodrigo Alencar via B4 Relay wrote:
> >
> > > Use guarded mutex lock to facilitate code review when adding new
> > > attributes. This will allow for early returns, avoiding error-prone
> > > locking and unlocking in error paths. This also adds missing include
> > > linux/cleanup.h. Gain-control support will allow the scale attribute
> > > to be configurable.
> >
> > What about interrupt handler? You just added yet another mutex lock/unlock
> > there. Perhaps this patch should be done before the previous one?
>
> Can't add the guard(mutex)() in the trigger handler because of the goto.

It might need a refactoring then?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 11/12] iio: dac: ad5686: read_raw/write_raw: use guard(mutex)()

[Rodrigo Alencar]

On 16/06/26 13:52, Andy Shevchenko wrote:
> On Tue, Jun 16, 2026 at 1:50 PM Rodrigo Alencar
> <455.rodrigo.alencar@gmail.com> wrote:
> > On 16/06/26 13:43, Andy Shevchenko wrote:
> > > On Tue, Jun 16, 2026 at 09:21:17AM +0100, Rodrigo Alencar via B4 Relay wrote:
> > >
> > > > Use guarded mutex lock to facilitate code review when adding new
> > > > attributes. This will allow for early returns, avoiding error-prone
> > > > locking and unlocking in error paths. This also adds missing include
> > > > linux/cleanup.h. Gain-control support will allow the scale attribute
> > > > to be configurable.
> > >
> > > What about interrupt handler? You just added yet another mutex lock/unlock
> > > there. Perhaps this patch should be done before the previous one?
> >
> > Can't add the guard(mutex)() in the trigger handler because of the goto.
> 
> It might need a refactoring then?

It would deviate from other examples... would either repeat the code in the end
inside an if statement, or wrap most of the function implementation inside
one. I suppose the first option would be better.

-- 
Kind regards,

Rodrigo Alencar

Re: [PATCH v3 01/12] dt-bindings: iio: dac: ad5696: add reset/ldac/gain support

[Rob Herring]

On Tue, Jun 16, 2026 at 09:21:07AM +0100, Rodrigo Alencar wrote:
> Add GPIO property for RESET, LDAC and GAIN pin. RESET is active-low, LDAC
> is used to load DAC channels with values from input registers and GAIN
> can double the voltage in output channels. The gain-gpios property is
> not available to all supported parts. The adi,range-double property
> indicates that GAIN pin is hardwired to high in case gain-gpios is not
> set, otherwise it sets the initial value for the gain setting.
> 
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
> ---
>  .../devicetree/bindings/iio/dac/adi,ad5696.yaml    | 41 +++++++++++++++++++++-
>  1 file changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5696.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5696.yaml
> index b5a88b03dc2f..c55158c464fd 100644
> --- a/Documentation/devicetree/bindings/iio/dac/adi,ad5696.yaml
> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5696.yaml
> @@ -37,14 +37,52 @@ properties:
>      description: |
>        The regulator supply for DAC reference voltage.
>  
> +  reset-gpios:
> +    description: Active-low RESET pin to reset the device.
> +    maxItems: 1
> +
> +  ldac-gpios:
> +    description:
> +      Active-low LDAC pin used to asynchronously update the DAC channels.
> +    maxItems: 1
> +
> +  gain-gpios:
> +    description:
> +      GAIN pin that sets a multiplier for the DAC output voltage. When high,
> +      the DAC output voltage is multiplied by 2, otherwise it is unchanged.
> +    maxItems: 1
> +
> +  adi,range-double:
> +    description:
> +      Sets the initial voltage output range from 0 to 2xVREF. On devices that
> +      have a GAIN pin and no gain-gpios property is set, this indicates the pin
> +      is hardwired high.
> +    type: boolean
> +
>  required:
>    - compatible
>    - reg
>  
> -additionalProperties: false
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            anyOf:
> +              - const: adi,ad5311r
> +              - const: adi,ad5691r
> +              - const: adi,ad5692r
> +              - const: adi,ad5693
> +              - const: adi,ad5693r

Just 'enum' instead of anyOf+const.

> +    then:
> +      properties:
> +        gain-gpios: false
> +
> +unevaluatedProperties: false

sashiko is correct. No need for unevaluatedProperties here.

Rob

Re: [PATCH v3 02/12] dt-bindings: iio: dac: ad5696: rework on power supplies

[Rob Herring]

On Tue, Jun 16, 2026 at 09:21:08AM +0100, Rodrigo Alencar wrote:
> Add supplies for VDD, VLOGIC and VREF input voltage pins. The vcc-supply
> property is deprecated, once it does not really exist as none of the
> devices describe any power input with that name. VCC is also misleading as
> it sounds like the input power supply, but it is being used as an external
> voltage reference, which should be called VREF. Certain devices require
> vref-supply to be available once an internal reference voltage is absent.
> For correct operation vdd and vlogic supplies are required.
> 
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
> ---
>  .../devicetree/bindings/iio/dac/adi,ad5696.yaml    | 34 ++++++++++++++++++++--
>  1 file changed, 31 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5696.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5696.yaml
> index c55158c464fd..7b936824917e 100644
> --- a/Documentation/devicetree/bindings/iio/dac/adi,ad5696.yaml
> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5696.yaml
> @@ -33,9 +33,22 @@ properties:
>    reg:
>      maxItems: 1
>  
> +  vdd-supply:
> +    description: Input power supply.
> +
> +  vlogic-supply:
> +    description:
> +      Digital power supply. On some tiny package variants for single-channel
> +      devices, this supply is internally connected to vdd; in that case, specify
> +      this property with the same regulator as vdd.
> +
> +  vref-supply:
> +    description:
> +      Reference voltage supply. If not supplied the internal reference is used.
> +
>    vcc-supply:
> -    description: |
> -      The regulator supply for DAC reference voltage.
> +    deprecated: true
> +    description: Use vref-supply instead.
>  
>    reset-gpios:
>      description: Active-low RESET pin to reset the device.
> @@ -62,8 +75,21 @@ properties:
>  required:
>    - compatible
>    - reg
> +  - vdd-supply
> +  - vlogic-supply
>  
>  allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            anyOf:
> +              - const: adi,ad5693
> +              - const: adi,ad5694
> +              - const: adi,ad5696

enum rather than anyOf+const.

Rob