Re: [PATCH v1 3/4] iio: adc: ltc2378: Enable high-speed data capture

[Jonathan Cameron]

On Wed, 20 May 2026 14:31:51 -0300
Marcelo Schmitt <marcelo.schmitt1@xxxxxxxxx> wrote:

> > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > > index 70fec8e3e891..b5368ee783f7 100644
> > > --- a/drivers/iio/adc/Kconfig
> > > +++ b/drivers/iio/adc/Kconfig
> > > @@ -944,6 +944,7 @@ config LTC2378
> > >  	depends on SPI
> > >  	depends on GPIOLIB || PWM
> > >  	select IIO_BUFFER
> > > +	imply LTC2378_OFFLOAD_BUFFER
> > >  	help
> > >  	  Say yes here to build support for Analog Devices LTC2378-20 and
> > >  	  similar analog to digital converters.
> > > @@ -951,6 +952,17 @@ config LTC2378
> > >  	  This driver can also be built as a module. If so, the module will
> > >  	  be called ltc2378.
> > >  
> > > +config LTC2378_OFFLOAD_BUFFER
> > > +	bool "Offloaded data capture with LTC2378"
> > > +	depends on SPI && LTC2378
> > > +	depends on SPI_OFFLOAD=y
> > > +	depends on PWM=y
> > > +	depends on SPI_OFFLOAD_TRIGGER_PWM=y
> > > +	depends on IIO_BUFFER_DMA=y
> > > +	depends on IIO_BUFFER_DMAENGINE=y  
> > 
> > why do all these have to be built in?  In general I think we need to jusitfy
> > why this driver needs to work on systems with out this stuff being built.
> > I.e. why do we need the separate optional file at all?  
> 
> Hi Jonathan, thank you for your review.
> 
> I've applied fixes to all issues reported by sashiko, except for Kconfig
> configuration (see below) and the is_visible() thing I'm still working on.
> 
> My intent with this design choice was to keep chip intrinsic dependencies
> separate from SPI offload dependencies. The indented benefit is to enable using
> the device without having to enable SPI_OFFLOAD (which is boolean and might not
> be supported on all platforms). The offload feature is indeed highly desirable
> hence the imply on the main driver Kconfig entry. The other symbols are tristate
> so, if we keep this approach, will drop =y.

I can see there will be platforms where SPI_OFFLOAD doesn't make sense, but 
are there cases where it can't be built anyway?  It's not that big and doesn't
seem to have dependencies beyond those of SPI.
> 
> The separate optional file is part of the push for dependency separation. It
> helps avoid #ifdef in c code. If the proposal is not far from something
> acceptable, I'd like to have another try of making it work. Still, it's arguable
> that the real interest is only on supporting the offload thing. Also, since I
> don't have any request to support these without offload, I think it will also be
> okay to have everything combined into a single driver. 

I don't mind the separate file, but I do mind the need for so much to be built
in rather than modular. So I think the Kconfig needs some changes rather than
it being a fundamental problem.

Jonathan

> 
> Will follow all the other suggestions.
> 
> >   
> > > +	help
> > > +	  Say yes here to build support for high speed data capture with LTC2378
> > > +
> > >  config LTC2471
> > >  	tristate "Linear Technology LTC2471 and LTC2473 ADC driver"
> > >  	depends on I2C
> > > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> > > index 1814fb78dde3..2fa5dce0ceea 100644
> > > --- a/drivers/iio/adc/Makefile
> > > +++ b/drivers/iio/adc/Makefile
> > > @@ -82,6 +82,7 @@ obj-$(CONFIG_LPC18XX_ADC) += lpc18xx_adc.o
> > >  obj-$(CONFIG_LPC32XX_ADC) += lpc32xx_adc.o
> > >  obj-$(CONFIG_LTC2309) += ltc2309.o
> > >  obj-$(CONFIG_LTC2378) += ltc2378.o
> > > +obj-$(CONFIG_LTC2378_OFFLOAD_BUFFER) += ltc2378-offload-buffer.o  
> > 
> > Hmm. This is odd. The driver can be modular but not this and it's build seperately
> > rather than as part of the module. I think these bit needs a redesign.
> > We definitely want this extra support to be part of the module.  See how something
> > like the ADIS IMU library in iio/imu/Makefile is done.  
> 
> Will check this out too.
> 
> >   
> > >  obj-$(CONFIG_LTC2471) += ltc2471.o
> > >  obj-$(CONFIG_LTC2485) += ltc2485.o
> > >  obj-$(CONFIG_LTC2496) += ltc2496.o ltc2497-core.o  
>