lkml.org 
[lkml]   [2024]   [Apr]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH 4/5] iio: adis16480: add support for adis16545/7 families
Date
> -----Original Message-----
> From: Nuno Sá <noname.nuno@gmail.com>
> Sent: Monday, April 29, 2024 10:59 AM
> To: Jonathan Cameron <jic23@kernel.org>; Ramona Gradinariu
> <ramona.bolboaca13@gmail.com>
> Cc: linux-kernel@vger.kernel.org; linux-iio@vger.kernel.org; linux-
> doc@vger.kernel.org; devicetree@vger.kernel.org; corbet@lwn.net;
> conor+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; robh@kernel.org;
> Gradinariu, Ramona <Ramona.Gradinariu@analog.com>; Sa, Nuno
> <Nuno.Sa@analog.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@gmail.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@analog.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@analog.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.
\
 
 \ /
  Last update: 2024-04-29 15:18    [W:0.074 / U:0.872 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site