mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v7 0/8] Fixes and cleanups for the AD5686 IIO driver
@ 2026-05-24 10:17 Rodrigo Alencar via B4 Relay
  2026-05-24 10:17 ` [PATCH v7 1/8] iio: dac: ad5686: refactor include headers Rodrigo Alencar via B4 Relay
                   ` (9 more replies)
  0 siblings, 10 replies; 12+ messages in thread
From: Rodrigo Alencar via B4 Relay @ 2026-05-24 10:17 UTC (permalink / raw)
  To: linux-iio, linux-kernel, Stefan Popa, Jonathan Cameron,
	Greg Kroah-Hartman, Michael Auchter, Jonathan Cameron
  Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	David Lechner, Andy Shevchenko, Rodrigo Alencar, Andy Shevchenko

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

A bigger patch series was sent before ("Extend device support for AD5686 driver"):

https://lore.kernel.org/r/20260422-ad5313r-iio-support-v1-0-ed7dca001d1b@analog.com

This one adds a number of cleanups and fixes, like:
- Refactor include headers (IWYU);
- Remove redundant register definition;
- Drop enum chip id in favor of per-device chip_info structs;
- Fix internal voltage reference control for single-channel devices;
- Acquire lock when doing power down control;
- Fix powerdown control for dual-channel devices;

Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
Changes in v7:
- Drop fixes (already accepted).
- Use named fields in device tables.
- Link to v6: https://lore.kernel.org/r/20260505-ad5686-fixes-v6-0-c2d5f7be32be@analog.com

Changes in v6:
- Protect powerdown masks on read access too.
- Minor changes to power down handling and constants.
- Link to v5: https://lore.kernel.org/r/20260501-ad5686-fixes-v5-0-0b2f45488418@analog.com

Changes in v5:
- Drop I2C read operation changes.
- Create helpers for reading and writing powerdown mask bits
- Link to v4: https://lore.kernel.org/r/20260429-ad5686-fixes-v4-0-bb8f1cbd68e1@analog.com

Changes in v4:
- Address issues spotted by sashiko.
- Link to v3: https://lore.kernel.org/r/20260428-ad5686-fixes-v3-0-9cff7bd67a15@analog.com

Changes in v3:
- Misc changes like parenthesis removal and line breaks
- Link to v2: https://lore.kernel.org/r/20260427-ad5686-fixes-v2-0-188e05199368@analog.com

Changes in v2:
- Bring fixes first and cleanups later
- Link to v1: https://lore.kernel.org/r/20260426-ad5686-fixes-v1-0-7c946a77794e@analog.com

---
Rodrigo Alencar (8):
      iio: dac: ad5686: refactor include headers
      iio: dac: ad5686: remove redundant register definition
      iio: dac: ad5686: drop enum id
      iio: dac: ad5686: add of_match table to the spi driver
      iio: dac: ad5686: add helpers to handle powerdown masks
      iio: dac: ad5686: add control_sync() for single-channel devices
      iio: dac: ad5686: cleanup doc header of local structs
      iio: dac: ad5686: create bus ops struct

 drivers/iio/dac/ad5686-spi.c |  73 ++++--
 drivers/iio/dac/ad5686.c     | 528 +++++++++++++++++++++----------------------
 drivers/iio/dac/ad5686.h     | 133 +++++------
 drivers/iio/dac/ad5696-i2c.c |  80 ++++---
 4 files changed, 418 insertions(+), 396 deletions(-)
---
base-commit: bbc109ae655c8a5d4db117fea01947eb6bf51e71
change-id: 20260426-ad5686-fixes-63ea68811bdb

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



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v7 1/8] iio: dac: ad5686: refactor include headers
  2026-05-24 10:17 [PATCH v7 0/8] Fixes and cleanups for the AD5686 IIO driver Rodrigo Alencar via B4 Relay
@ 2026-05-24 10:17 ` Rodrigo Alencar via B4 Relay
  2026-05-24 10:17 ` [PATCH v7 2/8] iio: dac: ad5686: remove redundant register definition Rodrigo Alencar via B4 Relay
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Rodrigo Alencar via B4 Relay @ 2026-05-24 10:17 UTC (permalink / raw)
  To: linux-iio, linux-kernel, Stefan Popa, Jonathan Cameron,
	Greg Kroah-Hartman, Michael Auchter, Jonathan Cameron
  Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	David Lechner, Andy Shevchenko, Rodrigo Alencar, Andy Shevchenko

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

Apply IWYU principle, replacing unused/generic headers for
specific/missing headers. The resulting include directive lists are sorted
accordingly.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
 drivers/iio/dac/ad5686-spi.c |  9 +++++++--
 drivers/iio/dac/ad5686.c     | 13 ++++++-------
 drivers/iio/dac/ad5686.h     |  5 ++---
 drivers/iio/dac/ad5696-i2c.c | 10 +++++++---
 4 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/drivers/iio/dac/ad5686-spi.c b/drivers/iio/dac/ad5686-spi.c
index df8619e0c092..cec784b617e6 100644
--- a/drivers/iio/dac/ad5686-spi.c
+++ b/drivers/iio/dac/ad5686-spi.c
@@ -8,11 +8,16 @@
  * Copyright 2018 Analog Devices Inc.
  */
 
-#include "ad5686.h"
-
+#include <linux/array_size.h>
+#include <linux/errno.h>
+#include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/spi/spi.h>
 
