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

[Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>]

On Fri, 17 Jul 2026 13:38:07 -0300
Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:

> On 07/16, sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > - [Medium] The `LTC2378_DIFF_CHANNEL` macro defines an IIO channel that is intended to be differential but lacks the `.differential = 1`, `.indexed = 1`, `.channel = 0`, and `.channel2 = 1` attributes.
> > --
> > 
> > --- Patch [2]: [PATCH v7 2/4] iio: adc: ltc2378: Add support for LTC2378-20 and similar ADCs ---
> > commit 5710d5f25b45906977e2522567c1964b1d3219f8
> > 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..5600c820c7f45
> > > --- /dev/null
> > > +++ b/drivers/iio/adc/ltc2378.c
> > > @@ -0,0 +1,416 @@  
> > [ ... ]  
> > > +#define LTC2378_DIFF_CHANNEL(_sign, _real_bits, _storage_bits)			\
> > > +{										\
> > > +	.type = IIO_VOLTAGE,							\
> > > +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |				\
> > > +			      BIT(IIO_CHAN_INFO_SCALE),				\
> > > +	.scan_index = 0,							\  
> > 
> > [Severity: Medium]
> > Does this macro need to set .differential = 1, .indexed = 1, .channel = 0,
> > and .channel2 = 1? The name suggests these are differential channels, but
> > without these attributes, the IIO ABI will expose them as single-ended
> > (e.g., in_voltage_raw instead of in_voltage0-voltage1_raw).  
> 
> For parts with multiple inputs and configurable differential channels (e.g.
> AD4130-8, MCP3561/2/4R, NAFE13388 [1, 2, 3]), the voltageY-voltageZ ABI helps
> clarifying what physical inputs make up a channel. For differential only parts,
> the ABI seems to be flexible by only requiring a 'consistent labeling' (either
> voltageY or voltageY-voltageZ IIUC). Will set the channel with the differential
> ABI if maintainers believe that's the best way to have it. Might slightly tweak
> the channel macros to keep .differential = 0 for pseudo-differential chips
> (e.g.LTC2369-18).

Whilst I get your point on differential only cases not really requiring the
specification of two inputs, I think that does provide a simple indication
to users that they are differential channel.

I would prefer that we make them 'look' like differential channels so make up
a suitable number for the negative side.

I vaguely recall we did once consider handling these as inX-inX but it seemed
a little too weird.  Hohum. ABI was designed against the max1363 and I didn't
think much about device that always did differential.  What can I say, I was
young and foolish ;)

Jonathan

> 
> [1]: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4130-8.pdf
> [2]: https://ww1.microchip.com/downloads/aemDocuments/documents/APID/ProductDocuments/DataSheets/MCP3561_2_4R-Data-Sheet-DS200006391C.pdf
> [3]: https://www.nxp.com/docs/en/data-sheet/NAFE13388.pdf
> 
> >   
> > > +	.scan_type = {								\
> > > +		.format = _sign ? IIO_SCAN_FORMAT_SIGNED_INT :			\
> > > +				  IIO_SCAN_FORMAT_UNSIGNED_INT,			\
> > > +		.realbits = _real_bits,						\
> > > +		.storagebits = _storage_bits,					\
> > > +		.shift = _storage_bits - _real_bits,				\
> > > +		.endianness = IIO_BE,						\
> > > +	},									\
> > > +}
> > > +
> > > +#define LTC2378_BIPOLAR_DIFF_CHANNEL(_real_bits)				\
> > > +	LTC2378_DIFF_CHANNEL(1, _real_bits, (((_real_bits) > 16) ? 32 : 16))
> > > +
> > > +#define LTC2378_UNIPOLAR_DIFF_CHANNEL(_real_bits)				\
> > > +	LTC2378_DIFF_CHANNEL(0, _real_bits, (((_real_bits) > 16) ? 32 : 16))
> > > +  
> > [ ... ]
> > 
> > -- 
> > Sashiko AI review · https://sashiko.dev/#/patchset/cover.1784235595.git.marcelo.schmitt@analog.com?part=2  
>