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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The voltage from the 'refin' regulator is not multiplied by the internal buffer gain, resulting in incorrect ADC scale.
- [Medium] External 'ref' regulator is completely ignored for chips with an internal reference, breaking the ability to overdrive the internal reference.
--

commit 06f09452b5298cd69acd94f1c33c2c91342a505d
Author: Marcelo Schmitt <marcelo.schmitt@analog.com>

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

This commit adds a new driver for the Analog Devices 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..9a9f32e4989ba
> --- /dev/null
> +++ b/drivers/iio/adc/ltc2378.c
[ ... ]
> +static int ltc2378_regulator_setup(struct device *dev, struct ltc2378_state *st)
> +{
> +	int ret;
> +
> +	ret = devm_regulator_get_enable_read_voltage(dev, "refin");
> +	if (ret < 0 && ret != -ENODEV) {
> +		return dev_err_probe(dev, ret, "failed to read refin regulator\n");
> +	} else if (ret > 0) {
> +		st->ref_uV = ret;

[Severity: High]
Does this assignment account for the internal buffer's amplification gain?

For example, on chips like the LTC2338-18, the internal reference buffer
amplifies the REFIN voltage by a factor of 2. If the 'refin' regulator provides
2.048V, the internal REF pin sees 4.096V. But this code appears to assign the
direct REFIN voltage to st->ref_uV, which would result in an incorrect ADC
scale calculation.

> +		return 0;
> +	}
> +
> +	if (st->info->internal_ref_uv) {
> +		st->ref_uV = st->info->internal_ref_uv;
> +		return 0;

[Severity: Medium]
Does this early return prevent a board from overdriving the internal reference
with an external 'ref' supply?

If the hardware supports overdriving the internal reference and a 'ref-supply'
is provided in the device tree, it looks like this code will silently ignore
it and use the internal reference value instead.

> +	}
> +
> +	ret = devm_regulator_get_enable_read_voltage(dev, "ref");
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "failed to read ref regulator\n");
> +
> +	st->ref_uV = ret;
> +
> +	return 0;
> +}
[ ... ]

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