* [PATCH v2 2/3] power: supply: bq27xxx: fix power_avg for newer ICs
2021-03-03 9:54 [PATCH v2 1/3] power: supply: bq27xxx: fix sign of current_now for newer ICs Matthias Schiffer
@ 2021-03-03 9:54 ` Matthias Schiffer
2021-03-15 1:55 ` Sebastian Reichel
2021-03-03 9:54 ` [PATCH v2 3/3] power: supply: bq27xxx: make status more robust Matthias Schiffer
2021-03-15 1:54 ` [PATCH v2 1/3] power: supply: bq27xxx: fix sign of current_now for newer ICs Sebastian Reichel
2 siblings, 1 reply; 6+ messages in thread
From: Matthias Schiffer @ 2021-03-03 9:54 UTC (permalink / raw)
To: Pali Rohár, Sebastian Reichel
Cc: Andreas Kemnade, linux-pm, linux-kernel, Matthias Schiffer
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 1mW and not 10mW. 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 1mW. At least, the new code shouldn't
be *less* correct than the old version for any device.
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@ew.tq-group.com>
---
v2: fixed units in commit message
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;
};
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 2/3] power: supply: bq27xxx: fix power_avg for newer ICs
2021-03-03 9:54 ` [PATCH v2 2/3] power: supply: bq27xxx: fix power_avg " Matthias Schiffer
@ 2021-03-15 1:55 ` Sebastian Reichel
0 siblings, 0 replies; 6+ messages in thread
From: Sebastian Reichel @ 2021-03-15 1:55 UTC (permalink / raw)
To: Matthias Schiffer
Cc: Pali Rohár, Andreas Kemnade, linux-pm, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 4465 bytes --]
Hi,
On Wed, Mar 03, 2021 at 10:54:19AM +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 1mW and not 10mW. 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 1mW. At least, the new code shouldn't
> be *less* correct than the old version for any device.
>
> 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@ew.tq-group.com>
> ---
Thanks, queued.
-- Sebastian
>
> v2: fixed units in commit message
>
> 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;
> };
>
> --
> 2.17.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 3/3] power: supply: bq27xxx: make status more robust
2021-03-03 9:54 [PATCH v2 1/3] power: supply: bq27xxx: fix sign of current_now for newer ICs Matthias Schiffer
2021-03-03 9:54 ` [PATCH v2 2/3] power: supply: bq27xxx: fix power_avg " Matthias Schiffer
@ 2021-03-03 9:54 ` Matthias Schiffer
2021-03-15 1:55 ` Sebastian Reichel
2021-03-15 1:54 ` [PATCH v2 1/3] power: supply: bq27xxx: fix sign of current_now for newer ICs Sebastian Reichel
2 siblings, 1 reply; 6+ messages in thread
From: Matthias Schiffer @ 2021-03-03 9:54 UTC (permalink / raw)
To: Pali Rohár, Sebastian Reichel
Cc: Andreas Kemnade, linux-pm, linux-kernel, Matthias Schiffer
There are multiple issues in bq27xxx_battery_status():
- On BQ28Q610 is was observed that the "full" flag may be set even while
the battery is charging or discharging. With the current logic to make
"full" override everything else, it look a very long time (>20min) for
the status to change from "full" to "discharging" after unplugging the
supply on a device with low power consumption
- The POWER_SUPPLY_STATUS_NOT_CHARGING check depends on
power_supply_am_i_supplied(), which will not work when the supply
doesn't exist as a separate device known to Linux
We can solve both issues by deriving the status from the current instead
of the flags field. The flags are now only used to distinguish "full"
from "not charging", and to determine the sign of the current on
BQ27XXX_O_ZERO devices.
Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
---
v2: no changes
drivers/power/supply/bq27xxx_battery.c | 88 +++++++++++++-------------
1 file changed, 43 insertions(+), 45 deletions(-)
diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
index 20e1dc8a87cf..b62a8cfd9d09 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -1777,14 +1777,27 @@ static void bq27xxx_battery_poll(struct work_struct *work)
schedule_delayed_work(&di->work, poll_interval * HZ);
}
+static bool bq27xxx_battery_is_full(struct bq27xxx_device_info *di, int flags)
+{
+ if (di->opts & BQ27XXX_O_ZERO)
+ return (flags & BQ27000_FLAG_FC);
+ else if (di->opts & BQ27Z561_O_BITS)
+ return (flags & BQ27Z561_FLAG_FC);
+ else
+ return (flags & BQ27XXX_FLAG_FC);
+}
+
/*
- * Return the battery average current in µA
+ * Return the battery average current in µA and the status
* Note that current can be negative signed as well
* Or 0 if something fails.
*/
-static int bq27xxx_battery_current(struct bq27xxx_device_info *di,
- union power_supply_propval *val)
+static int bq27xxx_battery_current_and_status(
+ struct bq27xxx_device_info *di,
+ union power_supply_propval *val_curr,
+ union power_supply_propval *val_status)
{
+ bool single_flags = (di->opts & BQ27XXX_O_ZERO);
int curr;
int flags;
@@ -1794,17 +1807,39 @@ static int bq27xxx_battery_current(struct bq27xxx_device_info *di,
return curr;
}
+ flags = bq27xxx_read(di, BQ27XXX_REG_FLAGS, single_flags);
+ if (flags < 0) {
+ dev_err(di->dev, "error reading flags\n");
+ return flags;
+ }
+
if (di->opts & BQ27XXX_O_ZERO) {
- flags = bq27xxx_read(di, BQ27XXX_REG_FLAGS, true);
if (!(flags & BQ27000_FLAG_CHGS)) {
dev_dbg(di->dev, "negative current!\n");
curr = -curr;
}
- val->intval = curr * BQ27XXX_CURRENT_CONSTANT / BQ27XXX_RS;
+ curr = curr * BQ27XXX_CURRENT_CONSTANT / BQ27XXX_RS;
} else {
/* Other gauges return signed value */
- val->intval = (int)((s16)curr) * 1000;
+ curr = (int)((s16)curr) * 1000;
+ }
+
+ if (val_curr)
+ val_curr->intval = curr;
+
+ if (val_status) {
+ if (curr > 0) {
+ val_status->intval = POWER_SUPPLY_STATUS_CHARGING;
+ } else if (curr < 0) {
+ val_status->intval = POWER_SUPPLY_STATUS_DISCHARGING;
+ } else {
+ if (bq27xxx_battery_is_full(di, flags))
+ val_status->intval = POWER_SUPPLY_STATUS_FULL;
+ else
+ val_status->intval =
+ POWER_SUPPLY_STATUS_NOT_CHARGING;
+ }
}
return 0;
@@ -1836,43 +1871,6 @@ static int bq27xxx_battery_pwr_avg(struct bq27xxx_device_info *di,
return 0;
}
-static int bq27xxx_battery_status(struct bq27xxx_device_info *di,
- union power_supply_propval *val)
-{
- int status;
-
- if (di->opts & BQ27XXX_O_ZERO) {
- if (di->cache.flags & BQ27000_FLAG_FC)
- status = POWER_SUPPLY_STATUS_FULL;
- else if (di->cache.flags & BQ27000_FLAG_CHGS)
- status = POWER_SUPPLY_STATUS_CHARGING;
- else
- status = POWER_SUPPLY_STATUS_DISCHARGING;
- } else if (di->opts & BQ27Z561_O_BITS) {
- if (di->cache.flags & BQ27Z561_FLAG_FC)
- status = POWER_SUPPLY_STATUS_FULL;
- else if (di->cache.flags & BQ27Z561_FLAG_DIS_CH)
- status = POWER_SUPPLY_STATUS_DISCHARGING;
- else
- status = POWER_SUPPLY_STATUS_CHARGING;
- } else {
- if (di->cache.flags & BQ27XXX_FLAG_FC)
- status = POWER_SUPPLY_STATUS_FULL;
- else if (di->cache.flags & BQ27XXX_FLAG_DSC)
- status = POWER_SUPPLY_STATUS_DISCHARGING;
- else
- status = POWER_SUPPLY_STATUS_CHARGING;
- }
-
- if ((status == POWER_SUPPLY_STATUS_DISCHARGING) &&
- (power_supply_am_i_supplied(di->bat) > 0))
- status = POWER_SUPPLY_STATUS_NOT_CHARGING;
-
- val->intval = status;
-
- return 0;
-}
-
static int bq27xxx_battery_capacity_level(struct bq27xxx_device_info *di,
union power_supply_propval *val)
{
@@ -1960,7 +1958,7 @@ static int bq27xxx_battery_get_property(struct power_supply *psy,
switch (psp) {
case POWER_SUPPLY_PROP_STATUS:
- ret = bq27xxx_battery_status(di, val);
+ ret = bq27xxx_battery_current_and_status(di, NULL, val);
break;
case POWER_SUPPLY_PROP_VOLTAGE_NOW:
ret = bq27xxx_battery_voltage(di, val);
@@ -1969,7 +1967,7 @@ static int bq27xxx_battery_get_property(struct power_supply *psy,
val->intval = di->cache.flags < 0 ? 0 : 1;
break;
case POWER_SUPPLY_PROP_CURRENT_NOW:
- ret = bq27xxx_battery_current(di, val);
+ ret = bq27xxx_battery_current_and_status(di, val, NULL);
break;
case POWER_SUPPLY_PROP_CAPACITY:
ret = bq27xxx_simple_value(di->cache.capacity, val);
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 3/3] power: supply: bq27xxx: make status more robust
2021-03-03 9:54 ` [PATCH v2 3/3] power: supply: bq27xxx: make status more robust Matthias Schiffer
@ 2021-03-15 1:55 ` Sebastian Reichel
0 siblings, 0 replies; 6+ messages in thread
From: Sebastian Reichel @ 2021-03-15 1:55 UTC (permalink / raw)
To: Matthias Schiffer
Cc: Pali Rohár, Andreas Kemnade, linux-pm, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 6030 bytes --]
Hi,
On Wed, Mar 03, 2021 at 10:54:20AM +0100, Matthias Schiffer wrote:
> There are multiple issues in bq27xxx_battery_status():
>
> - On BQ28Q610 is was observed that the "full" flag may be set even while
> the battery is charging or discharging. With the current logic to make
> "full" override everything else, it look a very long time (>20min) for
> the status to change from "full" to "discharging" after unplugging the
> supply on a device with low power consumption
> - The POWER_SUPPLY_STATUS_NOT_CHARGING check depends on
> power_supply_am_i_supplied(), which will not work when the supply
> doesn't exist as a separate device known to Linux
>
> We can solve both issues by deriving the status from the current instead
> of the flags field. The flags are now only used to distinguish "full"
> from "not charging", and to determine the sign of the current on
> BQ27XXX_O_ZERO devices.
>
> Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> ---
Thanks, queued.
-- Sebastian
>
> v2: no changes
>
> drivers/power/supply/bq27xxx_battery.c | 88 +++++++++++++-------------
> 1 file changed, 43 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
> index 20e1dc8a87cf..b62a8cfd9d09 100644
> --- a/drivers/power/supply/bq27xxx_battery.c
> +++ b/drivers/power/supply/bq27xxx_battery.c
> @@ -1777,14 +1777,27 @@ static void bq27xxx_battery_poll(struct work_struct *work)
> schedule_delayed_work(&di->work, poll_interval * HZ);
> }
>
> +static bool bq27xxx_battery_is_full(struct bq27xxx_device_info *di, int flags)
> +{
> + if (di->opts & BQ27XXX_O_ZERO)
> + return (flags & BQ27000_FLAG_FC);
> + else if (di->opts & BQ27Z561_O_BITS)
> + return (flags & BQ27Z561_FLAG_FC);
> + else
> + return (flags & BQ27XXX_FLAG_FC);
> +}
> +
> /*
> - * Return the battery average current in µA
> + * Return the battery average current in µA and the status
> * Note that current can be negative signed as well
> * Or 0 if something fails.
> */
> -static int bq27xxx_battery_current(struct bq27xxx_device_info *di,
> - union power_supply_propval *val)
> +static int bq27xxx_battery_current_and_status(
> + struct bq27xxx_device_info *di,
> + union power_supply_propval *val_curr,
> + union power_supply_propval *val_status)
> {
> + bool single_flags = (di->opts & BQ27XXX_O_ZERO);
> int curr;
> int flags;
>
> @@ -1794,17 +1807,39 @@ static int bq27xxx_battery_current(struct bq27xxx_device_info *di,
> return curr;
> }
>
> + flags = bq27xxx_read(di, BQ27XXX_REG_FLAGS, single_flags);
> + if (flags < 0) {
> + dev_err(di->dev, "error reading flags\n");
> + return flags;
> + }
> +
> if (di->opts & BQ27XXX_O_ZERO) {
> - flags = bq27xxx_read(di, BQ27XXX_REG_FLAGS, true);
> if (!(flags & BQ27000_FLAG_CHGS)) {
> dev_dbg(di->dev, "negative current!\n");
> curr = -curr;
> }
>
> - val->intval = curr * BQ27XXX_CURRENT_CONSTANT / BQ27XXX_RS;
> + curr = curr * BQ27XXX_CURRENT_CONSTANT / BQ27XXX_RS;
> } else {
> /* Other gauges return signed value */
> - val->intval = (int)((s16)curr) * 1000;
> + curr = (int)((s16)curr) * 1000;
> + }
> +
> + if (val_curr)
> + val_curr->intval = curr;
> +
> + if (val_status) {
> + if (curr > 0) {
> + val_status->intval = POWER_SUPPLY_STATUS_CHARGING;
> + } else if (curr < 0) {
> + val_status->intval = POWER_SUPPLY_STATUS_DISCHARGING;
> + } else {
> + if (bq27xxx_battery_is_full(di, flags))
> + val_status->intval = POWER_SUPPLY_STATUS_FULL;
> + else
> + val_status->intval =
> + POWER_SUPPLY_STATUS_NOT_CHARGING;
> + }
> }
>
> return 0;
> @@ -1836,43 +1871,6 @@ static int bq27xxx_battery_pwr_avg(struct bq27xxx_device_info *di,
> return 0;
> }
>
> -static int bq27xxx_battery_status(struct bq27xxx_device_info *di,
> - union power_supply_propval *val)
> -{
> - int status;
> -
> - if (di->opts & BQ27XXX_O_ZERO) {
> - if (di->cache.flags & BQ27000_FLAG_FC)
> - status = POWER_SUPPLY_STATUS_FULL;
> - else if (di->cache.flags & BQ27000_FLAG_CHGS)
> - status = POWER_SUPPLY_STATUS_CHARGING;
> - else
> - status = POWER_SUPPLY_STATUS_DISCHARGING;
> - } else if (di->opts & BQ27Z561_O_BITS) {
> - if (di->cache.flags & BQ27Z561_FLAG_FC)
> - status = POWER_SUPPLY_STATUS_FULL;
> - else if (di->cache.flags & BQ27Z561_FLAG_DIS_CH)
> - status = POWER_SUPPLY_STATUS_DISCHARGING;
> - else
> - status = POWER_SUPPLY_STATUS_CHARGING;
> - } else {
> - if (di->cache.flags & BQ27XXX_FLAG_FC)
> - status = POWER_SUPPLY_STATUS_FULL;
> - else if (di->cache.flags & BQ27XXX_FLAG_DSC)
> - status = POWER_SUPPLY_STATUS_DISCHARGING;
> - else
> - status = POWER_SUPPLY_STATUS_CHARGING;
> - }
> -
> - if ((status == POWER_SUPPLY_STATUS_DISCHARGING) &&
> - (power_supply_am_i_supplied(di->bat) > 0))
> - status = POWER_SUPPLY_STATUS_NOT_CHARGING;
> -
> - val->intval = status;
> -
> - return 0;
> -}
> -
> static int bq27xxx_battery_capacity_level(struct bq27xxx_device_info *di,
> union power_supply_propval *val)
> {
> @@ -1960,7 +1958,7 @@ static int bq27xxx_battery_get_property(struct power_supply *psy,
>
> switch (psp) {
> case POWER_SUPPLY_PROP_STATUS:
> - ret = bq27xxx_battery_status(di, val);
> + ret = bq27xxx_battery_current_and_status(di, NULL, val);
> break;
> case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> ret = bq27xxx_battery_voltage(di, val);
> @@ -1969,7 +1967,7 @@ static int bq27xxx_battery_get_property(struct power_supply *psy,
> val->intval = di->cache.flags < 0 ? 0 : 1;
> break;
> case POWER_SUPPLY_PROP_CURRENT_NOW:
> - ret = bq27xxx_battery_current(di, val);
> + ret = bq27xxx_battery_current_and_status(di, val, NULL);
> break;
> case POWER_SUPPLY_PROP_CAPACITY:
> ret = bq27xxx_simple_value(di->cache.capacity, val);
> --
> 2.17.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/3] power: supply: bq27xxx: fix sign of current_now for newer ICs
2021-03-03 9:54 [PATCH v2 1/3] power: supply: bq27xxx: fix sign of current_now for newer ICs Matthias Schiffer
2021-03-03 9:54 ` [PATCH v2 2/3] power: supply: bq27xxx: fix power_avg " Matthias Schiffer
2021-03-03 9:54 ` [PATCH v2 3/3] power: supply: bq27xxx: make status more robust Matthias Schiffer
@ 2021-03-15 1:54 ` Sebastian Reichel
2 siblings, 0 replies; 6+ messages in thread
From: Sebastian Reichel @ 2021-03-15 1:54 UTC (permalink / raw)
To: Matthias Schiffer
Cc: Pali Rohár, Andreas Kemnade, linux-pm, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1393 bytes --]
Hi,
On Wed, Mar 03, 2021 at 10:54:18AM +0100, Matthias Schiffer wrote:
> Commit cd060b4d0868 ("power: supply: bq27xxx: fix polarity of current_now")
> changed the sign of current_now for all bq27xxx variants, but on BQ28Z610
> I'm now seeing negated values *with* that patch.
>
> The GTA04/Openmoko device that was used for testing uses a BQ27000 or
> BQ27010 IC, so I assume only the BQ27XXX_O_ZERO code path was incorrect.
> Revert the behaviour for newer ICs.
>
> Fixes: cd060b4d0868 "power: supply: bq27xxx: fix polarity of current_now"
> Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> ---
Thanks, queued.
-- Sebastian
>
> v2: no changes
>
> drivers/power/supply/bq27xxx_battery.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
> index 4c4a7b1c64c5..cb6ebd2f905e 100644
> --- a/drivers/power/supply/bq27xxx_battery.c
> +++ b/drivers/power/supply/bq27xxx_battery.c
> @@ -1827,7 +1827,7 @@ static int bq27xxx_battery_current(struct bq27xxx_device_info *di,
> val->intval = curr * BQ27XXX_CURRENT_CONSTANT / BQ27XXX_RS;
> } else {
> /* Other gauges return signed value */
> - val->intval = -(int)((s16)curr) * 1000;
> + val->intval = (int)((s16)curr) * 1000;
> }
>
> return 0;
> --
> 2.17.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread