linux-kernel - RE: [RESEND PATCH v8 5/6] iio: imu: adis16550: add adis16550 support

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: <45d64de8a1074788b7c4bffc29788742@analog.com>
Date: Thu, 20 Feb 2025 13:43:04 +0000
From: "Budai, Robert" <Robert.Budai@...log.com>
To: Nuno Sá <noname.nuno@...il.com>,
        Lars-Peter Clausen
	<lars@...afoo.de>,
        "Hennerich, Michael" <Michael.Hennerich@...log.com>,
        "Sa,
 Nuno" <Nuno.Sa@...log.com>,
        "Gradinariu, Ramona"
	<Ramona.Gradinariu@...log.com>,
        "Miclaus, Antoniu"
	<Antoniu.Miclaus@...log.com>,
        Jonathan Cameron <jic23@...nel.org>, "Rob
 Herring" <robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        "Conor
 Dooley" <conor+dt@...nel.org>,
        Jonathan Corbet <corbet@....net>,
        "linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>
Subject: RE: [RESEND PATCH v8 5/6] iio: imu: adis16550: add adis16550 support



> -----Original Message-----
> From: Nuno Sá <noname.nuno@...il.com>
> Sent: Thursday, February 20, 2025 10:22 AM
> To: Budai, Robert <Robert.Budai@...log.com>; Lars-Peter Clausen
> <lars@...afoo.de>; Hennerich, Michael <Michael.Hennerich@...log.com>;
> Sa, Nuno <Nuno.Sa@...log.com>; Gradinariu, Ramona
> <Ramona.Gradinariu@...log.com>; Miclaus, Antoniu
> <Antoniu.Miclaus@...log.com>; Jonathan Cameron <jic23@...nel.org>; Rob
> Herring <robh@...nel.org>; Krzysztof Kozlowski <krzk+dt@...nel.org>; Conor
> Dooley <conor+dt@...nel.org>; Jonathan Corbet <corbet@....net>; linux-
> iio@...r.kernel.org; devicetree@...r.kernel.org; linux-kernel@...r.kernel.org;
> linux-doc@...r.kernel.org
> Subject: Re: [RESEND PATCH v8 5/6] iio: imu: adis16550: add adis16550
> support
> 
> [External]
> 
> On Mon, 2025-02-17 at 12:57 +0200, Robert Budai wrote:
> > The ADIS16550 is a complete inertial system that includes a triaxis
> > gyroscope and a triaxis accelerometer. Each inertial sensor in the
> > ADIS16550 combines industry leading MEMS only technology with signal
> > conditioning that optimizes dynamic performance. The factory calibration
> > characterizes each sensor for sensitivity, bias, and alignment. As a
> > result, each sensor has its own dynamic compensation formulas that
> > provide accurate sensor measurements.
> >
> > Co-developed-by: Ramona Gradinariu <ramona.gradinariu@...log.com>
> > Signed-off-by: Ramona Gradinariu <ramona.gradinariu@...log.com>
> > Co-developed-by: Antoniu Miclaus <antoniu.miclaus@...log.com>
> > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@...log.com>
> > Signed-off-by: Nuno Sá <nuno.sa@...log.com>
> > Signed-off-by: Robert Budai <robert.budai@...log.com>
> > ---
> >
> 
> I guess it would make sense a Co-developed-by: for Robert?
> 
> Anyways, all looks good except for one thing that I just spotted...
> 
> > v8:
> > - removed __aligned from struct adis16550, as suggested
> > - crc buffer extraction into the crc check function
> > - passed buffer into crc validation as original, __be32 and performed check
> > using be32_to_cpu conversion of the buffer
> > - added trailing comma to line 993
> > - removed trailing comma from line 877
> >
> >  drivers/iio/imu/Kconfig     |   13 +
> >  drivers/iio/imu/Makefile    |    1 +
> >  drivers/iio/imu/adis16550.c | 1149
> +++++++++++++++++++++++++++++++++++
> >  3 files changed, 1163 insertions(+)
> >  create mode 100644 drivers/iio/imu/adis16550.c
> >
> 
> ...
> 
> >
> > +static int adis16550_set_freq_hz(struct adis16550 *st, u32 freq_hz)
> > +{
> > +	u16 dec;
> > +	int ret;
> > +	u32 sample_rate = st->clk_freq_hz;
> > +	/*
> > +	 * The optimal sample rate for the supported IMUs is between
> > +	 * int_clk - 1000 and int_clk + 500.
> > +	 */
> > +	u32 max_sample_rate = st->info->int_clk * 1000 + 500000;
> > +	u32 min_sample_rate = st->info->int_clk * 1000 - 1000000;
> > +
> > +	if (!freq_hz)
> > +		return -EINVAL;
> > +
> > +	adis_dev_auto_lock(&st->adis);
> > +
> > +	if (st->sync_mode == ADIS16550_SYNC_MODE_SCALED) {
> > +		unsigned long scaled_rate = lcm(st->clk_freq_hz, freq_hz);
> > +		int sync_scale;
> > +
> > +		if (scaled_rate > max_sample_rate)
> > +			scaled_rate = max_sample_rate / st->clk_freq_hz * st-
> > >clk_freq_hz;
> > +		else
> > +			scaled_rate = max_sample_rate / scaled_rate *
> > scaled_rate;
> > +
> > +		if (scaled_rate < min_sample_rate)
> > +			scaled_rate = roundup(min_sample_rate, st-
> > >clk_freq_hz);
> > +
> 
> I would imagine the above is the same deal as in other devices [1] or do you
> know for a fact this one is different? Maybe it's simple enough for Jonathan to
> tweak while applying...
> 
> [1]:
> https://urldefense.com/v3/__https://elixir.bootlin.com/linux/v6.13.3/source
> /drivers/iio/imu/adis16475.c*L364__;Iw!!A3Ni8CS0y2Y!7Y71yPaQAxVzNRd
> O_jT7wEz4k-
> s6z4tJHOcES84HYkq8qNGsgJH7zxwjfPNjLF3OEGVInSolo1ennLU_mwpmEbo$
> 
> - Nuno Sá

[Robert Budai] 
No differences were found in the scaled_sync behavior of the ADIS16475 and
ADIS16550. It is safe to add from my side.

Best regards,
Robert B