[PATCH v3 0/9] iio: ad3552r-hs: add support for ad3541/42r Angelo Dureghello

[PATCH v3 0/9] iio: ad3552r-hs: add support for ad3541/42r Angelo Dureghello

[Angelo Dureghello]

This patchset add support for ad3541/42r, 16MUPS, respectively
single and dual channel DACs, together with some minor fixes.

The ad354xr connect through a DSPI bus (while ad355xr through
QSPI), so a new fpga HDL supporting bus mode switch has been
developed to support them.

Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
Changes in v2:
- reproposing the patchset using platform data for bus mode
  change functionality,
- improve commit messages,
- add separate patch for instruction mode,
- add separate patch for sharing model data structures,
- remove error on wrong id,
- fix id detection info message in case of wrong id.

Changes in v3:
- improve commit messages,
- removed some not useful dev_err, 
- comment syntax fixes,
- add lock guards for bus mode change,
- remove externs for range tables.

Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>

---
Angelo Dureghello (9):
      iio: dac: ad3552r-common: fix ad3541/2r ranges
      iio: dac: ad3552r-hs: clear reset status flag
      iio: dac: adi-axi-dac: modify stream enable
      iio: dac: adi-axi-dac: add bus mode setup
      iio: dac: ad3552r-hs: fix message on wrong chip id
      iio: dac: ad3552r-hs: use instruction mode for configuration
      iio: dac: ad3552r: share model data structures
      iio: dac: ad3552r-hs: add ad3541/2r support
      iio: dac: ad3552r-hs: update function name (non functional)

 drivers/iio/dac/ad3552r-common.c |  55 +++++++-
 drivers/iio/dac/ad3552r-hs.c     | 296 ++++++++++++++++++++++++++++++---------
 drivers/iio/dac/ad3552r-hs.h     |   8 ++
 drivers/iio/dac/ad3552r.c        |  36 -----
 drivers/iio/dac/ad3552r.h        |  17 ++-
 drivers/iio/dac/adi-axi-dac.c    |  30 +++-
 6 files changed, 326 insertions(+), 116 deletions(-)
---
base-commit: 74022f59b0a17567c0d7bad8f197977d24d06525
change-id: 20250110-wip-bl-ad3552r-axi-v0-iio-testing-carlos-db63c474ca9e

Best regards,
-- 
Angelo Dureghello <adureghello@baylibre.com>

[PATCH v3 1/9] iio: dac: ad3552r-common: fix ad3541/2r ranges

[Angelo Dureghello]

From: Angelo Dureghello <adureghello@baylibre.com>

Fix ad3541/2r voltage ranges to be as per ad3542r datasheet,
rev. C, table 38 (page 57).

The wrong ad354xr ranges was generating erroneous Vpp output.

In more details:
- fix wrong number of ranges, they are 5 ranges, not 6,
- remove non-existent 0-3V range,
- adjust order, since ad3552r_find_range() get a wrong index,
  producing a wrong Vpp as output.

Retested all the ranges on real hardware, EVALAD3542RFMCZ:

adi,output-range-microvolt (fdt):
<(000000) (2500000)>;   ok (Rfbx1, switch 10)
<(000000) (5000000)>;   ok (Rfbx1, switch 10)
<(000000) (10000000)>;  ok (Rfbx1, switch 10)
<(-5000000) (5000000)>; ok (Rfbx2, switch +/- 5)
<(-2500000) (7500000)>; ok (Rfbx2, switch -2.5/7.5)

Fixes: 8f2b54824b28 ("drivers:iio:dac: Add AD3552R driver support")
Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
 drivers/iio/dac/ad3552r-common.c | 5 ++---
 drivers/iio/dac/ad3552r.h        | 8 +++-----
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/dac/ad3552r-common.c b/drivers/iio/dac/ad3552r-common.c
index 0f495df2e5ce..03e0864f5084 100644
--- a/drivers/iio/dac/ad3552r-common.c
+++ b/drivers/iio/dac/ad3552r-common.c
@@ -22,11 +22,10 @@ EXPORT_SYMBOL_NS_GPL(ad3552r_ch_ranges, "IIO_AD3552R");
 
 const s32 ad3542r_ch_ranges[AD3542R_MAX_RANGES][2] = {
 	[AD3542R_CH_OUTPUT_RANGE_0__2P5V]	= { 0, 2500 },
-	[AD3542R_CH_OUTPUT_RANGE_0__3V]		= { 0, 3000 },
 	[AD3542R_CH_OUTPUT_RANGE_0__5V]		= { 0, 5000 },
 	[AD3542R_CH_OUTPUT_RANGE_0__10V]	= { 0, 10000 },
-	[AD3542R_CH_OUTPUT_RANGE_NEG_2P5__7P5V]	= { -2500, 7500 },
-	[AD3542R_CH_OUTPUT_RANGE_NEG_5__5V]	= { -5000, 5000 }
+	[AD3542R_CH_OUTPUT_RANGE_NEG_5__5V]	= { -5000, 5000 },
+	[AD3542R_CH_OUTPUT_RANGE_NEG_2P5__7P5V]	= { -2500, 7500 }
 };
 EXPORT_SYMBOL_NS_GPL(ad3542r_ch_ranges, "IIO_AD3552R");
 
diff --git a/drivers/iio/dac/ad3552r.h b/drivers/iio/dac/ad3552r.h
index fd5a3dfd1d1c..4b5581039ae9 100644
--- a/drivers/iio/dac/ad3552r.h
+++ b/drivers/iio/dac/ad3552r.h
@@ -131,7 +131,7 @@
 #define AD3552R_CH1_ACTIVE				BIT(1)
 
 #define AD3552R_MAX_RANGES	5
-#define AD3542R_MAX_RANGES	6
+#define AD3542R_MAX_RANGES	5
 #define AD3552R_QUAD_SPI	2
 
 extern const s32 ad3552r_ch_ranges[AD3552R_MAX_RANGES][2];
@@ -189,16 +189,14 @@ enum ad3552r_ch_vref_select {
 enum ad3542r_ch_output_range {
 	/* Range from 0 V to 2.5 V. Requires Rfb1x connection */
 	AD3542R_CH_OUTPUT_RANGE_0__2P5V,
-	/* Range from 0 V to 3 V. Requires Rfb1x connection  */
-	AD3542R_CH_OUTPUT_RANGE_0__3V,
 	/* Range from 0 V to 5 V. Requires Rfb1x connection  */
 	AD3542R_CH_OUTPUT_RANGE_0__5V,
 	/* Range from 0 V to 10 V. Requires Rfb2x connection  */
 	AD3542R_CH_OUTPUT_RANGE_0__10V,
-	/* Range from -2.5 V to 7.5 V. Requires Rfb2x connection  */
-	AD3542R_CH_OUTPUT_RANGE_NEG_2P5__7P5V,
 	/* Range from -5 V to 5 V. Requires Rfb2x connection  */
 	AD3542R_CH_OUTPUT_RANGE_NEG_5__5V,
+	/* Range from -2.5 V to 7.5 V. Requires Rfb2x connection  */
+	AD3542R_CH_OUTPUT_RANGE_NEG_2P5__7P5V,
 };
 
 enum ad3552r_ch_output_range {

-- 
2.47.0

[PATCH v3 2/9] iio: dac: ad3552r-hs: clear reset status flag

[Angelo Dureghello]

From: Angelo Dureghello <adureghello@baylibre.com>

Clear reset status flag, to keep error status register
clean after reset (ad3552r manual, rev B table 38).

Reset error flag was left to 1, so debugging registers, the
"Error Status Register" was dirty (0x01). It is important
to clear this bit, so if there is any reset event over normal
working mode, it is possible to detect it.

Fixes: 0b4d9fe58be8 ("iio: dac: ad3552r: add high-speed platform driver")
Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
 drivers/iio/dac/ad3552r-hs.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c
index 216c634f3eaf..8974df625670 100644
--- a/drivers/iio/dac/ad3552r-hs.c
+++ b/drivers/iio/dac/ad3552r-hs.c
@@ -329,6 +329,12 @@ static int ad3552r_hs_setup(struct ad3552r_hs_state *st)
 		dev_info(st->dev, "Chip ID error. Expected 0x%x, Read 0x%x\n",
 			 AD3552R_ID, id);
 
+	/* Clear reset error flag, see ad3552r manual, rev B table 38. */
+	ret = st->data->bus_reg_write(st->back, AD3552R_REG_ADDR_ERR_STATUS,
+				      AD3552R_MASK_RESET_STATUS, 1);
+	if (ret)
+		return ret;
+
 	ret = st->data->bus_reg_write(st->back,
 				      AD3552R_REG_ADDR_SH_REFERENCE_CONFIG,
 				      0, 1);

-- 
2.47.0

[PATCH v3 3/9] iio: dac: adi-axi-dac: modify stream enable

[Angelo Dureghello]

From: Angelo Dureghello <adureghello@baylibre.com>

Change suggested from the AXI HDL team, modify the function
axi_dac_data_stream_enable() to check for interface busy, to avoid
possible issues when starting the stream.

Fixes: e61d7178429a ("iio: dac: adi-axi-dac: extend features")
Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
 drivers/iio/dac/adi-axi-dac.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/iio/dac/adi-axi-dac.c b/drivers/iio/dac/adi-axi-dac.c
index b143f7ed6847..ac871deb8063 100644
--- a/drivers/iio/dac/adi-axi-dac.c
+++ b/drivers/iio/dac/adi-axi-dac.c
@@ -585,6 +585,14 @@ static int axi_dac_ddr_disable(struct iio_backend *back)
 static int axi_dac_data_stream_enable(struct iio_backend *back)
 {
 	struct axi_dac_state *st = iio_backend_get_priv(back);
+	int ret, val;
+
+	ret = regmap_read_poll_timeout(st->regmap,
+				AXI_DAC_UI_STATUS_REG, val,
+				FIELD_GET(AXI_DAC_UI_STATUS_IF_BUSY, val) == 0,
+				10, 100 * KILO);
+	if (ret)
+		return ret;
 
 	return regmap_set_bits(st->regmap, AXI_DAC_CUSTOM_CTRL_REG,
 			       AXI_DAC_CUSTOM_CTRL_STREAM_ENABLE);

-- 
2.47.0

[PATCH v3 4/9] iio: dac: adi-axi-dac: add bus mode setup

[Angelo Dureghello]

From: Angelo Dureghello <adureghello@baylibre.com>

The ad354xr requires DSPI mode (2 data lanes) to work in buffering
mode, so backend needs to allow a mode selection between:
    SPI  (entire ad35xxr family),
    DSPI (ad354xr),
    QSPI (ad355xr).

About removal of AXI_DAC_CUSTOM_CTRL_SYNCED_TRANSFER, according to
the HDL history the flag has never been used. So looks like the driver
was including it by mistake or in anticipation for something that was
never implemented on HDL side.

Current HDL updated documentation confirm it is actually not in use
anymore and replaced by the IO_MODE bits.

Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
 drivers/iio/dac/ad3552r-hs.h  |  8 ++++++++
 drivers/iio/dac/adi-axi-dac.c | 22 +++++++++++++++++++++-
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/dac/ad3552r-hs.h b/drivers/iio/dac/ad3552r-hs.h
index 724261d38dea..4a9e35234124 100644
--- a/drivers/iio/dac/ad3552r-hs.h
+++ b/drivers/iio/dac/ad3552r-hs.h
@@ -8,11 +8,19 @@
 
 struct iio_backend;
 
+enum ad3552r_io_mode {
+	AD3552R_IO_MODE_SPI,
+	AD3552R_IO_MODE_DSPI,
+	AD3552R_IO_MODE_QSPI,
+};
+
 struct ad3552r_hs_platform_data {
 	int (*bus_reg_read)(struct iio_backend *back, u32 reg, u32 *val,
 			    size_t data_size);
 	int (*bus_reg_write)(struct iio_backend *back, u32 reg, u32 val,
 			     size_t data_size);
+	int (*bus_set_io_mode)(struct iio_backend *back,
+			       enum ad3552r_io_mode mode);
 	u32 bus_sample_data_clock_hz;
 };
 
diff --git a/drivers/iio/dac/adi-axi-dac.c b/drivers/iio/dac/adi-axi-dac.c
index ac871deb8063..bcaf365feef4 100644
--- a/drivers/iio/dac/adi-axi-dac.c
+++ b/drivers/iio/dac/adi-axi-dac.c
@@ -64,7 +64,7 @@
 #define   AXI_DAC_UI_STATUS_IF_BUSY		BIT(4)
 #define AXI_DAC_CUSTOM_CTRL_REG			0x008C
 #define   AXI_DAC_CUSTOM_CTRL_ADDRESS		GENMASK(31, 24)
-#define   AXI_DAC_CUSTOM_CTRL_SYNCED_TRANSFER	BIT(2)
+#define   AXI_DAC_CUSTOM_CTRL_MULTI_IO_MODE	GENMASK(3, 2)
 #define   AXI_DAC_CUSTOM_CTRL_STREAM		BIT(1)
 #define   AXI_DAC_CUSTOM_CTRL_TRANSFER_DATA	BIT(0)
 
@@ -722,6 +722,25 @@ static int axi_dac_bus_reg_read(struct iio_backend *back, u32 reg, u32 *val,
 	return regmap_read(st->regmap, AXI_DAC_CUSTOM_RD_REG, val);
 }
 
+static int axi_dac_bus_set_io_mode(struct iio_backend *back,
+				   enum ad3552r_io_mode mode)
+{
+	struct axi_dac_state *st = iio_backend_get_priv(back);
+	int ival, ret;
+
+	guard(mutex)(&st->lock);
+
+	ret = regmap_update_bits(st->regmap, AXI_DAC_CUSTOM_CTRL_REG,
+			AXI_DAC_CUSTOM_CTRL_MULTI_IO_MODE,
+			FIELD_PREP(AXI_DAC_CUSTOM_CTRL_MULTI_IO_MODE, mode));
+	if (ret)
+		return ret;
+
+	return regmap_read_poll_timeout(st->regmap, AXI_DAC_UI_STATUS_REG, ival,
+			FIELD_GET(AXI_DAC_UI_STATUS_IF_BUSY, ival) == 0, 10,
+			100 * KILO);
+}
+
 static void axi_dac_child_remove(void *data)
 {
 	platform_device_unregister(data);
@@ -733,6 +752,7 @@ static int axi_dac_create_platform_device(struct axi_dac_state *st,
 	struct ad3552r_hs_platform_data pdata = {
 		.bus_reg_read = axi_dac_bus_reg_read,
 		.bus_reg_write = axi_dac_bus_reg_write,
+		.bus_set_io_mode = axi_dac_bus_set_io_mode,
 		.bus_sample_data_clock_hz = st->dac_clk_rate,
 	};
 	struct platform_device_info pi = {

-- 
2.47.0

[PATCH v3 5/9] iio: dac: ad3552r-hs: fix message on wrong chip id

[Angelo Dureghello]

From: Angelo Dureghello <adureghello@baylibre.com>

Set a better info message on wrong chip id, fixing the
expected value as read from the info struct.

Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
 drivers/iio/dac/ad3552r-hs.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c
index 8974df625670..27949f207d42 100644
--- a/drivers/iio/dac/ad3552r-hs.c
+++ b/drivers/iio/dac/ad3552r-hs.c
@@ -326,8 +326,9 @@ static int ad3552r_hs_setup(struct ad3552r_hs_state *st)
 
 	id |= val << 8;
 	if (id != st->model_data->chip_id)
-		dev_info(st->dev, "Chip ID error. Expected 0x%x, Read 0x%x\n",
-			 AD3552R_ID, id);
+		dev_info(st->dev,
+			 "Chip ID mismatch, detected 0x%x but expected 0x%x\n",
+			 id, st->model_data->chip_id);
 
 	/* Clear reset error flag, see ad3552r manual, rev B table 38. */
 	ret = st->data->bus_reg_write(st->back, AD3552R_REG_ADDR_ERR_STATUS,

-- 
2.47.0

[PATCH v3 6/9] iio: dac: ad3552r-hs: use instruction mode for configuration

[Angelo Dureghello]

From: Angelo Dureghello <adureghello@baylibre.com>

Use "instruction" mode over initial configuration and all other
non-streaming operations.

DAC boots in streaming mode as default, and the driver is not
changing this mode.

Instruction r/w is still working becouse instruction is processed
from the DAC after chip select is deasserted, this works until
loop mode is 0 or greater than the instruction size.

All initial operations should be more safely done in instruction
mode, a mode provided for this.

Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
 drivers/iio/dac/ad3552r-hs.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c
index 27949f207d42..991b11702273 100644
--- a/drivers/iio/dac/ad3552r-hs.c
+++ b/drivers/iio/dac/ad3552r-hs.c
@@ -132,6 +132,13 @@ static int ad3552r_hs_buffer_postenable(struct iio_dev *indio_dev)
 		return -EINVAL;
 	}
 
+	/* Primary region access, set streaming mode (now in SPI + SDR). */
+	ret = ad3552r_qspi_update_reg_bits(st,
+					   AD3552R_REG_ADDR_INTERFACE_CONFIG_B,
+					   AD3552R_MASK_SINGLE_INST, 0, 1);
+	if (ret)
+		return ret;
+
 	ret = st->data->bus_reg_write(st->back, AD3552R_REG_ADDR_STREAM_MODE,
 				      loop_len, 1);
 	if (ret)
@@ -198,6 +205,14 @@ static int ad3552r_hs_buffer_predisable(struct iio_dev *indio_dev)
 	if (ret)
 		return ret;
 
+	/* Back to single instruction mode, disabling loop. */
+	ret = ad3552r_qspi_update_reg_bits(st,
+					   AD3552R_REG_ADDR_INTERFACE_CONFIG_B,
+					   AD3552R_MASK_SINGLE_INST,
+					   AD3552R_MASK_SINGLE_INST, 1);
+	if (ret)
+		return ret;
+
 	return 0;
 }
 
@@ -308,6 +323,13 @@ static int ad3552r_hs_setup(struct ad3552r_hs_state *st)
 	if (ret)
 		return ret;
 
+	ret = st->data->bus_reg_write(st->back,
+				      AD3552R_REG_ADDR_INTERFACE_CONFIG_B,
+				      AD3552R_MASK_SINGLE_INST |
+				      AD3552R_MASK_SHORT_INSTRUCTION, 1);
+	if (ret)
+		return ret;
+
 	ret = ad3552r_hs_scratch_pad_test(st);
 	if (ret)
 		return ret;

-- 
2.47.0

[PATCH v3 7/9] iio: dac: ad3552r: share model data structures

[Angelo Dureghello]

From: Angelo Dureghello <adureghello@baylibre.com>

Preparing for new parts to be added also in the hs driver,
set model data structures in ad3552r-common.c, to be accessible
from both -hs and non hs driver.

Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
 drivers/iio/dac/ad3552r-common.c | 46 ++++++++++++++++++++++++++++++++++++----
 drivers/iio/dac/ad3552r-hs.c     |  8 -------
 drivers/iio/dac/ad3552r.c        | 36 -------------------------------
 drivers/iio/dac/ad3552r.h        |  6 ++++--
 4 files changed, 46 insertions(+), 50 deletions(-)

diff --git a/drivers/iio/dac/ad3552r-common.c b/drivers/iio/dac/ad3552r-common.c
index 03e0864f5084..ded90bf57baf 100644
--- a/drivers/iio/dac/ad3552r-common.c
+++ b/drivers/iio/dac/ad3552r-common.c
@@ -11,23 +11,21 @@
 
 #include "ad3552r.h"
 
-const s32 ad3552r_ch_ranges[AD3552R_MAX_RANGES][2] = {
+static const s32 ad3552r_ch_ranges[AD3552R_MAX_RANGES][2] = {
 	[AD3552R_CH_OUTPUT_RANGE_0__2P5V]	= { 0, 2500 },
 	[AD3552R_CH_OUTPUT_RANGE_0__5V]		= { 0, 5000 },
 	[AD3552R_CH_OUTPUT_RANGE_0__10V]	= { 0, 10000 },
 	[AD3552R_CH_OUTPUT_RANGE_NEG_5__5V]	= { -5000, 5000 },
 	[AD3552R_CH_OUTPUT_RANGE_NEG_10__10V]	= { -10000, 10000 }
 };
-EXPORT_SYMBOL_NS_GPL(ad3552r_ch_ranges, "IIO_AD3552R");
 
-const s32 ad3542r_ch_ranges[AD3542R_MAX_RANGES][2] = {
+static const s32 ad3542r_ch_ranges[AD3542R_MAX_RANGES][2] = {
 	[AD3542R_CH_OUTPUT_RANGE_0__2P5V]	= { 0, 2500 },
 	[AD3542R_CH_OUTPUT_RANGE_0__5V]		= { 0, 5000 },
 	[AD3542R_CH_OUTPUT_RANGE_0__10V]	= { 0, 10000 },
 	[AD3542R_CH_OUTPUT_RANGE_NEG_5__5V]	= { -5000, 5000 },
 	[AD3542R_CH_OUTPUT_RANGE_NEG_2P5__7P5V]	= { -2500, 7500 }
 };
-EXPORT_SYMBOL_NS_GPL(ad3542r_ch_ranges, "IIO_AD3552R");
 
 /* Gain * AD3552R_GAIN_SCALE */
 static const s32 gains_scaling_table[] = {
@@ -37,6 +35,46 @@ static const s32 gains_scaling_table[] = {
 	[AD3552R_CH_GAIN_SCALING_0_125]		= 125
 };
 
+const struct ad3552r_model_data ad3541r_model_data = {
+	.model_name = "ad3541r",
+	.chip_id = AD3541R_ID,
+	.num_hw_channels = 1,
+	.ranges_table = ad3542r_ch_ranges,
+	.num_ranges = ARRAY_SIZE(ad3542r_ch_ranges),
+	.requires_output_range = true,
+};
+EXPORT_SYMBOL_NS_GPL(ad3541r_model_data, "IIO_AD3552R");
+
+const struct ad3552r_model_data ad3542r_model_data = {
+	.model_name = "ad3542r",
+	.chip_id = AD3542R_ID,
+	.num_hw_channels = 2,
+	.ranges_table = ad3542r_ch_ranges,
+	.num_ranges = ARRAY_SIZE(ad3542r_ch_ranges),
+	.requires_output_range = true,
+};
+EXPORT_SYMBOL_NS_GPL(ad3542r_model_data, "IIO_AD3552R");
+
+const struct ad3552r_model_data ad3551r_model_data = {
+	.model_name = "ad3551r",
+	.chip_id = AD3551R_ID,
+	.num_hw_channels = 1,
+	.ranges_table = ad3552r_ch_ranges,
+	.num_ranges = ARRAY_SIZE(ad3552r_ch_ranges),
+	.requires_output_range = false,
+};
+EXPORT_SYMBOL_NS_GPL(ad3551r_model_data, "IIO_AD3552R");
+
+const struct ad3552r_model_data ad3552r_model_data = {
+	.model_name = "ad3552r",
+	.chip_id = AD3552R_ID,
+	.num_hw_channels = 2,
+	.ranges_table = ad3552r_ch_ranges,
+	.num_ranges = ARRAY_SIZE(ad3552r_ch_ranges),
+	.requires_output_range = false,
+};
+EXPORT_SYMBOL_NS_GPL(ad3552r_model_data, "IIO_AD3552R");
+
 u16 ad3552r_calc_custom_gain(u8 p, u8 n, s16 goffs)
 {
 	return FIELD_PREP(AD3552R_MASK_CH_RANGE_OVERRIDE, 1) |
diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c
index 991b11702273..bfb6228c9b9b 100644
--- a/drivers/iio/dac/ad3552r-hs.c
+++ b/drivers/iio/dac/ad3552r-hs.c
@@ -527,14 +527,6 @@ static int ad3552r_hs_probe(struct platform_device *pdev)
 	return devm_iio_device_register(&pdev->dev, indio_dev);
 }
 
-static const struct ad3552r_model_data ad3552r_model_data = {
-	.model_name = "ad3552r",
-	.chip_id = AD3552R_ID,
-	.num_hw_channels = 2,
-	.ranges_table = ad3552r_ch_ranges,
-	.num_ranges = ARRAY_SIZE(ad3552r_ch_ranges),
-};
-
 static const struct of_device_id ad3552r_hs_of_id[] = {
 	{ .compatible = "adi,ad3552r", .data = &ad3552r_model_data },
 	{ }
diff --git a/drivers/iio/dac/ad3552r.c b/drivers/iio/dac/ad3552r.c
index e7206af53af6..9d28e06b80c0 100644
--- a/drivers/iio/dac/ad3552r.c
+++ b/drivers/iio/dac/ad3552r.c
@@ -649,42 +649,6 @@ static int ad3552r_probe(struct spi_device *spi)
 	return devm_iio_device_register(&spi->dev, indio_dev);
 }
 
-static const struct ad3552r_model_data ad3541r_model_data = {
-	.model_name = "ad3541r",
-	.chip_id = AD3541R_ID,
-	.num_hw_channels = 1,
-	.ranges_table = ad3542r_ch_ranges,
-	.num_ranges = ARRAY_SIZE(ad3542r_ch_ranges),
-	.requires_output_range = true,
-};
-
-static const struct ad3552r_model_data ad3542r_model_data = {
-	.model_name = "ad3542r",
-	.chip_id = AD3542R_ID,
-	.num_hw_channels = 2,
-	.ranges_table = ad3542r_ch_ranges,
-	.num_ranges = ARRAY_SIZE(ad3542r_ch_ranges),
-	.requires_output_range = true,
-};
-
-static const struct ad3552r_model_data ad3551r_model_data = {
-	.model_name = "ad3551r",
-	.chip_id = AD3551R_ID,
-	.num_hw_channels = 1,
-	.ranges_table = ad3552r_ch_ranges,
-	.num_ranges = ARRAY_SIZE(ad3552r_ch_ranges),
-	.requires_output_range = false,
-};
-
-static const struct ad3552r_model_data ad3552r_model_data = {
-	.model_name = "ad3552r",
-	.chip_id = AD3552R_ID,
-	.num_hw_channels = 2,
-	.ranges_table = ad3552r_ch_ranges,
-	.num_ranges = ARRAY_SIZE(ad3552r_ch_ranges),
-	.requires_output_range = false,
-};
-
 static const struct spi_device_id ad3552r_id[] = {
 	{
 		.name = "ad3541r",
diff --git a/drivers/iio/dac/ad3552r.h b/drivers/iio/dac/ad3552r.h
index 4b5581039ae9..3dc8d1d9c0f9 100644
--- a/drivers/iio/dac/ad3552r.h
+++ b/drivers/iio/dac/ad3552r.h
@@ -134,8 +134,10 @@
 #define AD3542R_MAX_RANGES	5
 #define AD3552R_QUAD_SPI	2
 
-extern const s32 ad3552r_ch_ranges[AD3552R_MAX_RANGES][2];
-extern const s32 ad3542r_ch_ranges[AD3542R_MAX_RANGES][2];
+extern const struct ad3552r_model_data ad3541r_model_data;
+extern const struct ad3552r_model_data ad3542r_model_data;
+extern const struct ad3552r_model_data ad3551r_model_data;
+extern const struct ad3552r_model_data ad3552r_model_data;
 
 enum ad3552r_id {
 	AD3541R_ID = 0x400b,

-- 
2.47.0

[PATCH v3 8/9] iio: dac: ad3552r-hs: add ad3541/2r support

[Angelo Dureghello]

From: Angelo Dureghello <adureghello@baylibre.com>

A new FPGA HDL has been developed from ADI to support ad354xr
devices.

Add support for ad3541r and ad3542r with following additions:

- use common device_info structures for hs and non hs drivers,
- DMA buffering, use DSPI mode for ad354xr and QSPI for ad355xr,
- change sample rate to respect number of lanes.

Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
 drivers/iio/dac/ad3552r-common.c |   4 +
 drivers/iio/dac/ad3552r-hs.c     | 225 ++++++++++++++++++++++++++++++++-------
 drivers/iio/dac/ad3552r.h        |   3 +
 3 files changed, 195 insertions(+), 37 deletions(-)

diff --git a/drivers/iio/dac/ad3552r-common.c b/drivers/iio/dac/ad3552r-common.c
index ded90bf57baf..b8807e54fa05 100644
--- a/drivers/iio/dac/ad3552r-common.c
+++ b/drivers/iio/dac/ad3552r-common.c
@@ -42,6 +42,7 @@ const struct ad3552r_model_data ad3541r_model_data = {
 	.ranges_table = ad3542r_ch_ranges,
 	.num_ranges = ARRAY_SIZE(ad3542r_ch_ranges),
 	.requires_output_range = true,
+	.num_spi_data_lanes = 2,
 };
 EXPORT_SYMBOL_NS_GPL(ad3541r_model_data, "IIO_AD3552R");
 
@@ -52,6 +53,7 @@ const struct ad3552r_model_data ad3542r_model_data = {
 	.ranges_table = ad3542r_ch_ranges,
 	.num_ranges = ARRAY_SIZE(ad3542r_ch_ranges),
 	.requires_output_range = true,
+	.num_spi_data_lanes = 2,
 };
 EXPORT_SYMBOL_NS_GPL(ad3542r_model_data, "IIO_AD3552R");
 
@@ -62,6 +64,7 @@ const struct ad3552r_model_data ad3551r_model_data = {
 	.ranges_table = ad3552r_ch_ranges,
 	.num_ranges = ARRAY_SIZE(ad3552r_ch_ranges),
 	.requires_output_range = false,
+	.num_spi_data_lanes = 4,
 };
 EXPORT_SYMBOL_NS_GPL(ad3551r_model_data, "IIO_AD3552R");
 
@@ -72,6 +75,7 @@ const struct ad3552r_model_data ad3552r_model_data = {
 	.ranges_table = ad3552r_ch_ranges,
 	.num_ranges = ARRAY_SIZE(ad3552r_ch_ranges),
 	.requires_output_range = false,
+	.num_spi_data_lanes = 4,
 };
 EXPORT_SYMBOL_NS_GPL(ad3552r_model_data, "IIO_AD3552R");
 
diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c
index bfb6228c9b9b..4600a9e84dfc 100644
--- a/drivers/iio/dac/ad3552r-hs.c
+++ b/drivers/iio/dac/ad3552r-hs.c
@@ -19,6 +19,31 @@
 #include "ad3552r.h"
 #include "ad3552r-hs.h"
 
+/*
+ * Important notes for register map access:
+ * ========================================
+ *
+ * Register address space is divided in 2 regions, primary (config) and
+ * secondary (DAC). Primary region can only be accessed in simple SPI mode,
+ * with exception for ad355x models where setting QSPI pin high allows QSPI
+ * access to both the regions.
+ *
+ * Due to the fact that ad3541/2r do not implement QSPI, for proper device
+ * detection, HDL keeps "QSPI" pin level low at boot (see ad3552r manual, rev B
+ * table 7, pin 31, digital input). For this reason, actually the working mode
+ * between SPI, DSPI and QSPI must be set via software, configuring the target
+ * DAC appropriately, together with the backend API to configure the bus mode
+ * accordingly.
+ *
+ * Also, important to note that none of the three modes allow to read in DDR.
+ *
+ * In non-buffering operations, mode is set to simple SPI SDR for all primary
+ * and secondary region r/w accesses, to avoid to switch the mode each time DAC
+ * register is accessed (raw accesses, r/w), and to be able to dump registers
+ * content (possible as non DDR only).
+ * In buffering mode, driver sets best possible mode, D/QSPI and DDR.
+ */
+
 struct ad3552r_hs_state {
 	const struct ad3552r_model_data *model_data;
 	struct gpio_desc *reset_gpio;
@@ -27,6 +52,8 @@ struct ad3552r_hs_state {
 	bool single_channel;
 	struct ad3552r_ch_data ch_data[AD3552R_MAX_CH];
 	struct ad3552r_hs_platform_data *data;
+	/* INTERFACE_CONFIG_D register cache, in DDR we cannot read values. */
+	u32 config_d;
 };
 
 static int ad3552r_qspi_update_reg_bits(struct ad3552r_hs_state *st,
@@ -56,15 +83,19 @@ static int ad3552r_hs_read_raw(struct iio_dev *indio_dev,
 	switch (mask) {
 	case IIO_CHAN_INFO_SAMP_FREQ:
 		/*
-		 * Using 4 lanes (QSPI), then using 2 as DDR mode is
-		 * considered always on (considering buffering mode always).
+		 * Using a "num_spi_data_lanes" variable since ad3541/2 have
+		 * only DSPI interface, while ad355x is QSPI. Then using 2 as
+		 * DDR mode is considered always on (considering buffering
+		 * mode always).
 		 */
 		*val = DIV_ROUND_CLOSEST(st->data->bus_sample_data_clock_hz *
-					 4 * 2, chan->scan_type.realbits);
+					 st->model_data->num_spi_data_lanes * 2,
+					 chan->scan_type.realbits);
 
 		return IIO_VAL_INT;
 
 	case IIO_CHAN_INFO_RAW:
+		/* For RAW accesses, stay always in simple-spi. */
 		ret = st->data->bus_reg_read(st->back,
 				AD3552R_REG_ADDR_CH_DAC_16B(chan->channel),
 				val, 2);
@@ -93,6 +124,7 @@ static int ad3552r_hs_write_raw(struct iio_dev *indio_dev,
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
+		/* For RAW accesses, stay always in simple-spi. */
 		iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
 			return st->data->bus_reg_write(st->back,
 				    AD3552R_REG_ADDR_CH_DAC_16B(chan->channel),
@@ -104,6 +136,42 @@ static int ad3552r_hs_write_raw(struct iio_dev *indio_dev,
 	}
 }
 
+static int ad3552r_hs_set_bus_io_mode_hs(struct ad3552r_hs_state *st)
+{
+	int bus_mode;
+
+	if (st->model_data->num_spi_data_lanes == 4)
+		bus_mode = AD3552R_IO_MODE_QSPI;
+	else
+		bus_mode = AD3552R_IO_MODE_DSPI;
+
+	return st->data->bus_set_io_mode(st->back, bus_mode);
+}
+
+static int ad3552r_hs_set_target_io_mode_hs(struct ad3552r_hs_state *st)
+{
+	int mode_target;
+
+	/*
+	 * Best access for secondary reg area, QSPI where possible,
+	 * else as DSPI.
+	 */
+	if (st->model_data->num_spi_data_lanes == 4)
+		mode_target = AD3552R_QUAD_SPI;
+	else
+		mode_target = AD3552R_DUAL_SPI;
+
+	/*
+	 * Better to not use update here, since generally it is already
+	 * set as DDR mode, and it's not possible to read in DDR mode.
+	 */
+	return st->data->bus_reg_write(st->back,
+				AD3552R_REG_ADDR_TRANSFER_REGISTER,
+				FIELD_PREP(AD3552R_MASK_MULTI_IO_MODE,
+					   mode_target) |
+				AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE, 1);
+}
+
 static int ad3552r_hs_buffer_postenable(struct iio_dev *indio_dev)
 {
 	struct ad3552r_hs_state *st = iio_priv(indio_dev);
@@ -132,6 +200,11 @@ static int ad3552r_hs_buffer_postenable(struct iio_dev *indio_dev)
 		return -EINVAL;
 	}
 
+	/*
+	 * With ad3541/2r support, QSPI pin is held low at reset from HDL,
+	 * streaming start sequence must respect strictly the order below.
+	 */
+
 	/* Primary region access, set streaming mode (now in SPI + SDR). */
 	ret = ad3552r_qspi_update_reg_bits(st,
 					   AD3552R_REG_ADDR_INTERFACE_CONFIG_B,
@@ -139,48 +212,106 @@ static int ad3552r_hs_buffer_postenable(struct iio_dev *indio_dev)
 	if (ret)
 		return ret;
 
-	ret = st->data->bus_reg_write(st->back, AD3552R_REG_ADDR_STREAM_MODE,
+	/*
+	 * Set target loop len, 0x2c 0r 0x2a, descending loop, and keeping loop
+	 * len value so it's not cleared hereafter when enabling streaming mode
+	 * (cleared by CS_ up).
+	 */
+	ret = ad3552r_qspi_update_reg_bits(st,
+		AD3552R_REG_ADDR_TRANSFER_REGISTER,
+		AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE,
+		AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE, 1);
+	if (ret)
+		goto exit_err_streaming;
+
+	ret = st->data->bus_reg_write(st->back,
+				      AD3552R_REG_ADDR_STREAM_MODE,
 				      loop_len, 1);
 	if (ret)
-		return ret;
+		goto exit_err_streaming;
 
-	/* Inform DAC chip to switch into DDR mode */
-	ret = ad3552r_qspi_update_reg_bits(st,
-					   AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
-					   AD3552R_MASK_SPI_CONFIG_DDR,
-					   AD3552R_MASK_SPI_CONFIG_DDR, 1);
+	/* Setting DDR now, caching current config_d. */
+	ret = st->data->bus_reg_read(st->back,
+				     AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
+				     &st->config_d, 1);
 	if (ret)
-		return ret;
+		goto exit_err_streaming;
+
+	st->config_d |= AD3552R_MASK_SPI_CONFIG_DDR;
+	ret = st->data->bus_reg_write(st->back,
+				      AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
+				      st->config_d, 1);
+	if (ret)
+		goto exit_err_streaming;
 
-	/* Inform DAC IP to go for DDR mode from now on */
 	ret = iio_backend_ddr_enable(st->back);
-	if (ret) {
-		dev_err(st->dev, "could not set DDR mode, not streaming");
-		goto exit_err;
-	}
+	if (ret)
+		goto exit_err_ddr_mode_target;
+
+	/*
+	 * From here onward mode is DDR, so reading any register is not possible
+	 * anymore, including calling "ad3552r_qspi_update_reg_bits" function.
+	 */
+
+	/* Set target to best high speed mode (D or QSPI). */
+	ret = ad3552r_hs_set_target_io_mode_hs(st);
+	if (ret)
+		goto exit_err_ddr_mode;
+
+	/* Set bus to best high speed mode (D or QSPI). */
+	ret = ad3552r_hs_set_bus_io_mode_hs(st);
+	if (ret)
+		goto exit_err_bus_mode_target;
 
+	/*
+	 * Backend setup must be done now only, or related register values will
+	 * be disrupted by previous bus accesses.
+	 */
 	ret = iio_backend_data_transfer_addr(st->back, val);
 	if (ret)
-		goto exit_err;
+		goto exit_err_bus_mode_target;
 
 	ret = iio_backend_data_format_set(st->back, 0, &fmt);
 	if (ret)
-		goto exit_err;
+		goto exit_err_bus_mode_target;
 
 	ret = iio_backend_data_stream_enable(st->back);
 	if (ret)
-		goto exit_err;
+		goto exit_err_bus_mode_target;
 
 	return 0;
 
-exit_err:
-	ad3552r_qspi_update_reg_bits(st,
-				     AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
-				     AD3552R_MASK_SPI_CONFIG_DDR,
-				     0, 1);
+exit_err_bus_mode_target:
+	/* Back to simple SPI, not using update to avoid read. */
+	st->data->bus_reg_write(st->back, AD3552R_REG_ADDR_TRANSFER_REGISTER,
+				FIELD_PREP(AD3552R_MASK_MULTI_IO_MODE,
+					   AD3552R_SPI) |
+				AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE, 1);
+
+	/*
+	 * Back bus to simple SPI, this must be executed together with above
+	 * target mode unwind, and can be done only after it.
+	 */
+	st->data->bus_set_io_mode(st->back, AD3552R_IO_MODE_SPI);
 
+exit_err_ddr_mode:
 	iio_backend_ddr_disable(st->back);
 
+exit_err_ddr_mode_target:
+	/*
+	 * Back to SDR. In DDR we cannot read, whatever the mode is, so not
+	 * using update.
+	 */
+	st->data->bus_reg_write(st->back, AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
+				FIELD_PREP(AD3552R_MASK_SDO_DRIVE_STRENGTH, 1),
+				1);
+
+exit_err_streaming:
+	/* Back to single instruction mode, disabling loop. */
+	st->data->bus_reg_write(st->back, AD3552R_REG_ADDR_INTERFACE_CONFIG_B,
+				AD3552R_MASK_SINGLE_INST |
+				AD3552R_MASK_SHORT_INSTRUCTION, 1);
+
 	return ret;
 }
 
@@ -193,11 +324,22 @@ static int ad3552r_hs_buffer_predisable(struct iio_dev *indio_dev)
 	if (ret)
 		return ret;
 
-	/* Inform DAC to set in SDR mode */
-	ret = ad3552r_qspi_update_reg_bits(st,
-					   AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
-					   AD3552R_MASK_SPI_CONFIG_DDR,
-					   0, 1);
+	/*
+	 * Set us to simple SPI, even if still in ddr, so to be able to write
+	 * in primary region.
+	 */
+	ret = st->data->bus_set_io_mode(st->back, AD3552R_IO_MODE_SPI);
+	if (ret)
+		return ret;
+
+	/*
+	 * Back to SDR (in DDR we cannot read, whatever the mode is, so not
+	 * using update).
+	 */
+	st->config_d &= ~AD3552R_MASK_SPI_CONFIG_DDR;
+	ret = st->data->bus_reg_write(st->back,
+				      AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
+				      st->config_d, 1);
 	if (ret)
 		return ret;
 
@@ -205,6 +347,17 @@ static int ad3552r_hs_buffer_predisable(struct iio_dev *indio_dev)
 	if (ret)
 		return ret;
 
+	/*
+	 * Back to simple SPI for secondary region too now, so to be able to
+	 * dump/read registers there too if needed.
+	 */
+	ret = ad3552r_qspi_update_reg_bits(st,
+					   AD3552R_REG_ADDR_TRANSFER_REGISTER,
+					   AD3552R_MASK_MULTI_IO_MODE,
+					   AD3552R_SPI, 1);
+	if (ret)
+		return ret;
+
 	/* Back to single instruction mode, disabling loop. */
 	ret = ad3552r_qspi_update_reg_bits(st,
 					   AD3552R_REG_ADDR_INTERFACE_CONFIG_B,
@@ -319,6 +472,7 @@ static int ad3552r_hs_setup(struct ad3552r_hs_state *st)
 	if (ret)
 		return ret;
 
+	/* HDL starts with DDR enabled, disabling it. */
 	ret = iio_backend_ddr_disable(st->back);
 	if (ret)
 		return ret;
@@ -352,6 +506,8 @@ static int ad3552r_hs_setup(struct ad3552r_hs_state *st)
 			 "Chip ID mismatch, detected 0x%x but expected 0x%x\n",
 			 id, st->model_data->chip_id);
 
+	dev_dbg(st->dev, "chip id %s detected", st->model_data->model_name);
+
 	/* Clear reset error flag, see ad3552r manual, rev B table 38. */
 	ret = st->data->bus_reg_write(st->back, AD3552R_REG_ADDR_ERR_STATUS,
 				      AD3552R_MASK_RESET_STATUS, 1);
@@ -364,14 +520,6 @@ static int ad3552r_hs_setup(struct ad3552r_hs_state *st)
 	if (ret)
 		return ret;
 
-	ret = st->data->bus_reg_write(st->back,
-				AD3552R_REG_ADDR_TRANSFER_REGISTER,
-				FIELD_PREP(AD3552R_MASK_MULTI_IO_MODE,
-					   AD3552R_QUAD_SPI) |
-				AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE, 1);
-	if (ret)
-		return ret;
-
 	ret = iio_backend_data_source_set(st->back, 0, IIO_BACKEND_EXTERNAL);
 	if (ret)
 		return ret;
@@ -528,6 +676,9 @@ static int ad3552r_hs_probe(struct platform_device *pdev)
 }
 
 static const struct of_device_id ad3552r_hs_of_id[] = {
+	{ .compatible = "adi,ad3541r", .data = &ad3541r_model_data },
+	{ .compatible = "adi,ad3542r", .data = &ad3542r_model_data },
+	{ .compatible = "adi,ad3551r", .data = &ad3551r_model_data },
 	{ .compatible = "adi,ad3552r", .data = &ad3552r_model_data },
 	{ }
 };
diff --git a/drivers/iio/dac/ad3552r.h b/drivers/iio/dac/ad3552r.h
index 3dc8d1d9c0f9..768fa264d39e 100644
--- a/drivers/iio/dac/ad3552r.h
+++ b/drivers/iio/dac/ad3552r.h
@@ -132,6 +132,8 @@
 
 #define AD3552R_MAX_RANGES	5
 #define AD3542R_MAX_RANGES	5
+#define AD3552R_SPI		0
+#define AD3552R_DUAL_SPI	1
 #define AD3552R_QUAD_SPI	2
 
 extern const struct ad3552r_model_data ad3541r_model_data;
@@ -153,6 +155,7 @@ struct ad3552r_model_data {
 	const s32 (*ranges_table)[2];
 	int num_ranges;
 	bool requires_output_range;
+	int num_spi_data_lanes;
 };
 
 struct ad3552r_ch_data {

-- 
2.47.0

[PATCH v3 9/9] iio: dac: ad3552r-hs: update function name (non functional)

[Angelo Dureghello]

From: Angelo Dureghello <adureghello@baylibre.com>

Update ad3552r_qspi_update_reg_bits function name to a more
generic name, since used mode can be SIMPLE/DUAL/QUAD SPI.

Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
 drivers/iio/dac/ad3552r-hs.c | 64 ++++++++++++++++++++------------------------
 1 file changed, 29 insertions(+), 35 deletions(-)

diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c
index 4600a9e84dfc..7f3a70cfbef8 100644
--- a/drivers/iio/dac/ad3552r-hs.c
+++ b/drivers/iio/dac/ad3552r-hs.c
@@ -56,9 +56,9 @@ struct ad3552r_hs_state {
 	u32 config_d;
 };
 
-static int ad3552r_qspi_update_reg_bits(struct ad3552r_hs_state *st,
-					u32 reg, u32 mask, u32 val,
-					size_t xfer_size)
+static int ad3552r_update_reg_bits(struct ad3552r_hs_state *st,
+				   u32 reg, u32 mask, u32 val,
+				   size_t xfer_size)
 {
 	u32 rval;
 	int ret;
@@ -206,9 +206,8 @@ static int ad3552r_hs_buffer_postenable(struct iio_dev *indio_dev)
 	 */
 
 	/* Primary region access, set streaming mode (now in SPI + SDR). */
-	ret = ad3552r_qspi_update_reg_bits(st,
-					   AD3552R_REG_ADDR_INTERFACE_CONFIG_B,
-					   AD3552R_MASK_SINGLE_INST, 0, 1);
+	ret = ad3552r_update_reg_bits(st, AD3552R_REG_ADDR_INTERFACE_CONFIG_B,
+				      AD3552R_MASK_SINGLE_INST, 0, 1);
 	if (ret)
 		return ret;
 
@@ -217,10 +216,9 @@ static int ad3552r_hs_buffer_postenable(struct iio_dev *indio_dev)
 	 * len value so it's not cleared hereafter when enabling streaming mode
 	 * (cleared by CS_ up).
 	 */
-	ret = ad3552r_qspi_update_reg_bits(st,
-		AD3552R_REG_ADDR_TRANSFER_REGISTER,
-		AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE,
-		AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE, 1);
+	ret = ad3552r_update_reg_bits(st, AD3552R_REG_ADDR_TRANSFER_REGISTER,
+				      AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE,
+				      AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE, 1);
 	if (ret)
 		goto exit_err_streaming;
 
@@ -250,7 +248,7 @@ static int ad3552r_hs_buffer_postenable(struct iio_dev *indio_dev)
 
 	/*
 	 * From here onward mode is DDR, so reading any register is not possible
-	 * anymore, including calling "ad3552r_qspi_update_reg_bits" function.
+	 * anymore, including calling "ad3552r_update_reg_bits" function.
 	 */
 
 	/* Set target to best high speed mode (D or QSPI). */
@@ -351,18 +349,16 @@ static int ad3552r_hs_buffer_predisable(struct iio_dev *indio_dev)
 	 * Back to simple SPI for secondary region too now, so to be able to
 	 * dump/read registers there too if needed.
 	 */
-	ret = ad3552r_qspi_update_reg_bits(st,
-					   AD3552R_REG_ADDR_TRANSFER_REGISTER,
-					   AD3552R_MASK_MULTI_IO_MODE,
-					   AD3552R_SPI, 1);
+	ret = ad3552r_update_reg_bits(st, AD3552R_REG_ADDR_TRANSFER_REGISTER,
+				      AD3552R_MASK_MULTI_IO_MODE,
+				      AD3552R_SPI, 1);
 	if (ret)
 		return ret;
 
 	/* Back to single instruction mode, disabling loop. */
-	ret = ad3552r_qspi_update_reg_bits(st,
-					   AD3552R_REG_ADDR_INTERFACE_CONFIG_B,
-					   AD3552R_MASK_SINGLE_INST,
-					   AD3552R_MASK_SINGLE_INST, 1);
+	ret = ad3552r_update_reg_bits(st, AD3552R_REG_ADDR_INTERFACE_CONFIG_B,
+				      AD3552R_MASK_SINGLE_INST,
+				      AD3552R_MASK_SINGLE_INST, 1);
 	if (ret)
 		return ret;
 
@@ -379,10 +375,10 @@ static inline int ad3552r_hs_set_output_range(struct ad3552r_hs_state *st,
 	else
 		val = FIELD_PREP(AD3552R_MASK_CH1_RANGE, mode);
 
-	return ad3552r_qspi_update_reg_bits(st,
-					AD3552R_REG_ADDR_CH0_CH1_OUTPUT_RANGE,
-					AD3552R_MASK_CH_OUTPUT_RANGE_SEL(ch),
-					val, 1);
+	return ad3552r_update_reg_bits(st,
+				       AD3552R_REG_ADDR_CH0_CH1_OUTPUT_RANGE,
+				       AD3552R_MASK_CH_OUTPUT_RANGE_SEL(ch),
+				       val, 1);
 }
 
 static int ad3552r_hs_reset(struct ad3552r_hs_state *st)
@@ -398,10 +394,10 @@ static int ad3552r_hs_reset(struct ad3552r_hs_state *st)
 		fsleep(10);
 		gpiod_set_value_cansleep(st->reset_gpio, 0);
 	} else {
-		ret = ad3552r_qspi_update_reg_bits(st,
-					AD3552R_REG_ADDR_INTERFACE_CONFIG_A,
-					AD3552R_MASK_SOFTWARE_RESET,
-					AD3552R_MASK_SOFTWARE_RESET, 1);
+		ret = ad3552r_update_reg_bits(st,
+			AD3552R_REG_ADDR_INTERFACE_CONFIG_A,
+			AD3552R_MASK_SOFTWARE_RESET,
+			AD3552R_MASK_SOFTWARE_RESET, 1);
 		if (ret)
 			return ret;
 	}
@@ -534,19 +530,17 @@ static int ad3552r_hs_setup(struct ad3552r_hs_state *st)
 
 	val = ret;
 
-	ret = ad3552r_qspi_update_reg_bits(st,
-				AD3552R_REG_ADDR_SH_REFERENCE_CONFIG,
-				AD3552R_MASK_REFERENCE_VOLTAGE_SEL,
-				val, 1);
+	ret = ad3552r_update_reg_bits(st, AD3552R_REG_ADDR_SH_REFERENCE_CONFIG,
+				      AD3552R_MASK_REFERENCE_VOLTAGE_SEL,
+				      val, 1);
 	if (ret)
 		return ret;
 
 	ret = ad3552r_get_drive_strength(st->dev, &val);
 	if (!ret) {
-		ret = ad3552r_qspi_update_reg_bits(st,
-					AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
-					AD3552R_MASK_SDO_DRIVE_STRENGTH,
-					val, 1);
+		ret = ad3552r_update_reg_bits(st,
+			AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
+			AD3552R_MASK_SDO_DRIVE_STRENGTH, val, 1);
 		if (ret)
 			return ret;
 	}

-- 
2.47.0

Re: [PATCH v3 6/9] iio: dac: ad3552r-hs: use instruction mode for configuration

[David Lechner]

On 1/10/25 4:24 AM, Angelo Dureghello wrote:
> From: Angelo Dureghello <adureghello@baylibre.com>
> 
> Use "instruction" mode over initial configuration and all other
> non-streaming operations.
> 
> DAC boots in streaming mode as default, and the driver is not
> changing this mode.
> 
> Instruction r/w is still working becouse instruction is processed

s/becouse/because/

> from the DAC after chip select is deasserted, this works until
> loop mode is 0 or greater than the instruction size.
> 
> All initial operations should be more safely done in instruction
> mode, a mode provided for this.
> 
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> ---
>  drivers/iio/dac/ad3552r-hs.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c
> index 27949f207d42..991b11702273 100644
> --- a/drivers/iio/dac/ad3552r-hs.c
> +++ b/drivers/iio/dac/ad3552r-hs.c
> @@ -132,6 +132,13 @@ static int ad3552r_hs_buffer_postenable(struct iio_dev *indio_dev)
>  		return -EINVAL;
>  	}
>  
> +	/* Primary region access, set streaming mode (now in SPI + SDR). */
> +	ret = ad3552r_qspi_update_reg_bits(st,
> +					   AD3552R_REG_ADDR_INTERFACE_CONFIG_B,
> +					   AD3552R_MASK_SINGLE_INST, 0, 1);
> +	if (ret)
> +		return ret;

Do we need to undo this operation before we return in the case of an error
later in this function?

> +
>  	ret = st->data->bus_reg_write(st->back, AD3552R_REG_ADDR_STREAM_MODE,
>  				      loop_len, 1);
>  	if (ret)

Re: [PATCH v3 7/9] iio: dac: ad3552r: share model data structures

[David Lechner]

On 1/10/25 4:24 AM, Angelo Dureghello wrote:
> From: Angelo Dureghello <adureghello@baylibre.com>
> 
> Preparing for new parts to be added also in the hs driver,
> set model data structures in ad3552r-common.c, to be accessible
> from both -hs and non hs driver.
> 
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> ---
Reviewed-by: David Lechner <dlechner@baylibre.com>

Re: [PATCH v3 8/9] iio: dac: ad3552r-hs: add ad3541/2r support

[David Lechner]

On 1/10/25 4:24 AM, Angelo Dureghello wrote:
> From: Angelo Dureghello <adureghello@baylibre.com>
> 
> A new FPGA HDL has been developed from ADI to support ad354xr
> devices.
> 
> Add support for ad3541r and ad3542r with following additions:
> 
> - use common device_info structures for hs and non hs drivers,
> - DMA buffering, use DSPI mode for ad354xr and QSPI for ad355xr,
> - change sample rate to respect number of lanes.
> 
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> ---
>  drivers/iio/dac/ad3552r-common.c |   4 +
>  drivers/iio/dac/ad3552r-hs.c     | 225 ++++++++++++++++++++++++++++++++-------
>  drivers/iio/dac/ad3552r.h        |   3 +
>  3 files changed, 195 insertions(+), 37 deletions(-)
> 

...

> diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c
> index bfb6228c9b9b..4600a9e84dfc 100644
> --- a/drivers/iio/dac/ad3552r-hs.c
> +++ b/drivers/iio/dac/ad3552r-hs.c

...

> @@ -104,6 +136,42 @@ static int ad3552r_hs_write_raw(struct iio_dev *indio_dev,
>  	}
>  }
>  
> +static int ad3552r_hs_set_bus_io_mode_hs(struct ad3552r_hs_state *st)
> +{
> +	int bus_mode;
> +
> +	if (st->model_data->num_spi_data_lanes == 4)
> +		bus_mode = AD3552R_IO_MODE_QSPI;
> +	else
> +		bus_mode = AD3552R_IO_MODE_DSPI;
> +
> +	return st->data->bus_set_io_mode(st->back, bus_mode);
> +}
> +
> +static int ad3552r_hs_set_target_io_mode_hs(struct ad3552r_hs_state *st)
> +{
> +	int mode_target;

u32 would be more natural for register value. Or make an enum for this.

> +
> +	/*
> +	 * Best access for secondary reg area, QSPI where possible,
> +	 * else as DSPI.
> +	 */
> +	if (st->model_data->num_spi_data_lanes == 4)
> +		mode_target = AD3552R_QUAD_SPI;
> +	else
> +		mode_target = AD3552R_DUAL_SPI;
> +
> +	/*
> +	 * Better to not use update here, since generally it is already
> +	 * set as DDR mode, and it's not possible to read in DDR mode.
> +	 */
> +	return st->data->bus_reg_write(st->back,
> +				AD3552R_REG_ADDR_TRANSFER_REGISTER,
> +				FIELD_PREP(AD3552R_MASK_MULTI_IO_MODE,
> +					   mode_target) |
> +				AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE, 1);
> +}
> +
>  static int ad3552r_hs_buffer_postenable(struct iio_dev *indio_dev)
>  {
>  	struct ad3552r_hs_state *st = iio_priv(indio_dev);
> @@ -132,6 +200,11 @@ static int ad3552r_hs_buffer_postenable(struct iio_dev *indio_dev)
>  		return -EINVAL;
>  	}
>  
> +	/*
> +	 * With ad3541/2r support, QSPI pin is held low at reset from HDL,
> +	 * streaming start sequence must respect strictly the order below.
> +	 */
> +
>  	/* Primary region access, set streaming mode (now in SPI + SDR). */
>  	ret = ad3552r_qspi_update_reg_bits(st,
>  					   AD3552R_REG_ADDR_INTERFACE_CONFIG_B,
> @@ -139,48 +212,106 @@ static int ad3552r_hs_buffer_postenable(struct iio_dev *indio_dev)
>  	if (ret)
>  		return ret;
>  
> -	ret = st->data->bus_reg_write(st->back, AD3552R_REG_ADDR_STREAM_MODE,
> +	/*
> +	 * Set target loop len, 0x2c 0r 0x2a, descending loop, and keeping loop

Not sure what 0x2c and 0x2a mean, so comment could be improved. And 0r looks
like a typo.

> +	 * len value so it's not cleared hereafter when enabling streaming mode
> +	 * (cleared by CS_ up).
> +	 */
> +	ret = ad3552r_qspi_update_reg_bits(st,
> +		AD3552R_REG_ADDR_TRANSFER_REGISTER,
> +		AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE,
> +		AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE, 1);
> +	if (ret)
> +		goto exit_err_streaming;
> +
> +	ret = st->data->bus_reg_write(st->back,
> +				      AD3552R_REG_ADDR_STREAM_MODE,
>  				      loop_len, 1);
>  	if (ret)
> -		return ret;
> +		goto exit_err_streaming;
>  
> -	/* Inform DAC chip to switch into DDR mode */
> -	ret = ad3552r_qspi_update_reg_bits(st,
> -					   AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
> -					   AD3552R_MASK_SPI_CONFIG_DDR,
> -					   AD3552R_MASK_SPI_CONFIG_DDR, 1);
> +	/* Setting DDR now, caching current config_d. */
> +	ret = st->data->bus_reg_read(st->back,
> +				     AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
> +				     &st->config_d, 1);
>  	if (ret)
> -		return ret;
> +		goto exit_err_streaming;
> +
> +	st->config_d |= AD3552R_MASK_SPI_CONFIG_DDR;
> +	ret = st->data->bus_reg_write(st->back,
> +				      AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
> +				      st->config_d, 1);
> +	if (ret)
> +		goto exit_err_streaming;
>  
> -	/* Inform DAC IP to go for DDR mode from now on */
>  	ret = iio_backend_ddr_enable(st->back);
> -	if (ret) {
> -		dev_err(st->dev, "could not set DDR mode, not streaming");
> -		goto exit_err;
> -	}
> +	if (ret)
> +		goto exit_err_ddr_mode_target;
> +
> +	/*
> +	 * From here onward mode is DDR, so reading any register is not possible
> +	 * anymore, including calling "ad3552r_qspi_update_reg_bits" function.
> +	 */
> +
> +	/* Set target to best high speed mode (D or QSPI). */
> +	ret = ad3552r_hs_set_target_io_mode_hs(st);
> +	if (ret)
> +		goto exit_err_ddr_mode;
> +
> +	/* Set bus to best high speed mode (D or QSPI). */
> +	ret = ad3552r_hs_set_bus_io_mode_hs(st);
> +	if (ret)
> +		goto exit_err_bus_mode_target;
>  
> +	/*
> +	 * Backend setup must be done now only, or related register values will
> +	 * be disrupted by previous bus accesses.
> +	 */
>  	ret = iio_backend_data_transfer_addr(st->back, val);
>  	if (ret)
> -		goto exit_err;
> +		goto exit_err_bus_mode_target;
>  
>  	ret = iio_backend_data_format_set(st->back, 0, &fmt);
>  	if (ret)
> -		goto exit_err;
> +		goto exit_err_bus_mode_target;
>  
>  	ret = iio_backend_data_stream_enable(st->back);
>  	if (ret)
> -		goto exit_err;
> +		goto exit_err_bus_mode_target;
>  
>  	return 0;
>  
> -exit_err:
> -	ad3552r_qspi_update_reg_bits(st,
> -				     AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
> -				     AD3552R_MASK_SPI_CONFIG_DDR,
> -				     0, 1);
> +exit_err_bus_mode_target:
> +	/* Back to simple SPI, not using update to avoid read. */
> +	st->data->bus_reg_write(st->back, AD3552R_REG_ADDR_TRANSFER_REGISTER,
> +				FIELD_PREP(AD3552R_MASK_MULTI_IO_MODE,
> +					   AD3552R_SPI) |
> +				AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE, 1);
> +
> +	/*
> +	 * Back bus to simple SPI, this must be executed together with above
> +	 * target mode unwind, and can be done only after it.
> +	 */
> +	st->data->bus_set_io_mode(st->back, AD3552R_IO_MODE_SPI);
>  
> +exit_err_ddr_mode:
>  	iio_backend_ddr_disable(st->back);
>  
> +exit_err_ddr_mode_target:
> +	/*
> +	 * Back to SDR. In DDR we cannot read, whatever the mode is, so not
> +	 * using update.
> +	 */
> +	st->data->bus_reg_write(st->back, AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
> +				FIELD_PREP(AD3552R_MASK_SDO_DRIVE_STRENGTH, 1),
> +				1);

Should be using st->config_d & ~AD3552R_MASK_SPI_CONFIG_DDR here instead of
hard-coding FIELD_PREP(AD3552R_MASK_SDO_DRIVE_STRENGTH, 1).

> +
> +exit_err_streaming:
> +	/* Back to single instruction mode, disabling loop. */
> +	st->data->bus_reg_write(st->back, AD3552R_REG_ADDR_INTERFACE_CONFIG_B,
> +				AD3552R_MASK_SINGLE_INST |
> +				AD3552R_MASK_SHORT_INSTRUCTION, 1);
> +
>  	return ret;
>  }
>  

Re: [PATCH v3 9/9] iio: dac: ad3552r-hs: update function name (non functional)

[David Lechner]

On 1/10/25 4:24 AM, Angelo Dureghello wrote:
> From: Angelo Dureghello <adureghello@baylibre.com>
> 
> Update ad3552r_qspi_update_reg_bits function name to a more
> generic name, since used mode can be SIMPLE/DUAL/QUAD SPI.
> 
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> ---
Reviewed-by: David Lechner <dlechner@baylibre.com>

Re: [PATCH v3 2/9] iio: dac: ad3552r-hs: clear reset status flag

[Jonathan Cameron]

On Fri, 10 Jan 2025 11:24:14 +0100
Angelo Dureghello <adureghello@baylibre.com> wrote:

> From: Angelo Dureghello <adureghello@baylibre.com>
> 
> Clear reset status flag, to keep error status register
> clean after reset (ad3552r manual, rev B table 38).
> 
> Reset error flag was left to 1, so debugging registers, the
> "Error Status Register" was dirty (0x01). It is important
> to clear this bit, so if there is any reset event over normal
> working mode, it is possible to detect it.
> 
> Fixes: 0b4d9fe58be8 ("iio: dac: ad3552r: add high-speed platform driver")
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
Ah. I should have checked for newer versions. Anyhow, picked up v2 of patches
1 and 2. 

> ---
>  drivers/iio/dac/ad3552r-hs.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c
> index 216c634f3eaf..8974df625670 100644
> --- a/drivers/iio/dac/ad3552r-hs.c
> +++ b/drivers/iio/dac/ad3552r-hs.c
> @@ -329,6 +329,12 @@ static int ad3552r_hs_setup(struct ad3552r_hs_state *st)
>  		dev_info(st->dev, "Chip ID error. Expected 0x%x, Read 0x%x\n",
>  			 AD3552R_ID, id);
>  
> +	/* Clear reset error flag, see ad3552r manual, rev B table 38. */
> +	ret = st->data->bus_reg_write(st->back, AD3552R_REG_ADDR_ERR_STATUS,
> +				      AD3552R_MASK_RESET_STATUS, 1);
> +	if (ret)
> +		return ret;
> +
>  	ret = st->data->bus_reg_write(st->back,
>  				      AD3552R_REG_ADDR_SH_REFERENCE_CONFIG,
>  				      0, 1);
> 

Re: [PATCH v3 8/9] iio: dac: ad3552r-hs: add ad3541/2r support

[Jonathan Cameron]

On Fri, 10 Jan 2025 11:24:20 +0100
Angelo Dureghello <adureghello@baylibre.com> wrote:

> From: Angelo Dureghello <adureghello@baylibre.com>
> 
> A new FPGA HDL has been developed from ADI to support ad354xr
> devices.
> 
> Add support for ad3541r and ad3542r with following additions:
> 
> - use common device_info structures for hs and non hs drivers,
> - DMA buffering, use DSPI mode for ad354xr and QSPI for ad355xr,
> - change sample rate to respect number of lanes.
> 
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
I think the question I posted on v2 (missing there was a v3)
still applies. Please check that thread.

Jonathan

Re: [PATCH v3 1/9] iio: dac: ad3552r-common: fix ad3541/2r ranges

[Nuno Sá]

On Fri, 2025-01-10 at 11:24 +0100, Angelo Dureghello wrote:
> From: Angelo Dureghello <adureghello@baylibre.com>
> 
> Fix ad3541/2r voltage ranges to be as per ad3542r datasheet,
> rev. C, table 38 (page 57).
> 
> The wrong ad354xr ranges was generating erroneous Vpp output.
> 
> In more details:
> - fix wrong number of ranges, they are 5 ranges, not 6,
> - remove non-existent 0-3V range,
> - adjust order, since ad3552r_find_range() get a wrong index,
>   producing a wrong Vpp as output.
> 
> Retested all the ranges on real hardware, EVALAD3542RFMCZ:
> 
> adi,output-range-microvolt (fdt):
> <(000000) (2500000)>;   ok (Rfbx1, switch 10)
> <(000000) (5000000)>;   ok (Rfbx1, switch 10)
> <(000000) (10000000)>;  ok (Rfbx1, switch 10)
> <(-5000000) (5000000)>; ok (Rfbx2, switch +/- 5)
> <(-2500000) (7500000)>; ok (Rfbx2, switch -2.5/7.5)
> 
> Fixes: 8f2b54824b28 ("drivers:iio:dac: Add AD3552R driver support")
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> ---

Reviewed-by: Nuno Sa <nuno.sa@analog.com>

>  drivers/iio/dac/ad3552r-common.c | 5 ++---
>  drivers/iio/dac/ad3552r.h        | 8 +++-----
>  2 files changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/iio/dac/ad3552r-common.c b/drivers/iio/dac/ad3552r-
> common.c
> index 0f495df2e5ce..03e0864f5084 100644
> --- a/drivers/iio/dac/ad3552r-common.c
> +++ b/drivers/iio/dac/ad3552r-common.c
> @@ -22,11 +22,10 @@ EXPORT_SYMBOL_NS_GPL(ad3552r_ch_ranges, "IIO_AD3552R");
>  
>  const s32 ad3542r_ch_ranges[AD3542R_MAX_RANGES][2] = {
>  	[AD3542R_CH_OUTPUT_RANGE_0__2P5V]	= { 0, 2500 },
> -	[AD3542R_CH_OUTPUT_RANGE_0__3V]		= { 0, 3000 },
>  	[AD3542R_CH_OUTPUT_RANGE_0__5V]		= { 0, 5000 },
>  	[AD3542R_CH_OUTPUT_RANGE_0__10V]	= { 0, 10000 },
> -	[AD3542R_CH_OUTPUT_RANGE_NEG_2P5__7P5V]	= { -2500, 7500 },
> -	[AD3542R_CH_OUTPUT_RANGE_NEG_5__5V]	= { -5000, 5000 }
> +	[AD3542R_CH_OUTPUT_RANGE_NEG_5__5V]	= { -5000, 5000 },
> +	[AD3542R_CH_OUTPUT_RANGE_NEG_2P5__7P5V]	= { -2500, 7500 }
>  };
>  EXPORT_SYMBOL_NS_GPL(ad3542r_ch_ranges, "IIO_AD3552R");
>  
> diff --git a/drivers/iio/dac/ad3552r.h b/drivers/iio/dac/ad3552r.h
> index fd5a3dfd1d1c..4b5581039ae9 100644
> --- a/drivers/iio/dac/ad3552r.h
> +++ b/drivers/iio/dac/ad3552r.h
> @@ -131,7 +131,7 @@
>  #define AD3552R_CH1_ACTIVE				BIT(1)
>  
>  #define AD3552R_MAX_RANGES	5
> -#define AD3542R_MAX_RANGES	6
> +#define AD3542R_MAX_RANGES	5
>  #define AD3552R_QUAD_SPI	2
>  
>  extern const s32 ad3552r_ch_ranges[AD3552R_MAX_RANGES][2];
> @@ -189,16 +189,14 @@ enum ad3552r_ch_vref_select {
>  enum ad3542r_ch_output_range {
>  	/* Range from 0 V to 2.5 V. Requires Rfb1x connection */
>  	AD3542R_CH_OUTPUT_RANGE_0__2P5V,
> -	/* Range from 0 V to 3 V. Requires Rfb1x connection  */
> -	AD3542R_CH_OUTPUT_RANGE_0__3V,
>  	/* Range from 0 V to 5 V. Requires Rfb1x connection  */
>  	AD3542R_CH_OUTPUT_RANGE_0__5V,
>  	/* Range from 0 V to 10 V. Requires Rfb2x connection  */
>  	AD3542R_CH_OUTPUT_RANGE_0__10V,
> -	/* Range from -2.5 V to 7.5 V. Requires Rfb2x connection  */
> -	AD3542R_CH_OUTPUT_RANGE_NEG_2P5__7P5V,
>  	/* Range from -5 V to 5 V. Requires Rfb2x connection  */
>  	AD3542R_CH_OUTPUT_RANGE_NEG_5__5V,
> +	/* Range from -2.5 V to 7.5 V. Requires Rfb2x connection  */
> +	AD3542R_CH_OUTPUT_RANGE_NEG_2P5__7P5V,
>  };
>  
>  enum ad3552r_ch_output_range {
> 

Re: [PATCH v3 2/9] iio: dac: ad3552r-hs: clear reset status flag

[Nuno Sá]

On Sun, 2025-01-12 at 15:04 +0000, Jonathan Cameron wrote:
> On Fri, 10 Jan 2025 11:24:14 +0100
> Angelo Dureghello <adureghello@baylibre.com> wrote:
> 
> > From: Angelo Dureghello <adureghello@baylibre.com>
> > 
> > Clear reset status flag, to keep error status register
> > clean after reset (ad3552r manual, rev B table 38).
> > 
> > Reset error flag was left to 1, so debugging registers, the
> > "Error Status Register" was dirty (0x01). It is important
> > to clear this bit, so if there is any reset event over normal
> > working mode, it is possible to detect it.
> > 
> > Fixes: 0b4d9fe58be8 ("iio: dac: ad3552r: add high-speed platform driver")
> > Reviewed-by: David Lechner <dlechner@baylibre.com>
> > Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> Ah. I should have checked for newer versions. Anyhow, picked up v2 of patches
> 1 and 2.

Oh just saw this now. Anyways, just gave my tag for patch 1. If you can still
take it, feel free to do so. Same for this one:

Reviewed-by: Nuno Sa <nuno.sa@analog.com>

> 
> > ---
> >  drivers/iio/dac/ad3552r-hs.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c
> > index 216c634f3eaf..8974df625670 100644
> > --- a/drivers/iio/dac/ad3552r-hs.c
> > +++ b/drivers/iio/dac/ad3552r-hs.c
> > @@ -329,6 +329,12 @@ static int ad3552r_hs_setup(struct ad3552r_hs_state
> > *st)
> >  		dev_info(st->dev, "Chip ID error. Expected 0x%x, Read
> > 0x%x\n",
> >  			 AD3552R_ID, id);
> >  
> > +	/* Clear reset error flag, see ad3552r manual, rev B table 38. */
> > +	ret = st->data->bus_reg_write(st->back,
> > AD3552R_REG_ADDR_ERR_STATUS,
> > +				      AD3552R_MASK_RESET_STATUS, 1);
> > +	if (ret)
> > +		return ret;
> > +
> >  	ret = st->data->bus_reg_write(st->back,
> >  				      AD3552R_REG_ADDR_SH_REFERENCE_CONFIG,
> >  				      0, 1);
> > 
> 

Re: [PATCH v3 3/9] iio: dac: adi-axi-dac: modify stream enable

[Nuno Sá]

On Fri, 2025-01-10 at 11:24 +0100, Angelo Dureghello wrote:
> From: Angelo Dureghello <adureghello@baylibre.com>
> 
> Change suggested from the AXI HDL team, modify the function
> axi_dac_data_stream_enable() to check for interface busy, to avoid
> possible issues when starting the stream.
> 
> Fixes: e61d7178429a ("iio: dac: adi-axi-dac: extend features")
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> ---

LGTM... One minor thing that come to mind is that if it wouldn't be more
intuitive to return -EBUSY in case of a timeout? Anyways:

Reviewed-by: Nuno Sa <nuno.sa@analog.com>

>  drivers/iio/dac/adi-axi-dac.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/iio/dac/adi-axi-dac.c b/drivers/iio/dac/adi-axi-dac.c
> index b143f7ed6847..ac871deb8063 100644
> --- a/drivers/iio/dac/adi-axi-dac.c
> +++ b/drivers/iio/dac/adi-axi-dac.c
> @@ -585,6 +585,14 @@ static int axi_dac_ddr_disable(struct iio_backend *back)
>  static int axi_dac_data_stream_enable(struct iio_backend *back)
>  {
>  	struct axi_dac_state *st = iio_backend_get_priv(back);
> +	int ret, val;
> +
> +	ret = regmap_read_poll_timeout(st->regmap,
> +				AXI_DAC_UI_STATUS_REG, val,
> +				FIELD_GET(AXI_DAC_UI_STATUS_IF_BUSY, val) ==
> 0,
> +				10, 100 * KILO);
> +	if (ret)
> +		return ret;
>  
>  	return regmap_set_bits(st->regmap, AXI_DAC_CUSTOM_CTRL_REG,
>  			       AXI_DAC_CUSTOM_CTRL_STREAM_ENABLE);
> 

Re: [PATCH v3 4/9] iio: dac: adi-axi-dac: add bus mode setup

[Nuno Sá]

On Fri, 2025-01-10 at 11:24 +0100, Angelo Dureghello wrote:
> From: Angelo Dureghello <adureghello@baylibre.com>
> 
> The ad354xr requires DSPI mode (2 data lanes) to work in buffering
> mode, so backend needs to allow a mode selection between:
>     SPI  (entire ad35xxr family),
>     DSPI (ad354xr),
>     QSPI (ad355xr).
> 

I guess this could be misleading people to think this is being handled by the
backend framework when it's not. I would rephrase things a bit in here.

> About removal of AXI_DAC_CUSTOM_CTRL_SYNCED_TRANSFER, according to
> the HDL history the flag has never been used. So looks like the driver
> was including it by mistake or in anticipation for something that was
> never implemented on HDL side.
> 
> Current HDL updated documentation confirm it is actually not in use
> anymore and replaced by the IO_MODE bits.
> 
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> ---

With the improved change and the inline note:

Reviewed-by: Nuno Sa <nuno.sa@analog.com>

>  drivers/iio/dac/ad3552r-hs.h  |  8 ++++++++
>  drivers/iio/dac/adi-axi-dac.c | 22 +++++++++++++++++++++-
>  2 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/dac/ad3552r-hs.h b/drivers/iio/dac/ad3552r-hs.h
> index 724261d38dea..4a9e35234124 100644
> --- a/drivers/iio/dac/ad3552r-hs.h
> +++ b/drivers/iio/dac/ad3552r-hs.h
> @@ -8,11 +8,19 @@
>  
>  struct iio_backend;
>  
> +enum ad3552r_io_mode {
> +	AD3552R_IO_MODE_SPI,
> +	AD3552R_IO_MODE_DSPI,
> +	AD3552R_IO_MODE_QSPI,
> +};
> +
>  struct ad3552r_hs_platform_data {
>  	int (*bus_reg_read)(struct iio_backend *back, u32 reg, u32 *val,
>  			    size_t data_size);
>  	int (*bus_reg_write)(struct iio_backend *back, u32 reg, u32 val,
>  			     size_t data_size);
> +	int (*bus_set_io_mode)(struct iio_backend *back,
> +			       enum ad3552r_io_mode mode);
>  	u32 bus_sample_data_clock_hz;
>  };
>  
> diff --git a/drivers/iio/dac/adi-axi-dac.c b/drivers/iio/dac/adi-axi-dac.c
> index ac871deb8063..bcaf365feef4 100644
> --- a/drivers/iio/dac/adi-axi-dac.c
> +++ b/drivers/iio/dac/adi-axi-dac.c
> @@ -64,7 +64,7 @@
>  #define   AXI_DAC_UI_STATUS_IF_BUSY		BIT(4)
>  #define AXI_DAC_CUSTOM_CTRL_REG			0x008C
>  #define   AXI_DAC_CUSTOM_CTRL_ADDRESS		GENMASK(31, 24)
> -#define   AXI_DAC_CUSTOM_CTRL_SYNCED_TRANSFER	BIT(2)
> +#define   AXI_DAC_CUSTOM_CTRL_MULTI_IO_MODE	GENMASK(3, 2)
>  #define   AXI_DAC_CUSTOM_CTRL_STREAM		BIT(1)
>  #define   AXI_DAC_CUSTOM_CTRL_TRANSFER_DATA	BIT(0)
>  
> @@ -722,6 +722,25 @@ static int axi_dac_bus_reg_read(struct iio_backend *back,
> u32 reg, u32 *val,
>  	return regmap_read(st->regmap, AXI_DAC_CUSTOM_RD_REG, val);
>  }
>  
> +static int axi_dac_bus_set_io_mode(struct iio_backend *back,
> +				   enum ad3552r_io_mode mode)
> +{
> +	struct axi_dac_state *st = iio_backend_get_priv(back);
> +	int ival, ret;
> +

No harm in doing some validation on 'mode'.

- Nuno Sá

> +	guard(mutex)(&st->lock);
> +
> +	ret = regmap_update_bits(st->regmap, AXI_DAC_CUSTOM_CTRL_REG,
> +			AXI_DAC_CUSTOM_CTRL_MULTI_IO_MODE,
> +			FIELD_PREP(AXI_DAC_CUSTOM_CTRL_MULTI_IO_MODE, mode));
> +	if (ret)
> +		return ret;
> +
> +	return regmap_read_poll_timeout(st->regmap, AXI_DAC_UI_STATUS_REG,
> ival,
> +			FIELD_GET(AXI_DAC_UI_STATUS_IF_BUSY, ival) == 0, 10,
> +			100 * KILO);
> +}
> +
>  static void axi_dac_child_remove(void *data)
>  {
>  	platform_device_unregister(data);
> @@ -733,6 +752,7 @@ static int axi_dac_create_platform_device(struct
> axi_dac_state *st,
>  	struct ad3552r_hs_platform_data pdata = {
>  		.bus_reg_read = axi_dac_bus_reg_read,
>  		.bus_reg_write = axi_dac_bus_reg_write,
> +		.bus_set_io_mode = axi_dac_bus_set_io_mode,
>  		.bus_sample_data_clock_hz = st->dac_clk_rate,
>  	};
>  	struct platform_device_info pi = {
> 

Re: [PATCH v3 5/9] iio: dac: ad3552r-hs: fix message on wrong chip id

[Nuno Sá]

On Fri, 2025-01-10 at 11:24 +0100, Angelo Dureghello wrote:
> From: Angelo Dureghello <adureghello@baylibre.com>
> 
> Set a better info message on wrong chip id, fixing the
> expected value as read from the info struct.
> 
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> ---
>  drivers/iio/dac/ad3552r-hs.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c
> index 8974df625670..27949f207d42 100644
> --- a/drivers/iio/dac/ad3552r-hs.c
> +++ b/drivers/iio/dac/ad3552r-hs.c
> @@ -326,8 +326,9 @@ static int ad3552r_hs_setup(struct ad3552r_hs_state *st)
>  
>  	id |= val << 8;
>  	if (id != st->model_data->chip_id)
> -		dev_info(st->dev, "Chip ID error. Expected 0x%x, Read
> 0x%x\n",
> -			 AD3552R_ID, id);
> +		dev_info(st->dev,
> +			 "Chip ID mismatch, detected 0x%x but expected
> 0x%x\n",
> +			 id, st->model_data->chip_id);

Since you're doing this, I would prefer dev_warn() over dev_info()

- Nuno Sá

>  
>  	/* Clear reset error flag, see ad3552r manual, rev B table 38. */
>  	ret = st->data->bus_reg_write(st->back, AD3552R_REG_ADDR_ERR_STATUS,
> 

Re: [PATCH v3 6/9] iio: dac: ad3552r-hs: use instruction mode for configuration

[Nuno Sá]

On Fri, 2025-01-10 at 09:37 -0600, David Lechner wrote:
> On 1/10/25 4:24 AM, Angelo Dureghello wrote:
> > From: Angelo Dureghello <adureghello@baylibre.com>
> > 
> > Use "instruction" mode over initial configuration and all other
> > non-streaming operations.
> > 
> > DAC boots in streaming mode as default, and the driver is not
> > changing this mode.
> > 
> > Instruction r/w is still working becouse instruction is processed
> 
> s/becouse/because/
> 
> > from the DAC after chip select is deasserted, this works until
> > loop mode is 0 or greater than the instruction size.
> > 
> > All initial operations should be more safely done in instruction
> > mode, a mode provided for this.
> > 
> > Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> > ---
> >  drivers/iio/dac/ad3552r-hs.c | 22 ++++++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> > 
> > diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c
> > index 27949f207d42..991b11702273 100644
> > --- a/drivers/iio/dac/ad3552r-hs.c
> > +++ b/drivers/iio/dac/ad3552r-hs.c
> > @@ -132,6 +132,13 @@ static int ad3552r_hs_buffer_postenable(struct iio_dev
> > *indio_dev)
> >  		return -EINVAL;
> >  	}
> >  
> > +	/* Primary region access, set streaming mode (now in SPI + SDR). */
> > +	ret = ad3552r_qspi_update_reg_bits(st,
> > +					  
> > AD3552R_REG_ADDR_INTERFACE_CONFIG_B,
> > +					   AD3552R_MASK_SINGLE_INST, 0, 1);
> > +	if (ret)
> > +		return ret;
> 
> Do we need to undo this operation before we return in the case of an error
> later in this function?

Seems reasonable to me...

- Nuno Sá

Re: [PATCH v3 7/9] iio: dac: ad3552r: share model data structures

[Nuno Sá]

On Fri, 2025-01-10 at 11:24 +0100, Angelo Dureghello wrote:
> From: Angelo Dureghello <adureghello@baylibre.com>
> 
> Preparing for new parts to be added also in the hs driver,
> set model data structures in ad3552r-common.c, to be accessible
> from both -hs and non hs driver.
> 
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> ---

Reviewed-by: Nuno Sa <nuno.sa@analog.com>

>  drivers/iio/dac/ad3552r-common.c | 46 ++++++++++++++++++++++++++++++++++++---
> -
>  drivers/iio/dac/ad3552r-hs.c     |  8 -------
>  drivers/iio/dac/ad3552r.c        | 36 -------------------------------
>  drivers/iio/dac/ad3552r.h        |  6 ++++--
>  4 files changed, 46 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/iio/dac/ad3552r-common.c b/drivers/iio/dac/ad3552r-
> common.c
> index 03e0864f5084..ded90bf57baf 100644
> --- a/drivers/iio/dac/ad3552r-common.c
> +++ b/drivers/iio/dac/ad3552r-common.c
> @@ -11,23 +11,21 @@
>  
>  #include "ad3552r.h"
>  
> -const s32 ad3552r_ch_ranges[AD3552R_MAX_RANGES][2] = {
> +static const s32 ad3552r_ch_ranges[AD3552R_MAX_RANGES][2] = {
>  	[AD3552R_CH_OUTPUT_RANGE_0__2P5V]	= { 0, 2500 },
>  	[AD3552R_CH_OUTPUT_RANGE_0__5V]		= { 0, 5000 },
>  	[AD3552R_CH_OUTPUT_RANGE_0__10V]	= { 0, 10000 },
>  	[AD3552R_CH_OUTPUT_RANGE_NEG_5__5V]	= { -5000, 5000 },
>  	[AD3552R_CH_OUTPUT_RANGE_NEG_10__10V]	= { -10000, 10000 }
>  };
> -EXPORT_SYMBOL_NS_GPL(ad3552r_ch_ranges, "IIO_AD3552R");
>  
> -const s32 ad3542r_ch_ranges[AD3542R_MAX_RANGES][2] = {
> +static const s32 ad3542r_ch_ranges[AD3542R_MAX_RANGES][2] = {
>  	[AD3542R_CH_OUTPUT_RANGE_0__2P5V]	= { 0, 2500 },
>  	[AD3542R_CH_OUTPUT_RANGE_0__5V]		= { 0, 5000 },
>  	[AD3542R_CH_OUTPUT_RANGE_0__10V]	= { 0, 10000 },
>  	[AD3542R_CH_OUTPUT_RANGE_NEG_5__5V]	= { -5000, 5000 },
>  	[AD3542R_CH_OUTPUT_RANGE_NEG_2P5__7P5V]	= { -2500, 7500 }
>  };
> -EXPORT_SYMBOL_NS_GPL(ad3542r_ch_ranges, "IIO_AD3552R");
>  
>  /* Gain * AD3552R_GAIN_SCALE */
>  static const s32 gains_scaling_table[] = {
> @@ -37,6 +35,46 @@ static const s32 gains_scaling_table[] = {
>  	[AD3552R_CH_GAIN_SCALING_0_125]		= 125
>  };
>  
> +const struct ad3552r_model_data ad3541r_model_data = {
> +	.model_name = "ad3541r",
> +	.chip_id = AD3541R_ID,
> +	.num_hw_channels = 1,
> +	.ranges_table = ad3542r_ch_ranges,
> +	.num_ranges = ARRAY_SIZE(ad3542r_ch_ranges),
> +	.requires_output_range = true,
> +};
> +EXPORT_SYMBOL_NS_GPL(ad3541r_model_data, "IIO_AD3552R");
> +
> +const struct ad3552r_model_data ad3542r_model_data = {
> +	.model_name = "ad3542r",
> +	.chip_id = AD3542R_ID,
> +	.num_hw_channels = 2,
> +	.ranges_table = ad3542r_ch_ranges,
> +	.num_ranges = ARRAY_SIZE(ad3542r_ch_ranges),
> +	.requires_output_range = true,
> +};
> +EXPORT_SYMBOL_NS_GPL(ad3542r_model_data, "IIO_AD3552R");
> +
> +const struct ad3552r_model_data ad3551r_model_data = {
> +	.model_name = "ad3551r",
> +	.chip_id = AD3551R_ID,
> +	.num_hw_channels = 1,
> +	.ranges_table = ad3552r_ch_ranges,
> +	.num_ranges = ARRAY_SIZE(ad3552r_ch_ranges),
> +	.requires_output_range = false,
> +};
> +EXPORT_SYMBOL_NS_GPL(ad3551r_model_data, "IIO_AD3552R");
> +
> +const struct ad3552r_model_data ad3552r_model_data = {
> +	.model_name = "ad3552r",
> +	.chip_id = AD3552R_ID,
> +	.num_hw_channels = 2,
> +	.ranges_table = ad3552r_ch_ranges,
> +	.num_ranges = ARRAY_SIZE(ad3552r_ch_ranges),
> +	.requires_output_range = false,
> +};
> +EXPORT_SYMBOL_NS_GPL(ad3552r_model_data, "IIO_AD3552R");
> +
>  u16 ad3552r_calc_custom_gain(u8 p, u8 n, s16 goffs)
>  {
>  	return FIELD_PREP(AD3552R_MASK_CH_RANGE_OVERRIDE, 1) |
> diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c
> index 991b11702273..bfb6228c9b9b 100644
> --- a/drivers/iio/dac/ad3552r-hs.c
> +++ b/drivers/iio/dac/ad3552r-hs.c
> @@ -527,14 +527,6 @@ static int ad3552r_hs_probe(struct platform_device *pdev)
>  	return devm_iio_device_register(&pdev->dev, indio_dev);
>  }
>  
> -static const struct ad3552r_model_data ad3552r_model_data = {
> -	.model_name = "ad3552r",
> -	.chip_id = AD3552R_ID,
> -	.num_hw_channels = 2,
> -	.ranges_table = ad3552r_ch_ranges,
> -	.num_ranges = ARRAY_SIZE(ad3552r_ch_ranges),
> -};
> -
>  static const struct of_device_id ad3552r_hs_of_id[] = {
>  	{ .compatible = "adi,ad3552r", .data = &ad3552r_model_data },
>  	{ }
> diff --git a/drivers/iio/dac/ad3552r.c b/drivers/iio/dac/ad3552r.c
> index e7206af53af6..9d28e06b80c0 100644
> --- a/drivers/iio/dac/ad3552r.c
> +++ b/drivers/iio/dac/ad3552r.c
> @@ -649,42 +649,6 @@ static int ad3552r_probe(struct spi_device *spi)
>  	return devm_iio_device_register(&spi->dev, indio_dev);
>  }
>  
> -static const struct ad3552r_model_data ad3541r_model_data = {
> -	.model_name = "ad3541r",
> -	.chip_id = AD3541R_ID,
> -	.num_hw_channels = 1,
> -	.ranges_table = ad3542r_ch_ranges,
> -	.num_ranges = ARRAY_SIZE(ad3542r_ch_ranges),
> -	.requires_output_range = true,
> -};
> -
> -static const struct ad3552r_model_data ad3542r_model_data = {
> -	.model_name = "ad3542r",
> -	.chip_id = AD3542R_ID,
> -	.num_hw_channels = 2,
> -	.ranges_table = ad3542r_ch_ranges,
> -	.num_ranges = ARRAY_SIZE(ad3542r_ch_ranges),
> -	.requires_output_range = true,
> -};
> -
> -static const struct ad3552r_model_data ad3551r_model_data = {
> -	.model_name = "ad3551r",
> -	.chip_id = AD3551R_ID,
> -	.num_hw_channels = 1,
> -	.ranges_table = ad3552r_ch_ranges,
> -	.num_ranges = ARRAY_SIZE(ad3552r_ch_ranges),
> -	.requires_output_range = false,
> -};
> -
> -static const struct ad3552r_model_data ad3552r_model_data = {
> -	.model_name = "ad3552r",
> -	.chip_id = AD3552R_ID,
> -	.num_hw_channels = 2,
> -	.ranges_table = ad3552r_ch_ranges,
> -	.num_ranges = ARRAY_SIZE(ad3552r_ch_ranges),
> -	.requires_output_range = false,
> -};
> -
>  static const struct spi_device_id ad3552r_id[] = {
>  	{
>  		.name = "ad3541r",
> diff --git a/drivers/iio/dac/ad3552r.h b/drivers/iio/dac/ad3552r.h
> index 4b5581039ae9..3dc8d1d9c0f9 100644
> --- a/drivers/iio/dac/ad3552r.h
> +++ b/drivers/iio/dac/ad3552r.h
> @@ -134,8 +134,10 @@
>  #define AD3542R_MAX_RANGES	5
>  #define AD3552R_QUAD_SPI	2
>  
> -extern const s32 ad3552r_ch_ranges[AD3552R_MAX_RANGES][2];
> -extern const s32 ad3542r_ch_ranges[AD3542R_MAX_RANGES][2];
> +extern const struct ad3552r_model_data ad3541r_model_data;
> +extern const struct ad3552r_model_data ad3542r_model_data;
> +extern const struct ad3552r_model_data ad3551r_model_data;
> +extern const struct ad3552r_model_data ad3552r_model_data;
>  
>  enum ad3552r_id {
>  	AD3541R_ID = 0x400b,
> 

Re: [PATCH v3 9/9] iio: dac: ad3552r-hs: update function name (non functional)

[Nuno Sá]

On Fri, 2025-01-10 at 11:24 +0100, Angelo Dureghello wrote:
> From: Angelo Dureghello <adureghello@baylibre.com>
> 
> Update ad3552r_qspi_update_reg_bits function name to a more
> generic name, since used mode can be SIMPLE/DUAL/QUAD SPI.
> 
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> ---

Reviewed-by: Nuno Sa <nuno.sa@analog.com>

>  drivers/iio/dac/ad3552r-hs.c | 64 ++++++++++++++++++++-----------------------
> -
>  1 file changed, 29 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c
> index 4600a9e84dfc..7f3a70cfbef8 100644
> --- a/drivers/iio/dac/ad3552r-hs.c
> +++ b/drivers/iio/dac/ad3552r-hs.c
> @@ -56,9 +56,9 @@ struct ad3552r_hs_state {
>  	u32 config_d;
>  };
>  
> -static int ad3552r_qspi_update_reg_bits(struct ad3552r_hs_state *st,
> -					u32 reg, u32 mask, u32 val,
> -					size_t xfer_size)
> +static int ad3552r_update_reg_bits(struct ad3552r_hs_state *st,
> +				   u32 reg, u32 mask, u32 val,
> +				   size_t xfer_size)
>  {
>  	u32 rval;
>  	int ret;
> @@ -206,9 +206,8 @@ static int ad3552r_hs_buffer_postenable(struct iio_dev
> *indio_dev)
>  	 */
>  
>  	/* Primary region access, set streaming mode (now in SPI + SDR). */
> -	ret = ad3552r_qspi_update_reg_bits(st,
> -					  
> AD3552R_REG_ADDR_INTERFACE_CONFIG_B,
> -					   AD3552R_MASK_SINGLE_INST, 0, 1);
> +	ret = ad3552r_update_reg_bits(st,
> AD3552R_REG_ADDR_INTERFACE_CONFIG_B,
> +				      AD3552R_MASK_SINGLE_INST, 0, 1);
>  	if (ret)
>  		return ret;
>  
> @@ -217,10 +216,9 @@ static int ad3552r_hs_buffer_postenable(struct iio_dev
> *indio_dev)
>  	 * len value so it's not cleared hereafter when enabling streaming
> mode
>  	 * (cleared by CS_ up).
>  	 */
> -	ret = ad3552r_qspi_update_reg_bits(st,
> -		AD3552R_REG_ADDR_TRANSFER_REGISTER,
> -		AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE,
> -		AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE, 1);
> +	ret = ad3552r_update_reg_bits(st, AD3552R_REG_ADDR_TRANSFER_REGISTER,
> +				      AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE,
> +				      AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE,
> 1);
>  	if (ret)
>  		goto exit_err_streaming;
>  
> @@ -250,7 +248,7 @@ static int ad3552r_hs_buffer_postenable(struct iio_dev
> *indio_dev)
>  
>  	/*
>  	 * From here onward mode is DDR, so reading any register is not
> possible
> -	 * anymore, including calling "ad3552r_qspi_update_reg_bits"
> function.
> +	 * anymore, including calling "ad3552r_update_reg_bits" function.
>  	 */
>  
>  	/* Set target to best high speed mode (D or QSPI). */
> @@ -351,18 +349,16 @@ static int ad3552r_hs_buffer_predisable(struct iio_dev
> *indio_dev)
>  	 * Back to simple SPI for secondary region too now, so to be able to
>  	 * dump/read registers there too if needed.
>  	 */
> -	ret = ad3552r_qspi_update_reg_bits(st,
> -					  
> AD3552R_REG_ADDR_TRANSFER_REGISTER,
> -					   AD3552R_MASK_MULTI_IO_MODE,
> -					   AD3552R_SPI, 1);
> +	ret = ad3552r_update_reg_bits(st, AD3552R_REG_ADDR_TRANSFER_REGISTER,
> +				      AD3552R_MASK_MULTI_IO_MODE,
> +				      AD3552R_SPI, 1);
>  	if (ret)
>  		return ret;
>  
>  	/* Back to single instruction mode, disabling loop. */
> -	ret = ad3552r_qspi_update_reg_bits(st,
> -					  
> AD3552R_REG_ADDR_INTERFACE_CONFIG_B,
> -					   AD3552R_MASK_SINGLE_INST,
> -					   AD3552R_MASK_SINGLE_INST, 1);
> +	ret = ad3552r_update_reg_bits(st,
> AD3552R_REG_ADDR_INTERFACE_CONFIG_B,
> +				      AD3552R_MASK_SINGLE_INST,
> +				      AD3552R_MASK_SINGLE_INST, 1);
>  	if (ret)
>  		return ret;
>  
> @@ -379,10 +375,10 @@ static inline int ad3552r_hs_set_output_range(struct
> ad3552r_hs_state *st,
>  	else
>  		val = FIELD_PREP(AD3552R_MASK_CH1_RANGE, mode);
>  
> -	return ad3552r_qspi_update_reg_bits(st,
> -
> 					AD3552R_REG_ADDR_CH0_CH1_OUTPUT_RANGE,
> -					AD3552R_MASK_CH_OUTPUT_RANGE_SEL(ch),
> -					val, 1);
> +	return ad3552r_update_reg_bits(st,
> +				       AD3552R_REG_ADDR_CH0_CH1_OUTPUT_RANGE,
> +				       AD3552R_MASK_CH_OUTPUT_RANGE_SEL(ch),
> +				       val, 1);
>  }
>  
>  static int ad3552r_hs_reset(struct ad3552r_hs_state *st)
> @@ -398,10 +394,10 @@ static int ad3552r_hs_reset(struct ad3552r_hs_state *st)
>  		fsleep(10);
>  		gpiod_set_value_cansleep(st->reset_gpio, 0);
>  	} else {
> -		ret = ad3552r_qspi_update_reg_bits(st,
> -					AD3552R_REG_ADDR_INTERFACE_CONFIG_A,
> -					AD3552R_MASK_SOFTWARE_RESET,
> -					AD3552R_MASK_SOFTWARE_RESET, 1);
> +		ret = ad3552r_update_reg_bits(st,
> +			AD3552R_REG_ADDR_INTERFACE_CONFIG_A,
> +			AD3552R_MASK_SOFTWARE_RESET,
> +			AD3552R_MASK_SOFTWARE_RESET, 1);
>  		if (ret)
>  			return ret;
>  	}
> @@ -534,19 +530,17 @@ static int ad3552r_hs_setup(struct ad3552r_hs_state *st)
>  
>  	val = ret;
>  
> -	ret = ad3552r_qspi_update_reg_bits(st,
> -				AD3552R_REG_ADDR_SH_REFERENCE_CONFIG,
> -				AD3552R_MASK_REFERENCE_VOLTAGE_SEL,
> -				val, 1);
> +	ret = ad3552r_update_reg_bits(st,
> AD3552R_REG_ADDR_SH_REFERENCE_CONFIG,
> +				      AD3552R_MASK_REFERENCE_VOLTAGE_SEL,
> +				      val, 1);
>  	if (ret)
>  		return ret;
>  
>  	ret = ad3552r_get_drive_strength(st->dev, &val);
>  	if (!ret) {
> -		ret = ad3552r_qspi_update_reg_bits(st,
> -					AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
> -					AD3552R_MASK_SDO_DRIVE_STRENGTH,
> -					val, 1);
> +		ret = ad3552r_update_reg_bits(st,
> +			AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
> +			AD3552R_MASK_SDO_DRIVE_STRENGTH, val, 1);
>  		if (ret)
>  			return ret;
>  	}
> 

Re: [PATCH v3 2/9] iio: dac: ad3552r-hs: clear reset status flag

[Angelo Dureghello]

Hi,

On 13.01.2025 15:58, Nuno Sá wrote:
> On Sun, 2025-01-12 at 15:04 +0000, Jonathan Cameron wrote:
> > On Fri, 10 Jan 2025 11:24:14 +0100
> > Angelo Dureghello <adureghello@baylibre.com> wrote:
> > 
> > > From: Angelo Dureghello <adureghello@baylibre.com>
> > > 
> > > Clear reset status flag, to keep error status register
> > > clean after reset (ad3552r manual, rev B table 38).
> > > 
> > > Reset error flag was left to 1, so debugging registers, the
> > > "Error Status Register" was dirty (0x01). It is important
> > > to clear this bit, so if there is any reset event over normal
> > > working mode, it is possible to detect it.
> > > 
> > > Fixes: 0b4d9fe58be8 ("iio: dac: ad3552r: add high-speed platform driver")
> > > Reviewed-by: David Lechner <dlechner@baylibre.com>
> > > Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> > Ah. I should have checked for newer versions. Anyhow, picked up v2 of patches
> > 1 and 2.
> 
> Oh just saw this now. Anyways, just gave my tag for patch 1. If you can still
> take it, feel free to do so. Same for this one:
> 
> Reviewed-by: Nuno Sa <nuno.sa@analog.com>
> 

thanks a lot.

Patch 1/9 has no changes, i'll re post it anyway with updated tags.
Patch 2/9 has a dev_err removal, should eventually be re-applied.

Will track this on the changhe log.


> > 
> > > ---
> > >  drivers/iio/dac/ad3552r-hs.c | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c
> > > index 216c634f3eaf..8974df625670 100644
> > > --- a/drivers/iio/dac/ad3552r-hs.c
> > > +++ b/drivers/iio/dac/ad3552r-hs.c
> > > @@ -329,6 +329,12 @@ static int ad3552r_hs_setup(struct ad3552r_hs_state
> > > *st)
> > >  		dev_info(st->dev, "Chip ID error. Expected 0x%x, Read
> > > 0x%x\n",
> > >  			 AD3552R_ID, id);
> > >  
> > > +	/* Clear reset error flag, see ad3552r manual, rev B table 38. */
> > > +	ret = st->data->bus_reg_write(st->back,
> > > AD3552R_REG_ADDR_ERR_STATUS,
> > > +				      AD3552R_MASK_RESET_STATUS, 1);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > >  	ret = st->data->bus_reg_write(st->back,
> > >  				      AD3552R_REG_ADDR_SH_REFERENCE_CONFIG,
> > >  				      0, 1);
> > > 
> > 
> 


Regards,
angelo

Re: [PATCH v3 9/9] iio: dac: ad3552r-hs: update function name (non functional)

[Angelo Dureghello]

On 13.01.2025 16:11, Nuno Sá wrote:
> On Fri, 2025-01-10 at 11:24 +0100, Angelo Dureghello wrote:
> > From: Angelo Dureghello <adureghello@baylibre.com>
> > 
> > Update ad3552r_qspi_update_reg_bits function name to a more
> > generic name, since used mode can be SIMPLE/DUAL/QUAD SPI.
> > 
> > Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> > ---
> 
> Reviewed-by: Nuno Sa <nuno.sa@analog.com>
>

I think i have to use driver prefix "ad3552r_hs", so ad3552r_hs_update_reg_bits
not ad3552r_update_reg_bits.

Going to adjust it.

> >  drivers/iio/dac/ad3552r-hs.c | 64 ++++++++++++++++++++-----------------------
> > -
> >  1 file changed, 29 insertions(+), 35 deletions(-)
> > 
> > diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c
> > index 4600a9e84dfc..7f3a70cfbef8 100644
> > --- a/drivers/iio/dac/ad3552r-hs.c
> > +++ b/drivers/iio/dac/ad3552r-hs.c
> > @@ -56,9 +56,9 @@ struct ad3552r_hs_state {
> >  	u32 config_d;
> >  };
> >  
> > -static int ad3552r_qspi_update_reg_bits(struct ad3552r_hs_state *st,
> > -					u32 reg, u32 mask, u32 val,
> > -					size_t xfer_size)
> > +static int ad3552r_update_reg_bits(struct ad3552r_hs_state *st,
> > +				   u32 reg, u32 mask, u32 val,
> > +				   size_t xfer_size)
> >  {
> >  	u32 rval;
> >  	int ret;
> > @@ -206,9 +206,8 @@ static int ad3552r_hs_buffer_postenable(struct iio_dev
> > *indio_dev)
> >  	 */
> >  
> >  	/* Primary region access, set streaming mode (now in SPI + SDR). */
> > -	ret = ad3552r_qspi_update_reg_bits(st,
> > -					  
> > AD3552R_REG_ADDR_INTERFACE_CONFIG_B,
> > -					   AD3552R_MASK_SINGLE_INST, 0, 1);
> > +	ret = ad3552r_update_reg_bits(st,
> > AD3552R_REG_ADDR_INTERFACE_CONFIG_B,
> > +				      AD3552R_MASK_SINGLE_INST, 0, 1);
> >  	if (ret)
> >  		return ret;
> >  
> > @@ -217,10 +216,9 @@ static int ad3552r_hs_buffer_postenable(struct iio_dev
> > *indio_dev)
> >  	 * len value so it's not cleared hereafter when enabling streaming
> > mode
> >  	 * (cleared by CS_ up).
> >  	 */
> > -	ret = ad3552r_qspi_update_reg_bits(st,
> > -		AD3552R_REG_ADDR_TRANSFER_REGISTER,
> > -		AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE,
> > -		AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE, 1);
> > +	ret = ad3552r_update_reg_bits(st, AD3552R_REG_ADDR_TRANSFER_REGISTER,
> > +				      AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE,
> > +				      AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE,
> > 1);
> >  	if (ret)
> >  		goto exit_err_streaming;
> >  
> > @@ -250,7 +248,7 @@ static int ad3552r_hs_buffer_postenable(struct iio_dev
> > *indio_dev)
> >  
> >  	/*
> >  	 * From here onward mode is DDR, so reading any register is not
> > possible
> > -	 * anymore, including calling "ad3552r_qspi_update_reg_bits"
> > function.
> > +	 * anymore, including calling "ad3552r_update_reg_bits" function.
> >  	 */
> >  
> >  	/* Set target to best high speed mode (D or QSPI). */
> > @@ -351,18 +349,16 @@ static int ad3552r_hs_buffer_predisable(struct iio_dev
> > *indio_dev)
> >  	 * Back to simple SPI for secondary region too now, so to be able to
> >  	 * dump/read registers there too if needed.
> >  	 */
> > -	ret = ad3552r_qspi_update_reg_bits(st,
> > -					  
> > AD3552R_REG_ADDR_TRANSFER_REGISTER,
> > -					   AD3552R_MASK_MULTI_IO_MODE,
> > -					   AD3552R_SPI, 1);
> > +	ret = ad3552r_update_reg_bits(st, AD3552R_REG_ADDR_TRANSFER_REGISTER,
> > +				      AD3552R_MASK_MULTI_IO_MODE,
> > +				      AD3552R_SPI, 1);
> >  	if (ret)
> >  		return ret;
> >  
> >  	/* Back to single instruction mode, disabling loop. */
> > -	ret = ad3552r_qspi_update_reg_bits(st,
> > -					  
> > AD3552R_REG_ADDR_INTERFACE_CONFIG_B,
> > -					   AD3552R_MASK_SINGLE_INST,
> > -					   AD3552R_MASK_SINGLE_INST, 1);
> > +	ret = ad3552r_update_reg_bits(st,
> > AD3552R_REG_ADDR_INTERFACE_CONFIG_B,
> > +				      AD3552R_MASK_SINGLE_INST,
> > +				      AD3552R_MASK_SINGLE_INST, 1);
> >  	if (ret)
> >  		return ret;
> >  
> > @@ -379,10 +375,10 @@ static inline int ad3552r_hs_set_output_range(struct
> > ad3552r_hs_state *st,
> >  	else
> >  		val = FIELD_PREP(AD3552R_MASK_CH1_RANGE, mode);
> >  
> > -	return ad3552r_qspi_update_reg_bits(st,
> > -
> > 					AD3552R_REG_ADDR_CH0_CH1_OUTPUT_RANGE,
> > -					AD3552R_MASK_CH_OUTPUT_RANGE_SEL(ch),
> > -					val, 1);
> > +	return ad3552r_update_reg_bits(st,
> > +				       AD3552R_REG_ADDR_CH0_CH1_OUTPUT_RANGE,
> > +				       AD3552R_MASK_CH_OUTPUT_RANGE_SEL(ch),
> > +				       val, 1);
> >  }
> >  
> >  static int ad3552r_hs_reset(struct ad3552r_hs_state *st)
> > @@ -398,10 +394,10 @@ static int ad3552r_hs_reset(struct ad3552r_hs_state *st)
> >  		fsleep(10);
> >  		gpiod_set_value_cansleep(st->reset_gpio, 0);
> >  	} else {
> > -		ret = ad3552r_qspi_update_reg_bits(st,
> > -					AD3552R_REG_ADDR_INTERFACE_CONFIG_A,
> > -					AD3552R_MASK_SOFTWARE_RESET,
> > -					AD3552R_MASK_SOFTWARE_RESET, 1);
> > +		ret = ad3552r_update_reg_bits(st,
> > +			AD3552R_REG_ADDR_INTERFACE_CONFIG_A,
> > +			AD3552R_MASK_SOFTWARE_RESET,
> > +			AD3552R_MASK_SOFTWARE_RESET, 1);
> >  		if (ret)
> >  			return ret;
> >  	}
> > @@ -534,19 +530,17 @@ static int ad3552r_hs_setup(struct ad3552r_hs_state *st)
> >  
> >  	val = ret;
> >  
> > -	ret = ad3552r_qspi_update_reg_bits(st,
> > -				AD3552R_REG_ADDR_SH_REFERENCE_CONFIG,
> > -				AD3552R_MASK_REFERENCE_VOLTAGE_SEL,
> > -				val, 1);
> > +	ret = ad3552r_update_reg_bits(st,
> > AD3552R_REG_ADDR_SH_REFERENCE_CONFIG,
> > +				      AD3552R_MASK_REFERENCE_VOLTAGE_SEL,
> > +				      val, 1);
> >  	if (ret)
> >  		return ret;
> >  
> >  	ret = ad3552r_get_drive_strength(st->dev, &val);
> >  	if (!ret) {
> > -		ret = ad3552r_qspi_update_reg_bits(st,
> > -					AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
> > -					AD3552R_MASK_SDO_DRIVE_STRENGTH,
> > -					val, 1);
> > +		ret = ad3552r_update_reg_bits(st,
> > +			AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
> > +			AD3552R_MASK_SDO_DRIVE_STRENGTH, val, 1);
> >  		if (ret)
> >  			return ret;
> >  	}
> > 
>