* [PATCH] battery: max17042: Fix temperature unit to milli centigrade. @ 2013-10-25 4:53 Jonghwa Lee 2013-10-25 23:17 ` Anton Vorontsov 0 siblings, 1 reply; 8+ messages in thread From: Jonghwa Lee @ 2013-10-25 4:53 UTC (permalink / raw) To: linux-pm; +Cc: anton, dwmw2, myungjoo.ham, cw00.choi, Jonghwa Lee Standard temp node represents temperature of power supply class in milli centigrade. So fix max17042 fuel gauge driver to follow the standard. (deci-centigrade --> milli-centigrade) Signed-off-by: Jonghwa Lee <jonghwa3.lee@samsung.com> Signed-off-by: Myungjoo Ham <myungjoo.ham@samsung.com> --- drivers/power/max17042_battery.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/power/max17042_battery.c b/drivers/power/max17042_battery.c index d664ef5..892ecfc 100644 --- a/drivers/power/max17042_battery.c +++ b/drivers/power/max17042_battery.c @@ -220,9 +220,9 @@ static int max17042_get_property(struct power_supply *psy, val->intval = (0x7fff & ~val->intval) + 1; val->intval *= -1; } - /* The value is converted into deci-centigrade scale */ + /* The value is converted into milli-centigrade scale */ /* Units of LSB = 1 / 256 degree Celsius */ - val->intval = val->intval * 10 / 256; + val->intval = val->intval * 1000 / 256; break; case POWER_SUPPLY_PROP_CURRENT_NOW: if (chip->pdata->enable_current_sense) { -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] battery: max17042: Fix temperature unit to milli centigrade. 2013-10-25 4:53 [PATCH] battery: max17042: Fix temperature unit to milli centigrade Jonghwa Lee @ 2013-10-25 23:17 ` Anton Vorontsov 2013-10-28 2:00 ` jonghwa3.lee 0 siblings, 1 reply; 8+ messages in thread From: Anton Vorontsov @ 2013-10-25 23:17 UTC (permalink / raw) To: Jonghwa Lee; +Cc: linux-pm, dwmw2, myungjoo.ham, cw00.choi On Fri, Oct 25, 2013 at 01:53:18PM +0900, Jonghwa Lee wrote: > Standard temp node represents temperature of power supply class > in milli centigrade. So fix max17042 fuel gauge driver to follow > the standard. (deci-centigrade --> milli-centigrade) Maybe I am confused, but the doc says: * 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. Thanks, Anton > Signed-off-by: Jonghwa Lee <jonghwa3.lee@samsung.com> > Signed-off-by: Myungjoo Ham <myungjoo.ham@samsung.com> > --- > drivers/power/max17042_battery.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/power/max17042_battery.c b/drivers/power/max17042_battery.c > index d664ef5..892ecfc 100644 > --- a/drivers/power/max17042_battery.c > +++ b/drivers/power/max17042_battery.c > @@ -220,9 +220,9 @@ static int max17042_get_property(struct power_supply *psy, > val->intval = (0x7fff & ~val->intval) + 1; > val->intval *= -1; > } > - /* The value is converted into deci-centigrade scale */ > + /* The value is converted into milli-centigrade scale */ > /* Units of LSB = 1 / 256 degree Celsius */ > - val->intval = val->intval * 10 / 256; > + val->intval = val->intval * 1000 / 256; > break; > case POWER_SUPPLY_PROP_CURRENT_NOW: > if (chip->pdata->enable_current_sense) { > -- > 1.7.9.5 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] battery: max17042: Fix temperature unit to milli centigrade. 2013-10-25 23:17 ` Anton Vorontsov @ 2013-10-28 2:00 ` jonghwa3.lee 2013-10-28 5:27 ` [PATCH] power_supply: Fix documentation for TEMP_*ALERT* properties Anton Vorontsov 0 siblings, 1 reply; 8+ messages in thread From: jonghwa3.lee @ 2013-10-28 2:00 UTC (permalink / raw) To: Anton Vorontsov; +Cc: linux-pm, dwmw2, myungjoo.ham, cw00.choi Hi, On 2013년 10월 26일 08:17, Anton Vorontsov wrote: > On Fri, Oct 25, 2013 at 01:53:18PM +0900, Jonghwa Lee wrote: >> Standard temp node represents temperature of power supply class >> in milli centigrade. So fix max17042 fuel gauge driver to follow >> the standard. (deci-centigrade --> milli-centigrade) > > Maybe I am confused, but the doc says: > > * 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? Thanks, Jonghwa > Thanks, > > Anton > >> Signed-off-by: Jonghwa Lee <jonghwa3.lee@samsung.com> >> Signed-off-by: Myungjoo Ham <myungjoo.ham@samsung.com> >> --- >> drivers/power/max17042_battery.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/power/max17042_battery.c b/drivers/power/max17042_battery.c >> index d664ef5..892ecfc 100644 >> --- a/drivers/power/max17042_battery.c >> +++ b/drivers/power/max17042_battery.c >> @@ -220,9 +220,9 @@ static int max17042_get_property(struct power_supply *psy, >> val->intval = (0x7fff & ~val->intval) + 1; >> val->intval *= -1; >> } >> - /* The value is converted into deci-centigrade scale */ >> + /* The value is converted into milli-centigrade scale */ >> /* Units of LSB = 1 / 256 degree Celsius */ >> - val->intval = val->intval * 10 / 256; >> + val->intval = val->intval * 1000 / 256; >> break; >> case POWER_SUPPLY_PROP_CURRENT_NOW: >> if (chip->pdata->enable_current_sense) { >> -- >> 1.7.9.5 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] power_supply: Fix documentation for TEMP_*ALERT* properties 2013-10-28 2:00 ` jonghwa3.lee @ 2013-10-28 5:27 ` Anton Vorontsov 2013-10-28 6:22 ` jonghwa3.lee 2013-10-28 17:20 ` Mark A. Greer 0 siblings, 2 replies; 8+ messages in thread From: Anton Vorontsov @ 2013-10-28 5:27 UTC (permalink / raw) To: jonghwa3.lee, Mark A. Greer Cc: linux-pm, dwmw2, myungjoo.ham, cw00.choi, Ramakrishna Pallala 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] power_supply: Fix documentation for TEMP_*ALERT* properties 2013-10-28 5:27 ` [PATCH] power_supply: Fix documentation for TEMP_*ALERT* properties Anton Vorontsov @ 2013-10-28 6:22 ` jonghwa3.lee 2013-10-28 6:43 ` Anton Vorontsov 2013-10-28 17:20 ` Mark A. Greer 1 sibling, 1 reply; 8+ messages in thread From: jonghwa3.lee @ 2013-10-28 6:22 UTC (permalink / raw) To: Anton Vorontsov Cc: Mark A. Greer, linux-pm, dwmw2, myungjoo.ham, cw00.choi, Ramakrishna Pallala 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) ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] power_supply: Fix documentation for TEMP_*ALERT* properties 2013-10-28 6:22 ` jonghwa3.lee @ 2013-10-28 6:43 ` Anton Vorontsov 0 siblings, 0 replies; 8+ messages in thread From: Anton Vorontsov @ 2013-10-28 6:43 UTC (permalink / raw) To: jonghwa3.lee Cc: Mark A. Greer, linux-pm, dwmw2, myungjoo.ham, cw00.choi, Ramakrishna Pallala 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] power_supply: Fix documentation for TEMP_*ALERT* properties 2013-10-28 5:27 ` [PATCH] power_supply: Fix documentation for TEMP_*ALERT* properties Anton Vorontsov 2013-10-28 6:22 ` jonghwa3.lee @ 2013-10-28 17:20 ` Mark A. Greer 2013-10-29 3:13 ` Anton Vorontsov 1 sibling, 1 reply; 8+ messages in thread From: Mark A. Greer @ 2013-10-28 17:20 UTC (permalink / raw) To: Anton Vorontsov Cc: jonghwa3.lee, linux-pm, dwmw2, myungjoo.ham, cw00.choi, Ramakrishna Pallala 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 -- ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] power_supply: Fix documentation for TEMP_*ALERT* properties 2013-10-28 17:20 ` Mark A. Greer @ 2013-10-29 3:13 ` Anton Vorontsov 0 siblings, 0 replies; 8+ messages in thread From: Anton Vorontsov @ 2013-10-29 3:13 UTC (permalink / raw) To: Mark A. Greer Cc: jonghwa3.lee, linux-pm, dwmw2, myungjoo.ham, cw00.choi, Ramakrishna Pallala 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2013-10-29 3:13 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-10-25 4:53 [PATCH] battery: max17042: Fix temperature unit to milli centigrade Jonghwa Lee 2013-10-25 23:17 ` Anton Vorontsov 2013-10-28 2:00 ` jonghwa3.lee 2013-10-28 5:27 ` [PATCH] power_supply: Fix documentation for TEMP_*ALERT* properties Anton Vorontsov 2013-10-28 6:22 ` jonghwa3.lee 2013-10-28 6:43 ` Anton Vorontsov 2013-10-28 17:20 ` Mark A. Greer 2013-10-29 3:13 ` Anton Vorontsov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox