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