> -----Original Message----- > From: Nuno Sá <noname.nuno@xxxxxxxxx> > Sent: Monday, April 29, 2024 10:59 AM > To: Jonathan Cameron <jic23@xxxxxxxxxx>; Ramona Gradinariu > <ramona.bolboaca13@xxxxxxxxx> > Cc: linux-kernel@xxxxxxxxxxxxxxx; linux-iio@xxxxxxxxxxxxxxx; linux- > doc@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; corbet@xxxxxxx; > conor+dt@xxxxxxxxxx; krzysztof.kozlowski+dt@xxxxxxxxxx; robh@xxxxxxxxxx; > Gradinariu, Ramona <Ramona.Gradinariu@xxxxxxxxxx>; Sa, Nuno > <Nuno.Sa@xxxxxxxxxx> > 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@xxxxxxxxx> 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@xxxxxxxxxx> > > > > 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@xxxxxxxxxx> > > 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.
