Re: [PATCH] iio/mlx90632: restyle mlx90632_calc_temp_object_iteration

[Crt Mori <cmo@melexis.com>]

Hi Yanteng Si,
Why did you limit yourself to restyling just one function in the whole driver?

If you decide that we should restyle the driver, then please separate
the words with underscores to provide some more meaningful variable
names because name of variable for calculated Ks coefficient for
Temperature Object is kinda more meaningful than calcedksto without
any split or capitalization. Same goes for Ha_customer and Hb_customer
which are coefficients, capitalized in datasheet (hence retained
capitalization in driver) and we want to maintain some sort of
resemblance to the datasheet.

This patch is introducing a strange mix where coefficients from
datasheet are capitalized, but local variables referencing/expanding
them are not and all in just one function.

Best regards,
Crt Mori


On Thu, 21 Oct 2021 at 14:11, Yanteng Si <siyanteng01@gmail.com> wrote:
>
> ref: Documentation/process/coding-style.rst:
> C programmers do not use cute names like ThisVariableIsATemporaryCounter
>
> so,restyle mlx90632_calc_temp_object_iteration()
>
> Signed-off-by: Yanteng Si <siyanteng@loongson.cn>
> ---
>  drivers/iio/temperature/mlx90632.c | 30 +++++++++++++++---------------
>  1 file changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/iio/temperature/mlx90632.c b/drivers/iio/temperature/mlx90632.c
> index 608ccb1d8bc8..03257da92db4 100644
> --- a/drivers/iio/temperature/mlx90632.c
> +++ b/drivers/iio/temperature/mlx90632.c
> @@ -547,24 +547,24 @@ static s32 mlx90632_calc_temp_object_iteration(s32 prev_object_temp, s64 object,
>                                                s32 Ga, s16 Ha, s16 Hb,
>                                                u16 emissivity)
>  {
> -       s64 calcedKsTO, calcedKsTA, ir_Alpha, Alpha_corr;
> -       s64 Ha_customer, Hb_customer;
> +       s64 calcedksto, calcedksta, ir_alpha, alpha_corr;
> +       s64 ha_customer, hb_customer;
>
> -       Ha_customer = ((s64)Ha * 1000000LL) >> 14ULL;
> -       Hb_customer = ((s64)Hb * 100) >> 10ULL;
> +       ha_customer = ((s64)Ha * 1000000LL) >> 14ULL;
> +       hb_customer = ((s64)Hb * 100) >> 10ULL;
>
> -       calcedKsTO = ((s64)((s64)Ga * (prev_object_temp - 25 * 1000LL)
> +       calcedksto = ((s64)((s64)Ga * (prev_object_temp - 25 * 1000LL)
>                              * 1000LL)) >> 36LL;
> -       calcedKsTA = ((s64)(Fb * (TAdut - 25 * 1000000LL))) >> 36LL;
> -       Alpha_corr = div64_s64((((s64)(Fa * 10000000000LL) >> 46LL)
> -                               * Ha_customer), 1000LL);
> -       Alpha_corr *= ((s64)(1 * 1000000LL + calcedKsTO + calcedKsTA));
> -       Alpha_corr = emissivity * div64_s64(Alpha_corr, 100000LL);
> -       Alpha_corr = div64_s64(Alpha_corr, 1000LL);
> -       ir_Alpha = div64_s64((s64)object * 10000000LL, Alpha_corr);
> -
> -       return (int_sqrt64(int_sqrt64(ir_Alpha * 1000000000000LL + TAdut4))
> -               - 27315 - Hb_customer) * 10;
> +       calcedksta = ((s64)(Fb * (TAdut - 25 * 1000000LL))) >> 36LL;
> +       alpha_corr = div64_s64((((s64)(Fa * 10000000000LL) >> 46LL)
> +                               * ha_customer), 1000LL);
> +       alpha_corr *= ((s64)(1 * 1000000LL + calcedksto + calcedksta));
> +       alpha_corr = emissivity * div64_s64(alpha_corr, 100000LL);
> +       alpha_corr = div64_s64(alpha_corr, 1000LL);
> +       ir_alpha = div64_s64((s64)object * 10000000LL, alpha_corr);
> +
> +       return (int_sqrt64(int_sqrt64(ir_alpha * 1000000000000LL + TAdut4))
> +               - 27315 - hb_customer) * 10;
>  }
>
>  static s64 mlx90632_calc_ta4(s64 TAdut, s64 scale)
> --
> 2.27.0
>