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

[David Lechner]

On 6/25/26 9:35 AM, Marcelo Schmitt wrote:
> Support for LTC2378-20 and similar analog-to-digital converters.

This should mention that it contains some future prep for offloading.
(Although would be a bit easier to review if we deferred adding the
offload stuff).

> 
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@xxxxxxxxxx>
> ---
> Change log v3 -> v4:
> - Reworked the driver to make IIO channels static according to v3 feedback.
> - Updated to use default 8 bits_per_word for non-offloaded transfers.
> - Handled refin voltage reference supply for LTC2338.
> - Used spi_bpw_to_bytes() where applicable.
> 
>  MAINTAINERS               |   1 +
>  drivers/iio/adc/Kconfig   |  12 ++
>  drivers/iio/adc/Makefile  |   1 +
>  drivers/iio/adc/ltc2378.c | 395 ++++++++++++++++++++++++++++++++++++++
>  4 files changed, 409 insertions(+)
>  create mode 100644 drivers/iio/adc/ltc2378.c
> 

...

> diff --git a/drivers/iio/adc/ltc2378.c b/drivers/iio/adc/ltc2378.c
> new file mode 100644
> index 000000000000..9a9f32e4989b
> --- /dev/null
> +++ b/drivers/iio/adc/ltc2378.c
> @@ -0,0 +1,395 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Analog Devices LTC2378 ADC series driver
> + *
> + * Copyright (C) 2026 Analog Devices Inc.
> + * Author: Marcelo Schmitt <marcelo.schmitt@xxxxxxxxxx>
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/bits.h>
> +#include <linux/cleanup.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +#include <linux/types.h>
> +#include <linux/units.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/types.h>
> +
> +#define __LTC2378_DIFF_CHANNEL(_sign, _real_bits, _storage_bits, _offl)\

Why the leading underscores? I don't see a conflicting name.

> +{										\
> +	.type = IIO_VOLTAGE,							\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |				\
> +			      BIT(IIO_CHAN_INFO_SCALE) |			\
> +			      (_offl ? BIT(IIO_CHAN_INFO_SAMP_FREQ) : 0),	\
> +	.info_mask_separate_available = _offl ? BIT(IIO_CHAN_INFO_SAMP_FREQ) : 0,\
> +	.scan_index = 0,							\
> +	.scan_type = {								\
> +		.format = _sign ? IIO_SCAN_FORMAT_SIGNED_INT :			\
> +				  IIO_SCAN_FORMAT_UNSIGNED_INT,			\
> +		.realbits = _real_bits,						\
> +		.storagebits = _storage_bits,					\
> +		.shift = (_offl ? 0 : _storage_bits - _real_bits),		\
> +		.endianness = _offl ? IIO_CPU : IIO_BE				\
> +	},									\
> +}
> +
> +#define LTC2378_BIPOLAR_DIFF_CHANNEL(_real_bits)				\
> +	__LTC2378_DIFF_CHANNEL(1, _real_bits, (((_real_bits) > 16) ? 32 : 16), 0)
> +
> +#define LTC2378_UNIPOLAR_DIFF_CHANNEL(_real_bits)				\
> +	__LTC2378_DIFF_CHANNEL(0, _real_bits, (((_real_bits) > 16) ? 32 : 16), 0)

Why not move the (((_real_bits) > 16) ? 32 : 16) into the __LTC2378_DIFF_CHANNEL()
macro to avoid repeating it?

> +
> +struct ltc2378_chip_info {
> +	const char *name;
> +	unsigned int internal_ref_uv;

_uV (to be consistent)

> +	struct iio_chan_spec chan;
> +};
> +

...

> +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) {

Else is not needed here.

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

I would be tempted to have two separate functions here and only call one depending
on the chip. Otherwise, it allows incorrect devicetree.

> +
> +	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;
> +}
> +