linux-kernel - [PATCH v4 02/13] iio: dac: ds4424: refactor raw access to use bitwise operations

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20260203093434.2548978-3-o.rempel@pengutronix.de>
Date: Tue,  3 Feb 2026 10:34:22 +0100
From: Oleksij Rempel <o.rempel@...gutronix.de>
To: Jonathan Cameron <jic23@...nel.org>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>
Cc: Oleksij Rempel <o.rempel@...gutronix.de>,
	Andy Shevchenko <andriy.shevchenko@...el.com>,
	kernel@...gutronix.de,
	linux-kernel@...r.kernel.org,
	linux-iio@...r.kernel.org,
	devicetree@...r.kernel.org,
	Andy Shevchenko <andy@...nel.org>,
	David Lechner <dlechner@...libre.com>,
	Nuno Sá <nuno.sa@...log.com>,
	David Jander <david@...tonic.nl>
Subject: [PATCH v4 02/13] iio: dac: ds4424: refactor raw access to use bitwise operations

Refactor the raw access logic to use standard GENMASK() and BIT()
macros. Use abs() for magnitude calculation to simplify the logic and
make the data flow clearer.

Signed-off-by: Oleksij Rempel <o.rempel@...gutronix.de>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@...el.com>
---
changes v4:
- Split patch: The functional fix for -128 was moved to the previous
  patch (1/9) for stable backporting.
- This patch now contains only the refactoring (GENMASK/BIT/abs) on top
  of the fix.
changes v3:
- Remove "Rebase on top of regmap" note as this is now patch 1/8.
- Add #include <linux/bits.h>
- Clarify 0mA sink/source behavior in comments
- Remove redundant blank line in write_raw
changes v2:
- Replace S8_MIN/MAX checks with abs() > DS4424_DAC_MASK to enforce the
  correct [-127, 127] physical range.
- Refactor read_raw/write_raw to use symmetrical bitwise operations,
  removing the custom bitfield union.
- Rebase on top of regmap port
---
 drivers/iio/dac/ds4424.c | 55 +++++++++++++++-------------------------
 1 file changed, 21 insertions(+), 34 deletions(-)

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