[PATCH] thermal: rcar: reduce inaccuracy in the calculate rounding for temperature conversion

[PATCH] thermal: rcar: reduce inaccuracy in the calculate rounding for temperature conversion

[Nguyen An Hoan]

From: Hoan Nguyen An <na-hoan@jinso.co.jp>

This patch to avoid and reduce the inaccuracy in the calculation when DIV_ROUND() and then calculates the conversion, I think, should change the order: calculate the conversion and then DIV_ROUND().
Although I'm not sure there will be an error, but I have encountered problems when troubleshooting interrupts not counting up.

Hoan Nguyen An (1):
  thermal: rcar: reduce inaccuracy in calculate rounding

 drivers/thermal/rcar_gen3_thermal.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

-- 
2.7.4

[PATCH] thermal: rcar: reduce inaccuracy in calculate rounding

[Nguyen An Hoan]

From: Hoan Nguyen An <na-hoan@jinso.co.jp>

About the formula for temperature calculation
[reg] = [temp] * a + b <=> [temp] = ([reg] - b) / a

Using "(mcelsius * tsc-> coef.aX) / 1000" instead of "DIV_ROUND_CLOSEST(mcelsius, 1000) * tsc-> coef.aX"
to avoid and reduce inaccuracy due to rounding the integer division.

Signed-off-by: Hoan Nguyen An <na-hoan@jinso.co.jp>
---
 drivers/thermal/rcar_gen3_thermal.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
index 7aed533..dbb31b8 100644
--- a/drivers/thermal/rcar_gen3_thermal.c
+++ b/drivers/thermal/rcar_gen3_thermal.c
@@ -185,11 +185,10 @@ static int rcar_gen3_thermal_get_temp(void *devdata, int *temp)
 static int rcar_gen3_thermal_mcelsius_to_temp(struct rcar_gen3_thermal_tsc *tsc,
 					      int mcelsius)
 {
-	int celsius, val1, val2;
+	int val1, val2;
 
-	celsius = DIV_ROUND_CLOSEST(mcelsius, 1000);
-	val1 = celsius * tsc->coef.a1 + tsc->coef.b1;
-	val2 = celsius * tsc->coef.a2 + tsc->coef.b2;
+	val1 = (mcelsius * tsc->coef.a1)/1000 + tsc->coef.b1;
+	val2 = (mcelsius * tsc->coef.a2)/1000 + tsc->coef.b2;
 
 	return INT_FIXPT((val1 + val2) / 2);
 }
-- 
2.7.4

Re: [PATCH] thermal: rcar: reduce inaccuracy in calculate rounding

[Geert Uytterhoeven]

Hi Hoan,

On Thu, Aug 30, 2018 at 4:39 AM Nguyen An Hoan <na-hoan@jinso.co.jp> wrote:
> From: Hoan Nguyen An <na-hoan@jinso.co.jp>
>
> About the formula for temperature calculation
> [reg] = [temp] * a + b <=> [temp] = ([reg] - b) / a
>
> Using "(mcelsius * tsc-> coef.aX) / 1000" instead of "DIV_ROUND_CLOSEST(mcelsius, 1000) * tsc-> coef.aX"
> to avoid and reduce inaccuracy due to rounding the integer division.
>
> Signed-off-by: Hoan Nguyen An <na-hoan@jinso.co.jp>

Thanks for your patch!

> --- a/drivers/thermal/rcar_gen3_thermal.c
> +++ b/drivers/thermal/rcar_gen3_thermal.c
> @@ -185,11 +185,10 @@ static int rcar_gen3_thermal_get_temp(void *devdata, int *temp)
>  static int rcar_gen3_thermal_mcelsius_to_temp(struct rcar_gen3_thermal_tsc *tsc,
>                                               int mcelsius)
>  {
> -       int celsius, val1, val2;
> +       int val1, val2;
>
> -       celsius = DIV_ROUND_CLOSEST(mcelsius, 1000);
> -       val1 = celsius * tsc->coef.a1 + tsc->coef.b1;
> -       val2 = celsius * tsc->coef.a2 + tsc->coef.b2;
> +       val1 = (mcelsius * tsc->coef.a1)/1000 + tsc->coef.b1;
> +       val2 = (mcelsius * tsc->coef.a2)/1000 + tsc->coef.b2;

No rounding anymoe?

    val1 = DIV_ROUND_CLOSEST(mcelsius * tsc->coef.a1, 1000) + tsc->coef.b1;
    val2 = DIV_ROUND_CLOSEST(mcelsius * tsc->coef.a2, 1000) + tsc->coef.b2;

Can the multiplication overflow?

I know mcelsius = -40000..120000/
What is the range of a1 and a2?

>
>         return INT_FIXPT((val1 + val2) / 2);

BTW, do we want to introduce rounding here, too?
(for INT_FIXPT(), it doesn't matter much for /2)?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] thermal: rcar: reduce inaccuracy in calculate rounding

[Sergei Shtylyov]

Hello!

On 8/30/2018 5:39 AM, Nguyen An Hoan wrote:

> From: Hoan Nguyen An <na-hoan@jinso.co.jp>
> 
> About the formula for temperature calculation
> [reg] = [temp] * a + b <=> [temp] = ([reg] - b) / a
> 
> Using "(mcelsius * tsc-> coef.aX) / 1000" instead of "DIV_ROUND_CLOSEST(mcelsius, 1000) * tsc-> coef.aX"
> to avoid and reduce inaccuracy due to rounding the integer division.
> 
> Signed-off-by: Hoan Nguyen An <na-hoan@jinso.co.jp>
> ---
>   drivers/thermal/rcar_gen3_thermal.c | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
> index 7aed533..dbb31b8 100644
> --- a/drivers/thermal/rcar_gen3_thermal.c
> +++ b/drivers/thermal/rcar_gen3_thermal.c
> @@ -185,11 +185,10 @@ static int rcar_gen3_thermal_get_temp(void *devdata, int *temp)
>   static int rcar_gen3_thermal_mcelsius_to_temp(struct rcar_gen3_thermal_tsc *tsc,
>   					      int mcelsius)
>   {
> -	int celsius, val1, val2;
> +	int val1, val2;
>   
> -	celsius = DIV_ROUND_CLOSEST(mcelsius, 1000);
> -	val1 = celsius * tsc->coef.a1 + tsc->coef.b1;
> -	val2 = celsius * tsc->coef.a2 + tsc->coef.b2;
> +	val1 = (mcelsius * tsc->coef.a1)/1000 + tsc->coef.b1;
> +	val2 = (mcelsius * tsc->coef.a2)/1000 + tsc->coef.b2;

    Please be consistent and enclose / in sopaces the same as you do for other 
operators.

[...]

MBR, Sergei