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.schif...@ew.tq-group.com>
> ---
> 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;
> };
>
