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

vamoirid posted 13 patches 1 month, 2 weeks ago
[PATCH v1 02/13] iio: chemical: bme680: avoid using camel case
Posted by vamoirid 1 month, 2 weeks ago
From: Vasileios Amoiridis <vassilisamir@gmail.com>

Rename camel case variable, as checkpatch.pl complains.

Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
 drivers/iio/chemical/bme680_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/chemical/bme680_core.c b/drivers/iio/chemical/bme680_core.c
index 95a667d56527..3b4431998ca4 100644
--- a/drivers/iio/chemical/bme680_core.c
+++ b/drivers/iio/chemical/bme680_core.c
@@ -437,7 +437,7 @@ static u32 bme680_compensate_gas(struct bme680_data *data, u16 gas_res_adc,
 	u32 calc_gas_res;
 
 	/* 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,
@@ -445,7 +445,7 @@ static u32 bme680_compensate_gas(struct bme680_data *data, u16 gas_res_adc,
 				2147483647u, 2147483647u};
 
 	var1 = ((1340 + (5 * (s64)calib->range_sw_err)) *
-			((s64)lookupTable[gas_range])) >> 16;
+			((s64)lookup_table[gas_range])) >> 16;
 	var2 = ((gas_res_adc << 15) - 16777216) + var1;
 	var3 = ((125000 << (15 - gas_range)) * var1) >> 9;
 	var3 += (var2 >> 1);
-- 
2.43.0
Re: [PATCH v1 02/13] iio: chemical: bme680: avoid using camel case
Posted by Andy Shevchenko 1 month, 2 weeks ago
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
Re: [PATCH v1 02/13] iio: chemical: bme680: avoid using camel case
Posted by Vasileios Aoiridis 1 month, 2 weeks ago
On Fri, Oct 11, 2024 at 01:00:33PM +0300, Andy Shevchenko wrote:
> 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...
>

Hi Andy,

> ...
> 
> >  	/* 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).
> 

ACK.

> ...
> 
> >  	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
> 
>

I think the 2nd cast indeed is not needed since the 1st one forces the
s64 so I can remove it.

Cheers,
Vasilis
Re: [PATCH v1 02/13] iio: chemical: bme680: avoid using camel case
Posted by Andy Shevchenko 1 month, 2 weeks ago
Fri, Oct 11, 2024 at 08:50:27PM +0200, Vasileios Aoiridis kirjoitti:
> On Fri, Oct 11, 2024 at 01:00:33PM +0300, Andy Shevchenko wrote:
> > On Thu, Oct 10, 2024 at 11:00:19PM +0200, vamoirid wrote:

...

> > >  	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?
> 
> I think the 2nd cast indeed is not needed since the 1st one forces the
> s64 so I can remove it.

Thinking about this more, you don't need the first either,
if using 5LL instead of 5.

-- 
With Best Regards,
Andy Shevchenko