[PATCH v2 0/6] minor fixes and improvements

[PATCH v2 0/6] minor fixes and improvements

[Angelo Dureghello]

From: Angelo Dureghello <adureghello@baylibre.com>

After testing this driver, add some minor fixes and improvements,
as adding single channel variants support (ad3541r, ad3551r), also as a
preparatory step to bigger future improvements related to fast-rate mode
for this DAC family.

Previous patches (v1, 3/3)
https://lore.kernel.org/linux-iio/20240510141836.1624009-1-adureghello@baylibre.org
https://lore.kernel.org/linux-iio/20240510141836.1624009-2-adureghello@baylibre.org/
https://lore.kernel.org/linux-iio/20240510141836.1624009-3-adureghello@baylibre.org/

Angelo Dureghello (6):
  dt-bindings: iio: dac: fix ad3552r gain parameter names
  dt-bindings: iio: dac: add ad35xxr single output variants
  iio: dac: ad3552r: add model data structure
  iio: dac: ad3552r: add support for ad3541r and ad3551r
  iio: dac: ad3552r: change AD3552R_NUM_CH define name
  iio: dac: ad3552r: uniform structure names

 .../bindings/iio/dac/adi,ad3552r.yaml         |  43 ++++--
 drivers/iio/dac/ad3552r.c                     | 140 ++++++++++++------
 2 files changed, 128 insertions(+), 55 deletions(-)

-- 
2.45.0.rc1

[PATCH v2 1/6] dt-bindings: iio: dac: fix ad3552r gain parameter names

[Angelo Dureghello]

From: Angelo Dureghello <adureghello@baylibre.com>

The adi,gain-scaling-p/n values are an inverted log2,
so initial naming was set correctly, but the driver uses just
adi,gain-scaling-p/n, and if fdt is created accordingly with
the fdt bindings documentation, driver fails the probe.

Observing that:
- the Linux driver is the only consumer,
- there are no upstreamed dts nodes related to ad3552r,

the fix to the documentation side is preferred and less-risk.

Fixes: b0a96c5f599e ("dt-bindings: iio: dac: Add adi,ad3552r.yaml")
Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
Changes for v2:
- better explanation in the commit notes
---
 .../devicetree/bindings/iio/dac/adi,ad3552r.yaml | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml
index 8265d709094d..4e9f80445405 100644
--- a/Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml
+++ b/Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml
@@ -92,13 +92,13 @@ patternProperties:
             maximum: 511
             minimum: -511
 
-          adi,gain-scaling-p-inv-log2:
-            description: GainP = 1 / ( 2 ^ adi,gain-scaling-p-inv-log2)
+          adi,gain-scaling-p:
+            description: GainP = 1 / ( 2 ^ adi,gain-scaling-p)
             $ref: /schemas/types.yaml#/definitions/uint32
             enum: [0, 1, 2, 3]
 
-          adi,gain-scaling-n-inv-log2:
-            description: GainN = 1 / ( 2 ^ adi,gain-scaling-n-inv-log2)
+          adi,gain-scaling-n:
+            description: GainN = 1 / ( 2 ^ adi,gain-scaling-n)
             $ref: /schemas/types.yaml#/definitions/uint32
             enum: [0, 1, 2, 3]
 
@@ -107,8 +107,8 @@ patternProperties:
 
         required:
           - adi,gain-offset
-          - adi,gain-scaling-p-inv-log2
-          - adi,gain-scaling-n-inv-log2
+          - adi,gain-scaling-p
+          - adi,gain-scaling-n
           - adi,rfb-ohms
 
     required:
@@ -208,8 +208,8 @@ examples:
                 reg = <1>;
                 custom-output-range-config {
                     adi,gain-offset = <5>;
-                    adi,gain-scaling-p-inv-log2 = <1>;
-                    adi,gain-scaling-n-inv-log2 = <2>;
+                    adi,gain-scaling-p = <1>;
+                    adi,gain-scaling-n = <2>;
                     adi,rfb-ohms = <1>;
                 };
             };
-- 
2.45.0.rc1

[PATCH v2 2/6] dt-bindings: iio: dac: add ad35xxr single output variants

[Angelo Dureghello]

From: Angelo Dureghello <adureghello@baylibre.com>

Add support for ad3541r and ad3551r single output variants.

Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
Changes for v2:
- bounds reg value to 0 and channel nodes for the 1-channel models.
---
 .../bindings/iio/dac/adi,ad3552r.yaml         | 27 +++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml
index 4e9f80445405..fc8b97f82077 100644
--- a/Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml
+++ b/Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml
@@ -13,13 +13,17 @@ maintainers:
 description: |
   Bindings for the Analog Devices AD3552R DAC device and similar.
   Datasheet can be found here:
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ad3541r.pdf
     https://www.analog.com/media/en/technical-documentation/data-sheets/ad3542r.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ad3551r.pdf
     https://www.analog.com/media/en/technical-documentation/data-sheets/ad3552r.pdf
 
 properties:
   compatible:
     enum:
+      - adi,ad3541r
       - adi,ad3542r
+      - adi,ad3551r
       - adi,ad3552r
 
   reg:
@@ -128,7 +132,9 @@ allOf:
       properties:
         compatible:
           contains:
-            const: adi,ad3542r
+            enum:
+              - adi,ad3541r
+              - adi,ad3542r
     then:
       patternProperties:
         "^channel@([0-1])$":
@@ -158,7 +164,9 @@ allOf:
       properties:
         compatible:
           contains:
-            const: adi,ad3552r
+            enum:
+              - adi,ad3551r
+              - adi,ad3552r
     then:
       patternProperties:
         "^channel@([0-1])$":
@@ -182,6 +190,21 @@ allOf:
                     - const: -10000000
                     - const: 10000000
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - adi,ad3541r
+              - adi,ad3551r
+    then:
+      properties:
+        channel@1: false
+        channel@0:
+          properties:
+            reg:
+              const: 0
+
 required:
   - compatible
   - reg
-- 
2.45.0.rc1

[PATCH v2 3/6] iio: dac: ad3552r: add model data structure

[Angelo Dureghello]

From: Angelo Dureghello <adureghello@baylibre.com>

Add a "model data" structure to keep useful hardware-related
information as from datasheet, avoiding id-based conditional
choices later on.

Removed id-based checks and filled model-specific structures
with device specific features, In particular, num_hw_channels
is introduced to keep the number of hardware implemented
channels, since 1-channel versions of the DACs are added
in this same patchset.

Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
Changes for v2:
- patch added in v2
---
 drivers/iio/dac/ad3552r.c | 98 +++++++++++++++++++++++----------------
 1 file changed, 59 insertions(+), 39 deletions(-)

diff --git a/drivers/iio/dac/ad3552r.c b/drivers/iio/dac/ad3552r.c
index a492e8f2fc0f..6a40c7eece1f 100644
--- a/drivers/iio/dac/ad3552r.c
+++ b/drivers/iio/dac/ad3552r.c
@@ -261,7 +261,17 @@ struct ad3552r_ch_data {
 	bool	range_override;
 };
 
