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

On 07/27, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Holding CNV high during SPI transfer breaks 3-wire SPI wiring mode.
> --
> 
> --- Patch [2]: [PATCH v9 2/6] iio: adc: ltc2378: Add support for LTC2378-20 and similar ADCs ---
> commit 4d4231cee28f9742280dd61ed84f85cf3b4e0386
> Author: Marcelo Schmitt <[email protected]>
> 
> 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.
> 
> > diff --git a/drivers/iio/adc/ltc2378.c b/drivers/iio/adc/ltc2378.c
> > new file mode 100644
> > --- /dev/null
> > +++ b/drivers/iio/adc/ltc2378.c
> [ ... ]
> > +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(LTC2378_MAX_DATA_WAIT_US);
> > +	ret = spi_sync_transfer(st->spi, &st->xfer, 1);
> 
> [Severity: High]
> Does holding CNV high during the SPI transfer break 3-wire SPI wiring mode?
> In 3-wire mode (where SDI is tied to GND), the SDO pin of the ADC remains in
> a high-impedance state as long as the CNV pin is held high.
No.

> 
> Since the CNV pin remains high throughout the SPI transfer, could the SPI
> controller clock in garbage data from a floating bus?
No. That would only happen if the controller runs more SCLKs than the amount of
precision/resolution bits (non-offload use case) but in those cases we right
shift by _storage_bits - _real_bits, discarding any potential unwanted noise.

> 
> Would lowering CNV after the fsleep() wait and before calling
> spi_sync_transfer() correctly enable SDO for 3-wire mode while remaining
> compatible with 4-wire (CS) mode?
These devices don't have a CS pin. SDI is tied to GND in the currently supported
wiring configuration (single device normal mode). There is no SPI 4-wire mode.

> 
> > +	gpiod_set_value_cansleep(st->cnv_gpio, 0);
> > +
> > +	return ret;
> > +}
> [ ... ]
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2