Messages in this thread | | | Date | Tue, 13 Mar 2012 22:34:52 +0400 | From | Anton Vorontsov <> | Subject | Re: [PATCH 5/5] max17042: Change capacity property to use reported SOC register |
| |
On Tue, Jan 24, 2012 at 09:26:08AM -0800, dirk.brandewie@gmail.com wrote: > From: Dirk Brandewie <dirk.brandewie@gmail.com> > > The SOC register (0dh) reports the state of charge before empty > compensation adjustments are applied. The max value reported by this > register will decrease as the battery ages. > > Use the RepSOC register (06h) to report the capacity of the > battery. RepSOC contains a filtered version of the battery capacity > after empty compensation adjustments have been applied. > > Reported-by: Gary Keyes <gary.e.keyes@intel.com> > > Signed-off-by: Dirk Brandewie <dirk.brandewie@gmail.com> > --- > drivers/power/max17042_battery.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/drivers/power/max17042_battery.c b/drivers/power/max17042_battery.c > index 6e96b58..2194278 100644 > --- a/drivers/power/max17042_battery.c > +++ b/drivers/power/max17042_battery.c > @@ -160,7 +160,7 @@ static int max17042_get_property(struct power_supply *psy, > break; > case POWER_SUPPLY_PROP_CAPACITY: > val->intval = max17042_read_reg(chip->client, > - MAX17042_SOC) / 256; > + MAX17042_RepSOC) / 256;
Applied. But I don't get it: PROP_CAPACITY should report percents! And it looks that it reports something very different.
Also, if you look down the file, you'll see this:
case POWER_SUPPLY_PROP_CHARGE_FULL: ret = max17042_read_reg(chip->client, MAX17042_RepSOC); if (ret < 0) return ret;
if ((ret >> 8) >= MAX17042_BATTERY_FULL) val->intval = 1; else if (ret >= 0) val->intval = 0; break;
Wut? This is also wrong. PROP_CHARGE_FULL reports uAh, not "fully charged" boolean status.
Please, read Documentation/power/power_supply_class.txt:
~ ~ ~ ~ ~ ~ ~ Charge/Energy/Capacity - how to not confuse ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ Because both "charge" (µAh) and "energy" (µWh) represents "capacity" ~ ~ of battery, this class distinguish these terms. Don't mix them! ~ ~ ~ ~ CHARGE_* attributes represents capacity in µAh only. ~ ~ ENERGY_* attributes represents capacity in µWh only. ~ ~ CAPACITY attribute represents capacity in *percents*, from 0 to 100. ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~
The driver is seriously broken, very. I'd even consider adding 'depends on BROKEN', as the driver might be the source of the confusion for drivers based on this one and for userland devs.
Thanks,
-- Anton Vorontsov Email: cbouatmailru@gmail.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |