Re: [PATCH 2/2] iio: dac: mcp47a1: add support for new device

[Jonathan Cameron]

On Tue, 21 Jul 2026 20:24:39 +0200
Joshua Crofts <joshua.crofts1@xxxxxxxxx> wrote:

> The Microchip MCP47A1 is a 6-bit volatile Digital-to-Analog converted
> which communicates via I2C.
> 
> Signed-off-by: Joshua Crofts <joshua.crofts1@xxxxxxxxx>

A few really small comments inline.  Nice little driver.

Given the really small number of registers, regmap does seem like
it might be overkill

> diff --git a/drivers/iio/dac/mcp47a1.c b/drivers/iio/dac/mcp47a1.c
> new file mode 100644
> index 000000000000..251c2281634f
> --- /dev/null
> +++ b/drivers/iio/dac/mcp47a1.c
> @@ -0,0 +1,193 @@

...


> +static int mcp47a1_probe(struct i2c_client *client)
> +{

...

> +	/*
> +	 * This is a volatile DAC which doesn't have an ID register, instead
> +	 * the value register is set to 0x20 every Power-on-Reset (table 4-1).
> +	 * Any other read value at startup could indicate that the device is
> +	 * damaged etc.

or some earlier software wrote it and the all the regulators are stubs
provided as they can't be turned off by software.

I'd drop this test - it is a bit too flakey as a way to tell the chip
is there and working. 

> +	 */
> +	ret = regmap_read(data->regmap, MCP47A1_CMD_CODE, &reg_val);
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "Failed to read register on startup\n");
> +
> +	if (reg_val != 0x20)
> +		dev_info(dev, "Read value 0x%02x at startup\n", reg_val);
> +
> +	indio_dev->name = "mcp47a1";
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->info = &mcp47a1_info;
> +	indio_dev->channels = mcp47a1_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(mcp47a1_channels);
> +
> +	return devm_iio_device_register(dev, indio_dev);
> +}