On Fri, 21 Feb 2025 20:33:52 +0000 Lothar Rubusch <l.rubusch@xxxxxxxxx> wrote: > Fix setting the odr value to update activity time based on frequency > derrived by recent odr, and not by obsolete odr value. > > The [small] bug: When _adxl367_set_odr() is called with a new odr value, > it first writes the new odr value to the hardware register > ADXL367_REG_FILTER_CTL. > Second, it calls _adxl367_set_act_time_ms(), which calls > adxl367_time_ms_to_samples(). Here st->odr still holds the old odr value. > This st->odr member is used to derrive a frequency value, which is > applied to update ADXL367_REG_TIME_ACT. Hence, the idea is to update > activity time, based on possibilities and power consumption by the > current ODR rate. > Finally, when the function calls return, again in _adxl367_set_odr() the > new ODR is assigned to st->odr. > > The fix: When setting a new ODR value is set to ADXL367_REG_FILTER_CTL, > also ADXL367_REG_TIME_ACT should probably be updated with a frequency > based on the recent ODR value and not the old one. Changing the location > of the assignment to st->odr fixes this. > > Signed-off-by: Lothar Rubusch <l.rubusch@xxxxxxxxx> Fixes tag? Otherwise looks good to me. > --- > drivers/iio/accel/adxl367.c | 10 +++------- > 1 file changed, 3 insertions(+), 7 deletions(-) > > diff --git a/drivers/iio/accel/adxl367.c b/drivers/iio/accel/adxl367.c > index add4053e7a02..0c04b2bb7efb 100644 > --- a/drivers/iio/accel/adxl367.c > +++ b/drivers/iio/accel/adxl367.c > @@ -601,18 +601,14 @@ static int _adxl367_set_odr(struct adxl367_state *st, enum adxl367_odr odr) > if (ret) > return ret; > > + st->odr = odr; > + > /* Activity timers depend on ODR */ > ret = _adxl367_set_act_time_ms(st, st->act_time_ms); > if (ret) > return ret; > > - ret = _adxl367_set_inact_time_ms(st, st->inact_time_ms); > - if (ret) > - return ret; > - > - st->odr = odr; > - > - return 0; > + return _adxl367_set_inact_time_ms(st, st->inact_time_ms); > } > > static int adxl367_set_odr(struct iio_dev *indio_dev, enum adxl367_odr odr)
