Thread (24 messages) 24 messages, 5 authors, 2026-01-26

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

From: Andy Shevchenko <hidden>
Date: 2026-01-19 19:11:29
Also in: linux-iio, lkml

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

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help