Re: [PATCH v2 5/8] iio: dac: ds4424: convert to regmap
From: Sander Vanheule <sander@svanheule.net>
Date: 2026-02-01 14:52:17
Also in:
linux-iio, lkml
Hi Oleksij, On Tue, 2026-01-27 at 07:09 +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).
[...]
+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_FLAT,
+ .max_register = DS4424_DAC_ADDR(1),
+ .rd_table = &ds44x2_table,
+ .wr_table = &ds44x2_table,
+};Note that REGCACHE_FLAT will allocate 0xF8 unsigned longs you will never use. REGCACHE_MAPLE will probably be much closer to the size of the original value cache, for a small look-up performance penalty (but always fast compared to the I2C bus). [...]
quoted hunk ↗ jump to hunk
@@ -163,49 +184,52 @@ static int ds4424_write_raw(struct iio_dev *indio_dev,static int ds4424_verify_chip(struct iio_dev *indio_dev) { - int ret, val; + struct ds4424_data *data = iio_priv(indio_dev); + u8 raw_values[DS4424_MAX_DAC_CHANNELS]; + int ret; - ret = ds4424_get_value(indio_dev, &val, 0); - if (ret < 0) - dev_err(&indio_dev->dev, - "%s failed. ret: %d\n", __func__, ret); + /* Bulk read all channels starting at 0xf8. + * This populates the regmap cache with current HW values. + */ + ret = regmap_bulk_read(data->regmap, DS4424_DAC_ADDR(0), + raw_values, indio_dev->num_channels);
Are you forcing a HW-to-cache sync for performance? Previously this function would just read a single value to verify a reply was sent, instead of seeding the cache in data->raw, so that's a change in behavior (and purpose) of this function, meaning you may want to change the name if you keep this. You could (should IMHO) use a sparse cache, which is actually aware of its content's validity and will transparently access the device on the first access to initialize itself. I would recommend REGCACHE_MAPLE as above, but REGCACHE_FLAT_S also works. REGCACHE_FLAT is tricky to properly initialize [1], and I would not recommend using it in new code. Using a single regmap_read() here (as ds4424_get_value() before) with a sparse cache would result in an error if there was a bus error, giving you the verification behavior again. [1] https://patch.msgid.link/20251023135032.229511-1-sander@svanheule.net/ Best, Sander