linux-kernel - Re: [PATCH 4/5] iio: adis16480: add support for adis16545/7 families

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <e62f8df4b06abc371b1e9fe3232cb593e468d54c.camel@gmail.com>
Date: Mon, 29 Apr 2024 09:58:40 +0200
From: Nuno Sá <noname.nuno@...il.com>
To: Jonathan Cameron <jic23@...nel.org>, Ramona Gradinariu
	 <ramona.bolboaca13@...il.com>
Cc: linux-kernel@...r.kernel.org, linux-iio@...r.kernel.org, 
 linux-doc@...r.kernel.org, devicetree@...r.kernel.org, corbet@....net, 
 conor+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org, robh@...nel.org,
 Ramona Gradinariu <ramona.gradinariu@...log.com>, Nuno
 Sá <nuno.sa@...log.com>
Subject: Re: [PATCH 4/5] iio: adis16480: add support for adis16545/7 families

On Sun, 2024-04-28 at 16:25 +0100, Jonathan Cameron wrote:
> On Tue, 23 Apr 2024 11:42:09 +0300
> Ramona Gradinariu <ramona.bolboaca13@...il.com> wrote:
> 
> > The ADIS16545 and ADIS16547 are a complete inertial system that
> > includes a triaxis gyroscope and a triaxis accelerometer.
> > The serial peripheral interface (SPI) and register structure provide a
> > simple interface for data collection and configuration control.
> > 
> > These devices are similar to the ones already supported in the driver,
> > with changes in the scales, timings and the max spi speed in burst
> > mode.
> > Also, they support delta angle and delta velocity readings in burst
> > mode, for which support was added in the trigger handler.
> > 
> > Signed-off-by: Nuno Sá <nuno.sa@...log.com>
> 
> What is Nuno's relationship to this patch?  You are author and the sender
> which is fine, but in that case you need to make Nuno's involvement explicit.
> Perhaps a Co-developed-by or similar is appropriate?
> 
> > Signed-off-by: Ramona Gradinariu <ramona.gradinariu@...log.com>
> A few comments inline.  Biggest one is I'd like a clear statement of why you
> can't do a burst of one type, then a burst of other.  My guess is that the
> transition is very time consuming?  If so, that is fine, but you should be
> able
> to let available_scan_masks handle the disjoint channel sets.

Yeah, the burst message is a special spi transfer that brings you all of the
channels data at once but for the accel/gyro you need to explicitly configure
the chip to either give you the "normal vs "delta" readings. Re-configuring the
chip and then do another burst would destroy performance I think. We could do
the manual readings as we do in adis16475 for chips not supporting burst32. But
in the burst32 case those manual readings should be minimal while in here we
could have to do 6 of them which could also be very time consuming...

Now, why we don't use available_scan_masks is something I can't really remember
but this implementation goes in line with what we have in the adis16475 driver.

- Nuno Sá