| Message ID | 20131028052743.GA31070@teo (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On 2013? 10? 28? 14:27, Anton Vorontsov wrote: > All temperatures should be in tenth degrees Celsius. > Let me tell you one thing that thermal framework uses milli centigrade for temperature. And also we have some relation with thermal framework already in power suppply core. So, what do think of using milli centigrade in power supply class either? Thanks, Jonghwa > bq24190_charger.c probably should be fixed. > > Reported-by: Jonghwa Lee <jonghwa3.lee@samsung.com> > Cc: Mark A. Greer <mgreer@animalcreek.com> > Signed-off-by: Anton Vorontsov <anton@enomsg.org> > --- > > On Mon, Oct 28, 2013 at 11:00:52AM +0900, jonghwa3.lee@samsung.com wrote: >>> * All voltages, currents, charges, energies, time and temperatures in uV, >>> * µA, µAh, µWh, seconds and tenths of degree Celsius unless otherwise >>> * stated >>> >>> So, the current code seems to be correct. >> >> Honestly, I missed the above paragraph you showed rather I read following one. >> >> TEMP - temperature of the power supply. >> TEMP_ALERT_MIN - minimum battery temperature alert value in milli centigrade. >> TEMP_ALERT_MAX - maximum battery temperature alert value in milli centigrade. >> TEMP_AMBIENT - ambient temperature. >> TEMP_AMBIENT_ALERT_MIN - minimum ambient temperature alert value in milli >> centigrade. >> TEMP_AMBIENT_ALERT_MAX - maximum ambient temperature alert value in milli >> centigrade. >> >> So, we use different unit for properties related temperature, right? >> current temperature is in tenth of centigrade and threshold temperatures and >> ambient temperature are in milli centigrade. Wouldn't it have to be in same unit? > > :( They should. Thanks for spotting. > > The patch down below should fix the issue... > > Documentation/power/power_supply_class.txt | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/Documentation/power/power_supply_class.txt b/Documentation/power/power_supply_class.txt > index 3f10b39..89a8816 100644 > --- a/Documentation/power/power_supply_class.txt > +++ b/Documentation/power/power_supply_class.txt > @@ -135,11 +135,11 @@ CAPACITY_LEVEL - capacity level. This corresponds to > POWER_SUPPLY_CAPACITY_LEVEL_*. > > TEMP - temperature of the power supply. > -TEMP_ALERT_MIN - minimum battery temperature alert value in milli centigrade. > -TEMP_ALERT_MAX - maximum battery temperature alert value in milli centigrade. > +TEMP_ALERT_MIN - minimum battery temperature alert. > +TEMP_ALERT_MAX - maximum battery temperature alert. > TEMP_AMBIENT - ambient temperature. > -TEMP_AMBIENT_ALERT_MIN - minimum ambient temperature alert value in milli centigrade. > -TEMP_AMBIENT_ALERT_MAX - maximum ambient temperature alert value in milli centigrade. > +TEMP_AMBIENT_ALERT_MIN - minimum ambient temperature alert. > +TEMP_AMBIENT_ALERT_MAX - maximum ambient temperature alert. > > TIME_TO_EMPTY - seconds left for battery to be considered empty (i.e. > while battery powers a load) -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Oct 28, 2013 at 03:22:53PM +0900, jonghwa3.lee@samsung.com wrote: > > All temperatures should be in tenth degrees Celsius. > > Let me tell you one thing that thermal framework uses milli centigrade for > temperature. And also we have some relation with thermal framework already in > power suppply core. So, what do think of using milli centigrade in power supply > class either? Since tenth degrees stuff has been exposed to the userland for 5+ years we can't just change it. So, no. If for any reason you need a higher precision temperatures (do you?), then you are welcome to introduce a set of new properties with a _MILLI affix. Anton -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Oct 27, 2013 at 10:27:43PM -0700, Anton Vorontsov wrote: > All temperatures should be in tenth degrees Celsius. > bq24190_charger.c probably should be fixed. ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > Reported-by: Jonghwa Lee <jonghwa3.lee@samsung.com> > Cc: Mark A. Greer <mgreer@animalcreek.com> > Signed-off-by: Anton Vorontsov <anton@enomsg.org> > --- > > On Mon, Oct 28, 2013 at 11:00:52AM +0900, jonghwa3.lee@samsung.com wrote: > > > * All voltages, currents, charges, energies, time and temperatures in uV, > > > * µA, µAh, µWh, seconds and tenths of degree Celsius unless otherwise > > > * stated > > > > > > So, the current code seems to be correct. > > > > Honestly, I missed the above paragraph you showed rather I read following one. > > > > TEMP - temperature of the power supply. > > TEMP_ALERT_MIN - minimum battery temperature alert value in milli centigrade. > > TEMP_ALERT_MAX - maximum battery temperature alert value in milli centigrade. > > TEMP_AMBIENT - ambient temperature. > > TEMP_AMBIENT_ALERT_MIN - minimum ambient temperature alert value in milli > > centigrade. > > TEMP_AMBIENT_ALERT_MAX - maximum ambient temperature alert value in milli > > centigrade. > > > > So, we use different unit for properties related temperature, right? > > current temperature is in tenth of centigrade and threshold temperatures and > > ambient temperature are in milli centigrade. Wouldn't it have to be in same unit? > > :( They should. Thanks for spotting. > > The patch down below should fix the issue... > > Documentation/power/power_supply_class.txt | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/Documentation/power/power_supply_class.txt b/Documentation/power/power_supply_class.txt > index 3f10b39..89a8816 100644 > --- a/Documentation/power/power_supply_class.txt > +++ b/Documentation/power/power_supply_class.txt > @@ -135,11 +135,11 @@ CAPACITY_LEVEL - capacity level. This corresponds to > POWER_SUPPLY_CAPACITY_LEVEL_*. > > TEMP - temperature of the power supply. > -TEMP_ALERT_MIN - minimum battery temperature alert value in milli centigrade. > -TEMP_ALERT_MAX - maximum battery temperature alert value in milli centigrade. > +TEMP_ALERT_MIN - minimum battery temperature alert. > +TEMP_ALERT_MAX - maximum battery temperature alert. > TEMP_AMBIENT - ambient temperature. > -TEMP_AMBIENT_ALERT_MIN - minimum ambient temperature alert value in milli centigrade. > -TEMP_AMBIENT_ALERT_MAX - maximum ambient temperature alert value in milli centigrade. > +TEMP_AMBIENT_ALERT_MIN - minimum ambient temperature alert. > +TEMP_AMBIENT_ALERT_MAX - maximum ambient temperature alert. > > TIME_TO_EMPTY - seconds left for battery to be considered empty (i.e. > while battery powers a load) > -- > 1.8.3.1 I missed the beginning of this thread so I may be missing something but the bq24190_charger driver already uses tenth of degrees Celcius for POWER_SUPPLY_PROP_TEMP_ALERT_MAX. IIUC, that's correct so there's is nothing to fix in the bq24190_charger.c driver, correct? Mark -- -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Oct 28, 2013 at 10:20:08AM -0700, Mark A. Greer wrote: > On Sun, Oct 27, 2013 at 10:27:43PM -0700, Anton Vorontsov wrote: > > All temperatures should be in tenth degrees Celsius. > > > bq24190_charger.c probably should be fixed. ... > I missed the beginning of this thread so I may be missing something but > the bq24190_charger driver already uses tenth of degrees Celcius for > POWER_SUPPLY_PROP_TEMP_ALERT_MAX. IIUC, that's correct so there's is > nothing to fix in the bq24190_charger.c driver, correct? Yup, correct. Thanks a lot for confirming! Anton -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/power/power_supply_class.txt b/Documentation/power/power_supply_class.txt index 3f10b39..89a8816 100644 --- a/Documentation/power/power_supply_class.txt +++ b/Documentation/power/power_supply_class.txt @@ -135,11 +135,11 @@ CAPACITY_LEVEL - capacity level. This corresponds to POWER_SUPPLY_CAPACITY_LEVEL_*. TEMP - temperature of the power supply. -TEMP_ALERT_MIN - minimum battery temperature alert value in milli centigrade. -TEMP_ALERT_MAX - maximum battery temperature alert value in milli centigrade. +TEMP_ALERT_MIN - minimum battery temperature alert. +TEMP_ALERT_MAX - maximum battery temperature alert. TEMP_AMBIENT - ambient temperature. -TEMP_AMBIENT_ALERT_MIN - minimum ambient temperature alert value in milli centigrade. -TEMP_AMBIENT_ALERT_MAX - maximum ambient temperature alert value in milli centigrade. +TEMP_AMBIENT_ALERT_MIN - minimum ambient temperature alert. +TEMP_AMBIENT_ALERT_MAX - maximum ambient temperature alert. TIME_TO_EMPTY - seconds left for battery to be considered empty (i.e. while battery powers a load)