On Tue, Mar 4, 2025 at 2:40 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote: > > On Thu, 20 Feb 2025 10:42:30 +0000 > Lothar Rubusch <l.rubusch@xxxxxxxxx> wrote: > > > Introduce means to configure and work with the available g-ranges > > keeping the precision of 13 digits. > > > > This is in preparation for the activity/inactivity feature. > > I'm not really following why adding range control is anything > much to do with that. Mostly we do this to improve accuracy for > low accelerations. > As you probably saw the connection comes a bit over the link in adjusting the activity/inactivity parameters (times and threshold) by a given range in the follow up patches. If the question is rather why at all adding this g-range control. My idea was that adjusting i.e. lowering precision, less odr, etc might also help adjusting power consumption. In other words from a user perspective I assume there is more configuration possibility. I did not pretend to tune the implementation for lowest possible power consumption, though. It was just an idea. [Also, I was curious about implementing it here. My patch here is rather meant as a proposal, if you strongly oppose the idea, pls let me know.] > > > > Signed-off-by: Lothar Rubusch <l.rubusch@xxxxxxxxx> > > > > @@ -483,12 +518,48 @@ static int adxl345_set_odr(struct adxl345_state *st, enum adxl345_odr odr) > > return 0; > > } > > > > +static int adxl345_find_range(struct adxl345_state *st, int val, int val2, > > + enum adxl345_range *range) > > +{ > > + int i; > > + > > + for (i = 0; i < ARRAY_SIZE(adxl345_fullres_range_tbl); i++) > > + if (val == adxl345_fullres_range_tbl[i][0] && > > + val2 == adxl345_fullres_range_tbl[i][1]) > > + break; > Similar to case in earlier patch, maybe set *range and return in here > so that any finish of the loop is an error. > > + > > + if (i == ARRAY_SIZE(adxl345_fullres_range_tbl)) > > + return -EINVAL; > > + > > + *range = i; > > + > > + return 0; > > +} > > + > > +static int adxl345_set_range(struct adxl345_state *st, enum adxl345_range range) > > +{ > > + int ret; > > + > > + ret = regmap_update_bits(st->regmap, ADXL345_REG_DATA_FORMAT, > > + ADXL345_DATA_FORMAT_RANGE, > > + FIELD_PREP(ADXL345_DATA_FORMAT_RANGE, range)); > > + if (ret) > > + return ret; > > + > > return regmap_update_bits() unless this gets more complex in later patch. > > > + return 0; > > +} > > + > > > @@ -558,6 +634,7 @@ static int adxl345_write_raw(struct iio_dev *indio_dev, > > int val, int val2, long mask) > > { > > struct adxl345_state *st = iio_priv(indio_dev); > > + enum adxl345_range range; > > enum adxl345_odr odr; > > int ret; > > > > @@ -581,6 +658,12 @@ static int adxl345_write_raw(struct iio_dev *indio_dev, > > return ret; > > ret = adxl345_set_odr(st, odr); > > break; > > + case IIO_CHAN_INFO_SCALE: > > + ret = adxl345_find_range(st, val, val2, &range); > > + if (ret) > > + return ret; > > + ret = adxl345_set_range(st, range); > as in previous I'd have the > if (ret) > return ret; > here for consistency with the other one immediately above this. > > + break; > > default: > > return -EINVAL; > > } >