+struct ad3552r_model_data {
+	const char *model_name;
+	enum ad3542r_id chip_id;
+	unsigned int num_hw_channels;
+	const s32 (*ranges_table)[2];
+	int num_ranges;
+	bool requires_output_range;
+};
+
 struct ad3552r_desc {
+	const struct ad3552r_model_data *model_data;
 	/* Used to look the spi bus for atomic operations where needed */
 	struct mutex		lock;
 	struct gpio_desc	*gpio_reset;
@@ -271,7 +281,6 @@ struct ad3552r_desc {
 	struct iio_chan_spec	channels[AD3552R_NUM_CH + 1];
 	unsigned long		enabled_ch;
 	unsigned int		num_ch;
-	enum ad3542r_id		chip_id;
 };
 
 static const u16 addr_mask_map[][2] = {
@@ -745,13 +754,8 @@ static void ad3552r_calc_gain_and_offset(struct ad3552r_desc *dac, s32 ch)
 	} else {
 		/* Normal range */
 		idx = dac->ch_data[ch].range;
-		if (dac->chip_id == AD3542R_ID) {
-			v_min = ad3542r_ch_ranges[idx][0];
-			v_max = ad3542r_ch_ranges[idx][1];
-		} else {
-			v_min = ad3552r_ch_ranges[idx][0];
-			v_max = ad3552r_ch_ranges[idx][1];
-		}
+		v_min = dac->model_data->ranges_table[idx][0];
+		v_max = dac->model_data->ranges_table[idx][1];
 	}
 
 	/*
@@ -775,22 +779,14 @@ static void ad3552r_calc_gain_and_offset(struct ad3552r_desc *dac, s32 ch)
 	dac->ch_data[ch].offset_dec = div_s64(tmp, span);
 }
 
-static int ad3552r_find_range(u16 id, s32 *vals)
+static int ad3552r_find_range(const struct ad3552r_model_data *model_data,
+			      s32 *vals)
 {
-	int i, len;
-	const s32 (*ranges)[2];
+	int i;
 
-	if (id == AD3542R_ID) {
-		len = ARRAY_SIZE(ad3542r_ch_ranges);
-		ranges = ad3542r_ch_ranges;
-	} else {
-		len = ARRAY_SIZE(ad3552r_ch_ranges);
-		ranges = ad3552r_ch_ranges;
-	}
-
-	for (i = 0; i < len; i++)
-		if (vals[0] == ranges[i][0] * 1000 &&
-		    vals[1] == ranges[i][1] * 1000)
+	for (i = 0; i < model_data->num_ranges; i++)
+		if (vals[0] == model_data->ranges_table[i][0] * 1000 &&
+		    vals[1] == model_data->ranges_table[i][1] * 1000)
 			return i;
 
 	return -EINVAL;
@@ -955,9 +951,9 @@ static int ad3552r_configure_device(struct ad3552r_desc *dac)
 			dev_err(dev, "mandatory reg property missing\n");
 			goto put_child;
 		}
-		if (ch >= AD3552R_NUM_CH) {
+		if (ch >= dac->model_data->num_hw_channels) {
 			dev_err(dev, "reg must be less than %d\n",
-				AD3552R_NUM_CH);
+				dac->model_data->num_hw_channels);
 			err = -EINVAL;
 			goto put_child;
 		}
@@ -973,7 +969,7 @@ static int ad3552r_configure_device(struct ad3552r_desc *dac)
 				goto put_child;
 			}
 
-			err = ad3552r_find_range(dac->chip_id, vals);
+			err = ad3552r_find_range(dac->model_data, vals);
 			if (err < 0) {
 				dev_err(dev,
 					"Invalid adi,output-range-microvolt value\n");
@@ -987,9 +983,10 @@ static int ad3552r_configure_device(struct ad3552r_desc *dac)
 				goto put_child;
 
 			dac->ch_data[ch].range = val;
-		} else if (dac->chip_id == AD3542R_ID) {
+		} else if (dac->model_data->requires_output_range) {
 			dev_err(dev,
-				"adi,output-range-microvolt is required for ad3542r\n");
+				"adi,output-range-microvolt is required for %s\n",
+				dac->model_data->model_name);
 			err = -EINVAL;
 			goto put_child;
 		} else {
@@ -1011,7 +1008,8 @@ static int ad3552r_configure_device(struct ad3552r_desc *dac)
 	}
 
 	/* Disable unused channels */
-	for_each_clear_bit(ch, &dac->enabled_ch, AD3552R_NUM_CH) {
+	for_each_clear_bit(ch, &dac->enabled_ch,
+			   dac->model_data->num_hw_channels) {
 		err = ad3552r_set_ch_value(dac, AD3552R_CH_AMPLIFIER_POWERDOWN,
 					   ch, 1);
 		if (err)
@@ -1058,7 +1056,7 @@ static int ad3552r_init(struct ad3552r_desc *dac)
 	}
 
 	id |= val << 8;
-	if (id != dac->chip_id) {
+	if (id != dac->model_data->chip_id) {
 		dev_err(&dac->spi->dev, "Product id not matching\n");
 		return -ENODEV;
 	}
@@ -1068,7 +1066,6 @@ static int ad3552r_init(struct ad3552r_desc *dac)
 
 static int ad3552r_probe(struct spi_device *spi)
 {
-	const struct spi_device_id *id = spi_get_device_id(spi);
 	struct ad3552r_desc *dac;
 	struct iio_dev *indio_dev;
 	int err;
@@ -1079,7 +1076,9 @@ static int ad3552r_probe(struct spi_device *spi)
 
 	dac = iio_priv(indio_dev);
 	dac->spi = spi;
-	dac->chip_id = id->driver_data;
+	dac->model_data = spi_get_device_match_data(spi);
+	if (!dac->model_data)
+		return -EINVAL;
 
 	mutex_init(&dac->lock);
 
@@ -1088,10 +1087,7 @@ static int ad3552r_probe(struct spi_device *spi)
 		return err;
 
 	/* Config triggered buffer device */
-	if (dac->chip_id == AD3552R_ID)
-		indio_dev->name = "ad3552r";
-	else
-		indio_dev->name = "ad3542r";
+	indio_dev->name = dac->model_data->model_name;
 	indio_dev->dev.parent = &spi->dev;
 	indio_dev->info = &ad3552r_iio_info;
 	indio_dev->num_channels = dac->num_ch;
@@ -1109,16 +1105,40 @@ static int ad3552r_probe(struct spi_device *spi)
 	return devm_iio_device_register(&spi->dev, indio_dev);
 }
 
+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 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[] = {
-	{ "ad3542r", AD3542R_ID },
-	{ "ad3552r", AD3552R_ID },
+	{
+		.name = "ad3542r",
+		.driver_data = (kernel_ulong_t)&ad3542r_model_data
+	},
+	{
+		.name = "ad3552r",
+		.driver_data = (kernel_ulong_t)&ad3552r_model_data
+	},
 	{ }
 };
 MODULE_DEVICE_TABLE(spi, ad3552r_id);
 
 static const struct of_device_id ad3552r_of_match[] = {
-	{ .compatible = "adi,ad3542r"},
-	{ .compatible = "adi,ad3552r"},
+	{ .compatible = "adi,ad3542r", .data = &ad3542r_model_data },
+	{ .compatible = "adi,ad3552r", .data = &ad3552r_model_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, ad3552r_of_match);
-- 
2.45.0.rc1

[PATCH v2 4/6] iio: dac: ad3552r: add support for ad3541r and ad3551r

[Angelo Dureghello]

From: Angelo Dureghello <adureghello@baylibre.com>

Add support for single-output DAC variants.

Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
Changes for v2:
- adding new models using model data structure.
---
 drivers/iio/dac/ad3552r.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/iio/dac/ad3552r.c b/drivers/iio/dac/ad3552r.c
index 6a40c7eece1f..b4630fb89334 100644
--- a/drivers/iio/dac/ad3552r.c
+++ b/drivers/iio/dac/ad3552r.c
@@ -140,7 +140,9 @@ enum ad3552r_ch_vref_select {
 };
 
 enum ad3542r_id {
+	AD3541R_ID = 0x400b,
 	AD3542R_ID = 0x4009,
+	AD3551R_ID = 0x400a,
 	AD3552R_ID = 0x4008,
 };
 
@@ -1105,6 +1107,15 @@ 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,
@@ -1114,6 +1125,15 @@ static const struct ad3552r_model_data ad3542r_model_data = {
 	.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,
@@ -1124,10 +1144,18 @@ static const struct ad3552r_model_data ad3552r_model_data = {
 };
 
 static const struct spi_device_id ad3552r_id[] = {
+	{
+		.name = "ad3541r",
+		.driver_data = (kernel_ulong_t)&ad3541r_model_data
+	},
 	{
 		.name = "ad3542r",
 		.driver_data = (kernel_ulong_t)&ad3542r_model_data
 	},
+	{
+		.name = "ad3551r",
+		.driver_data = (kernel_ulong_t)&ad3551r_model_data
+	},
 	{
 		.name = "ad3552r",
 		.driver_data = (kernel_ulong_t)&ad3552r_model_data
@@ -1137,7 +1165,9 @@ static const struct spi_device_id ad3552r_id[] = {
 MODULE_DEVICE_TABLE(spi, ad3552r_id);
 
 static const struct of_device_id ad3552r_of_match[] = {
+	{ .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 },
 	{ }
 };
-- 
2.45.0.rc1

[PATCH v2 5/6] iio: dac: ad3552r: change AD3552R_NUM_CH define name

[Angelo Dureghello]

From: Angelo Dureghello <adureghello@baylibre.com>

After model data and num_hw_channles introduction, we have:

ad3552r_desc, num_ch: used to keep channel number set in fdt,
ad35xxr_model_data, num_hw_channels: for max channel checks,
AD3552R_NUM_CH: just actually used to define the max array size
on allocated arrays.

Renaming AD3552R_NUM_CH to a more consistent name, as AD3552R_MAX_CH.

Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
Changes for v2:
- patch added in v2
---
 drivers/iio/dac/ad3552r.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/dac/ad3552r.c b/drivers/iio/dac/ad3552r.c
index b4630fb89334..b8bdbfed22e3 100644
--- a/drivers/iio/dac/ad3552r.c
+++ b/drivers/iio/dac/ad3552r.c
@@ -117,7 +117,7 @@
 #define AD3552R_REG_ADDR_CH_INPUT_24B(ch)		(0x4B - (1 - ch) * 3)
 
 /* Useful defines */
-#define AD3552R_NUM_CH					2
+#define AD3552R_MAX_CH					2
 #define AD3552R_MASK_CH(ch)				BIT(ch)
 #define AD3552R_MASK_ALL_CH				GENMASK(1, 0)
 #define AD3552R_MAX_REG_SIZE				3
@@ -279,8 +279,8 @@ struct ad3552r_desc {
 	struct gpio_desc	*gpio_reset;
 	struct gpio_desc	*gpio_ldac;
 	struct spi_device	*spi;
-	struct ad3552r_ch_data	ch_data[AD3552R_NUM_CH];
-	struct iio_chan_spec	channels[AD3552R_NUM_CH + 1];
+	struct ad3552r_ch_data	ch_data[AD3552R_MAX_CH];
+	struct iio_chan_spec	channels[AD3552R_MAX_CH + 1];
 	unsigned long		enabled_ch;
 	unsigned int		num_ch;
 };
@@ -539,7 +539,7 @@ static int32_t ad3552r_trigger_hw_ldac(struct gpio_desc *ldac)
 static int ad3552r_write_all_channels(struct ad3552r_desc *dac, u8 *data)
 {
 	int err, len;
-	u8 addr, buff[AD3552R_NUM_CH * AD3552R_MAX_REG_SIZE + 1];
+	u8 addr, buff[AD3552R_MAX_CH * AD3552R_MAX_REG_SIZE + 1];
 
 	addr = AD3552R_REG_ADDR_CH_INPUT_24B(1);
 	/* CH1 */
@@ -597,7 +597,7 @@ static irqreturn_t ad3552r_trigger_handler(int irq, void *p)
 	struct iio_buffer *buf = indio_dev->buffer;
 	struct ad3552r_desc *dac = iio_priv(indio_dev);
 	/* Maximum size of a scan */
-	u8 buff[AD3552R_NUM_CH * AD3552R_MAX_REG_SIZE];
+	u8 buff[AD3552R_MAX_CH * AD3552R_MAX_REG_SIZE];
 	int err;
 
 	memset(buff, 0, sizeof(buff));
-- 
2.45.0.rc1

[PATCH v2 6/6] iio: dac: ad3552r: uniform structure names

[Angelo Dureghello]

From: Angelo Dureghello <adureghello@baylibre.com>

Use same driver file name (ad3552r) for structure names used
for all variants.

Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
Changes for v2:
- patch added in v2
---
 drivers/iio/dac/ad3552r.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/dac/ad3552r.c b/drivers/iio/dac/ad3552r.c
index b8bdbfed22e3..b060cdbc715d 100644
--- a/drivers/iio/dac/ad3552r.c
+++ b/drivers/iio/dac/ad3552r.c
@@ -139,7 +139,7 @@ enum ad3552r_ch_vref_select {
 	AD3552R_EXTERNAL_VREF_PIN_INPUT
 };
 
-enum ad3542r_id {
+enum ad3552r_id {
 	AD3541R_ID = 0x400b,
 	AD3542R_ID = 0x4009,
 	AD3551R_ID = 0x400a,
@@ -265,7 +265,7 @@ struct ad3552r_ch_data {
 
 struct ad3552r_model_data {
 	const char *model_name;
-	enum ad3542r_id chip_id;
+	enum ad3552r_id chip_id;
 	unsigned int num_hw_channels;
 	const s32 (*ranges_table)[2];
 	int num_ranges;
-- 
2.45.0.rc1

Re: [PATCH v2 1/6] dt-bindings: iio: dac: fix ad3552r gain parameter names

[Krzysztof Kozlowski]

On 22/05/2024 17:01, Angelo Dureghello wrote:
> From: Angelo Dureghello <adureghello@baylibre.com>
> 
> The adi,gain-scaling-p/n values are an inverted log2,
> so initial naming was set correctly, but the driver uses just
> adi,gain-scaling-p/n, and if fdt is created accordingly with
> the fdt bindings documentation, driver fails the probe.
> 
> Observing that:
> - the Linux driver is the only consumer,
> - there are no upstreamed dts nodes related to ad3552r,
> 
> the fix to the documentation side is preferred and less-risk.
> 
> Fixes: b0a96c5f599e ("dt-bindings: iio: dac: Add adi,ad3552r.yaml")
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> ---
> Changes for v2:
> - better explanation in the commit notes

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v2 2/6] dt-bindings: iio: dac: add ad35xxr single output variants

[Krzysztof Kozlowski]

On 22/05/2024 17:01, Angelo Dureghello wrote:
> From: Angelo Dureghello <adureghello@baylibre.com>
> 
> Add support for ad3541r and ad3551r single output variants.
> 
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> ---
> Changes for v2:
> - bounds reg value to 0 and channel nodes for the 1-channel models.
> ---

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v2 3/6] iio: dac: ad3552r: add model data structure

[Nuno Sá]

On Wed, 2024-05-22 at 17:01 +0200, Angelo Dureghello wrote:
> From: Angelo Dureghello <adureghello@baylibre.com>
> 
> Add a "model data" structure to keep useful hardware-related
> information as from datasheet, avoiding id-based conditional
> choices later on.
> 
> Removed id-based checks and filled model-specific structures
> with device specific features, In particular, num_hw_channels
> is introduced to keep the number of hardware implemented
> channels, since 1-channel versions of the DACs are added
> in this same patchset.
> 
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> ---
> Changes for v2:
> - patch added in v2
> ---
>  drivers/iio/dac/ad3552r.c | 98 +++++++++++++++++++++++----------------
>  1 file changed, 59 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/iio/dac/ad3552r.c b/drivers/iio/dac/ad3552r.c
> index a492e8f2fc0f..6a40c7eece1f 100644
> --- a/drivers/iio/dac/ad3552r.c
> +++ b/drivers/iio/dac/ad3552r.c
> @@ -261,7 +261,17 @@ struct ad3552r_ch_data {
>  	bool	range_override;
>  };
>  
> +struct ad3552r_model_data {
> +	const char *model_name;
> +	enum ad3542r_id chip_id;
> +	unsigned int num_hw_channels;
> +	const s32 (*ranges_table)[2];
> +	int num_ranges;
> +	bool requires_output_range;
> +};
> +

nit: we often would call this (in IIO) ad3552r_chip_info. Then...
>  struct ad3552r_desc {
> +	const struct ad3552r_model_data *model_data;

*chip_info;

Anyways, not really something worth a re-spin. But if you need one, something to
consider :)

- Nuno Sá

Re: [PATCH v2 0/6] minor fixes and improvements

[Nuno Sá]

On Wed, 2024-05-22 at 17:01 +0200, Angelo Dureghello wrote:
> From: Angelo Dureghello <adureghello@baylibre.com>
> 
> After testing this driver, add some minor fixes and improvements,
> as adding single channel variants support (ad3541r, ad3551r), also as a
> preparatory step to bigger future improvements related to fast-rate mode
> for this DAC family.
> 
> Previous patches (v1, 3/3)
> https://lore.kernel.org/linux-iio/20240510141836.1624009-1-adureghello@baylibre.org
> https://lore.kernel.org/linux-iio/20240510141836.1624009-2-adureghello@baylibre.org/
> https://lore.kernel.org/linux-iio/20240510141836.1624009-3-adureghello@baylibre.org/
> 
> Angelo Dureghello (6):
>   dt-bindings: iio: dac: fix ad3552r gain parameter names
>   dt-bindings: iio: dac: add ad35xxr single output variants
>   iio: dac: ad3552r: add model data structure
>   iio: dac: ad3552r: add support for ad3541r and ad3551r
>   iio: dac: ad3552r: change AD3552R_NUM_CH define name
>   iio: dac: ad3552r: uniform structure names
> 
>  .../bindings/iio/dac/adi,ad3552r.yaml         |  43 ++++--
>  drivers/iio/dac/ad3552r.c                     | 140 ++++++++++++------
>  2 files changed, 128 insertions(+), 55 deletions(-)
> 

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

Re: [PATCH v2 0/6] minor fixes and improvements

[Jonathan Cameron]

On Thu, 23 May 2024 14:45:01 +0200
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Wed, 2024-05-22 at 17:01 +0200, Angelo Dureghello wrote:
> > From: Angelo Dureghello <adureghello@baylibre.com>
> > 
> > After testing this driver, add some minor fixes and improvements,
> > as adding single channel variants support (ad3541r, ad3551r), also as a
> > preparatory step to bigger future improvements related to fast-rate mode
> > for this DAC family.
> > 
> > Previous patches (v1, 3/3)
> > https://lore.kernel.org/linux-iio/20240510141836.1624009-1-adureghello@baylibre.org
> > https://lore.kernel.org/linux-iio/20240510141836.1624009-2-adureghello@baylibre.org/
> > https://lore.kernel.org/linux-iio/20240510141836.1624009-3-adureghello@baylibre.org/
> > 
> > Angelo Dureghello (6):
> >   dt-bindings: iio: dac: fix ad3552r gain parameter names
> >   dt-bindings: iio: dac: add ad35xxr single output variants
> >   iio: dac: ad3552r: add model data structure
> >   iio: dac: ad3552r: add support for ad3541r and ad3551r
> >   iio: dac: ad3552r: change AD3552R_NUM_CH define name
> >   iio: dac: ad3552r: uniform structure names
> > 
> >  .../bindings/iio/dac/adi,ad3552r.yaml         |  43 ++++--
> >  drivers/iio/dac/ad3552r.c                     | 140 ++++++++++++------
> >  2 files changed, 128 insertions(+), 55 deletions(-)
> >   
> 
> Reviewed-by: Nuno Sa <nuno.sa@analog.com>
> 
This series crossed with a series using
device_for_each_child_node_scoped()

I've rebased on top of that. Was moderately straightforwards but
given last week I messed a similar change up completely please
check the testing branch of iio.git!

The mess was all it the patch adding model_data

Thanks,

Jonathan

> 

Re: [PATCH v2 0/6] minor fixes and improvements

[Angelo Dureghello]

Hi,

On 25/05/24 7:06 PM, Jonathan Cameron wrote:
> On Thu, 23 May 2024 14:45:01 +0200
> Nuno Sá <noname.nuno@gmail.com> wrote:
>
>> On Wed, 2024-05-22 at 17:01 +0200, Angelo Dureghello wrote:
>>> From: Angelo Dureghello <adureghello@baylibre.com>
>>>
>>> After testing this driver, add some minor fixes and improvements,
>>> as adding single channel variants support (ad3541r, ad3551r), also as a
>>> preparatory step to bigger future improvements related to fast-rate mode
>>> for this DAC family.
>>>
>>> Previous patches (v1, 3/3)
>>> https://lore.kernel.org/linux-iio/20240510141836.1624009-1-adureghello@baylibre.org
>>> https://lore.kernel.org/linux-iio/20240510141836.1624009-2-adureghello@baylibre.org/
>>> https://lore.kernel.org/linux-iio/20240510141836.1624009-3-adureghello@baylibre.org/
>>>
>>> Angelo Dureghello (6):
>>>    dt-bindings: iio: dac: fix ad3552r gain parameter names
>>>    dt-bindings: iio: dac: add ad35xxr single output variants
>>>    iio: dac: ad3552r: add model data structure
>>>    iio: dac: ad3552r: add support for ad3541r and ad3551r
>>>    iio: dac: ad3552r: change AD3552R_NUM_CH define name
>>>    iio: dac: ad3552r: uniform structure names
>>>
>>>   .../bindings/iio/dac/adi,ad3552r.yaml         |  43 ++++--
>>>   drivers/iio/dac/ad3552r.c                     | 140 ++++++++++++------
>>>   2 files changed, 128 insertions(+), 55 deletions(-)
>>>    
>> Reviewed-by: Nuno Sa <nuno.sa@analog.com>
>>
> This series crossed with a series using
> device_for_each_child_node_scoped()
>
> I've rebased on top of that. Was moderately straightforwards but
> given last week I messed a similar change up completely please
> check the testing branch of iio.git!
>
> The mess was all it the patch adding model_data

Thanks for this, sure, will check.


> Thanks,
>
> Jonathan

Regards,
angelo

Re: [PATCH v2 0/6] minor fixes and improvements

[Angelo Dureghello]

Hi Jonathan,

On 25/05/24 7:06 PM, Jonathan Cameron wrote:
> On Thu, 23 May 2024 14:45:01 +0200
> Nuno Sá <noname.nuno@gmail.com> wrote:
>
>> On Wed, 2024-05-22 at 17:01 +0200, Angelo Dureghello wrote:
>>> From: Angelo Dureghello <adureghello@baylibre.com>
>>>
>>> After testing this driver, add some minor fixes and improvements,
>>> as adding single channel variants support (ad3541r, ad3551r), also as a
>>> preparatory step to bigger future improvements related to fast-rate mode
>>> for this DAC family.
>>>
>>> Previous patches (v1, 3/3)
>>> https://lore.kernel.org/linux-iio/20240510141836.1624009-1-adureghello@baylibre.org
>>> https://lore.kernel.org/linux-iio/20240510141836.1624009-2-adureghello@baylibre.org/
>>> https://lore.kernel.org/linux-iio/20240510141836.1624009-3-adureghello@baylibre.org/
>>>
>>> Angelo Dureghello (6):
>>>    dt-bindings: iio: dac: fix ad3552r gain parameter names
>>>    dt-bindings: iio: dac: add ad35xxr single output variants
>>>    iio: dac: ad3552r: add model data structure
>>>    iio: dac: ad3552r: add support for ad3541r and ad3551r
>>>    iio: dac: ad3552r: change AD3552R_NUM_CH define name
>>>    iio: dac: ad3552r: uniform structure names
>>>
>>>   .../bindings/iio/dac/adi,ad3552r.yaml         |  43 ++++--
>>>   drivers/iio/dac/ad3552r.c                     | 140 ++++++++++++------
>>>   2 files changed, 128 insertions(+), 55 deletions(-)
>>>    
>> Reviewed-by: Nuno Sa <nuno.sa@analog.com>
>>
> This series crossed with a series using
> device_for_each_child_node_scoped()
>
> I've rebased on top of that. Was moderately straightforwards but
> given last week I messed a similar change up completely please
> check the testing branch of iio.git!
>
> The mess was all it the patch adding model_data

i tested the driver from the iio testing beranch,
it works as expected.

> Thanks,
>
> Jonathan

Thanks,

Regards,
angelo

Re: [PATCH v2 0/6] minor fixes and improvements

[Angelo Dureghello]

Hi Jonathan, Nuno,

just to be sure, is there something else needed from my side
for this patch-set ?


Thanks,
regards,
angelo

On 28/05/24 9:16 PM, Angelo Dureghello wrote:
> Hi Jonathan,
>
> On 25/05/24 7:06 PM, Jonathan Cameron wrote:
>> On Thu, 23 May 2024 14:45:01 +0200
>> Nuno Sá <noname.nuno@gmail.com> wrote:
>>
>>> On Wed, 2024-05-22 at 17:01 +0200, Angelo Dureghello wrote:
>>>> From: Angelo Dureghello <adureghello@baylibre.com>
>>>>
>>>> After testing this driver, add some minor fixes and improvements,
>>>> as adding single channel variants support (ad3541r, ad3551r), also 
>>>> as a
>>>> preparatory step to bigger future improvements related to fast-rate 
>>>> mode
>>>> for this DAC family.
>>>>
>>>> Previous patches (v1, 3/3)
>>>> https://lore.kernel.org/linux-iio/20240510141836.1624009-1-adureghello@baylibre.org 
>>>>
>>>> https://lore.kernel.org/linux-iio/20240510141836.1624009-2-adureghello@baylibre.org/ 
>>>>
>>>> https://lore.kernel.org/linux-iio/20240510141836.1624009-3-adureghello@baylibre.org/ 
>>>>
>>>>
>>>> Angelo Dureghello (6):
>>>>    dt-bindings: iio: dac: fix ad3552r gain parameter names
>>>>    dt-bindings: iio: dac: add ad35xxr single output variants
>>>>    iio: dac: ad3552r: add model data structure
>>>>    iio: dac: ad3552r: add support for ad3541r and ad3551r
>>>>    iio: dac: ad3552r: change AD3552R_NUM_CH define name
>>>>    iio: dac: ad3552r: uniform structure names
>>>>
>>>>   .../bindings/iio/dac/adi,ad3552r.yaml         |  43 ++++--
>>>>   drivers/iio/dac/ad3552r.c                     | 140 
>>>> ++++++++++++------
>>>>   2 files changed, 128 insertions(+), 55 deletions(-)
>>> Reviewed-by: Nuno Sa <nuno.sa@analog.com>
>>>
>> This series crossed with a series using
>> device_for_each_child_node_scoped()
>>
>> I've rebased on top of that. Was moderately straightforwards but
>> given last week I messed a similar change up completely please
>> check the testing branch of iio.git!
>>
>> The mess was all it the patch adding model_data
>
> i tested the driver from the iio testing beranch,
> it works as expected.
>
>> Thanks,
>>
>> Jonathan
>
> Thanks,
>
> Regards,
> angelo
>
>
-- 
  ,,,      Angelo Dureghello
:: :.     BayLibre -runtime team- Developer
:`___:
  `____:

Re: [PATCH v2 0/6] minor fixes and improvements

[Jonathan Cameron]

On Wed, 12 Jun 2024 18:35:32 +0200
Angelo Dureghello <adureghello@baylibre.com> wrote:

> Hi Jonathan, Nuno,
> 
> just to be sure, is there something else needed from my side
> for this patch-set ?
> 

All good now.  It's queued up and so just a case of me sending a pull request
to Greg KH. If he takes that it'll then be in char-misc and Greg will then send
pull request to Linus when the merge window opens.

Jonathan

> 
> Thanks,
> regards,
> angelo
> 
> On 28/05/24 9:16 PM, Angelo Dureghello wrote:
> > Hi Jonathan,
> >
> > On 25/05/24 7:06 PM, Jonathan Cameron wrote:  
> >> On Thu, 23 May 2024 14:45:01 +0200
> >> Nuno Sá <noname.nuno@gmail.com> wrote:
> >>  
> >>> On Wed, 2024-05-22 at 17:01 +0200, Angelo Dureghello wrote:  
> >>>> From: Angelo Dureghello <adureghello@baylibre.com>
> >>>>
> >>>> After testing this driver, add some minor fixes and improvements,
> >>>> as adding single channel variants support (ad3541r, ad3551r), also 
> >>>> as a
> >>>> preparatory step to bigger future improvements related to fast-rate 
> >>>> mode
> >>>> for this DAC family.
> >>>>
> >>>> Previous patches (v1, 3/3)
> >>>> https://lore.kernel.org/linux-iio/20240510141836.1624009-1-adureghello@baylibre.org 
> >>>>
> >>>> https://lore.kernel.org/linux-iio/20240510141836.1624009-2-adureghello@baylibre.org/ 
> >>>>
> >>>> https://lore.kernel.org/linux-iio/20240510141836.1624009-3-adureghello@baylibre.org/ 
> >>>>
> >>>>
> >>>> Angelo Dureghello (6):
> >>>>    dt-bindings: iio: dac: fix ad3552r gain parameter names
> >>>>    dt-bindings: iio: dac: add ad35xxr single output variants
> >>>>    iio: dac: ad3552r: add model data structure
> >>>>    iio: dac: ad3552r: add support for ad3541r and ad3551r
> >>>>    iio: dac: ad3552r: change AD3552R_NUM_CH define name
> >>>>    iio: dac: ad3552r: uniform structure names
> >>>>
> >>>>   .../bindings/iio/dac/adi,ad3552r.yaml         |  43 ++++--
> >>>>   drivers/iio/dac/ad3552r.c                     | 140 
> >>>> ++++++++++++------
> >>>>   2 files changed, 128 insertions(+), 55 deletions(-)  
> >>> Reviewed-by: Nuno Sa <nuno.sa@analog.com>
> >>>  
> >> This series crossed with a series using
> >> device_for_each_child_node_scoped()
> >>
> >> I've rebased on top of that. Was moderately straightforwards but
> >> given last week I messed a similar change up completely please
> >> check the testing branch of iio.git!
> >>
> >> The mess was all it the patch adding model_data  
> >
> > i tested the driver from the iio testing beranch,
> > it works as expected.
> >  
> >> Thanks,
> >>
> >> Jonathan  
> >
> > Thanks,
> >
> > Regards,
> > angelo
> >
> >