On Sat, Feb 22, 2025 at 4:03 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote: > > 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? > Hi IIO ML readers - Hi Jonathan, AFAIK there is no tracked bug which I could refer to. Alternatively, I could refer to the commit hash of the original commit which introduced the code this patch is supposed to fix. Is this ok? Could you please help me here with the process? > 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) >
