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

[Jonathan Cameron <jic23@kernel.org>]

On Fri, 3 Jul 2026 14:14:50 -0300
Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:

> On 07/02, sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
> > - [High] Hardware protocol violation: CNV is held high during SPI read
> > - [High] SPI bus not locked during ADC conversion phase
> > - [Medium] Incorrect scale calculation for LTC2338-18 and similar ADCs
> > - [Low] Undefined direct mode macros lead to build failure
> > --
> > 
> > commit b74b536d18af0084ea31bd1e1e8cf9454da5e29c
> > Author: Marcelo Schmitt <marcelo.schmitt@analog.com>
> > 
> > iio: adc: ltc2378: Add support for LTC2378-20 and similar ADCs
> > 
> > This commit adds support for the Analog Devices LTC2378-20 and similar
> > analog-to-digital converters. It introduces a new IIO driver using the
> > SPI subsystem.
> >   
> > > diff --git a/drivers/iio/adc/ltc2378.c b/drivers/iio/adc/ltc2378.c
> > > new file mode 100644
> > > index 0000000000000..b5cf2e974daca
> > > --- /dev/null
> > > +++ b/drivers/iio/adc/ltc2378.c
> > > @@ -0,0 +1,404 @@  
> > 
> > [ ... ]
> >   
> > > +static const struct ltc2378_chip_info ltc2338_18_chip_info = {
> > > +	.name = "ltc2338-18",
> > > +	.internal_ref_uV = 4096000,
> > > +	.chan = LTC2378_BIPOLAR_DIFF_CHANNEL(18),
> > > +};  
> > 
> > [ ... ]
> >   
> > > +static int ltc2378_convert_and_acquire(struct ltc2378_state *st)
> > > +{
> > > +	int ret;
> > > +
> > > +	/* Cause a rising edge of CNV to initiate a new ADC conversion */
> > > +	gpiod_set_value_cansleep(st->cnv_gpio, 1);
> > > +	fsleep(4);  
> > 
> > [Severity: High]
> > Can this sequence cause data corruption on a shared SPI bus?
> > 
> > The CNV pin is toggled and an fsleep(4) occurs before spi_sync_transfer()
> > locks the bus. If another SPI transaction occurs during this sleep window,
> > could it toggle the SCK line and prematurely clock data out of the ADC?  
> Makes sense. 
> > 
> > Should the conversion-and-read sequence be wrapped in spi_bus_lock()
> > and spi_bus_unlock()?  
> Because these chips don't have CS line, it seems that locking the bus actually
> makes sense.

What does it even mean?  If there is no CS then the bus can only sensibly
have one device on it. I'm not sure if the SPI core enforces that but
in practice you should never see a board that has multiple devices.

Hence locking could be at the device driver level rather than the bus.
I guess bus locking would provide the same conditions and maybe acts
as documentation.  

Jonathan