Re: [PATCH v1 02/13] iio: chemical: bme680: avoid using camel case

Andy Shevchenko
2024-10-11T10:00:39Z

On Thu, Oct 10, 2024 at 11:00:19PM +0200, vamoirid wrote:
> From: Vasileios Amoiridis <vassilisamir@gmail.com>
>
> Rename camel case variable, as checkpatch.pl complains.

With given reply to the first patch...

...

> /* Look up table for the possible gas range values */
> - static const u32 lookupTable[16] = {2147483647u, 2147483647u,
> + static const u32 lookup_table[16] = {2147483647u, 2147483647u,
> 2147483647u, 2147483647u, 2147483647u,
> 2126008810u, 2147483647u, 2130303777u,
> 2147483647u, 2147483647u, 2143188679u,

...here is the opportunity to fix indentation while at fixing the code.
I.o.w. I would reformat the entire table to be

static const u32 lookup_table[16] = {
2147483647u, 2147483647u, 2147483647u, 2147483647u,
2147483647u, 2126008810u, 2147483647u, 2130303777u,
2147483647u, 2147483647u, 2143188679u, ...

(also note power-of-2 number of items per line which much easier to read and
find one you need).

...

> var1 = ((1340 + (5 * (s64)calib->range_sw_err)) *
> - ((s64)lookupTable[gas_range])) >> 16;
> + ((s64)lookup_table[gas_range])) >> 16;

Also an opportunity to make this neater like

var1 = (1340 + (5 * (s64)calib->range_sw_err)) * (s64)lookup_table[gas_range]);
var1 >>= 16;

So, at bare minumym there are redundant parentheses. And looking at the table
and the first argument of multiplication I'm puzzled why casting is needed for
the second? Shouldn't s64 already be implied by the first one?

--
With Best Regards,
Andy Shevchenko