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

On Mon, 27 Jul 2026 18:42:26 +0000
[email protected] wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Potential off-by-one in DAC scale calculation denominator.
> --

Responding to Sashiko's comment.

> --- Patch [2]: [PATCH v2 2/2] iio: dac: mcp47a1: add support for new device ---
> commit 38bec2fbb020c3f749c52a26e5cc3e78e72eeaf5
> Author: Joshua Crofts <[email protected]>
> 
> iio: dac: mcp47a1: add support for new device
> 
> The Microchip MCP47A1 is a 6-bit volatile Digital-to-Analog converter
> which communicates via I2C.
> 
> > diff --git a/drivers/iio/dac/mcp47a1.c b/drivers/iio/dac/mcp47a1.c
> > new file mode 100644
> > index 0000000000000..140e93ff2ba01
> > --- /dev/null
> > +++ b/drivers/iio/dac/mcp47a1.c  
> [ ... ]
> > +static int mcp47a1_read(struct iio_dev *indio_dev,
> > +			struct iio_chan_spec const *chan,
> > +			int *val, int *val2, long mask)
> > +{
> > +	struct mcp47a1_data *data = iio_priv(indio_dev);
> > +	int ret;
> > +
> > +	switch (mask) {  
> [ ... ]
> > +	case IIO_CHAN_INFO_SCALE:
> > +		*val = data->vref_mV;
> > +		*val2 = MCP47A1_MAX_STEP;  
> 
> [Severity: Medium]
> Does this code use the correct denominator for the scale calculation?
> 
> Since this is a 6-bit DAC, shouldn't the scale denominator be 64 (2^6)
> instead of the maximum step value of 63? Using MCP47A1_MAX_STEP (63) here
> introduces an off-by-one error that could cause userspace to calculate
> output voltages with a ~1.5% LSB scaling error when reading the sysfs
> attribute.

Yes, the max amount of steps is 64, which should be passed here. I'll
add a separate macro for this in v3.

-- 
Kind regards,
Joshua Crofts