Re: [PATCH v3 2/2] iio: adc: ti-ads1298: Add driver

[Jonathan Cameron]

On Wed, 14 Feb 2024 07:48:40 +0100
Mike Looijmans <mike.looijmans@xxxxxxxx> wrote:

> On 10-02-2024 17:27, Jonathan Cameron wrote:
> > On Tue, 6 Feb 2024 18:38:29 +0100
> > Mike Looijmans <mike.looijmans@xxxxxxxx> wrote:
> >   
> >> On 06-02-2024 17:32, Andy Shevchenko wrote:  
> >>> On Tue, Feb 06, 2024 at 04:44:03PM +0100, Mike Looijmans wrote:  
> >>>> On 06-02-2024 16:09, Andy Shevchenko wrote:  
> >>>>> On Tue, Feb 06, 2024 at 03:47:45PM +0100, Mike Looijmans wrote:  
> >>> ...
> >>>     
> >>>>> But it's up to you what to do with that.
> >>>>> Maybe Jonathan can advice something different.
> >>>>>     
> >>>> The spinlock also protects the call to spi_async().  
> >>> I don't get this. Locks usually protect the data and not the code.
> >>> Can you elaborate?
> >>>     
> >> Either the DRDY or SPI completion handler will call spi_async(), the
> >> lock assures that it's only called by one.  
> > 
> > Arguably it's protecting the destination buffer of the spi_async()
> > call.  We don't really care if we issue two reads (it's a waste
> > of time and we would store two sets of readings but meh), but we do
> > care about being sure that don't issue a second read into a buffer
> > that we are potentially simultaneously getting data back from.  
> 
> Indeed, that.
> 
> > 
> > There are comments where the release is to describe when it can
> > be safely unlocked.
> > 
> > I'm not super keen on this whole structure but I don't really have a better
> > idea.  Who builds a device where you have no latched way of seeing
> > if there is new data? (some) Hardware folk love to assume they have a RTOS only
> > talking to their device and that no pulse signals will ever be missed.
> > 
> > We get to educate them when ever the opportunity arises :)  
> 
> Even on RTOS this chip was a pain - to get it to work reliably I had to set up 
> a DMA controller to run the SPI transactions, which took some smart 
> daisy-chaining (I recall having the DMA controller write to its own control 
> registers to avoid involving the CPU).

Always fun when that sort of mess is needed!

> 
> It's probably possible to trick audio hardware (I2S controller) into grabbing 
> the data (my chip doesn't have that) without involving the CPU.

Yeah, sometimes it feels like these ADCs have been designed with that sort
of bus in mind.

> 
> As the code is now, I can grab data and display it with the IIO oscilloscope 
> over network at 4kHz without losing samples on an A9 at 600Mhz.

Nice.
>