Re: [PATCH 2/4] iio: imu: inv_icm42600: use 2 sensors fixed packet size of 16 bytes

[Jean-Baptiste Maneyrol]

>
>
>________________________________________
>From: Jonathan Cameron <jic23@xxxxxxxxxx>
>Sent: Friday, August 21, 2026 02:09
>To: Jean-Baptiste Maneyrol via B4 Relay
>Cc: Jean-Baptiste Maneyrol; David Lechner; Nuno Sá; Andy Shevchenko; linux-iio@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Jean-Baptiste Maneyrol
>Subject: Re: [PATCH 2/4] iio: imu: inv_icm42600: use 2 sensors fixed packet size of 16 bytes
>
>On Thu, 20 Aug 2026 21: 02: 38 +0200 Jean-Baptiste Maneyrol via B4 Relay <devnull+jean-baptiste. maneyrol. tdk. com@ kernel. org> wrote: > From: Jean-Baptiste Maneyrol <jean-baptiste. maneyrol@ tdk. com> > > Sometimes dynamic switch
>ZjQcmQRYFpfptBannerStart
>This Message Is From an External Sender
>This message came from outside your organization.
>
>ZjQcmQRYFpfptBannerEnd
>
>On Thu, 20 Aug 2026 21:02:38 +0200
>Jean-Baptiste Maneyrol via B4 Relay <devnull+jean-baptiste.maneyrol.tdk.com@xxxxxxxxxx> wrote:
>
>> From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@xxxxxxx>
>>
>> Sometimes dynamic switch between 1 sensor frame to 2 sensors frame is
>> not working when there are too much frames already in the FIFO. By
>> moving accel/gyro on sleep before turning FIFO on, we are storing many
>> FIFO frames before updating the frame format hitting the bug everytime.
>>
>> Fix that by always using the 2 sensors frame of 16 bytes. Also update
>> the hwfifo max watermark reported.
>>
>> Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@xxxxxxx>
>
>So from that description this sometimes happens without the reorder?
>If so is it appropriate to give this a Fixes tag?

Hello Jonathan,

it may be possible, but I never encountered it before. That's why I didn't
make a fixes tag.

>
>> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
>> index 998d312f7bde..043ae9deee65 100644
>> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
>> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
>> @@ -131,13 +131,13 @@ int inv_icm42600_buffer_set_fifo_en(struct inv_icm42600_state *st,
>>               INV_ICM42600_FIFO_CONFIG1_GYRO_EN |
>>               INV_ICM42600_FIFO_CONFIG1_ACCEL_EN;
>>
>> -     val = 0;
>> -     if (fifo_en & INV_ICM42600_SENSOR_GYRO)
>> -             val |= INV_ICM42600_FIFO_CONFIG1_GYRO_EN;
>> -     if (fifo_en & INV_ICM42600_SENSOR_ACCEL)
>> -             val |= INV_ICM42600_FIFO_CONFIG1_ACCEL_EN;
>> -     if (fifo_en & INV_ICM42600_SENSOR_TEMP)
>> -             val |= INV_ICM42600_FIFO_CONFIG1_TEMP_EN;
>> +     /* always enable/disable all bits for allowing dynamic transitions */
>
>This needs a little more.  It makes sense in the context of
>this patch but will seem rather odd if we look at it sometime in
>the distant future.  Dynamic transitions of what?

It is dynamic transition of the FIFO frame format from 8 bits to 16 bits. I
will reword that in V2.

>
>> +     if (fifo_en)
>> +             val = INV_ICM42600_FIFO_CONFIG1_TEMP_EN |
>> +                   INV_ICM42600_FIFO_CONFIG1_GYRO_EN |
>> +                   INV_ICM42600_FIFO_CONFIG1_ACCEL_EN;
>> +     else
>> +             val = 0;
>>
>>       ret = regmap_update_bits(st->map, INV_ICM42600_REG_FIFO_CONFIG1, mask, val);
>>       if (ret)
>> @@ -149,19 +149,6 @@ int inv_icm42600_buffer_set_fifo_en(struct inv_icm42600_state *st,
>>       return 0;
>>  }
>
>

Thanks,
JB