Re: [PATCH v7 1/7] iio: accel: adxl345: add function to switch measuring mode

[Christophe JAILLET]

Le 15/12/2024 à 10:32, Lothar Rubusch a écrit :
> On Sat, Dec 14, 2024 at 12:33 PM Christophe JAILLET
> <christophe.jaillet-39ZsbGIQGT5GWvitb5QawA@xxxxxxxxxxxxxxxx> wrote:
> > Le 13/12/2024 à 22:19, Lothar Rubusch a écrit :
> > > Replace the powerup / powerdown functions by a generic function to put
> > > the sensor in STANDBY, or MEASURE mode. When configuring the FIFO for
> > > several features of the accelerometer, it is recommended to put
> > > measuring in STANDBY mode.
> >
> > ...
> >
> > > +static int adxl345_set_measure_en(struct adxl345_state *st, bool en)
> > > +{
> > > +     unsigned int val = 0;
> >
> > Nitpick: useless init
> >
> > > +
> > > +     val = (en) ? ADXL345_POWER_CTL_MEASURE : ADXL345_POWER_CTL_STANDBY;
> >
> > Nitpick: useless () around en.
>
> Thank you for pointing out. I agree. This is better just in one line,
> initialization directly and no parens. Anyway, since I already can see
> the patch on the iio branch, I'll probably better leave it as is for
> now (?).

It is just a nitpick, it can stay as-is forever.

> Question: Since I'm currently about to build up tooling for linters
> and static checkers. I'm doing checkpatch. I'm running Dan's smatch,
> now. I'm correcting indention/spaces/tabs by some emacs settings.
> DT/bindings I was doing wrong, and just figured out why after
> submitting the last patch (...).
>
> What is the best approach to find such kind of nitpick issues automatically?

I don't know any.
I only uses ./scripts/checkpatch.pl --strict

I guess that some coccinelle scripts could also spot some things, (the 
unneeded init in this case), but I don't really think that the effort 
would worth it.

Maybe using -Wall and or -Wextra gcc gcc would also trigger additional 
warning that could look relevant?


Adding Dan if he has more ideas.

CJ

> Best,
> L
>
> > > +     return regmap_write(st->regmap, ADXL345_REG_POWER_CTL, val);
> > > +}
> >
> > ...