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: 
 <BL1PR03MB5992DEBF82C0DB7BDC5EA0FF971B2@BL1PR03MB5992.namprd03.prod.outlook.com>
Date: Mon, 29 Apr 2024 13:17:42 +0000
From: "Gradinariu, Ramona" <Ramona.Gradinariu@...log.com>
To: Nuno Sá <noname.nuno@...il.com>,
        Jonathan Cameron
	<jic23@...nel.org>,
        Ramona Gradinariu <ramona.bolboaca13@...il.com>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "corbet@....net"
	<corbet@....net>,
        "conor+dt@...nel.org" <conor+dt@...nel.org>,
        "krzysztof.kozlowski+dt@...aro.org" <krzysztof.kozlowski+dt@...aro.org>,
        "robh@...nel.org" <robh@...nel.org>, "Sa, Nuno" <Nuno.Sa@...log.com>
Subject: RE: [PATCH 4/5] iio: adis16480: add support for adis16545/7 families

> -----Original Message-----
> From: Nuno Sá <noname.nuno@...il.com>
> Sent: Monday, April 29, 2024 10:59 AM
> 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;
> Gradinariu, Ramona <Ramona.Gradinariu@...log.com>; Sa, Nuno
> <Nuno.Sa@...log.com>
> Subject: Re: [PATCH 4/5] iio: adis16480: add support for adis16545/7 families
> 
> [External]
> 
> 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á
> 

Thank you Nuno for all the additional explanations.
Regarding the use of available_scan_masks, the idea is to have any possible
combination for accel, gyro, temp and timestamp channels or delta angle, delta 
velocity, temp and  timestamp channels. There are a lot of combinations for 
this and it does not seem like a good idea to write them all manually. That is 
why adis16480_update_scan_mode is used for checking the correct channels 
selection.

Ramona G.