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

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

 



On Thu, Oct 10, 2024 at 11:00:19PM +0200, vamoirid wrote:
> From: Vasileios Amoiridis <vassilisamir@xxxxxxxxx>
> 
> 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






[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux