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

[PATCH v1 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:
- Reject -128 in RAW writes on DS442x, which cannot be represented with
  sign-magnitude encoding and could silently program an unintended
  output.
- Clear outputs on probe to avoid retaining preconfigured values across
  warm reset and to make suspend/resume behavior deterministic.
- Ratelimit read error logging and use device context.
- Add small comments documenting sign handling and probe verification.

Oleksij Rempel (8):
  dt-bindings: iio: dac: maxim,ds4424: add ds4402/ds4404
  dt-bindings: iio: dac: maxim,ds4424: add maxim,rfs-ohms property
  iio: dac: ds4424: add DS4402/DS4404 device IDs
  iio: dac: ds4424: reject -128 RAW value
  iio: dac: ds4424: add Rfs-based scale and per-variant limits
  iio: dac: ds4424: clear outputs on probe
  iio: dac: ds4424: ratelimit read errors and use device context
  iio: dac: ds4424: document output sign and probe verification

 .../bindings/iio/dac/maxim,ds4424.yaml        |  18 +-
 drivers/iio/dac/ds4424.c                      | 175 +++++++++++++++++-
 2 files changed, 184 insertions(+), 9 deletions(-)

--
2.47.3

[PATCH v1 1/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>
---
 .../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

Re: [PATCH v1 1/8] dt-bindings: iio: dac: maxim,ds4424: add ds4402/ds4404

[Conor Dooley]

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

On Mon, Jan 19, 2026 at 07:24:17PM +0100, Oleksij Rempel wrote:
> 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>
pw-bot: not-applicable

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

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

[Oleksij Rempel]

Introduce the 'maxim,rfs-ohms' property. The full-scale output current
of these DACs is determined by external resistors (Rfs) connected to
the FS pins. The driver requires the physical resistance values to
calculate the correct current scale (Amps per step) for the IIO
subsystem.

Keep it optional to avoid forcing updates of existing DTs; without it
the driver cannot derive a correct IIO scale.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 .../devicetree/bindings/iio/dac/maxim,ds4424.yaml     | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml b/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml
index efe63e6cb55d..b7af8464a4fc 100644
--- a/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml
+++ b/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml
@@ -27,6 +27,17 @@ properties:
 
   vcc-supply: true
 
+  maxim,rfs-ohms:
+    description: |
+      Array of resistance values in Ohms for the external Rfs resistors
+      connected to the FS pins.
+      - For DS44x2 (2 channels): 2 values required.
+      - For DS44x4 (4 channels): 4 values required.
+      Typical values range from 40000 (40 kOhm) to 160000 (160 kOhm).
+    minItems: 2
+    maxItems: 4
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+
 required:
   - compatible
   - reg
-- 
2.47.3

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

[Conor Dooley]

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

On Mon, Jan 19, 2026 at 07:24:18PM +0100, Oleksij Rempel wrote:
> Introduce the 'maxim,rfs-ohms' property. The full-scale output current
> of these DACs is determined by external resistors (Rfs) connected to
> the FS pins. The driver requires the physical resistance values to
> calculate the correct current scale (Amps per step) for the IIO
> subsystem.
> 
> Keep it optional to avoid forcing updates of existing DTs; without it
> the driver cannot derive a correct IIO scale.

I don't really follow the logic here, if the driver doesn't work
properly without it, shouldn't it be a required property even if that
means existing devicetrees get new warnings? Warnings are preferable to
the drivers malfunctioning on those devices, after all!

> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  .../devicetree/bindings/iio/dac/maxim,ds4424.yaml     | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml b/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml
> index efe63e6cb55d..b7af8464a4fc 100644
> --- a/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml
> +++ b/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml
> @@ -27,6 +27,17 @@ properties:
>  
>    vcc-supply: true
>  
> +  maxim,rfs-ohms:
> +    description: |
> +      Array of resistance values in Ohms for the external Rfs resistors
> +      connected to the FS pins.
> +      - For DS44x2 (2 channels): 2 values required.
> +      - For DS44x4 (4 channels): 4 values required.
> +      Typical values range from 40000 (40 kOhm) to 160000 (160 kOhm).
> +    minItems: 2
> +    maxItems: 4
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +
>  required:
>    - compatible
>    - reg
> -- 
> 2.47.3
> 

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

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

[Jonathan Cameron]

On Mon, 19 Jan 2026 18:27:52 +0000
Conor Dooley <conor@kernel.org> wrote:

> On Mon, Jan 19, 2026 at 07:24:18PM +0100, Oleksij Rempel wrote:
> > Introduce the 'maxim,rfs-ohms' property. The full-scale output current
> > of these DACs is determined by external resistors (Rfs) connected to
> > the FS pins. The driver requires the physical resistance values to
> > calculate the correct current scale (Amps per step) for the IIO
> > subsystem.
> > 
> > Keep it optional to avoid forcing updates of existing DTs; without it
> > the driver cannot derive a correct IIO scale.  
> 
> I don't really follow the logic here, if the driver doesn't work
> properly without it, shouldn't it be a required property even if that
> means existing devicetrees get new warnings? Warnings are preferable to
> the drivers malfunctioning on those devices, after all!

Agreed. The driver can paper over holes or I guess we could provide a default
if the datasheet has some reference value or similar?  

DT binding should require it.

> 
> > 
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > ---
> >  .../devicetree/bindings/iio/dac/maxim,ds4424.yaml     | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml b/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml
> > index efe63e6cb55d..b7af8464a4fc 100644
> > --- a/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml
> > +++ b/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml
> > @@ -27,6 +27,17 @@ properties:
> >  
> >    vcc-supply: true
> >  
> > +  maxim,rfs-ohms:
> > +    description: |
> > +      Array of resistance values in Ohms for the external Rfs resistors
> > +      connected to the FS pins.
> > +      - For DS44x2 (2 channels): 2 values required.
> > +      - For DS44x4 (4 channels): 4 values required.
> > +      Typical values range from 40000 (40 kOhm) to 160000 (160 kOhm).
> > +    minItems: 2
> > +    maxItems: 4
> > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > +
> >  required:
> >    - compatible
> >    - reg
> > -- 
> > 2.47.3
> >   

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

[Oleksij Rempel]

On Fri, Jan 23, 2026 at 09:28:19AM +0000, Jonathan Cameron wrote:
> On Mon, 19 Jan 2026 18:27:52 +0000
> Conor Dooley <conor@kernel.org> wrote:
> 
> > On Mon, Jan 19, 2026 at 07:24:18PM +0100, Oleksij Rempel wrote:
> > > Introduce the 'maxim,rfs-ohms' property. The full-scale output current
> > > of these DACs is determined by external resistors (Rfs) connected to
> > > the FS pins. The driver requires the physical resistance values to
> > > calculate the correct current scale (Amps per step) for the IIO
> > > subsystem.
> > > 
> > > Keep it optional to avoid forcing updates of existing DTs; without it
> > > the driver cannot derive a correct IIO scale.  
> > 
> > I don't really follow the logic here, if the driver doesn't work
> > properly without it, shouldn't it be a required property even if that
> > means existing devicetrees get new warnings? Warnings are preferable to
> > the drivers malfunctioning on those devices, after all!
> 
> Agreed. The driver can paper over holes or I guess we could provide a default
> if the datasheet has some reference value or similar?  

No, this DAC controls the current. The current range is application specific and
defined by the load resistors. Is it better to not provide scale if this
information not available, guessing it will make things worse and
potentially damage some HW.

> DT binding should require it.

ACK
-- 
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 v1 2/8] dt-bindings: iio: dac: maxim,ds4424: add maxim,rfs-ohms property

[Jonathan Cameron]

On Fri, 23 Jan 2026 11:03:38 +0100
Oleksij Rempel <o.rempel@pengutronix.de> wrote:

> On Fri, Jan 23, 2026 at 09:28:19AM +0000, Jonathan Cameron wrote:
> > On Mon, 19 Jan 2026 18:27:52 +0000
> > Conor Dooley <conor@kernel.org> wrote:
> >   
> > > On Mon, Jan 19, 2026 at 07:24:18PM +0100, Oleksij Rempel wrote:  
> > > > Introduce the 'maxim,rfs-ohms' property. The full-scale output current
> > > > of these DACs is determined by external resistors (Rfs) connected to
> > > > the FS pins. The driver requires the physical resistance values to
> > > > calculate the correct current scale (Amps per step) for the IIO
> > > > subsystem.
> > > > 
> > > > Keep it optional to avoid forcing updates of existing DTs; without it
> > > > the driver cannot derive a correct IIO scale.    
> > > 
> > > I don't really follow the logic here, if the driver doesn't work
> > > properly without it, shouldn't it be a required property even if that
> > > means existing devicetrees get new warnings? Warnings are preferable to
> > > the drivers malfunctioning on those devices, after all!  
> > 
> > Agreed. The driver can paper over holes or I guess we could provide a default
> > if the datasheet has some reference value or similar?    
> 
> No, this DAC controls the current. The current range is application specific and
> defined by the load resistors. Is it better to not provide scale if this
> information not available, guessing it will make things worse and
> potentially damage some HW.
> 
Then does this need to be a fix on the driver side?
Or are you just suggesting not proving the scale if we don't have enough info?
That's fine if so.

> > DT binding should require it.  
> 
> ACK

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

[Oleksij Rempel]

On Sun, Jan 25, 2026 at 10:22:07AM +0000, Jonathan Cameron wrote:
> On Fri, 23 Jan 2026 11:03:38 +0100
> Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> 
> > On Fri, Jan 23, 2026 at 09:28:19AM +0000, Jonathan Cameron wrote:
> > > On Mon, 19 Jan 2026 18:27:52 +0000
> > > Conor Dooley <conor@kernel.org> wrote:
> > >   
> > > > On Mon, Jan 19, 2026 at 07:24:18PM +0100, Oleksij Rempel wrote:  
> > > > > Introduce the 'maxim,rfs-ohms' property. The full-scale output current
> > > > > of these DACs is determined by external resistors (Rfs) connected to
> > > > > the FS pins. The driver requires the physical resistance values to
> > > > > calculate the correct current scale (Amps per step) for the IIO
> > > > > subsystem.
> > > > > 
> > > > > Keep it optional to avoid forcing updates of existing DTs; without it
> > > > > the driver cannot derive a correct IIO scale.    
> > > > 
> > > > I don't really follow the logic here, if the driver doesn't work
> > > > properly without it, shouldn't it be a required property even if that
> > > > means existing devicetrees get new warnings? Warnings are preferable to
> > > > the drivers malfunctioning on those devices, after all!  
> > > 
> > > Agreed. The driver can paper over holes or I guess we could provide a default
> > > if the datasheet has some reference value or similar?    
> > 
> > No, this DAC controls the current. The current range is application specific and
> > defined by the load resistors. Is it better to not provide scale if this
> > information not available, guessing it will make things worse and
> > potentially damage some HW.
> > 
> Then does this need to be a fix on the driver side?
> Or are you just suggesting not proving the scale if we don't have enough info?
> That's fine if so.

I suggest to not provide scale if we don't have enough info. It will be
disabled in the driver if maxim,rfs-ohms is not provided in the DT.

-- 
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 v1 2/8] dt-bindings: iio: dac: maxim,ds4424: add maxim,rfs-ohms property

[Krzysztof Kozlowski]

On Mon, Jan 19, 2026 at 07:24:18PM +0100, Oleksij Rempel wrote:
> Introduce the 'maxim,rfs-ohms' property. The full-scale output current
> of these DACs is determined by external resistors (Rfs) connected to
> the FS pins. The driver requires the physical resistance values to
> calculate the correct current scale (Amps per step) for the IIO
> subsystem.
> 
> Keep it optional to avoid forcing updates of existing DTs; without it
> the driver cannot derive a correct IIO scale.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  .../devicetree/bindings/iio/dac/maxim,ds4424.yaml     | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml b/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml
> index efe63e6cb55d..b7af8464a4fc 100644
> --- a/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml
> +++ b/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml
> @@ -27,6 +27,17 @@ properties:
>  
>    vcc-supply: true
>  
> +  maxim,rfs-ohms:
> +    description: |
> +      Array of resistance values in Ohms for the external Rfs resistors
> +      connected to the FS pins.
> +      - For DS44x2 (2 channels): 2 values required.
> +      - For DS44x4 (4 channels): 4 values required.
> +      Typical values range from 40000 (40 kOhm) to 160000 (160 kOhm).
> +    minItems: 2
> +    maxItems: 4
> +    $ref: /schemas/types.yaml#/definitions/uint32-array

Drop. You should see a warning, so I claim this wasn't tested/checked.

Best regards,
Krzysztof

[PATCH v1 3/8] iio: dac: ds4424: add DS4402/DS4404 device IDs

[Oleksij Rempel]

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.

Signed-off-by: David Jander <david@protonic.nl>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 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 a8198ba4f98a..072b7e6672cf 100644
--- a/drivers/iio/dac/ds4424.c
+++ b/drivers/iio/dac/ds4424.c
@@ -48,6 +48,8 @@ union ds4424_raw_data {
 };
 
 enum ds4424_device_ids {
+	ID_DS4402,
+	ID_DS4404,
 	ID_DS4422,
 	ID_DS4424,
 };
@@ -248,6 +250,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;
@@ -289,6 +297,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 },
 	{ }
@@ -297,6 +307,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

[PATCH v1 4/8] iio: dac: ds4424: reject -128 RAW value

[Oleksij Rempel]

The DS442x DAC uses sign-magnitude encoding, so -128 cannot be
represented in hardware.

With the previous check, userspace could pass -128, which gets converted
to a magnitude of 128 and then truncated by the 7-bit DAC field. This
ends up programming a zero magnitude with the sign bit set, i.e. an
unintended output (effectively 0 mA instead of -128 steps).

Reject -128 to avoid silently producing the wrong current.

Fixes: d632a2bd8ffc ("iio: dac: ds4422/ds4424 dac driver")
Cc: <stable@vger.kernel.org>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/iio/dac/ds4424.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/dac/ds4424.c b/drivers/iio/dac/ds4424.c
index 072b7e6672cf..9c24c37d3c42 100644
--- a/drivers/iio/dac/ds4424.c
+++ b/drivers/iio/dac/ds4424.c
@@ -143,7 +143,7 @@ static int ds4424_write_raw(struct iio_dev *indio_dev,
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		if (val < S8_MIN || val > S8_MAX)
+		if (val <= S8_MIN || val > S8_MAX)
 			return -EINVAL;
 
 		if (val > 0) {
-- 
2.47.3

Re: [PATCH v1 4/8] iio: dac: ds4424: reject -128 RAW value

[Andy Shevchenko]

On Mon, Jan 19, 2026 at 07:24:20PM +0100, Oleksij Rempel wrote:
> The DS442x DAC uses sign-magnitude encoding, so -128 cannot be
> represented in hardware.
> 
> With the previous check, userspace could pass -128, which gets converted
> to a magnitude of 128 and then truncated by the 7-bit DAC field. This
> ends up programming a zero magnitude with the sign bit set, i.e. an
> unintended output (effectively 0 mA instead of -128 steps).
> 
> Reject -128 to avoid silently producing the wrong current.

...

> -		if (val < S8_MIN || val > S8_MAX)
> +		if (val <= S8_MIN || val > S8_MAX)
>  			return -EINVAL;

Hmm... So the range is [ -127 .. 0 .. 127 ] ?

I think in such case the plain numbers would be more specific than
the type related limits.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 4/8] iio: dac: ds4424: reject -128 RAW value

[Jonathan Cameron]

On Mon, 19 Jan 2026 21:03:46 +0200
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Mon, Jan 19, 2026 at 07:24:20PM +0100, Oleksij Rempel wrote:
> > The DS442x DAC uses sign-magnitude encoding, so -128 cannot be
> > represented in hardware.
> > 
> > With the previous check, userspace could pass -128, which gets converted
> > to a magnitude of 128 and then truncated by the 7-bit DAC field. This
> > ends up programming a zero magnitude with the sign bit set, i.e. an
> > unintended output (effectively 0 mA instead of -128 steps).
> > 
> > Reject -128 to avoid silently producing the wrong current.  
> 
> ...
> 
> > -		if (val < S8_MIN || val > S8_MAX)
> > +		if (val <= S8_MIN || val > S8_MAX)
> >  			return -EINVAL;  
> 
> Hmm... So the range is [ -127 .. 0 .. 127 ] ?
> 
> I think in such case the plain numbers would be more specific than
> the type related limits.
> 

Check the abs(val) <= 127 given that's what we care about I think?
Or make it explicit and do
FIELD_FIT() against a mask that you then use to fill the register
value (another mask for the sign bit).

Btw use abs(val) to set raw.dx and drop it out of the conditional.
Even better get rid of the bitfield stuff and just add
two defines + fill val directly in this function using FIELD_PREP().
Then both the checking and the field filling use the same defines
and it should be easy to see what is going on.

Jonathan

Re: [PATCH v1 4/8] iio: dac: ds4424: reject -128 RAW value

[Oleksij Rempel]

On Fri, Jan 23, 2026 at 09:33:57AM +0000, Jonathan Cameron wrote:
> On Mon, 19 Jan 2026 21:03:46 +0200
> Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> 
> > On Mon, Jan 19, 2026 at 07:24:20PM +0100, Oleksij Rempel wrote:
> > > The DS442x DAC uses sign-magnitude encoding, so -128 cannot be
> > > represented in hardware.
> > > 
> > > With the previous check, userspace could pass -128, which gets converted
> > > to a magnitude of 128 and then truncated by the 7-bit DAC field. This
> > > ends up programming a zero magnitude with the sign bit set, i.e. an
> > > unintended output (effectively 0 mA instead of -128 steps).
> > > 
> > > Reject -128 to avoid silently producing the wrong current.  
> > 
> > ...
> > 
> > > -		if (val < S8_MIN || val > S8_MAX)
> > > +		if (val <= S8_MIN || val > S8_MAX)
> > >  			return -EINVAL;  
> > 
> > Hmm... So the range is [ -127 .. 0 .. 127 ] ?
> > 
> > I think in such case the plain numbers would be more specific than
> > the type related limits.
> > 
> 
> Check the abs(val) <= 127 given that's what we care about I think?
> Or make it explicit and do
> FIELD_FIT() against a mask that you then use to fill the register
> value (another mask for the sign bit).
> 
> Btw use abs(val) to set raw.dx and drop it out of the conditional.
> Even better get rid of the bitfield stuff and just add
> two defines + fill val directly in this function using FIELD_PREP().
> Then both the checking and the field filling use the same defines
> and it should be easy to see what is going on.

FIELD_* macros require compile-time constant masks. Since the next patch
adds support for variants with different data widths (making the mask a
runtime variable), I prefer using an implementation now that remains
consistent with the followup changes.

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

[PATCH v1 5/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>
---
 drivers/iio/dac/ds4424.c | 137 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 132 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/dac/ds4424.c b/drivers/iio/dac/ds4424.c
index 9c24c37d3c42..a0c60eb89717 100644
--- a/drivers/iio/dac/ds4424.c
+++ b/drivers/iio/dac/ds4424.c
@@ -8,6 +8,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/i2c.h>
+#include <linux/property.h>
 #include <linux/regulator/consumer.h>
 #include <linux/err.h>
 #include <linux/delay.h>
@@ -54,12 +55,41 @@ 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 {
+	u8 result_mask;
+	int vref_mv;
+	int scale_denom;
+};
+
+static const struct ds4424_chip_info ds4424_info = {
+	.result_mask = 0x7F,
+	.vref_mv = 976,
+	.scale_denom = 16,
+};
+
+/* DS4402 is handled like DS4404 (same resolution and scale formula). */
+static const struct ds4424_chip_info ds4404_info = {
+	.result_mask = 0x1F,
+	.vref_mv = 1230,
+	.scale_denom = 4,
+};
+
 struct ds4424_data {
 	struct i2c_client *client;
 	struct mutex lock;
 	uint8_t save[DS4424_MAX_DAC_CHANNELS];
 	struct regulator *vcc_reg;
 	uint8_t raw[DS4424_MAX_DAC_CHANNELS];
+	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[] = {
@@ -111,6 +141,7 @@ static int ds4424_read_raw(struct iio_dev *indio_dev,
 			   int *val, int *val2, long mask)
 {
 	union ds4424_raw_data raw;
+	struct ds4424_data *data = iio_priv(indio_dev);
 	int ret;
 
 	switch (mask) {
@@ -122,10 +153,19 @@ static int ds4424_read_raw(struct iio_dev *indio_dev,
 			return ret;
 		}
 		raw.bits = *val;
-		*val = raw.dx;
+		*val = raw.dx & data->chip_info->result_mask;
 		if (raw.source_bit == DS4424_SINK_I)
 			*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;
@@ -136,6 +176,8 @@ 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);
+	int max_val = data->chip_info->result_mask;
 	union ds4424_raw_data raw;
 
 	if (val2 != 0)
@@ -143,7 +185,12 @@ static int ds4424_write_raw(struct iio_dev *indio_dev,
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		if (val <= S8_MIN || val > S8_MAX)
+		/*
+		 * The hardware uses sign-magnitude representation (not
+		 * two's complement). Therefore, the range is symmetric:
+		 * [-max_val, +max_val].
+		 */
+		if (val < -max_val || val > max_val)
 			return -EINVAL;
 
 		if (val > 0) {
@@ -173,6 +220,74 @@ static int ds4424_verify_chip(struct iio_dev *indio_dev)
 	return ret;
 }
 
+static int ds4424_setup_channels(struct i2c_client *client,
+				 struct ds4424_data *data,
+				 struct iio_dev *indio_dev)
+{
+	struct iio_chan_spec channels[DS4424_MAX_DAC_CHANNELS];
+	size_t channels_size;
+	int i;
+
+	channels_size = indio_dev->num_channels * sizeof(*channels);
+	memcpy(channels, ds4424_channels, channels_size);
+
+	/* Enable scale only when rfs is available. */
+	if (data->has_rfs) {
+		for (i = 0; i < indio_dev->num_channels; i++)
+			channels[i].info_mask_separate |=
+				BIT(IIO_CHAN_INFO_SCALE);
+	}
+
+	indio_dev->channels = devm_kmemdup(&client->dev, channels,
+					   channels_size, GFP_KERNEL);
+	if (!indio_dev->channels)
+		return -ENOMEM;
+
+	return 0;
+}
+
+static int ds4424_parse_rfs(struct i2c_client *client,
+			    struct ds4424_data *data,
+			    struct iio_dev *indio_dev)
+{
+	int count, i, ret;
+
+	if (!device_property_present(&client->dev, "maxim,rfs-ohms")) {
+		dev_info_once(&client->dev, "maxim,rfs-ohms missing, scale not supported\n");
+		return 0;
+	}
+
+	count = device_property_count_u32(&client->dev, "maxim,rfs-ohms");
+	if (count != indio_dev->num_channels) {
+		dev_err(&client->dev,
+			"maxim,rfs-ohms must have %u entries\n",
+			indio_dev->num_channels);
+		return -EINVAL;
+	}
+
+	ret = device_property_read_u32_array(&client->dev,
+					     "maxim,rfs-ohms",
+					     data->rfs_ohms,
+					     indio_dev->num_channels);
+	if (ret) {
+		dev_err(&client->dev,
+			"Failed to read maxim,rfs-ohms property\n");
+		return ret;
+	}
+
+	for (i = 0; i < indio_dev->num_channels; i++) {
+		if (!data->rfs_ohms[i]) {
+			dev_err(&client->dev,
+				"maxim,rfs-ohms entry %d is zero\n",
+				i);
+			return -EINVAL;
+		}
+	}
+
+	data->has_rfs = true;
+	return 0;
+}
+
 static int ds4424_suspend(struct device *dev)
 {
 	struct i2c_client *client = to_i2c_client(dev);
@@ -210,7 +325,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,
 };
@@ -252,15 +367,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,
@@ -269,9 +389,16 @@ static int ds4424_probe(struct i2c_client *client)
 		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 v1 5/8] iio: dac: ds4424: add Rfs-based scale and per-variant limits

[Andy Shevchenko]

On Mon, Jan 19, 2026 at 07:24:21PM +0100, Oleksij Rempel 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).

...

> +struct ds4424_chip_info {
> +	u8 result_mask;
> +	int vref_mv;
> +	int scale_denom;

Wondering if `pahole` is fine with the proposed layout.
Otherwise I would move u8 to the end.

> +};

...

> +static const struct ds4424_chip_info ds4424_info = {
> +	.result_mask = 0x7F,

GENMASK() ?
(Note, we also have GENMASK_U8() IIRC)

> +	.vref_mv = 976,
> +	.scale_denom = 16,
> +};

...

> +/* DS4402 is handled like DS4404 (same resolution and scale formula). */
> +static const struct ds4424_chip_info ds4404_info = {
> +	.result_mask = 0x1F,

Ditto.

> +	.vref_mv = 1230,
> +	.scale_denom = 4,
> +};

...

> -		if (val <= S8_MIN || val > S8_MAX)
> +		/*
> +		 * The hardware uses sign-magnitude representation (not
> +		 * two's complement). Therefore, the range is symmetric:
> +		 * [-max_val, +max_val].
> +		 */
> +		if (val < -max_val || val > max_val)
>  			return -EINVAL;

Right, cool, but I still think even in the fix it would be good to fix that and
here it will be quite logical and clear change. Maybe even introduce a comment
in the fix-patch and just update limits here.

...

> +static int ds4424_setup_channels(struct i2c_client *client,
> +				 struct ds4424_data *data,
> +				 struct iio_dev *indio_dev)
> +{
> +	struct iio_chan_spec channels[DS4424_MAX_DAC_CHANNELS];
> +	size_t channels_size;
> +	int i;
> +
> +	channels_size = indio_dev->num_channels * sizeof(*channels);

Sounds like devm_kmemdup_array() to me...

> +	memcpy(channels, ds4424_channels, channels_size);

> +	/* Enable scale only when rfs is available. */
> +	if (data->has_rfs) {
> +		for (i = 0; i < indio_dev->num_channels; i++)
> +			channels[i].info_mask_separate |=
> +				BIT(IIO_CHAN_INFO_SCALE);
> +	}

Can be done after kmemdup(), right?

> +	indio_dev->channels = devm_kmemdup(&client->dev, channels,
> +					   channels_size, GFP_KERNEL);
> +	if (!indio_dev->channels)
> +		return -ENOMEM;
> +
> +	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, i, ret;
> +
> +	if (!device_property_present(&client->dev, "maxim,rfs-ohms")) {
> +		dev_info_once(&client->dev, "maxim,rfs-ohms missing, scale not supported\n");
> +		return 0;
> +	}
> +
> +	count = device_property_count_u32(&client->dev, "maxim,rfs-ohms");
> +	if (count != indio_dev->num_channels) {
> +		dev_err(&client->dev,
> +			"maxim,rfs-ohms must have %u entries\n",
> +			indio_dev->num_channels);
> +		return -EINVAL;
> +	}
> +
> +	ret = device_property_read_u32_array(&client->dev,
> +					     "maxim,rfs-ohms",
> +					     data->rfs_ohms,
> +					     indio_dev->num_channels);
> +	if (ret) {
> +		dev_err(&client->dev,
> +			"Failed to read maxim,rfs-ohms property\n");
> +		return ret;
> +	}
> +
> +	for (i = 0; i < indio_dev->num_channels; i++) {
> +		if (!data->rfs_ohms[i]) {

> +			dev_err(&client->dev,
> +				"maxim,rfs-ohms entry %d is zero\n",
> +				i);

> +			return -EINVAL;

This is only for probe stage, right?

Then

			return dev_err_probe();

?

> +		}
> +	}
> +
> +	data->has_rfs = true;
> +	return 0;
> +}

-- 
With Best Regards,
Andy Shevchenko

[PATCH v1 6/8] iio: dac: ds4424: clear outputs on probe

[Oleksij Rempel]

The DS44xx devices have no reset pin or reset bit, so output registers
may retain preconfigured values across reboot or warm reset.

Also, the driver suspend/resume path restores from data->raw. When the
device is first probed, data->raw is zero-initialized and may not match
the actual hardware state. A later suspend/resume can therefore change an
output from a preconfigured non-zero value to 0 mA.

Initialize all channels to 0 output current during probe to ensure a
deterministic baseline and consistent suspend/resume behavior.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/iio/dac/ds4424.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/iio/dac/ds4424.c b/drivers/iio/dac/ds4424.c
index a0c60eb89717..2d299a52cede 100644
--- a/drivers/iio/dac/ds4424.c
+++ b/drivers/iio/dac/ds4424.c
@@ -220,6 +220,20 @@ static int ds4424_verify_chip(struct iio_dev *indio_dev)
 	return ret;
 }
 
+static int ds4424_init(struct iio_dev *indio_dev)
+{
+	int i, ret;
+
+	/* Set all channels to 0 current. */
+	for (i = 0; i < indio_dev->num_channels; i++) {
+		ret = ds4424_set_value(indio_dev, 0, &indio_dev->channels[i]);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
 static int ds4424_setup_channels(struct i2c_client *client,
 				 struct ds4424_data *data,
 				 struct iio_dev *indio_dev)
@@ -397,6 +411,11 @@ static int ds4424_probe(struct i2c_client *client)
 	if (ret)
 		goto fail;
 
+	/* No reset pin/bit: clear any preconfigured output on probe. */
+	ret = ds4424_init(indio_dev);
+	if (ret)
+		goto fail;
+
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->info = &ds4424_iio_info;
 
-- 
2.47.3

Re: [PATCH v1 6/8] iio: dac: ds4424: clear outputs on probe

[Andy Shevchenko]

On Mon, Jan 19, 2026 at 07:24:22PM +0100, Oleksij Rempel wrote:
> The DS44xx devices have no reset pin or reset bit, so output registers
> may retain preconfigured values across reboot or warm reset.
> 
> Also, the driver suspend/resume path restores from data->raw. When the
> device is first probed, data->raw is zero-initialized and may not match
> the actual hardware state. A later suspend/resume can therefore change an
> output from a preconfigured non-zero value to 0 mA.
> 
> Initialize all channels to 0 output current during probe to ensure a
> deterministic baseline and consistent suspend/resume behavior.

...

> +static int ds4424_init(struct iio_dev *indio_dev)
> +{
> +	int i, ret;

Why is 'i' signed?

> +
> +	/* Set all channels to 0 current. */
> +	for (i = 0; i < indio_dev->num_channels; i++) {

	for (unsigned int i = ...) {

> +		ret = ds4424_set_value(indio_dev, 0, &indio_dev->channels[i]);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	return 0;
> +}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 6/8] iio: dac: ds4424: clear outputs on probe

[Jonathan Cameron]

On Mon, 19 Jan 2026 19:24:22 +0100
Oleksij Rempel <o.rempel@pengutronix.de> wrote:

> The DS44xx devices have no reset pin or reset bit, so output registers
> may retain preconfigured values across reboot or warm reset.
> 
> Also, the driver suspend/resume path restores from data->raw. When the
> device is first probed, data->raw is zero-initialized and may not match
> the actual hardware state. A later suspend/resume can therefore change an
> output from a preconfigured non-zero value to 0 mA.

For DACs we often want to retain settings from before kernel load (or
on exit of the driver).  Can we just read them back from the device to
fill in the cached versions?  If we can I think that would be preferred
option.

Jonathan


> 
> Initialize all channels to 0 output current during probe to ensure a
> deterministic baseline and consistent suspend/resume behavior.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  drivers/iio/dac/ds4424.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/iio/dac/ds4424.c b/drivers/iio/dac/ds4424.c
> index a0c60eb89717..2d299a52cede 100644
> --- a/drivers/iio/dac/ds4424.c
> +++ b/drivers/iio/dac/ds4424.c
> @@ -220,6 +220,20 @@ static int ds4424_verify_chip(struct iio_dev *indio_dev)
>  	return ret;
>  }
>  
> +static int ds4424_init(struct iio_dev *indio_dev)
> +{
> +	int i, ret;
> +
> +	/* Set all channels to 0 current. */
> +	for (i = 0; i < indio_dev->num_channels; i++) {
> +		ret = ds4424_set_value(indio_dev, 0, &indio_dev->channels[i]);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
>  static int ds4424_setup_channels(struct i2c_client *client,
>  				 struct ds4424_data *data,
>  				 struct iio_dev *indio_dev)
> @@ -397,6 +411,11 @@ static int ds4424_probe(struct i2c_client *client)
>  	if (ret)
>  		goto fail;
>  
> +	/* No reset pin/bit: clear any preconfigured output on probe. */
> +	ret = ds4424_init(indio_dev);
> +	if (ret)
> +		goto fail;
> +
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  	indio_dev->info = &ds4424_iio_info;
>  

Re: [PATCH v1 6/8] iio: dac: ds4424: clear outputs on probe

[Oleksij Rempel]

On Fri, Jan 23, 2026 at 09:38:04AM +0000, Jonathan Cameron wrote:
> On Mon, 19 Jan 2026 19:24:22 +0100
> Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> 
> > The DS44xx devices have no reset pin or reset bit, so output registers
> > may retain preconfigured values across reboot or warm reset.
> > 
> > Also, the driver suspend/resume path restores from data->raw. When the
> > device is first probed, data->raw is zero-initialized and may not match
> > the actual hardware state. A later suspend/resume can therefore change an
> > output from a preconfigured non-zero value to 0 mA.
> 
> For DACs we often want to retain settings from before kernel load (or
> on exit of the driver).  Can we just read them back from the device to
> fill in the cached versions?  If we can I think that would be preferred
> option.

ACK, i was not sure what is the best way here.

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 |

[PATCH v1 7/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.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 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 2d299a52cede..13acb427d43b 100644
--- a/drivers/iio/dac/ds4424.c
+++ b/drivers/iio/dac/ds4424.c
@@ -148,8 +148,9 @@ static int ds4424_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_RAW:
 		ret = ds4424_get_value(indio_dev, val, chan->channel);
 		if (ret < 0) {
-			pr_err("%s : ds4424_get_value returned %d\n",
-							__func__, ret);
+			dev_err_ratelimited(&indio_dev->dev,
+					    "%s: ds4424_get_value failed %pe\n",
+					    __func__, ERR_PTR(ret));
 			return ret;
 		}
 		raw.bits = *val;
-- 
2.47.3

Re: [PATCH v1 7/8] iio: dac: ds4424: ratelimit read errors and use device context

[Andy Shevchenko]

On Mon, Jan 19, 2026 at 07:24:23PM +0100, Oleksij Rempel wrote:
> 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.

...

> -			pr_err("%s : ds4424_get_value returned %d\n",
> -							__func__, ret);
> +			dev_err_ratelimited(&indio_dev->dev,
> +					    "%s: ds4424_get_value failed %pe\n",
> +					    __func__, ERR_PTR(ret));

Too many information in the output now. __func__ obviously is redundant from
pr_*() limits, since you have now device, no need to have that, I believe. In
general, if you want to use __func__ it's most likely that the message itself
is badly written (not so unique). Rephrasing helps. Also "ds4424_get_value()"
will hint the user that this is about returned value from the call to the
function.

>  			return ret;

-- 
With Best Regards,
Andy Shevchenko

[PATCH v1 8/8] iio: dac: ds4424: document output sign and probe verification

[Oleksij Rempel]

Document that a zero output value is encoded with a don't-care sign bit
and that the driver verifies device presence via a readable register
since the hardware provides no device ID.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/iio/dac/ds4424.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iio/dac/ds4424.c b/drivers/iio/dac/ds4424.c
index 13acb427d43b..2ccac674878e 100644
--- a/drivers/iio/dac/ds4424.c
+++ b/drivers/iio/dac/ds4424.c
@@ -194,6 +194,7 @@ static int ds4424_write_raw(struct iio_dev *indio_dev,
 		if (val < -max_val || val > max_val)
 			return -EINVAL;
 
+		/* Canonicalize 0 to sink; datasheet treats sign as don't-care. */
 		if (val > 0) {
 			raw.source_bit = DS4424_SOURCE_I;
 			raw.dx = val;
@@ -213,6 +214,7 @@ static int ds4424_verify_chip(struct iio_dev *indio_dev)
 {
 	int ret, val;
 
+	/* No device ID; verify presence by a readable register. */
 	ret = ds4424_get_value(indio_dev, &val, 0);
 	if (ret < 0)
 		dev_err(&indio_dev->dev,
-- 
2.47.3