Received-SPF: pass (google.com: domain of linux-kernel+bounces-161884-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <e62f8df4b06abc371b1e9fe3232cb593e468d54c.camel@gmail.com>
Subject: Re: [PATCH 4/5] iio: adis16480: add support for adis16545/7 families
From: Nuno =?ISO-8859-1?Q?S=E1?= <noname.nuno@gmail.com>
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,
 Ramona Gradinariu <ramona.gradinariu@analog.com>, Nuno
 =?ISO-8859-1?Q?S=E1?= <nuno.sa@analog.com>
Date: Mon, 29 Apr 2024 09:58:40 +0200
In-Reply-To: <20240428162555.3ddf31ea@jic23-huawei>
References: <20240423084210.191987-1-ramona.gradinariu@analog.com>
	 <20240423084210.191987-5-ramona.gradinariu@analog.com>
	 <20240428162555.3ddf31ea@jic23-huawei>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.52.1 
Precedence: bulk
MIME-Version: 1.0

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:
>=20
> > 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.
> >=20
> > 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.
> >=20
> > Signed-off-by: Nuno S=C3=A1 <nuno.sa@analog.com>
>=20
> What is Nuno's relationship to this patch?=C2=A0 You are author and the s=
ender
> which is fine, but in that case you need to make Nuno's involvement expli=
cit.
> Perhaps a Co-developed-by or similar is appropriate?
>=20
> > Signed-off-by: Ramona Gradinariu <ramona.gradinariu@analog.com>
> A few comments inline.=C2=A0 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.=C2=A0 My guess is th=
at the
> transition is very time consuming?=C2=A0 If so, that is fine, but you sho=
uld 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 th=
e
channels data at once but for the accel/gyro you need to explicitly configu=
re
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 w=
e
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 reme=
mber
but this implementation goes in line with what we have in the adis16475 dri=
ver.

- Nuno S=C3=A1