[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<FR2PPF4571F02BC6BF945DD634338CC5D6D8C00A@FR2PPF4571F02BC.DEUP281.PROD.OUTLOOK.COM>
Date: Thu, 4 Sep 2025 13:06:23 +0000
From: Remi Buisson <Remi.Buisson@....com>
To: Jonathan Cameron <jic23@...nel.org>,
Remi Buisson via B4 Relay
<devnull+remi.buisson.tdk.com@...nel.org>
CC: David Lechner <dlechner@...libre.com>,
Nuno Sá
<nuno.sa@...log.com>,
Andy Shevchenko <andy@...nel.org>, Rob Herring
<robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley
<conor+dt@...nel.org>,
"linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>,
"linux-iio@...r.kernel.org"
<linux-iio@...r.kernel.org>,
"devicetree@...r.kernel.org"
<devicetree@...r.kernel.org>
Subject: RE: [PATCH v5 4/9] iio: imu: inv_icm45600: add IMU IIO gyroscope
device
>
>
>From: Jonathan Cameron <jic23@...nel.org>
>Sent: Monday, August 25, 2025 12:56 PM
>To: Remi Buisson via B4 Relay <devnull+remi.buisson.tdk.com@...nel.org>
>Cc: Remi Buisson <Remi.Buisson@....com>; David Lechner <dlechner@...libre.com>; Nuno Sá <nuno.sa@...log.com>; Andy Shevchenko <andy@...nel.org>; Rob Herring <robh@...nel.org>; Krzysztof Kozlowski <krzk+dt@...nel.org>; Conor Dooley <conor+dt@...nel.org>; linux-kernel@...r.kernel.org; linux-iio@...r.kernel.org; devicetree@...r.kernel.org
>Subject: Re: [PATCH v5 4/9] iio: imu: inv_icm45600: add IMU IIO gyroscope device
>
>On Wed, 20 Aug 2025 14:24:22 +0000
>Remi Buisson via B4 Relay <devnull+remi.buisson.tdk.com@...nel.org> wrote:
>
>> From: Remi Buisson <remi.buisson@....com>
>>
>> Add IIO device for gyroscope sensor
>> with data polling interface and FIFO parsing.
>
>Wrap at 75 chars for commit messages.
This commit message does not use more than that.
But I will fix it in the cover letter patch.
>
>> Attributes: raw, scale, sampling_frequency, calibbias.
>> Temperature is available as a processed channel.
>>
>> Signed-off-by: Remi Buisson <remi.buisson@....com>
>> ---
>
>Only thing I noticed is some accelerometer calls are made
>where the definitions are in the next patch. To sanity check this
>tweak the Kconfig to allow you to build it and make sure that it
>builds after each patch. At the end of the day we don't want to have
>this build without the bus drivers, but hacking it to test 'it could'
>is a great way to avoid issues of code in wrong patches etc, missing
>definitions etc.
>
>Jonathan
Thanks for this advice, compilation is actually quite easy to test.
>
>> drivers/iio/imu/inv_icm45600/Kconfig | 2 +
>> drivers/iio/imu/inv_icm45600/Makefile | 1 +
>> drivers/iio/imu/inv_icm45600/inv_icm45600.h | 29 +
>> drivers/iio/imu/inv_icm45600/inv_icm45600_buffer.c | 76 +-
>> drivers/iio/imu/inv_icm45600/inv_icm45600_buffer.h | 5 +-
>> drivers/iio/imu/inv_icm45600/inv_icm45600_core.c | 104 ++-
>> drivers/iio/imu/inv_icm45600/inv_icm45600_gyro.c | 792 +++++++++++++++++++++
>> 7 files changed, 1006 insertions(+), 3 deletions(-)
>
>> diff --git a/drivers/iio/imu/inv_icm45600/inv_icm45600_buffer.c b/drivers/iio/imu/inv_icm45600/inv_icm45600_buffer.c
>> index 50fd21a24e34decfbe10426946a51c61353eb6a9..5bf9535e27e8942312fe9749ce0c733149de0a9d 100644
>> --- a/drivers/iio/imu/inv_icm45600/inv_icm45600_buffer.c
>> +++ b/drivers/iio/imu/inv_icm45600/inv_icm45600_buffer.c
>
>> @@ -459,6 +462,77 @@ int inv_icm45600_buffer_fifo_read(struct inv_icm45600_state *st)
>> return 0;
>> }
>>
>> +int inv_icm45600_buffer_fifo_parse(struct inv_icm45600_state *st)
>> +{
>> + struct inv_icm45600_sensor_state *gyro_st = iio_priv(st->indio_gyro);
>> + struct inv_icm45600_sensor_state *accel_st = iio_priv(st->indio_accel);
>> + struct inv_sensors_timestamp *ts;
>> + int ret;
>> +
>> + if (st->fifo.nb.total == 0)
>> + return 0;
>> +
>> + /* Handle gyroscope timestamp and FIFO data parsing. */
>> + if (st->fifo.nb.gyro > 0) {
>> + ts = &gyro_st->ts;
>> + inv_sensors_timestamp_interrupt(ts, st->fifo.watermark.eff_gyro,
>> + st->timestamp.gyro);
>> + ret = inv_icm45600_gyro_parse_fifo(st->indio_gyro);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> + /* Handle accelerometer timestamp and FIFO data parsing. */
>> + if (st->fifo.nb.accel > 0) {
>> + ts = &accel_st->ts;
>> + inv_sensors_timestamp_interrupt(ts, st->fifo.watermark.eff_accel,
>> + st->timestamp.accel);
>> + ret = inv_icm45600_accel_parse_fifo(st->indio_accel);
>
>As below. No accel stuff available yet.
Correct, will push to next patch.
>
>> + if (ret)
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +int inv_icm45600_buffer_hwfifo_flush(struct inv_icm45600_state *st,
>> + unsigned int count)
>> +{
>> + struct inv_icm45600_sensor_state *gyro_st = iio_priv(st->indio_gyro);
>> + struct inv_icm45600_sensor_state *accel_st = iio_priv(st->indio_accel);
>> + struct inv_sensors_timestamp *ts;
>> + s64 gyro_ts, accel_ts;
>> + int ret;
>> +
>> + gyro_ts = iio_get_time_ns(st->indio_gyro);
>> + accel_ts = iio_get_time_ns(st->indio_accel);
>> +
>> + ret = inv_icm45600_buffer_fifo_read(st, count);
>> + if (ret)
>> + return ret;
>> +
>> + if (st->fifo.nb.total == 0)
>> + return 0;
>> +
>> + if (st->fifo.nb.gyro > 0) {
>> + ts = &gyro_st->ts;
>> + inv_sensors_timestamp_interrupt(ts, st->fifo.nb.gyro, gyro_ts);
>> + ret = inv_icm45600_gyro_parse_fifo(st->indio_gyro);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> + if (st->fifo.nb.accel > 0) {
>> + ts = &accel_st->ts;
>> + inv_sensors_timestamp_interrupt(ts, st->fifo.nb.accel, accel_ts);
>> + ret = inv_icm45600_accel_parse_fifo(st->indio_accel);
>
>This call should be in the next patch as it's not defined yet.
Sure.
>
>
>> + if (ret)
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>
Powered by blists - more mailing lists