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

[PATCH v2 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.

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: sort headers alphabetically
  iio: dac: ds4424: convert to regmap
  iio: dac: ds4424: fix -128 rejection and refactor raw access
  iio: dac: ds4424: add Rfs-based scale and per-variant limits
  iio: dac: ds4424: ratelimit read errors and use device context

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

--
2.47.3

[PATCH v2 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>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
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 v2 2/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 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        | 36 +++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml b/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml
index efe63e6cb55d..400afd8771aa 100644
--- a/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml
+++ b/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml
@@ -27,9 +27,44 @@ 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).
+
 required:
   - compatible
   - reg
+  - maxim,rfs-ohms
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - maxim,ds4402
+              - maxim,ds4422
+    then:
+      properties:
+        maxim,rfs-ohms:
+          minItems: 2
+          maxItems: 2
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - maxim,ds4404
+              - maxim,ds4424
+    then:
+      properties:
+        maxim,rfs-ohms:
+          minItems: 4
+          maxItems: 4
 
 additionalProperties: false
 
@@ -43,6 +78,7 @@ examples:
             compatible = "maxim,ds4424";
             reg = <0x10>; /* When A0, A1 pins are ground */
             vcc-supply = <&vcc_3v3>;
+            maxim,rfs-ohms = <5100>, <5100>, <5100>, <5100>;
         };
     };
 ...
-- 
2.47.3

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

[Conor Dooley]

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

On Tue, Jan 27, 2026 at 07:09:33AM +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>
> ---
> 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        | 36 +++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml b/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml
> index efe63e6cb55d..400afd8771aa 100644
> --- a/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml
> +++ b/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml
> @@ -27,9 +27,44 @@ 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).

Add here
maxItems: 4
minItems: 2

Are 40kOhm and 160kOhm actual max/min values, or just a suggested range?

> +
>  required:
>    - compatible
>    - reg
> +  - maxim,rfs-ohms
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - maxim,ds4402
> +              - maxim,ds4422
> +    then:
> +      properties:
> +        maxim,rfs-ohms:

> +          minItems: 2

Remove this...

> +          maxItems: 2
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - maxim,ds4404
> +              - maxim,ds4424
> +    then:
> +      properties:
> +        maxim,rfs-ohms:
> +          minItems: 4


> +          maxItems: 4

...and this. The outer-most constraints should be in the definition and
the if/else bit just adjusts whichever bounds it needs to.

Cheers,
Conor.

pw-bot: changes-requested

>  
>  additionalProperties: false
>  
> @@ -43,6 +78,7 @@ examples:
>              compatible = "maxim,ds4424";
>              reg = <0x10>; /* When A0, A1 pins are ground */
>              vcc-supply = <&vcc_3v3>;
> +            maxim,rfs-ohms = <5100>, <5100>, <5100>, <5100>;
>          };
>      };
>  ...
> -- 
> 2.47.3
> 

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

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

[Conor Dooley]

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

On Tue, Jan 27, 2026 at 07:49:20PM +0000, Conor Dooley wrote:
> On Tue, Jan 27, 2026 at 07:09:33AM +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>
> > ---
> > 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        | 36 +++++++++++++++++++
> >  1 file changed, 36 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml b/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml
> > index efe63e6cb55d..400afd8771aa 100644
> > --- a/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml
> > +++ b/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml
> > @@ -27,9 +27,44 @@ 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).
> 
> Add here
> maxItems: 4
> minItems: 2
> 
> Are 40kOhm and 160kOhm actual max/min values, or just a suggested range?

Datasheet for the ds4424 seems to imply they're actual max/min values:
"Input resistors (RFS) must be between the speciifed values to ensure the
device meets its accuracy and linearity specifications."
In that case, consider also adding something like:
    items:
      maximum: 40000
      minimum: 160000

Although, that would complain about the 5100 Ohms you're using. How come
the example lies outside of the "typical" range?

> > +
> >  required:
> >    - compatible
> >    - reg
> > +  - maxim,rfs-ohms
> > +
> > +allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - maxim,ds4402
> > +              - maxim,ds4422
> > +    then:
> > +      properties:
> > +        maxim,rfs-ohms:
> 
> > +          minItems: 2
> 
> Remove this...
> 
> > +          maxItems: 2
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - maxim,ds4404
> > +              - maxim,ds4424
> > +    then:
> > +      properties:
> > +        maxim,rfs-ohms:
> > +          minItems: 4
> 
> 
> > +          maxItems: 4
> 
> ...and this. The outer-most constraints should be in the definition and
> the if/else bit just adjusts whichever bounds it needs to.
> 
> Cheers,
> Conor.
> 
> pw-bot: changes-requested
> 
> >  
> >  additionalProperties: false
> >  
> > @@ -43,6 +78,7 @@ examples:
> >              compatible = "maxim,ds4424";
> >              reg = <0x10>; /* When A0, A1 pins are ground */
> >              vcc-supply = <&vcc_3v3>;
> > +            maxim,rfs-ohms = <5100>, <5100>, <5100>, <5100>;
> >          };
> >      };
> >  ...
> > -- 
> > 2.47.3
> > 



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

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

[David Jander]

On Tue, 27 Jan 2026 19:55:26 +0000
Conor Dooley <conor@kernel.org> wrote:

> On Tue, Jan 27, 2026 at 07:49:20PM +0000, Conor Dooley wrote:
> > On Tue, Jan 27, 2026 at 07:09:33AM +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>
> > > ---
> > > 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        | 36 +++++++++++++++++++
> > >  1 file changed, 36 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml b/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml
> > > index efe63e6cb55d..400afd8771aa 100644
> > > --- a/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml
> > > +++ b/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml
> > > @@ -27,9 +27,44 @@ 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).  
> > 
> > Add here
> > maxItems: 4
> > minItems: 2
> > 
> > Are 40kOhm and 160kOhm actual max/min values, or just a suggested range?  
> 
> Datasheet for the ds4424 seems to imply they're actual max/min values:
> "Input resistors (RFS) must be between the speciifed values to ensure the
> device meets its accuracy and linearity specifications."
> In that case, consider also adding something like:
>     items:
>       maximum: 40000
>       minimum: 160000
> 
> Although, that would complain about the 5100 Ohms you're using. How come
> the example lies outside of the "typical" range?

Sorry to chime in here out of nowhere with this, but 2 things:
1. Rfs for DS4402/4 has a different "typical" range than DS4424 (different
Vref and different output current range).
2. "Typical" or "recommended" ranges should not translate to a hard limit in
the driver. IMHO, no max or min value should be enforced here.

> > > +
> > >  required:
> > >    - compatible
> > >    - reg
> > > +  - maxim,rfs-ohms
> > > +
> > > +allOf:
> > > +  - if:
> > > +      properties:
> > > +        compatible:
> > > +          contains:
> > > +            enum:
> > > +              - maxim,ds4402
> > > +              - maxim,ds4422
> > > +    then:
> > > +      properties:
> > > +        maxim,rfs-ohms:  
> >   
> > > +          minItems: 2  
> > 
> > Remove this...
> >   
> > > +          maxItems: 2
> > > +  - if:
> > > +      properties:
> > > +        compatible:
> > > +          contains:
> > > +            enum:
> > > +              - maxim,ds4404
> > > +              - maxim,ds4424
> > > +    then:
> > > +      properties:
> > > +        maxim,rfs-ohms:
> > > +          minItems: 4  
> > 
> >   
> > > +          maxItems: 4  
> > 
> > ...and this. The outer-most constraints should be in the definition and
> > the if/else bit just adjusts whichever bounds it needs to.
> > 
> > Cheers,
> > Conor.
> > 
> > pw-bot: changes-requested
> >   
> > >  
> > >  additionalProperties: false
> > >  
> > > @@ -43,6 +78,7 @@ examples:
> > >              compatible = "maxim,ds4424";
> > >              reg = <0x10>; /* When A0, A1 pins are ground */
> > >              vcc-supply = <&vcc_3v3>;
> > > +            maxim,rfs-ohms = <5100>, <5100>, <5100>, <5100>;
> > >          };
> > >      };
> > >  ...
> > > -- 
> > > 2.47.3
> > >   
> 
> 



-- 
David Jander

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

[Conor Dooley]

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

On Wed, Jan 28, 2026 at 09:01:18AM +0100, David Jander wrote:
> On Tue, 27 Jan 2026 19:55:26 +0000
> Conor Dooley <conor@kernel.org> wrote:
> 
> > On Tue, Jan 27, 2026 at 07:49:20PM +0000, Conor Dooley wrote:
> > > On Tue, Jan 27, 2026 at 07:09:33AM +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>
> > > > ---
> > > > 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        | 36 +++++++++++++++++++
> > > >  1 file changed, 36 insertions(+)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml b/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml
> > > > index efe63e6cb55d..400afd8771aa 100644
> > > > --- a/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml
> > > > +++ b/Documentation/devicetree/bindings/iio/dac/maxim,ds4424.yaml
> > > > @@ -27,9 +27,44 @@ 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).  
> > > 
> > > Add here
> > > maxItems: 4
> > > minItems: 2
> > > 
> > > Are 40kOhm and 160kOhm actual max/min values, or just a suggested range?  
> > 
> > Datasheet for the ds4424 seems to imply they're actual max/min values:
> > "Input resistors (RFS) must be between the speciifed values to ensure the
> > device meets its accuracy and linearity specifications."
> > In that case, consider also adding something like:
> >     items:
> >       maximum: 40000
> >       minimum: 160000
> > 
> > Although, that would complain about the 5100 Ohms you're using. How come
> > the example lies outside of the "typical" range?
> 
> Sorry to chime in here out of nowhere with this, but 2 things:

No no, don't apologise - that's good info and makes sense. Thanks.

> 1. Rfs for DS4402/4 has a different "typical" range than DS4424 (different
> Vref and different output current range).
> 2. "Typical" or "recommended" ranges should not translate to a hard limit in
> the driver. IMHO, no max or min value should be enforced here.

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

[PATCH v2 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>
---
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 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

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

[Andy Shevchenko]

On Tue, Jan 27, 2026 at 07:09:34AM +0100, Oleksij Rempel wrote:
> 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>

Unclear who that is. Co-developed-by? Original author? The From is set to your
address, should it be otherwise?

> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>

-- 
With Best Regards,
Andy Shevchenko

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

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
changes v2:
- new patch
---
 drivers/iio/dac/ds4424.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/dac/ds4424.c b/drivers/iio/dac/ds4424.c
index 072b7e6672cf..a4f00045f17b 100644
--- a/drivers/iio/dac/ds4424.c
+++ b/drivers/iio/dac/ds4424.c
@@ -5,16 +5,16 @@
  * Copyright (C) 2017 Maxim Integrated
  */
 
-#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/err.h>
+#include <linux/i2c.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>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/regulator/consumer.h>
 
 #define DS4422_MAX_DAC_CHANNELS		2
 #define DS4424_MAX_DAC_CHANNELS		4
-- 
2.47.3

Re: [PATCH v2 4/8] iio: dac: ds4424: sort headers alphabetically

[Andy Shevchenko]

On Tue, Jan 27, 2026 at 07:09:35AM +0100, Oleksij Rempel wrote:
> Sort the header inclusions alphabetically. This improves readability and
> simplifies adding new includes in the future.

...

> -#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/err.h>
> +#include <linux/i2c.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>

This was at the end and I assume we want this to be kept at the end as a
separate group of linux/iio/* to emphasize on the fact that the driver is
related to that subsystem.

> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/regulator/consumer.h>

With that being said, I think the result should look as

#include <linux/delay.h>
#include <linux/err.h>
#include <linux/i2c.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/regulator/consumer.h>

#include <linux/iio/consumer.h>
#include <linux/iio/driver.h>
#include <linux/iio/iio.h>
#include <linux/iio/machine.h>

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 5/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 v2:
- new patch
---
 drivers/iio/dac/Kconfig  |   1 +
 drivers/iio/dac/ds4424.c | 163 +++++++++++++++++++++++----------------
 2 files changed, 96 insertions(+), 68 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 a4f00045f17b..f9ab7f4b97ff 100644
--- a/drivers/iio/dac/ds4424.c
+++ b/drivers/iio/dac/ds4424.c
@@ -14,6 +14,7 @@
 #include <linux/iio/machine.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 
 #define DS4422_MAX_DAC_CHANNELS		2
@@ -55,11 +56,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[] = {
@@ -69,59 +67,80 @@ 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_FLAT,
+	.max_register = DS4424_DAC_ADDR(1),
+	.rd_table = &ds44x2_table,
+	.wr_table = &ds44x2_table,
+};
+
+static const struct regmap_config ds44x4_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.cache_type = REGCACHE_FLAT,
+	.max_register = DS4424_DAC_ADDR(3),
+	.rd_table = &ds44x4_table,
+	.wr_table = &ds44x4_table,
+};
+
+static int ds4424_init_regmap(struct i2c_client *client,
+			      struct iio_dev *indio_dev)
 {
 	struct ds4424_data *data = iio_priv(indio_dev);
-	int ret;
+	const struct regmap_config *regmap_config;
 
-	mutex_lock(&data->lock);
-	ret = i2c_smbus_write_byte_data(data->client,
-			DS4424_DAC_ADDR(chan->channel), val);
-	if (ret < 0)
-		goto fail;
+	if (indio_dev->num_channels == DS4424_MAX_DAC_CHANNELS)
+		regmap_config = &ds44x4_regmap_config;
+	else
+		regmap_config = &ds44x2_regmap_config;
 
-	data->raw[chan->channel] = val;
+	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");
 
-fail:
-	mutex_unlock(&data->lock);
-	return ret;
+	return 0;
 }
 
 static int ds4424_read_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);
 	union ds4424_raw_data raw;
+	unsigned int regval;
 	int ret;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		ret = ds4424_get_value(indio_dev, val, chan->channel);
+		ret = regmap_read(data->regmap, DS4424_DAC_ADDR(chan->channel),
+				  &regval);
 		if (ret < 0) {
-			pr_err("%s : ds4424_get_value returned %d\n",
-							__func__, ret);
+			pr_err("%s : regmap_read returned %d\n",
+						__func__, ret);
 			return ret;
 		}
-		raw.bits = *val;
+		raw.bits = regval;
 		*val = raw.dx;
 		if (raw.source_bit == DS4424_SINK_I)
 			*val = -*val;
@@ -136,6 +155,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);
 	union ds4424_raw_data raw;
 
 	if (val2 != 0)
@@ -154,7 +174,8 @@ static int ds4424_write_raw(struct iio_dev *indio_dev,
 			raw.dx = -val;
 		}
 
-		return ds4424_set_value(indio_dev, raw.bits, chan);
+		return regmap_write(data->regmap, DS4424_DAC_ADDR(chan->channel),
+				    raw.bits);
 
 	default:
 		return -EINVAL;
@@ -163,49 +184,52 @@ static int ds4424_write_raw(struct iio_dev *indio_dev,
 
 static int ds4424_verify_chip(struct iio_dev *indio_dev)
 {
-	int ret, val;
+	struct ds4424_data *data = iio_priv(indio_dev);
+	u8 raw_values[DS4424_MAX_DAC_CHANNELS];
+	int ret;
 
-	ret = ds4424_get_value(indio_dev, &val, 0);
-	if (ret < 0)
-		dev_err(&indio_dev->dev,
-				"%s failed. ret: %d\n", __func__, ret);
+	/* Bulk read all channels starting at 0xf8.
+	 * This populates the regmap cache with current HW values.
+	 */
+	ret = regmap_bulk_read(data->regmap, DS4424_DAC_ADDR(0),
+			       raw_values, indio_dev->num_channels);
+	if (ret)
+		return dev_err_probe(&indio_dev->dev, ret, "Failed to seed cache\n");
 
-	return ret;
+	return 0;
 }
 
 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)
+	int ret;
+
+	/* Disable all outputs, bypass cache so the '0' isn't saved */
+	regcache_cache_bypass(data->regmap, true);
+	for (unsigned int i = 0; i < indio_dev->num_channels; i++) {
+		ret = regmap_write(data->regmap, DS4424_DAC_ADDR(i), 0);
+		if (ret) {
+			dev_err(dev, "Failed to zero channel %d: %d\n", i, ret);
+			regcache_cache_bypass(data->regmap, false);
 			return ret;
+		}
 	}
-	return ret;
+	regcache_cache_bypass(data->regmap, false);
+
+	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);
@@ -228,7 +252,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");
@@ -236,7 +259,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,
@@ -245,9 +267,6 @@ static int ds4424_probe(struct i2c_client *client)
 	}
 
 	usleep_range(1000, 1200);
-	ret = ds4424_verify_chip(indio_dev);
-	if (ret < 0)
-		goto fail;
 
 	switch (id->driver_data) {
 	case ID_DS4402:
@@ -269,6 +288,14 @@ static int ds4424_probe(struct i2c_client *client)
 		goto fail;
 	}
 
+	ret = ds4424_init_regmap(client, indio_dev);
+	if (ret < 0)
+		goto fail;
+
+	ret = ds4424_verify_chip(indio_dev);
+	if (ret < 0)
+		goto fail;
+
 	indio_dev->channels = ds4424_channels;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->info = &ds4424_info;
-- 
2.47.3

Re: [PATCH v2 5/8] iio: dac: ds4424: convert to regmap

[Andy Shevchenko]

On Tue, Jan 27, 2026 at 07:09:36AM +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.

...

> +static const struct regmap_access_table ds44x4_table = {
> +	.yes_ranges = ds44x4_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(ds44x4_ranges),

+ array_size.h

> +};

...

> +		ret = regmap_read(data->regmap, DS4424_DAC_ADDR(chan->channel),
> +				  &regval);
>  		if (ret < 0) {
> -			pr_err("%s : ds4424_get_value returned %d\n",
> -							__func__, ret);
> +			pr_err("%s : regmap_read returned %d\n",
> +						__func__, ret);

This should be dev_err() to begin with. Perhaps you want a new patch for that.

>  			return ret;
>  		}

...

> +	/* Bulk read all channels starting at 0xf8.
> +	 * This populates the regmap cache with current HW values.
> +	 */

/*
 * Use proper style for multi-line
 * comments.
 */

...

> +	if (ret)
> +		return dev_err_probe(&indio_dev->dev, ret, "Failed to seed cache\n");

Why not physical device? I assume during probe we use physical device, when we
do IIO callbacks, we use IIO device.

...

>  static int ds4424_suspend(struct device *dev)
>  {
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct ds4424_data *data = iio_priv(indio_dev);
> +	int ret;
> +
> +	/* Disable all outputs, bypass cache so the '0' isn't saved */
> +	regcache_cache_bypass(data->regmap, true);
> +	for (unsigned int i = 0; i < indio_dev->num_channels; i++) {
> +		ret = regmap_write(data->regmap, DS4424_DAC_ADDR(i), 0);
> +		if (ret) {
> +			dev_err(dev, "Failed to zero channel %d: %d\n", i, ret);

%u for 'i'.

> +			regcache_cache_bypass(data->regmap, false);
>  			return ret;
> +		}
>  	}
> +	regcache_cache_bypass(data->regmap, false);
> +
> +	regcache_cache_only(data->regmap, true);
> +	regcache_mark_dirty(data->regmap);
> +
> +	return 0;
>  }

...

>  	usleep_range(1000, 1200);

Side note: Perhaps fsleep() in the future...

> +	ret = ds4424_init_regmap(client, indio_dev);
> +	if (ret < 0)

Do we need ' < 0' part?

> +		goto fail;
> +
> +	ret = ds4424_verify_chip(indio_dev);
> +	if (ret < 0)

Ditto.

> +		goto fail;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 5/8] iio: dac: ds4424: convert to regmap

[Sander Vanheule]

Hi Oleksij,

On Tue, 2026-01-27 at 07:09 +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).


[...]

> +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_FLAT,
> +	.max_register = DS4424_DAC_ADDR(1),
> +	.rd_table = &ds44x2_table,
> +	.wr_table = &ds44x2_table,
> +};

Note that REGCACHE_FLAT will allocate 0xF8 unsigned longs you will never use.
REGCACHE_MAPLE will probably be much closer to the size of the original value
cache, for a small look-up performance penalty (but always fast compared to the
I2C bus).

[...]

> @@ -163,49 +184,52 @@ static int ds4424_write_raw(struct iio_dev *indio_dev,
>  
>  static int ds4424_verify_chip(struct iio_dev *indio_dev)
>  {
> -	int ret, val;
> +	struct ds4424_data *data = iio_priv(indio_dev);
> +	u8 raw_values[DS4424_MAX_DAC_CHANNELS];
> +	int ret;
>  
> -	ret = ds4424_get_value(indio_dev, &val, 0);
> -	if (ret < 0)
> -		dev_err(&indio_dev->dev,
> -				"%s failed. ret: %d\n", __func__, ret);
> +	/* Bulk read all channels starting at 0xf8.
> +	 * This populates the regmap cache with current HW values.
> +	 */
> +	ret = regmap_bulk_read(data->regmap, DS4424_DAC_ADDR(0),
> +			       raw_values, indio_dev->num_channels);

Are you forcing a HW-to-cache sync for performance?

Previously this function would just read a single value to verify a reply was
sent, instead of seeding the cache in data->raw, so that's a change in behavior
(and purpose) of this function, meaning you may want to change the name if you
keep this.

You could (should IMHO) use a sparse cache, which is actually aware of its
content's validity and will transparently access the device on the first access
to initialize itself. I would recommend REGCACHE_MAPLE as above, but
REGCACHE_FLAT_S also works. REGCACHE_FLAT is tricky to properly initialize [1],
and I would not recommend using it in new code.

Using a single regmap_read() here (as ds4424_get_value() before) with a sparse
cache would result in an error if there was a bus error, giving you the
verification behavior again.

[1] https://patch.msgid.link/20251023135032.229511-1-sander@svanheule.net/

Best,
Sander

Re: [PATCH v2 5/8] iio: dac: ds4424: convert to regmap

[Oleksij Rempel]

Hi Sander,

On Sun, Feb 01, 2026 at 03:42:28PM +0100, Sander Vanheule wrote:
> Hi Oleksij,
> 
> On Tue, 2026-01-27 at 07:09 +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).
> 
> 
> [...]
> 
> > +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_FLAT,
> > +	.max_register = DS4424_DAC_ADDR(1),
> > +	.rd_table = &ds44x2_table,
> > +	.wr_table = &ds44x2_table,
> > +};
> 
> Note that REGCACHE_FLAT will allocate 0xF8 unsigned longs you will never use.
> REGCACHE_MAPLE will probably be much closer to the size of the original value
> cache, for a small look-up performance penalty (but always fast compared to the
> I2C bus).

ACK, already migrated to REGCACHE_MAPLE in the v3:
https://lore.kernel.org/all/20260128153824.3679187-8-o.rempel@pengutronix.de/

Which works mostly fine except of the cache initialisation. If I use
num_reg_defaults_raw with REGCACHE_MAPLE as proposed by Andy
Shevchenko, first access to regmap values over debugfs will explode with
NULL pointer etc...
If I remove num_reg_defaults_raw, I need to read register manually
to init defaul values as implemented in v3.

The REGCACHE_FLAT has one more problem, the cache will be inited with
i2c NACKs (0xFF) if used with num_reg_defaults_raw.

> [...]
> 
> > @@ -163,49 +184,52 @@ static int ds4424_write_raw(struct iio_dev *indio_dev,
> >  
> >  static int ds4424_verify_chip(struct iio_dev *indio_dev)
> >  {
> > -	int ret, val;
> > +	struct ds4424_data *data = iio_priv(indio_dev);
> > +	u8 raw_values[DS4424_MAX_DAC_CHANNELS];
> > +	int ret;
> >  
> > -	ret = ds4424_get_value(indio_dev, &val, 0);
> > -	if (ret < 0)
> > -		dev_err(&indio_dev->dev,
> > -				"%s failed. ret: %d\n", __func__, ret);
> > +	/* Bulk read all channels starting at 0xf8.
> > +	 * This populates the regmap cache with current HW values.
> > +	 */
> > +	ret = regmap_bulk_read(data->regmap, DS4424_DAC_ADDR(0),
> > +			       raw_values, indio_dev->num_channels);
> 
> Are you forcing a HW-to-cache sync for performance?

See response above.

> Previously this function would just read a single value to verify a reply was
> sent, instead of seeding the cache in data->raw, so that's a change in behavior
> (and purpose) of this function, meaning you may want to change the name if you
> keep this.
> 
> You could (should IMHO) use a sparse cache, which is actually aware of its
> content's validity and will transparently access the device on the first access
> to initialize itself. I would recommend REGCACHE_MAPLE as above, but
> REGCACHE_FLAT_S also works. REGCACHE_FLAT is tricky to properly initialize [1],
> and I would not recommend using it in new code.

Ok, thx!

-- 
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 v2 5/8] iio: dac: ds4424: convert to regmap

[Sander Vanheule]

Hi Oleksij,

Thanks for the quick reply!

On Sun, 2026-02-01 at 17:16 +0100, Oleksij Rempel wrote:
> Hi Sander,
> 
> On Sun, Feb 01, 2026 at 03:42:28PM +0100, Sander Vanheule wrote:
> > Hi Oleksij,
> > 
> > On Tue, 2026-01-27 at 07:09 +0100, Oleksij Rempel wrote:
> > > +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_FLAT,
> > > + .max_register = DS4424_DAC_ADDR(1),
> > > + .rd_table = &ds44x2_table,
> > > + .wr_table = &ds44x2_table,
> > > +};
> > 
> > Note that REGCACHE_FLAT will allocate 0xF8 unsigned longs you will never
> > use.
> > REGCACHE_MAPLE will probably be much closer to the size of the original
> > value
> > cache, for a small look-up performance penalty (but always fast compared to
> > the
> > I2C bus).
> 
> ACK, already migrated to REGCACHE_MAPLE in the v3:
> https://lore.kernel.org/all/20260128153824.3679187-8-o.rempel@pengutronix.de/

Thanks for the link. I'm not on the IIO list, so I hadn't seen this version yet.

> Which works mostly fine except of the cache initialisation. If I use
> num_reg_defaults_raw with REGCACHE_MAPLE as proposed by Andy
> Shevchenko, first access to regmap values over debugfs will explode with
> NULL pointer etc...
> If I remove num_reg_defaults_raw, I need to read register manually
> to init defaul values as implemented in v3.

num_reg_defaults_raw without a raw register value buffer is also problematic
IMHO. If it is used without a buffer, values will be read from hardware. This is
indeed widely used to prime a (flat) cache, but if you want to maintain the
documented behavior of regmap_sync() it should only be used if the hardware
really just came out of reset.

Not sure how likely it is that not all channels values are written after probe,
but for this driver specifically you might see:

   1. Driver probes with num_reg_defaults_raw, using current HW values as
      initial cache _and_ register defaults.
   2. No updates were written before device is suspended. DAC registers are set
      to 0, cache marked dirty.
   3. No updates were written before device comes out of suspend and
      regmap_sync() is called. regmap_sync() sees a cache state which is still
      identical to the register defaults and omits (all) writes.

After 3. your device is considered online again, but the HW registers are still
set to 0 and your cache is out-of-sync.

This also made me realize you do want to read all the channels at some point to
be able to resume to the same state after suspending.


> The REGCACHE_FLAT has one more problem, the cache will be inited with
> i2c NACKs (0xFF) if used with num_reg_defaults_raw.

That's rather nasty. I don't suppose you investigated why that's happening? (not
saying you should, just wondering)

Best,
Sander

Re: [PATCH v2 5/8] iio: dac: ds4424: convert to regmap

[Andy Shevchenko]

On Sun, Feb 01, 2026 at 05:16:53PM +0100, Oleksij Rempel wrote:
> On Sun, Feb 01, 2026 at 03:42:28PM +0100, Sander Vanheule wrote:
> > On Tue, 2026-01-27 at 07:09 +0100, Oleksij Rempel wrote:

...

> Which works mostly fine except of the cache initialisation. If I use
> num_reg_defaults_raw with REGCACHE_MAPLE as proposed by Andy
> Shevchenko, first access to regmap values over debugfs will explode with
> NULL pointer etc...

Yeah, this is known issue in regcache core. I don't remember how it was worked
around in pinctrl-cy8c95x0.c.

> If I remove num_reg_defaults_raw, I need to read register manually
> to init defaul values as implemented in v3.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 5/8] iio: dac: ds4424: convert to regmap

[Andy Shevchenko]

On Tue, Feb 03, 2026 at 12:10:29PM +0200, Andy Shevchenko wrote:
> On Sun, Feb 01, 2026 at 05:16:53PM +0100, Oleksij Rempel wrote:
> > On Sun, Feb 01, 2026 at 03:42:28PM +0100, Sander Vanheule wrote:
> > > On Tue, 2026-01-27 at 07:09 +0100, Oleksij Rempel wrote:

...

> > Which works mostly fine except of the cache initialisation. If I use
> > num_reg_defaults_raw with REGCACHE_MAPLE as proposed by Andy
> > Shevchenko, first access to regmap values over debugfs will explode with
> > NULL pointer etc...
> 
> Yeah, this is known issue in regcache core. I don't remember how it was worked
> around in pinctrl-cy8c95x0.c.

That said, have you run the v6.19-rcX kernels? It should have some patches
related to the regcache, but I need to test and send the actual fix for that
NULL case.

> > If I remove num_reg_defaults_raw, I need to read register manually
> > to init defaul values as implemented in v3.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 6/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 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 | 50 +++++++++++++++-------------------------
 1 file changed, 18 insertions(+), 32 deletions(-)

diff --git a/drivers/iio/dac/ds4424.c b/drivers/iio/dac/ds4424.c
index f9ab7f4b97ff..8110ca7f062f 100644
--- a/drivers/iio/dac/ds4424.c
+++ b/drivers/iio/dac/ds4424.c
@@ -20,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, \
@@ -32,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_DS4402,
 	ID_DS4404,
@@ -127,7 +112,6 @@ static int ds4424_read_raw(struct iio_dev *indio_dev,
 			   int *val, int *val2, long mask)
 {
 	struct ds4424_data *data = iio_priv(indio_dev);
-	union ds4424_raw_data raw;
 	unsigned int regval;
 	int ret;
 
@@ -140,10 +124,11 @@ static int ds4424_read_raw(struct iio_dev *indio_dev,
 						__func__, ret);
 			return ret;
 		}
-		raw.bits = regval;
-		*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:
@@ -156,26 +141,27 @@ static int ds4424_write_raw(struct iio_dev *indio_dev,
 			     int val, int val2, long mask)
 {
 	struct ds4424_data *data = iio_priv(indio_dev);
-	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.
+		 * Canonicalize 0 to sink; datasheet treats sign as don't-care.
+		 */
+		if (val > 0)
+			abs_val |= DS4424_DAC_SOURCE;
 
 		return regmap_write(data->regmap, DS4424_DAC_ADDR(chan->channel),
-				    raw.bits);
+				    abs_val);
 
 	default:
 		return -EINVAL;
-- 
2.47.3

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

[Andy Shevchenko]

On Tue, Jan 27, 2026 at 07:09:37AM +0100, Oleksij Rempel 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")

Usually fixes go first in the series...

...

> +#define DS4424_DAC_MASK			GENMASK(6, 0)
> +#define DS4424_DAC_SOURCE		BIT(7)

+ bits.h ?

...

>  	case IIO_CHAN_INFO_RAW:
> -		if (val < S8_MIN || val > S8_MAX)
> +		abs_val = abs(val);

> +

Redundant blank line.

> +		if (abs_val > DS4424_DAC_MASK)
>  			return -EINVAL;

...

> +		/*
> +		 * Currents exiting the IC (Source) are positive.
> +		 * Canonicalize 0 to sink; datasheet treats sign as don't-care.
> +		 */
> +		if (val > 0)
> +			abs_val |= DS4424_DAC_SOURCE;

Hmm... Maybe 0 should be excluded as invalid?

-- 
With Best Regards,
Andy Shevchenko

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

[Oleksij Rempel]

On Tue, Jan 27, 2026 at 12:42:24PM +0200, Andy Shevchenko wrote:
> On Tue, Jan 27, 2026 at 07:09:37AM +0100, Oleksij Rempel 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")
> > +		/*
> > +		 * Currents exiting the IC (Source) are positive.
> > +		 * Canonicalize 0 to sink; datasheet treats sign as don't-care.
> > +		 */
> > +		if (val > 0)
> > +			abs_val |= DS4424_DAC_SOURCE;
> 
> Hmm... Maybe 0 should be excluded as invalid?

