[PATCH 0/8] iio: ad3552r-hs: add support for ad3541/42r

[PATCH 0/8] iio: ad3552r-hs: add support for ad3541/42r

[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>
---
Angelo Dureghello (7):
      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: exit for error on wrong chip id
      iio: dac: ad3552r-hs: add ad3541/2r support
      iio: dac: ad3552r-hs: update function name (non functional)

Antoniu Miclaus (1):
      iio: backend: add API for interface configuration

 drivers/iio/dac/ad3552r-common.c   |  49 +++++-
 drivers/iio/dac/ad3552r-hs.c       | 305 +++++++++++++++++++++++++++++--------
 drivers/iio/dac/ad3552r.c          |  36 -----
 drivers/iio/dac/ad3552r.h          |  16 +-
 drivers/iio/dac/adi-axi-dac.c      |  57 ++++++-
 drivers/iio/industrialio-backend.c |  42 +++++
 include/linux/iio/backend.h        |  19 +++
 7 files changed, 414 insertions(+), 110 deletions(-)
---
base-commit: 5f8c6f117400b7b21ad248959ae2cb6e0d634e97
change-id: 20241216-wip-bl-ad3552r-axi-v0-iio-testing-carlos-119d780305c5

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

[PATCH 1/8] 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).

Fixes: 8f2b54824b28 ("drivers:iio:dac: Add AD3552R driver support")
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

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

[Jonathan Cameron]

On Mon, 16 Dec 2024 21:36:21 +0100
Angelo Dureghello <adureghello@baylibre.com> wrote:

> From: Angelo Dureghello <adureghello@baylibre.com>
> 
> Fix ad3541/2r voltage ranges to be as per ad3542r datasheet,
> rev. C, table 38 (page 57).
> 
> Fixes: 8f2b54824b28 ("drivers:iio:dac: Add AD3552R driver support")
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>

Could you provide a brief statement of the impact of these being wrong.
What does it look like to userspace / configuration of what we are
measuring?

Thanks,

Jonathan

> ---
>  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 {
> 

[PATCH 2/8] 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).

Fixes: 0b4d9fe58be8 ("iio: dac: ad3552r: add high-speed platform driver")
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

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

[Jonathan Cameron]

On Mon, 16 Dec 2024 21:36:22 +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).
> 
> Fixes: 0b4d9fe58be8 ("iio: dac: ad3552r: add high-speed platform driver")
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
Again, something on what the visible effects of this area.
That helps people decide whether to backport.

Thanks,

Jonathan

> ---
>  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);
> 

[PATCH 3/8] 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 | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/iio/dac/adi-axi-dac.c b/drivers/iio/dac/adi-axi-dac.c
index b143f7ed6847..d02eb535b648 100644
--- a/drivers/iio/dac/adi-axi-dac.c
+++ b/drivers/iio/dac/adi-axi-dac.c
@@ -585,6 +585,17 @@ 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) {
+		if (ret == -ETIMEDOUT)
+			dev_err(st->dev, "AXI read timeout\n");
+		return ret;
+	}
 
 	return regmap_set_bits(st->regmap, AXI_DAC_CUSTOM_CTRL_REG,
 			       AXI_DAC_CUSTOM_CTRL_STREAM_ENABLE);

-- 
2.47.0

[PATCH 4/8] iio: backend: add API for interface configuration

[Angelo Dureghello]

From: Antoniu Miclaus <antoniu.miclaus@analog.com>

Add backend support for setting and getting the interface type
in use.

Link: https://lore.kernel.org/linux-iio/20241129153546.63584-1-antoniu.miclaus@analog.com/T/#m6d86939078d780512824f1540145aade38b0990b
Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
Co-developed-by: Angelo Dureghello <adureghello@baylibre.com>
Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
This patch has been picked up from the Antoniu patchset
still not accepted, and extended with the interface setter,
fixing also namespace names to be between quotation marks.
---
 drivers/iio/industrialio-backend.c | 42 ++++++++++++++++++++++++++++++++++++++
 include/linux/iio/backend.h        | 19 +++++++++++++++++
 2 files changed, 61 insertions(+)

diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
index 363281272035..6edc3e685f6a 100644
--- a/drivers/iio/industrialio-backend.c
+++ b/drivers/iio/industrialio-backend.c
@@ -636,6 +636,48 @@ ssize_t iio_backend_ext_info_set(struct iio_dev *indio_dev, uintptr_t private,
 }
 EXPORT_SYMBOL_NS_GPL(iio_backend_ext_info_set, "IIO_BACKEND");
 
+/**
+ * iio_backend_interface_type_get - get the interface type used.
+ * @back: Backend device
+ * @type: Interface type
+ *
+ * RETURNS:
+ * 0 on success, negative error number on failure.
+ */
+int iio_backend_interface_type_get(struct iio_backend *back,
+				   enum iio_backend_interface_type *type)
+{
+	int ret;
+
+	ret = iio_backend_op_call(back, interface_type_get, type);
+	if (ret)
+		return ret;
+
+	if (*type >= IIO_BACKEND_INTERFACE_MAX)
+		return -EINVAL;
+
+	return 0;
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_interface_type_get, "IIO_BACKEND");
+
+/**
+ * iio_backend_interface_type_set - set the interface type used.
+ * @back: Backend device
+ * @type: Interface type
+ *
+ * RETURNS:
+ * 0 on success, negative error number on failure.
+ */
+int iio_backend_interface_type_set(struct iio_backend *back,
+				   enum iio_backend_interface_type type)
+{
+	if (type >= IIO_BACKEND_INTERFACE_MAX)
+		return -EINVAL;
+
+	return  iio_backend_op_call(back, interface_type_set, type);
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_interface_type_set, "IIO_BACKEND");
+
 /**
  * iio_backend_extend_chan_spec - Extend an IIO channel
  * @back: Backend device
diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h
index 10be00f3b120..2b7221099d8c 100644
--- a/include/linux/iio/backend.h
+++ b/include/linux/iio/backend.h
@@ -70,6 +70,15 @@ enum iio_backend_sample_trigger {
 	IIO_BACKEND_SAMPLE_TRIGGER_MAX
 };
 
+enum iio_backend_interface_type {
+	IIO_BACKEND_INTERFACE_SERIAL_LVDS,
+	IIO_BACKEND_INTERFACE_SERIAL_CMOS,
+	IIO_BACKEND_INTERFACE_SERIAL_SPI,
+	IIO_BACKEND_INTERFACE_SERIAL_DSPI,
+	IIO_BACKEND_INTERFACE_SERIAL_QSPI,
+	IIO_BACKEND_INTERFACE_MAX
+};
+
 /**
  * struct iio_backend_ops - operations structure for an iio_backend
  * @enable: Enable backend.
@@ -88,6 +97,8 @@ enum iio_backend_sample_trigger {
  * @extend_chan_spec: Extend an IIO channel.
  * @ext_info_set: Extended info setter.
  * @ext_info_get: Extended info getter.
+ * @interface_type_get: Interface type.
+ * @interface_type_set: Interface type setter.
  * @read_raw: Read a channel attribute from a backend device
  * @debugfs_print_chan_status: Print channel status into a buffer.
  * @debugfs_reg_access: Read or write register value of backend.
@@ -128,6 +139,10 @@ struct iio_backend_ops {
 			    const char *buf, size_t len);
 	int (*ext_info_get)(struct iio_backend *back, uintptr_t private,
 			    const struct iio_chan_spec *chan, char *buf);
+	int (*interface_type_get)(struct iio_backend *back,
+				  enum iio_backend_interface_type *type);
+	int (*interface_type_set)(struct iio_backend *back,
+				  enum iio_backend_interface_type type);
 	int (*read_raw)(struct iio_backend *back,
 			struct iio_chan_spec const *chan, int *val, int *val2,
 			long mask);
@@ -186,6 +201,10 @@ ssize_t iio_backend_ext_info_set(struct iio_dev *indio_dev, uintptr_t private,
 				 const char *buf, size_t len);
 ssize_t iio_backend_ext_info_get(struct iio_dev *indio_dev, uintptr_t private,
 				 const struct iio_chan_spec *chan, char *buf);
+int iio_backend_interface_type_get(struct iio_backend *back,
+				   enum iio_backend_interface_type *type);
+int iio_backend_interface_type_set(struct iio_backend *back,
+				   enum iio_backend_interface_type type);
 int iio_backend_read_raw(struct iio_backend *back,
 			 struct iio_chan_spec const *chan, int *val, int *val2,
 			 long mask);

-- 
2.47.0

Re: [PATCH 4/8] iio: backend: add API for interface configuration

[Nuno Sá]

On Mon, 2024-12-16 at 21:36 +0100, Angelo Dureghello wrote:
> From: Antoniu Miclaus <antoniu.miclaus@analog.com>
> 
> Add backend support for setting and getting the interface type
> in use.
> 
> Link:
> https://lore.kernel.org/linux-iio/20241129153546.63584-1-antoniu.miclaus@analog.com/T/#m6d86939078d780512824f1540145aade38b0990b
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> Co-developed-by: Angelo Dureghello <adureghello@baylibre.com>
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> ---
> This patch has been picked up from the Antoniu patchset
> still not accepted, and extended with the interface setter,
> fixing also namespace names to be between quotation marks.
> ---
>  drivers/iio/industrialio-backend.c | 42
> ++++++++++++++++++++++++++++++++++++++
>  include/linux/iio/backend.h        | 19 +++++++++++++++++
>  2 files changed, 61 insertions(+)
> 
> diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-
> backend.c
> index 363281272035..6edc3e685f6a 100644
> --- a/drivers/iio/industrialio-backend.c
> +++ b/drivers/iio/industrialio-backend.c
> @@ -636,6 +636,48 @@ ssize_t iio_backend_ext_info_set(struct iio_dev
> *indio_dev, uintptr_t private,
>  }
>  EXPORT_SYMBOL_NS_GPL(iio_backend_ext_info_set, "IIO_BACKEND");
>  
> +/**
> + * iio_backend_interface_type_get - get the interface type used.
> + * @back: Backend device
> + * @type: Interface type
> + *
> + * RETURNS:
> + * 0 on success, negative error number on failure.
> + */
> +int iio_backend_interface_type_get(struct iio_backend *back,
> +				   enum iio_backend_interface_type *type)
> +{
> +	int ret;
> +
> +	ret = iio_backend_op_call(back, interface_type_get, type);
> +	if (ret)
> +		return ret;
> +
> +	if (*type >= IIO_BACKEND_INTERFACE_MAX)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_NS_GPL(iio_backend_interface_type_get, "IIO_BACKEND");
> +
> +/**
> + * iio_backend_interface_type_set - set the interface type used.
> + * @back: Backend device
> + * @type: Interface type
> + *
> + * RETURNS:
> + * 0 on success, negative error number on failure.
> + */
> +int iio_backend_interface_type_set(struct iio_backend *back,
> +				   enum iio_backend_interface_type type)
> +{
> +	if (type >= IIO_BACKEND_INTERFACE_MAX)
> +		return -EINVAL;
> +
> +	return  iio_backend_op_call(back, interface_type_set, type);
> +}
> +EXPORT_SYMBOL_NS_GPL(iio_backend_interface_type_set, "IIO_BACKEND");
> +
>  /**
>   * iio_backend_extend_chan_spec - Extend an IIO channel
>   * @back: Backend device
> diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h
> index 10be00f3b120..2b7221099d8c 100644
> --- a/include/linux/iio/backend.h
> +++ b/include/linux/iio/backend.h
> @@ -70,6 +70,15 @@ enum iio_backend_sample_trigger {
>  	IIO_BACKEND_SAMPLE_TRIGGER_MAX
>  };
>  
> +enum iio_backend_interface_type {
> +	IIO_BACKEND_INTERFACE_SERIAL_LVDS,
> +	IIO_BACKEND_INTERFACE_SERIAL_CMOS,

The above are apparently not used in the next patch so I would not add them now.
> +	IIO_BACKEND_INTERFACE_SERIAL_SPI,
> +	IIO_BACKEND_INTERFACE_SERIAL_DSPI,
> +	IIO_BACKEND_INTERFACE_SERIAL_QSPI,

I'll throw my 2 cents but it would be nice to have more feedback on this. I'm
not completely sure about the xSPI stuff in here. We treated the QSPI as a bus
both for control and data in which we also add child devices. And we've been
adding specific bus operations/configurations through the 'struct
ad3552r_hs_platform_data' interface. So, I'm wondering if this should also not
be set through that interface?

LVDS/CMOS still looks slightly different to me...

- Nuno Sá

Re: [PATCH 4/8] iio: backend: add API for interface configuration

[Jonathan Cameron]

On Tue, 17 Dec 2024 11:13:59 +0100
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Mon, 2024-12-16 at 21:36 +0100, Angelo Dureghello wrote:
> > From: Antoniu Miclaus <antoniu.miclaus@analog.com>
> > 
> > Add backend support for setting and getting the interface type
> > in use.
> > 
> > Link:
> > https://lore.kernel.org/linux-iio/20241129153546.63584-1-antoniu.miclaus@analog.com/T/#m6d86939078d780512824f1540145aade38b0990b
> > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> > Co-developed-by: Angelo Dureghello <adureghello@baylibre.com>
> > Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> > ---
> > This patch has been picked up from the Antoniu patchset
> > still not accepted, and extended with the interface setter,
> > fixing also namespace names to be between quotation marks.
> > ---
> >  drivers/iio/industrialio-backend.c | 42
> > ++++++++++++++++++++++++++++++++++++++
> >  include/linux/iio/backend.h        | 19 +++++++++++++++++
> >  2 files changed, 61 insertions(+)
> > 
> > diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-
> > backend.c
> > index 363281272035..6edc3e685f6a 100644
> > --- a/drivers/iio/industrialio-backend.c
> > +++ b/drivers/iio/industrialio-backend.c
> > @@ -636,6 +636,48 @@ ssize_t iio_backend_ext_info_set(struct iio_dev
> > *indio_dev, uintptr_t private,
> >  }
> >  EXPORT_SYMBOL_NS_GPL(iio_backend_ext_info_set, "IIO_BACKEND");
> >  
> > +/**
> > + * iio_backend_interface_type_get - get the interface type used.
> > + * @back: Backend device
> > + * @type: Interface type
> > + *
> > + * RETURNS:
> > + * 0 on success, negative error number on failure.
> > + */
> > +int iio_backend_interface_type_get(struct iio_backend *back,
> > +				   enum iio_backend_interface_type *type)
> > +{
> > +	int ret;
> > +
> > +	ret = iio_backend_op_call(back, interface_type_get, type);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (*type >= IIO_BACKEND_INTERFACE_MAX)
> > +		return -EINVAL;
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_NS_GPL(iio_backend_interface_type_get, "IIO_BACKEND");
> > +
> > +/**
> > + * iio_backend_interface_type_set - set the interface type used.
> > + * @back: Backend device
> > + * @type: Interface type
> > + *
> > + * RETURNS:
> > + * 0 on success, negative error number on failure.
> > + */
> > +int iio_backend_interface_type_set(struct iio_backend *back,
> > +				   enum iio_backend_interface_type type)
> > +{
> > +	if (type >= IIO_BACKEND_INTERFACE_MAX)
> > +		return -EINVAL;
> > +
> > +	return  iio_backend_op_call(back, interface_type_set, type);
> > +}
> > +EXPORT_SYMBOL_NS_GPL(iio_backend_interface_type_set, "IIO_BACKEND");
> > +
> >  /**
> >   * iio_backend_extend_chan_spec - Extend an IIO channel
> >   * @back: Backend device
> > diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h
> > index 10be00f3b120..2b7221099d8c 100644
> > --- a/include/linux/iio/backend.h
> > +++ b/include/linux/iio/backend.h
> > @@ -70,6 +70,15 @@ enum iio_backend_sample_trigger {
> >  	IIO_BACKEND_SAMPLE_TRIGGER_MAX
> >  };
> >  
> > +enum iio_backend_interface_type {
> > +	IIO_BACKEND_INTERFACE_SERIAL_LVDS,
> > +	IIO_BACKEND_INTERFACE_SERIAL_CMOS,  
> 
> The above are apparently not used in the next patch so I would not add them now.
> > +	IIO_BACKEND_INTERFACE_SERIAL_SPI,
> > +	IIO_BACKEND_INTERFACE_SERIAL_DSPI,
> > +	IIO_BACKEND_INTERFACE_SERIAL_QSPI,  
> 
> I'll throw my 2 cents but it would be nice to have more feedback on this. I'm
> not completely sure about the xSPI stuff in here. We treated the QSPI as a bus
> both for control and data in which we also add child devices. And we've been
> adding specific bus operations/configurations through the 'struct
> ad3552r_hs_platform_data' interface. So, I'm wondering if this should also not
> be set through that interface?

Maybe - kind of hard to tell until we actually have code.
I'd go for kicking them into the long grass for now if they aren't used and
just dropping them from this patch.  If we ever need them,easy to bring back
and then we should have a justification for why!

J


> 
> LVDS/CMOS still looks slightly different to me...
> 
> - Nuno Sá
> 
> 
> 

Re: [PATCH 4/8] iio: backend: add API for interface configuration

[Jonathan Cameron]

On Thu, 19 Dec 2024 16:42:33 +0000
Jonathan Cameron <jic23@kernel.org> wrote:

> On Tue, 17 Dec 2024 11:13:59 +0100
> Nuno Sá <noname.nuno@gmail.com> wrote:
> 
> > On Mon, 2024-12-16 at 21:36 +0100, Angelo Dureghello wrote:  
> > > From: Antoniu Miclaus <antoniu.miclaus@analog.com>
> > > 
> > > Add backend support for setting and getting the interface type
> > > in use.
> > > 
> > > Link:
> > > https://lore.kernel.org/linux-iio/20241129153546.63584-1-antoniu.miclaus@analog.com/T/#m6d86939078d780512824f1540145aade38b0990b
> > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> > > Co-developed-by: Angelo Dureghello <adureghello@baylibre.com>
> > > Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> > > ---
> > > This patch has been picked up from the Antoniu patchset
> > > still not accepted, and extended with the interface setter,
> > > fixing also namespace names to be between quotation marks.
> > > ---
> > >  drivers/iio/industrialio-backend.c | 42
> > > ++++++++++++++++++++++++++++++++++++++
> > >  include/linux/iio/backend.h        | 19 +++++++++++++++++
> > >  2 files changed, 61 insertions(+)
> > > 
> > > diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-
> > > backend.c
> > > index 363281272035..6edc3e685f6a 100644
> > > --- a/drivers/iio/industrialio-backend.c
> > > +++ b/drivers/iio/industrialio-backend.c
> > > @@ -636,6 +636,48 @@ ssize_t iio_backend_ext_info_set(struct iio_dev
> > > *indio_dev, uintptr_t private,
> > >  }
> > >  EXPORT_SYMBOL_NS_GPL(iio_backend_ext_info_set, "IIO_BACKEND");
> > >  
> > > +/**
> > > + * iio_backend_interface_type_get - get the interface type used.
> > > + * @back: Backend device
> > > + * @type: Interface type
> > > + *
> > > + * RETURNS:
> > > + * 0 on success, negative error number on failure.
> > > + */
> > > +int iio_backend_interface_type_get(struct iio_backend *back,
> > > +				   enum iio_backend_interface_type *type)
> > > +{
> > > +	int ret;
> > > +
> > > +	ret = iio_backend_op_call(back, interface_type_get, type);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	if (*type >= IIO_BACKEND_INTERFACE_MAX)
> > > +		return -EINVAL;
> > > +
> > > +	return 0;
> > > +}
> > > +EXPORT_SYMBOL_NS_GPL(iio_backend_interface_type_get, "IIO_BACKEND");
> > > +
> > > +/**
> > > + * iio_backend_interface_type_set - set the interface type used.
> > > + * @back: Backend device
> > > + * @type: Interface type
> > > + *
> > > + * RETURNS:
> > > + * 0 on success, negative error number on failure.
> > > + */
> > > +int iio_backend_interface_type_set(struct iio_backend *back,
> > > +				   enum iio_backend_interface_type type)
> > > +{
> > > +	if (type >= IIO_BACKEND_INTERFACE_MAX)
> > > +		return -EINVAL;
> > > +
> > > +	return  iio_backend_op_call(back, interface_type_set, type);
> > > +}
> > > +EXPORT_SYMBOL_NS_GPL(iio_backend_interface_type_set, "IIO_BACKEND");
> > > +
> > >  /**
> > >   * iio_backend_extend_chan_spec - Extend an IIO channel
> > >   * @back: Backend device
> > > diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h
> > > index 10be00f3b120..2b7221099d8c 100644
> > > --- a/include/linux/iio/backend.h
> > > +++ b/include/linux/iio/backend.h
> > > @@ -70,6 +70,15 @@ enum iio_backend_sample_trigger {
> > >  	IIO_BACKEND_SAMPLE_TRIGGER_MAX
> > >  };
> > >  
> > > +enum iio_backend_interface_type {
> > > +	IIO_BACKEND_INTERFACE_SERIAL_LVDS,
> > > +	IIO_BACKEND_INTERFACE_SERIAL_CMOS,    
> > 
> > The above are apparently not used in the next patch so I would not add them now.  
> > > +	IIO_BACKEND_INTERFACE_SERIAL_SPI,
> > > +	IIO_BACKEND_INTERFACE_SERIAL_DSPI,
> > > +	IIO_BACKEND_INTERFACE_SERIAL_QSPI,    
> > 
> > I'll throw my 2 cents but it would be nice to have more feedback on this. I'm
> > not completely sure about the xSPI stuff in here. We treated the QSPI as a bus
> > both for control and data in which we also add child devices. And we've been
> > adding specific bus operations/configurations through the 'struct
> > ad3552r_hs_platform_data' interface. So, I'm wondering if this should also not
> > be set through that interface?  
> 
> Maybe - kind of hard to tell until we actually have code.
> I'd go for kicking them into the long grass for now if they aren't used and
> just dropping them from this patch.  If we ever need them,easy to bring back
> and then we should have a justification for why!

oops. Misread. Obviously Nuno was saying the ones above aren't used, not the
SPI ones...  I don't feel strongly either way on setting these via
this generic interface, or via the other path.

Jonathan

> 
> J
> 
> 
> > 
> > LVDS/CMOS still looks slightly different to me...
> > 
> > - Nuno Sá
> > 
> > 
> >   
> 
> 

Re: [PATCH 4/8] iio: backend: add API for interface configuration

[David Lechner]

On 12/19/24 10:51 AM, Jonathan Cameron wrote:
> On Thu, 19 Dec 2024 16:42:33 +0000
> Jonathan Cameron <jic23@kernel.org> wrote:
> 
>> On Tue, 17 Dec 2024 11:13:59 +0100
>> Nuno Sá <noname.nuno@gmail.com> wrote:
>>
>>> On Mon, 2024-12-16 at 21:36 +0100, Angelo Dureghello wrote:  
>>>> From: Antoniu Miclaus <antoniu.miclaus@analog.com>
>>>>
>>>> Add backend support for setting and getting the interface type
>>>> in use.
>>>>
>>>> Link:
>>>> https://lore.kernel.org/linux-iio/20241129153546.63584-1-antoniu.miclaus@analog.com/T/#m6d86939078d780512824f1540145aade38b0990b
>>>> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
>>>> Co-developed-by: Angelo Dureghello <adureghello@baylibre.com>
>>>> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
>>>> ---
>>>> This patch has been picked up from the Antoniu patchset
>>>> still not accepted, and extended with the interface setter,
>>>> fixing also namespace names to be between quotation marks.
>>>> ---
>>>>  drivers/iio/industrialio-backend.c | 42
>>>> ++++++++++++++++++++++++++++++++++++++
>>>>  include/linux/iio/backend.h        | 19 +++++++++++++++++
>>>>  2 files changed, 61 insertions(+)
>>>>
>>>> diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-
>>>> backend.c
>>>> index 363281272035..6edc3e685f6a 100644
>>>> --- a/drivers/iio/industrialio-backend.c
>>>> +++ b/drivers/iio/industrialio-backend.c
>>>> @@ -636,6 +636,48 @@ ssize_t iio_backend_ext_info_set(struct iio_dev
>>>> *indio_dev, uintptr_t private,
>>>>  }
>>>>  EXPORT_SYMBOL_NS_GPL(iio_backend_ext_info_set, "IIO_BACKEND");
>>>>  
>>>> +/**
>>>> + * iio_backend_interface_type_get - get the interface type used.
>>>> + * @back: Backend device
>>>> + * @type: Interface type
>>>> + *
>>>> + * RETURNS:
>>>> + * 0 on success, negative error number on failure.
>>>> + */
>>>> +int iio_backend_interface_type_get(struct iio_backend *back,
>>>> +				   enum iio_backend_interface_type *type)
>>>> +{
>>>> +	int ret;
>>>> +
>>>> +	ret = iio_backend_op_call(back, interface_type_get, type);
>>>> +	if (ret)
>>>> +		return ret;
>>>> +
>>>> +	if (*type >= IIO_BACKEND_INTERFACE_MAX)
>>>> +		return -EINVAL;
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +EXPORT_SYMBOL_NS_GPL(iio_backend_interface_type_get, "IIO_BACKEND");
>>>> +
>>>> +/**
>>>> + * iio_backend_interface_type_set - set the interface type used.
>>>> + * @back: Backend device
>>>> + * @type: Interface type
>>>> + *
>>>> + * RETURNS:
>>>> + * 0 on success, negative error number on failure.
>>>> + */
>>>> +int iio_backend_interface_type_set(struct iio_backend *back,
>>>> +				   enum iio_backend_interface_type type)
>>>> +{
>>>> +	if (type >= IIO_BACKEND_INTERFACE_MAX)
>>>> +		return -EINVAL;
>>>> +
>>>> +	return  iio_backend_op_call(back, interface_type_set, type);
>>>> +}
>>>> +EXPORT_SYMBOL_NS_GPL(iio_backend_interface_type_set, "IIO_BACKEND");
>>>> +
>>>>  /**
>>>>   * iio_backend_extend_chan_spec - Extend an IIO channel
>>>>   * @back: Backend device
>>>> diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h
>>>> index 10be00f3b120..2b7221099d8c 100644
>>>> --- a/include/linux/iio/backend.h
>>>> +++ b/include/linux/iio/backend.h
>>>> @@ -70,6 +70,15 @@ enum iio_backend_sample_trigger {
>>>>  	IIO_BACKEND_SAMPLE_TRIGGER_MAX
>>>>  };
>>>>  
>>>> +enum iio_backend_interface_type {
>>>> +	IIO_BACKEND_INTERFACE_SERIAL_LVDS,
>>>> +	IIO_BACKEND_INTERFACE_SERIAL_CMOS,    
>>>
>>> The above are apparently not used in the next patch so I would not add them now.  
>>>> +	IIO_BACKEND_INTERFACE_SERIAL_SPI,
>>>> +	IIO_BACKEND_INTERFACE_SERIAL_DSPI,
>>>> +	IIO_BACKEND_INTERFACE_SERIAL_QSPI,    
>>>
>>> I'll throw my 2 cents but it would be nice to have more feedback on this. I'm
>>> not completely sure about the xSPI stuff in here. We treated the QSPI as a bus
>>> both for control and data in which we also add child devices. And we've been
>>> adding specific bus operations/configurations through the 'struct
>>> ad3552r_hs_platform_data' interface. So, I'm wondering if this should also not
>>> be set through that interface?  
>>
>> Maybe - kind of hard to tell until we actually have code.
>> I'd go for kicking them into the long grass for now if they aren't used and
>> just dropping them from this patch.  If we ever need them,easy to bring back
>> and then we should have a justification for why!
> 
> oops. Misread. Obviously Nuno was saying the ones above aren't used, not the
> SPI ones...  I don't feel strongly either way on setting these via
> this generic interface, or via the other path.
> 
> Jonathan
> 
>>
>> J
>>
>>
>>>
>>> LVDS/CMOS still looks slightly different to me...
>>>
>>> - Nuno Sá
>>>
>>>
>>>   
>>
>>
> 

I'm the one that suggested doing it this way to Angelo, so I'll chime in
with my thinking. In Angelo's previous series where we added IIO backend
support for this family of chips, in the devicetree discussion, we
considered the possibility of two SPI controllers, one for register
access and one for the high-speed streaming provided by the backend.
Since the dual and quad SPI here is for IIO backend (the high-speed sample
data interface) is what made me think we should put the control here rather
than putting it in the platform data interface.

Since this is new HDL, maybe we could avoid this issue altogether and
tweak the implementation in the FPGA a bit. Clearly we had the AD3552R
working without needing to poke this registers, so why can't we do the
same for the new chip? In other words, make these things a compile time
option in the HDL rather than a runtime option?

Re: [PATCH 4/8] iio: backend: add API for interface configuration

[Angelo Dureghello]

On 19.12.2024 11:01, David Lechner wrote:
> On 12/19/24 10:51 AM, Jonathan Cameron wrote:
> > On Thu, 19 Dec 2024 16:42:33 +0000
> > Jonathan Cameron <jic23@kernel.org> wrote:
> > 
> >> On Tue, 17 Dec 2024 11:13:59 +0100
> >> Nuno Sá <noname.nuno@gmail.com> wrote:
> >>
> >>> On Mon, 2024-12-16 at 21:36 +0100, Angelo Dureghello wrote:  
> >>>> From: Antoniu Miclaus <antoniu.miclaus@analog.com>
> >>>>
> >>>> Add backend support for setting and getting the interface type
> >>>> in use.
> >>>>
> >>>> Link:
> >>>> https://lore.kernel.org/linux-iio/20241129153546.63584-1-antoniu.miclaus@analog.com/T/#m6d86939078d780512824f1540145aade38b0990b
> >>>> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> >>>> Co-developed-by: Angelo Dureghello <adureghello@baylibre.com>
> >>>> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> >>>> ---
> >>>> This patch has been picked up from the Antoniu patchset
> >>>> still not accepted, and extended with the interface setter,
> >>>> fixing also namespace names to be between quotation marks.
> >>>> ---
> >>>>  drivers/iio/industrialio-backend.c | 42
> >>>> ++++++++++++++++++++++++++++++++++++++
> >>>>  include/linux/iio/backend.h        | 19 +++++++++++++++++
> >>>>  2 files changed, 61 insertions(+)
> >>>>
> >>>> diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-
> >>>> backend.c
> >>>> index 363281272035..6edc3e685f6a 100644
> >>>> --- a/drivers/iio/industrialio-backend.c
> >>>> +++ b/drivers/iio/industrialio-backend.c
> >>>> @@ -636,6 +636,48 @@ ssize_t iio_backend_ext_info_set(struct iio_dev
> >>>> *indio_dev, uintptr_t private,
> >>>>  }
> >>>>  EXPORT_SYMBOL_NS_GPL(iio_backend_ext_info_set, "IIO_BACKEND");
> >>>>  
> >>>> +/**
> >>>> + * iio_backend_interface_type_get - get the interface type used.
> >>>> + * @back: Backend device
> >>>> + * @type: Interface type
> >>>> + *
> >>>> + * RETURNS:
> >>>> + * 0 on success, negative error number on failure.
> >>>> + */
> >>>> +int iio_backend_interface_type_get(struct iio_backend *back,
> >>>> +				   enum iio_backend_interface_type *type)
> >>>> +{
> >>>> +	int ret;
> >>>> +
> >>>> +	ret = iio_backend_op_call(back, interface_type_get, type);
> >>>> +	if (ret)
> >>>> +		return ret;
> >>>> +
> >>>> +	if (*type >= IIO_BACKEND_INTERFACE_MAX)
> >>>> +		return -EINVAL;
> >>>> +
> >>>> +	return 0;
> >>>> +}
> >>>> +EXPORT_SYMBOL_NS_GPL(iio_backend_interface_type_get, "IIO_BACKEND");
> >>>> +
> >>>> +/**
> >>>> + * iio_backend_interface_type_set - set the interface type used.
> >>>> + * @back: Backend device
> >>>> + * @type: Interface type
> >>>> + *
> >>>> + * RETURNS:
> >>>> + * 0 on success, negative error number on failure.
> >>>> + */
> >>>> +int iio_backend_interface_type_set(struct iio_backend *back,
> >>>> +				   enum iio_backend_interface_type type)
> >>>> +{
> >>>> +	if (type >= IIO_BACKEND_INTERFACE_MAX)
> >>>> +		return -EINVAL;
> >>>> +
> >>>> +	return  iio_backend_op_call(back, interface_type_set, type);
> >>>> +}
> >>>> +EXPORT_SYMBOL_NS_GPL(iio_backend_interface_type_set, "IIO_BACKEND");
> >>>> +
> >>>>  /**
> >>>>   * iio_backend_extend_chan_spec - Extend an IIO channel
> >>>>   * @back: Backend device
> >>>> diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h
> >>>> index 10be00f3b120..2b7221099d8c 100644
> >>>> --- a/include/linux/iio/backend.h
> >>>> +++ b/include/linux/iio/backend.h
> >>>> @@ -70,6 +70,15 @@ enum iio_backend_sample_trigger {
> >>>>  	IIO_BACKEND_SAMPLE_TRIGGER_MAX
> >>>>  };
> >>>>  
> >>>> +enum iio_backend_interface_type {
> >>>> +	IIO_BACKEND_INTERFACE_SERIAL_LVDS,
> >>>> +	IIO_BACKEND_INTERFACE_SERIAL_CMOS,    
> >>>
> >>> The above are apparently not used in the next patch so I would not add them now.  
> >>>> +	IIO_BACKEND_INTERFACE_SERIAL_SPI,
> >>>> +	IIO_BACKEND_INTERFACE_SERIAL_DSPI,
> >>>> +	IIO_BACKEND_INTERFACE_SERIAL_QSPI,    
> >>>
> >>> I'll throw my 2 cents but it would be nice to have more feedback on this. I'm
> >>> not completely sure about the xSPI stuff in here. We treated the QSPI as a bus
> >>> both for control and data in which we also add child devices. And we've been
> >>> adding specific bus operations/configurations through the 'struct
> >>> ad3552r_hs_platform_data' interface. So, I'm wondering if this should also not
> >>> be set through that interface?  
> >>
> >> Maybe - kind of hard to tell until we actually have code.
> >> I'd go for kicking them into the long grass for now if they aren't used and
> >> just dropping them from this patch.  If we ever need them,easy to bring back
> >> and then we should have a justification for why!
> > 
> > oops. Misread. Obviously Nuno was saying the ones above aren't used, not the
> > SPI ones...  I don't feel strongly either way on setting these via
> > this generic interface, or via the other path.
> > 
> > Jonathan
> > 
> >>
> >> J
> >>
> >>
> >>>
> >>> LVDS/CMOS still looks slightly different to me...
> >>>
> >>> - Nuno Sá
> >>>
> >>>
> >>>   
> >>
> >>
> > 
> 
> I'm the one that suggested doing it this way to Angelo, so I'll chime in
> with my thinking. In Angelo's previous series where we added IIO backend
> support for this family of chips, in the devicetree discussion, we
> considered the possibility of two SPI controllers, one for register
> access and one for the high-speed streaming provided by the backend.
> Since the dual and quad SPI here is for IIO backend (the high-speed sample
> data interface) is what made me think we should put the control here rather
> than putting it in the platform data interface.
> 
> Since this is new HDL, maybe we could avoid this issue altogether and
> tweak the implementation in the FPGA a bit. Clearly we had the AD3552R
> working without needing to poke this registers, so why can't we do the
> same for the new chip? In other words, make these things a compile time
> option in the HDL rather than a runtime option?

The purpose of the new HDL is to support all the devices of the family,
(DSPI/QSPI) just acting on axi MULTI_IO_MODE.

Thinking to this a bit better, changing MULTI_IO_MODE is not exactly changing 
"interface" type so maybe is more appropriarte to use a patform data
"bus_mode" function.

New ad354x chips do not have QSPI pin, so no chance to access the whole
primary + secondary area in DSPI without changes in configuration registers,
as done before with ad355xr.

Regards,
angelo

[PATCH 5/8] iio: dac: adi-axi-dac: add bus mode setup

[Angelo Dureghello]

From: Angelo Dureghello <adureghello@baylibre.com>

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

Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
 drivers/iio/dac/adi-axi-dac.c | 46 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/dac/adi-axi-dac.c b/drivers/iio/dac/adi-axi-dac.c
index d02eb535b648..f7d22409e9b3 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)
 
@@ -95,6 +95,12 @@ enum {
 	AXI_DAC_DATA_INTERNAL_RAMP_16BIT = 11,
 };
 
+enum multi_io_mode {
+	AXI_DAC_MULTI_IO_MODE_SPI,
+	AXI_DAC_MULTI_IO_MODE_DSPI,
+	AXI_DAC_MULTI_IO_MODE_QSPI,
+};
+
 struct axi_dac_info {
 	unsigned int version;
 	const struct iio_backend_info *backend_info;
@@ -725,6 +731,43 @@ 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_interface_type_set(struct iio_backend *back,
+				      enum iio_backend_interface_type type)
+{
+	struct axi_dac_state *st = iio_backend_get_priv(back);
+	int mode, ival, ret;
+
+	switch (type) {
+	case IIO_BACKEND_INTERFACE_SERIAL_SPI:
+		mode = AXI_DAC_MULTI_IO_MODE_SPI;
+		break;
+	case IIO_BACKEND_INTERFACE_SERIAL_DSPI:
+		mode = AXI_DAC_MULTI_IO_MODE_DSPI;
+		break;
+	case IIO_BACKEND_INTERFACE_SERIAL_QSPI:
+		mode = AXI_DAC_MULTI_IO_MODE_QSPI;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	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;
+
+	ret = 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);
+
+	if (ret == -ETIMEDOUT)
+		dev_err(st->dev, "AXI read timeout\n");
+
+	return ret;
+}
+
 static void axi_dac_child_remove(void *data)
 {
 	platform_device_unregister(data);
@@ -774,6 +817,7 @@ static const struct iio_backend_ops axi_ad3552r_ops = {
 	.request_buffer = axi_dac_request_buffer,
 	.free_buffer = axi_dac_free_buffer,
 	.data_source_set = axi_dac_data_source_set,
+	.interface_type_set = axi_dac_interface_type_set,
 	.ddr_enable = axi_dac_ddr_enable,
 	.ddr_disable = axi_dac_ddr_disable,
 	.data_stream_enable = axi_dac_data_stream_enable,

-- 
2.47.0

[PATCH 6/8] iio: dac: ad3552r-hs: exit for error on wrong chip id

[Angelo Dureghello]

From: Angelo Dureghello <adureghello@baylibre.com>

Exit for error on wrong chip id, otherwise driver continues
with wrong assumptions.

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..e613eee7fc11 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);
+		return dev_err_probe(st->dev, -ENODEV,
+				     "chip id error, expected 0x%x, got 0x%x\n",
+				     st->model_data->chip_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,

-- 
2.47.0

Re: [PATCH 6/8] iio: dac: ad3552r-hs: exit for error on wrong chip id

[Jonathan Cameron]

On Mon, 16 Dec 2024 21:36:26 +0100
Angelo Dureghello <adureghello@baylibre.com> wrote:

> From: Angelo Dureghello <adureghello@baylibre.com>
> 
> Exit for error on wrong chip id, otherwise driver continues
> with wrong assumptions.
Why? Chip ID does not define all future compatible parts, just the
ones we know about today.

The reason not failing is that the moment we do exit on a mismatch
we can never support fallback device tree compatible IDs.  Is there
no chance that ADI will release a backwards compatible part in the
future that we'd like to work with old kernels?

Any mismatch in DT vs hardware present is considered a firmware
bug, not a kernel problem.
We used to reject missmatched IDs but after a long discussion with
DT maintainers it became clear that broke their model.

Thanks,

Jonathan

> 
> 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..e613eee7fc11 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);
> +		return dev_err_probe(st->dev, -ENODEV,
> +				     "chip id error, expected 0x%x, got 0x%x\n",
> +				     st->model_data->chip_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,
> 

Re: [PATCH 6/8] iio: dac: ad3552r-hs: exit for error on wrong chip id

[Conor Dooley]

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

On Thu, Dec 19, 2024 at 04:54:46PM +0000, Jonathan Cameron wrote:
> On Mon, 16 Dec 2024 21:36:26 +0100
> Angelo Dureghello <adureghello@baylibre.com> wrote:
> 
> > From: Angelo Dureghello <adureghello@baylibre.com>
> > 
> > Exit for error on wrong chip id, otherwise driver continues
> > with wrong assumptions.
> Why? Chip ID does not define all future compatible parts, just the
> ones we know about today.
> 
> The reason not failing is that the moment we do exit on a mismatch
> we can never support fallback device tree compatible IDs.  Is there
> no chance that ADI will release a backwards compatible part in the
> future that we'd like to work with old kernels?
> 
> Any mismatch in DT vs hardware present is considered a firmware
> bug, not a kernel problem.
> We used to reject missmatched IDs but after a long discussion with
> DT maintainers it became clear that broke their model.

I'd probably still say the warning of the info message is still too
harsh, and that it should be something like "Chip ID mismatch, operating
detected 0x%x as a 0x%x" ;)

> Thanks,
> 
> Jonathan
> 
> > 
> > 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..e613eee7fc11 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);
> > +		return dev_err_probe(st->dev, -ENODEV,
> > +				     "chip id error, expected 0x%x, got 0x%x\n",
> > +				     st->model_data->chip_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,
> > 
> 

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

Re: [PATCH 6/8] iio: dac: ad3552r-hs: exit for error on wrong chip id

[Angelo Dureghello]

On 19.12.2024 16:54, Jonathan Cameron wrote:
> On Mon, 16 Dec 2024 21:36:26 +0100
> Angelo Dureghello <adureghello@baylibre.com> wrote:
> 
> > From: Angelo Dureghello <adureghello@baylibre.com>
> > 
> > Exit for error on wrong chip id, otherwise driver continues
> > with wrong assumptions.
> Why? Chip ID does not define all future compatible parts, just the
> ones we know about today.
> 
> The reason not failing is that the moment we do exit on a mismatch
> we can never support fallback device tree compatible IDs.  Is there
> no chance that ADI will release a backwards compatible part in the
> future that we'd like to work with old kernels?
>
> Any mismatch in DT vs hardware present is considered a firmware
> bug, not a kernel problem.
> We used to reject missmatched IDs but after a long discussion with
> DT maintainers it became clear that broke their model.
> 
Ok, i will apply to what decided so.  

> Thanks,
> 
> Jonathan
> 
> > 
> > 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..e613eee7fc11 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);
> > +		return dev_err_probe(st->dev, -ENODEV,
> > +				     "chip id error, expected 0x%x, got 0x%x\n",
> > +				     st->model_data->chip_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,
> > 
> 

[PATCH 7/8] 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,
- use DAC "instruction mode" when backend is not buffering, suggested
  from the ADI HDL team as more proper configuration mode to be used
  for all ad35xxr devices,
- change samplerate to respect number of lanes.

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

diff --git a/drivers/iio/dac/ad3552r-common.c b/drivers/iio/dac/ad3552r-common.c
index 03e0864f5084..2a0dd18ca906 100644
--- a/drivers/iio/dac/ad3552r-common.c
+++ b/drivers/iio/dac/ad3552r-common.c
@@ -47,6 +47,50 @@ u16 ad3552r_calc_custom_gain(u8 p, u8 n, s16 goffs)
 }
 EXPORT_SYMBOL_NS_GPL(ad3552r_calc_custom_gain, "IIO_AD3552R");
 
+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,
+	.num_spi_data_lanes = 2,
+};
+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,
+	.num_spi_data_lanes = 2,
+};
+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,
+	.num_spi_data_lanes = 4,
+};
+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,
+	.num_spi_data_lanes = 4,
+};
+EXPORT_SYMBOL_NS_GPL(ad3552r_model_data, "IIO_AD3552R");
+
 static void ad3552r_get_custom_range(struct ad3552r_ch_data *ch_data,
 				     s32 *v_min, s32 *v_max)
 {
diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c
index e613eee7fc11..58c8661f483b 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 = IIO_BACKEND_INTERFACE_SERIAL_QSPI;
+	else
+		bus_mode = IIO_BACKEND_INTERFACE_SERIAL_DSPI;
+
+	return iio_backend_interface_type_set(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,48 +200,127 @@ static int ad3552r_hs_buffer_postenable(struct iio_dev *indio_dev)
 		return -EINVAL;
 	}
 
-	ret = st->data->bus_reg_write(st->back, AD3552R_REG_ADDR_STREAM_MODE,
-				      loop_len, 1);
+	/*
+	 * With ad3541/2r supoport, 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,
+					   AD3552R_MASK_SINGLE_INST, 0, 1);
 	if (ret)
 		return ret;
 
-	/* Inform DAC chip to switch into DDR 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_INTERFACE_CONFIG_D,
-					   AD3552R_MASK_SPI_CONFIG_DDR,
-					   AD3552R_MASK_SPI_CONFIG_DDR, 1);
+		AD3552R_REG_ADDR_TRANSFER_REGISTER,
+		AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE,
+		AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE, 1);
 	if (ret)
-		return ret;
+		goto exit_err_streaming;
+
+	ret = st->data->bus_reg_write(st->back,
+				      AD3552R_REG_ADDR_STREAM_MODE,
+				      loop_len, 1);
+	if (ret)
+		goto exit_err_streaming;
+
+	/*
+	 * Registers dump for debug purposes is only possible until here,
+	 * read in primary region must be SPI SDR (DDR read is never possible,
+	 * D/QSPI SDR read in primary region is also not possible).
+	 */
+
+	/* 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)
+		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.
+	 */
+	iio_backend_interface_type_set(st->back,
+				       IIO_BACKEND_INTERFACE_SERIAL_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;
 }
 
@@ -186,11 +333,23 @@ 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 = iio_backend_interface_type_set(st->back,
+					     IIO_BACKEND_INTERFACE_SERIAL_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;
 
@@ -198,6 +357,24 @@ 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_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;
 }
 
@@ -304,10 +481,18 @@ 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;
 
+	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;
@@ -330,6 +515,8 @@ static int ad3552r_hs_setup(struct ad3552r_hs_state *st)
 				     "chip id error, expected 0x%x, got 0x%x\n",
 				     st->model_data->chip_id, id);
 
+	dev_info(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);
@@ -342,14 +529,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;
@@ -505,15 +684,10 @@ 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,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.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..9d450019ece9 100644
--- a/drivers/iio/dac/ad3552r.h
+++ b/drivers/iio/dac/ad3552r.h
@@ -132,11 +132,18 @@
 
 #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 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,
 	AD3542R_ID = 0x4009,
@@ -151,6 +158,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

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

[Jonathan Cameron]

On Mon, 16 Dec 2024 21:36:27 +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.
> 
Hi Angelo,

A few comments inline, but mostly / I think this could do with breaking
up into several patches. Particularly as that instruction mode
one sounds like it may turn out to be a fix we need to backport at
some point if problems are seen with previous approach.

There is also a move / consolidation of structures that could be a trivial
precursor and make the final patch simpler.

> 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,
> - use DAC "instruction mode" when backend is not buffering, suggested
>   from the ADI HDL team as more proper configuration mode to be used
>   for all ad35xxr devices,
Perhaps that change should be a precursor patch? 

> - change samplerate to respect number of lanes.
> 
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>

> ---
>  drivers/iio/dac/ad3552r-common.c |  44 +++++++
>  drivers/iio/dac/ad3552r-hs.c     | 262 ++++++++++++++++++++++++++++++++-------
>  drivers/iio/dac/ad3552r.c        |  36 ------
>  drivers/iio/dac/ad3552r.h        |   8 ++
>  4 files changed, 270 insertions(+), 80 deletions(-)
> 
> diff --git a/drivers/iio/dac/ad3552r-common.c b/drivers/iio/dac/ad3552r-common.c
> index 03e0864f5084..2a0dd18ca906 100644
> --- a/drivers/iio/dac/ad3552r-common.c
> +++ b/drivers/iio/dac/ad3552r-common.c
> @@ -47,6 +47,50 @@ u16 ad3552r_calc_custom_gain(u8 p, u8 n, s16 goffs)
>  }
>  EXPORT_SYMBOL_NS_GPL(ad3552r_calc_custom_gain, "IIO_AD3552R");
>  
> +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,
> +	.num_spi_data_lanes = 2,
> +};
> +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,
> +	.num_spi_data_lanes = 2,
> +};
> +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,
> +	.num_spi_data_lanes = 4,
> +};
> +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,
> +	.num_spi_data_lanes = 4,
> +};
> +EXPORT_SYMBOL_NS_GPL(ad3552r_model_data, "IIO_AD3552R");

