Re: [PATCH v2 5/8] iio: dac: ds4424: convert to regmap

[Sander Vanheule <sander@svanheule.net>]

Hi Oleksij,

Thanks for the quick reply!

On Sun, 2026-02-01 at 17:16 +0100, Oleksij Rempel wrote:
> Hi Sander,
> 
> On Sun, Feb 01, 2026 at 03:42:28PM +0100, Sander Vanheule wrote:
> > Hi Oleksij,
> > 
> > On Tue, 2026-01-27 at 07:09 +0100, Oleksij Rempel wrote:
> > > +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).
> 
> ACK, already migrated to REGCACHE_MAPLE in the v3:
> https://lore.kernel.org/all/20260128153824.3679187-8-o.rempel@pengutronix.de/

Thanks for the link. I'm not on the IIO list, so I hadn't seen this version yet.

> Which works mostly fine except of the cache initialisation. If I use
> num_reg_defaults_raw with REGCACHE_MAPLE as proposed by Andy
> Shevchenko, first access to regmap values over debugfs will explode with
> NULL pointer etc...
> If I remove num_reg_defaults_raw, I need to read register manually
> to init defaul values as implemented in v3.

num_reg_defaults_raw without a raw register value buffer is also problematic
IMHO. If it is used without a buffer, values will be read from hardware. This is
indeed widely used to prime a (flat) cache, but if you want to maintain the
documented behavior of regmap_sync() it should only be used if the hardware
really just came out of reset.

Not sure how likely it is that not all channels values are written after probe,
but for this driver specifically you might see:

   1. Driver probes with num_reg_defaults_raw, using current HW values as
      initial cache _and_ register defaults.
   2. No updates were written before device is suspended. DAC registers are set
      to 0, cache marked dirty.
   3. No updates were written before device comes out of suspend and
      regmap_sync() is called. regmap_sync() sees a cache state which is still
      identical to the register defaults and omits (all) writes.

After 3. your device is considered online again, but the HW registers are still
set to 0 and your cache is out-of-sync.

This also made me realize you do want to read all the channels at some point to
be able to resume to the same state after suspending.


> The REGCACHE_FLAT has one more problem, the cache will be inited with
> i2c NACKs (0xFF) if used with num_reg_defaults_raw.

That's rather nasty. I don't suppose you investigated why that's happening? (not
saying you should, just wondering)

Best,
Sander