0 is valid value for no current flow (power off). The direction bit
DS4424_DAC_SOURCE  will just make no difference if value is 0.

-- 
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 v2 6/8] iio: dac: ds4424: fix -128 rejection and refactor raw access

[Andy Shevchenko]

On Tue, Jan 27, 2026 at 11:49:05AM +0100, Oleksij Rempel wrote:
> On Tue, Jan 27, 2026 at 12:42:24PM +0200, Andy Shevchenko wrote:
> > On Tue, Jan 27, 2026 at 07:09:37AM +0100, Oleksij Rempel wrote:

...

> > > +		/*
> > > +		 * Currents exiting the IC (Source) are positive.
> > > +		 * Canonicalize 0 to sink; datasheet treats sign as don't-care.
> > > +		 */
> > > +		if (val > 0)
> > > +			abs_val |= DS4424_DAC_SOURCE;
> > 
> > Hmm... Maybe 0 should be excluded as invalid?
> 
> 0 is valid value for no current flow (power off). The direction bit
> DS4424_DAC_SOURCE  will just make no difference if value is 0.

Perhaps elaborate this in the comment above?

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 7/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 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 8110ca7f062f..891069d8c80a 100644
--- a/drivers/iio/dac/ds4424.c
+++ b/drivers/iio/dac/ds4424.c
@@ -14,6 +14,7 @@
 #include <linux/iio/machine.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/property.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 
@@ -21,6 +22,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)
@@ -40,9 +42,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[] = {
@@ -125,11 +156,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;
@@ -150,7 +190,7 @@ static int ds4424_write_raw(struct iio_dev *indio_dev,
 	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;
 
 		/*
@@ -185,6 +225,65 @@ static int ds4424_verify_chip(struct iio_dev *indio_dev)
 	return 0;
 }
 
+static int ds4424_setup_channels(struct i2c_client *client,
+				 struct ds4424_data *data,
+				 struct iio_dev *indio_dev)
+{
+	struct iio_chan_spec *channels;
+	size_t channels_size;
+
+	channels_size = indio_dev->num_channels * sizeof(ds4424_channels[0]);
+	/* Use a local non-const pointer for modification */
+	channels = devm_kmemdup(&client->dev, ds4424_channels, channels_size,
+				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 != 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 %d 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);
@@ -220,7 +319,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,
 };
@@ -257,15 +356,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,
@@ -282,9 +386,16 @@ static int ds4424_probe(struct i2c_client *client)
 	if (ret < 0)
 		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 v2 7/8] iio: dac: ds4424: add Rfs-based scale and per-variant limits

[Andy Shevchenko]

On Tue, Jan 27, 2026 at 07:09:38AM +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 {
> +	int vref_mv;

_mV ?

> +	int scale_denom;
> +	u8 result_mask;
> +};

...

> +static int ds4424_setup_channels(struct i2c_client *client,
> +				 struct ds4424_data *data,
> +				 struct iio_dev *indio_dev)
> +{
> +	struct iio_chan_spec *channels;
> +	size_t channels_size;
> +
> +	channels_size = indio_dev->num_channels * sizeof(ds4424_channels[0]);
> +	/* Use a local non-const pointer for modification */
> +	channels = devm_kmemdup(&client->dev, ds4424_channels, channels_size,
> +				GFP_KERNEL);

Why not devm_kmemdup_array()?

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

Can count be negative?

> +	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 != 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 %d is zero\n",

%u

> +					     i);

I would leave it on the same line.

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

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 8/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>
---
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 891069d8c80a..3e762d4e84ef 100644
--- a/drivers/iio/dac/ds4424.c
+++ b/drivers/iio/dac/ds4424.c
@@ -151,8 +151,9 @@ static int ds4424_read_raw(struct iio_dev *indio_dev,
 		ret = regmap_read(data->regmap, DS4424_DAC_ADDR(chan->channel),
 				  &regval);
 		if (ret < 0) {
-			pr_err("%s : regmap_read returned %d\n",
-						__func__, ret);
+			dev_err_ratelimited(&indio_dev->dev,
+					    "Failed to read channel %d:  %pe\n",
+					    chan->channel, ERR_PTR(ret));
 			return ret;
 		}
 
-- 
2.47.3

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

[Andy Shevchenko]

On Tue, Jan 27, 2026 at 07:09:39AM +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.

This should have been done before touching this line in the other patch.

...

>  		if (ret < 0) {
> -			pr_err("%s : regmap_read returned %d\n",
> -						__func__, ret);
> +			dev_err_ratelimited(&indio_dev->dev,

Why not physical device?

> +					    "Failed to read channel %d:  %pe\n",

Too many spaces.

> +					    chan->channel, ERR_PTR(ret));
>  			return ret;
>  		}

-- 
With Best Regards,
Andy Shevchenko