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

[Christophe JAILLET <christophe.jaillet@wanadoo.fr>]

Le 15/12/2024 à 10:32, Lothar Rubusch a écrit :
> On Sat, Dec 14, 2024 at 12:33 PM Christophe JAILLET
> <christophe.jaillet-39ZsbGIQGT5GWvitb5QawA@public.gmane.org> 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);
>>> +}
>>
>> ...
> 
>