diff mbox series

iio/mlx90632: restyle mlx90632_calc_temp_object_iteration

Message ID 20211021121042.1372803-1-siyanteng@loongson.cn (mailing list archive)
State Superseded
Headers show
Series iio/mlx90632: restyle mlx90632_calc_temp_object_iteration | expand

Commit Message

yanteng si Oct. 21, 2021, 12:10 p.m. UTC 
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(-)

Comments

Crt Mori Oct. 21, 2021, 12:39 p.m. UTC | #1 
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
>
teng sterling Oct. 21, 2021, 1:28 p.m. UTC | #2 
Crt Mori <cmo@melexis.com> 于2021年10月21日周四 下午8:39写道：
>
> Hi Yanteng Si,
> Why did you limit yourself to restyling just one function in the whole driver?
Sorry, I'll restyle the whole driver next.
>
> 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.
OK, Thanks!

>
> 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.
OK, I see. Thanks!

Thanks,
Yanteng
diff mbox series

Patch

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)