[PATCH v3 0/8] iio: dac: ds4424: add DS4402/DS4404 support and scale

[PATCH v3 0/8] iio: dac: ds4424: add DS4402/DS4404 support and scale

[Oleksij Rempel]

This series extends the ds4424 IIO DAC driver and its devicetree binding
to support the DS4402 and DS4404 current DAC variants.

DS440x devices share the same register map as DS442x but use a different
resolution (5-bit vs 7-bit) and a different full-scale current formula.
The full-scale current depends on external Rfs resistors connected to
the FS pins, so a new optional DT property is added to provide the
per-channel Rfs values and allow the driver to report a correct IIO
SCALE (mA/step).

While adding DS440x support, a few related issues were addressed:
- Port to regmap
- Reject -128 in RAW writes on DS442x, which cannot be represented with
  sign-magnitude encoding and could silently program an unintended
  output.
- Preserve preconfigured values on probe.
- Ratelimit read error logging and use device context.

David Jander (1):
  iio: dac: ds4424: add DS4402/DS4404 device IDs

Oleksij Rempel (7):
  iio: dac: ds4424: fix -128 rejection and refactor raw access
  iio: dac: ds4424: ratelimit read errors and use device context
  iio: dac: ds4424: sort headers alphabetically
  dt-bindings: iio: dac: maxim,ds4424: add ds4402/ds4404
  dt-bindings: iio: dac: maxim,ds4424: add maxim,rfs-ohms property
  iio: dac: ds4424: convert to regmap
  iio: dac: ds4424: add Rfs-based scale and per-variant limits

 .../bindings/iio/dac/maxim,ds4424.yaml        |  42 ++-
 drivers/iio/dac/Kconfig                       |   1 +
 drivers/iio/dac/ds4424.c                      | 351 ++++++++++++------
 3 files changed, 283 insertions(+), 111 deletions(-)

--
2.47.3

[PATCH v3 1/8] iio: dac: ds4424: fix -128 rejection and refactor raw access

[Oleksij Rempel]

The DS442x DAC uses sign-magnitude encoding, so -128 cannot be represented.
Previously, passing -128 resulted in a truncated value that programmed 0mA.

Fix this by validating the input against the 7-bit magnitude limit.
Additionally, refactor the raw access logic to use symmetrical bitwise
operations, replacing the union structure.

Fixes: d632a2bd8ffc ("iio: dac: ds4422/ds4424 dac driver")
Cc: <stable@vger.kernel.org>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
changes v3:
- Remove "Rebase on top of regmap" note as this is now patch 1/8.
- Add #include <linux/bits.h>
- Clarify 0mA sink/source behavior in comments
- Remove redundant blank line in write_raw
changes v2:
- Replace S8_MIN/MAX checks with abs() > DS4424_DAC_MASK to enforce the
  correct [-127, 127] physical range.
- Refactor read_raw/write_raw to use symmetrical bitwise operations,
  removing the custom bitfield union.
- Rebase on top of regmap port
---
 drivers/iio/dac/ds4424.c | 55 +++++++++++++++-------------------------
 1 file changed, 21 insertions(+), 34 deletions(-)

diff --git a/drivers/iio/dac/ds4424.c b/drivers/iio/dac/ds4424.c
index a8198ba4f98a..596ff5999271 100644
--- a/drivers/iio/dac/ds4424.c
+++ b/drivers/iio/dac/ds4424.c
@@ -5,6 +5,7 @@
  * Copyright (C) 2017 Maxim Integrated
  */
 
+#include <linux/bits.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/i2c.h>
@@ -19,9 +20,10 @@
 #define DS4422_MAX_DAC_CHANNELS		2
 #define DS4424_MAX_DAC_CHANNELS		4
 
+#define DS4424_DAC_MASK			GENMASK(6, 0)
+#define DS4424_DAC_SOURCE		BIT(7)
+
 #define DS4424_DAC_ADDR(chan)   ((chan) + 0xf8)
-#define DS4424_SOURCE_I		1
-#define DS4424_SINK_I		0
 
 #define DS4424_CHANNEL(chan) { \
 	.type = IIO_CURRENT, \
@@ -31,22 +33,6 @@
 	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
 }
 
-/*
- * DS4424 DAC control register 8 bits
- * [7]		0: to sink; 1: to source
- * [6:0]	steps to sink/source
- * bit[7] looks like a sign bit, but the value of the register is
- * not a two's complement code considering the bit[6:0] is a absolute
- * distance from the zero point.
- */
-union ds4424_raw_data {
-	struct {
-		u8 dx:7;
-		u8 source_bit:1;
-	};
-	u8 bits;
-};
-
 enum ds4424_device_ids {
 	ID_DS4422,
 	ID_DS4424,
@@ -108,21 +94,21 @@ static int ds4424_read_raw(struct iio_dev *indio_dev,
 			   struct iio_chan_spec const *chan,
 			   int *val, int *val2, long mask)
 {
-	union ds4424_raw_data raw;
-	int ret;
+	int ret, regval;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		ret = ds4424_get_value(indio_dev, val, chan->channel);
+		ret = ds4424_get_value(indio_dev, &regval, chan->channel);
 		if (ret < 0) {
 			pr_err("%s : ds4424_get_value returned %d\n",
 							__func__, ret);
 			return ret;
 		}
-		raw.bits = *val;
-		*val = raw.dx;
-		if (raw.source_bit == DS4424_SINK_I)
+
+		*val = regval & DS4424_DAC_MASK;
+		if (!(regval & DS4424_DAC_SOURCE))
 			*val = -*val;
+
 		return IIO_VAL_INT;
 
 	default:
@@ -134,25 +120,26 @@ static int ds4424_write_raw(struct iio_dev *indio_dev,
 			     struct iio_chan_spec const *chan,
 			     int val, int val2, long mask)
 {
-	union ds4424_raw_data raw;
+	unsigned int abs_val;
 
 	if (val2 != 0)
 		return -EINVAL;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		if (val < S8_MIN || val > S8_MAX)
+		abs_val = abs(val);
+		if (abs_val > DS4424_DAC_MASK)
 			return -EINVAL;
 
-		if (val > 0) {
-			raw.source_bit = DS4424_SOURCE_I;
-			raw.dx = val;
-		} else {
-			raw.source_bit = DS4424_SINK_I;
-			raw.dx = -val;
-		}
+		/*
+		 * Currents exiting the IC (Source) are positive. 0 is a valid
+		 * value for no current flow; the direction bit (Source vs Sink)
+		 * is treated as don't-care by the hardware at 0.
+		 */
+		if (val > 0)
+			abs_val |= DS4424_DAC_SOURCE;
 
-		return ds4424_set_value(indio_dev, raw.bits, chan);
+		return ds4424_set_value(indio_dev, abs_val, chan);
 
 	default:
 		return -EINVAL;
-- 
2.47.3

Re: [PATCH v3 1/8] iio: dac: ds4424: fix -128 rejection and refactor raw access

[Jonathan Cameron]

On Wed, 28 Jan 2026 16:38:17 +0100
Oleksij Rempel <o.rempel@pengutronix.de> wrote:

> The DS442x DAC uses sign-magnitude encoding, so -128 cannot be represented.
> Previously, passing -128 resulted in a truncated value that programmed 0mA.
> 
> Fix this by validating the input against the 7-bit magnitude limit.
> Additionally, refactor the raw access logic to use symmetrical bitwise
> operations, replacing the union structure.
> 
> Fixes: d632a2bd8ffc ("iio: dac: ds4422/ds4424 dac driver")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Hi Olkesij

This is good stuff but, this fails the test of being the minimal fix
suited for a trivial backport.

The right solution here is split it.  Just apply the correct
limit in the fix patch, then the refactors in a patch on top of
that which most likely won't be backported for stable.

Thanks,

Jonathan


> ---
> changes v3:
> - Remove "Rebase on top of regmap" note as this is now patch 1/8.
> - Add #include <linux/bits.h>
> - Clarify 0mA sink/source behavior in comments
> - Remove redundant blank line in write_raw
> changes v2:
> - Replace S8_MIN/MAX checks with abs() > DS4424_DAC_MASK to enforce the
>   correct [-127, 127] physical range.
> - Refactor read_raw/write_raw to use symmetrical bitwise operations,
>   removing the custom bitfield union.
> - Rebase on top of regmap port
> ---
>  drivers/iio/dac/ds4424.c | 55 +++++++++++++++-------------------------
>  1 file changed, 21 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/iio/dac/ds4424.c b/drivers/iio/dac/ds4424.c
> index a8198ba4f98a..596ff5999271 100644
> --- a/drivers/iio/dac/ds4424.c
> +++ b/drivers/iio/dac/ds4424.c
> @@ -5,6 +5,7 @@
>   * Copyright (C) 2017 Maxim Integrated
>   */
>  
> +#include <linux/bits.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/i2c.h>
> @@ -19,9 +20,10 @@
>  #define DS4422_MAX_DAC_CHANNELS		2
>  #define DS4424_MAX_DAC_CHANNELS		4
>  
> +#define DS4424_DAC_MASK			GENMASK(6, 0)
> +#define DS4424_DAC_SOURCE		BIT(7)
> +
>  #define DS4424_DAC_ADDR(chan)   ((chan) + 0xf8)
> -#define DS4424_SOURCE_I		1
> -#define DS4424_SINK_I		0
>  
>  #define DS4424_CHANNEL(chan) { \
>  	.type = IIO_CURRENT, \
> @@ -31,22 +33,6 @@
>  	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
>  }
>  
> -/*
> - * DS4424 DAC control register 8 bits
> - * [7]		0: to sink; 1: to source
> - * [6:0]	steps to sink/source
> - * bit[7] looks like a sign bit, but the value of the register is
> - * not a two's complement code considering the bit[6:0] is a absolute
> - * distance from the zero point.
> - */
> -union ds4424_raw_data {
> -	struct {
> -		u8 dx:7;
> -		u8 source_bit:1;
> -	};
> -	u8 bits;
> -};
> -
>  enum ds4424_device_ids {
>  	ID_DS4422,
>  	ID_DS4424,
> @@ -108,21 +94,21 @@ static int ds4424_read_raw(struct iio_dev *indio_dev,
>  			   struct iio_chan_spec const *chan,
>  			   int *val, int *val2, long mask)
>  {
> -	union ds4424_raw_data raw;
> -	int ret;
> +	int ret, regval;
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_RAW:
> -		ret = ds4424_get_value(indio_dev, val, chan->channel);
> +		ret = ds4424_get_value(indio_dev, &regval, chan->channel);
>  		if (ret < 0) {
>  			pr_err("%s : ds4424_get_value returned %d\n",
>  							__func__, ret);
>  			return ret;
>  		}
> -		raw.bits = *val;
> -		*val = raw.dx;
> -		if (raw.source_bit == DS4424_SINK_I)
> +
> +		*val = regval & DS4424_DAC_MASK;
> +		if (!(regval & DS4424_DAC_SOURCE))
>  			*val = -*val;
> +
>  		return IIO_VAL_INT;
>  
>  	default:
> @@ -134,25 +120,26 @@ static int ds4424_write_raw(struct iio_dev *indio_dev,
>  			     struct iio_chan_spec const *chan,
>  			     int val, int val2, long mask)
>  {
> -	union ds4424_raw_data raw;
> +	unsigned int abs_val;
>  
>  	if (val2 != 0)
>  		return -EINVAL;
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_RAW:
> -		if (val < S8_MIN || val > S8_MAX)
> +		abs_val = abs(val);
> +		if (abs_val > DS4424_DAC_MASK)
>  			return -EINVAL;

Just this bit belongs in fix patch.


>  
> -		if (val > 0) {
> -			raw.source_bit = DS4424_SOURCE_I;
> -			raw.dx = val;
> -		} else {
> -			raw.source_bit = DS4424_SINK_I;
> -			raw.dx = -val;
> -		}
> +		/*
> +		 * Currents exiting the IC (Source) are positive. 0 is a valid
> +		 * value for no current flow; the direction bit (Source vs Sink)
> +		 * is treated as don't-care by the hardware at 0.
> +		 */
> +		if (val > 0)
> +			abs_val |= DS4424_DAC_SOURCE;
>  
> -		return ds4424_set_value(indio_dev, raw.bits, chan);
> +		return ds4424_set_value(indio_dev, abs_val, chan);
>  
>  	default:
>  		return -EINVAL;

Re: [PATCH v3 1/8] iio: dac: ds4424: fix -128 rejection and refactor raw access

[Oleksij Rempel]

Hi Jonathan,

On Thu, Jan 29, 2026 at 05:58:19PM +0000, Jonathan Cameron wrote:
> On Wed, 28 Jan 2026 16:38:17 +0100
> Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> 
> > The DS442x DAC uses sign-magnitude encoding, so -128 cannot be represented.
> > Previously, passing -128 resulted in a truncated value that programmed 0mA.
> > 
> > Fix this by validating the input against the 7-bit magnitude limit.
> > Additionally, refactor the raw access logic to use symmetrical bitwise
> > operations, replacing the union structure.
> > 
> > Fixes: d632a2bd8ffc ("iio: dac: ds4422/ds4424 dac driver")
> > Cc: <stable@vger.kernel.org>
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> Hi Olkesij
> 
> This is good stuff but, this fails the test of being the minimal fix
> suited for a trivial backport.
> 
> The right solution here is split it.  Just apply the correct
> limit in the fix patch, then the refactors in a patch on top of
> that which most likely won't be backported for stable.

The v1 of this patch was implemented according to the fix patch
requirements:
https://lore.kernel.org/all/20260119182424.1660601-5-o.rempel@pengutronix.de/

May be keep v1 as is and rebase v3 as refactoring stage on top of it?

Best Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH v3 1/8] iio: dac: ds4424: fix -128 rejection and refactor raw access

[Jonathan Cameron]

On Sun, 1 Feb 2026 13:40:42 +0100
Oleksij Rempel <o.rempel@pengutronix.de> wrote:

> Hi Jonathan,
> 
> On Thu, Jan 29, 2026 at 05:58:19PM +0000, Jonathan Cameron wrote:
> > On Wed, 28 Jan 2026 16:38:17 +0100
> > Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> >   
> > > The DS442x DAC uses sign-magnitude encoding, so -128 cannot be represented.
> > > Previously, passing -128 resulted in a truncated value that programmed 0mA.
> > > 
> > > Fix this by validating the input against the 7-bit magnitude limit.
> > > Additionally, refactor the raw access logic to use symmetrical bitwise
> > > operations, replacing the union structure.
> > > 
> > > Fixes: d632a2bd8ffc ("iio: dac: ds4422/ds4424 dac driver")
> > > Cc: <stable@vger.kernel.org>
> > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>  
> > Hi Olkesij
> > 
> > This is good stuff but, this fails the test of being the minimal fix
> > suited for a trivial backport.
> > 
> > The right solution here is split it.  Just apply the correct
> > limit in the fix patch, then the refactors in a patch on top of
> > that which most likely won't be backported for stable.  
> 
> The v1 of this patch was implemented according to the fix patch
> requirements:
> https://lore.kernel.org/all/20260119182424.1660601-5-o.rempel@pengutronix.de/
> 
> May be keep v1 as is and rebase v3 as refactoring stage on top of it?
Perfect.

Thanks!

Jonathan

> 
> Best Regards,
> Oleksij

[PATCH v3 2/8] iio: dac: ds4424: ratelimit read errors and use device context

[Oleksij Rempel]

Replace pr_err() with dev_err_ratelimited() in the RAW read path to avoid
log spam on repeated I2C failures and to include the device context.

Use %pe to print errno names for faster debugging. Use the parent
device context to identify the physical hardware causing the error.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
changes v3:
- Use indio_dev->dev.parent to reference the physical device.
- Remove redundant space in error message: "channel %d: %pe".
- This patch now precedes regmap refactoring.
changes v2:
- Update error message
- Rebase against regmap refactoring
---
 drivers/iio/dac/ds4424.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/dac/ds4424.c b/drivers/iio/dac/ds4424.c
index 596ff5999271..36286e4923af 100644
--- a/drivers/iio/dac/ds4424.c
+++ b/drivers/iio/dac/ds4424.c
@@ -100,8 +100,9 @@ static int ds4424_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_RAW:
 		ret = ds4424_get_value(indio_dev, &regval, chan->channel);
 		if (ret < 0) {
-			pr_err("%s : ds4424_get_value returned %d\n",
-							__func__, ret);
+			dev_err_ratelimited(indio_dev->dev.parent,
+					    "Failed to read channel %d: %pe\n",
+					    chan->channel, ERR_PTR(ret));
 			return ret;
 		}
 
-- 
2.47.3

[PATCH v3 3/8] iio: dac: ds4424: sort headers alphabetically

[Oleksij Rempel]

Sort the header inclusions alphabetically. This improves readability and
simplifies adding new includes in the future.

Group subsystem-specific headers (linux/iio/*) separately at the end
to clarify subsystem context.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
changes v3:
- Keep linux/iio/* headers in a separate group at the end of the includes.
changes v2:
- new patch
---
 drivers/iio/dac/ds4424.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/dac/ds4424.c b/drivers/iio/dac/ds4424.c
index 36286e4923af..c03051dc763e 100644
--- a/drivers/iio/dac/ds4424.c
+++ b/drivers/iio/dac/ds4424.c
@@ -6,16 +6,17 @@
  */
 
 #include <linux/bits.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
-#include <linux/i2c.h>
 #include <linux/regulator/consumer.h>
-#include <linux/err.h>
-#include <linux/delay.h>
-#include <linux/iio/iio.h>
+
+#include <linux/iio/consumer.h>
 #include <linux/iio/driver.h>
+#include <linux/iio/iio.h>
 #include <linux/iio/machine.h>
-#include <linux/iio/consumer.h>
 
 #define DS4422_MAX_DAC_CHANNELS		2
 #define DS4424_MAX_DAC_CHANNELS		4
-- 
2.47.3

[PATCH v3 4/8] dt-bindings: iio: dac: maxim,ds4424: add ds4402/ds4404

[Oleksij Rempel]

Add compatible strings for Maxim DS4402 and DS4404 current DACs.
These devices are 5-bit variants of the DS4422/DS4424 family.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
changes v3:
- No changes.
changes v2:
  - add Acked-by: Conor ..
---
 .../devicetree/bindings/iio/dac/maxim,ds4424.yaml          | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml b/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml
index 264fa7c5fe3a..efe63e6cb55d 100644
--- a/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml
+++ b/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml
@@ -4,18 +4,21 @@
 $id: http://devicetree.org/schemas/iio/dac/maxim,ds4424.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Maxim Integrated DS4422/DS4424 7-bit Sink/Source Current DAC
+title: Maxim Integrated DS4402/DS4404 and DS4422/DS4424 Current DACs
 
 maintainers:
   - Ismail Kose <ihkose@gmail.com>
 
 description: |
-  Datasheet publicly available at:
+  Datasheets publicly available at:
+  https://datasheets.maximintegrated.com/en/ds/DS4402-DS4404.pdf
   https://datasheets.maximintegrated.com/en/ds/DS4422-DS4424.pdf
 
 properties:
   compatible:
     enum:
+      - maxim,ds4402
+      - maxim,ds4404
       - maxim,ds4422
       - maxim,ds4424
 
-- 
2.47.3

[PATCH v3 5/8] dt-bindings: iio: dac: maxim,ds4424: add maxim,rfs-ohms property

[Oleksij Rempel]

The Maxim DS4422/DS4424 and DS4402/DS4404 current DACs determine their
full-scale output current via external resistors (Rfs) connected to the
FSx pins. Without knowing these values, the full-scale range of the
hardware is undefined.

Add the 'maxim,rfs-ohms' property to describe these physical components.
This property is required to provide a complete description of the
hardware configuration.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
changes v3:
- Moved minItems and maxItems to the main property definition
- Refined property description to state that values depend on chip
  variant and hardware requirements, deliberately avoiding specific
  "typical" ranges to prevent unnecessary hard-limit enforcement.
- Corrected the rfs-ohms example to use a plausible value within
  datasheet typical range.
- Removed redundant constraint definitions from the allOf logic.
changes v2:
- make maxim,rfs-ohms a required property as the hardware range is undefined
  without external resistors.
- add allOf constraints to enforce 2 vs 4 items in maxim,rfs-ohms based on
  compatible string.
- drop explicit $ref for maxim,rfs-ohms to fix dt_binding_check warning.
- update example in binding to include the new required property.
---
 .../bindings/iio/dac/maxim,ds4424.yaml        | 35 +++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml b/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml
index efe63e6cb55d..4323df2036ac 100644
--- a/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml
+++ b/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml
@@ -27,9 +27,43 @@ properties:
 
   vcc-supply: true
 
+  maxim,rfs-ohms:
+    description: |
+      Array of resistance values in Ohms for the external Rfs resistors
+      connected to the FS pins. These values determine the full-scale
+      output current. The actual resistance depends on the chip variant
+      and specific hardware design requirements.
+    minItems: 2
+    maxItems: 4
+
 required:
   - compatible
   - reg
+  - maxim,rfs-ohms
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - maxim,ds4402
+              - maxim,ds4422
+    then:
+      properties:
+        maxim,rfs-ohms:
+          maxItems: 2
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - maxim,ds4404
+              - maxim,ds4424
+    then:
+      properties:
+        maxim,rfs-ohms:
+          minItems: 4
 
 additionalProperties: false
 
@@ -43,6 +77,7 @@ examples:
             compatible = "maxim,ds4424";
             reg = <0x10>; /* When A0, A1 pins are ground */
             vcc-supply = <&vcc_3v3>;
+            maxim,rfs-ohms = <40000>, <40000>, <40000>, <40000>;
         };
     };
 ...
-- 
2.47.3

Re: [PATCH v3 5/8] dt-bindings: iio: dac: maxim,ds4424: add maxim,rfs-ohms property

[Conor Dooley]

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

On Wed, Jan 28, 2026 at 04:38:21PM +0100, Oleksij Rempel wrote:
> The Maxim DS4422/DS4424 and DS4402/DS4404 current DACs determine their
> full-scale output current via external resistors (Rfs) connected to the
> FSx pins. Without knowing these values, the full-scale range of the
> hardware is undefined.
> 
> Add the 'maxim,rfs-ohms' property to describe these physical components.
> This property is required to provide a complete description of the
> hardware configuration.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>

Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable

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

[PATCH v3 6/8] iio: dac: ds4424: add DS4402/DS4404 device IDs

[Oleksij Rempel]

From: David Jander <david@protonic.nl>

Add I2C/OF IDs for DS4402 and DS4404 and set the correct channel count.
Follow-up changes add per-variant scaling based on external Rfs.

Co-developed-by: Oleksij Rempel <o.rempel@pengutronix.de>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Signed-off-by: David Jander <david@protonic.nl>
---
changes v3:
- Reset author to David Jander and added Co-developed-by tag for
  Oleksij Rempel to clarify roles
changes v2:
  - No changes.
---
 drivers/iio/dac/ds4424.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/iio/dac/ds4424.c b/drivers/iio/dac/ds4424.c
index c03051dc763e..f340d491fcc1 100644
--- a/drivers/iio/dac/ds4424.c
+++ b/drivers/iio/dac/ds4424.c
@@ -35,6 +35,8 @@
 }
 
 enum ds4424_device_ids {
+	ID_DS4402,
+	ID_DS4404,
 	ID_DS4422,
 	ID_DS4424,
 };
@@ -237,6 +239,12 @@ static int ds4424_probe(struct i2c_client *client)
 		goto fail;
 
 	switch (id->driver_data) {
+	case ID_DS4402:
+		indio_dev->num_channels = DS4422_MAX_DAC_CHANNELS;
+		break;
+	case ID_DS4404:
+		indio_dev->num_channels = DS4424_MAX_DAC_CHANNELS;
+		break;
 	case ID_DS4422:
 		indio_dev->num_channels = DS4422_MAX_DAC_CHANNELS;
 		break;
@@ -278,6 +286,8 @@ static void ds4424_remove(struct i2c_client *client)
 }
 
 static const struct i2c_device_id ds4424_id[] = {
+	{ "ds4402", ID_DS4402 },
+	{ "ds4404", ID_DS4404 },
 	{ "ds4422", ID_DS4422 },
 	{ "ds4424", ID_DS4424 },
 	{ }
@@ -286,6 +296,8 @@ static const struct i2c_device_id ds4424_id[] = {
 MODULE_DEVICE_TABLE(i2c, ds4424_id);
 
 static const struct of_device_id ds4424_of_match[] = {
+	{ .compatible = "maxim,ds4402" },
+	{ .compatible = "maxim,ds4404" },
 	{ .compatible = "maxim,ds4422" },
 	{ .compatible = "maxim,ds4424" },
 	{ }
-- 
2.47.3

Re: [PATCH v3 6/8] iio: dac: ds4424: add DS4402/DS4404 device IDs

[David Lechner]

On 1/28/26 9:38 AM, Oleksij Rempel wrote:
> From: David Jander <david@protonic.nl>
> 
> Add I2C/OF IDs for DS4402 and DS4404 and set the correct channel count.
> Follow-up changes add per-variant scaling based on external Rfs.
> 
> Co-developed-by: Oleksij Rempel <o.rempel@pengutronix.de>
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> Signed-off-by: David Jander <david@protonic.nl>
> ---
> changes v3:
> - Reset author to David Jander and added Co-developed-by tag for
>   Oleksij Rempel to clarify roles
> changes v2:
>   - No changes.
> ---
>  drivers/iio/dac/ds4424.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/iio/dac/ds4424.c b/drivers/iio/dac/ds4424.c
> index c03051dc763e..f340d491fcc1 100644
> --- a/drivers/iio/dac/ds4424.c
> +++ b/drivers/iio/dac/ds4424.c
> @@ -35,6 +35,8 @@
>  }
>  
>  enum ds4424_device_ids {
> +	ID_DS4402,
> +	ID_DS4404,
>  	ID_DS4422,
>  	ID_DS4424,
>  };

I suppose it was already suggested in previous reviews that we should
be dropping these IDs and using device info instead of expanding the
switch statement below.

> @@ -237,6 +239,12 @@ static int ds4424_probe(struct i2c_client *client)
>  		goto fail;
>  
>  	switch (id->driver_data) {
> +	case ID_DS4402:
> +		indio_dev->num_channels = DS4422_MAX_DAC_CHANNELS;
> +		break;
> +	case ID_DS4404:
> +		indio_dev->num_channels = DS4424_MAX_DAC_CHANNELS;
> +		break;
>  	case ID_DS4422:
>  		indio_dev->num_channels = DS4422_MAX_DAC_CHANNELS;
>  		break;
> @@ -278,6 +286,8 @@ static void ds4424_remove(struct i2c_client *client)
>  }
>  
>  static const struct i2c_device_id ds4424_id[] = {
> +	{ "ds4402", ID_DS4402 },
> +	{ "ds4404", ID_DS4404 },
>  	{ "ds4422", ID_DS4422 },
>  	{ "ds4424", ID_DS4424 },
>  	{ }
> @@ -286,6 +296,8 @@ static const struct i2c_device_id ds4424_id[] = {
>  MODULE_DEVICE_TABLE(i2c, ds4424_id);
>  
>  static const struct of_device_id ds4424_of_match[] = {
> +	{ .compatible = "maxim,ds4402" },
> +	{ .compatible = "maxim,ds4404" },
>  	{ .compatible = "maxim,ds4422" },
>  	{ .compatible = "maxim,ds4424" },
>  	{ }

[PATCH v3 7/8] iio: dac: ds4424: convert to regmap

[Oleksij Rempel]

Refactor the driver to use the regmap API.

Replace the driver-specific mutex and manual shadow buffers with the
standard regmap infrastructure for locking and caching.

This ensures the cache is populated from hardware at probe, preventing
state desynchronization (e.g. across suspend/resume).

Define access tables to validate the different register maps of DS44x2
and DS44x4.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
changes v3:
- Switch to REGCACHE_MAPLE to efficiently handle the sparse register map
  (offset 0xF8) and avoid allocating memory for the unused 0x00-0xF7 range.
- Use explicit regmap_bulk_read() in probe to seed the cache with the
  bootloader configuration. This avoids the invalid read from address 0x00
  that occurred with generic cache defaults.
- Remove ds4424_verify_chip(); devm_regmap_init_i2c() and the subsequent
  bulk read implicitly validate the device presence.
- Use regmap_bulk_write() in ds4424_suspend() to efficiently zero all
  channels.
- Adopt fsleep() for delays and include <linux/array_size.h>.
- Use dev_err_ratelimited() with the physical device context in the read
  path (incorporating feedback aimed at v2 patch 8).
changes v2:
- new patch
---
 drivers/iio/dac/Kconfig  |   1 +
 drivers/iio/dac/ds4424.c | 171 ++++++++++++++++++++++-----------------
 2 files changed, 97 insertions(+), 75 deletions(-)

diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index 7cd3caec1262..dbbbc45e8718 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -408,6 +408,7 @@ config DPOT_DAC
 config DS4424
 	tristate "Maxim Integrated DS4422/DS4424 DAC driver"
 	depends on I2C
+	select REGMAP_I2C
 	help
 	  If you say yes here you get support for Maxim chips DS4422, DS4424.
 
diff --git a/drivers/iio/dac/ds4424.c b/drivers/iio/dac/ds4424.c
index f340d491fcc1..9bef1c60b2eb 100644
--- a/drivers/iio/dac/ds4424.c
+++ b/drivers/iio/dac/ds4424.c
@@ -5,12 +5,14 @@
  * Copyright (C) 2017 Maxim Integrated
  */
 
+#include <linux/array_size.h>
 #include <linux/bits.h>
 #include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/i2c.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 
 #include <linux/iio/consumer.h>
@@ -42,11 +44,8 @@ enum ds4424_device_ids {
 };
 
 struct ds4424_data {
-	struct i2c_client *client;
-	struct mutex lock;
-	uint8_t save[DS4424_MAX_DAC_CHANNELS];
+	struct regmap *regmap;
 	struct regulator *vcc_reg;
-	uint8_t raw[DS4424_MAX_DAC_CHANNELS];
 };
 
 static const struct iio_chan_spec ds4424_channels[] = {
@@ -56,52 +55,88 @@ static const struct iio_chan_spec ds4424_channels[] = {
 	DS4424_CHANNEL(3),
 };
 
-static int ds4424_get_value(struct iio_dev *indio_dev,
-			     int *val, int channel)
-{
-	struct ds4424_data *data = iio_priv(indio_dev);
-	int ret;
+static const struct regmap_range ds44x2_ranges[] = {
+	regmap_reg_range(DS4424_DAC_ADDR(0), DS4424_DAC_ADDR(1)),
+};
 
-	mutex_lock(&data->lock);
-	ret = i2c_smbus_read_byte_data(data->client, DS4424_DAC_ADDR(channel));
-	if (ret < 0)
-		goto fail;
+static const struct regmap_range ds44x4_ranges[] = {
+	regmap_reg_range(DS4424_DAC_ADDR(0), DS4424_DAC_ADDR(3)),
+};
 
-	*val = ret;
+static const struct regmap_access_table ds44x2_table = {
+	.yes_ranges = ds44x2_ranges,
+	.n_yes_ranges = ARRAY_SIZE(ds44x2_ranges),
+};
 
-fail:
-	mutex_unlock(&data->lock);
-	return ret;
-}
+static const struct regmap_access_table ds44x4_table = {
+	.yes_ranges = ds44x4_ranges,
+	.n_yes_ranges = ARRAY_SIZE(ds44x4_ranges),
+};
 
-static int ds4424_set_value(struct iio_dev *indio_dev,
-			     int val, struct iio_chan_spec const *chan)
+static const struct regmap_config ds44x2_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.cache_type = REGCACHE_MAPLE,
+	.max_register = DS4424_DAC_ADDR(1),
+	.rd_table = &ds44x2_table,
+	.wr_table = &ds44x2_table,
+	/* Seed cache from HW during regmap_init */
+};
+
+static const struct regmap_config ds44x4_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.cache_type = REGCACHE_MAPLE,
+	.max_register = DS4424_DAC_ADDR(3),
+	.rd_table = &ds44x4_table,
+	.wr_table = &ds44x4_table,
+	/* Seed cache from HW during regmap_init */
+};
+
+static int ds4424_init_regmap(struct i2c_client *client,
+			      struct iio_dev *indio_dev)
 {
 	struct ds4424_data *data = iio_priv(indio_dev);
+	const struct regmap_config *regmap_config;
+	u8 vals[DS4424_MAX_DAC_CHANNELS];
 	int ret;
 
-	mutex_lock(&data->lock);
-	ret = i2c_smbus_write_byte_data(data->client,
-			DS4424_DAC_ADDR(chan->channel), val);
-	if (ret < 0)
-		goto fail;
-
-	data->raw[chan->channel] = val;
-
-fail:
-	mutex_unlock(&data->lock);
-	return ret;
+	if (indio_dev->num_channels == DS4424_MAX_DAC_CHANNELS)
+		regmap_config = &ds44x4_regmap_config;
+	else
+		regmap_config = &ds44x2_regmap_config;
+
+	data->regmap = devm_regmap_init_i2c(client, regmap_config);
+	if (IS_ERR(data->regmap))
+		return dev_err_probe(&client->dev, PTR_ERR(data->regmap),
+				     "Failed to init regmap.\n");
+
+	/*
+	 * Prime the cache with the bootloader's configuration.
+	 * regmap_bulk_read will automatically populate the cache with
+	 * the values read from the hardware.
+	 */
+	ret = regmap_bulk_read(data->regmap, DS4424_DAC_ADDR(0), vals,
+			       indio_dev->num_channels);
+	if (ret)
+		return dev_err_probe(&client->dev, ret,
+				     "Failed to read hardware defaults\n");
+
+	return 0;
 }
 
 static int ds4424_read_raw(struct iio_dev *indio_dev,
 			   struct iio_chan_spec const *chan,
 			   int *val, int *val2, long mask)
 {
-	int ret, regval;
+	struct ds4424_data *data = iio_priv(indio_dev);
+	unsigned int regval;
+	int ret;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		ret = ds4424_get_value(indio_dev, &regval, chan->channel);
+		ret = regmap_read(data->regmap, DS4424_DAC_ADDR(chan->channel),
+				  &regval);
 		if (ret < 0) {
 			dev_err_ratelimited(indio_dev->dev.parent,
 					    "Failed to read channel %d: %pe\n",
@@ -124,6 +159,7 @@ static int ds4424_write_raw(struct iio_dev *indio_dev,
 			     struct iio_chan_spec const *chan,
 			     int val, int val2, long mask)
 {
+	struct ds4424_data *data = iio_priv(indio_dev);
 	unsigned int abs_val;
 
 	if (val2 != 0)
@@ -143,58 +179,44 @@ static int ds4424_write_raw(struct iio_dev *indio_dev,
 		if (val > 0)
 			abs_val |= DS4424_DAC_SOURCE;
 
-		return ds4424_set_value(indio_dev, abs_val, chan);
+		return regmap_write(data->regmap, DS4424_DAC_ADDR(chan->channel),
+				    abs_val);
 
 	default:
 		return -EINVAL;
 	}
 }
 
-static int ds4424_verify_chip(struct iio_dev *indio_dev)
-{
-	int ret, val;
-
-	ret = ds4424_get_value(indio_dev, &val, 0);
-	if (ret < 0)
-		dev_err(&indio_dev->dev,
-				"%s failed. ret: %d\n", __func__, ret);
-
-	return ret;
-}
-
 static int ds4424_suspend(struct device *dev)
 {
-	struct i2c_client *client = to_i2c_client(dev);
-	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ds4424_data *data = iio_priv(indio_dev);
-	int ret = 0;
-	int i;
-
-	for (i = 0; i < indio_dev->num_channels; i++) {
-		data->save[i] = data->raw[i];
-		ret = ds4424_set_value(indio_dev, 0,
-				&indio_dev->channels[i]);
-		if (ret < 0)
-			return ret;
+	u8 zero_buf[DS4424_MAX_DAC_CHANNELS] = { 0 };
+	int ret;
+
+	/* Disable all outputs, bypass cache so the '0' isn't saved */
+	regcache_cache_bypass(data->regmap, true);
+	ret = regmap_bulk_write(data->regmap, DS4424_DAC_ADDR(0),
+				zero_buf, indio_dev->num_channels);
+	regcache_cache_bypass(data->regmap, false);
+	if (ret) {
+		dev_err(dev, "Failed to zero outputs: %pe\n", ERR_PTR(ret));
+		return ret;
 	}
-	return ret;
+
+	regcache_cache_only(data->regmap, true);
+	regcache_mark_dirty(data->regmap);
+
+	return 0;
 }
 
 static int ds4424_resume(struct device *dev)
 {
-	struct i2c_client *client = to_i2c_client(dev);
-	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ds4424_data *data = iio_priv(indio_dev);
-	int ret = 0;
-	int i;
 
-	for (i = 0; i < indio_dev->num_channels; i++) {
-		ret = ds4424_set_value(indio_dev, data->save[i],
-				&indio_dev->channels[i]);
-		if (ret < 0)
-			return ret;
-	}
-	return ret;
+	regcache_cache_only(data->regmap, false);
+	return regcache_sync(data->regmap);
 }
 
 static DEFINE_SIMPLE_DEV_PM_OPS(ds4424_pm_ops, ds4424_suspend, ds4424_resume);
@@ -217,7 +239,6 @@ static int ds4424_probe(struct i2c_client *client)
 
 	data = iio_priv(indio_dev);
 	i2c_set_clientdata(client, indio_dev);
-	data->client = client;
 	indio_dev->name = id->name;
 
 	data->vcc_reg = devm_regulator_get(&client->dev, "vcc");
@@ -225,7 +246,6 @@ static int ds4424_probe(struct i2c_client *client)
 		return dev_err_probe(&client->dev, PTR_ERR(data->vcc_reg),
 				     "Failed to get vcc-supply regulator.\n");
 
-	mutex_init(&data->lock);
 	ret = regulator_enable(data->vcc_reg);
 	if (ret < 0) {
 		dev_err(&client->dev,
@@ -233,10 +253,7 @@ static int ds4424_probe(struct i2c_client *client)
 		return ret;
 	}
 
-	usleep_range(1000, 1200);
-	ret = ds4424_verify_chip(indio_dev);
-	if (ret < 0)
-		goto fail;
+	fsleep(1000);
 
 	switch (id->driver_data) {
 	case ID_DS4402:
@@ -258,6 +275,10 @@ static int ds4424_probe(struct i2c_client *client)
 		goto fail;
 	}
 
+	ret = ds4424_init_regmap(client, indio_dev);
+	if (ret)
+		goto fail;
+
 	indio_dev->channels = ds4424_channels;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->info = &ds4424_info;
-- 
2.47.3

Re: [PATCH v3 7/8] iio: dac: ds4424: convert to regmap

[Andy Shevchenko]

On Wed, Jan 28, 2026 at 04:38:23PM +0100, Oleksij Rempel wrote:
> Refactor the driver to use the regmap API.
> 
> Replace the driver-specific mutex and manual shadow buffers with the
> standard regmap infrastructure for locking and caching.
> 
> This ensures the cache is populated from hardware at probe, preventing
> state desynchronization (e.g. across suspend/resume).
> 
> Define access tables to validate the different register maps of DS44x2
> and DS44x4.

...

> changes v3:
> - Switch to REGCACHE_MAPLE to efficiently handle the sparse register map
>   (offset 0xF8) and avoid allocating memory for the unused 0x00-0xF7 range.

> - Use explicit regmap_bulk_read() in probe to seed the cache with the
>   bootloader configuration. This avoids the invalid read from address 0x00
>   that occurred with generic cache defaults.

Isn't regmap has an option to do it for you?
(I'm talking about num_reg_defaults_raw without setting reg_defaults_raw)

> - Remove ds4424_verify_chip(); devm_regmap_init_i2c() and the subsequent
>   bulk read implicitly validate the device presence.
> - Use regmap_bulk_write() in ds4424_suspend() to efficiently zero all
>   channels.
> - Adopt fsleep() for delays and include <linux/array_size.h>.
> - Use dev_err_ratelimited() with the physical device context in the read
>   path (incorporating feedback aimed at v2 patch 8).

...

> -	usleep_range(1000, 1200);
> +	fsleep(1000);

Seems like undescribed / unrelated change.
Also needs a comment to explain the delay.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 7/8] iio: dac: ds4424: convert to regmap

[Jonathan Cameron]

On Wed, 28 Jan 2026 16:38:23 +0100
Oleksij Rempel <o.rempel@pengutronix.de> wrote:

> Refactor the driver to use the regmap API.
> 
> Replace the driver-specific mutex and manual shadow buffers with the
> standard regmap infrastructure for locking and caching.
> 
> This ensures the cache is populated from hardware at probe, preventing
> state desynchronization (e.g. across suspend/resume).
> 
> Define access tables to validate the different register maps of DS44x2
> and DS44x4.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Hi Oleksij

A few comments that might not overlap with what Andy already provided.

> diff --git a/drivers/iio/dac/ds4424.c b/drivers/iio/dac/ds4424.c
> index f340d491fcc1..9bef1c60b2eb 100644
> --- a/drivers/iio/dac/ds4424.c
> +++ b/drivers/iio/dac/ds4424.c


> +static int ds4424_init_regmap(struct i2c_client *client,
> +			      struct iio_dev *indio_dev)
>  {
>  	struct ds4424_data *data = iio_priv(indio_dev);
> +	const struct regmap_config *regmap_config;
> +	u8 vals[DS4424_MAX_DAC_CHANNELS];
>  	int ret;
>  
> -	mutex_lock(&data->lock);
> -	ret = i2c_smbus_write_byte_data(data->client,
> -			DS4424_DAC_ADDR(chan->channel), val);
> -	if (ret < 0)
> -		goto fail;
> -
> -	data->raw[chan->channel] = val;
> -
> -fail:
> -	mutex_unlock(&data->lock);
> -	return ret;
> +	if (indio_dev->num_channels == DS4424_MAX_DAC_CHANNELS)
> +		regmap_config = &ds44x4_regmap_config;
> +	else
> +		regmap_config = &ds44x2_regmap_config;
> +
> +	data->regmap = devm_regmap_init_i2c(client, regmap_config);
> +	if (IS_ERR(data->regmap))
> +		return dev_err_probe(&client->dev, PTR_ERR(data->regmap),
> +				     "Failed to init regmap.\n");
> +
> +	/*
> +	 * Prime the cache with the bootloader's configuration.
> +	 * regmap_bulk_read will automatically populate the cache with

regmap_bulk_read() 
style preferred for functions mentioned in comments.

> +	 * the values read from the hardware.
> +	 */
> +	ret = regmap_bulk_read(data->regmap, DS4424_DAC_ADDR(0), vals,

> @@ -233,10 +253,7 @@ static int ds4424_probe(struct i2c_client *client)
>  		return ret;
>  	}
>  
> -	usleep_range(1000, 1200);
> -	ret = ds4424_verify_chip(indio_dev);
> -	if (ret < 0)
> -		goto fail;
> +	fsleep(1000);

This change to fsleep() is unrelated to regmap stuff. So separate patch.

Thanks,

Jonathan

Re: [PATCH v3 7/8] iio: dac: ds4424: convert to regmap

[Sander Vanheule]

Hi Oleksij,

On Wed, 2026-01-28 at 16:38 +0100, Oleksij Rempel wrote:
> +static const struct regmap_config ds44x2_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.cache_type = REGCACHE_MAPLE,
> +	.max_register = DS4424_DAC_ADDR(1),
> +	.rd_table = &ds44x2_table,
> +	.wr_table = &ds44x2_table,
> +	/* Seed cache from HW during regmap_init */

Nit: You're seeding the cache (manually) in ds4424_init_regmap(). But you can
also just drop this comment as far as I'm concerned. The comment in
ds4424_init_regmap() explains it sufficiently.



> +	if (ret)
> +		return dev_err_probe(&client->dev, ret,
> +				     "Failed to read hardware defaults\n");

Nit: "hardware defaults" -> "hardware values"


Nothing too serious from my side, so FWIW, with these things addressed:

Reviewed-by: Sander Vanheule <sander@svanheule.net>

Best,
Sander

[PATCH v3 8/8] iio: dac: ds4424: add Rfs-based scale and per-variant limits

[Oleksij Rempel]

Parse optional maxim,rfs-ohms values to derive the per-channel output
current scale (mA per step) for the IIO current ABI.

Select per-variant parameters to match the shared register map while
handling different data widths and full-scale current calculations.

Behavior changes:
- If maxim,rfs-ohms is present, IIO_CHAN_INFO_SCALE becomes available
  and reports mA/step derived from Rfs.
- If maxim,rfs-ohms is missing, SCALE is not exposed to keep older DTs
  working without requiring updates.
- RAW writes are now limited to the representable sign-magnitude range
  of the detected variant to avoid silent truncation (e.g. +/-31 on
  DS440x).

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
changes v3:
- Added explicit check for negative return from device_property_count_u32().
- Rename vref_mv to vref_mV
- Use devm_kmemdup_array() instead of devm_kmemdup()
- Use %u for unsigned index in Rfs error logs.
- Consolidated Rfs parse logs to a single line.
changes v2:
- Reorder struct ds4424_chip_info members to optimize padding.
- Use GENMASK() for chip variant masks instead of hex constants.
- Simplify ds4424_setup_channels: use direct devm_kmemdup to avoid stack
  usage and memcpy.
- Use local 'dev' pointer and dev_err_probe() in ds4424_parse_rfs for
  cleaner error handling.
- Rename the static iio_info struct to ds4424_iio_info to prevent name
  collision with the new hardware chip_info structs.
- Use unsigned int for loop counters.
- Rebase on top of regmap and symmetrical raw_access refactoring.
---
 drivers/iio/dac/ds4424.c | 121 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 116 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/dac/ds4424.c b/drivers/iio/dac/ds4424.c
index 9bef1c60b2eb..2b01e20daee4 100644
--- a/drivers/iio/dac/ds4424.c
+++ b/drivers/iio/dac/ds4424.c
@@ -12,6 +12,7 @@
 #include <linux/i2c.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/property.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 
@@ -24,6 +25,7 @@
 #define DS4424_MAX_DAC_CHANNELS		4
 
 #define DS4424_DAC_MASK			GENMASK(6, 0)
+#define DS4404_DAC_MASK			GENMASK(4, 0)
 #define DS4424_DAC_SOURCE		BIT(7)
 
 #define DS4424_DAC_ADDR(chan)   ((chan) + 0xf8)
@@ -43,9 +45,38 @@ enum ds4424_device_ids {
 	ID_DS4424,
 };
 
+/*
+ * Two variant groups share the same register map but differ in:
+ * - resolution/data mask (DS4402/DS4404: 5-bit, DS4422/DS4424: 7-bit)
+ * - full-scale current calculation (different Vref and divider)
+ * Addressing also differs (DS440x tri-level, DS442x bi-level), but is
+ * handled via board configuration, not driver logic.
+ */
+struct ds4424_chip_info {
+	int vref_mV;
+	int scale_denom;
+	u8 result_mask;
+};
+
+static const struct ds4424_chip_info ds4424_info = {
+	.vref_mV = 976,
+	.scale_denom = 16,
+	.result_mask = DS4424_DAC_MASK,
+};
+
+/* DS4402 is handled like DS4404 (same resolution and scale formula). */
+static const struct ds4424_chip_info ds4404_info = {
+	.vref_mV = 1230,
+	.scale_denom = 4,
+	.result_mask = DS4404_DAC_MASK,
+};
+
 struct ds4424_data {
 	struct regmap *regmap;
 	struct regulator *vcc_reg;
+	const struct ds4424_chip_info *chip_info;
+	u32 rfs_ohms[DS4424_MAX_DAC_CHANNELS];
+	bool has_rfs;
 };
 
 static const struct iio_chan_spec ds4424_channels[] = {
@@ -144,11 +175,20 @@ static int ds4424_read_raw(struct iio_dev *indio_dev,
 			return ret;
 		}
 
-		*val = regval & DS4424_DAC_MASK;
+		*val = regval & data->chip_info->result_mask;
 		if (!(regval & DS4424_DAC_SOURCE))
 			*val = -*val;
 
 		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		if (!data->has_rfs)
+			return -EINVAL;
+
+		/* SCALE is mA/step: mV / Ohm = mA. */
+		*val = data->chip_info->vref_mV;
+		*val2 = data->rfs_ohms[chan->channel] *
+			data->chip_info->scale_denom;
+		return IIO_VAL_FRACTIONAL;
 
 	default:
 		return -EINVAL;
@@ -168,7 +208,7 @@ static int ds4424_write_raw(struct iio_dev *indio_dev,
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
 		abs_val = abs(val);
-		if (abs_val > DS4424_DAC_MASK)
+		if (abs_val > data->chip_info->result_mask)
 			return -EINVAL;
 
 		/*
@@ -187,6 +227,65 @@ static int ds4424_write_raw(struct iio_dev *indio_dev,
 	}
 }
 
+static int ds4424_setup_channels(struct i2c_client *client,
+				 struct ds4424_data *data,
+				 struct iio_dev *indio_dev)
+{
+	struct iio_chan_spec *channels;
+
+	/* Use a local non-const pointer for modification */
+	channels = devm_kmemdup_array(&client->dev, ds4424_channels,
+				      indio_dev->num_channels,
+				      sizeof(ds4424_channels[0]), GFP_KERNEL);
+	if (!channels)
+		return -ENOMEM;
+
+	if (data->has_rfs) {
+		for (unsigned int i = 0; i < indio_dev->num_channels; i++)
+			channels[i].info_mask_separate |=
+				BIT(IIO_CHAN_INFO_SCALE);
+	}
+
+	indio_dev->channels = channels;
+
+	return 0;
+}
+
+static int ds4424_parse_rfs(struct i2c_client *client,
+			    struct ds4424_data *data,
+			    struct iio_dev *indio_dev)
+{
+	struct device *dev = &client->dev;
+	int count, ret;
+
+	if (!device_property_present(dev, "maxim,rfs-ohms")) {
+		dev_info_once(dev, "maxim,rfs-ohms missing, scale not supported\n");
+		return 0;
+	}
+
+	count = device_property_count_u32(dev, "maxim,rfs-ohms");
+	if (count < 0)
+		return dev_err_probe(dev, count, "Failed to count maxim,rfs-ohms entries\n");
+	if (count != indio_dev->num_channels)
+		return dev_err_probe(dev, -EINVAL, "maxim,rfs-ohms must have %u entries\n",
+				     indio_dev->num_channels);
+
+	ret = device_property_read_u32_array(dev, "maxim,rfs-ohms",
+					     data->rfs_ohms,
+					     indio_dev->num_channels);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to read maxim,rfs-ohms property\n");
+
+	for (unsigned int i = 0; i < indio_dev->num_channels; i++) {
+		if (!data->rfs_ohms[i])
+			return dev_err_probe(dev, -EINVAL, "maxim,rfs-ohms entry %u is zero\n", i);
+	}
+
+	data->has_rfs = true;
+
+	return 0;
+}
+
 static int ds4424_suspend(struct device *dev)
 {
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
@@ -221,7 +320,7 @@ static int ds4424_resume(struct device *dev)
 
 static DEFINE_SIMPLE_DEV_PM_OPS(ds4424_pm_ops, ds4424_suspend, ds4424_resume);
 
-static const struct iio_info ds4424_info = {
+static const struct iio_info ds4424_iio_info = {
 	.read_raw = ds4424_read_raw,
 	.write_raw = ds4424_write_raw,
 };
@@ -258,15 +357,20 @@ static int ds4424_probe(struct i2c_client *client)
 	switch (id->driver_data) {
 	case ID_DS4402:
 		indio_dev->num_channels = DS4422_MAX_DAC_CHANNELS;
+		/* See ds4404_info comment above. */
+		data->chip_info = &ds4404_info;
 		break;
 	case ID_DS4404:
 		indio_dev->num_channels = DS4424_MAX_DAC_CHANNELS;
+		data->chip_info = &ds4404_info;
 		break;
 	case ID_DS4422:
 		indio_dev->num_channels = DS4422_MAX_DAC_CHANNELS;
+		data->chip_info = &ds4424_info;
 		break;
 	case ID_DS4424:
 		indio_dev->num_channels = DS4424_MAX_DAC_CHANNELS;
+		data->chip_info = &ds4424_info;
 		break;
 	default:
 		dev_err(&client->dev,
@@ -279,9 +383,16 @@ static int ds4424_probe(struct i2c_client *client)
 	if (ret)
 		goto fail;
 
-	indio_dev->channels = ds4424_channels;
+	ret = ds4424_parse_rfs(client, data, indio_dev);
+	if (ret)
+		goto fail;
+
+	ret = ds4424_setup_channels(client, data, indio_dev);
+	if (ret)
+		goto fail;
+
 	indio_dev->modes = INDIO_DIRECT_MODE;
-	indio_dev->info = &ds4424_info;
+	indio_dev->info = &ds4424_iio_info;
 
 	ret = iio_device_register(indio_dev);
 	if (ret < 0) {
-- 
2.47.3

Re: [PATCH v3 8/8] iio: dac: ds4424: add Rfs-based scale and per-variant limits

[Jonathan Cameron]

On Wed, 28 Jan 2026 16:38:24 +0100
Oleksij Rempel <o.rempel@pengutronix.de> wrote:

> Parse optional maxim,rfs-ohms values to derive the per-channel output
> current scale (mA per step) for the IIO current ABI.
> 
> Select per-variant parameters to match the shared register map while
> handling different data widths and full-scale current calculations.
> 
> Behavior changes:
> - If maxim,rfs-ohms is present, IIO_CHAN_INFO_SCALE becomes available
>   and reports mA/step derived from Rfs.
> - If maxim,rfs-ohms is missing, SCALE is not exposed to keep older DTs
>   working without requiring updates.
> - RAW writes are now limited to the representable sign-magnitude range
>   of the detected variant to avoid silent truncation (e.g. +/-31 on
>   DS440x).

This seems to be doing several things.
Split it into a patch introducing the structure for per channel information
and then one adding the maxim,rfs-ohms support.

Various more specific suggestions inline.

Thanks,

Jonathan

> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
> changes v3:
> - Added explicit check for negative return from device_property_count_u32().
> - Rename vref_mv to vref_mV
> - Use devm_kmemdup_array() instead of devm_kmemdup()
> - Use %u for unsigned index in Rfs error logs.
> - Consolidated Rfs parse logs to a single line.
> changes v2:
> - Reorder struct ds4424_chip_info members to optimize padding.
> - Use GENMASK() for chip variant masks instead of hex constants.
> - Simplify ds4424_setup_channels: use direct devm_kmemdup to avoid stack
>   usage and memcpy.
> - Use local 'dev' pointer and dev_err_probe() in ds4424_parse_rfs for
>   cleaner error handling.
> - Rename the static iio_info struct to ds4424_iio_info to prevent name
>   collision with the new hardware chip_info structs.
> - Use unsigned int for loop counters.
> - Rebase on top of regmap and symmetrical raw_access refactoring.
> ---
>  drivers/iio/dac/ds4424.c | 121 +++++++++++++++++++++++++++++++++++++--
>  1 file changed, 116 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/dac/ds4424.c b/drivers/iio/dac/ds4424.c
> index 9bef1c60b2eb..2b01e20daee4 100644
> --- a/drivers/iio/dac/ds4424.c
> +++ b/drivers/iio/dac/ds4424.c
> @@ -12,6 +12,7 @@
>  #include <linux/i2c.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> +#include <linux/property.h>
>  #include <linux/regmap.h>
>  #include <linux/regulator/consumer.h>
>  
> @@ -24,6 +25,7 @@
>  #define DS4424_MAX_DAC_CHANNELS		4
>  
>  #define DS4424_DAC_MASK			GENMASK(6, 0)
> +#define DS4404_DAC_MASK			GENMASK(4, 0)
>  #define DS4424_DAC_SOURCE		BIT(7)
>  
>  #define DS4424_DAC_ADDR(chan)   ((chan) + 0xf8)
> @@ -43,9 +45,38 @@ enum ds4424_device_ids {
>  	ID_DS4424,
>  };
>  
> +/*
> + * Two variant groups share the same register map but differ in:
> + * - resolution/data mask (DS4402/DS4404: 5-bit, DS4422/DS4424: 7-bit)
> + * - full-scale current calculation (different Vref and divider)
> + * Addressing also differs (DS440x tri-level, DS442x bi-level), but is
> + * handled via board configuration, not driver logic.
> + */
> +struct ds4424_chip_info {
As mentioned below. This being introduced should occur
without anything related to the rfs. I'd also like the enum to go away
in favour of using these structures directly as the per device type data
in the i2c_device_id tables and the dt ones.
Note there is a nice helper that tries to get that data first from DT
and then falls back to the i2c_device_id table if it fails
i2c_get_match_data().

> +	int vref_mV;
> +	int scale_denom;
> +	u8 result_mask;
> +};
> +
> +static const struct ds4424_chip_info ds4424_info = {
> +	.vref_mV = 976,
> +	.scale_denom = 16,
> +	.result_mask = DS4424_DAC_MASK,
> +};
> +
> +/* DS4402 is handled like DS4404 (same resolution and scale formula). */
> +static const struct ds4424_chip_info ds4404_info = {
> +	.vref_mV = 1230,
> +	.scale_denom = 4,
> +	.result_mask = DS4404_DAC_MASK,
> +};
> +
>  struct ds4424_data {
>  	struct regmap *regmap;
>  	struct regulator *vcc_reg;
> +	const struct ds4424_chip_info *chip_info;
> +	u32 rfs_ohms[DS4424_MAX_DAC_CHANNELS];
> +	bool has_rfs;
>  };
>  
>  static const struct iio_chan_spec ds4424_channels[] = {
> @@ -144,11 +175,20 @@ static int ds4424_read_raw(struct iio_dev *indio_dev,
>  			return ret;
>  		}
>  
> -		*val = regval & DS4424_DAC_MASK;
> +		*val = regval & data->chip_info->result_mask;
>  		if (!(regval & DS4424_DAC_SOURCE))
>  			*val = -*val;
>  
>  		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		if (!data->has_rfs)
> +			return -EINVAL;
> +
> +		/* SCALE is mA/step: mV / Ohm = mA. */
> +		*val = data->chip_info->vref_mV;
> +		*val2 = data->rfs_ohms[chan->channel] *
> +			data->chip_info->scale_denom;
> +		return IIO_VAL_FRACTIONAL;
>  
>  	default:
>  		return -EINVAL;
> @@ -168,7 +208,7 @@ static int ds4424_write_raw(struct iio_dev *indio_dev,
>  	switch (mask) {
>  	case IIO_CHAN_INFO_RAW:
>  		abs_val = abs(val);
> -		if (abs_val > DS4424_DAC_MASK)
> +		if (abs_val > data->chip_info->result_mask)
This confused me as nothing to do with the rfs.
So this should be multiple patches.

>  			return -EINVAL;
>  
>  		/*
> @@ -187,6 +227,65 @@ static int ds4424_write_raw(struct iio_dev *indio_dev,
>  	}
>  }
>  
> +static int ds4424_setup_channels(struct i2c_client *client,
> +				 struct ds4424_data *data,
> +				 struct iio_dev *indio_dev)
> +{
> +	struct iio_chan_spec *channels;
> +
> +	/* Use a local non-const pointer for modification */
> +	channels = devm_kmemdup_array(&client->dev, ds4424_channels,
> +				      indio_dev->num_channels,
> +				      sizeof(ds4424_channels[0]), GFP_KERNEL);
> +	if (!channels)
> +		return -ENOMEM;
> +
> +	if (data->has_rfs) {
> +		for (unsigned int i = 0; i < indio_dev->num_channels; i++)
> +			channels[i].info_mask_separate |=
> +				BIT(IIO_CHAN_INFO_SCALE);

There are only two options.  Just have 2 static const iio_chan_spec []
and pick between them.  One has the scale and the other doesn't.

Dynamic channel specification creation is fine if there are lots
of variants, but not worth the effort for 2.

> +	}
> +
> +	indio_dev->channels = channels;
> +
> +	return 0;
> +}
> +
> +static int ds4424_parse_rfs(struct i2c_client *client,
> +			    struct ds4424_data *data,
> +			    struct iio_dev *indio_dev)
> +{
> +	struct device *dev = &client->dev;
> +	int count, ret;
> +
> +	if (!device_property_present(dev, "maxim,rfs-ohms")) {
> +		dev_info_once(dev, "maxim,rfs-ohms missing, scale not supported\n");

Absences of the _scale sysfs file should be enough for that rather than filling up
the kernel log. dev_dbg() only.

> +		return 0;
> +	}
> +
> +	count = device_property_count_u32(dev, "maxim,rfs-ohms");
> +	if (count < 0)
> +		return dev_err_probe(dev, count, "Failed to count maxim,rfs-ohms entries\n");
> +	if (count != indio_dev->num_channels)
> +		return dev_err_probe(dev, -EINVAL, "maxim,rfs-ohms must have %u entries\n",
> +				     indio_dev->num_channels);
> +
> +	ret = device_property_read_u32_array(dev, "maxim,rfs-ohms",
> +					     data->rfs_ohms,
> +					     indio_dev->num_channels);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to read maxim,rfs-ohms property\n");
> +
> +	for (unsigned int i = 0; i < indio_dev->num_channels; i++) {
> +		if (!data->rfs_ohms[i])
> +			return dev_err_probe(dev, -EINVAL, "maxim,rfs-ohms entry %u is zero\n", i);
> +	}
> +
> +	data->has_rfs = true;
> +
> +	return 0;
> +}

> @@ -258,15 +357,20 @@ static int ds4424_probe(struct i2c_client *client)
>  	switch (id->driver_data) {
>  	case ID_DS4402:
>  		indio_dev->num_channels = DS4422_MAX_DAC_CHANNELS;
> +		/* See ds4404_info comment above. */
> +		data->chip_info = &ds4404_info;

I'd strongly prefer to see the driver_data become a pointer to this structure
(and have it it for all firmware types rather than relying on matching
 across them which sometimes goes wrong).

That would need to be a precursor patch but would end up simplifying
things because you'd just put num_channels in there as well and no
switch statement would be on these enum values (get rid of that enum)

>  		break;
>  	case ID_DS4404:
>  		indio_dev->num_channels = DS4424_MAX_DAC_CHANNELS;
> +		data->chip_info = &ds4404_info;
>  		break;
>  	case ID_DS4422:
>  		indio_dev->num_channels = DS4422_MAX_DAC_CHANNELS;
> +		data->chip_info = &ds4424_info;
>  		break;
>  	case ID_DS4424:
>  		indio_dev->num_channels = DS4424_MAX_DAC_CHANNELS;
> +		data->chip_info = &ds4424_info;
>  		break;
>  	default:
>  		dev_err(&client->dev,
> @@ -279,9 +383,16 @@ static int ds4424_probe(struct i2c_client *client)
>  	if (ret)
>  		goto fail;
>  
> -	indio_dev->channels = ds4424_channels;
> +	ret = ds4424_parse_rfs(client, data, indio_dev);
> +	if (ret)
> +		goto fail;
> +
> +	ret = ds4424_setup_channels(client, data, indio_dev);
> +	if (ret)
> +		goto fail;
> +
>  	indio_dev->modes = INDIO_DIRECT_MODE;
> -	indio_dev->info = &ds4424_info;
> +	indio_dev->info = &ds4424_iio_info;
I'd be tempted to do the rename as at trivial precursor patch
that just says you need the namespace for the use made in this patch.

Otherwise the code becomes a bit fiddly to read in here as meaning
changes between removed and added code.

Jonathan
>  
>  	ret = iio_device_register(indio_dev);
>  	if (ret < 0) {

Re: [PATCH v3 0/8] iio: dac: ds4424: add DS4402/DS4404 support and scale

[Andy Shevchenko]

On Wed, Jan 28, 2026 at 04:38:16PM +0100, Oleksij Rempel wrote:
> This series extends the ds4424 IIO DAC driver and its devicetree binding
> to support the DS4402 and DS4404 current DAC variants.
> 
> DS440x devices share the same register map as DS442x but use a different
> resolution (5-bit vs 7-bit) and a different full-scale current formula.
> The full-scale current depends on external Rfs resistors connected to
> the FS pins, so a new optional DT property is added to provide the
> per-channel Rfs values and allow the driver to report a correct IIO
> SCALE (mA/step).
> 
> While adding DS440x support, a few related issues were addressed:
> - Port to regmap
> - Reject -128 in RAW writes on DS442x, which cannot be represented with
>   sign-magnitude encoding and could silently program an unintended
>   output.
> - Preserve preconfigured values on probe.
> - Ratelimit read error logging and use device context.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
for patches 1,2,3,6, and 8.

-- 
With Best Regards,
Andy Shevchenko