+#include <asm/byteorder.h>
+
+#include "ad5686.h"
+
 static int ad5686_spi_write(struct ad5686_state *st,
 			    u8 cmd, u8 addr, u16 val)
 {
diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
index a7213bc6b156..ae1ef52b62a2 100644
--- a/drivers/iio/dac/ad5686.c
+++ b/drivers/iio/dac/ad5686.c
@@ -5,17 +5,16 @@
  * Copyright 2011 Analog Devices Inc.
  */
 
-#include <linux/interrupt.h>
-#include <linux/fs.h>
-#include <linux/device.h>
+#include <linux/array_size.h>
+#include <linux/bitops.h>
+#include <linux/errno.h>
+#include <linux/export.h>
+#include <linux/kstrtox.h>
 #include <linux/module.h>
-#include <linux/kernel.h>
-#include <linux/slab.h>
-#include <linux/sysfs.h>
 #include <linux/regulator/consumer.h>
+#include <linux/sysfs.h>
 
 #include <linux/iio/iio.h>
-#include <linux/iio/sysfs.h>
 
 #include "ad5686.h"
 
diff --git a/drivers/iio/dac/ad5686.h b/drivers/iio/dac/ad5686.h
index 36e16c5c4581..d08160e7fad9 100644
--- a/drivers/iio/dac/ad5686.h
+++ b/drivers/iio/dac/ad5686.h
@@ -8,10 +8,9 @@
 #ifndef __DRIVERS_IIO_DAC_AD5686_H__
 #define __DRIVERS_IIO_DAC_AD5686_H__
 
-#include <linux/types.h>
-#include <linux/cache.h>
+#include <linux/bits.h>
 #include <linux/mutex.h>
-#include <linux/kernel.h>
+#include <linux/types.h>
 
 #include <linux/iio/iio.h>
 
diff --git a/drivers/iio/dac/ad5696-i2c.c b/drivers/iio/dac/ad5696-i2c.c
index d3327bca0e07..20f04b74d831 100644
--- a/drivers/iio/dac/ad5696-i2c.c
+++ b/drivers/iio/dac/ad5696-i2c.c
@@ -7,10 +7,14 @@
  * Copyright 2018 Analog Devices Inc.
  */
 
-#include "ad5686.h"
-
-#include <linux/module.h>
+#include <linux/errno.h>
 #include <linux/i2c.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+
+#include <asm/byteorder.h>
+
+#include "ad5686.h"
 
 static int ad5686_i2c_read(struct ad5686_state *st, u8 addr)
 {

-- 
2.43.0



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v7 2/8] iio: dac: ad5686: remove redundant register definition
  2026-05-24 10:17 [PATCH v7 0/8] Fixes and cleanups for the AD5686 IIO driver Rodrigo Alencar via B4 Relay
  2026-05-24 10:17 ` [PATCH v7 1/8] iio: dac: ad5686: refactor include headers Rodrigo Alencar via B4 Relay
@ 2026-05-24 10:17 ` Rodrigo Alencar via B4 Relay
  2026-05-24 10:17 ` [PATCH v7 3/8] iio: dac: ad5686: drop enum id Rodrigo Alencar via B4 Relay
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Rodrigo Alencar via B4 Relay @ 2026-05-24 10:17 UTC (permalink / raw)
  To: linux-iio, linux-kernel, Stefan Popa, Jonathan Cameron,
	Greg Kroah-Hartman, Michael Auchter, Jonathan Cameron
  Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	David Lechner, Andy Shevchenko, Rodrigo Alencar, Andy Shevchenko

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

AD5683_REGMAP and AD5693_REGMAP behave the same way in the common code,
and that is because they target single channel devices from the same
sub-family. There is no reason to separate them and it will make things
simpler when refactoring the chip info table.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
 drivers/iio/dac/ad5686.c | 19 +++++--------------
 drivers/iio/dac/ad5686.h |  2 --
 2 files changed, 5 insertions(+), 16 deletions(-)

diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
index ae1ef52b62a2..110ea24c31d1 100644
--- a/drivers/iio/dac/ad5686.c
+++ b/drivers/iio/dac/ad5686.c
@@ -116,10 +116,6 @@ static ssize_t ad5686_write_dac_powerdown(struct iio_dev *indio_dev,
 		if (chan->channel > 0x7)
 			address = 0x8;
 		break;
-	case AD5693_REGMAP:
-		shift = 13;
-		ref_bit_msk = AD5693_REF_BIT_MSK;
-		break;
 	default:
 		return -EINVAL;
 	}
@@ -300,7 +296,7 @@ static const struct ad5686_chip_info ad5686_chip_info_tbl[] = {
 		.channels = ad5311r_channels,
 		.int_vref_mv = 2500,
 		.num_channels = 1,
-		.regmap_type = AD5693_REGMAP,
+		.regmap_type = AD5683_REGMAP,
 	},
 	[ID_AD5337R] = {
 		.channels = ad5337r_channels,
@@ -422,24 +418,24 @@ static const struct ad5686_chip_info ad5686_chip_info_tbl[] = {
 		.channels = ad5691r_channels,
 		.int_vref_mv = 2500,
 		.num_channels = 1,
-		.regmap_type = AD5693_REGMAP,
+		.regmap_type = AD5683_REGMAP,
 	},
 	[ID_AD5692R] = {
 		.channels = ad5692r_channels,
 		.int_vref_mv = 2500,
 		.num_channels = 1,
-		.regmap_type = AD5693_REGMAP,
+		.regmap_type = AD5683_REGMAP,
 	},
 	[ID_AD5693] = {
 		.channels = ad5693_channels,
 		.num_channels = 1,
-		.regmap_type = AD5693_REGMAP,
+		.regmap_type = AD5683_REGMAP,
 	},
 	[ID_AD5693R] = {
 		.channels = ad5693_channels,
 		.int_vref_mv = 2500,
 		.num_channels = 1,
-		.regmap_type = AD5693_REGMAP,
+		.regmap_type = AD5683_REGMAP,
 	},
 	[ID_AD5694] = {
 		.channels = ad5684_channels,
@@ -538,11 +534,6 @@ int ad5686_probe(struct device *dev,
 		cmd = AD5686_CMD_INTERNAL_REFER_SETUP;
 		ref_bit_msk = AD5686_REF_BIT_MSK;
 		break;
-	case AD5693_REGMAP:
-		cmd = AD5686_CMD_CONTROL_REG;
-		ref_bit_msk = AD5693_REF_BIT_MSK;
-		st->use_internal_vref = !has_external_vref;
-		break;
 	default:
 		return -EINVAL;
 	}
diff --git a/drivers/iio/dac/ad5686.h b/drivers/iio/dac/ad5686.h
index d08160e7fad9..af15994c01ad 100644
--- a/drivers/iio/dac/ad5686.h
+++ b/drivers/iio/dac/ad5686.h
@@ -46,7 +46,6 @@
 #define AD5310_REF_BIT_MSK			BIT(8)
 #define AD5683_REF_BIT_MSK			BIT(12)
 #define AD5686_REF_BIT_MSK			BIT(0)
-#define AD5693_REF_BIT_MSK			BIT(12)
 
 /**
  * ad5686_supported_device_ids:
@@ -89,7 +88,6 @@ enum ad5686_regmap_type {
 	AD5310_REGMAP,
 	AD5683_REGMAP,
 	AD5686_REGMAP,
-	AD5693_REGMAP
 };
 
 struct ad5686_state;

-- 
2.43.0



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v7 3/8] iio: dac: ad5686: drop enum id
  2026-05-24 10:17 [PATCH v7 0/8] Fixes and cleanups for the AD5686 IIO driver Rodrigo Alencar via B4 Relay
  2026-05-24 10:17 ` [PATCH v7 1/8] iio: dac: ad5686: refactor include headers Rodrigo Alencar via B4 Relay
  2026-05-24 10:17 ` [PATCH v7 2/8] iio: dac: ad5686: remove redundant register definition Rodrigo Alencar via B4 Relay
@ 2026-05-24 10:17 ` Rodrigo Alencar via B4 Relay
  2026-06-02 13:32   ` Jonathan Cameron
  2026-05-24 10:17 ` [PATCH v7 4/8] iio: dac: ad5686: add of_match table to the spi driver Rodrigo Alencar via B4 Relay
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 12+ messages in thread
From: Rodrigo Alencar via B4 Relay @ 2026-05-24 10:17 UTC (permalink / raw)
  To: linux-iio, linux-kernel, Stefan Popa, Jonathan Cameron,
	Greg Kroah-Hartman, Michael Auchter, Jonathan Cameron
  Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	David Lechner, Andy Shevchenko, Rodrigo Alencar, Andy Shevchenko

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

Split chip info table into separate structs and expose them to the spi
i2c drivers. That is the preferrable approach and allows for the drivers
to have knowledge of the device info before the common probe function gets
called. Those chip info structs may be shared by SPI and I2C driver
variants.
Channel declaration definitions are grouped according to channel count and
DECLARE_AD5693_CHANNELS() macro is renamed to DECLARE_AD5683_CHANNELS() to
match the regmap_type enum.
Use spi_get_device_match_data() and i2c_get_match_data() to get chip info
struct reference, passing it as parameter to the core probe function.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
 drivers/iio/dac/ad5686-spi.c |  38 +++--
 drivers/iio/dac/ad5686.c     | 358 ++++++++++++++++++++-----------------------
 drivers/iio/dac/ad5686.h     |  66 ++++----
 drivers/iio/dac/ad5696-i2c.c |  65 ++++----
 4 files changed, 241 insertions(+), 286 deletions(-)

diff --git a/drivers/iio/dac/ad5686-spi.c b/drivers/iio/dac/ad5686-spi.c
index cec784b617e6..3a2eafedce92 100644
--- a/drivers/iio/dac/ad5686-spi.c
+++ b/drivers/iio/dac/ad5686-spi.c
@@ -94,29 +94,27 @@ static int ad5686_spi_read(struct ad5686_state *st, u8 addr)
 
 static int ad5686_spi_probe(struct spi_device *spi)
 {
-	const struct spi_device_id *id = spi_get_device_id(spi);
-
-	return ad5686_probe(&spi->dev, id->driver_data, id->name,
-			    ad5686_spi_write, ad5686_spi_read);
+	return ad5686_probe(&spi->dev, spi_get_device_match_data(spi),
+			    spi->modalias, ad5686_spi_write, ad5686_spi_read);
 }
 
 static const struct spi_device_id ad5686_spi_id[] = {
-	{"ad5310r", ID_AD5310R},
-	{"ad5672r", ID_AD5672R},
-	{"ad5674r", ID_AD5674R},
-	{"ad5676", ID_AD5676},
-	{"ad5676r", ID_AD5676R},
-	{"ad5679r", ID_AD5679R},
-	{"ad5681r", ID_AD5681R},
-	{"ad5682r", ID_AD5682R},
-	{"ad5683", ID_AD5683},
-	{"ad5683r", ID_AD5683R},
-	{"ad5684", ID_AD5684},
-	{"ad5684r", ID_AD5684R},
-	{"ad5685", ID_AD5685R}, /* Does not exist */
-	{"ad5685r", ID_AD5685R},
-	{"ad5686", ID_AD5686},
-	{"ad5686r", ID_AD5686R},
+	{ .name = "ad5310r", .driver_data = (kernel_ulong_t)&ad5310r_chip_info },
+	{ .name = "ad5672r", .driver_data = (kernel_ulong_t)&ad5672r_chip_info },
+	{ .name = "ad5674r", .driver_data = (kernel_ulong_t)&ad5674r_chip_info },
+	{ .name = "ad5676",  .driver_data = (kernel_ulong_t)&ad5676_chip_info },
+	{ .name = "ad5676r", .driver_data = (kernel_ulong_t)&ad5676r_chip_info },
+	{ .name = "ad5679r", .driver_data = (kernel_ulong_t)&ad5679r_chip_info },
+	{ .name = "ad5681r", .driver_data = (kernel_ulong_t)&ad5681r_chip_info },
+	{ .name = "ad5682r", .driver_data = (kernel_ulong_t)&ad5682r_chip_info },
+	{ .name = "ad5683",  .driver_data = (kernel_ulong_t)&ad5683_chip_info },
+	{ .name = "ad5683r", .driver_data = (kernel_ulong_t)&ad5683r_chip_info },
+	{ .name = "ad5684",  .driver_data = (kernel_ulong_t)&ad5684_chip_info },
+	{ .name = "ad5684r", .driver_data = (kernel_ulong_t)&ad5684r_chip_info },
+	{ .name = "ad5685",  .driver_data = (kernel_ulong_t)&ad5685r_chip_info }, /* nonexistent */
+	{ .name = "ad5685r", .driver_data = (kernel_ulong_t)&ad5685r_chip_info },
+	{ .name = "ad5686",  .driver_data = (kernel_ulong_t)&ad5686_chip_info },
+	{ .name = "ad5686r", .driver_data = (kernel_ulong_t)&ad5686r_chip_info },
 	{ }
 };
 MODULE_DEVICE_TABLE(spi, ad5686_spi_id);
diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
index 110ea24c31d1..50d4d5e8acf8 100644
--- a/drivers/iio/dac/ad5686.c
+++ b/drivers/iio/dac/ad5686.c
@@ -219,7 +219,7 @@ static const struct iio_chan_spec_ext_info ad5686_ext_info[] = {
 		.ext_info = ad5686_ext_info,			\
 }
 
-#define DECLARE_AD5693_CHANNELS(name, bits, _shift)		\
+#define DECLARE_AD5683_CHANNELS(name, bits, _shift)		\
 static const struct iio_chan_spec name[] = {			\
 		AD5868_CHANNEL(0, 0, bits, _shift),		\
 }
@@ -270,205 +270,172 @@ static const struct iio_chan_spec name[] = {			\
 		AD5868_CHANNEL(15, 15, bits, _shift),		\
 }
 
-DECLARE_AD5693_CHANNELS(ad5310r_channels, 10, 2);
-DECLARE_AD5693_CHANNELS(ad5311r_channels, 10, 6);
+/* single-channel */
+DECLARE_AD5683_CHANNELS(ad5310r_channels, 10, 2);
+DECLARE_AD5683_CHANNELS(ad5311r_channels, 10, 6);
+DECLARE_AD5683_CHANNELS(ad5681r_channels, 12, 4);
+DECLARE_AD5683_CHANNELS(ad5682r_channels, 14, 2);
+DECLARE_AD5683_CHANNELS(ad5683r_channels, 16, 0);
+
+/* dual-channel */
 DECLARE_AD5338_CHANNELS(ad5337r_channels, 8, 8);
 DECLARE_AD5338_CHANNELS(ad5338r_channels, 10, 6);
-DECLARE_AD5676_CHANNELS(ad5672_channels, 12, 4);
-DECLARE_AD5679_CHANNELS(ad5674r_channels, 12, 4);
-DECLARE_AD5676_CHANNELS(ad5676_channels, 16, 0);
-DECLARE_AD5679_CHANNELS(ad5679r_channels, 16, 0);
-DECLARE_AD5686_CHANNELS(ad5684_channels, 12, 4);
-DECLARE_AD5686_CHANNELS(ad5685r_channels, 14, 2);
-DECLARE_AD5686_CHANNELS(ad5686_channels, 16, 0);
-DECLARE_AD5693_CHANNELS(ad5693_channels, 16, 0);
-DECLARE_AD5693_CHANNELS(ad5692r_channels, 14, 2);
-DECLARE_AD5693_CHANNELS(ad5691r_channels, 12, 4);
 
-static const struct ad5686_chip_info ad5686_chip_info_tbl[] = {
-	[ID_AD5310R] = {
-		.channels = ad5310r_channels,
-		.int_vref_mv = 2500,
-		.num_channels = 1,
-		.regmap_type = AD5310_REGMAP,
-	},
-	[ID_AD5311R] = {
-		.channels = ad5311r_channels,
-		.int_vref_mv = 2500,
-		.num_channels = 1,
-		.regmap_type = AD5683_REGMAP,
-	},
-	[ID_AD5337R] = {
-		.channels = ad5337r_channels,
-		.int_vref_mv = 2500,
-		.num_channels = 2,
-		.regmap_type = AD5686_REGMAP,
-	},
-	[ID_AD5338R] = {
-		.channels = ad5338r_channels,
-		.int_vref_mv = 2500,
-		.num_channels = 2,
-		.regmap_type = AD5686_REGMAP,
-	},
-	[ID_AD5671R] = {
-		.channels = ad5672_channels,
-		.int_vref_mv = 2500,
-		.num_channels = 8,
-		.regmap_type = AD5686_REGMAP,
-	},
-	[ID_AD5672R] = {
-		.channels = ad5672_channels,
-		.int_vref_mv = 2500,
-		.num_channels = 8,
-		.regmap_type = AD5686_REGMAP,
-	},
-	[ID_AD5673R] = {
-		.channels = ad5674r_channels,
-		.int_vref_mv = 2500,
-		.num_channels = 16,
-		.regmap_type = AD5686_REGMAP,
-	},
-	[ID_AD5674R] = {
-		.channels = ad5674r_channels,
-		.int_vref_mv = 2500,
-		.num_channels = 16,
-		.regmap_type = AD5686_REGMAP,
-	},
-	[ID_AD5675R] = {
-		.channels = ad5676_channels,
-		.int_vref_mv = 2500,
-		.num_channels = 8,
-		.regmap_type = AD5686_REGMAP,
-	},
-	[ID_AD5676] = {
-		.channels = ad5676_channels,
-		.num_channels = 8,
-		.regmap_type = AD5686_REGMAP,
-	},
-	[ID_AD5676R] = {
-		.channels = ad5676_channels,
-		.int_vref_mv = 2500,
-		.num_channels = 8,
-		.regmap_type = AD5686_REGMAP,
-	},
-	[ID_AD5677R] = {
-		.channels = ad5679r_channels,
-		.int_vref_mv = 2500,
-		.num_channels = 16,
-		.regmap_type = AD5686_REGMAP,
-	},
-	[ID_AD5679R] = {
-		.channels = ad5679r_channels,
-		.int_vref_mv = 2500,
-		.num_channels = 16,
-		.regmap_type = AD5686_REGMAP,
-	},
-	[ID_AD5681R] = {
-		.channels = ad5691r_channels,
-		.int_vref_mv = 2500,
-		.num_channels = 1,
-		.regmap_type = AD5683_REGMAP,
-	},
-	[ID_AD5682R] = {
-		.channels = ad5692r_channels,
-		.int_vref_mv = 2500,
-		.num_channels = 1,
-		.regmap_type = AD5683_REGMAP,
-	},
-	[ID_AD5683] = {
-		.channels = ad5693_channels,
-		.num_channels = 1,
-		.regmap_type = AD5683_REGMAP,
-	},
-	[ID_AD5683R] = {
-		.channels = ad5693_channels,
-		.int_vref_mv = 2500,
-		.num_channels = 1,
-		.regmap_type = AD5683_REGMAP,
-	},
-	[ID_AD5684] = {
-		.channels = ad5684_channels,
-		.num_channels = 4,
-		.regmap_type = AD5686_REGMAP,
-	},
-	[ID_AD5684R] = {
-		.channels = ad5684_channels,
-		.int_vref_mv = 2500,
-		.num_channels = 4,
-		.regmap_type = AD5686_REGMAP,
-	},
-	[ID_AD5685R] = {
-		.channels = ad5685r_channels,
-		.int_vref_mv = 2500,
-		.num_channels = 4,
-		.regmap_type = AD5686_REGMAP,
-	},
-	[ID_AD5686] = {
-		.channels = ad5686_channels,
-		.num_channels = 4,
-		.regmap_type = AD5686_REGMAP,
-	},
-	[ID_AD5686R] = {
-		.channels = ad5686_channels,
-		.int_vref_mv = 2500,
-		.num_channels = 4,
-		.regmap_type = AD5686_REGMAP,
-	},
-	[ID_AD5691R] = {
-		.channels = ad5691r_channels,
-		.int_vref_mv = 2500,
-		.num_channels = 1,
-		.regmap_type = AD5683_REGMAP,
-	},
-	[ID_AD5692R] = {
-		.channels = ad5692r_channels,
-		.int_vref_mv = 2500,
-		.num_channels = 1,
-		.regmap_type = AD5683_REGMAP,
-	},
-	[ID_AD5693] = {
-		.channels = ad5693_channels,
-		.num_channels = 1,
-		.regmap_type = AD5683_REGMAP,
-	},
-	[ID_AD5693R] = {
-		.channels = ad5693_channels,
-		.int_vref_mv = 2500,
-		.num_channels = 1,
-		.regmap_type = AD5683_REGMAP,
-	},
-	[ID_AD5694] = {
-		.channels = ad5684_channels,
-		.num_channels = 4,
-		.regmap_type = AD5686_REGMAP,
-	},
-	[ID_AD5694R] = {
-		.channels = ad5684_channels,
-		.int_vref_mv = 2500,
-		.num_channels = 4,
-		.regmap_type = AD5686_REGMAP,
-	},
-	[ID_AD5695R] = {
-		.channels = ad5685r_channels,
-		.int_vref_mv = 2500,
-		.num_channels = 4,
-		.regmap_type = AD5686_REGMAP,
-	},
-	[ID_AD5696] = {
-		.channels = ad5686_channels,
-		.num_channels = 4,
-		.regmap_type = AD5686_REGMAP,
-	},
-	[ID_AD5696R] = {
-		.channels = ad5686_channels,
-		.int_vref_mv = 2500,
-		.num_channels = 4,
-		.regmap_type = AD5686_REGMAP,
-	},
+/* quad-channel */
+DECLARE_AD5686_CHANNELS(ad5684r_channels, 12, 4);
+DECLARE_AD5686_CHANNELS(ad5685r_channels, 14, 2);
+DECLARE_AD5686_CHANNELS(ad5686r_channels, 16, 0);
+
+/* 8-channel */
+DECLARE_AD5676_CHANNELS(ad5672r_channels, 12, 4);
+DECLARE_AD5676_CHANNELS(ad5676r_channels, 16, 0);
+
+/* 16-channel */
+DECLARE_AD5679_CHANNELS(ad5674r_channels, 12, 4);
+DECLARE_AD5679_CHANNELS(ad5679r_channels, 16, 0);
+
+const struct ad5686_chip_info ad5310r_chip_info = {
+	.channels = ad5310r_channels,
+	.int_vref_mv = 2500,
+	.num_channels = 1,
+	.regmap_type = AD5310_REGMAP,
 };
+EXPORT_SYMBOL_NS_GPL(ad5310r_chip_info, "IIO_AD5686");
+
+const struct ad5686_chip_info ad5311r_chip_info = {
+	.channels = ad5311r_channels,
+	.int_vref_mv = 2500,
+	.num_channels = 1,
+	.regmap_type = AD5683_REGMAP,
+};
+EXPORT_SYMBOL_NS_GPL(ad5311r_chip_info, "IIO_AD5686");
+
+const struct ad5686_chip_info ad5681r_chip_info = {
+	.channels = ad5681r_channels,
+	.int_vref_mv = 2500,
+	.num_channels = 1,
+	.regmap_type = AD5683_REGMAP,
+};
+EXPORT_SYMBOL_NS_GPL(ad5681r_chip_info, "IIO_AD5686");
+
+const struct ad5686_chip_info ad5682r_chip_info = {
+	.channels = ad5682r_channels,
+	.int_vref_mv = 2500,
+	.num_channels = 1,
+	.regmap_type = AD5683_REGMAP,
+};
+EXPORT_SYMBOL_NS_GPL(ad5682r_chip_info, "IIO_AD5686");
+
+const struct ad5686_chip_info ad5683_chip_info = {
+	.channels = ad5683r_channels,
+	.num_channels = 1,
+	.regmap_type = AD5683_REGMAP,
+};
+EXPORT_SYMBOL_NS_GPL(ad5683_chip_info, "IIO_AD5686");
+
+const struct ad5686_chip_info ad5683r_chip_info = {
+	.channels = ad5683r_channels,
+	.int_vref_mv = 2500,
+	.num_channels = 1,
+	.regmap_type = AD5683_REGMAP,
+};
+EXPORT_SYMBOL_NS_GPL(ad5683r_chip_info, "IIO_AD5686");
+
+const struct ad5686_chip_info ad5337r_chip_info = {
+	.channels = ad5337r_channels,
+	.int_vref_mv = 2500,
+	.num_channels = 2,
+	.regmap_type = AD5686_REGMAP,
+};
+EXPORT_SYMBOL_NS_GPL(ad5337r_chip_info, "IIO_AD5686");
+
+const struct ad5686_chip_info ad5338r_chip_info = {
+	.channels = ad5338r_channels,
+	.int_vref_mv = 2500,
+	.num_channels = 2,
+	.regmap_type = AD5686_REGMAP,
+};
+EXPORT_SYMBOL_NS_GPL(ad5338r_chip_info, "IIO_AD5686");
+
+const struct ad5686_chip_info ad5684_chip_info = {
+	.channels = ad5684r_channels,
+	.num_channels = 4,
+	.regmap_type = AD5686_REGMAP,
+};
+EXPORT_SYMBOL_NS_GPL(ad5684_chip_info, "IIO_AD5686");
+
+const struct ad5686_chip_info ad5684r_chip_info = {
+	.channels = ad5684r_channels,
+	.int_vref_mv = 2500,
+	.num_channels = 4,
+	.regmap_type = AD5686_REGMAP,
+};
+EXPORT_SYMBOL_NS_GPL(ad5684r_chip_info, "IIO_AD5686");
+
+const struct ad5686_chip_info ad5685r_chip_info = {
+	.channels = ad5685r_channels,
+	.int_vref_mv = 2500,
+	.num_channels = 4,
+	.regmap_type = AD5686_REGMAP,
+};
+EXPORT_SYMBOL_NS_GPL(ad5685r_chip_info, "IIO_AD5686");
+
+const struct ad5686_chip_info ad5686_chip_info = {
+	.channels = ad5686r_channels,
+	.num_channels = 4,
+	.regmap_type = AD5686_REGMAP,
+};
+EXPORT_SYMBOL_NS_GPL(ad5686_chip_info, "IIO_AD5686");
+
+const struct ad5686_chip_info ad5686r_chip_info = {
+	.channels = ad5686r_channels,
+	.int_vref_mv = 2500,
+	.num_channels = 4,
+	.regmap_type = AD5686_REGMAP,
+};
+EXPORT_SYMBOL_NS_GPL(ad5686r_chip_info, "IIO_AD5686");
+
+const struct ad5686_chip_info ad5672r_chip_info = {
+	.channels = ad5672r_channels,
+	.int_vref_mv = 2500,
+	.num_channels = 8,
+	.regmap_type = AD5686_REGMAP,
+};
+EXPORT_SYMBOL_NS_GPL(ad5672r_chip_info, "IIO_AD5686");
+
+const struct ad5686_chip_info ad5676_chip_info = {
+	.channels = ad5676r_channels,
+	.num_channels = 8,
+	.regmap_type = AD5686_REGMAP,
+};
+EXPORT_SYMBOL_NS_GPL(ad5676_chip_info, "IIO_AD5686");
+
+const struct ad5686_chip_info ad5676r_chip_info = {
+	.channels = ad5676r_channels,
+	.int_vref_mv = 2500,
+	.num_channels = 8,
+	.regmap_type = AD5686_REGMAP,
+};
+EXPORT_SYMBOL_NS_GPL(ad5676r_chip_info, "IIO_AD5686");
+
+const struct ad5686_chip_info ad5674r_chip_info = {
+	.channels = ad5674r_channels,
+	.int_vref_mv = 2500,
+	.num_channels = 16,
+	.regmap_type = AD5686_REGMAP,
+};
+EXPORT_SYMBOL_NS_GPL(ad5674r_chip_info, "IIO_AD5686");
+
+const struct ad5686_chip_info ad5679r_chip_info = {
+	.channels = ad5679r_channels,
+	.int_vref_mv = 2500,
+	.num_channels = 16,
+	.regmap_type = AD5686_REGMAP,
+};
+EXPORT_SYMBOL_NS_GPL(ad5679r_chip_info, "IIO_AD5686");
 
 int ad5686_probe(struct device *dev,
-		 enum ad5686_supported_device_ids chip_type,
+		 const struct ad5686_chip_info *chip_info,
 		 const char *name, ad5686_write_func write,
 		 ad5686_read_func read)
 {
@@ -488,8 +455,7 @@ int ad5686_probe(struct device *dev,
 	st->dev = dev;
 	st->write = write;
 	st->read = read;
-
-	st->chip_info = &ad5686_chip_info_tbl[chip_type];
+	st->chip_info = chip_info;
 
 	ret = devm_regulator_get_enable_read_voltage(dev, "vcc");
 	if (ret < 0 && ret != -ENODEV)
diff --git a/drivers/iio/dac/ad5686.h b/drivers/iio/dac/ad5686.h
index af15994c01ad..caadc7403da1 100644
--- a/drivers/iio/dac/ad5686.h
+++ b/drivers/iio/dac/ad5686.h
@@ -47,42 +47,6 @@
 #define AD5683_REF_BIT_MSK			BIT(12)
 #define AD5686_REF_BIT_MSK			BIT(0)
 
-/**
- * ad5686_supported_device_ids:
- */
-enum ad5686_supported_device_ids {
-	ID_AD5310R,
-	ID_AD5311R,
-	ID_AD5337R,
-	ID_AD5338R,
-	ID_AD5671R,
-	ID_AD5672R,
-	ID_AD5673R,
-	ID_AD5674R,
-	ID_AD5675R,
-	ID_AD5676,
-	ID_AD5676R,
-	ID_AD5677R,
-	ID_AD5679R,
-	ID_AD5681R,
-	ID_AD5682R,
-	ID_AD5683,
-	ID_AD5683R,
-	ID_AD5684,
-	ID_AD5684R,
-	ID_AD5685R,
-	ID_AD5686,
-	ID_AD5686R,
-	ID_AD5691R,
-	ID_AD5692R,
-	ID_AD5693,
-	ID_AD5693R,
-	ID_AD5694,
-	ID_AD5694R,
-	ID_AD5695R,
-	ID_AD5696,
-	ID_AD5696R,
-};
 
 enum ad5686_regmap_type {
 	AD5310_REGMAP,
@@ -112,6 +76,34 @@ struct ad5686_chip_info {
 	enum ad5686_regmap_type		regmap_type;
 };
 
+/* single-channel instances */
+extern const struct ad5686_chip_info ad5310r_chip_info;
+extern const struct ad5686_chip_info ad5311r_chip_info;
+extern const struct ad5686_chip_info ad5681r_chip_info;
+extern const struct ad5686_chip_info ad5682r_chip_info;
+extern const struct ad5686_chip_info ad5683_chip_info;
+extern const struct ad5686_chip_info ad5683r_chip_info;
+
+/* dual-channel instances */
+extern const struct ad5686_chip_info ad5337r_chip_info;
+extern const struct ad5686_chip_info ad5338r_chip_info;
+
+/* quad-channel instances */
+extern const struct ad5686_chip_info ad5684_chip_info;
+extern const struct ad5686_chip_info ad5684r_chip_info;
+extern const struct ad5686_chip_info ad5685r_chip_info;
+extern const struct ad5686_chip_info ad5686_chip_info;
+extern const struct ad5686_chip_info ad5686r_chip_info;
+
+/* 8-channel instances */
+extern const struct ad5686_chip_info ad5672r_chip_info;
+extern const struct ad5686_chip_info ad5676_chip_info;
+extern const struct ad5686_chip_info ad5676r_chip_info;
+
+/* 16-channel instances */
+extern const struct ad5686_chip_info ad5674r_chip_info;
+extern const struct ad5686_chip_info ad5679r_chip_info;
+
 /**
  * struct ad5686_state - driver instance specific data
  * @spi:		spi_device
@@ -149,7 +141,7 @@ struct ad5686_state {
 
 
 int ad5686_probe(struct device *dev,
-		 enum ad5686_supported_device_ids chip_type,
+		 const struct ad5686_chip_info *chip_info,
 		 const char *name, ad5686_write_func write,
 		 ad5686_read_func read);
 
diff --git a/drivers/iio/dac/ad5696-i2c.c b/drivers/iio/dac/ad5696-i2c.c
index 20f04b74d831..c1f3cebabaf8 100644
--- a/drivers/iio/dac/ad5696-i2c.c
+++ b/drivers/iio/dac/ad5696-i2c.c
@@ -64,47 +64,46 @@ static int ad5686_i2c_write(struct ad5686_state *st,
 
 static int ad5686_i2c_probe(struct i2c_client *i2c)
 {
-	const struct i2c_device_id *id = i2c_client_get_device_id(i2c);
-	return ad5686_probe(&i2c->dev, id->driver_data, id->name,
-			    ad5686_i2c_write, ad5686_i2c_read);
+	return ad5686_probe(&i2c->dev, i2c_get_match_data(i2c),
+			    i2c->name, ad5686_i2c_write, ad5686_i2c_read);
 }
 
 static const struct i2c_device_id ad5686_i2c_id[] = {
-	{"ad5311r", ID_AD5311R},
-	{"ad5337r", ID_AD5337R},
-	{"ad5338r", ID_AD5338R},
-	{"ad5671r", ID_AD5671R},
-	{"ad5673r", ID_AD5673R},
-	{"ad5675r", ID_AD5675R},
-	{"ad5677r", ID_AD5677R},
-	{"ad5691r", ID_AD5691R},
-	{"ad5692r", ID_AD5692R},
-	{"ad5693", ID_AD5693},
-	{"ad5693r", ID_AD5693R},
-	{"ad5694", ID_AD5694},
-	{"ad5694r", ID_AD5694R},
-	{"ad5695r", ID_AD5695R},
-	{"ad5696", ID_AD5696},
-	{"ad5696r", ID_AD5696R},
+	{ .name = "ad5311r", .driver_data = (kernel_ulong_t)&ad5311r_chip_info },
+	{ .name = "ad5337r", .driver_data = (kernel_ulong_t)&ad5337r_chip_info },
+	{ .name = "ad5338r", .driver_data = (kernel_ulong_t)&ad5338r_chip_info },
+	{ .name = "ad5671r", .driver_data = (kernel_ulong_t)&ad5672r_chip_info },
+	{ .name = "ad5673r", .driver_data = (kernel_ulong_t)&ad5674r_chip_info },
+	{ .name = "ad5675r", .driver_data = (kernel_ulong_t)&ad5676r_chip_info },
+	{ .name = "ad5677r", .driver_data = (kernel_ulong_t)&ad5679r_chip_info },
+	{ .name = "ad5691r", .driver_data = (kernel_ulong_t)&ad5681r_chip_info },
+	{ .name = "ad5692r", .driver_data = (kernel_ulong_t)&ad5682r_chip_info },
+	{ .name = "ad5693",  .driver_data = (kernel_ulong_t)&ad5683_chip_info },
+	{ .name = "ad5693r", .driver_data = (kernel_ulong_t)&ad5683r_chip_info },
+	{ .name = "ad5694",  .driver_data = (kernel_ulong_t)&ad5684_chip_info },
+	{ .name = "ad5694r", .driver_data = (kernel_ulong_t)&ad5684r_chip_info },
+	{ .name = "ad5695r", .driver_data = (kernel_ulong_t)&ad5685r_chip_info },
+	{ .name = "ad5696",  .driver_data = (kernel_ulong_t)&ad5686_chip_info },
+	{ .name = "ad5696r", .driver_data = (kernel_ulong_t)&ad5686r_chip_info },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, ad5686_i2c_id);
 
 static const struct of_device_id ad5686_of_match[] = {
-	{ .compatible = "adi,ad5311r" },
-	{ .compatible = "adi,ad5337r" },
-	{ .compatible = "adi,ad5338r" },
-	{ .compatible = "adi,ad5671r" },
-	{ .compatible = "adi,ad5675r" },
-	{ .compatible = "adi,ad5691r" },
-	{ .compatible = "adi,ad5692r" },
-	{ .compatible = "adi,ad5693" },
-	{ .compatible = "adi,ad5693r" },
-	{ .compatible = "adi,ad5694" },
-	{ .compatible = "adi,ad5694r" },
-	{ .compatible = "adi,ad5695r" },
-	{ .compatible = "adi,ad5696" },
-	{ .compatible = "adi,ad5696r" },
+	{ .compatible = "adi,ad5311r", .data = &ad5311r_chip_info },
+	{ .compatible = "adi,ad5337r", .data = &ad5337r_chip_info },
+	{ .compatible = "adi,ad5338r", .data = &ad5338r_chip_info },
+	{ .compatible = "adi,ad5671r", .data = &ad5672r_chip_info },
+	{ .compatible = "adi,ad5675r", .data = &ad5676r_chip_info },
+	{ .compatible = "adi,ad5691r", .data = &ad5681r_chip_info },
+	{ .compatible = "adi,ad5692r", .data = &ad5682r_chip_info },
+	{ .compatible = "adi,ad5693",  .data = &ad5683_chip_info },
+	{ .compatible = "adi,ad5693r", .data = &ad5683r_chip_info },
+	{ .compatible = "adi,ad5694",  .data = &ad5684_chip_info },
+	{ .compatible = "adi,ad5694r", .data = &ad5684r_chip_info },
+	{ .compatible = "adi,ad5695r", .data = &ad5685r_chip_info },
+	{ .compatible = "adi,ad5696",  .data = &ad5686_chip_info },
+	{ .compatible = "adi,ad5696r", .data = &ad5686r_chip_info },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, ad5686_of_match);

-- 
2.43.0



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v7 4/8] iio: dac: ad5686: add of_match table to the spi driver
  2026-05-24 10:17 [PATCH v7 0/8] Fixes and cleanups for the AD5686 IIO driver Rodrigo Alencar via B4 Relay
                   ` (2 preceding siblings ...)
  2026-05-24 10:17 ` [PATCH v7 3/8] iio: dac: ad5686: drop enum id Rodrigo Alencar via B4 Relay
@ 2026-05-24 10:17 ` Rodrigo Alencar via B4 Relay
  2026-05-24 10:17 ` [PATCH v7 5/8] iio: dac: ad5686: add helpers to handle powerdown masks Rodrigo Alencar via B4 Relay
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Rodrigo Alencar via B4 Relay @ 2026-05-24 10:17 UTC (permalink / raw)
  To: linux-iio, linux-kernel, Stefan Popa, Jonathan Cameron,
	Greg Kroah-Hartman, Michael Auchter, Jonathan Cameron
  Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	David Lechner, Andy Shevchenko, Rodrigo Alencar, Andy Shevchenko

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

Add of_match table for the SPI device variants to be consistent with the
AD5696 I2C driver.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
 drivers/iio/dac/ad5686-spi.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/iio/dac/ad5686-spi.c b/drivers/iio/dac/ad5686-spi.c
index 3a2eafedce92..7eddffe86c00 100644
--- a/drivers/iio/dac/ad5686-spi.c
+++ b/drivers/iio/dac/ad5686-spi.c
@@ -119,9 +119,30 @@ static const struct spi_device_id ad5686_spi_id[] = {
 };
 MODULE_DEVICE_TABLE(spi, ad5686_spi_id);
 
+static const struct of_device_id ad5686_of_match[] = {
+	{ .compatible = "adi,ad5310r", .data = &ad5310r_chip_info },
+	{ .compatible = "adi,ad5672r", .data = &ad5672r_chip_info },
+	{ .compatible = "adi,ad5674r", .data = &ad5674r_chip_info },
+	{ .compatible = "adi,ad5676",  .data = &ad5676_chip_info },
+	{ .compatible = "adi,ad5676r", .data = &ad5676r_chip_info },
+	{ .compatible = "adi,ad5679r", .data = &ad5679r_chip_info },
+	{ .compatible = "adi,ad5681r", .data = &ad5681r_chip_info },
+	{ .compatible = "adi,ad5682r", .data = &ad5682r_chip_info },
+	{ .compatible = "adi,ad5683",  .data = &ad5683_chip_info },
+	{ .compatible = "adi,ad5683r", .data = &ad5683r_chip_info },
+	{ .compatible = "adi,ad5684",  .data = &ad5684_chip_info },
+	{ .compatible = "adi,ad5684r", .data = &ad5684r_chip_info },
+	{ .compatible = "adi,ad5685r", .data = &ad5685r_chip_info },
+	{ .compatible = "adi,ad5686",  .data = &ad5686_chip_info },
+	{ .compatible = "adi,ad5686r", .data = &ad5686r_chip_info },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ad5686_of_match);
+
 static struct spi_driver ad5686_spi_driver = {
 	.driver = {
 		.name = "ad5686",
+		.of_match_table = ad5686_of_match,
 	},
 	.probe = ad5686_spi_probe,
 	.id_table = ad5686_spi_id,

-- 
2.43.0



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v7 5/8] iio: dac: ad5686: add helpers to handle powerdown masks
  2026-05-24 10:17 [PATCH v7 0/8] Fixes and cleanups for the AD5686 IIO driver Rodrigo Alencar via B4 Relay
                   ` (3 preceding siblings ...)
  2026-05-24 10:17 ` [PATCH v7 4/8] iio: dac: ad5686: add of_match table to the spi driver Rodrigo Alencar via B4 Relay
@ 2026-05-24 10:17 ` Rodrigo Alencar via B4 Relay
  2026-05-24 10:17 ` [PATCH v7 6/8] iio: dac: ad5686: add control_sync() for single-channel devices Rodrigo Alencar via B4 Relay
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Rodrigo Alencar via B4 Relay @ 2026-05-24 10:17 UTC (permalink / raw)
  To: linux-iio, linux-kernel, Stefan Popa, Jonathan Cameron,
	Greg Kroah-Hartman, Michael Auchter, Jonathan Cameron
  Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	David Lechner, Andy Shevchenko, Rodrigo Alencar

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

Add ad5686_pd_field_set() and ad5686_pd_field_get() helpers to cleanup
powerdown mask control. Define AD5686_PD_* constants, e.g. AD5686_PD_MSK
to hold powerdown mask value for a single channel. AD5686_LDAC_PWRDN_*
macros are replaced by AD5686_PD_MODE_*, because they are unused and the
LDAC feature for async load of DAC channel values is not related to power
down control.

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

diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
index 50d4d5e8acf8..f024a7c05ddb 100644
--- a/drivers/iio/dac/ad5686.c
+++ b/drivers/iio/dac/ad5686.c
@@ -33,28 +33,40 @@ static inline unsigned int ad5686_pd_mask_shift(const struct iio_chan_spec *chan
 	return __ffs(chan->address) * 2;
 }
 
+static inline void ad5686_pd_field_set(const struct iio_chan_spec *chan,
+				       unsigned int *pd, unsigned int val)
+{
+	unsigned int shift = ad5686_pd_mask_shift(chan);
+
+	*pd = (*pd & ~(AD5686_PD_MSK << shift)) | ((val & AD5686_PD_MSK) << shift);
+}
+
+static inline unsigned int ad5686_pd_field_get(const struct iio_chan_spec *chan,
+					       unsigned int pd)
+{
+	unsigned int shift = ad5686_pd_mask_shift(chan);
+
+	return (pd >> shift) & AD5686_PD_MSK;
+}
+
 static int ad5686_get_powerdown_mode(struct iio_dev *indio_dev,
 				     const struct iio_chan_spec *chan)
 {
-	unsigned int shift = ad5686_pd_mask_shift(chan);
 	struct ad5686_state *st = iio_priv(indio_dev);
 
 	guard(mutex)(&st->lock);
 
-	return ((st->pwr_down_mode >> shift) & 0x3U) - 1;
+	return ad5686_pd_field_get(chan, st->pwr_down_mode) - 1;
 }
 
 static int ad5686_set_powerdown_mode(struct iio_dev *indio_dev,
 				     const struct iio_chan_spec *chan,
 				     unsigned int mode)
 {
-	unsigned int shift = ad5686_pd_mask_shift(chan);
 	struct ad5686_state *st = iio_priv(indio_dev);
 
 	guard(mutex)(&st->lock);
-
-	st->pwr_down_mode &= ~(0x3U << shift);
-	st->pwr_down_mode |= (mode + 1) << shift;
+	ad5686_pd_field_set(chan, &st->pwr_down_mode, mode + 1);
 
 	return 0;
 }
@@ -69,12 +81,12 @@ static const struct iio_enum ad5686_powerdown_mode_enum = {
 static ssize_t ad5686_read_dac_powerdown(struct iio_dev *indio_dev,
 		uintptr_t private, const struct iio_chan_spec *chan, char *buf)
 {
-	unsigned int shift = ad5686_pd_mask_shift(chan);
 	struct ad5686_state *st = iio_priv(indio_dev);
 
 	guard(mutex)(&st->lock);
 
-	return sysfs_emit(buf, "%d\n", !!(st->pwr_down_mask & (0x3U << shift)));
+	return sysfs_emit(buf, "%d\n",
+			  !!ad5686_pd_field_get(chan, st->pwr_down_mask));
 }
 
 static ssize_t ad5686_write_dac_powerdown(struct iio_dev *indio_dev,
@@ -96,9 +108,9 @@ static ssize_t ad5686_write_dac_powerdown(struct iio_dev *indio_dev,
 	guard(mutex)(&st->lock);
 
 	if (readin)
-		st->pwr_down_mask |= 0x3U << ad5686_pd_mask_shift(chan);
+		ad5686_pd_field_set(chan, &st->pwr_down_mask, AD5686_PD_MSK_PWR_DOWN);
 	else
-		st->pwr_down_mask &= ~(0x3U << ad5686_pd_mask_shift(chan));
+		ad5686_pd_field_set(chan, &st->pwr_down_mask, AD5686_PD_MSK_PWR_UP);
 
 	switch (st->chip_info->regmap_type) {
 	case AD5310_REGMAP:
@@ -471,10 +483,10 @@ int ad5686_probe(struct device *dev,
 
 	/* Set all the power down mode for all channels to 1K pulldown */
 	for (i = 0; i < st->chip_info->num_channels; i++) {
-		shift = ad5686_pd_mask_shift(&st->chip_info->channels[i]);
-		st->pwr_down_mask &= ~(0x3U << shift); /* powered up state */
-		st->pwr_down_mode &= ~(0x3U << shift);
-		st->pwr_down_mode |= 0x01U << shift;
+		ad5686_pd_field_set(&st->chip_info->channels[i],
+				    &st->pwr_down_mask, AD5686_PD_MSK_PWR_UP);
+		ad5686_pd_field_set(&st->chip_info->channels[i],
+				    &st->pwr_down_mode, AD5686_PD_MODE_1K_TO_GND);
 	}
 
 	indio_dev->name = name;
diff --git a/drivers/iio/dac/ad5686.h b/drivers/iio/dac/ad5686.h
index caadc7403da1..176d41966985 100644
--- a/drivers/iio/dac/ad5686.h
+++ b/drivers/iio/dac/ad5686.h
@@ -35,11 +35,6 @@
 #define AD5686_CMD_DAISY_CHAIN_ENABLE		0x8
 #define AD5686_CMD_READBACK_ENABLE		0x9
 
-#define AD5686_LDAC_PWRDN_NONE			0x0
-#define AD5686_LDAC_PWRDN_1K			0x1
-#define AD5686_LDAC_PWRDN_100K			0x2
-#define AD5686_LDAC_PWRDN_3STATE		0x3
-
 #define AD5686_CMD_CONTROL_REG			0x4
 #define AD5686_CMD_READBACK_ENABLE_V2		0x5
 
@@ -47,6 +42,14 @@
 #define AD5683_REF_BIT_MSK			BIT(12)
 #define AD5686_REF_BIT_MSK			BIT(0)
 
+#define AD5686_PD_MSK				GENMASK(1, 0)
+
+#define AD5686_PD_MODE_1K_TO_GND		0x1
+#define AD5686_PD_MODE_100K_TO_GND		0x2
+#define AD5686_PD_MODE_THREE_STATE		0x3
+
+#define AD5686_PD_MSK_PWR_UP			0x0
+#define AD5686_PD_MSK_PWR_DOWN			0x3
 
 enum ad5686_regmap_type {
 	AD5310_REGMAP,

-- 
2.43.0



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v7 6/8] iio: dac: ad5686: add control_sync() for single-channel devices
  2026-05-24 10:17 [PATCH v7 0/8] Fixes and cleanups for the AD5686 IIO driver Rodrigo Alencar via B4 Relay
                   ` (4 preceding siblings ...)
  2026-05-24 10:17 ` [PATCH v7 5/8] iio: dac: ad5686: add helpers to handle powerdown masks Rodrigo Alencar via B4 Relay
@ 2026-05-24 10:17 ` Rodrigo Alencar via B4 Relay
  2026-05-24 10:17 ` [PATCH v7 7/8] iio: dac: ad5686: cleanup doc header of local structs Rodrigo Alencar via B4 Relay
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Rodrigo Alencar via B4 Relay @ 2026-05-24 10:17 UTC (permalink / raw)
  To: linux-iio, linux-kernel, Stefan Popa, Jonathan Cameron,
	Greg Kroah-Hartman, Michael Auchter, Jonathan Cameron
  Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	David Lechner, Andy Shevchenko, Rodrigo Alencar

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

Create ad5310_control_sync() and ad5683_control_sync() functions that
properly consume the mask definitions with FIELD_PREP(). This allows to
reuse a function that updates the control register with cached values,
without relying on confusing logic that depends on st->use_internal_vref,
which is initialized earlier in ad5686_probe() because it is also
applicable to the AD5686_REGMAP case, removing the need for the
has_external_vref. Powerdown masks initialization is simplified as
*_control_sync() masks outs any unused bits for the single-channel case.
The change cleans up ad5686_write_dac_powerdown() and ad5686_probe(),
organizing the code for feature extension, e.g. gain control support for
single-channel devices.

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

diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
index f024a7c05ddb..98a027224b05 100644
--- a/drivers/iio/dac/ad5686.c
+++ b/drivers/iio/dac/ad5686.c
@@ -6,6 +6,7 @@
  */
 
 #include <linux/array_size.h>
+#include <linux/bitfield.h>
 #include <linux/bitops.h>
 #include <linux/errno.h>
 #include <linux/export.h>
@@ -13,6 +14,7 @@
 #include <linux/module.h>
 #include <linux/regulator/consumer.h>
 #include <linux/sysfs.h>
+#include <linux/wordpart.h>
 
 #include <linux/iio/iio.h>
 
@@ -24,6 +26,24 @@ static const char * const ad5686_powerdown_modes[] = {
 	"three_state"
 };
 
+static int ad5310_control_sync(struct ad5686_state *st)
+{
+	unsigned int pd_val = st->pwr_down_mask & st->pwr_down_mode;
+
+	return st->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));
+}
+
+static int ad5683_control_sync(struct ad5686_state *st)
+{
+	unsigned int pd_val = st->pwr_down_mask & st->pwr_down_mode;
+
+	return st->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));
+}
+
 static inline unsigned int ad5686_pd_mask_shift(const struct iio_chan_spec *chan)
 {
 	if (chan->channel == chan->address)
@@ -98,8 +118,8 @@ static ssize_t ad5686_write_dac_powerdown(struct iio_dev *indio_dev,
 	bool readin;
 	int ret;
 	struct ad5686_state *st = iio_priv(indio_dev);
-	unsigned int val, ref_bit_msk;
-	u8 shift, address = 0;
+	unsigned int val;
+	u8 address;
 
 	ret = kstrtobool(buf, &readin);
 	if (ret)
@@ -114,32 +134,34 @@ static ssize_t ad5686_write_dac_powerdown(struct iio_dev *indio_dev,
 
 	switch (st->chip_info->regmap_type) {
 	case AD5310_REGMAP:
-		shift = 9;
-		ref_bit_msk = AD5310_REF_BIT_MSK;
+		ret = ad5310_control_sync(st);
+		if (ret)
+			return ret;
 		break;
 	case AD5683_REGMAP:
-		shift = 13;
-		ref_bit_msk = AD5683_REF_BIT_MSK;
+		ret = ad5683_control_sync(st);
+		if (ret)
+			return ret;
 		break;
 	case AD5686_REGMAP:
-		shift = 0;
-		ref_bit_msk = 0;
 		/* AD5674R/AD5679R have 16 channels and 2 powerdown registers */
-		if (chan->channel > 0x7)
+		val = st->pwr_down_mask & st->pwr_down_mode;
+		if (chan->channel > 0x7) {
 			address = 0x8;
+			val = upper_16_bits(val);
+		} else {
+			address = 0x0;
+			val = lower_16_bits(val);
+		}
+		ret = st->write(st, AD5686_CMD_POWERDOWN_DAC, address, val);
+		if (ret)
+			return ret;
 		break;
 	default:
 		return -EINVAL;
 	}
 
-	val = ((st->pwr_down_mask & st->pwr_down_mode) << shift);
-	if (!st->use_internal_vref)
-		val |= ref_bit_msk;
-
-	ret = st->write(st, AD5686_CMD_POWERDOWN_DAC,
-			address, val >> (address * 2));
-
-	return ret ? ret : len;
+	return len;
 }
 
 static int ad5686_read_raw(struct iio_dev *indio_dev,
@@ -453,9 +475,6 @@ int ad5686_probe(struct device *dev,
 {
 	struct ad5686_state *st;
 	struct iio_dev *indio_dev;
-	unsigned int val, ref_bit_msk, shift;
-	bool has_external_vref;
-	u8 cmd;
 	int ret, i;
 
 	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
@@ -473,13 +492,12 @@ int ad5686_probe(struct device *dev,
 	if (ret < 0 && ret != -ENODEV)
 		return ret;
 
-	has_external_vref = ret != -ENODEV;
-	st->vref_mv = has_external_vref ? ret / 1000 : st->chip_info->int_vref_mv;
+	st->use_internal_vref = ret == -ENODEV;
+	st->vref_mv = st->use_internal_vref ? st->chip_info->int_vref_mv : ret / 1000;
 
-	/* Initialize masks to all ones provided the max shift (last channel) */
-	shift = ad5686_pd_mask_shift(&st->chip_info->channels[st->chip_info->num_channels - 1]);
-	st->pwr_down_mask = GENMASK(shift + 1, 0);
-	st->pwr_down_mode = GENMASK(shift + 1, 0);
+	/* Initialize masks to all ones */
+	st->pwr_down_mask = ~0;
+	st->pwr_down_mode = ~0;
 
 	/* Set all the power down mode for all channels to 1K pulldown */
 	for (i = 0; i < st->chip_info->num_channels; i++) {
@@ -499,29 +517,25 @@ int ad5686_probe(struct device *dev,
 
 	switch (st->chip_info->regmap_type) {
 	case AD5310_REGMAP:
-		cmd = AD5686_CMD_CONTROL_REG;
-		ref_bit_msk = AD5310_REF_BIT_MSK;
-		st->use_internal_vref = !has_external_vref;
+		ret = ad5310_control_sync(st);
+		if (ret)
+			return ret;
 		break;
 	case AD5683_REGMAP:
-		cmd = AD5686_CMD_CONTROL_REG;
-		ref_bit_msk = AD5683_REF_BIT_MSK;
-		st->use_internal_vref = !has_external_vref;
+		ret = ad5683_control_sync(st);
+		if (ret)
+			return ret;
 		break;
 	case AD5686_REGMAP:
-		cmd = AD5686_CMD_INTERNAL_REFER_SETUP;
-		ref_bit_msk = AD5686_REF_BIT_MSK;
+		ret = st->write(st, AD5686_CMD_INTERNAL_REFER_SETUP, 0,
+				st->use_internal_vref ? 0 : AD5686_REF_BIT_MSK);
+		if (ret)
+			return ret;
 		break;
 	default:
 		return -EINVAL;
 	}
 
-	val = has_external_vref ? ref_bit_msk : 0;
-
-	ret = st->write(st, cmd, 0, val);
-	if (ret)
-		return ret;
-
 	return devm_iio_device_register(dev, indio_dev);
 }
 EXPORT_SYMBOL_NS_GPL(ad5686_probe, "IIO_AD5686");
diff --git a/drivers/iio/dac/ad5686.h b/drivers/iio/dac/ad5686.h
index 176d41966985..17980c54839c 100644
--- a/drivers/iio/dac/ad5686.h
+++ b/drivers/iio/dac/ad5686.h
@@ -39,9 +39,12 @@
 #define AD5686_CMD_READBACK_ENABLE_V2		0x5
 
 #define AD5310_REF_BIT_MSK			BIT(8)
-#define AD5683_REF_BIT_MSK			BIT(12)
-#define AD5686_REF_BIT_MSK			BIT(0)
+#define AD5310_PD_MSK				GENMASK(10, 9)
 
+#define AD5683_REF_BIT_MSK			BIT(12)
+#define AD5683_PD_MSK				GENMASK(14, 13)
+
+#define AD5686_REF_BIT_MSK			BIT(0)
 #define AD5686_PD_MSK				GENMASK(1, 0)
 
 #define AD5686_PD_MODE_1K_TO_GND		0x1

-- 
2.43.0



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v7 7/8] iio: dac: ad5686: cleanup doc header of local structs
  2026-05-24 10:17 [PATCH v7 0/8] Fixes and cleanups for the AD5686 IIO driver Rodrigo Alencar via B4 Relay
                   ` (5 preceding siblings ...)
  2026-05-24 10:17 ` [PATCH v7 6/8] iio: dac: ad5686: add control_sync() for single-channel devices Rodrigo Alencar via B4 Relay
@ 2026-05-24 10:17 ` Rodrigo Alencar via B4 Relay
  2026-05-24 10:17 ` [PATCH v7 8/8] iio: dac: ad5686: create bus ops struct Rodrigo Alencar via B4 Relay
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Rodrigo Alencar via B4 Relay @ 2026-05-24 10:17 UTC (permalink / raw)
  To: linux-iio, linux-kernel, Stefan Popa, Jonathan Cameron,
	Greg Kroah-Hartman, Michael Auchter, Jonathan Cameron
  Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	David Lechner, Andy Shevchenko, Rodrigo Alencar, Andy Shevchenko

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

Review documentation comment header for ad5686_chip_info and ad5686_state.
Update variable names and description and remove unnecessary blank line
between comment and struct declaration.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
 drivers/iio/dac/ad5686.h | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/dac/ad5686.h b/drivers/iio/dac/ad5686.h
index 17980c54839c..3945f5fb6b7e 100644
--- a/drivers/iio/dac/ad5686.h
+++ b/drivers/iio/dac/ad5686.h
@@ -69,12 +69,11 @@ typedef int (*ad5686_read_func)(struct ad5686_state *st, u8 addr);
 
 /**
  * struct ad5686_chip_info - chip specific information
- * @int_vref_mv:	AD5620/40/60: the internal reference voltage
+ * @int_vref_mv:	the internal reference voltage
  * @num_channels:	number of channels
  * @channel:		channel specification
  * @regmap_type:	register map layout variant
  */
-
 struct ad5686_chip_info {
 	u16				int_vref_mv;
 	unsigned int			num_channels;
@@ -112,16 +111,16 @@ extern const struct ad5686_chip_info ad5679r_chip_info;
 
 /**
  * struct ad5686_state - driver instance specific data
- * @spi:		spi_device
+ * @dev:		device instance
  * @chip_info:		chip model specific constants, available modes etc
  * @vref_mv:		actual reference voltage used
  * @pwr_down_mask:	power down mask
  * @pwr_down_mode:	current power down mode
  * @use_internal_vref:	set to true if the internal reference voltage is used
- * @lock		lock to protect the data buffer during regmap ops
- * @data:		spi transfer buffers
+ * @lock:		lock to protect access to state fields, which includes
+ *			the data buffer during regmap ops
+ * @data:		transfer buffers
  */
-
 struct ad5686_state {
 	struct device			*dev;
 	const struct ad5686_chip_info	*chip_info;

-- 
2.43.0



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v7 8/8] iio: dac: ad5686: create bus ops struct
  2026-05-24 10:17 [PATCH v7 0/8] Fixes and cleanups for the AD5686 IIO driver Rodrigo Alencar via B4 Relay
                   ` (6 preceding siblings ...)
  2026-05-24 10:17 ` [PATCH v7 7/8] iio: dac: ad5686: cleanup doc header of local structs Rodrigo Alencar via B4 Relay
@ 2026-05-24 10:17 ` Rodrigo Alencar via B4 Relay
  2026-05-26 15:34 ` [PATCH v7 0/8] Fixes and cleanups for the AD5686 IIO driver Jonathan Cameron
  2026-06-02 13:35 ` Jonathan Cameron
  9 siblings, 0 replies; 12+ messages in thread
From: Rodrigo Alencar via B4 Relay @ 2026-05-24 10:17 UTC (permalink / raw)
  To: linux-iio, linux-kernel, Stefan Popa, Jonathan Cameron,
	Greg Kroah-Hartman, Michael Auchter, Jonathan Cameron
  Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	David Lechner, Andy Shevchenko, Rodrigo Alencar, Andy Shevchenko

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

Create struct with bus operations, which will be used to extend bus
implementation features. Auxiliary functions ad5686_write() and
ad5686_read() are created and ad5686_probe() now receives an ops struct
pointer rather than individual read and write functions.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
 drivers/iio/dac/ad5686-spi.c |  7 ++++++-
 drivers/iio/dac/ad5686.c     | 32 ++++++++++++++------------------
 drivers/iio/dac/ad5686.h     | 29 +++++++++++++++++++++--------
 drivers/iio/dac/ad5696-i2c.c |  7 ++++++-
 4 files changed, 47 insertions(+), 28 deletions(-)

diff --git a/drivers/iio/dac/ad5686-spi.c b/drivers/iio/dac/ad5686-spi.c
index 7eddffe86c00..6b6ef1d7071f 100644
--- a/drivers/iio/dac/ad5686-spi.c
+++ b/drivers/iio/dac/ad5686-spi.c
@@ -92,10 +92,15 @@ static int ad5686_spi_read(struct ad5686_state *st, u8 addr)
 	return be32_to_cpu(st->data[2].d32);
 }
 
+static const struct ad5686_bus_ops ad5686_spi_ops = {
+	.write = ad5686_spi_write,
+	.read = ad5686_spi_read,
+};
+
 static int ad5686_spi_probe(struct spi_device *spi)
 {
 	return ad5686_probe(&spi->dev, spi_get_device_match_data(spi),
-			    spi->modalias, ad5686_spi_write, ad5686_spi_read);
+			    spi->modalias, &ad5686_spi_ops);
 }
 
 static const struct spi_device_id ad5686_spi_id[] = {
diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
index 98a027224b05..337ebad39ab4 100644
--- a/drivers/iio/dac/ad5686.c
+++ b/drivers/iio/dac/ad5686.c
@@ -30,18 +30,18 @@ static int ad5310_control_sync(struct ad5686_state *st)
 {
 	unsigned int pd_val = st->pwr_down_mask & st->pwr_down_mode;
 
-	return st->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));
+	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));
 }
 
 static int ad5683_control_sync(struct ad5686_state *st)
 {
 	unsigned int pd_val = st->pwr_down_mask & st->pwr_down_mode;
 
-	return st->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));
+	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));
 }
 
 static inline unsigned int ad5686_pd_mask_shift(const struct iio_chan_spec *chan)
@@ -153,7 +153,7 @@ static ssize_t ad5686_write_dac_powerdown(struct iio_dev *indio_dev,
 			address = 0x0;
 			val = lower_16_bits(val);
 		}
-		ret = st->write(st, AD5686_CMD_POWERDOWN_DAC, address, val);
+		ret = ad5686_write(st, AD5686_CMD_POWERDOWN_DAC, address, val);
 		if (ret)
 			return ret;
 		break;
@@ -176,7 +176,7 @@ static int ad5686_read_raw(struct iio_dev *indio_dev,
 	switch (m) {
 	case IIO_CHAN_INFO_RAW:
 		mutex_lock(&st->lock);
-		ret = st->read(st, chan->address);
+		ret = ad5686_read(st, chan->address);
 		mutex_unlock(&st->lock);
 		if (ret < 0)
 			return ret;
@@ -206,10 +206,8 @@ static int ad5686_write_raw(struct iio_dev *indio_dev,
 			return -EINVAL;
 
 		mutex_lock(&st->lock);
-		ret = st->write(st,
-				AD5686_CMD_WRITE_INPUT_N_UPDATE_N,
-				chan->address,
-				val << chan->scan_type.shift);
+		ret = ad5686_write(st, AD5686_CMD_WRITE_INPUT_N_UPDATE_N,
+				   chan->address, val << chan->scan_type.shift);
 		mutex_unlock(&st->lock);
 		break;
 	default:
@@ -470,8 +468,7 @@ 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, ad5686_write_func write,
-		 ad5686_read_func read)
+		 const char *name, const struct ad5686_bus_ops *ops)
 {
 	struct ad5686_state *st;
 	struct iio_dev *indio_dev;
@@ -484,8 +481,7 @@ int ad5686_probe(struct device *dev,
 	st = iio_priv(indio_dev);
 
 	st->dev = dev;
-	st->write = write;
-	st->read = read;
+	st->ops = ops;
 	st->chip_info = chip_info;
 
 	ret = devm_regulator_get_enable_read_voltage(dev, "vcc");
@@ -527,8 +523,8 @@ int ad5686_probe(struct device *dev,
 			return ret;
 		break;
 	case AD5686_REGMAP:
-		ret = st->write(st, AD5686_CMD_INTERNAL_REFER_SETUP, 0,
-				st->use_internal_vref ? 0 : AD5686_REF_BIT_MSK);
+		ret = ad5686_write(st, AD5686_CMD_INTERNAL_REFER_SETUP, 0,
+				   st->use_internal_vref ? 0 : AD5686_REF_BIT_MSK);
 		if (ret)
 			return ret;
 		break;
diff --git a/drivers/iio/dac/ad5686.h b/drivers/iio/dac/ad5686.h
index 3945f5fb6b7e..a06fe7d89305 100644
--- a/drivers/iio/dac/ad5686.h
+++ b/drivers/iio/dac/ad5686.h
@@ -62,10 +62,15 @@ enum ad5686_regmap_type {
 
 struct ad5686_state;
 
-typedef int (*ad5686_write_func)(struct ad5686_state *st,
-				 u8 cmd, u8 addr, u16 val);
-
-typedef int (*ad5686_read_func)(struct ad5686_state *st, u8 addr);
+/**
+ * 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
+ */
+struct ad5686_bus_ops {
+	int (*read)(struct ad5686_state *st, u8 addr);
+	int (*write)(struct ad5686_state *st, u8 cmd, u8 addr, u16 val);
+};
 
 /**
  * struct ad5686_chip_info - chip specific information
@@ -113,6 +118,7 @@ extern const struct ad5686_chip_info ad5679r_chip_info;
  * struct ad5686_state - driver instance specific data
  * @dev:		device instance
  * @chip_info:		chip model specific constants, available modes etc
+ * @ops:		bus specific operations
  * @vref_mv:		actual reference voltage used
  * @pwr_down_mask:	power down mask
  * @pwr_down_mode:	current power down mode
@@ -124,11 +130,10 @@ extern const struct ad5686_chip_info ad5679r_chip_info;
 struct ad5686_state {
 	struct device			*dev;
 	const struct ad5686_chip_info	*chip_info;
+	const struct ad5686_bus_ops	*ops;
 	unsigned short			vref_mv;
 	unsigned int			pwr_down_mask;
 	unsigned int			pwr_down_mode;
-	ad5686_write_func		write;
-	ad5686_read_func		read;
 	bool				use_internal_vref;
 	struct mutex			lock;
 
@@ -147,8 +152,16 @@ struct ad5686_state {
 
 int ad5686_probe(struct device *dev,
 		 const struct ad5686_chip_info *chip_info,
-		 const char *name, ad5686_write_func write,
-		 ad5686_read_func read);
+		 const char *name, const struct ad5686_bus_ops *ops);
 
+static inline int ad5686_write(struct ad5686_state *st, u8 cmd, u8 addr, u16 val)
+{
+	return st->ops->write(st, cmd, addr, val);
+}
+
+static inline int ad5686_read(struct ad5686_state *st, u8 addr)
+{
+	return st->ops->read(st, addr);
+}
 
 #endif /* __DRIVERS_IIO_DAC_AD5686_H__ */
diff --git a/drivers/iio/dac/ad5696-i2c.c b/drivers/iio/dac/ad5696-i2c.c
index c1f3cebabaf8..279309329b64 100644
--- a/drivers/iio/dac/ad5696-i2c.c
+++ b/drivers/iio/dac/ad5696-i2c.c
@@ -62,10 +62,15 @@ static int ad5686_i2c_write(struct ad5686_state *st,
 	return (ret != 3) ? -EIO : 0;
 }
 
+static const struct ad5686_bus_ops ad5686_i2c_ops = {
+	.write = ad5686_i2c_write,
+	.read = ad5686_i2c_read,
+};
+
 static int ad5686_i2c_probe(struct i2c_client *i2c)
 {
 	return ad5686_probe(&i2c->dev, i2c_get_match_data(i2c),
-			    i2c->name, ad5686_i2c_write, ad5686_i2c_read);
+			    i2c->name, &ad5686_i2c_ops);
 }
 
 static const struct i2c_device_id ad5686_i2c_id[] = {

-- 
2.43.0



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v7 0/8] Fixes and cleanups for the AD5686 IIO driver
  2026-05-24 10:17 [PATCH v7 0/8] Fixes and cleanups for the AD5686 IIO driver Rodrigo Alencar via B4 Relay
                   ` (7 preceding siblings ...)
  2026-05-24 10:17 ` [PATCH v7 8/8] iio: dac: ad5686: create bus ops struct Rodrigo Alencar via B4 Relay
@ 2026-05-26 15:34 ` Jonathan Cameron
  2026-06-02 13:35 ` Jonathan Cameron
  9 siblings, 0 replies; 12+ messages in thread
From: Jonathan Cameron @ 2026-05-26 15:34 UTC (permalink / raw)
  To: Rodrigo Alencar via B4 Relay
  Cc: rodrigo.alencar, linux-iio, linux-kernel, Stefan Popa,
	Jonathan Cameron, Greg Kroah-Hartman, Michael Auchter,
	Lars-Peter Clausen, Michael Hennerich, David Lechner,
	Andy Shevchenko, Andy Shevchenko

On Sun, 24 May 2026 11:17:00 +0100
Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote:

> This is the first series of three on updating the AD5686 driver.
> 
> A bigger patch series was sent before ("Extend device support for AD5686 driver"):
> 
> https://lore.kernel.org/r/20260422-ad5313r-iio-support-v1-0-ed7dca001d1b@analog.com
> 
> This one adds a number of cleanups and fixes, like:
> - Refactor include headers (IWYU);
> - Remove redundant register definition;
> - Drop enum chip id in favor of per-device chip_info structs;
> - Fix internal voltage reference control for single-channel devices;
> - Acquire lock when doing power down control;
> - Fix powerdown control for dual-channel devices;
> 
> Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
Series looks good to me but I think we are waiting on the fixes to be available
upstream - so might be a little while before I can apply it.

Thanks,

Jonathan

> ---
> Changes in v7:
> - Drop fixes (already accepted).
> - Use named fields in device tables.
> - Link to v6: https://lore.kernel.org/r/20260505-ad5686-fixes-v6-0-c2d5f7be32be@analog.com
> 
> Changes in v6:
> - Protect powerdown masks on read access too.
> - Minor changes to power down handling and constants.
> - Link to v5: https://lore.kernel.org/r/20260501-ad5686-fixes-v5-0-0b2f45488418@analog.com
> 
> Changes in v5:
> - Drop I2C read operation changes.
> - Create helpers for reading and writing powerdown mask bits
> - Link to v4: https://lore.kernel.org/r/20260429-ad5686-fixes-v4-0-bb8f1cbd68e1@analog.com
> 
> Changes in v4:
> - Address issues spotted by sashiko.
> - Link to v3: https://lore.kernel.org/r/20260428-ad5686-fixes-v3-0-9cff7bd67a15@analog.com
> 
> Changes in v3:
> - Misc changes like parenthesis removal and line breaks
> - Link to v2: https://lore.kernel.org/r/20260427-ad5686-fixes-v2-0-188e05199368@analog.com
> 
> Changes in v2:
> - Bring fixes first and cleanups later
> - Link to v1: https://lore.kernel.org/r/20260426-ad5686-fixes-v1-0-7c946a77794e@analog.com
> 
> ---
> Rodrigo Alencar (8):
>       iio: dac: ad5686: refactor include headers
>       iio: dac: ad5686: remove redundant register definition
>       iio: dac: ad5686: drop enum id
>       iio: dac: ad5686: add of_match table to the spi driver
>       iio: dac: ad5686: add helpers to handle powerdown masks
>       iio: dac: ad5686: add control_sync() for single-channel devices
>       iio: dac: ad5686: cleanup doc header of local structs
>       iio: dac: ad5686: create bus ops struct
> 
>  drivers/iio/dac/ad5686-spi.c |  73 ++++--
>  drivers/iio/dac/ad5686.c     | 528 +++++++++++++++++++++----------------------
>  drivers/iio/dac/ad5686.h     | 133 +++++------
>  drivers/iio/dac/ad5696-i2c.c |  80 ++++---
>  4 files changed, 418 insertions(+), 396 deletions(-)
> ---
> base-commit: bbc109ae655c8a5d4db117fea01947eb6bf51e71
> change-id: 20260426-ad5686-fixes-63ea68811bdb
> 
> Best regards,


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v7 3/8] iio: dac: ad5686: drop enum id
  2026-05-24 10:17 ` [PATCH v7 3/8] iio: dac: ad5686: drop enum id Rodrigo Alencar via B4 Relay
@ 2026-06-02 13:32   ` Jonathan Cameron
  0 siblings, 0 replies; 12+ messages in thread
From: Jonathan Cameron @ 2026-06-02 13:32 UTC (permalink / raw)
  To: Rodrigo Alencar via B4 Relay
  Cc: rodrigo.alencar, linux-iio, linux-kernel, Stefan Popa,
	Jonathan Cameron, Greg Kroah-Hartman, Michael Auchter,
	Lars-Peter Clausen, Michael Hennerich, David Lechner,
	Andy Shevchenko, Andy Shevchenko

On Sun, 24 May 2026 11:17:03 +0100
Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote:

> From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> 
> Split chip info table into separate structs and expose them to the spi
> i2c drivers. That is the preferrable approach and allows for the drivers
> to have knowledge of the device info before the common probe function gets
> called. Those chip info structs may be shared by SPI and I2C driver
> variants.
> Channel declaration definitions are grouped according to channel count and
> DECLARE_AD5693_CHANNELS() macro is renamed to DECLARE_AD5683_CHANNELS() to
> match the regmap_type enum.
> Use spi_get_device_match_data() and i2c_get_match_data() to get chip info
> struct reference, passing it as parameter to the core probe function.
> 
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
This clashed with Uwe's work on using named initializers for all
id tables.  Resolution was obvious given the problem was in the code
removed, not what you added.

I fixed it up.

J

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v7 0/8] Fixes and cleanups for the AD5686 IIO driver
  2026-05-24 10:17 [PATCH v7 0/8] Fixes and cleanups for the AD5686 IIO driver Rodrigo Alencar via B4 Relay
                   ` (8 preceding siblings ...)
  2026-05-26 15:34 ` [PATCH v7 0/8] Fixes and cleanups for the AD5686 IIO driver Jonathan Cameron
@ 2026-06-02 13:35 ` Jonathan Cameron
  9 siblings, 0 replies; 12+ messages in thread
From: Jonathan Cameron @ 2026-06-02 13:35 UTC (permalink / raw)
  To: Rodrigo Alencar via B4 Relay
  Cc: rodrigo.alencar, linux-iio, linux-kernel, Stefan Popa,
	Jonathan Cameron, Greg Kroah-Hartman, Michael Auchter,
	Lars-Peter Clausen, Michael Hennerich, David Lechner,
	Andy Shevchenko, Andy Shevchenko

On Sun, 24 May 2026 11:17:00 +0100
Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote:

> This is the first series of three on updating the AD5686 driver.
> 
> A bigger patch series was sent before ("Extend device support for AD5686 driver"):
> 
> https://lore.kernel.org/r/20260422-ad5313r-iio-support-v1-0-ed7dca001d1b@analog.com
> 
> This one adds a number of cleanups and fixes, like:
> - Refactor include headers (IWYU);
> - Remove redundant register definition;
> - Drop enum chip id in favor of per-device chip_info structs;
> - Fix internal voltage reference control for single-channel devices;
> - Acquire lock when doing power down control;
> - Fix powerdown control for dual-channel devices;
> 
> Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
I've just merged v7.1-rc6 into the testing branch which allows me to finally
apply this.  So applied to the testing branch of iio.git and pushed out for
the usual bots to take a look.

Thanks,

Jonathan

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2026-06-02 13:35 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-05-24 10:17 [PATCH v7 0/8] Fixes and cleanups for the AD5686 IIO driver Rodrigo Alencar via B4 Relay
2026-05-24 10:17 ` [PATCH v7 1/8] iio: dac: ad5686: refactor include headers Rodrigo Alencar via B4 Relay
2026-05-24 10:17 ` [PATCH v7 2/8] iio: dac: ad5686: remove redundant register definition Rodrigo Alencar via B4 Relay
2026-05-24 10:17 ` [PATCH v7 3/8] iio: dac: ad5686: drop enum id Rodrigo Alencar via B4 Relay
2026-06-02 13:32   ` Jonathan Cameron
2026-05-24 10:17 ` [PATCH v7 4/8] iio: dac: ad5686: add of_match table to the spi driver Rodrigo Alencar via B4 Relay
2026-05-24 10:17 ` [PATCH v7 5/8] iio: dac: ad5686: add helpers to handle powerdown masks Rodrigo Alencar via B4 Relay
2026-05-24 10:17 ` [PATCH v7 6/8] iio: dac: ad5686: add control_sync() for single-channel devices Rodrigo Alencar via B4 Relay
2026-05-24 10:17 ` [PATCH v7 7/8] iio: dac: ad5686: cleanup doc header of local structs Rodrigo Alencar via B4 Relay
2026-05-24 10:17 ` [PATCH v7 8/8] iio: dac: ad5686: create bus ops struct Rodrigo Alencar via B4 Relay
2026-05-26 15:34 ` [PATCH v7 0/8] Fixes and cleanups for the AD5686 IIO driver Jonathan Cameron
2026-06-02 13:35 ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®