Re: [PATCH] iio: invensense: fix timestamp glitches when switching frequency

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 26 Apr 2024 09:48:35 +0000
inv.git-commit@xxxxxxx wrote:

> From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@xxxxxxx>
> 
> When a sensor is running and there is a FIFO frequency change due to
> another sensor turned on/off, there are glitches on timestamp. Fix that
> by using only interrupt timestamp when there is the corresponding sensor
> data in the FIFO.
> 
> Delete FIFO period handling and simplify internal functions.
> 
> Update integration inside inv_mpu6050 and inv_icm42600 drivers.
> 
> Fixes: 0ecc363ccea7 ("iio: make invensense timestamp module generic)
> CC: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@xxxxxxx>

Whilst I don't fully follow the logic here, the new code is simpler
and seems reasonable.  Getting my head around this will probably take
longer than it's worth :(

Hence applied to the fixes-togreg branch of iio.git.

Jonathan

> ---
>  .../inv_sensors/inv_sensors_timestamp.c       | 24 +++++++++----------
>  .../imu/inv_icm42600/inv_icm42600_buffer.c    | 20 +++++++---------
>  drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c    |  2 +-
>  .../linux/iio/common/inv_sensors_timestamp.h  |  3 +--
>  4 files changed, 21 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/iio/common/inv_sensors/inv_sensors_timestamp.c b/drivers/iio/common/inv_sensors/inv_sensors_timestamp.c
> index 3b0f9598a7c7..5f3ba77da740 100644
> --- a/drivers/iio/common/inv_sensors/inv_sensors_timestamp.c
> +++ b/drivers/iio/common/inv_sensors/inv_sensors_timestamp.c
> @@ -70,13 +70,13 @@ int inv_sensors_timestamp_update_odr(struct inv_sensors_timestamp *ts,
>  }
>  EXPORT_SYMBOL_NS_GPL(inv_sensors_timestamp_update_odr, IIO_INV_SENSORS_TIMESTAMP);
> 
> -static bool inv_validate_period(struct inv_sensors_timestamp *ts, uint32_t period, uint32_t mult)
> +static bool inv_validate_period(struct inv_sensors_timestamp *ts, uint32_t period)
>  {
>  	uint32_t period_min, period_max;
> 
>  	/* check that period is acceptable */
> -	period_min = ts->min_period * mult;
> -	period_max = ts->max_period * mult;
> +	period_min = ts->min_period * ts->mult;
> +	period_max = ts->max_period * ts->mult;
>  	if (period > period_min && period < period_max)
>  		return true;
>  	else
> @@ -84,15 +84,15 @@ static bool inv_validate_period(struct inv_sensors_timestamp *ts, uint32_t perio
>  }
> 
>  static bool inv_update_chip_period(struct inv_sensors_timestamp *ts,
> -				    uint32_t mult, uint32_t period)
> +				   uint32_t period)
>  {
>  	uint32_t new_chip_period;
> 
> -	if (!inv_validate_period(ts, period, mult))
> +	if (!inv_validate_period(ts, period))
>  		return false;
> 
>  	/* update chip internal period estimation */
> -	new_chip_period = period / mult;
> +	new_chip_period = period / ts->mult;
>  	inv_update_acc(&ts->chip_period, new_chip_period);
>  	ts->period = ts->mult * ts->chip_period.val;
> 
> @@ -120,16 +120,14 @@ static void inv_align_timestamp_it(struct inv_sensors_timestamp *ts)
>  }
> 
>  void inv_sensors_timestamp_interrupt(struct inv_sensors_timestamp *ts,
> -				      uint32_t fifo_period, size_t fifo_nb,
> -				      size_t sensor_nb, int64_t timestamp)
> +				     size_t sample_nb, int64_t timestamp)
>  {
>  	struct inv_sensors_timestamp_interval *it;
>  	int64_t delta, interval;
> -	const uint32_t fifo_mult = fifo_period / ts->chip.clock_period;
>  	uint32_t period;
>  	bool valid = false;
> 
> -	if (fifo_nb == 0)
> +	if (sample_nb == 0)
>  		return;
> 
>  	/* update interrupt timestamp and compute chip and sensor periods */
> @@ -139,14 +137,14 @@ void inv_sensors_timestamp_interrupt(struct inv_sensors_timestamp *ts,
>  	delta = it->up - it->lo;
>  	if (it->lo != 0) {
>  		/* compute period: delta time divided by number of samples */
> -		period = div_s64(delta, fifo_nb);
> -		valid = inv_update_chip_period(ts, fifo_mult, period);
> +		period = div_s64(delta, sample_nb);
> +		valid = inv_update_chip_period(ts, period);
>  	}
> 
>  	/* no previous data, compute theoritical value from interrupt */
>  	if (ts->timestamp == 0) {
>  		/* elapsed time: sensor period * sensor samples number */
> -		interval = (int64_t)ts->period * (int64_t)sensor_nb;
> +		interval = (int64_t)ts->period * (int64_t)sample_nb;
>  		ts->timestamp = it->up - interval;
>  		return;
>  	}
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
> index b52f328fd26c..9cde9a9337ad 100644
> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
> @@ -509,20 +509,20 @@ int inv_icm42600_buffer_fifo_parse(struct inv_icm42600_state *st)
>  		return 0;
> 
>  	/* handle gyroscope timestamp and FIFO data parsing */
> -	ts = iio_priv(st->indio_gyro);
> -	inv_sensors_timestamp_interrupt(ts, st->fifo.period, st->fifo.nb.total,
> -					st->fifo.nb.gyro, st->timestamp.gyro);
>  	if (st->fifo.nb.gyro > 0) {
> +		ts = iio_priv(st->indio_gyro);
> +		inv_sensors_timestamp_interrupt(ts, st->fifo.nb.gyro,
> +						st->timestamp.gyro);
>  		ret = inv_icm42600_gyro_parse_fifo(st->indio_gyro);
>  		if (ret)
>  			return ret;
>  	}
> 
>  	/* handle accelerometer timestamp and FIFO data parsing */
> -	ts = iio_priv(st->indio_accel);
> -	inv_sensors_timestamp_interrupt(ts, st->fifo.period, st->fifo.nb.total,
> -					st->fifo.nb.accel, st->timestamp.accel);
>  	if (st->fifo.nb.accel > 0) {
> +		ts = iio_priv(st->indio_accel);
> +		inv_sensors_timestamp_interrupt(ts, st->fifo.nb.accel,
> +						st->timestamp.accel);
>  		ret = inv_icm42600_accel_parse_fifo(st->indio_accel);
>  		if (ret)
>  			return ret;
> @@ -550,9 +550,7 @@ int inv_icm42600_buffer_hwfifo_flush(struct inv_icm42600_state *st,
> 
>  	if (st->fifo.nb.gyro > 0) {
>  		ts = iio_priv(st->indio_gyro);
> -		inv_sensors_timestamp_interrupt(ts, st->fifo.period,
> -						st->fifo.nb.total, st->fifo.nb.gyro,
> -						gyro_ts);
> +		inv_sensors_timestamp_interrupt(ts, st->fifo.nb.gyro, gyro_ts);
>  		ret = inv_icm42600_gyro_parse_fifo(st->indio_gyro);
>  		if (ret)
>  			return ret;
> @@ -560,9 +558,7 @@ int inv_icm42600_buffer_hwfifo_flush(struct inv_icm42600_state *st,
> 
>  	if (st->fifo.nb.accel > 0) {
>  		ts = iio_priv(st->indio_accel);
> -		inv_sensors_timestamp_interrupt(ts, st->fifo.period,
> -						st->fifo.nb.total, st->fifo.nb.accel,
> -						accel_ts);
> +		inv_sensors_timestamp_interrupt(ts, st->fifo.nb.accel, accel_ts);
>  		ret = inv_icm42600_accel_parse_fifo(st->indio_accel);
>  		if (ret)
>  			return ret;
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> index 86465226f7e1..0dc0f22a5582 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> @@ -100,7 +100,7 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
>  		goto end_session;
>  	/* Each FIFO data contains all sensors, so same number for FIFO and sensor data */
>  	fifo_period = NSEC_PER_SEC / INV_MPU6050_DIVIDER_TO_FIFO_RATE(st->chip_config.divider);
> -	inv_sensors_timestamp_interrupt(&st->timestamp, fifo_period, nb, nb, pf->timestamp);
> +	inv_sensors_timestamp_interrupt(&st->timestamp, nb, pf->timestamp);
>  	inv_sensors_timestamp_apply_odr(&st->timestamp, fifo_period, nb, 0);
> 
>  	/* clear internal data buffer for avoiding kernel data leak */
> diff --git a/include/linux/iio/common/inv_sensors_timestamp.h b/include/linux/iio/common/inv_sensors_timestamp.h
> index a47d304d1ba7..8d506f1e9df2 100644
> --- a/include/linux/iio/common/inv_sensors_timestamp.h
> +++ b/include/linux/iio/common/inv_sensors_timestamp.h
> @@ -71,8 +71,7 @@ int inv_sensors_timestamp_update_odr(struct inv_sensors_timestamp *ts,
>  				     uint32_t period, bool fifo);
> 
>  void inv_sensors_timestamp_interrupt(struct inv_sensors_timestamp *ts,
> -				     uint32_t fifo_period, size_t fifo_nb,
> -				     size_t sensor_nb, int64_t timestamp);
> +				     size_t sample_nb, int64_t timestamp);
> 
>  static inline int64_t inv_sensors_timestamp_pop(struct inv_sensors_timestamp *ts)
>  {
> --
> 2.34.1
> 





[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux