1. Patchew
  2. linux
  3. [PATCH v3 0/8] iio: dac: ds4424: add DS4402/DS4404 support and scale
  4. View patch

[PATCH v3 7/8] iio: dac: ds4424: convert to regmap

Oleksij Rempel posted 8 patches 1 week, 5 days ago
There is a newer version of this series
[PATCH v3 7/8] iio: dac: ds4424: convert to regmap
Posted by Oleksij Rempel 1 week, 5 days ago 
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 v3:
- Switch to REGCACHE_MAPLE to efficiently handle the sparse register map
  (offset 0xF8) and avoid allocating memory for the unused 0x00-0xF7 range.
- Use explicit regmap_bulk_read() in probe to seed the cache with the
  bootloader configuration. This avoids the invalid read from address 0x00
  that occurred with generic cache defaults.
- Remove ds4424_verify_chip(); devm_regmap_init_i2c() and the subsequent
  bulk read implicitly validate the device presence.
- Use regmap_bulk_write() in ds4424_suspend() to efficiently zero all
  channels.
- Adopt fsleep() for delays and include <linux/array_size.h>.
- Use dev_err_ratelimited() with the physical device context in the read
  path (incorporating feedback aimed at v2 patch 8).
changes v2:
- new patch
---
 drivers/iio/dac/Kconfig  |   1 +
 drivers/iio/dac/ds4424.c | 171 ++++++++++++++++++++++-----------------
 2 files changed, 97 insertions(+), 75 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 f340d491fcc1..9bef1c60b2eb 100644
--- a/drivers/iio/dac/ds4424.c
+++ b/drivers/iio/dac/ds4424.c
@@ -5,12 +5,14 @@
  * Copyright (C) 2017 Maxim Integrated
  */
 
+#include <linux/array_size.h>
 #include <linux/bits.h>
 #include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/i2c.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 
 #include <linux/iio/consumer.h>
@@ -42,11 +44,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[] = {
@@ -56,52 +55,88 @@ 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_MAPLE,
+	.max_register = DS4424_DAC_ADDR(1),
+	.rd_table = &ds44x2_table,
+	.wr_table = &ds44x2_table,
+	/* Seed cache from HW during regmap_init */
+};
+
+static const struct regmap_config ds44x4_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.cache_type = REGCACHE_MAPLE,
+	.max_register = DS4424_DAC_ADDR(3),
+	.rd_table = &ds44x4_table,
+	.wr_table = &ds44x4_table,
+	/* Seed cache from HW during regmap_init */
+};
+
+static int ds4424_init_regmap(struct i2c_client *client,
+			      struct iio_dev *indio_dev)
 {
 	struct ds4424_data *data = iio_priv(indio_dev);
+	const struct regmap_config *regmap_config;
+	u8 vals[DS4424_MAX_DAC_CHANNELS];
 	int ret;
 
-	mutex_lock(&data->lock);
-	ret = i2c_smbus_write_byte_data(data->client,
-			DS4424_DAC_ADDR(chan->channel), val);
-	if (ret < 0)
-		goto fail;
-
-	data->raw[chan->channel] = val;
-
-fail:
-	mutex_unlock(&data->lock);
-	return ret;
+	if (indio_dev->num_channels == DS4424_MAX_DAC_CHANNELS)
+		regmap_config = &ds44x4_regmap_config;
+	else
+		regmap_config = &ds44x2_regmap_config;
+
+	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");
+
+	/*
+	 * Prime the cache with the bootloader's configuration.
+	 * regmap_bulk_read will automatically populate the cache with
+	 * the values read from the hardware.
+	 */
+	ret = regmap_bulk_read(data->regmap, DS4424_DAC_ADDR(0), vals,
+			       indio_dev->num_channels);
+	if (ret)
+		return dev_err_probe(&client->dev, ret,
+				     "Failed to read hardware defaults\n");
+
+	return 0;
 }
 
 static int ds4424_read_raw(struct iio_dev *indio_dev,
 			   struct iio_chan_spec const *chan,
 			   int *val, int *val2, long mask)
 {
-	int ret, regval;
+	struct ds4424_data *data = iio_priv(indio_dev);
+	unsigned int regval;
+	int ret;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		ret = ds4424_get_value(indio_dev, &regval, chan->channel);
+		ret = regmap_read(data->regmap, DS4424_DAC_ADDR(chan->channel),
+				  &regval);
 		if (ret < 0) {
 			dev_err_ratelimited(indio_dev->dev.parent,
 					    "Failed to read channel %d: %pe\n",
@@ -124,6 +159,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);
 	unsigned int abs_val;
 
 	if (val2 != 0)
@@ -143,58 +179,44 @@ static int ds4424_write_raw(struct iio_dev *indio_dev,
 		if (val > 0)
 			abs_val |= DS4424_DAC_SOURCE;
 
-		return ds4424_set_value(indio_dev, abs_val, chan);
+		return regmap_write(data->regmap, DS4424_DAC_ADDR(chan->channel),
+				    abs_val);
 
 	default:
 		return -EINVAL;
 	}
 }
 
-static int ds4424_verify_chip(struct iio_dev *indio_dev)
-{
-	int ret, val;
-
-	ret = ds4424_get_value(indio_dev, &val, 0);
-	if (ret < 0)
-		dev_err(&indio_dev->dev,
-				"%s failed. ret: %d\n", __func__, ret);
-
-	return ret;
-}
-
 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)
-			return ret;
+	u8 zero_buf[DS4424_MAX_DAC_CHANNELS] = { 0 };
+	int ret;
+
+	/* Disable all outputs, bypass cache so the '0' isn't saved */
+	regcache_cache_bypass(data->regmap, true);
+	ret = regmap_bulk_write(data->regmap, DS4424_DAC_ADDR(0),
+				zero_buf, indio_dev->num_channels);
+	regcache_cache_bypass(data->regmap, false);
+	if (ret) {
+		dev_err(dev, "Failed to zero outputs: %pe\n", ERR_PTR(ret));
+		return ret;
 	}
-	return ret;
+
+	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);
@@ -217,7 +239,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");
@@ -225,7 +246,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,
@@ -233,10 +253,7 @@ static int ds4424_probe(struct i2c_client *client)
 		return ret;
 	}
 
-	usleep_range(1000, 1200);
-	ret = ds4424_verify_chip(indio_dev);
-	if (ret < 0)
-		goto fail;
+	fsleep(1000);
 
 	switch (id->driver_data) {
 	case ID_DS4402:
@@ -258,6 +275,10 @@ static int ds4424_probe(struct i2c_client *client)
 		goto fail;
 	}
 
+	ret = ds4424_init_regmap(client, indio_dev);
+	if (ret)
+		goto fail;
+
 	indio_dev->channels = ds4424_channels;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->info = &ds4424_info;
-- 
2.47.3
Re: [PATCH v3 7/8] iio: dac: ds4424: convert to regmap
Posted by Sander Vanheule 1 week, 1 day ago 
Hi Oleksij,

On Wed, 2026-01-28 at 16:38 +0100, Oleksij Rempel wrote:
> +static const struct regmap_config ds44x2_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.cache_type = REGCACHE_MAPLE,
> +	.max_register = DS4424_DAC_ADDR(1),
> +	.rd_table = &ds44x2_table,
> +	.wr_table = &ds44x2_table,
> +	/* Seed cache from HW during regmap_init */

Nit: You're seeding the cache (manually) in ds4424_init_regmap(). But you can
also just drop this comment as far as I'm concerned. The comment in
ds4424_init_regmap() explains it sufficiently.



> +	if (ret)
> +		return dev_err_probe(&client->dev, ret,
> +				     "Failed to read hardware defaults\n");

Nit: "hardware defaults" -> "hardware values"


