[PATCH] thermal: generic-adc: Fix linear temperature approximation

[PATCH] thermal: generic-adc: Fix linear temperature approximation

[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 <paweljarosz3691@gmail.com>
---
 drivers/thermal/thermal-generic-adc.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/thermal/thermal-generic-adc.c b/drivers/thermal/thermal-generic-adc.c
index 73f55d6..1190aee 100644
--- a/drivers/thermal/thermal-generic-adc.c
+++ b/drivers/thermal/thermal-generic-adc.c
@@ -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;
-- 
2.7.4

Re: [PATCH] thermal: generic-adc: Fix linear temperature approximation

[Laxman Dewangan]

On Wednesday 19 October 2016 01:01 AM, Paweł Jarosz wrote:
> 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 <paweljarosz3691@gmail.com>

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.
-----------------------------------------------------------------------------------

Re: [PATCH] thermal: generic-adc: Fix linear temperature approximation

[Paweł Jarosz]

W dniu 20.10.2016 o 14:40, Laxman Dewangan pisze:
>
> On Wednesday 19 October 2016 01:01 AM, Paweł Jarosz wrote:
>> 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 <paweljarosz3691@gmail.com>
>
> 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?

Hi

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ł

Re: [PATCH] thermal: generic-adc: Fix linear temperature approximation

[Laxman Dewangan]

On Thursday 20 October 2016 08:26 PM, Paweł Jarosz wrote:
>
>
> W dniu 20.10.2016 o 14:40, Laxman Dewangan pisze:
>>
>> On Wednesday 19 October 2016 01:01 AM, Paweł Jarosz wrote:
>>> 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 <paweljarosz3691@gmail.com>
>>
>> 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?
>
> Hi
>
> 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
>
>


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.

-----------------------------------------------------------------------------------
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.
-----------------------------------------------------------------------------------

Re: [PATCH] thermal: generic-adc: Fix linear temperature approximation

[Laxman Dewangan]

On Thursday 20 October 2016 09:44 PM, Paweł Jarosz wrote:
>
>
> W dniu 20.10.2016 o 17:49, Laxman Dewangan pisze:
>>
>> On Thursday 20 October 2016 08:26 PM, Paweł Jarosz wrote:
>>>
>>>
>>> W dniu 20.10.2016 o 14:40, Laxman Dewangan pisze:
>>>>
>>>> On Wednesday 19 October 2016 01:01 AM, Paweł Jarosz wrote:
>>>>> 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 <paweljarosz3691@gmail.com>
>>>>
>>>> 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?
>>>
>>> Hi
>>>
>>> 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
>>>
>>>
>>
>>
>> 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.
>
> 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.

-----------------------------------------------------------------------------------
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.
-----------------------------------------------------------------------------------

Re: [PATCH] thermal: generic-adc: Fix linear temperature approximation

[Paweł Jarosz]

W dniu 20.10.2016 o 17:49, Laxman Dewangan pisze:
>
> On Thursday 20 October 2016 08:26 PM, Paweł Jarosz wrote:
>>
>>
>> W dniu 20.10.2016 o 14:40, Laxman Dewangan pisze:
>>>
>>> On Wednesday 19 October 2016 01:01 AM, Paweł Jarosz wrote:
>>>> 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 <paweljarosz3691@gmail.com>
>>>
>>> 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?
>>
>> Hi
>>
>> 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
>>
>>
>
>
> 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.

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ł

Re: [PATCH] thermal: generic-adc: Fix linear temperature approximation

[Laxman Dewangan]

On Thursday 20 October 2016 10:40 PM, Paweł Jarosz wrote:
>
>> 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.

>
>     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.
>
>

-----------------------------------------------------------------------------------
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.
-----------------------------------------------------------------------------------

Re: [PATCH] thermal: generic-adc: Fix linear temperature approximation

[Paweł Jarosz]

> 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.

Re: [PATCH] thermal: generic-adc: Fix linear temperature approximation

[Paweł Jarosz]

> 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.
>
>>
>>     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ł

Re: [PATCH] thermal: generic-adc: Fix linear temperature approximation

[Laxman Dewangan]

On Thursday 20 October 2016 10:56 PM, Paweł Jarosz wrote:
>
>> 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.
>>
>>>
>>>     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


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.



>
> 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.
-----------------------------------------------------------------------------------

Re: [PATCH] thermal: generic-adc: Fix linear temperature approximation

[Paweł Jarosz]

W dniu 21.10.2016 o 08:18, Laxman Dewangan pisze:
>
> On Thursday 20 October 2016 10:56 PM, Paweł Jarosz wrote:
>>
>>> 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.
>>>
>>>>
>>>>     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
>
>
> 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ł