Re: [PATCH 2/3] power: supply: bq27xxx: fix power_avg
From: Matthias Schiffer
Date: Wed Feb 24 2021 - 06:51:49 EST
On Tue, 2021年02月23日 at 15:11 +0100, Matthias Schiffer wrote:
>
On all newer bq27xxx ICs, the AveragePower register contains a signed
>
value; in addition to handling the raw value as unsigned, the driver
>
code also didn't convert it to µW as expected.
>
>
At least for the BQ28Z610, the reference manual incorrectly states that
>
the value is in units of 1mA and not 10mA. I have no way of knowing
>
whether the manuals of other supported ICs contain the same error, or if
>
there are models that actually use 1mA. At least, the new code shouldn't
>
be *less* correct than the old version for any device
Ugh, of course this section should talk about mW and not mA. I'll wait
if there is more feedback and then send a v2.
>
.
>
>
power_avg is removed from the cache structure, se we don't have to
>
extend it to store both a signed value and an error code. Always getting
>
an up-to-date value may be desirable anyways, as it avoids inconsistent
>
current and power readings when switching between charging and
>
discharging.
>
>
Signed-off-by: Matthias Schiffer <matthias.schiffer@xxxxxxxxxxxxxxx>
>
---
>
drivers/power/supply/bq27xxx_battery.c | 51 ++++++++++++++------------
>
include/linux/power/bq27xxx_battery.h | 1 -
>
2 files changed, 27 insertions(+), 25 deletions(-)
>
>
diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
>
index cb6ebd2f905e..20e1dc8a87cf 100644
>
--- a/drivers/power/supply/bq27xxx_battery.c
>
+++ b/drivers/power/supply/bq27xxx_battery.c
>
@@ -1661,27 +1661,6 @@ static int bq27xxx_battery_read_time(struct bq27xxx_device_info *di, u8 reg)
>
return tval * 60;
>
}
>
>
-/*
>
- * Read an average power register.
>
- * Return < 0 if something fails.
>
- */
>
-static int bq27xxx_battery_read_pwr_avg(struct bq27xxx_device_info *di)
>
-{
>
- int tval;
>
-
>
- tval = bq27xxx_read(di, BQ27XXX_REG_AP, false);
>
- if (tval < 0) {
>
- dev_err(di->dev, "error reading average power register %02x: %d\n",
>
- BQ27XXX_REG_AP, tval);
>
- return tval;
>
- }
>
-
>
- if (di->opts & BQ27XXX_O_ZERO)
>
- return (tval * BQ27XXX_POWER_CONSTANT) / BQ27XXX_RS;
>
- else
>
- return tval;
>
-}
>
-
>
/*
>
* Returns true if a battery over temperature condition is detected
>
*/
>
@@ -1769,8 +1748,6 @@ void bq27xxx_battery_update(struct bq27xxx_device_info *di)
>
}
>
if (di->regs[BQ27XXX_REG_CYCT] != INVALID_REG_ADDR)
>
cache.cycle_count = bq27xxx_battery_read_cyct(di);
>
- if (di->regs[BQ27XXX_REG_AP] != INVALID_REG_ADDR)
>
- cache.power_avg = bq27xxx_battery_read_pwr_avg(di);
>
>
/* We only have to read charge design full once */
>
if (di->charge_design_full <= 0)
>
@@ -1833,6 +1810,32 @@ static int bq27xxx_battery_current(struct bq27xxx_device_info *di,
>
return 0;
>
}
>
>
+/*
>
+ * Get the average power in µW
>
+ * Return < 0 if something fails.
>
+ */
>
+static int bq27xxx_battery_pwr_avg(struct bq27xxx_device_info *di,
>
+ union power_supply_propval *val)
>
+{
>
+ int power;
>
+
>
+ power = bq27xxx_read(di, BQ27XXX_REG_AP, false);
>
+ if (power < 0) {
>
+ dev_err(di->dev,
>
+ "error reading average power register %02x: %d\n",
>
+ BQ27XXX_REG_AP, power);
>
+ return power;
>
+ }
>
+
>
+ if (di->opts & BQ27XXX_O_ZERO)
>
+ val->intval = (power * BQ27XXX_POWER_CONSTANT) / BQ27XXX_RS;
>
+ else
>
+ /* Other gauges return a signed value in units of 10mW */
>
+ val->intval = (int)((s16)power) * 10000;
>
+
>
+ return 0;
>
+}
>
+
>
static int bq27xxx_battery_status(struct bq27xxx_device_info *di,
>
union power_supply_propval *val)
>
{
>
@@ -2020,7 +2023,7 @@ static int bq27xxx_battery_get_property(struct power_supply *psy,
>
ret = bq27xxx_simple_value(di->cache.energy, val);
>
break;
>
case POWER_SUPPLY_PROP_POWER_AVG:
>
- ret = bq27xxx_simple_value(di->cache.power_avg, val);
>
+ ret = bq27xxx_battery_pwr_avg(di, val);
>
break;
>
case POWER_SUPPLY_PROP_HEALTH:
>
ret = bq27xxx_simple_value(di->cache.health, val);
>
diff --git a/include/linux/power/bq27xxx_battery.h b/include/linux/power/bq27xxx_battery.h
>
index 111a40d0d3d5..8d5f4f40fb41 100644
>
--- a/include/linux/power/bq27xxx_battery.h
>
+++ b/include/linux/power/bq27xxx_battery.h
>
@@ -53,7 +53,6 @@ struct bq27xxx_reg_cache {
>
int capacity;
>
int energy;
>
int flags;
>
- int power_avg;
>
int health;
>
};
>