Re: [PATCH v9 08/11] iio: accel: adxl345: add inactivity feature

[Andy Shevchenko]

On Sat, Jun 14, 2025 at 4:55 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:

...

> > >     if (type == ADXL345_ACTIVITY) {
> > >             axis_ctrl = ADXL345_ACT_X_EN | ADXL345_ACT_Y_EN |
> > >                             ADXL345_ACT_Z_EN;
> > >     } else {
> > > -           axis_ctrl = 0x00;
> > > +           axis_ctrl = ADXL345_INACT_X_EN | ADXL345_INACT_Y_EN |
> > > +                           ADXL345_INACT_Z_EN;
> > >     }
> >
> > Now this can be as simple as
> >
> >       axis_ctrl = ADXL345_ACT_X_EN;
>
> That flag is only set in the activity case.  Confused with ADXL345_INACT_X_EN?
> (initially I thought you'd run into a bug!)

Ouch, you are right! Please, discard my above suggestion, it's indeed
simply wrong.

> >       if (type == ADXL345_ACTIVITY)
> >               axis_ctrl |= ADXL345_ACT_Y_EN | ADXL345_ACT_Z_EN;
> >       else
> >               axis_ctrl |= ADXL345_INACT_Y_EN | ADXL345_INACT_Z_EN;
> >
> > Yeah, I don't know how to make the diff better (it gets worse), but the end
> > result is better.
> >
> > One way, which I don't like much is to previously have this conditional written as:
> >
> >       axis_ctrl = ADXL345_ACT_X_EN;
> >       if (type == ADXL345_ACTIVITY)
> >               axis_ctrl |= ADXL345_ACT_Y_EN | ADXL345_ACT_Z_EN;
> >       else
> >               axis_ctrl = 0;
> >


-- 
With Best Regards,
Andy Shevchenko