thermal: generic-adc: Fix linear temperature approximation

PATCH In review Paweł Jarosz 2 participants 11 messages Oct 18, 2016 – Oct 21, 2016

Subsystems (2)

IIO Thermal

lore

Share & tools

lore

Paweł Jarosz

author

2016-10-18 19:30 UTC #0 lore raw

PATCH 1/1 thermal: generic-adc: Fix linear temperature approximation

In current version of the driver there is error in temperature calculation.

So lets fix it using proper linear function.

Signed-off-by Paweł Jarosz <[email protected]>

---
 drivers/thermal/thermal-generic-adc.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

drivers/thermal/thermal-generic-adc.c +8 -5

index 73f55d6..1190aee 100644
@@ -26,7 +26,7 @@ struct gadc_thermal_info {
 
 static int gadc_thermal_adc_to_temp(struct gadc_thermal_info *gti, int val)
 {
-	int temp, adc_hi, adc_lo;
+	int temp, adc1, adc2, temp1, temp2;
 	int i;
 
 	for (i = 0; i < gti->nlookup_table; i++) {

@@ -39,10 +39,13 @@ static int gadc_thermal_adc_to_temp(struct gadc_thermal_info *gti, int val)
 	} else if (i >= (gti->nlookup_table - 1)) {
 		temp = gti->lookup_table[2 * (gti->nlookup_table - 1)];
 	} else {
-		adc_hi = gti->lookup_table[2 * i - 1];
-		adc_lo = gti->lookup_table[2 * i + 1];
-		temp = gti->lookup_table[2 * i];
-		temp -= ((val - adc_lo) * 1000) / (adc_hi - adc_lo);
+		adc1 = gti->lookup_table[2 * i - 1];
+		adc2 = gti->lookup_table[2 * i + 1];
+
+		temp1 = gti->lookup_table[2 * i - 2];
+		temp2 = gti->lookup_table[2 * i];
+
+		temp = (val - adc1) * (temp2 - temp1) / (adc2 - adc1) + temp1;
 	}
 
 	return temp;

Signature · 1 line

2.7.4

Laxman Dewangan

2016-10-20 12:55 UTC #1 lore raw

Quoted 6 lines · Paweł Jarosz

In current version of the driver there is error in temperature calculation.
So lets fix it using proper linear function.
Signed-off-by Paweł Jarosz <[email protected]>

Per my calculation, existing and your equation is same as both are doing linear interpolation.

Only think I have seen is that I have used 1000 for two consecutive temp difference and you used (t2 - t1), so your is better in this case but still the equation is same.

The equation from you is

    t1 + (hi -val) * factor.

and existing one is

     t2 - (val -lo) * factor

factor is abs((t2-t1)/(hi-lo))

If my analysis is not correct then can you please provide the example with calculation for better understansing?

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

Paweł Jarosz

author

2016-10-20 14:55 UTC #2 lore raw

W dniu 20.10.2016 o 14:40, Laxman Dewangan pisze:

Quoted 17 lines · Paweł Jarosz

Quoted 6 lines
In current version of the driver there is error in temperature calculation.
So lets fix it using proper linear function.
Signed-off-by Paweł Jarosz <[email protected]>
Per my calculation, existing and your equation is same as both are doing linear interpolation.
Only think I have seen is that I have used 1000 for two consecutive temp difference and you used (t2 - t1), so your is better in this case but still the equation is same.
The equation from you is
    t1 + (hi -val) * factor.

Quoted 7 lines

and existing one is
     t2 - (val -lo) * factor
factor is abs((t2-t1)/(hi-lo))
If my analysis is not correct then can you please provide the example with calculation for better understansing?

For example if we try to calculate temp for val = 8, i = 1 and table:

<0 9
2 7>

	adc_hi = gti->lookup_table[2 * i - 1];
	adc_lo = gti->lookup_table[2 * i + 1];
	temp = gti->lookup_table[2 * i];
	temp -= ((val - adc_lo) * 1000) / (adc_hi - adc_lo);

Your method gives temp = -498

Paweł

Laxman Dewangan

2016-10-20 16:04 UTC #3 lore raw

Quoted 35 lines · Paweł Jarosz

W dniu 20.10.2016 o 14:40, Laxman Dewangan pisze:
Quoted 17 lines · Paweł Jarosz
Quoted 6 lines
In current version of the driver there is error in temperature calculation.
So lets fix it using proper linear function.
Signed-off-by Paweł Jarosz <[email protected]>
Per my calculation, existing and your equation is same as both are doing linear interpolation.
Only think I have seen is that I have used 1000 for two consecutive temp difference and you used (t2 - t1), so your is better in this case but still the equation is same.
The equation from you is
    t1 + (hi -val) * factor.
Quoted 7 lines
and existing one is
     t2 - (val -lo) * factor
factor is abs((t2-t1)/(hi-lo))
If my analysis is not correct then can you please provide the example with calculation for better understansing?
Hi
For example if we try to calculate temp for val = 8, i = 1 and table:
<0 9
2 7>

Quoted 8 lines

    adc_hi = gti->lookup_table[2 * i - 1];
    adc_lo = gti->lookup_table[2 * i + 1];
    temp = gti->lookup_table[2 * i];
    temp -= ((val - adc_lo) * 1000) / (adc_hi - adc_lo);

Your method gives temp = -498

The temp should be in the milli-centigrade in the table so it should be <0 9
2000 7>;

and if you relplace 1000 with the t2 -t1 where

t1 = gti->lookup_table[2*i -2]
t2 = gti->lookup_table[2 *i]

then it will work perfectly.

So your change for replacing 1000 is correct as what I said but need not to change whole equation altogether.

Paweł Jarosz

author

2016-10-20 16:13 UTC #4 lore raw

W dniu 20.10.2016 o 17:49, Laxman Dewangan pisze:

Quoted 51 lines · Paweł Jarosz

Quoted 35 lines
W dniu 20.10.2016 o 14:40, Laxman Dewangan pisze:
Quoted 17 lines · Paweł Jarosz
Quoted 6 lines
In current version of the driver there is error in temperature calculation.
So lets fix it using proper linear function.
Signed-off-by Paweł Jarosz <[email protected]>
Per my calculation, existing and your equation is same as both are doing linear interpolation.
Only think I have seen is that I have used 1000 for two consecutive temp difference and you used (t2 - t1), so your is better in this case but still the equation is same.
The equation from you is
    t1 + (hi -val) * factor.
Quoted 7 lines
and existing one is
     t2 - (val -lo) * factor
factor is abs((t2-t1)/(hi-lo))
If my analysis is not correct then can you please provide the example with calculation for better understansing?
Hi
For example if we try to calculate temp for val = 8, i = 1 and table:
<0 9
2 7>
Quoted 8 lines
    adc_hi = gti->lookup_table[2 * i - 1];
    adc_lo = gti->lookup_table[2 * i + 1];
    temp = gti->lookup_table[2 * i];
    temp -= ((val - adc_lo) * 1000) / (adc_hi - adc_lo);
Your method gives temp = -498
The temp should be in the milli-centigrade in the table so it should be <0 9
2000 7>;

Quoted 8 lines

and if you relplace 1000 with the t2 -t1 where
t1 = gti->lookup_table[2*i -2]
t2 = gti->lookup_table[2 *i]
then it will work perfectly.
So your change for replacing 1000 is correct as what I said but need not to change whole equation altogether.

Sorry for that ... when i was writing this i wasn't using your equation, but writing from scratch. I can send new patch with corrections or leave it as it is. Your call.

Paweł

Laxman Dewangan

2016-10-20 16:16 UTC #5 lore raw

Quoted 59 lines · Paweł Jarosz

W dniu 20.10.2016 o 17:49, Laxman Dewangan pisze:
Quoted 56 lines · Paweł Jarosz
Quoted 35 lines
W dniu 20.10.2016 o 14:40, Laxman Dewangan pisze:
Quoted 17 lines · Paweł Jarosz
Quoted 6 lines
In current version of the driver there is error in temperature calculation.
So lets fix it using proper linear function.
Signed-off-by Paweł Jarosz <[email protected]>
Per my calculation, existing and your equation is same as both are doing linear interpolation.
Only think I have seen is that I have used 1000 for two consecutive temp difference and you used (t2 - t1), so your is better in this case but still the equation is same.
The equation from you is
    t1 + (hi -val) * factor.
Quoted 7 lines
and existing one is
     t2 - (val -lo) * factor
factor is abs((t2-t1)/(hi-lo))
If my analysis is not correct then can you please provide the example with calculation for better understansing?
Hi
For example if we try to calculate temp for val = 8, i = 1 and table:
<0 9
2 7>
Quoted 8 lines
    adc_hi = gti->lookup_table[2 * i - 1];
    adc_lo = gti->lookup_table[2 * i + 1];
    temp = gti->lookup_table[2 * i];
    temp -= ((val - adc_lo) * 1000) / (adc_hi - adc_lo);
Your method gives temp = -498
The temp should be in the milli-centigrade in the table so it should be <0 9
2000 7>;
and if you relplace 1000 with the t2 -t1 where
t1 = gti->lookup_table[2*i -2]
t2 = gti->lookup_table[2 *i]

Quoted 8 lines

Quoted 3 lines
then it will work perfectly.
So your change for replacing 1000 is correct as what I said but need not to change whole equation altogether.
Sorry for that ... when i was writing this i wasn't using your equation, but writing from scratch. I can send new patch with corrections or leave it as it is. Your call.

Yes please send patch to replace the 1000 with the temp1 - temp2 as what you did in your original patch. This will fix the non-sequenced (un-equally spaced) temperature table.

Paweł Jarosz

author

2016-10-20 17:09 UTC #6 lore raw

In reply to Laxman Dewangan

Quoted 3 lines

Yes please send patch to replace the 1000 with the temp1 - temp2 as what you did in your original patch. This will fix the non-sequenced (un-equally spaced) temperature table.

replacing 1000 with (temp1 - temp2) is not enough as for example for val = 9 and table

<7000 8
9000 10>

	adc_hi = gti->lookup_table[2 * i - 1];
	adc_lo = gti->lookup_table[2 * i + 1];
	temp = gti->lookup_table[2 * i];
	temp -= (val - adc_lo) * (temp1 - temp2) / (adc_hi - adc_lo);

adc_hi = 8
adc_lo = 10
temp = 9000 - (9 - 10) * (7000 - 9000) / (8 - 10)
temp = 9000 - (-1) * (-2000) / (-2)
temp = 9000 - (-1000) = 10000

and for val = 9 temp should be 8000.

Laxman Dewangan

2016-10-20 17:16 UTC #7 lore raw

Quoted 8 lines · Paweł Jarosz

Quoted 3 lines
Yes please send patch to replace the 1000 with the temp1 - temp2 as what you did in your original patch. This will fix the non-sequenced (un-equally spaced) temperature table.
replacing 1000 with (temp1 - temp2) is not enough as for example for val = 9 and table
<7000 8
9000 10>

It is Negative Coefficient Thermistor (NCT) and so adc reads are different. Lower temp gives higher value.

So table should be

<7000 10
9000 8>;

temp1 = gti->lookup_table[2 * i];
temp2 = gti->lookup_table[2 * i - 2];

and then use in following equation.

Quoted 8 lines

    adc_hi = gti->lookup_table[2 * i - 1];
    adc_lo = gti->lookup_table[2 * i + 1];
    temp = gti->lookup_table[2 * i];
    temp -= (val - adc_lo) * (temp1 - temp2) / (adc_hi - adc_lo);

Quoted 9 lines

adc_hi = 8
adc_lo = 10
temp = 9000 - (9 - 10) * (7000 - 9000) / (8 - 10)
temp = 9000 - (-1) * (-2000) / (-2)
temp = 9000 - (-1000) = 10000
and for val = 9 temp should be 8000.

Paweł Jarosz

author

2016-10-20 17:24 UTC #8 lore raw

Quoted 6 lines

It is Negative Coefficient Thermistor (NCT) and so adc reads are different. Lower temp gives higher value.
So table should be

Quoted 13 lines

<7000 10
9000 8>;
temp1 = gti->lookup_table[2 * i];
temp2 = gti->lookup_table[2 * i - 2];
and then use in following equation.
Quoted 5 lines
    adc_hi = gti->lookup_table[2 * i - 1];
    adc_lo = gti->lookup_table[2 * i + 1];
    temp = gti->lookup_table[2 * i];
    temp -= (val - adc_lo) * (temp1 - temp2) / (adc_hi - adc_lo);

val = 9
adc_hi = 10
adc_lo = 8
temp = 9000 - (9 - 8)*(7000 - 9000)/(10 - 8)
temp = 9000 - (1)*(-2000)/(2)

temp = 9000 - (-1000) = 10000

still wrong ... temp for val = 9 should be 8000

Maybe we could leave this patch as it is?

Paweł

Laxman Dewangan

2016-10-21 06:34 UTC #9 lore raw

Quoted 26 lines · Paweł Jarosz

Quoted 6 lines
It is Negative Coefficient Thermistor (NCT) and so adc reads are different. Lower temp gives higher value.
So table should be
Quoted 13 lines
<7000 10
9000 8>;
temp1 = gti->lookup_table[2 * i];
temp2 = gti->lookup_table[2 * i - 2];
and then use in following equation.
Quoted 5 lines
    adc_hi = gti->lookup_table[2 * i - 1];
    adc_lo = gti->lookup_table[2 * i + 1];
    temp = gti->lookup_table[2 * i];
    temp -= (val - adc_lo) * (temp1 - temp2) / (adc_hi - adc_lo);
val = 9
adc_hi = 10
adc_lo = 8
temp = 9000 - (9 - 8)*(7000 - 9000)/(10 - 8)
temp = 9000 - (1)*(-2000)/(2)

Quoted 3 lines

temp = 9000 - (-1000) = 10000
still wrong ... temp for val = 9 should be 8000

Your temp1 and temp2 calculation is something wrong.

temp1 = gti->lookup_table[2 * i];
temp2 = gti->lookup_table[2 * i - 2];
so temp1 = 9000, temp2 = 7000

temp -= (val - adc_lo) * (temp1 - temp2) / (adc_hi - adc_lo);

temp = 9000 - (9 - 8)*(9000 - 7000)/(10 - 8) temp = 9000 - (1)*(2000)/(2) = 8000 which is expected.

Quoted 3 lines

Maybe we could leave this patch as it is?

temp = gti->lookup_table[2 * i];
+ tlow = gti->lookup_table[2 * i - 2];
(modify) temp -= (val - adc_lo) * (temp - tlow) / (adc_hi - adc_lo);

This will gives you correct result.

Let me know if you can send the updated patch. Else I will send.

Thanks,
Laxman
-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

Paweł Jarosz

author

2016-10-21 06:58 UTC #10 lore raw

W dniu 21.10.2016 o 08:18, Laxman Dewangan pisze:

Quoted 30 lines · Paweł Jarosz

Quoted 28 lines
Quoted 6 lines
It is Negative Coefficient Thermistor (NCT) and so adc reads are different. Lower temp gives higher value.
So table should be
Quoted 13 lines
<7000 10
9000 8>;
temp1 = gti->lookup_table[2 * i];
temp2 = gti->lookup_table[2 * i - 2];
and then use in following equation.
Quoted 5 lines
    adc_hi = gti->lookup_table[2 * i - 1];
    adc_lo = gti->lookup_table[2 * i + 1];
    temp = gti->lookup_table[2 * i];
    temp -= (val - adc_lo) * (temp1 - temp2) / (adc_hi - adc_lo);
val = 9
adc_hi = 10
adc_lo = 8
temp = 9000 - (9 - 8)*(7000 - 9000)/(10 - 8)
temp = 9000 - (1)*(-2000)/(2)
temp = 9000 - (-1000) = 10000

Quoted 9 lines

Quoted 1 line
still wrong ... temp for val = 9 should be 8000
Your temp1 and temp2 calculation is something wrong.
temp1 = gti->lookup_table[2 * i];
temp2 = gti->lookup_table[2 * i - 2];
so temp1 = 9000, temp2 = 7000

There was nothing wrong with my calculation but you changed the order to fix your equation.

You can send the patch.

Paweł