[PATCH v2 1/7] iio: backend: add API for interface get

[PATCH v2 1/7] iio: backend: add API for interface get

[Antoniu Miclaus]

Add backend support for obtaining the interface type used.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
changes in v2:
 - add IIO_BACKEND_INTERFACE_COUNT in enum.
 - add trailing commas where applies.
 drivers/iio/industrialio-backend.c | 24 ++++++++++++++++++++++++
 include/linux/iio/backend.h        | 11 +++++++++++
 2 files changed, 35 insertions(+)

diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
index efe05be284b6..a322b0be7b2c 100644
--- a/drivers/iio/industrialio-backend.c
+++ b/drivers/iio/industrialio-backend.c
@@ -449,6 +449,30 @@ 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 interace 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_COUNT)
+		return -EINVAL;
+
+	return 0;
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_interface_type_get, IIO_BACKEND);
+
 /**
  * iio_backend_extend_chan_spec - Extend an IIO channel
  * @indio_dev: IIO device
diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h
index 8099759d7242..34fc76c99d8a 100644
--- a/include/linux/iio/backend.h
+++ b/include/linux/iio/backend.h
@@ -63,6 +63,12 @@ enum iio_backend_sample_trigger {
 	IIO_BACKEND_SAMPLE_TRIGGER_MAX
 };
 
+enum iio_backend_interface_type {
+	IIO_BACKEND_INTERFACE_LVDS,
+	IIO_BACKEND_INTERFACE_CMOS,
+	IIO_BACKEND_INTERFACE_COUNT,
+};
+
 /**
  * struct iio_backend_ops - operations structure for an iio_backend
  * @enable: Enable backend.
@@ -81,6 +87,7 @@ 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.
  **/
 struct iio_backend_ops {
 	int (*enable)(struct iio_backend *back);
@@ -113,6 +120,8 @@ 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 iio_backend_chan_enable(struct iio_backend *back, unsigned int chan);
@@ -142,6 +151,8 @@ ssize_t iio_backend_ext_info_set(struct iio_dev *indio_dev, uintptr_t private,
 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_extend_chan_spec(struct iio_dev *indio_dev,
 				 struct iio_backend *back,
 				 struct iio_chan_spec *chan);
-- 
2.46.2

[PATCH v2 2/7] iio: backend: add support for data size set

[Antoniu Miclaus]

Add backend support for setting the data size used.
This setting can be adjusted within the IP cores interfacing devices.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
changes in v2:
 - use Return: instead of RETURN.
 - use size_t instead of ssize_t
 - improve commit message.
 drivers/iio/industrialio-backend.c | 21 +++++++++++++++++++++
 include/linux/iio/backend.h        |  3 +++
 2 files changed, 24 insertions(+)

diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
index a322b0be7b2c..9aaa90ed1274 100644
--- a/drivers/iio/industrialio-backend.c
+++ b/drivers/iio/industrialio-backend.c
@@ -473,6 +473,27 @@ int iio_backend_interface_type_get(struct iio_backend *back,
 }
 EXPORT_SYMBOL_NS_GPL(iio_backend_interface_type_get, IIO_BACKEND);
 
+/**
+ * iio_backend_data_size_set - set the data width/size in the data bus.
+ * @back: Backend device
+ * @size: Size in bits
+ *
+ * Some frontend devices can dynamically control the word/data size on the
+ * interface/data bus. Hence, the backend device needs to be aware of it so
+ * data can be correctly transferred.
+ *
+ * Return:
+ * 0 on success, negative error number on failure.
+ */
+size_t iio_backend_data_size_set(struct iio_backend *back, size_t size)
+{
+	if (!size)
+		return -EINVAL;
+
+	return iio_backend_op_call(back, data_size_set, size);
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_data_size_set, IIO_BACKEND);
+
 /**
  * iio_backend_extend_chan_spec - Extend an IIO channel
  * @indio_dev: IIO device
diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h
index 34fc76c99d8a..18e571a24251 100644
--- a/include/linux/iio/backend.h
+++ b/include/linux/iio/backend.h
@@ -88,6 +88,7 @@ enum iio_backend_interface_type {
  * @ext_info_set: Extended info setter.
  * @ext_info_get: Extended info getter.
  * @interface_type_get: Interface type.
+ * @data_size_set: Data size.
  **/
 struct iio_backend_ops {
 	int (*enable)(struct iio_backend *back);
@@ -122,6 +123,7 @@ struct iio_backend_ops {
 			    const struct iio_chan_spec *chan, char *buf);
 	int (*interface_type_get)(struct iio_backend *back,
 				  enum iio_backend_interface_type *type);
+	int (*data_size_set)(struct iio_backend *back, ssize_t size);
 };
 
 int iio_backend_chan_enable(struct iio_backend *back, unsigned int chan);
@@ -153,6 +155,7 @@ ssize_t iio_backend_ext_info_get(struct iio_dev *indio_dev, uintptr_t private,
 
 int iio_backend_interface_type_get(struct iio_backend *back,
 				   enum iio_backend_interface_type *type);
+size_t iio_backend_data_size_set(struct iio_backend *back, size_t size);
 int iio_backend_extend_chan_spec(struct iio_dev *indio_dev,
 				 struct iio_backend *back,
 				 struct iio_chan_spec *chan);
-- 
2.46.2

[PATCH v2 3/7] iio: adc: adi-axi-adc: add interface type

[Antoniu Miclaus]

Add support for getting the interface (CMOS or LVDS) used by the AXI ADC
ip.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
no changes in v2.
 drivers/iio/adc/adi-axi-adc.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
index 21ce7564e83d..ff48f26e02a3 100644
--- a/drivers/iio/adc/adi-axi-adc.c
+++ b/drivers/iio/adc/adi-axi-adc.c
@@ -39,6 +39,9 @@
 #define   ADI_AXI_REG_RSTN_MMCM_RSTN		BIT(1)
 #define   ADI_AXI_REG_RSTN_RSTN			BIT(0)
 
+#define ADI_AXI_ADC_REG_CONFIG			0x000c
+#define   ADI_AXI_ADC_REG_CONFIG_CMOS_OR_LVDS_N	BIT(7)
+
 #define ADI_AXI_ADC_REG_CTRL			0x0044
 #define    ADI_AXI_ADC_CTRL_DDR_EDGESEL_MASK	BIT(1)
 
@@ -249,6 +252,25 @@ static int axi_adc_chan_disable(struct iio_backend *back, unsigned int chan)
 				 ADI_AXI_REG_CHAN_CTRL_ENABLE);
 }
 
+static int axi_adc_interface_type_get(struct iio_backend *back,
+				      enum iio_backend_interface_type *type)
+{
+	struct adi_axi_adc_state *st = iio_backend_get_priv(back);
+	unsigned int val;
+	int ret;
+
+	ret = regmap_read(st->regmap, ADI_AXI_ADC_REG_CONFIG, &val);
+	if (ret)
+		return ret;
+
+	if (val & ADI_AXI_ADC_REG_CONFIG_CMOS_OR_LVDS_N)
+		*type = IIO_BACKEND_INTERFACE_CMOS;
+	else
+		*type = IIO_BACKEND_INTERFACE_LVDS;
+
+	return 0;
+}
+
 static struct iio_buffer *axi_adc_request_buffer(struct iio_backend *back,
 						 struct iio_dev *indio_dev)
 {
@@ -285,6 +307,7 @@ static const struct iio_backend_ops adi_axi_adc_generic = {
 	.iodelay_set = axi_adc_iodelays_set,
 	.test_pattern_set = axi_adc_test_pattern_set,
 	.chan_status = axi_adc_chan_status,
+	.interface_type_get = axi_adc_interface_type_get,
 };
 
 static int adi_axi_adc_probe(struct platform_device *pdev)
-- 
2.46.2

[PATCH v2 4/7] iio: adc: adi-axi-adc: set data format

[Antoniu Miclaus]

Add support for selecting the data format within the AXI ADC ip.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
changes in v2:
 - add ADI_AXI_ADC_CNTRL_3_CUSTOM_CONTROL_MSK and use regmap_update_bits
 drivers/iio/adc/adi-axi-adc.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
index ff48f26e02a3..363cc29b4c18 100644
--- a/drivers/iio/adc/adi-axi-adc.c
+++ b/drivers/iio/adc/adi-axi-adc.c
@@ -45,6 +45,9 @@
 #define ADI_AXI_ADC_REG_CTRL			0x0044
 #define    ADI_AXI_ADC_CTRL_DDR_EDGESEL_MASK	BIT(1)
 
+#define ADI_AXI_ADC_REG_CNTRL_3			0x004c
+#define ADI_AXI_ADC_CNTRL_3_CUSTOM_CONTROL_MSK	GENMASK(7, 0)
+
 #define ADI_AXI_ADC_REG_DRP_STATUS		0x0074
 #define   ADI_AXI_ADC_DRP_LOCKED		BIT(17)
 
@@ -271,6 +274,25 @@ static int axi_adc_interface_type_get(struct iio_backend *back,
 	return 0;
 }
 
+static int axi_adc_data_size_set(struct iio_backend *back,
+				 ssize_t size)
+{
+	struct adi_axi_adc_state *st = iio_backend_get_priv(back);
+	unsigned int val;
+
+	if (size <= 20)
+		val = 0;
+	else if (size <= 24)
+		val = 1;
+	else if (size <= 32)
+		val = 3;
+	else
+		return -EINVAL;
+
+	return regmap_update_bits(st->regmap, ADI_AXI_ADC_REG_CNTRL_3,
+				  ADI_AXI_ADC_CNTRL_3_CUSTOM_CONTROL_MSK, val);
+}
+
 static struct iio_buffer *axi_adc_request_buffer(struct iio_backend *back,
 						 struct iio_dev *indio_dev)
 {
@@ -308,6 +330,7 @@ static const struct iio_backend_ops adi_axi_adc_generic = {
 	.test_pattern_set = axi_adc_test_pattern_set,
 	.chan_status = axi_adc_chan_status,
 	.interface_type_get = axi_adc_interface_type_get,
+	.data_size_set = axi_adc_data_size_set,
 };
 
 static int adi_axi_adc_probe(struct platform_device *pdev)
-- 
2.46.2

[PATCH v2 5/7] dt-bindings: iio: adc: add ad485x

[Antoniu Miclaus]

Add devicetree bindings for ad485x family.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
changes in v2:
 - link all public parts in the description
 - add $ref: /schemas/spi/spi-peripheral-props.yaml#
 - add vee-supply
 - add vddl-supply
 - add description for pwms
 - add pd-gpios
 - update spi-max-frequency value
 - make vddh-supply optional, not required.
 - update example based on new properties added.
 - fix typos in commit message/title.
 - update year to 2024
 - drop "DAS" and "device driver" from bindings description.
 .../bindings/iio/adc/adi,ad485x.yaml          | 96 +++++++++++++++++++
 1 file changed, 96 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad485x.yaml

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad485x.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad485x.yaml
new file mode 100644
index 000000000000..899a65504f12
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad485x.yaml
@@ -0,0 +1,96 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright 2024 Analog Devices Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/adi,ad485x.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD485X family
+
+maintainers:
+  - Sergiu Cuciurean <sergiu.cuciurean@analog.com>
+  - Dragos Bogdan <dragos.bogdan@analog.com>
+  - Antoniu Miclaus <antoniu.miclaus@analog.com>
+
+description: |
+  Analog Devices AD485X family
+
+  https://www.analog.com/media/en/technical-documentation/data-sheets/ad4858.pdf
+  https://www.analog.com/media/en/technical-documentation/data-sheets/ad4857.pdf
+  https://www.analog.com/media/en/technical-documentation/data-sheets/ad4856.pdf
+  https://www.analog.com/media/en/technical-documentation/data-sheets/ad4855.pdf
+
+$ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+  compatible:
+    enum:
+      - adi,ad4858
+      - adi,ad4857
+      - adi,ad4856
+      - adi,ad4855
+      - adi,ad4854
+      - adi,ad4853
+      - adi,ad4852
+      - adi,ad4851
+      - adi,ad4858i
+
+  reg:
+    maxItems: 1
+
+  vcc-supply: true
+
+  vee-supply: true
+
+  vdd-supply: true
+
+  vddh-supply: true
+
+  vddl-supply: true
+
+  vio-supply: true
+
+  pwms:
+    description: PWM connected to the CNV pin.
+    maxItems: 1
+
+  io-backends:
+    maxItems: 1
+
+  pd-gpios:
+    maxItems: 1
+
+  spi-max-frequency:
+    maximum: 25000000
+
+required:
+  - compatible
+  - reg
+  - vcc-supply
+  - vee-supply
+  - vdd-supply
+  - vio-supply
+  - pwms
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        adc@0{
+            compatible = "adi,ad4858";
+            reg = <0>;
+            spi-max-frequency = <10000000>;
+            vcc-supply = <&vcc>;
+            vdd-supply = <&vdd>;
+            vee-supply = <&vee>;
+            vddh-supply = <&vddh>;
+            vddl-supply = <&vddh>;
+            vio-supply = <&vio>;
+            pwms = <&pwm_gen 0 0>;
+            io-backends = <&iio_backend>;
+        };
+    };
+...
-- 
2.46.2

Re: [PATCH v2 5/7] dt-bindings: iio: adc: add ad485x

[Conor Dooley]

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

On Fri, Oct 04, 2024 at 05:07:54PM +0300, Antoniu Miclaus wrote:
> Add devicetree bindings for ad485x family.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---
> changes in v2:
>  - link all public parts in the description
>  - add $ref: /schemas/spi/spi-peripheral-props.yaml#
>  - add vee-supply
>  - add vddl-supply
>  - add description for pwms
>  - add pd-gpios
>  - update spi-max-frequency value
>  - make vddh-supply optional, not required.
>  - update example based on new properties added.
>  - fix typos in commit message/title.
>  - update year to 2024
>  - drop "DAS" and "device driver" from bindings description.
>  .../bindings/iio/adc/adi,ad485x.yaml          | 96 +++++++++++++++++++
>  1 file changed, 96 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad485x.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad485x.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad485x.yaml
> new file mode 100644
> index 000000000000..899a65504f12
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad485x.yaml

With a filename matching a compatible,
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.

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

Re: [PATCH v2 5/7] dt-bindings: iio: adc: add ad485x

[David Lechner]

On 10/4/24 9:07 AM, Antoniu Miclaus wrote:
> Add devicetree bindings for ad485x family.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---

...

> +
> +  vcc-supply: true
> +
> +  vee-supply: true
> +
> +  vdd-supply: true
> +
> +  vddh-supply: true
> +
> +  vddl-supply: true
> +
> +  vio-supply: true

What about REFIO and REFBUF supplies? These are optional, but
no reason we can't add them now.

Re: [PATCH v2 5/7] dt-bindings: iio: adc: add ad485x

[Jonathan Cameron]

On Fri, 4 Oct 2024 17:07:54 +0300
Antoniu Miclaus <antoniu.miclaus@analog.com> wrote:

> Add devicetree bindings for ad485x family.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
Hi Antoniu

Trivial stuff inline to add to David's review.

Jonathan

> ---
>  1 file changed, 96 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad485x.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad485x.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad485x.yaml
> new file mode 100644
> index 000000000000..899a65504f12
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad485x.yaml
> @@ -0,0 +1,96 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright 2024 Analog Devices Inc.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/adi,ad485x.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD485X family
> +
> +maintainers:
> +  - Sergiu Cuciurean <sergiu.cuciurean@analog.com>
> +  - Dragos Bogdan <dragos.bogdan@analog.com>
> +  - Antoniu Miclaus <antoniu.miclaus@analog.com>
> +
> +description: |
> +  Analog Devices AD485X family

Give us some more idea of what these are.  Brief paragraph or two is fine giving
some key characteristics of these ADCs.  Whilst we know they are ADCs from
the path, maybe good to mention that :)


> +
> +  https://www.analog.com/media/en/technical-documentation/data-sheets/ad4858.pdf
> +  https://www.analog.com/media/en/technical-documentation/data-sheets/ad4857.pdf
> +  https://www.analog.com/media/en/technical-documentation/data-sheets/ad4856.pdf
> +  https://www.analog.com/media/en/technical-documentation/data-sheets/ad4855.pdf
Numeric order probably better than the reverse.

> +
> +$ref: /schemas/spi/spi-peripheral-props.yaml#

[PATCH v2 6/7] iio: adc: ad485x: add ad485x driver

[Antoniu Miclaus]

Add support for the AD485X DAS familiy.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
changes in v2:
 - update headers and include the missing ones, order them alphabetically.
 - use clamp()
 - use units macros where applies
 - convert int to unsigned variables where applies.
 - add trailing commas where applies.
 - return FIELD_GET directly.
 - shrink function header to one line where it fits.
 - update scale table values arrangement - pow-of-two per line
 - rename j to have a proper meaning.
 - invert if (st->offsets[chan->channel] != val) and drop next lines indentation.
 - drop whitespace from * val = ... (altough checkpatch complains about it)
 - drop comma in the terminator lines for ext_info.
 - fix inconsistency between chip_info structures.
 - use devm_mutex_init
 - return -ENOENT on max_cnt check.
 - check both val and val2 for negative before converting to unsigned.
 - remove val2 where not used.
 - use dev_info() instead of dev_warn()
 - add spaces after { and before } in ad485x_scale_table

 drivers/iio/adc/Kconfig  |   12 +
 drivers/iio/adc/Makefile |    1 +
 drivers/iio/adc/ad485x.c | 1094 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 1107 insertions(+)
 create mode 100644 drivers/iio/adc/ad485x.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index f60fe85a30d5..83f55229d731 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -36,6 +36,18 @@ config AD4130
 	  To compile this driver as a module, choose M here: the module will be
 	  called ad4130.
 
+config AD485X
+	tristate "Analog Device AD485x DAS Driver"
+	depends on SPI
+	select REGMAP_SPI
+	select IIO_BACKEND
+	help
+	  Say yes here to build support for Analog Devices AD485x high speed
+	  data acquisition system (DAS).
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called ad485x.
+
 config AD7091R
 	tristate
 
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index d370e066544e..26c1670c8913 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -7,6 +7,7 @@
 obj-$(CONFIG_AB8500_GPADC) += ab8500-gpadc.o
 obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o
 obj-$(CONFIG_AD4130) += ad4130.o
+obj-$(CONFIG_AD485X) += ad485x.o
 obj-$(CONFIG_AD7091R) += ad7091r-base.o
 obj-$(CONFIG_AD7091R5) += ad7091r5.o
 obj-$(CONFIG_AD7091R8) += ad7091r8.o
diff --git a/drivers/iio/adc/ad485x.c b/drivers/iio/adc/ad485x.c
new file mode 100644
index 000000000000..faa10d56a791
--- /dev/null
+++ b/drivers/iio/adc/ad485x.c
@@ -0,0 +1,1094 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Analog Devices AD485x DAS driver
+ *
+ * Copyright 2024 Analog Devices Inc.
+ */
+
+#include <linux/array_size.h>
+#include <linux/bits.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/minmax.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/pwm.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+#include <linux/types.h>
+#include <linux/units.h>
+
+#include <linux/iio/backend.h>
+#include <linux/iio/iio.h>
+
+#include <asm/unaligned.h>
+
+#define AD485X_REG_INTERFACE_CONFIG_A	0x00
+#define AD485X_REG_INTERFACE_CONFIG_B	0x01
+#define AD485X_REG_PRODUCT_ID_L		0x04
+#define AD485X_REG_PRODUCT_ID_H		0x05
+#define AD485X_REG_DEVICE_CTRL		0x25
+#define AD485X_REG_PACKET		0x26
+
+#define AD485X_REG_CH_CONFIG_BASE	0x2A
+#define AD485X_REG_CHX_SOFTSPAN(ch)	((0x12 * (ch)) + AD485X_REG_CH_CONFIG_BASE)
+#define AD485X_REG_CHX_OFFSET(ch)	(AD485X_REG_CHX_SOFTSPAN(ch) + 0x01)
+#define AD485X_REG_CHX_OFFSET_LSB(ch)	AD485X_REG_CHX_OFFSET(ch)
+#define AD485X_REG_CHX_OFFSET_MID(ch)	(AD485X_REG_CHX_OFFSET_LSB(ch) + 0x01)
+#define AD485X_REG_CHX_OFFSET_MSB(ch)	(AD485X_REG_CHX_OFFSET_MID(ch) + 0x01)
+#define AD485X_REG_CHX_GAIN(ch)		(AD485X_REG_CHX_OFFSET(ch) + 0x03)
+#define AD485X_REG_CHX_GAIN_LSB(ch)	AD485X_REG_CHX_GAIN(ch)
+#define AD485X_REG_CHX_GAIN_MSB(ch)	(AD485X_REG_CHX_GAIN(ch) + 0x01)
+#define AD485X_REG_CHX_PHASE(ch)	(AD485X_REG_CHX_GAIN(ch) + 0x02)
+#define AD485X_REG_CHX_PHASE_LSB(ch)	AD485X_REG_CHX_PHASE(ch)
+#define AD485X_REG_CHX_PHASE_MSB(ch)	(AD485X_REG_CHX_PHASE_LSB(ch) + 0x01)
+
+#define AD485X_REG_TESTPAT_0(c)		(0x38 + (c) * 0x12)
+#define AD485X_REG_TESTPAT_1(c)		(0x39 + (c) * 0x12)
+#define AD485X_REG_TESTPAT_2(c)		(0x3A + (c) * 0x12)
+#define AD485X_REG_TESTPAT_3(c)		(0x3B + (c) * 0x12)
+
+#define AD485X_SW_RESET			(BIT(7) | BIT(0))
+#define AD485X_SDO_ENABLE		BIT(4)
+#define AD485X_SINGLE_INSTRUCTION	BIT(7)
+#define AD485X_ECHO_CLOCK_MODE		BIT(0)
+
+#define AD485X_PACKET_FORMAT_0		0
+#define AD485X_PACKET_FORMAT_1		1
+#define AD485X_PACKET_FORMAT_MASK	GENMASK(1, 0)
+#define AD485X_OS_EN			BIT(7)
+
+#define AD485X_TEST_PAT			BIT(2)
+
+#define AD4858_PACKET_SIZE_20		0
+#define AD4858_PACKET_SIZE_24		1
+#define AD4858_PACKET_SIZE_32		2
+
+#define AD4857_PACKET_SIZE_16		0
+#define AD4857_PACKET_SIZE_24		1
+
+#define AD485X_TESTPAT_0_DEFAULT	0x2A
+#define AD485X_TESTPAT_1_DEFAULT	0x3C
+#define AD485X_TESTPAT_2_DEFAULT	0xCE
+#define AD485X_TESTPAT_3_DEFAULT(c)	(0x0A + (0x10 * (c)))
+
+#define AD485X_SOFTSPAN_0V_2V5		0
+#define AD485X_SOFTSPAN_N2V5_2V5	1
+#define AD485X_SOFTSPAN_0V_5V		2
+#define AD485X_SOFTSPAN_N5V_5V		3
+#define AD485X_SOFTSPAN_0V_6V25		4
+#define AD485X_SOFTSPAN_N6V25_6V25	5
+#define AD485X_SOFTSPAN_0V_10V		6
+#define AD485X_SOFTSPAN_N10V_10V	7
+#define AD485X_SOFTSPAN_0V_12V5		8
+#define AD485X_SOFTSPAN_N12V5_12V5	9
+#define AD485X_SOFTSPAN_0V_20V		10
+#define AD485X_SOFTSPAN_N20V_20V	11
+#define AD485X_SOFTSPAN_0V_25V		12
+#define AD485X_SOFTSPAN_N25V_25V	13
+#define AD485X_SOFTSPAN_0V_40V		14
+#define AD485X_SOFTSPAN_N40V_40V	15
+
+#define AD485X_MAX_LANES		8
+#define AD485X_MAX_IODELAY		32
+
+#define AD485X_T_CNVH_NS		40
+
+#define AD4858_PROD_ID			0x60
+#define AD4857_PROD_ID			0x61
+#define AD4856_PROD_ID			0x62
+#define AD4855_PROD_ID			0x63
+#define AD4854_PROD_ID			0x64
+#define AD4853_PROD_ID			0x65
+#define AD4852_PROD_ID			0x66
+#define AD4851_PROD_ID			0x67
+#define AD4858I_PROD_ID			0x6F
+
+struct ad485x_chip_info {
+	const char *name;
+	unsigned int product_id;
+	const unsigned int (*scale_table)[2];
+	int num_scales;
+	const int *offset_table;
+	int num_offset;
+	const struct iio_chan_spec *channels;
+	unsigned int num_channels;
+	unsigned long throughput;
+	unsigned int resolution;
+};
+
+struct ad485x_state {
+	struct spi_device *spi;
+	struct pwm_device *cnv;
+	struct iio_backend *back;
+	/*
+	 * Synchronize access to members the of driver state, and ensure
+	 * atomicity of consecutive regmap operations.
+	 */
+	struct mutex lock;
+	struct regmap *regmap;
+	const struct ad485x_chip_info *info;
+	struct gpio_desc *pd_gpio;
+	unsigned long sampling_freq;
+	unsigned int (*scales)[2];
+	int *offsets;
+};
+
+static int ad485x_reg_access(struct iio_dev *indio_dev,
+			     unsigned int reg,
+			     unsigned int writeval,
+			     unsigned int *readval)
+{
+	struct ad485x_state *st = iio_priv(indio_dev);
+
+	if (readval)
+		return regmap_read(st->regmap, reg, readval);
+
+	return regmap_write(st->regmap, reg, writeval);
+}
+
+static int ad485x_set_sampling_freq(struct ad485x_state *st, unsigned int freq)
+{
+	struct pwm_state cnv_state = {
+		.duty_cycle = AD485X_T_CNVH_NS,
+		.enabled = true,
+	};
+	int ret;
+
+	freq = clamp(freq, 0, st->info->throughput);
+
+	cnv_state.period = DIV_ROUND_CLOSEST_ULL(GIGA, freq);
+
+	ret = pwm_apply_might_sleep(st->cnv, &cnv_state);
+	if (ret)
+		return ret;
+
+	st->sampling_freq = freq;
+
+	return 0;
+}
+
+static int ad485x_setup(struct ad485x_state *st)
+{
+	unsigned int product_id;
+	int ret;
+
+	ret = ad485x_set_sampling_freq(st, HZ_PER_MHZ);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, AD485X_REG_INTERFACE_CONFIG_A,
+			   AD485X_SW_RESET);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, AD485X_REG_INTERFACE_CONFIG_B,
+			   AD485X_SINGLE_INSTRUCTION);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, AD485X_REG_INTERFACE_CONFIG_A,
+			   AD485X_SDO_ENABLE);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(st->regmap, AD485X_REG_PRODUCT_ID_L, &product_id);
+	if (ret)
+		return ret;
+
+	if (product_id != st->info->product_id)
+		dev_info(&st->spi->dev, "Unknown product ID: 0x%02X\n",
+			 product_id);
+
+	ret = regmap_write(st->regmap, AD485X_REG_DEVICE_CTRL,
+			   AD485X_ECHO_CLOCK_MODE);
+	if (ret)
+		return ret;
+
+	return regmap_write(st->regmap, AD485X_REG_PACKET, 0);
+}
+
+static int ad485x_find_opt(bool *field, u32 size, u32 *ret_start)
+{
+	unsigned int i, cnt = 0, max_cnt = 0, max_start = 0;
+	int start;
+
+	for (i = 0, start = -1; i < size; i++) {
+		if (field[i] == 0) {
+			if (start == -1)
+				start = i;
+			cnt++;
+		} else {
+			if (cnt > max_cnt) {
+				max_cnt = cnt;
+				max_start = start;
+			}
+			start = -1;
+			cnt = 0;
+		}
+	}
+
+	if (cnt > max_cnt) {
+		max_cnt = cnt;
+		max_start = start;
+	}
+
+	if (!max_cnt)
+		return -ENOENT;
+
+	*ret_start = max_start;
+
+	return max_cnt;
+}
+
+static int ad485x_calibrate(struct ad485x_state *st)
+{
+	unsigned int opt_delay, lane_num, delay, i, s, c;
+	enum iio_backend_interface_type interface_type;
+	bool pn_status[AD485X_MAX_LANES][AD485X_MAX_IODELAY];
+	int ret;
+
+	ret = iio_backend_interface_type_get(st->back, &interface_type);
+	if (ret)
+		return ret;
+
+	if (interface_type == IIO_BACKEND_INTERFACE_CMOS)
+		lane_num = st->info->num_channels;
+	else
+		lane_num = 1;
+
+	if (st->info->resolution == 16) {
+		ret = iio_backend_data_size_set(st->back, 24);
+		if (ret)
+			return ret;
+
+		ret = regmap_write(st->regmap, AD485X_REG_PACKET,
+				   AD485X_TEST_PAT | AD4857_PACKET_SIZE_24);
+		if (ret)
+			return ret;
+	} else {
+		ret = iio_backend_data_size_set(st->back, 32);
+		if (ret)
+			return ret;
+
+		ret = regmap_write(st->regmap, AD485X_REG_PACKET,
+				   AD485X_TEST_PAT | AD4858_PACKET_SIZE_32);
+		if (ret)
+			return ret;
+	}
+
+	for (i = 0; i < st->info->num_channels; i++) {
+		ret = regmap_write(st->regmap, AD485X_REG_TESTPAT_0(i),
+				   AD485X_TESTPAT_0_DEFAULT);
+		if (ret)
+			return ret;
+
+		ret = regmap_write(st->regmap, AD485X_REG_TESTPAT_1(i),
+				   AD485X_TESTPAT_1_DEFAULT);
+		if (ret)
+			return ret;
+
+		ret = regmap_write(st->regmap, AD485X_REG_TESTPAT_2(i),
+				   AD485X_TESTPAT_2_DEFAULT);
+		if (ret)
+			return ret;
+
+		ret = regmap_write(st->regmap, AD485X_REG_TESTPAT_3(i),
+				   AD485X_TESTPAT_3_DEFAULT(i));
+		if (ret)
+			return ret;
+
+		ret = iio_backend_chan_enable(st->back, i);
+		if (ret)
+			return ret;
+	}
+
+	for (i = 0; i < lane_num; i++) {
+		for (delay = 0; delay < AD485X_MAX_IODELAY; delay++) {
+			ret = iio_backend_iodelay_set(st->back, i, delay);
+			if (ret)
+				return ret;
+			ret = iio_backend_chan_status(st->back, i,
+						      &pn_status[i][delay]);
+			if (ret)
+				return ret;
+		}
+	}
+
+	for (i = 0; i < lane_num; i++) {
+		c = ad485x_find_opt(&pn_status[i][0], AD485X_MAX_IODELAY, &s);
+		if (c < 0)
+			return c;
+
+		opt_delay = s + c / 2;
+		ret = iio_backend_iodelay_set(st->back, i, opt_delay);
+		if (ret)
+			return ret;
+	}
+
+	for (i = 0; i < st->info->num_channels; i++) {
+		ret = iio_backend_chan_disable(st->back, i);
+		if (ret)
+			return ret;
+	}
+
+	ret = iio_backend_data_size_set(st->back, 20);
+	if (ret)
+		return ret;
+
+	return regmap_write(st->regmap, AD485X_REG_PACKET, 0);
+}
+
+static const char *const ad4858_packet_fmts[] = {
+	"20-bit", "24-bit", "32-bit",
+};
+
+static const char *const ad4857_packet_fmts[] = {
+	"16-bit", "24-bit",
+};
+
+static int ad485x_set_packet_format(struct iio_dev *indio_dev,
+				    const struct iio_chan_spec *chan,
+				    unsigned int format)
+{
+	struct ad485x_state *st = iio_priv(indio_dev);
+	unsigned int val;
+	int ret;
+
+	if (chan->scan_type.realbits == 20)
+		switch (format) {
+		case 0:
+			val = 20;
+			break;
+		case 1:
+			val = 24;
+			break;
+		case 2:
+			val = 32;
+			break;
+		default:
+			return -EINVAL;
+		}
+	else if (chan->scan_type.realbits == 16)
+		switch (format) {
+		case 0:
+			val = 16;
+			break;
+		case 1:
+			val = 24;
+			break;
+		default:
+			return -EINVAL;
+		}
+	else
+		return -EINVAL;
+
+	ret = iio_backend_data_size_set(st->back, val);
+	if (ret)
+		return ret;
+
+	return regmap_update_bits(st->regmap, AD485X_REG_PACKET,
+				  AD485X_PACKET_FORMAT_MASK, format);
+}
+
+static int ad485x_get_packet_format(struct iio_dev *indio_dev,
+				    const struct iio_chan_spec *chan)
+{
+	struct ad485x_state *st = iio_priv(indio_dev);
+	unsigned int format;
+	int ret;
+
+	ret = regmap_read(st->regmap, AD485X_REG_PACKET, &format);
+	if (ret)
+		return ret;
+
+	return FIELD_GET(AD485X_PACKET_FORMAT_MASK, format);
+}
+
+static const struct iio_enum ad4858_packet_fmt = {
+	.items = ad4858_packet_fmts,
+	.num_items = ARRAY_SIZE(ad4858_packet_fmts),
+	.set = ad485x_set_packet_format,
+	.get = ad485x_get_packet_format,
+};
+
+static const struct iio_enum ad4857_packet_fmt = {
+	.items = ad4857_packet_fmts,
+	.num_items = ARRAY_SIZE(ad4857_packet_fmts),
+	.set = ad485x_set_packet_format,
+	.get = ad485x_get_packet_format,
+};
+
+static int ad485x_get_calibscale(struct ad485x_state *st, int ch, int *val, int *val2)
+{
+	unsigned int reg_val;
+	int gain;
+	int ret;
+
+	guard(mutex)(&st->lock);
+
+	ret = regmap_read(st->regmap, AD485X_REG_CHX_GAIN_MSB(ch),
+			  &reg_val);
+	if (ret)
+		return ret;
+
+	gain = (reg_val & 0xFF) << 8;
+
+	ret = regmap_read(st->regmap, AD485X_REG_CHX_GAIN_LSB(ch),
+			  &reg_val);
+	if (ret)
+		return ret;
+
+	gain |= reg_val & 0xFF;
+
+	*val = gain;
+	*val2 = 32768;
+
+	return IIO_VAL_FRACTIONAL;
+}
+
+static int ad485x_set_calibscale(struct ad485x_state *st, int ch, int val,
+				 int val2)
+{
+	unsigned long long gain;
+	unsigned int reg_val;
+	int ret;
+
+	if (val < 0 || val2 < 0)
+		return -EINVAL;
+
+	gain = val * MICRO + val2;
+	gain = DIV_U64_ROUND_CLOSEST(gain * 32768, MICRO);
+
+	reg_val = gain;
+
+	guard(mutex)(&st->lock);
+
+	ret = regmap_write(st->regmap, AD485X_REG_CHX_GAIN_MSB(ch),
+			   reg_val >> 8);
+	if (ret)
+		return ret;
+
+	return regmap_write(st->regmap, AD485X_REG_CHX_GAIN_LSB(ch),
+			    reg_val & 0xFF);
+}
+
+static int ad485x_get_calibbias(struct ad485x_state *st, int ch, int *val)
+{
+	unsigned int lsb, mid, msb;
+	int ret;
+
+	guard(mutex)(&st->lock);
+
+	ret = regmap_read(st->regmap, AD485X_REG_CHX_OFFSET_MSB(ch),
+			  &msb);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(st->regmap, AD485X_REG_CHX_OFFSET_MID(ch),
+			  &mid);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(st->regmap, AD485X_REG_CHX_OFFSET_LSB(ch),
+			  &lsb);
+	if (ret)
+		return ret;
+
+	if (st->info->resolution == 16) {
+		*val = msb << 8;
+		*val |= mid;
+		*val = sign_extend32(*val, 15);
+	} else {
+		*val = msb << 12;
+		*val |= mid << 4;
+		*val |= lsb >> 4;
+		*val = sign_extend32(*val, 19);
+	}
+
+	return IIO_VAL_INT;
+}
+
+static int ad485x_set_calibbias(struct ad485x_state *st, int ch, int val)
+{
+	u8 buf[3] = { 0 };
+	int ret;
+
+	if (val < 0)
+		return -EINVAL;
+
+	if (st->info->resolution == 16)
+		put_unaligned_be16(val, buf);
+	else
+		put_unaligned_be24(val << 4, buf);
+
+	guard(mutex)(&st->lock);
+
+	ret = regmap_write(st->regmap, AD485X_REG_CHX_OFFSET_LSB(ch), buf[2]);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, AD485X_REG_CHX_OFFSET_MID(ch), buf[1]);
+	if (ret)
+		return ret;
+
+	return regmap_write(st->regmap, AD485X_REG_CHX_OFFSET_MSB(ch), buf[0]);
+}
+
+static const unsigned int ad485x_scale_table[][2] = {
+	{ 2500, 0x0 },
+	{ 5000, 0x1 },
+	{ 5000, 0x2 },
+	{ 10000, 0x3 },
+	{ 6250, 0x04 },
+	{ 12500, 0x5 },
+	{ 10000, 0x6 },
+	{ 20000, 0x7 },
+	{ 12500, 0x8 },
+	{ 25000, 0x9 },
+	{ 20000, 0xA },
+	{ 40000, 0xB },
+	{ 25000, 0xC },
+	{ 50000, 0xD },
+	{ 40000, 0xE },
+	{ 80000, 0xF },
+};
+
+static const int ad4857_offset_table[] = {
+	0, -32768,
+};
+
+static const int ad4858_offset_table[] = {
+	0, -524288,
+};
+
+static const unsigned int ad485x_scale_avail[] = {
+	2500, 5000, 10000, 6250, 12500, 20000, 25000, 40000, 50000, 80000,
+};
+
+static void __ad485x_get_scale(struct ad485x_state *st, int scale_tbl,
+			       unsigned int *val, unsigned int *val2)
+{
+	const struct ad485x_chip_info *info = st->info;
+	const struct iio_chan_spec *chan = &info->channels[0];
+	unsigned int tmp;
+
+	tmp = (scale_tbl * 1000000ULL) >> chan->scan_type.realbits;
+	*val = tmp / 1000000;
+	*val2 = tmp % 1000000;
+}
+
+static int ad485x_set_scale(struct ad485x_state *st,
+			    const struct iio_chan_spec *chan, int val, int val2)
+{
+	const struct ad485x_chip_info *info = st->info;
+	unsigned int scale_val[2];
+	unsigned int i;
+	bool single_ended = false;
+
+	for (i = 0; i < info->num_scales; i++) {
+		__ad485x_get_scale(st, info->scale_table[i][0],
+				   &scale_val[0], &scale_val[1]);
+		if (scale_val[0] != val || scale_val[1] != val2)
+			continue;
+
+		/*
+		 * Adjust the softspan value (differential or single ended)
+		 * based on the scale value selected and current offset of
+		 * the channel.
+		 *
+		 * If the offset is 0 then continue iterations until finding
+		 * the next matching scale value which always corresponds to
+		 * the single ended mode.
+		 */
+		if (!st->offsets[chan->channel] && !single_ended) {
+			single_ended = true;
+			continue;
+		}
+
+		return regmap_write(st->regmap,
+				    AD485X_REG_CHX_SOFTSPAN(chan->channel),
+				    info->scale_table[i][1]);
+	}
+
+	return -EINVAL;
+}
+
+static int ad485x_get_scale(struct ad485x_state *st,
+			    const struct iio_chan_spec *chan, int *val,
+			    int *val2)
+{
+	const struct ad485x_chip_info *info = st->info;
+	unsigned int i, softspan_val;
+	int ret;
+
+	ret = regmap_read(st->regmap, AD485X_REG_CHX_SOFTSPAN(chan->channel),
+			  &softspan_val);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < info->num_scales; i++) {
+		if (softspan_val == info->scale_table[i][1])
+			break;
+	}
+
+	if (i == info->num_scales)
+		return -EIO;
+
+	__ad485x_get_scale(st, info->scale_table[i][0], val, val2);
+
+	return IIO_VAL_INT_PLUS_MICRO;
+}
+
+static int ad485x_set_offset(struct ad485x_state *st,
+			     const struct iio_chan_spec *chan, int val)
+{
+	guard(mutex)(&st->lock);
+
+	if (val != st->offsets[chan->channel])
+		return 0;
+
+	st->offsets[chan->channel] = val;
+	/* Restore to the default range if offset changes */
+	if (st->offsets[chan->channel])
+		return regmap_write(st->regmap,
+					AD485X_REG_CHX_SOFTSPAN(chan->channel),
+					AD485X_SOFTSPAN_N40V_40V);
+	return regmap_write(st->regmap,
+				AD485X_REG_CHX_SOFTSPAN(chan->channel),
+				AD485X_SOFTSPAN_0V_40V);
+}
+
+static int ad485x_scale_offset_fill(struct ad485x_state *st)
+{
+	unsigned int i, val1, val2;
+
+	st->offsets = devm_kcalloc(&st->spi->dev, st->info->num_channels,
+				   sizeof(*st->offsets), GFP_KERNEL);
+	if (!st->offsets)
+		return -ENOMEM;
+
+	st->scales = devm_kmalloc_array(&st->spi->dev, ARRAY_SIZE(ad485x_scale_avail),
+					sizeof(*st->scales), GFP_KERNEL);
+	if (!st->scales)
+		return -ENOMEM;
+
+	for (i = 0; i < ARRAY_SIZE(ad485x_scale_avail); i++) {
+		__ad485x_get_scale(st, ad485x_scale_avail[i], &val1, &val2);
+		st->scales[i][0] = val1;
+		st->scales[i][1] = val2;
+	}
+
+	return 0;
+}
+
+static int ad485x_read_raw(struct iio_dev *indio_dev,
+			   const struct iio_chan_spec *chan,
+			   int *val, int *val2, long info)
+{
+	struct ad485x_state *st = iio_priv(indio_dev);
+
+	switch (info) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*val = st->sampling_freq;
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_CALIBSCALE:
+		return ad485x_get_calibscale(st, chan->channel, val, val2);
+	case IIO_CHAN_INFO_SCALE:
+		return ad485x_get_scale(st, chan, val, val2);
+	case IIO_CHAN_INFO_CALIBBIAS:
+		return ad485x_get_calibbias(st, chan->channel, val);
+	case IIO_CHAN_INFO_OFFSET:
+		scoped_guard(mutex, &st->lock)
+			*val = st->offsets[chan->channel];
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad485x_write_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int val, int val2, long info)
+{
+	struct ad485x_state *st = iio_priv(indio_dev);
+
+	switch (info) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		return ad485x_set_sampling_freq(st, val);
+	case IIO_CHAN_INFO_SCALE:
+		return ad485x_set_scale(st, chan, val, val2);
+	case IIO_CHAN_INFO_CALIBSCALE:
+		return ad485x_set_calibscale(st, chan->channel, val, val2);
+	case IIO_CHAN_INFO_CALIBBIAS:
+		return ad485x_set_calibbias(st, chan->channel, val);
+	case IIO_CHAN_INFO_OFFSET:
+		return ad485x_set_offset(st, chan, val);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad485x_update_scan_mode(struct iio_dev *indio_dev,
+				   const unsigned long *scan_mask)
+{
+	struct ad485x_state *st = iio_priv(indio_dev);
+	unsigned int c;
+	int ret;
+
+	for (c = 0; c < st->info->num_channels; c++) {
+		if (test_bit(c, scan_mask))
+			ret = iio_backend_chan_enable(st->back, c);
+		else
+			ret = iio_backend_chan_disable(st->back, c);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int ad485x_read_avail(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     const int **vals, int *type, int *length,
+			     long mask)
+{
+	struct ad485x_state *st = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		*vals = (const int *)st->scales;
+		*type = IIO_VAL_INT_PLUS_MICRO;
+		/* Values are stored in a 2D matrix */
+		*length = ARRAY_SIZE(ad485x_scale_avail) * 2;
+		return IIO_AVAIL_LIST;
+	case IIO_CHAN_INFO_OFFSET:
+		*vals = st->info->offset_table;
+		*type = IIO_VAL_INT;
+		*length = st->info->num_offset;
+		return IIO_AVAIL_LIST;
+	default:
+		return -EINVAL;
+	}
+}
+
+#define AD485X_IIO_CHANNEL(index, real, storage, info)			\
+{									\
+	.type = IIO_VOLTAGE,						\
+	.ext_info = info,						\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_CALIBSCALE) |		\
+		BIT(IIO_CHAN_INFO_CALIBBIAS) |				\
+		BIT(IIO_CHAN_INFO_SCALE) |				\
+		BIT(IIO_CHAN_INFO_OFFSET),				\
+	.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
+	.info_mask_shared_by_type_available =				\
+		BIT(IIO_CHAN_INFO_SCALE) |				\
+		BIT(IIO_CHAN_INFO_OFFSET),				\
+	.indexed = 1,							\
+	.channel = index,						\
+	.scan_index = index,						\
+	.scan_type = {							\
+		.sign = 's',						\
+		.realbits = real,					\
+		.storagebits = storage,					\
+	},								\
+}
+
+static struct iio_chan_spec_ext_info ad4858_ext_info[] = {
+	IIO_ENUM("packet_format", IIO_SHARED_BY_ALL, &ad4858_packet_fmt),
+	IIO_ENUM_AVAILABLE("packet_format",
+			   IIO_SHARED_BY_ALL, &ad4858_packet_fmt),
+	{}
+};
+
+static struct iio_chan_spec_ext_info ad4857_ext_info[] = {
+	IIO_ENUM("packet_format", IIO_SHARED_BY_ALL, &ad4857_packet_fmt),
+	IIO_ENUM_AVAILABLE("packet_format",
+			   IIO_SHARED_BY_ALL, &ad4857_packet_fmt),
+	{}
+};
+
+static const struct iio_chan_spec ad4858_channels[] = {
+	AD485X_IIO_CHANNEL(0, 20, 32, ad4858_ext_info),
+	AD485X_IIO_CHANNEL(1, 20, 32, ad4858_ext_info),
+	AD485X_IIO_CHANNEL(2, 20, 32, ad4858_ext_info),
+	AD485X_IIO_CHANNEL(3, 20, 32, ad4858_ext_info),
+	AD485X_IIO_CHANNEL(4, 20, 32, ad4858_ext_info),
+	AD485X_IIO_CHANNEL(5, 20, 32, ad4858_ext_info),
+	AD485X_IIO_CHANNEL(6, 20, 32, ad4858_ext_info),
+	AD485X_IIO_CHANNEL(7, 20, 32, ad4858_ext_info),
+};
+
+static const struct iio_chan_spec ad4857_channels[] = {
+	AD485X_IIO_CHANNEL(0, 16, 16, ad4857_ext_info),
+	AD485X_IIO_CHANNEL(1, 16, 16, ad4857_ext_info),
+	AD485X_IIO_CHANNEL(2, 16, 16, ad4857_ext_info),
+	AD485X_IIO_CHANNEL(3, 16, 16, ad4857_ext_info),
+	AD485X_IIO_CHANNEL(4, 16, 16, ad4857_ext_info),
+	AD485X_IIO_CHANNEL(5, 16, 16, ad4857_ext_info),
+	AD485X_IIO_CHANNEL(6, 16, 16, ad4857_ext_info),
+	AD485X_IIO_CHANNEL(7, 16, 16, ad4857_ext_info),
+};
+
+static const struct ad485x_chip_info ad4858_info = {
+	.name = "ad4858",
+	.product_id = AD4858_PROD_ID,
+	.scale_table = ad485x_scale_table,
+	.num_scales = ARRAY_SIZE(ad485x_scale_table),
+	.offset_table = ad4858_offset_table,
+	.num_offset = ARRAY_SIZE(ad4858_offset_table),
+	.channels = ad4858_channels,
+	.num_channels = ARRAY_SIZE(ad4858_channels),
+	.throughput = 1 * MEGA,
+	.resolution = 20,
+};
+
+static const struct ad485x_chip_info ad4857_info = {
+	.name = "ad4857",
+	.product_id = AD4857_PROD_ID,
+	.scale_table = ad485x_scale_table,
+	.num_scales = ARRAY_SIZE(ad485x_scale_table),
+	.offset_table = ad4857_offset_table,
+	.num_offset = ARRAY_SIZE(ad4857_offset_table),
+	.channels = ad4857_channels,
+	.num_channels = ARRAY_SIZE(ad4857_channels),
+	.throughput = 1 * MEGA,
+	.resolution = 16,
+};
+
+static const struct ad485x_chip_info ad4856_info = {
+	.name = "ad4856",
+	.product_id = AD4856_PROD_ID,
+	.scale_table = ad485x_scale_table,
+	.num_scales = ARRAY_SIZE(ad485x_scale_table),
+	.offset_table = ad4858_offset_table,
+	.num_offset = ARRAY_SIZE(ad4858_offset_table),
+	.channels = ad4858_channels,
+	.num_channels = ARRAY_SIZE(ad4858_channels),
+	.throughput = 250 * KILO,
+	.resolution = 20,
+};
+
+static const struct ad485x_chip_info ad4855_info = {
+	.name = "ad4855",
+	.product_id = AD4855_PROD_ID,
+	.scale_table = ad485x_scale_table,
+	.num_scales = ARRAY_SIZE(ad485x_scale_table),
+	.offset_table = ad4857_offset_table,
+	.num_offset = ARRAY_SIZE(ad4857_offset_table),
+	.channels = ad4857_channels,
+	.num_channels = ARRAY_SIZE(ad4857_channels),
+	.throughput = 250 * KILO,
+	.resolution = 16,
+};
+
+static const struct ad485x_chip_info ad4854_info = {
+	.name = "ad4854",
+	.product_id = AD4854_PROD_ID,
+	.scale_table = ad485x_scale_table,
+	.num_scales = ARRAY_SIZE(ad485x_scale_table),
+	.offset_table = ad4858_offset_table,
+	.num_offset = ARRAY_SIZE(ad4858_offset_table),
+	.channels = ad4858_channels,
+	.num_channels = ARRAY_SIZE(ad4858_channels),
+	.throughput = 1 * MEGA,
+	.resolution = 20,
+};
+
+static const struct ad485x_chip_info ad4853_info = {
+	.name = "ad4853",
+	.product_id = AD4853_PROD_ID,
+	.scale_table = ad485x_scale_table,
+	.num_scales = ARRAY_SIZE(ad485x_scale_table),
+	.offset_table = ad4857_offset_table,
+	.num_offset = ARRAY_SIZE(ad4857_offset_table),
+	.channels = ad4857_channels,
+	.num_channels = ARRAY_SIZE(ad4857_channels),
+	.throughput = 1 * MEGA,
+	.resolution = 16,
+};
+
+static const struct ad485x_chip_info ad4852_info = {
+	.name = "ad4852",
+	.product_id = AD4852_PROD_ID,
+	.scale_table = ad485x_scale_table,
+	.num_scales = ARRAY_SIZE(ad485x_scale_table),
+	.offset_table = ad4858_offset_table,
+	.num_offset = ARRAY_SIZE(ad4858_offset_table),
+	.channels = ad4858_channels,
+	.num_channels = ARRAY_SIZE(ad4858_channels),
+	.throughput = 250 * KILO,
+	.resolution = 20,
+};
+
+static const struct ad485x_chip_info ad4851_info = {
+	.name = "ad4851",
+	.product_id = AD4851_PROD_ID,
+	.scale_table = ad485x_scale_table,
+	.num_scales = ARRAY_SIZE(ad485x_scale_table),
+	.offset_table = ad4857_offset_table,
+	.num_offset = ARRAY_SIZE(ad4857_offset_table),
+	.channels = ad4857_channels,
+	.num_channels = ARRAY_SIZE(ad4857_channels),
+	.throughput = 250 * KILO,
+	.resolution = 16,
+};
+
+static const struct ad485x_chip_info ad4858i_info = {
+	.name = "ad4858i",
+	.product_id = AD4858I_PROD_ID,
+	.scale_table = ad485x_scale_table,
+	.num_scales = ARRAY_SIZE(ad485x_scale_table),
+	.offset_table = ad4858_offset_table,
+	.num_offset = ARRAY_SIZE(ad4858_offset_table),
+	.channels = ad4858_channels,
+	.num_channels = ARRAY_SIZE(ad4858_channels),
+	.throughput = 1 * MEGA,
+	.resolution = 20,
+};
+
+static const struct iio_info ad485x_info = {
+	.debugfs_reg_access = ad485x_reg_access,
+	.read_raw = ad485x_read_raw,
+	.write_raw = ad485x_write_raw,
+	.update_scan_mode = ad485x_update_scan_mode,
+	.read_avail = ad485x_read_avail,
+};
+
+static const struct regmap_config regmap_config = {
+	.reg_bits = 16,
+	.val_bits = 8,
+	.read_flag_mask = BIT(7),
+};
+
+static const char * const ad485x_power_supplies[] = {
+	"vcc",	"vdd", "vee", "vio",
+};
+
+static int ad485x_probe(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev;
+	struct ad485x_state *st;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+	st->spi = spi;
+
+	ret = devm_mutex_init(&spi->dev, &st->lock);
+	if (ret)
+		return ret;
+
+	ret = devm_regulator_bulk_get_enable(&spi->dev,
+					     ARRAY_SIZE(ad485x_power_supplies),
+					     ad485x_power_supplies);
+	if (ret)
+		return dev_err_probe(&spi->dev, ret,
+				     "failed to get and enable supplies\n");
+
+	ret = devm_regulator_get_enable_optional(&spi->dev, "vddh");
+	if (ret < 0 && ret != -ENODEV)
+		return dev_err_probe(&spi->dev, ret, "failed to get vddh voltage\n");
+
+	ret = devm_regulator_get_enable_optional(&spi->dev, "vddl");
+	if (ret < 0 && ret != -ENODEV)
+		return dev_err_probe(&spi->dev, ret, "failed to get vddl voltage\n");
+
+	st->pd_gpio = devm_gpiod_get_optional(&spi->dev, "pd", GPIOD_OUT_LOW);
+	if (IS_ERR(st->pd_gpio))
+		return dev_err_probe(&spi->dev, PTR_ERR(st->pd_gpio),
+				     "Error on requesting pd GPIO\n");
+
+	st->cnv = devm_pwm_get(&spi->dev, NULL);
+	if (IS_ERR(st->cnv))
+		return PTR_ERR(st->cnv);
+
+	st->info = spi_get_device_match_data(spi);
+	if (!st->info)
+		return -ENODEV;
+
+	st->regmap = devm_regmap_init_spi(spi, &regmap_config);
+	if (IS_ERR(st->regmap))
+		return PTR_ERR(st->regmap);
+
+	ret = ad485x_scale_offset_fill(st);
+	if (ret)
+		return ret;
+
+	ret = ad485x_setup(st);
+	if (ret)
+		return ret;
+
+	indio_dev->name = st->info->name;
+	indio_dev->channels = st->info->channels;
+	indio_dev->num_channels = st->info->num_channels;
+	indio_dev->info = &ad485x_info;
+
+	st->back = devm_iio_backend_get(&spi->dev, NULL);
+	if (IS_ERR(st->back))
+		return PTR_ERR(st->back);
+
+	ret = devm_iio_backend_request_buffer(&spi->dev, st->back, indio_dev);
+	if (ret)
+		return ret;
+
+	ret = devm_iio_backend_enable(&spi->dev, st->back);
+	if (ret)
+		return ret;
+
+	ret = ad485x_calibrate(st);
+	if (ret)
+		return ret;
+
+	return devm_iio_device_register(&spi->dev, indio_dev);
+}
+
+static const struct of_device_id ad485x_of_match[] = {
+	{ .compatible = "adi,ad4858", .data = &ad4858_info, },
+	{ .compatible = "adi,ad4857", .data = &ad4857_info, },
+	{ .compatible = "adi,ad4856", .data = &ad4856_info, },
+	{ .compatible = "adi,ad4855", .data = &ad4855_info, },
+	{ .compatible = "adi,ad4854", .data = &ad4854_info, },
+	{ .compatible = "adi,ad4853", .data = &ad4853_info, },
+	{ .compatible = "adi,ad4852", .data = &ad4852_info, },
+	{ .compatible = "adi,ad4851", .data = &ad4851_info, },
+	{ .compatible = "adi,ad4858i", .data = &ad4858i_info, },
+	{}
+};
+
+static const struct spi_device_id ad485x_spi_id[] = {
+	{ "ad4858", (kernel_ulong_t)&ad4858_info },
+	{ "ad4857", (kernel_ulong_t)&ad4857_info },
+	{ "ad4856", (kernel_ulong_t)&ad4856_info },
+	{ "ad4855", (kernel_ulong_t)&ad4855_info },
+	{ "ad4854", (kernel_ulong_t)&ad4854_info },
+	{ "ad4853", (kernel_ulong_t)&ad4853_info },
+	{ "ad4852", (kernel_ulong_t)&ad4852_info },
+	{ "ad4851", (kernel_ulong_t)&ad4851_info },
+	{ "ad4858i", (kernel_ulong_t)&ad4858i_info },
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, ad485x_spi_id);
+
+static struct spi_driver ad485x_driver = {
+	.probe = ad485x_probe,
+	.driver = {
+		.name   = "ad485x",
+		.of_match_table = ad485x_of_match,
+	},
+	.id_table = ad485x_spi_id,
+};
+module_spi_driver(ad485x_driver);
+
+MODULE_AUTHOR("Sergiu Cuciurean <sergiu.cuciurean@analog.com>");
+MODULE_AUTHOR("Dragos Bogdan <dragos.bogdan@analog.com>");
+MODULE_AUTHOR("Antoniu Miclaus <antoniu.miclaus@analog.com>");
+MODULE_DESCRIPTION("Analog Devices AD485x DAS driver");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(IIO_BACKEND);
-- 
2.46.2

Re: [PATCH v2 6/7] iio: adc: ad485x: add ad485x driver

[Jonathan Cameron]

On Fri, 4 Oct 2024 17:07:55 +0300
Antoniu Miclaus <antoniu.miclaus@analog.com> wrote:

> Add support for the AD485X DAS familiy.
As with the DT docs, it is good to have a little bit more info on the device
in this patch description. Just lets casual readers have some idea what
they are looking at!

I've left the packet_format ABI question for the next patch, but
I would like some info on the default.  The code doesn't set it currently 
but maybe for documentation purposes it should?

From the earlier discussion (ongoing) I think the oversampling control
requires particular formats. I'd like to see that in the driver
first with it overriding that format as necessary.

Then we can see whether we need the additional control

Various other minor comments inline.  With the exception of that
question of packet_format this is in a pretty good state.


Jonathan


> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---
> changes in v2:
>  - update headers and include the missing ones, order them alphabetically.
>  - use clamp()
>  - use units macros where applies
>  - convert int to unsigned variables where applies.
>  - add trailing commas where applies.
>  - return FIELD_GET directly.
>  - shrink function header to one line where it fits.
>  - update scale table values arrangement - pow-of-two per line
>  - rename j to have a proper meaning.
>  - invert if (st->offsets[chan->channel] != val) and drop next lines indentation.
>  - drop whitespace from * val = ... (altough checkpatch complains about it)
>  - drop comma in the terminator lines for ext_info.
>  - fix inconsistency between chip_info structures.
>  - use devm_mutex_init
>  - return -ENOENT on max_cnt check.
>  - check both val and val2 for negative before converting to unsigned.
>  - remove val2 where not used.
>  - use dev_info() instead of dev_warn()
>  - add spaces after { and before } in ad485x_scale_table
> 
>  drivers/iio/adc/Kconfig  |   12 +
>  drivers/iio/adc/Makefile |    1 +
>  drivers/iio/adc/ad485x.c | 1094 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 1107 insertions(+)
>  create mode 100644 drivers/iio/adc/ad485x.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index f60fe85a30d5..83f55229d731 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -36,6 +36,18 @@ config AD4130
>  	  To compile this driver as a module, choose M here: the module will be
>  	  called ad4130.
>  
> +config AD485X

No wild cards in drivers.  It goes wrong far too often!  Just
pick a part and name everything after that (files, functions etc).

For text, use 'and similar' to cover the range.

> +	tristate "Analog Device AD485x DAS Driver"
> +	depends on SPI
> +	select REGMAP_SPI
> +	select IIO_BACKEND
> +	help
> +	  Say yes here to build support for Analog Devices AD485x high speed
> +	  data acquisition system (DAS).

List all supported parts in here. Makes for easier grepping.

> +
> +	  To compile this driver as a module, choose M here: the module will be
> +	  called ad485x.
> +
>  config AD7091R
>  	tristate
>  
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index d370e066544e..26c1670c8913 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -7,6 +7,7 @@
>  obj-$(CONFIG_AB8500_GPADC) += ab8500-gpadc.o
>  obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o
>  obj-$(CONFIG_AD4130) += ad4130.o
> +obj-$(CONFIG_AD485X) += ad485x.o
>  obj-$(CONFIG_AD7091R) += ad7091r-base.o
>  obj-$(CONFIG_AD7091R5) += ad7091r5.o
>  obj-$(CONFIG_AD7091R8) += ad7091r8.o
> diff --git a/drivers/iio/adc/ad485x.c b/drivers/iio/adc/ad485x.c
> new file mode 100644
> index 000000000000..faa10d56a791
> --- /dev/null
> +++ b/drivers/iio/adc/ad485x.c
> @@ -0,0 +1,1094 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Analog Devices AD485x DAS driver
> + *
> + * Copyright 2024 Analog Devices Inc.
> + */
> +
> +#include <linux/array_size.h>
> +#include <linux/bits.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/minmax.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/pwm.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +#include <linux/types.h>
> +#include <linux/units.h>
> +
> +#include <linux/iio/backend.h>
> +#include <linux/iio/iio.h>
> +
> +#include <asm/unaligned.h>

> +
> +#define AD4858_PROD_ID			0x60
> +#define AD4857_PROD_ID			0x61
> +#define AD4856_PROD_ID			0x62
> +#define AD4855_PROD_ID			0x63
> +#define AD4854_PROD_ID			0x64
> +#define AD4853_PROD_ID			0x65
> +#define AD4852_PROD_ID			0x66
> +#define AD4851_PROD_ID			0x67
> +#define AD4858I_PROD_ID			0x6F
These don't add anything over just using the numeric value
to fill the chip_info structure field.  That field clearly documents
what these magic numbers are anyway.

Note we used to have this code pattern of defines at the top
for PROD_ID in a load more drivers but have gotten rid of them
in favour of inline values.  May well be more to do!

> +
> +struct ad485x_chip_info {
> +	const char *name;
> +	unsigned int product_id;
> +	const unsigned int (*scale_table)[2];
> +	int num_scales;
As below.  Currently we only seem to have one option for the
scale parameters.  If that is so for now, drop these and encode
that option directly.

> +	const int *offset_table;
> +	int num_offset;
> +	const struct iio_chan_spec *channels;
> +	unsigned int num_channels;
> +	unsigned long throughput;
> +	unsigned int resolution;
> +};


> +
> +static const char *const ad4858_packet_fmts[] = {
> +	"20-bit", "24-bit", "32-bit",
> +};
> +
> +static const char *const ad4857_packet_fmts[] = {
> +	"16-bit", "24-bit",
I'll leave discussing these for the docs patch.

> +};
> +
> +static int ad485x_set_packet_format(struct iio_dev *indio_dev,
> +				    const struct iio_chan_spec *chan,
> +				    unsigned int format)
> +{
> +	struct ad485x_state *st = iio_priv(indio_dev);
> +	unsigned int val;
> +	int ret;
> +
> +	if (chan->scan_type.realbits == 20)

Perhaps a switch would be cleaner?

> +		switch (format) {
> +		case 0:
> +			val = 20;
> +			break;
> +		case 1:
> +			val = 24;
> +			break;
> +		case 2:
> +			val = 32;
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +	else if (chan->scan_type.realbits == 16)
> +		switch (format) {
> +		case 0:
> +			val = 16;
> +			break;
> +		case 1:
> +			val = 24;
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +	else
> +		return -EINVAL;
> +
> +	ret = iio_backend_data_size_set(st->back, val);
> +	if (ret)
> +		return ret;
> +
> +	return regmap_update_bits(st->regmap, AD485X_REG_PACKET,
> +				  AD485X_PACKET_FORMAT_MASK, format);
> +}


> +static struct iio_chan_spec_ext_info ad4858_ext_info[] = {
> +	IIO_ENUM("packet_format", IIO_SHARED_BY_ALL, &ad4858_packet_fmt),
> +	IIO_ENUM_AVAILABLE("packet_format",
> +			   IIO_SHARED_BY_ALL, &ad4858_packet_fmt),
> +	{}
	{ }
> +};
> +
> +static struct iio_chan_spec_ext_info ad4857_ext_info[] = {
> +	IIO_ENUM("packet_format", IIO_SHARED_BY_ALL, &ad4857_packet_fmt),
> +	IIO_ENUM_AVAILABLE("packet_format",
> +			   IIO_SHARED_BY_ALL, &ad4857_packet_fmt),
> +	{}
	{ }

I'm slowly standardizing across IIO on this mostly so we have a 'right'
answer rather than a messy mix of styles.  The choice was a random
pick rather than there being anything better about having a space.

> +};

> +
> +static const struct ad485x_chip_info ad4858_info = {
> +	.name = "ad4858",
> +	.product_id = AD4858_PROD_ID,
> +	.scale_table = ad485x_scale_table,

There only seems to be one scale table.
Unless you plan to very soon add more devices, drop it from
here for now and access it directly in the code.

Reduces complexity a little + it is easy to bring back the
entries in here when you need them.

If you are planning to send out support for more parts shortly
then it is fine to keep this as it is and reduce the churn.
I'm just not keen on speculative flexibility in drivers
where it may never use used!

> +	.num_scales = ARRAY_SIZE(ad485x_scale_table),
> +	.offset_table = ad4858_offset_table,
> +	.num_offset = ARRAY_SIZE(ad4858_offset_table),
> +	.channels = ad4858_channels,
> +	.num_channels = ARRAY_SIZE(ad4858_channels),
> +	.throughput = 1 * MEGA,
> +	.resolution = 20,
> +};
...


> +
> +static const struct of_device_id ad485x_of_match[] = {
> +	{ .compatible = "adi,ad4858", .data = &ad4858_info, },
> +	{ .compatible = "adi,ad4857", .data = &ad4857_info, },
> +	{ .compatible = "adi,ad4856", .data = &ad4856_info, },
> +	{ .compatible = "adi,ad4855", .data = &ad4855_info, },
> +	{ .compatible = "adi,ad4854", .data = &ad4854_info, },
> +	{ .compatible = "adi,ad4853", .data = &ad4853_info, },
> +	{ .compatible = "adi,ad4852", .data = &ad4852_info, },
> +	{ .compatible = "adi,ad4851", .data = &ad4851_info, },
> +	{ .compatible = "adi,ad4858i", .data = &ad4858i_info, },
> +	{}
	{ }
+ reorder as below.


> +};
> +
> +static const struct spi_device_id ad485x_spi_id[] = {
> +	{ "ad4858", (kernel_ulong_t)&ad4858_info },
> +	{ "ad4857", (kernel_ulong_t)&ad4857_info },
> +	{ "ad4856", (kernel_ulong_t)&ad4856_info },
> +	{ "ad4855", (kernel_ulong_t)&ad4855_info },
> +	{ "ad4854", (kernel_ulong_t)&ad4854_info },
> +	{ "ad4853", (kernel_ulong_t)&ad4853_info },
> +	{ "ad4852", (kernel_ulong_t)&ad4852_info },
> +	{ "ad4851", (kernel_ulong_t)&ad4851_info },
alphanumeric order.  So pretty much the reverse.


> +	{ "ad4858i", (kernel_ulong_t)&ad4858i_info },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(spi, ad485x_spi_id);

Re: [PATCH v2 6/7] iio: adc: ad485x: add ad485x driver

[Andy Shevchenko]

On Fri, Oct 4, 2024 at 5:12 PM Antoniu Miclaus
<antoniu.miclaus@analog.com> wrote:
>
> Add support for the AD485X DAS familiy.

family.

...

> +#define AD485X_REG_PRODUCT_ID_L                0x04
> +#define AD485X_REG_PRODUCT_ID_H                0x05

Can this use bulk transfer with __le16 type?

...

> +#define AD485X_REG_CHX_OFFSET_LSB(ch)  AD485X_REG_CHX_OFFSET(ch)
> +#define AD485X_REG_CHX_OFFSET_MID(ch)  (AD485X_REG_CHX_OFFSET_LSB(ch) + 0x01)
> +#define AD485X_REG_CHX_OFFSET_MSB(ch)  (AD485X_REG_CHX_OFFSET_MID(ch) + 0x01)

But can you connect oscilloscope and actually see what's the difference?

...

> +#define AD485X_REG_CHX_GAIN_LSB(ch)    AD485X_REG_CHX_GAIN(ch)
> +#define AD485X_REG_CHX_GAIN_MSB(ch)    (AD485X_REG_CHX_GAIN(ch) + 0x01)

> +#define AD485X_REG_CHX_PHASE_LSB(ch)   AD485X_REG_CHX_PHASE(ch)
> +#define AD485X_REG_CHX_PHASE_MSB(ch)   (AD485X_REG_CHX_PHASE_LSB(ch) + 0x01)

__le16 + bulk transfers?

...

> +#define AD485X_SW_RESET                        (BIT(7) | BIT(0))

Is it really a bitfield? What then does each bit mean?

> +#define AD485X_SDO_ENABLE              BIT(4)
> +#define AD485X_SINGLE_INSTRUCTION      BIT(7)
> +#define AD485X_ECHO_CLOCK_MODE         BIT(0)

...

> +struct ad485x_chip_info {
> +       const char *name;
> +       unsigned int product_id;
> +       const unsigned int (*scale_table)[2];
> +       int num_scales;
> +       const int *offset_table;
> +       int num_offset;
> +       const struct iio_chan_spec *channels;
> +       unsigned int num_channels;
> +       unsigned long throughput;
> +       unsigned int resolution;

Have you run `pahole` for this and other data structures you
introduced? Is there any room for optimization?

> +};

...

> +static int ad485x_find_opt(bool *field, u32 size, u32 *ret_start)
> +{
> +       unsigned int i, cnt = 0, max_cnt = 0, max_start = 0;
> +       int start;
> +
> +       for (i = 0, start = -1; i < size; i++) {
> +               if (field[i] == 0) {
> +                       if (start == -1)
> +                               start = i;
> +                       cnt++;
> +               } else {
> +                       if (cnt > max_cnt) {
> +                               max_cnt = cnt;
> +                               max_start = start;
> +                       }
> +                       start = -1;
> +                       cnt = 0;
> +               }
> +       }
> +
> +       if (cnt > max_cnt) {
> +               max_cnt = cnt;
> +               max_start = start;
> +       }
> +
> +       if (!max_cnt)
> +               return -ENOENT;
> +
> +       *ret_start = max_start;
> +
> +       return max_cnt;
> +}

Can you describe the search algorithm in the comment?

...

> +static int ad485x_calibrate(struct ad485x_state *st)
> +{
> +       unsigned int opt_delay, lane_num, delay, i, s, c;
> +       enum iio_backend_interface_type interface_type;

> +       bool pn_status[AD485X_MAX_LANES][AD485X_MAX_IODELAY];

Why bool and not bitmap? I think I already asked this, but I don't
remember what you answered.

> +       int ret;

...

> +static int ad485x_set_packet_format(struct iio_dev *indio_dev,
> +                                   const struct iio_chan_spec *chan,
> +                                   unsigned int format)
> +{
> +       struct ad485x_state *st = iio_priv(indio_dev);
> +       unsigned int val;
> +       int ret;
> +
> +       if (chan->scan_type.realbits == 20)

Missing {}

> +               switch (format) {
> +               case 0:
> +                       val = 20;
> +                       break;
> +               case 1:
> +                       val = 24;
> +                       break;
> +               case 2:
> +                       val = 32;
> +                       break;
> +               default:
> +                       return -EINVAL;
> +               }
> +       else if (chan->scan_type.realbits == 16)

Ditto.

> +               switch (format) {
> +               case 0:
> +                       val = 16;
> +                       break;
> +               case 1:
> +                       val = 24;
> +                       break;
> +               default:
> +                       return -EINVAL;
> +               }
> +       else

Ditto.

> +               return -EINVAL;
> +
> +       ret = iio_backend_data_size_set(st->back, val);
> +       if (ret)
> +               return ret;
> +
> +       return regmap_update_bits(st->regmap, AD485X_REG_PACKET,
> +                                 AD485X_PACKET_FORMAT_MASK, format);
> +}

...

> +static int ad485x_get_calibscale(struct ad485x_state *st, int ch, int *val, int *val2)
> +{
> +       unsigned int reg_val;
> +       int gain;

Should be u8 gain[2] and...

> +       int ret;
> +
> +       guard(mutex)(&st->lock);
> +
> +       ret = regmap_read(st->regmap, AD485X_REG_CHX_GAIN_MSB(ch),
> +                         &reg_val);
> +       if (ret)
> +               return ret;
> +
> +       gain = (reg_val & 0xFF) << 8;
> +
> +       ret = regmap_read(st->regmap, AD485X_REG_CHX_GAIN_LSB(ch),
> +                         &reg_val);
> +       if (ret)
> +               return ret;
> +
> +       gain |= reg_val & 0xFF;

> +       *val = gain;

...get_unaligned_be/le16().

> +       *val2 = 32768;
> +
> +       return IIO_VAL_FRACTIONAL;
> +}

...

> +static int ad485x_set_calibscale(struct ad485x_state *st, int ch, int val,
> +                                int val2)
> +{
> +       unsigned long long gain;
> +       unsigned int reg_val;
> +       int ret;

> +       gain = val * MICRO + val2;
> +       gain = DIV_U64_ROUND_CLOSEST(gain * 32768, MICRO);
> +
> +       reg_val = gain;

In the similar way, put_unaligned and use gain[0], gain[1];

> +       ret = regmap_write(st->regmap, AD485X_REG_CHX_GAIN_MSB(ch),
> +                          reg_val >> 8);
> +       if (ret)
> +               return ret;
> +
> +       return regmap_write(st->regmap, AD485X_REG_CHX_GAIN_LSB(ch),
> +                           reg_val & 0xFF);
> +}

...

> +static int ad485x_get_calibbias(struct ad485x_state *st, int ch, int *val)
> +{

> +       if (st->info->resolution == 16) {
> +               *val = msb << 8;
> +               *val |= mid;
> +               *val = sign_extend32(*val, 15);

get_unaligned_be16()

> +       } else {
> +               *val = msb << 12;
> +               *val |= mid << 4;
> +               *val |= lsb >> 4;

get_unaligned_be24()

> +               *val = sign_extend32(*val, 19);
> +       }

> +}

...

> +static int ad485x_set_calibbias(struct ad485x_state *st, int ch, int val)
> +{
> +       u8 buf[3] = { 0 };

0 is not needed.

> +       int ret;
> +
> +       if (val < 0)
> +               return -EINVAL;
> +
> +       if (st->info->resolution == 16)
> +               put_unaligned_be16(val, buf);
> +       else
> +               put_unaligned_be24(val << 4, buf);

You see, you even did this! Why not in the above functions?

> +}

...

> +static const unsigned int ad485x_scale_avail[] = {
> +       2500, 5000, 10000, 6250, 12500, 20000, 25000, 40000, 50000, 80000,

It's better to have pow-of-2 of numbers on one line. So here each line up to 8?

       2500, 5000, 10000, 6250, 12500, 20000, 25000, 40000, /* 0-7 */
       50000, 80000,     /* 8-9 */

> +};
> +
> +static void __ad485x_get_scale(struct ad485x_state *st, int scale_tbl,
> +                              unsigned int *val, unsigned int *val2)
> +{
> +       const struct ad485x_chip_info *info = st->info;
> +       const struct iio_chan_spec *chan = &info->channels[0];
> +       unsigned int tmp;
> +
> +       tmp = (scale_tbl * 1000000ULL) >> chan->scan_type.realbits;

MICRO? MEGA?

> +       *val = tmp / 1000000;
> +       *val2 = tmp % 1000000;

Ditto.


> +}

--
With Best Regards,
Andy Shevchenko

RE: [PATCH v2 6/7] iio: adc: ad485x: add ad485x driver

[Miclaus, Antoniu]

> > +static int ad485x_get_calibscale(struct ad485x_state *st, int ch, int *val, int
> *val2)
> > +{
> > +       unsigned int reg_val;
> > +       int gain;
> 
> Should be u8 gain[2] and...

As discussed in previous patch series, the bulk operations won't work for these
chips. The CS needs to be raised between each byte read/written.

Therefore using u8 gain[2]  here  and in other places will be just an extra populated
array since  regmap_read requires `unsigned int` as input.

For the set functions indeed it is feasible since you can pass u8 directly to regmap_write.

Regards,
Antoniu
> > +       int ret;
> > +
> > +       guard(mutex)(&st->lock);
> > +
> > +       ret = regmap_read(st->regmap, AD485X_REG_CHX_GAIN_MSB(ch),
> > +                         &reg_val);
> > +       if (ret)
> > +               return ret;
> > +
> > +       gain = (reg_val & 0xFF) << 8;
> > +
> > +       ret = regmap_read(st->regmap, AD485X_REG_CHX_GAIN_LSB(ch),
> > +                         &reg_val);
> > +       if (ret)
> > +               return ret;
> > +
> > +       gain |= reg_val & 0xFF;
> 
...

Re: [PATCH v2 6/7] iio: adc: ad485x: add ad485x driver

[Andy Shevchenko]

On Tue, Oct 8, 2024 at 1:48 PM Miclaus, Antoniu
<Antoniu.Miclaus@analog.com> wrote:

...

> > > +       int gain;
> >
> > Should be u8 gain[2] and...

I should have added "...independently of the use of bulk operations."

> As discussed in previous patch series, the bulk operations won't work for these
> chips. The CS needs to be raised between each byte read/written.

And as it was put independently by two people, we should really know
the real issue on the wire with this. And if it's as described, try to
find an existing example in the kernel that uses a similar approach. I
think regmap SPI should have such a feature.

> Therefore using u8 gain[2]  here  and in other places will be just an extra populated
> array since  regmap_read requires `unsigned int` as input.

The problem here is open coded endianess conversion.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 6/7] iio: adc: ad485x: add ad485x driver

[David Lechner]

On 10/8/24 5:48 AM, Miclaus, Antoniu wrote:
>>> +static int ad485x_get_calibscale(struct ad485x_state *st, int ch, int *val, int
>> *val2)
>>> +{
>>> +       unsigned int reg_val;
>>> +       int gain;
>>
>> Should be u8 gain[2] and...
> 
> As discussed in previous patch series, the bulk operations won't work for these
> chips. The CS needs to be raised between each byte read/written.
> 

So the datasheet is wrong and Streaming Instruction Mode doesn't actually work?

There is also Nonstreaming Instruction Mode if we need to read/write nonconsecutive
registers without deasserting CS.

RE: [PATCH v2 6/7] iio: adc: ad485x: add ad485x driver

[Miclaus, Antoniu]

> On 10/8/24 5:48 AM, Miclaus, Antoniu wrote:
> >>> +static int ad485x_get_calibscale(struct ad485x_state *st, int ch, int *val,
> int
> >> *val2)
> >>> +{
> >>> +       unsigned int reg_val;
> >>> +       int gain;
> >>
> >> Should be u8 gain[2] and...
> >
> > As discussed in previous patch series, the bulk operations won't work for
> these
> > chips. The CS needs to be raised between each byte read/written.
> >
> 
> So the datasheet is wrong and Streaming Instruction Mode doesn't actually
> work?
> 
> There is also Nonstreaming Instruction Mode if we need to read/write
> nonconsecutive
> registers without deasserting CS.

The chip was set to Nonstreaming Instruction Mode from the start in the ad485x_setup() function.

And the datasheet specifies (page 49 of 70):
"In nonstreaming instruction mode, one or more SPI transactions can be provided in
a single SPI frame."

So I guess it is an error in the datasheet.

Re: [PATCH v2 6/7] iio: adc: ad485x: add ad485x driver

[David Lechner]

On 10/11/24 6:27 AM, Miclaus, Antoniu wrote:
>> On 10/8/24 5:48 AM, Miclaus, Antoniu wrote:
>>>>> +static int ad485x_get_calibscale(struct ad485x_state *st, int ch, int *val,
>> int
>>>> *val2)
>>>>> +{
>>>>> +       unsigned int reg_val;
>>>>> +       int gain;
>>>>
>>>> Should be u8 gain[2] and...
>>>
>>> As discussed in previous patch series, the bulk operations won't work for
>> these
>>> chips. The CS needs to be raised between each byte read/written.
>>>
>>
>> So the datasheet is wrong and Streaming Instruction Mode doesn't actually
>> work?
>>
>> There is also Nonstreaming Instruction Mode if we need to read/write
>> nonconsecutive
>> registers without deasserting CS.
> 
> The chip was set to Nonstreaming Instruction Mode from the start in the ad485x_setup() function.
> 
> And the datasheet specifies (page 49 of 70):
> "In nonstreaming instruction mode, one or more SPI transactions can be provided in
> a single SPI frame."
> 
> So I guess it is an error in the datasheet.

So maybe regmap bulk operations would work if we change it to
Streaming Instruction Mode (along with the correct increment
direction)?

RE: [PATCH v2 6/7] iio: adc: ad485x: add ad485x driver

[Miclaus, Antoniu]

> ...
> 
> > +static int ad485x_calibrate(struct ad485x_state *st)
> > +{
> > +       unsigned int opt_delay, lane_num, delay, i, s, c;
> > +       enum iio_backend_interface_type interface_type;
> 
> > +       bool pn_status[AD485X_MAX_LANES][AD485X_MAX_IODELAY];
> 
> Why bool and not bitmap? I think I already asked this, but I don't
> remember what you answered.

Both ` iio_backend_chan_status` and `ad4851_find_opt` require bool as
input parameter.

> 
> > +       int ret;
> 
> ...

Re: [PATCH v2 6/7] iio: adc: ad485x: add ad485x driver

[Andy Shevchenko]

On Tue, Oct 8, 2024 at 2:37 PM Miclaus, Antoniu
<Antoniu.Miclaus@analog.com> wrote:

...

> > > +       bool pn_status[AD485X_MAX_LANES][AD485X_MAX_IODELAY];
> >
> > Why bool and not bitmap? I think I already asked this, but I don't
> > remember what you answered.
>
> Both ` iio_backend_chan_status` and `ad4851_find_opt` require bool as
> input parameter.

test_bit() and family returns bool. So, what's the issue with that?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 6/7] iio: adc: ad485x: add ad485x driver

[kernel test robot]

Hi Antoniu,

kernel test robot noticed the following build errors:

[auto build test ERROR on v6.11]
[cannot apply to jic23-iio/togreg robh/for-next v6.12-rc1 linus/master next-20241004]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Antoniu-Miclaus/iio-backend-add-support-for-data-size-set/20241004-221608
base:   v6.11
patch link:    https://lore.kernel.org/r/20241004140922.233939-6-antoniu.miclaus%40analog.com
patch subject: [PATCH v2 6/7] iio: adc: ad485x: add ad485x driver
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20241008/202410080232.6SxmYFFA-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project fef3566a25ff0e34fb87339ba5e13eca17cec00f)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241008/202410080232.6SxmYFFA-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410080232.6SxmYFFA-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from drivers/iio/adc/ad485x.c:18:
   In file included from include/linux/regmap.h:20:
   In file included from include/linux/iopoll.h:14:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     548 |         val = __raw_readb(PCI_IOBASE + addr);
         |                           ~~~~~~~~~~ ^
   include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     561 |         val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
      37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
         |                                                   ^
   In file included from drivers/iio/adc/ad485x.c:18:
   In file included from include/linux/regmap.h:20:
   In file included from include/linux/iopoll.h:14:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     574 |         val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
      35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
         |                                                   ^
   In file included from drivers/iio/adc/ad485x.c:18:
   In file included from include/linux/regmap.h:20:
   In file included from include/linux/iopoll.h:14:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     585 |         __raw_writeb(value, PCI_IOBASE + addr);
         |                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     595 |         __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     605 |         __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   In file included from drivers/iio/adc/ad485x.c:19:
   In file included from include/linux/regulator/consumer.h:35:
   In file included from include/linux/suspend.h:5:
   In file included from include/linux/swap.h:9:
   In file included from include/linux/memcontrol.h:21:
   In file included from include/linux/mm.h:2232:
   include/linux/vmstat.h:517:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     517 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
>> drivers/iio/adc/ad485x.c:408:9: error: call to undeclared function 'FIELD_GET'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     408 |         return FIELD_GET(AD485X_PACKET_FORMAT_MASK, format);
         |                ^
   7 warnings and 1 error generated.


vim +/FIELD_GET +408 drivers/iio/adc/ad485x.c

   396	
   397	static int ad485x_get_packet_format(struct iio_dev *indio_dev,
   398					    const struct iio_chan_spec *chan)
   399	{
   400		struct ad485x_state *st = iio_priv(indio_dev);
   401		unsigned int format;
   402		int ret;
   403	
   404		ret = regmap_read(st->regmap, AD485X_REG_PACKET, &format);
   405		if (ret)
   406			return ret;
   407	
 > 408		return FIELD_GET(AD485X_PACKET_FORMAT_MASK, format);
   409	}
   410	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 6/7] iio: adc: ad485x: add ad485x driver

[Uwe Kleine-König]

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

Hello,

On Fri, Oct 04, 2024 at 05:07:55PM +0300, Antoniu Miclaus wrote:
> +static int ad485x_set_sampling_freq(struct ad485x_state *st, unsigned int freq)
> +{
> +	struct pwm_state cnv_state = {
> +		.duty_cycle = AD485X_T_CNVH_NS,
> +		.enabled = true,
> +	};
> +	int ret;
> +
> +	freq = clamp(freq, 0, st->info->throughput);

freq == 0 will become a problem in the next code line. Increase the
lower limit of the clamp to 1?!

> +	cnv_state.period = DIV_ROUND_CLOSEST_ULL(GIGA, freq);

Is ROUND_CLOSEST really the right thing here? The resulting period might
result in a frequency higher than freq, still more given that
pwm_apply_might_sleep() might round the period further down.

> +	ret = pwm_apply_might_sleep(st->cnv, &cnv_state);
> +	if (ret)
> +		return ret;
> +
> +	st->sampling_freq = freq;
> +
> +	return 0;
> +}
> [...]
> +static int ad485x_probe(struct spi_device *spi)
> +{
> [...]
> +	st->cnv = devm_pwm_get(&spi->dev, NULL);
> +	if (IS_ERR(st->cnv))
> +		return PTR_ERR(st->cnv);

devm_pwm_get() is silent on error, so adding an error message here would
be appropriate. I think the same applies to spi_get_device_match_data()
below.

> +	st->info = spi_get_device_match_data(spi);
> +	if (!st->info)
> +		return -ENODEV;
> [...]

Best regards
Uwe

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

[PATCH v2 7/7] Documentation: ABI: testing: ad485x: add ABI docs

[Antoniu Miclaus]

Add documentation for the packet size.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
changes in v2:
 - improve description for packet_format
 - add kernel version
 .../ABI/testing/sysfs-bus-iio-adc-ad485x         | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-ad485x

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-ad485x b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad485x
new file mode 100644
index 000000000000..5d69a8d30383
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad485x
@@ -0,0 +1,16 @@
+What:		/sys/bus/iio/devices/iio:deviceX/packet_format_available
+KernelVersion:	6.13
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Packet sizes on the CMOS or LVDS conversion data output bus.
+		Reading this returns the valid values that can be written to the
+		packet_format.
+
+What:		/sys/bus/iio/devices/iio:deviceX/packet_format
+KernelVersion:	6.13
+Contact:	linux-iio@vger.kernel.org
+Description:
+		This attribute configures the frame size on conversion data
+		output bus. See packet_format_available for available sizes
+		based on the device used.
+		Reading returns the actual size used.
-- 
2.46.2

Re: [PATCH v2 7/7] Documentation: ABI: testing: ad485x: add ABI docs

[Jonathan Cameron]

On Fri, 4 Oct 2024 17:07:56 +0300
Antoniu Miclaus <antoniu.miclaus@analog.com> wrote:

> Add documentation for the packet size.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---
> changes in v2:
>  - improve description for packet_format
>  - add kernel version
>  .../ABI/testing/sysfs-bus-iio-adc-ad485x         | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-ad485x
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-ad485x b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad485x
> new file mode 100644
> index 000000000000..5d69a8d30383
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad485x
> @@ -0,0 +1,16 @@
> +What:		/sys/bus/iio/devices/iio:deviceX/packet_format_available
> +KernelVersion:	6.13
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Packet sizes on the CMOS or LVDS conversion data output bus.
> +		Reading this returns the valid values that can be written to the
> +		packet_format.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/packet_format
> +KernelVersion:	6.13
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		This attribute configures the frame size on conversion data
> +		output bus. See packet_format_available for available sizes
> +		based on the device used.
> +		Reading returns the actual size used.
This needs to give some guidance to the user on 'why' they might pick a particular
format.

I'm also inclined to suggest that for now we pick a sensible default dependent
on the other options enabled (oversampling etc) and don't expose it to the user.

Eventually it looks like we may have to figure out a solution to describe
metadata packed alongside the channel readings but that may take a while
and I don't want to stall this driver on that discussion.

Thanks,

Jonathan

Re: [PATCH v2 7/7] Documentation: ABI: testing: ad485x: add ABI docs

[Nuno Sá]

On Sat, 2024-10-05 at 18:36 +0100, Jonathan Cameron wrote:
> On Fri, 4 Oct 2024 17:07:56 +0300
> Antoniu Miclaus <antoniu.miclaus@analog.com> wrote:
> 
> > Add documentation for the packet size.
> > 
> > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> > ---
> > changes in v2:
> >  - improve description for packet_format
> >  - add kernel version
> >  .../ABI/testing/sysfs-bus-iio-adc-ad485x         | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> >  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-ad485x
> > 
> > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-ad485x
> > b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad485x
> > new file mode 100644
> > index 000000000000..5d69a8d30383
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad485x
> > @@ -0,0 +1,16 @@
> > +What:		/sys/bus/iio/devices/iio:deviceX/packet_format_available
> > +KernelVersion:	6.13
> > +Contact:	linux-iio@vger.kernel.org
> > +Description:
> > +		Packet sizes on the CMOS or LVDS conversion data output
> > bus.
> > +		Reading this returns the valid values that can be written
> > to the
> > +		packet_format.
> > +
> > +What:		/sys/bus/iio/devices/iio:deviceX/packet_format
> > +KernelVersion:	6.13
> > +Contact:	linux-iio@vger.kernel.org
> > +Description:
> > +		This attribute configures the frame size on conversion data
> > +		output bus. See packet_format_available for available sizes
> > +		based on the device used.
> > +		Reading returns the actual size used.
> This needs to give some guidance to the user on 'why' they might pick a
> particular
> format.
> 
> I'm also inclined to suggest that for now we pick a sensible default dependent
> on the other options enabled (oversampling etc) and don't expose it to the
> user. 
> 

Just for documentations and if someone does not want to check the datasheet :):

- Nonoversampling Packet Formats:

			  -------------------
20bit (packet_size = 0) - |20 bit conversion|
			  -------------------

                          -------------------------------------------
24bit (packet_size = 1) - |20 bit conversion| OR/UR | 3 bit chan_id |
			  -------------------------------------------

                          --------------------------------------------------------------------
32bit (packet_size = 2) - |20 bit conversion| OR/UR | 3 bit chan_id | 4 bit softspan | 0s... |
			  --------------------------------------------------------------------

- Oversampling Packet Formats

			  -------------------
20bit (packet_size = 0) - |20 bit conversion|
			  -------------------

                          --------------------
24bit (packet_size = 1) - |24 bit conversion |
			  --------------------

                          ------------------------------------------------------------
32bit (packet_size = 2) - |24 bit conversion| OR/UR | 3 bit chan_id | 4 bit softspan |
			  ------------------------------------------------------------

> Eventually it looks like we may have to figure out a solution to describe
> metadata packed alongside the channel readings but that may take a while
> and I don't want to stall this driver on that discussion.
> 

There's something still not fully clear to me. So, oversampling would be easy to
deal with (for arguing about some packet size). OR/UR is the more tricky case
because of having metadata. But I'm trying to understand on how it could look
because we still need a way to enable/disable OR/UR. 

Do you have in mind to have a form of metadata description in 'struct
iio_scan_types' plus additional IIO_METADATA channels to enable/disable those
bits? For this usecase and as OR/UR actually depends on the packet format we
could flip things with something like in_metadataX_enable and then argue about
the packet size settings.

But things get messier if we look closer to the packets as we can see that for
non oversampled samples, the softspan info is optional. Now, I'm not convinced
about that information being useful in the sample as we already have the scale
attribute and I'm not expecting people to change it during buffering... But for
the fun of things, how could we handle it if we cared (or if we actually care)
about it? Custom ABI like in_metadataX_scan_enable?

Other thing that came to mind and that might be a sneaky way of bypassing the
metadata stuff (for now) is to use events. AFAIU, we have status registers were
we can check the OR/UR and push those as normal events. But we would need to
rely on an external trigger as hrtimer or something like that. So we could have
this "slowpath" for checking the channel status but still use the events ABI
(and this is the sneaky part) to argue about the packet_size and whether or not
that info will be available in the sample through DMA. Not sure if it's worth it
though...

- Nuno Sá

Re: [PATCH v2 1/7] iio: backend: add API for interface get

[David Lechner]

On 10/4/24 9:07 AM, Antoniu Miclaus wrote:
> Add backend support for obtaining the interface type used.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---
> changes in v2:
>  - add IIO_BACKEND_INTERFACE_COUNT in enum.
>  - add trailing commas where applies.
>  drivers/iio/industrialio-backend.c | 24 ++++++++++++++++++++++++
>  include/linux/iio/backend.h        | 11 +++++++++++
>  2 files changed, 35 insertions(+)
> 
> diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
> index efe05be284b6..a322b0be7b2c 100644
> --- a/drivers/iio/industrialio-backend.c
> +++ b/drivers/iio/industrialio-backend.c
> @@ -449,6 +449,30 @@ 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 interace type used.

spelling: s/interace/interface

> + * @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_COUNT)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_NS_GPL(iio_backend_interface_type_get, IIO_BACKEND);
> +
>  /**
>   * iio_backend_extend_chan_spec - Extend an IIO channel
>   * @indio_dev: IIO device
> diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h
> index 8099759d7242..34fc76c99d8a 100644
> --- a/include/linux/iio/backend.h
> +++ b/include/linux/iio/backend.h
> @@ -63,6 +63,12 @@ enum iio_backend_sample_trigger {
>  	IIO_BACKEND_SAMPLE_TRIGGER_MAX
>  };
>  
> +enum iio_backend_interface_type {
> +	IIO_BACKEND_INTERFACE_LVDS,
> +	IIO_BACKEND_INTERFACE_CMOS,

There are some chips that can do both serial and parallel, so I
think these qualifiers should be included in the enum as well.

	IIO_BACKEND_INTERFACE_SERIAL_LVDS,
	IIO_BACKEND_INTERFACE_SERIAL_CMOS,

> +	IIO_BACKEND_INTERFACE_COUNT,

No comma here, and add a comment that this should always be last
since it is a count and not an interface type descriptor.

> +};
> +
>  /**
>   * struct iio_backend_ops - operations structure for an iio_backend
>   * @enable: Enable backend.
> @@ -81,6 +87,7 @@ 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.
>   **/
>  struct iio_backend_ops {
>  	int (*enable)(struct iio_backend *back);
> @@ -113,6 +120,8 @@ 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 iio_backend_chan_enable(struct iio_backend *back, unsigned int chan);
> @@ -142,6 +151,8 @@ ssize_t iio_backend_ext_info_set(struct iio_dev *indio_dev, uintptr_t private,
>  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_extend_chan_spec(struct iio_dev *indio_dev,
>  				 struct iio_backend *back,
>  				 struct iio_chan_spec *chan);