Maybe move these in a precursor patch . That will make it obvious
that you are also squashing a duplicate.

> +
>  static void ad3552r_get_custom_range(struct ad3552r_ch_data *ch_data,
>  				     s32 *v_min, s32 *v_max)
>  {
> diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c
> index e613eee7fc11..58c8661f483b 100644
> --- a/drivers/iio/dac/ad3552r-hs.c
> +++ b/drivers/iio/dac/ad3552r-hs.c
> @@ -19,6 +19,31 @@

...

> @@ -304,10 +481,18 @@ static int ad3552r_hs_setup(struct ad3552r_hs_state *st)
>  	if (ret)
>  		return ret;
>  
> +	/* HDL starts with DDR enabled, disabling it. */

Seems obvious. Does the comment add anything?

>  	ret = iio_backend_ddr_disable(st->back);
>  	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;
> @@ -330,6 +515,8 @@ static int ad3552r_hs_setup(struct ad3552r_hs_state *st)
>  				     "chip id error, expected 0x%x, got 0x%x\n",
>  				     st->model_data->chip_id, id);
>  
> +	dev_info(st->dev, "chip id %s detected", st->model_data->model_name);

dev_dbg().  We have to much noise in the kernel log already! With dynamic debug
it is easy for people to turn this on if they want it.

> +
>  	/* 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);
> @@ -342,14 +529,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;
> @@ -505,15 +684,10 @@ static int ad3552r_hs_probe(struct platform_device *pdev)
>  	return devm_iio_device_register(&pdev->dev, indio_dev);

[PATCH 8/8] 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 | 58 ++++++++++++++++++++------------------------
 1 file changed, 26 insertions(+), 32 deletions(-)

diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c
index 58c8661f483b..931e6036da36 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)
 	 * 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);
+	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;
 
@@ -247,7 +245,6 @@ static int ad3552r_hs_buffer_postenable(struct iio_dev *indio_dev)
 	ret = st->data->bus_reg_write(st->back,
 				      AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
 				      st->config_d, 1);
-
 	if (ret)
 		goto exit_err_streaming;
 
@@ -257,7 +254,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"
+	 * possible anymore, including calling "ad3552r_update_reg_bits"
 	 * function.
 	 */
 
@@ -361,10 +358,9 @@ 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;
 
@@ -388,10 +384,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)
@@ -407,10 +403,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;
 	}
@@ -543,19 +539,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