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

[sashiko-bot@kernel.org]

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.
--

--- Patch [2]: [PATCH v2 2/2] iio: dac: mcp47a1: add support for new device ---
commit 38bec2fbb020c3f749c52a26e5cc3e78e72eeaf5
Author: Joshua Crofts <joshua.crofts1@gmail.com>

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.

> +
> +		return IIO_VAL_FRACTIONAL;
> +	default:
> +		return -EINVAL;
> +	}
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727-mcp47a1-add-support-v2-0-d4591a2ce69e@gmail.com?part=2