Nothing too serious from my side, so FWIW, with these things addressed:

Reviewed-by: Sander Vanheule <sander@svanheule.net>

Best,
Sander
Re: [PATCH v3 7/8] iio: dac: ds4424: convert to regmap
Posted by Jonathan Cameron 1 week, 4 days ago 
On Wed, 28 Jan 2026 16:38:23 +0100
Oleksij Rempel <o.rempel@pengutronix.de> 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.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Hi Oleksij

A few comments that might not overlap with what Andy already provided.

> diff --git a/drivers/iio/dac/ds4424.c b/drivers/iio/dac/ds4424.c
> index f340d491fcc1..9bef1c60b2eb 100644
> --- a/drivers/iio/dac/ds4424.c
> +++ b/drivers/iio/dac/ds4424.c


> +static int ds4424_init_regmap(struct i2c_client *client,
> +			      struct iio_dev *indio_dev)
>  {
>  	struct ds4424_data *data = iio_priv(indio_dev);
> +	const struct regmap_config *regmap_config;
> +	u8 vals[DS4424_MAX_DAC_CHANNELS];
>  	int ret;
>  
> -	mutex_lock(&data->lock);
> -	ret = i2c_smbus_write_byte_data(data->client,
> -			DS4424_DAC_ADDR(chan->channel), val);
> -	if (ret < 0)
> -		goto fail;
> -
> -	data->raw[chan->channel] = val;
> -
> -fail:
> -	mutex_unlock(&data->lock);
> -	return ret;
> +	if (indio_dev->num_channels == DS4424_MAX_DAC_CHANNELS)
> +		regmap_config = &ds44x4_regmap_config;
> +	else
> +		regmap_config = &ds44x2_regmap_config;
> +
> +	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");
> +
> +	/*
> +	 * Prime the cache with the bootloader's configuration.
> +	 * regmap_bulk_read will automatically populate the cache with

regmap_bulk_read() 
style preferred for functions mentioned in comments.

> +	 * the values read from the hardware.
> +	 */
> +	ret = regmap_bulk_read(data->regmap, DS4424_DAC_ADDR(0), vals,

> @@ -233,10 +253,7 @@ static int ds4424_probe(struct i2c_client *client)
>  		return ret;
>  	}
>  
> -	usleep_range(1000, 1200);
> -	ret = ds4424_verify_chip(indio_dev);
> -	if (ret < 0)
> -		goto fail;
> +	fsleep(1000);

This change to fsleep() is unrelated to regmap stuff. So separate patch.

Thanks,

Jonathan
Re: [PATCH v3 7/8] iio: dac: ds4424: convert to regmap
Posted by Andy Shevchenko 1 week, 5 days ago 
On Wed, Jan 28, 2026 at 04:38:23PM +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.

...

> changes v3:
> - Switch to REGCACHE_MAPLE to efficiently handle the sparse register map
>   (offset 0xF8) and avoid allocating memory for the unused 0x00-0xF7 range.

> - Use explicit regmap_bulk_read() in probe to seed the cache with the
>   bootloader configuration. This avoids the invalid read from address 0x00
>   that occurred with generic cache defaults.

Isn't regmap has an option to do it for you?
(I'm talking about num_reg_defaults_raw without setting reg_defaults_raw)

> - Remove ds4424_verify_chip(); devm_regmap_init_i2c() and the subsequent
>   bulk read implicitly validate the device presence.
> - Use regmap_bulk_write() in ds4424_suspend() to efficiently zero all
>   channels.
> - Adopt fsleep() for delays and include <linux/array_size.h>.
> - Use dev_err_ratelimited() with the physical device context in the read
>   path (incorporating feedback aimed at v2 patch 8).

...

> -	usleep_range(1000, 1200);
> +	fsleep(1000);

Seems like undescribed / unrelated change.
Also needs a comment to explain the delay.

-- 
With Best Regards,
Andy Shevchenko

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.