Re: [PATCH 2/2] iio: dac: add support for Microchip MCP48FEB02

[Jonathan Cameron]

On Thu, 12 Feb 2026 14:48:35 +0200
Ariana Lazar <ariana.lazar@xxxxxxxxxxxxx> wrote:

> This is the iio driver for Microchip MCP48FxBy1/2/4/8 series of buffered
> voltage output Digital-to-Analog Converters with nonvolatile or volatile
> memory and an SPI Interface.
> 
> The families support up to 8 output channels.
> 
> The devices can be 8-bit, 10-bit and 12-bit.
> 
> Signed-off-by: Ariana Lazar <ariana.lazar@xxxxxxxxxxxxx>
Hi Ariana,
Given the much larger outstanding question on whether this can share a driver
with other similar parts I only took a very quick look at this version.

Comments inline.

Thanks,

Jonathan

> diff --git a/drivers/iio/dac/mcp48feb02.c b/drivers/iio/dac/mcp48feb02.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..20955f77053329a9c385f55c7314032eb6b04dfd
> --- /dev/null
> +++ b/drivers/iio/dac/mcp48feb02.c

> +
> +static int mcp48feb02_init_ctrl_regs(struct mcp48feb02_data *data)
> +{
> +	unsigned int i, vref_ch, gain_ch, pd_ch;
> +	int ret;
> +
> +	ret = regmap_read(data->regmap, MCP48FEB02_VREF_REG_ADDR, &vref_ch);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(data->regmap, MCP48FEB02_GAIN_CTRL_STATUS_REG_ADDR, &gain_ch);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(data->regmap, MCP48FEB02_POWER_DOWN_REG_ADDR, &pd_ch);
> +	if (ret)
> +		return ret;
> +
> +	gain_ch = gain_ch & MCP48FEB02_GAIN_BITS_MASK;
> +	for_each_set_bit(i, &data->active_channels_mask, data->phys_channels) {
> +		struct device *dev = regmap_get_device(data->regmap);
> +		unsigned int pd_tmp;
> +
> +		data->chdata[i].ref_mode = (vref_ch >> (2 * i)) & MCP48FEB02_DAC_CTRL_MASK;
> +		data->chdata[i].use_2x_gain = (gain_ch >> i)  & MCP48FEB02_GAIN_BIT_MASK;
> +
> +		/*
> +		 * Inform the user that the current voltage reference read from the volatile
> +		 * register of the chip is different from the one specified in the device tree.
> +		 * Considering that the user cannot have an external voltage reference connected
> +		 * to the pin and select the internal Band Gap at the same time, in order to avoid
> +		 * miscofiguring the reference voltage, the volatile register will not be written.

Spell check comments.  misconfiguring

> +		 * In order to overwrite the setting from volatile register with the one from the
> +		 * device tree, the user needs to write the chosen scale.

I'm a little unsure of why we need this extra gate on updating things to match
the device tree provided config.  Why should the volatile register at this point
match what DT says?  If it does seems to me we should be noisier about it than dev_dbg()


> +		 */
> +		switch (data->chdata[i].ref_mode) {
> +		case MCP48FEB02_INTERNAL_BAND_GAP:
> +			if (data->phys_channels >= 4 && (i % 2) && data->use_vref1) {
> +				dev_dbg(dev, "ch[%u]: was configured to use internal band gap", i);
> +				dev_dbg(dev, "ch[%u]: reference voltage set to VREF1", i);
> +				break;
> +			}
> +			if ((data->phys_channels < 4 || (data->phys_channels >= 4 && !(i % 2))) &&
> +			    data->use_vref) {
> +				dev_dbg(dev, "ch[%u]: was configured to use internal band gap", i);
> +				dev_dbg(dev, "ch[%u]: reference voltage set to VREF", i);
> +				break;
> +			}