[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAFXKEHZ7bNuy+6X4NgPPMOsT3AkSXhacH=3HS33bLTNgYa3PWg@mail.gmail.com>
Date: Sat, 28 Jun 2025 23:10:44 +0200
From: Lothar Rubusch <l.rubusch@...il.com>
To: Jonathan Cameron <jic23@...nel.org>
Cc: Andy Shevchenko <andriy.shevchenko@...el.com>, lars@...afoo.de,
Michael.Hennerich@...log.com, dlechner@...libre.com, nuno.sa@...log.com,
andy@...nel.org, corbet@....net, linux-iio@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org, eraretuya@...il.com
Subject: Re: [PATCH v10 4/7] iio: accel: adxl345: add inactivity feature
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@...nel.org> 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
Powered by blists - more mailing lists