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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date: Wed, 14 Feb 2024 14:08:24 +0000
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: Mike Looijmans <mike.looijmans@...ic.nl>
CC: Jonathan Cameron <jic23@...nel.org>, Andy Shevchenko
	<andriy.shevchenko@...ux.intel.com>, <devicetree@...r.kernel.org>,
	<linux-iio@...r.kernel.org>, Lars-Peter Clausen <lars@...afoo.de>, "Liam
 Beguin" <liambeguin@...il.com>, Liam Girdwood <lgirdwood@...il.com>, "Maksim
 Kiselev" <bigunclemax@...il.com>, Marcus Folkesson
	<marcus.folkesson@...il.com>, Marius Cristea <marius.cristea@...rochip.com>,
	Mark Brown <broonie@...nel.org>, "Niklas Schnelle" <schnelle@...ux.ibm.com>,
	Okan Sahin <okan.sahin@...log.com>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 2/2] iio: adc: ti-ads1298: Add driver

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

> On 10-02-2024 17:27, Jonathan Cameron wrote:
> > On Tue, 6 Feb 2024 18:38:29 +0100
> > Mike Looijmans <mike.looijmans@...ic.nl> 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.
>