Re: [PATCH] iio: chemical: scd30: Cleanup initializations in scd30_float_to_fp()

[Jonathan Cameron <jic23@kernel.org>]

On Fri,  8 May 2026 17:55:00 -0500
Maxwell Doose <m32285159@gmail.com> wrote:

> The current variable declaration and initializations are barely readable
> and use comma separations across multiple lines. Refactor the
> initializations so that mantissa and exp have separate declarations and
> sign gets initialized later.
> 
> Signed-off-by: Maxwell Doose <m32285159@gmail.com>

This is good but we can take this opportunity to make it more
idiomatic - so more 'standard' for new kernel code.

> ---
>  ps:
>  Hi Jonathan, I noticed a potential divide-by-zero bug on line 241 in
>  scd30_read_raw(), where the value of tmp is dictated by hardware.
>  If the scd30_command_read() call on line 236 assigns 0 to tmp, then
>  when we run:
>  *val2 = 1000000000 / tmp;
>  we'll get a divide-by-zero. Will send a patch for this later.
Good to know but not sure why you'd put that comment in this patch.
Also as a fix, it should come first - ah so maybe you were indicating it
might cause a merge issue?
> 
>  best regards,
>  max
>  
>  drivers/iio/chemical/scd30_core.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/chemical/scd30_core.c b/drivers/iio/chemical/scd30_core.c
> index a665fcb78806..be8c055be184 100644
> --- a/drivers/iio/chemical/scd30_core.c
> +++ b/drivers/iio/chemical/scd30_core.c
> @@ -89,10 +89,15 @@ static int scd30_reset(struct scd30_state *state)
>  /* simplified float to fixed point conversion with a scaling factor of 0.01 */
>  static int scd30_float_to_fp(int float32)
>  {
> -	int fraction, shift,
> -	    mantissa = float32 & GENMASK(22, 0),
> -	    sign = (float32 & BIT(31)) ? -1 : 1,
> -	    exp = (float32 & ~BIT(31)) >> 23;
> +	int fraction, shift, sign;
> +	int mantissa = float32 & GENMASK(22, 0);
	int mantissa = FIELD_GET(GENMASK(22, 0), float32);

though maybe worth some field defines as well.
//up by defines at top of file.
#define SCD30_FLOAT_MANTISSA_MSK GENMASK(22, 0)

	int mantissa = FIELD_GET(SCD30_FLOAT_MANTISSA_MSK, float32);


> +	int exp = (float32 & ~BIT(31)) >> 23;
Again with a field define this can become something like:


//up by defines at top of file - and check carefully I got this right!
#define SCD30_FLOAT_EXPONENT_MSK GENMASK(30, 23)

	int exp = FIELD_GET(SCD30_FLOAT_EXPONENT_MSK, float32);
	
> +
> +	/* Determine sign of received float based on IEEE 754 standard */
> +	if (float32 & BIT(31))
	if (FIELD_GET(SCD30_FLOAT_SIGN, float32))
 
> +		sign = -1;
> +	else
> +		sign = 1;
> 
>  	/* special case 0 */
>  	if (!exp && !mantissa)
> --
> 2.54.0