Messages in this thread | | | Date | Tue, 13 Mar 2012 15:54:25 -0700 | From | Dirk Brandewie <> | Subject | Re: [PATCH 5/5] max17042: Change capacity property to use reported SOC register |
| |
On 03/13/2012 11:34 AM, Anton Vorontsov wrote: > 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. >
The resolution of the register is 1/256 of a percent so we are actually returning the value in percent.
> 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; >
I hadn't looked at this property. Looks like that code came from commit 60a1f6e4
> 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, >
-- 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/
| |