[PATCH] bq27x00_battery: Fix reporting battery temperature

[PATCH] bq27x00_battery: Fix reporting battery temperature

[Pali Rohár]

Reported temperature can be also negative, so cache value in non negative Kelvin degree.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/power/bq27x00_battery.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index 36b34ef..4f39dd8 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -299,7 +299,7 @@ static int bq27x00_battery_read_energy(struct bq27x00_device_info *di)
 }
 
 /*
- * Return the battery temperature in tenths of degree Celsius
+ * Return the battery temperature in tenths of degree Kelvin
  * Or < 0 if something fails.
  */
 static int bq27x00_battery_read_temperature(struct bq27x00_device_info *di)
@@ -312,10 +312,8 @@ static int bq27x00_battery_read_temperature(struct bq27x00_device_info *di)
 		return temp;
 	}
 
-	if (bq27xxx_is_chip_version_higher(di))
-		temp -= 2731;
-	else
-		temp = ((temp * 5) - 5463) / 2;
+	if (!bq27xxx_is_chip_version_higher(di))
+		temp = 5 * temp / 2;
 
 	return temp;
 }
@@ -642,6 +640,8 @@ static int bq27x00_battery_get_property(struct power_supply *psy,
 		break;
 	case POWER_SUPPLY_PROP_TEMP:
 		ret = bq27x00_simple_value(di->cache.temperature, val);
+		if (ret == 0)
+			val->intval -= 2731;
 		break;
 	case POWER_SUPPLY_PROP_TIME_TO_EMPTY_NOW:
 		ret = bq27x00_simple_value(di->cache.time_to_empty, val);
-- 
1.7.10.4

Re: [PATCH] bq27x00_battery: Fix reporting battery temperature

[Anton Vorontsov]

On Sat, Feb 02, 2013 at 11:06:09AM +0100, Pali Rohár wrote:
> Reported temperature can be also negative, so cache value in non negative Kelvin degree.
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> ---

Applied, thanks!

Re: [PATCH] bq27x00_battery: Fix reporting battery temperature

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 784 bytes --]

On Sunday 03 February 2013 04:44:51 Anton Vorontsov wrote:
> On Sat, Feb 02, 2013 at 11:06:09AM +0100, Pali Rohár wrote:
> > Reported temperature can be also negative, so cache value in
> > non negative Kelvin degree.
> > 
> > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > ---
> 
> Applied, thanks!

Now I looked at bq27x00_battery and rx51_battery drivers and I 
see that both drivers reporting temperature in different units. 
bq27x00_battery in 1/10 °C and rx51_battery in 1/100 °C. What is 
correct degree for kernel power power supply API? Maybe other 
kernel drivers have different units too... Note that my above 
patch did not changed anything units, only fixed reporting 
(possible) negative temperature.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] bq27x00_battery: Fix reporting battery temperature

[Anton Vorontsov]

On Sun, Feb 03, 2013 at 09:01:54PM +0100, Pali Rohár wrote:
> On Sunday 03 February 2013 04:44:51 Anton Vorontsov wrote:
> > On Sat, Feb 02, 2013 at 11:06:09AM +0100, Pali Rohár wrote:
> > > Reported temperature can be also negative, so cache value in
> > > non negative Kelvin degree.
> > > 
> > > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > > ---
> > 
> > Applied, thanks!
> 
> Now I looked at bq27x00_battery and rx51_battery drivers and I 
> see that both drivers reporting temperature in different units. 
> bq27x00_battery in 1/10 °C and rx51_battery in 1/100 °C. What is 
> correct degree for kernel power power supply API? Maybe other 
> kernel drivers have different units too... Note that my above 
> patch did not changed anything units, only fixed reporting 
> (possible) negative temperature.

Per Documentation/power/power_supply_class.txt and power_supply.h:

/*
 * All voltages, currents, charges, energies, time and temperatures in uV,
 * µA, µAh, µWh, seconds and tenths of degree Celsius unless otherwise
 * stated. It's driver's job to convert its raw values to units in which
 * this class operates.
 */

Feel free to fix the offending drivers.

Thanks,

Anton

Re: [PATCH] bq27x00_battery: Fix reporting battery temperature

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 3112 bytes --]

On Sunday 03 February 2013 21:05:57 Anton Vorontsov wrote:
> On Sun, Feb 03, 2013 at 09:01:54PM +0100, Pali Rohár wrote:
> > On Sunday 03 February 2013 04:44:51 Anton Vorontsov wrote:
> > > On Sat, Feb 02, 2013 at 11:06:09AM +0100, Pali Rohár wrote:
> > > > Reported temperature can be also negative, so cache
> > > > value in non negative Kelvin degree.
> > > > 
> > > > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > > > ---
> > > 
> > > Applied, thanks!
> > 
> > Now I looked at bq27x00_battery and rx51_battery drivers and
> > I see that both drivers reporting temperature in different
> > units. bq27x00_battery in 1/10 °C and rx51_battery in 1/100
> > °C. What is correct degree for kernel power power supply
> > API? Maybe other kernel drivers have different units too...
> > Note that my above patch did not changed anything units,
> > only fixed reporting (possible) negative temperature.
> 
> Per Documentation/power/power_supply_class.txt and
> power_supply.h:
> 
> /*
>  * All voltages, currents, charges, energies, time and
> temperatures in uV, * µA, µAh, µWh, seconds and tenths of
> degree Celsius unless otherwise * stated. It's driver's job
> to convert its raw values to units in which * this class
> operates.
>  */
> 
> Feel free to fix the offending drivers.
> 
> Thanks,
> 
> Anton

Ok, here is small patch which fixing it in bq27x00_battery driver.
Tested on Nokia N900 with patched 3.8-rc3 kernel.
bq27x00_battery reporting "POWER_SUPPLY_TEMP=2700"
and rx51_battery reporting "POWER_SUPPLY_TEMP=3590"

From 43a99615d6d7375770736303f1e000f3d85d75cb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali.rohar@gmail.com>
Date: Sun, 3 Feb 2013 22:06:20 +0100
Subject: [PATCH] bq27x00_battery: Report temperature in 1/100 degree Celsius
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Documentation/power/power_supply_class.txt say that temperature must be reported in tenths of degree 
Celsius

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/power/bq27x00_battery.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index 5b077af..482755f 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -312,8 +312,10 @@ static int bq27x00_battery_read_temperature(struct bq27x00_device_info *di)
 		return temp;
 	}
 
-	if (!bq27xxx_is_chip_version_higher(di))
-		temp = 5 * temp / 2;
+	if (bq27xxx_is_chip_version_higher(di))
+		temp *= 10;
+	else
+		temp *= 25;
 
 	return temp;
 }
@@ -640,7 +642,7 @@ static int bq27x00_battery_get_property(struct power_supply *psy,
 	case POWER_SUPPLY_PROP_TEMP:
 		ret = bq27x00_simple_value(di->cache.temperature, val);
 		if (ret == 0)
-			val->intval -= 2731;
+			val->intval -= 27310;
 		break;
 	case POWER_SUPPLY_PROP_TIME_TO_EMPTY_NOW:
 		ret = bq27x00_simple_value(di->cache.time_to_empty, val);
-- 
1.7.10.4

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] bq27x00_battery: Fix reporting battery temperature

[Anton Vorontsov]

On Wed, Feb 06, 2013 at 06:56:34PM +0100, Pali Rohár wrote:
[...]
> >  * All voltages, currents, charges, energies, time and
> > temperatures in uV, * µA, µAh, µWh, seconds and tenths of
> > degree Celsius unless otherwise * stated. It's driver's job
[...]
> bq27x00_battery reporting "POWER_SUPPLY_TEMP=2700"
> and rx51_battery reporting "POWER_SUPPLY_TEMP=3590"
[...]
> Subject: [PATCH] bq27x00_battery: Report temperature in 1/100 degree Celsius

Hm. The documentation says tenth (1/10) degrees, and you even restate it
in the commit message. But the subject, and your example seem to prove
that you still report it in 1/100 of Celsius.

Unless your phone was on fire during the time you took the values, I tend
to think the patch needs to be fixed. :-)

Thanks,
Anton

> * Documentation/power/power_supply_class.txt say that temperature must be reported in tenths of degree 
> Celsius
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> ---
>  drivers/power/bq27x00_battery.c |    8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
> index 5b077af..482755f 100644
> --- a/drivers/power/bq27x00_battery.c
> +++ b/drivers/power/bq27x00_battery.c
> @@ -312,8 +312,10 @@ static int bq27x00_battery_read_temperature(struct bq27x00_device_info *di)
>  		return temp;
>  	}
>  
> -	if (!bq27xxx_is_chip_version_higher(di))
> -		temp = 5 * temp / 2;
> +	if (bq27xxx_is_chip_version_higher(di))
> +		temp *= 10;
> +	else
> +		temp *= 25;
>  
>  	return temp;
>  }
> @@ -640,7 +642,7 @@ static int bq27x00_battery_get_property(struct power_supply *psy,
>  	case POWER_SUPPLY_PROP_TEMP:
>  		ret = bq27x00_simple_value(di->cache.temperature, val);
>  		if (ret == 0)
> -			val->intval -= 2731;
> +			val->intval -= 27310;
>  		break;
>  	case POWER_SUPPLY_PROP_TIME_TO_EMPTY_NOW:
>  		ret = bq27x00_simple_value(di->cache.time_to_empty, val);
> -- 
> 1.7.10.4
> 
> -- 
> Pali Rohár
> pali.rohar@gmail.com

Re: [PATCH] bq27x00_battery: Fix reporting battery temperature

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 2038 bytes --]

On Saturday 09 February 2013 02:57:18 Anton Vorontsov wrote:
> 
> Hm. The documentation says tenth (1/10) degrees, and you even
> restate it in the commit message. But the subject, and your
> example seem to prove that you still report it in 1/100 of
> Celsius.
> 
> Unless your phone was on fire during the time you took the
> values, I tend to think the patch needs to be fixed. :-)
> 
> Thanks,
> Anton
> 

Ah, my mistake. I very quicky looked that bq and rx driver
reporting different values... But bad conversation is in
rx51_battery driver, not in bq. So drop previous bq patch...

Here is patch for rx51_battery. I think now it should report
correct temperature. On Nokia N900 it reporting: POWER_SUPPLY_TEMP=350

From 92efbf24ac2fdc7af41abb12afd5e8c319479f6b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali.rohar@gmail.com>
Date: Sat, 9 Feb 2013 11:06:48 +0100
Subject: [PATCH] =?UTF-8?q?rx51=5Fbattery:=20Fix=20reporting=20temperature=20?=
 =?UTF-8?q?(in=201/10=20=C2=B0C)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 drivers/power/rx51_battery.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/power/rx51_battery.c b/drivers/power/rx51_battery.c
index 8208888..527d256 100644
--- a/drivers/power/rx51_battery.c
+++ b/drivers/power/rx51_battery.c
@@ -119,7 +119,7 @@ static int rx51_battery_read_temperature(struct rx51_device_info *di)
 
 	/* First check for temperature in first direct table */
 	if (raw < ARRAY_SIZE(rx51_temp_table1))
-		return rx51_temp_table1[raw] * 100;
+		return rx51_temp_table1[raw] * 10;
 
 	/* Binary search RAW value in second inverse table */
 	while (max - min > 1) {
@@ -132,7 +132,7 @@ static int rx51_battery_read_temperature(struct rx51_device_info *di)
 			break;
 	}
 
-	return (rx51_temp_table2_first - min) * 100;
+	return (rx51_temp_table2_first - min) * 10;
 }
 
 /*
-- 
1.7.10.4

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] bq27x00_battery: Fix reporting battery temperature

[Anton Vorontsov]

On Sat, Feb 09, 2013 at 12:02:15PM +0100, Pali Rohár wrote:
> On Saturday 09 February 2013 02:57:18 Anton Vorontsov wrote:
> > 
> > Hm. The documentation says tenth (1/10) degrees, and you even
> > restate it in the commit message. But the subject, and your
> > example seem to prove that you still report it in 1/100 of
> > Celsius.
> > 
> > Unless your phone was on fire during the time you took the
> > values, I tend to think the patch needs to be fixed. :-)
> > 
> > Thanks,
> > Anton
> > 
> 
> Ah, my mistake. I very quicky looked that bq and rx driver
> reporting different values... But bad conversation is in
> rx51_battery driver, not in bq. So drop previous bq patch...
> 
> Here is patch for rx51_battery. I think now it should report
> correct temperature. On Nokia N900 it reporting: POWER_SUPPLY_TEMP=350
> 
> From 92efbf24ac2fdc7af41abb12afd5e8c319479f6b Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali.rohar@gmail.com>
> Date: Sat, 9 Feb 2013 11:06:48 +0100
> Subject: [PATCH] =?UTF-8?q?rx51=5Fbattery:=20Fix=20reporting=20temperature=20?=
>  =?UTF-8?q?(in=201/10=20=C2=B0C)?=
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit

The patch is missing signed-off-by tag and commit message. :-/

Thanks,

> ---
>  drivers/power/rx51_battery.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/power/rx51_battery.c b/drivers/power/rx51_battery.c
> index 8208888..527d256 100644
> --- a/drivers/power/rx51_battery.c
> +++ b/drivers/power/rx51_battery.c
> @@ -119,7 +119,7 @@ static int rx51_battery_read_temperature(struct rx51_device_info *di)
>  
>  	/* First check for temperature in first direct table */
>  	if (raw < ARRAY_SIZE(rx51_temp_table1))
> -		return rx51_temp_table1[raw] * 100;
> +		return rx51_temp_table1[raw] * 10;
>  
>  	/* Binary search RAW value in second inverse table */
>  	while (max - min > 1) {
> @@ -132,7 +132,7 @@ static int rx51_battery_read_temperature(struct rx51_device_info *di)
>  			break;
>  	}
>  
> -	return (rx51_temp_table2_first - min) * 100;
> +	return (rx51_temp_table2_first - min) * 10;
>  }
>  
>  /*
> -- 
> 1.7.10.4
> 
> -- 
> Pali Rohár
> pali.rohar@gmail.com

[PATCH] power: rx51_battery: Fix reporting temperature

[Pali Rohár]

This patch fixing units (1/10 °C) in which is temperature reported.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/power/rx51_battery.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/power/rx51_battery.c b/drivers/power/rx51_battery.c
index 8208888..527d256 100644
--- a/drivers/power/rx51_battery.c
+++ b/drivers/power/rx51_battery.c
@@ -119,7 +119,7 @@ static int rx51_battery_read_temperature(struct rx51_device_info *di)
 
 	/* First check for temperature in first direct table */
 	if (raw < ARRAY_SIZE(rx51_temp_table1))
-		return rx51_temp_table1[raw] * 100;
+		return rx51_temp_table1[raw] * 10;
 
 	/* Binary search RAW value in second inverse table */
 	while (max - min > 1) {
@@ -132,7 +132,7 @@ static int rx51_battery_read_temperature(struct rx51_device_info *di)
 			break;
 	}
 
-	return (rx51_temp_table2_first - min) * 100;
+	return (rx51_temp_table2_first - min) * 10;
 }
 
 /*
-- 
1.7.10.4

Re: [PATCH] power: rx51_battery: Fix reporting temperature

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 1199 bytes --]

On Thursday 28 March 2013 17:42:23 Pali Rohár wrote:
> This patch fixing units (1/10 °C) in which is temperature
> reported.
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> ---
>  drivers/power/rx51_battery.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/power/rx51_battery.c
> b/drivers/power/rx51_battery.c index 8208888..527d256 100644
> --- a/drivers/power/rx51_battery.c
> +++ b/drivers/power/rx51_battery.c
> @@ -119,7 +119,7 @@ static int
> rx51_battery_read_temperature(struct rx51_device_info *di)
> 
>  	/* First check for temperature in first direct table */
>  	if (raw < ARRAY_SIZE(rx51_temp_table1))
> -		return rx51_temp_table1[raw] * 100;
> +		return rx51_temp_table1[raw] * 10;
> 
>  	/* Binary search RAW value in second inverse table */
>  	while (max - min > 1) {
> @@ -132,7 +132,7 @@ static int
> rx51_battery_read_temperature(struct rx51_device_info *di)
> break;
>  	}
> 
> -	return (rx51_temp_table2_first - min) * 100;
> +	return (rx51_temp_table2_first - min) * 10;
>  }
> 
>  /*

Hi Anton, please include this patch into your battery tree.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] power: rx51_battery: Fix reporting temperature

[Anton Vorontsov]

On Thu, Mar 28, 2013 at 05:42:23PM +0100, Pali Rohár wrote:
> This patch fixing units (1/10 °C) in which is temperature reported.
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> ---

Applied, thanks!

>  drivers/power/rx51_battery.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/power/rx51_battery.c b/drivers/power/rx51_battery.c
> index 8208888..527d256 100644
> --- a/drivers/power/rx51_battery.c
> +++ b/drivers/power/rx51_battery.c
> @@ -119,7 +119,7 @@ static int rx51_battery_read_temperature(struct rx51_device_info *di)
>  
>  	/* First check for temperature in first direct table */
>  	if (raw < ARRAY_SIZE(rx51_temp_table1))
> -		return rx51_temp_table1[raw] * 100;
> +		return rx51_temp_table1[raw] * 10;
>  
>  	/* Binary search RAW value in second inverse table */
>  	while (max - min > 1) {
> @@ -132,7 +132,7 @@ static int rx51_battery_read_temperature(struct rx51_device_info *di)
>  			break;
>  	}
>  
> -	return (rx51_temp_table2_first - min) * 100;
> +	return (rx51_temp_table2_first - min) * 10;
>  }
>  
>  /*
> -- 
> 1.7.10.4