Re: [PATCH v8 2/5] iio: adc: ltc2378: Add support for LTC2378-20 and similar ADCs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The IIO scale factor exported for the LTC2338-18 is incorrect due to an unaccounted internal resistor divider (missing 2.5x multiplier).
--

--- Patch [2]: [PATCH v8 2/5] iio: adc: ltc2378: Add support for LTC2378-20 and similar ADCs ---
commit 79bd7cc98e49057401d7fb8d26685b9092c4787e
Author: Marcelo Schmitt <marcelo.schmitt@analog.com>

iio: adc: ltc2378: Add support for LTC2378-20 and similar ADCs

Support for LTC2378-20 and similar analog-to-digital converters.

> diff --git a/drivers/iio/adc/ltc2378.c b/drivers/iio/adc/ltc2378.c
> new file mode 100644
> index 0000000000000..107388f870626
> --- /dev/null
> +++ b/drivers/iio/adc/ltc2378.c
[ ... ]
> +static const struct ltc2378_chip_info ltc2338_18_chip_info = {
> +	.name = "ltc2338-18",
> +	.internal_ref_uV = 2048000,
> +	.chan = LTC2378_DIFF_CHANNEL(18),
> +};

[Severity: Medium]
Does this chip info need a field to indicate the scale multiplier?

The LTC2338-18 has an internal resistor divider which acts as a 2.5x multiplier,
but this struct doesn't currently contain a way to specify it.

[ ... ]
> +static int ltc2378_read_raw(struct iio_dev *indio_dev,
> +			    const struct iio_chan_spec *chan,
> +			    int *val, int *val2, long mask)
> +{
> +	struct ltc2378_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
[ ... ]
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = st->ref_uV / MILLI;

[Severity: Medium]
Will this return an incorrect IIO scale factor for the LTC2338-18?

The scale is calculated purely based on the reference voltage here, which does
not account for the LTC2338-18's internal divider (missing the 2.5x multiplier).
This could lead to incorrect voltage calculations in userspace.

> +		/*
> +		 * For all LTC2378-like devices, the amount of bits that express
> +		 * voltage magnitude depend on the polarity / output code format:
> +		 * - straight binary: All precision/resolution bits are used.
> +		 * - 2's complement: One of the precision bits is used for sign.
> +		 */
> +		if (chan->scan_type.format == IIO_SCAN_FORMAT_SIGNED_INT)
> +			*val2 = chan->scan_type.realbits - 1;
> +		else
> +			*val2 = chan->scan_type.realbits;
> +
> +		return IIO_VAL_FRACTIONAL_LOG2;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1784835663.git.marcelo.schmitt@analog.com?part=2