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:
 <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

Powered by Openwall GNU/*/Linux Powered by OpenVZ