Re: [PATCH v10 4/7] iio: accel: adxl345: add inactivity feature

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Jonathan, Andy and the ML,
Thank you both for the review and feedback. I'll prepare another
version for the 313 and the 345.

On Sat, Jun 28, 2025 at 6:08 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>
>
> > ...
> >
> > > +   case ADXL345_INACTIVITY:
> > > +           axis_ctrl = ADXL345_INACT_X_EN | ADXL345_INACT_Y_EN |
> > > +                           ADXL345_INACT_Z_EN;
> >
> > Consider
> >               axis_ctrl =
> >                       ADXL345_INACT_X_EN | ADXL345_INACT_Y_EN | ADXL345_INACT_Z_EN;
> >
> > (yes, I see that it's longer than 80, but it might worth doing it for the sake of
> >  consistency with the previous suggestion).
> Hmm. I'd go longer rather than do that just because it looks really ugly.
>
>                 axis_ctrl = ADXL345_INACT_X_EN | ADXL345_INACT_Y_EN | ADXL345_INACT_Z_EN;
>
> I don't care that much as long as long lines are justified by readability. Here
> I think either Andy's suggestion or the all on one line are justified.
>
> Tomorrow I may have a different view :(
>

As I’ve seen quite a bit of discussion around this. In fact, using
binary OR here might not even be necessary, since I can define
ADXL345_ACT_XYZ_EN and ADXL345_INACT_XYZ_EN directly and OR the fields
in the header. If you have no objections, I’ll likely prepare this
change for the next version.

Best,
L





[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux