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.3On 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
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 |
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
© 2016 - 2026 Red Hat, Inc.