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;
> > }
>
