Re: [PATCH v2 2/2] iio: dac: dac8163: Add driver for DAC8163

[Jonathan Cameron <jic23@kernel.org>]

On Sun, 26 Jul 2026 17:01:51 +0000
"Siratul Islam" <siratul.islam@linux.dev> wrote:

> July 26, 2026 at 8:10 PM, "Lukas" <lukas.metz@gmx.net mailto:lukas.metz@gmx.net?to=%22Lukas%22%20%3Clukas.metz%40gmx.net%3E > wrote:
> 
> 
> > 
> > On Wed, Jul 08, 2026 at 09:58:13PM +0600, Siratul Islam wrote:
> >   
> > > 
> > > +
> > >  + const struct reg_default reg_defaults[] = {
> > >  + {
> > >  + .reg = CMD_SET(CMD_WRITE_UPDATE, 0),
> > >  + .def = info->default_output_reg,
> > >  + },
> > >  + {
> > >  + .reg = CMD_SET(CMD_WRITE_UPDATE, 1),
> > >  + .def = info->default_output_reg,
> > >  + },
> > >  + };
> > >  +
> > >  + const struct regmap_config regmap_config = {
> > >  + .reg_bits = 8,
> > >  + .val_bits = 16,
> > >  +
> > >  + .max_register = CMD_REF << 3,
> > >  + .cache_type = REGCACHE_MAPLE,
> > >  +
> > >  + .volatile_reg = dac8163_reg_false,
> > >  +
> > >  + .reg_defaults = reg_defaults,
> > >  + .num_reg_defaults = ARRAY_SIZE(reg_defaults),
> > >  + };
> > >  Why are these defined inside probe? Should be defined outside the function.
> > >   
> > The default values after reset are different for the compatible devices
> > (0 for the dacxxx2 devices and mid-scale for the dacxxx3 devices).
> > So these are runtime dependent. The alternative would be to have two
> > regmap_configs for the two device types and add these to the chip_info.
> > Or do you have another suggestion?  
> You don't need two regmap_configs. The way I would do it is, move the "regmap_config" out of probe and name it something 
> like "dac8563_regmap_config". And in chip info struct, replace "default_output_reg" with "reg_defaults[]". 
> Then for each "dacxxxx_chip_info" define the appropriate "reg_defaults" configuration.
> 
> This way you can do
> 
> struct regmap_config regmap_config = dac8563_regmap_config;
> ...
> regmap_config.reg_defaults = info->reg_defaults;
> regmap_config.num_reg_defaults = ARRAY_SIZE(info->reg_defaults);
> 
> A bit of work, but it would ensure most of the work is done at compile time,
> and we avoid reconstructing these sructs everytime probe runs.

As this has come up, I'd favour just having two static const regmap_config structures
and picking between them.  Obviously I haven't checked but I'd guess that any
overhead of extra text data for those is not that different to the extra code of the current
approach.  Generally if you can make this sort of thing data rather than code it
is a net gain.

Jonathan

> 
> > 
> > Best regards
> > 
> > Lukas
> >  
> --
> Best regards,
> Sirat