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@xxxxxxxxxx>
> Signed-off-by: Ramona Gradinariu <ramona.gradinariu@xxxxxxxxxx>
> Co-developed-by: Antoniu Miclaus <antoniu.miclaus@xxxxxxxxxx>
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@xxxxxxxxxx>
> Signed-off-by: Nuno Sá <nuno.sa@xxxxxxxxxx>
> Signed-off-by: Robert Budai <robert.budai@xxxxxxxxxx>
> ---
>
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://elixir.bootlin.com/linux/v6.13.3/source/drivers/iio/imu/adis16475.c#L364
- Nuno Sá
