+ Sebastian
I'm not sure if he expects to be on the To/Cc, but that usually is the way to get a maintainer to take your patch.
Brian
On Wed, Aug 21, 2019 at 06:46:55PM -0700, Brian Norris wrote:
On Fri, Aug 16, 2019 at 09:58:42AM +0200, Michael Nosthoff wrote:
when the battery is set to sbs-mode and no gpio detection is enabled "health" is always returning a value even when the battery is not present. All other fields return "not present". This leads to a scenario where the driver is constantly switching between "present" and "not present" state. This generates a lot of constant traffic on the i2c.
That depends on how often you're checking the "health" attribute, doesn't it? But anyway, the bug is real.
This commit changes the response of "health" to an error when the battery is not responding leading to a consistent "not present" state.
Ack, and thanks for the fix.
Fixes: 76b16f4cdfb8 ("power: supply: sbs-battery: don't assume MANUFACTURER_DATA formats")
Signed-off-by: Michael Nosthoff committed@heine.so Cc: Brian Norris briannorris@chromium.org Cc: stable@vger.kernel.org
Reviewed-by: Brian Norris briannorris@chromium.org Tested-by: Brian Norris briannorris@chromium.org
drivers/power/supply/sbs-battery.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/drivers/power/supply/sbs-battery.c b/drivers/power/supply/sbs-battery.c index 2e86cc1e0e35..f8d74e9f7931 100644 --- a/drivers/power/supply/sbs-battery.c +++ b/drivers/power/supply/sbs-battery.c @@ -314,17 +314,22 @@ static int sbs_get_battery_presence_and_health( { int ret;
- if (psp == POWER_SUPPLY_PROP_PRESENT) {
/* Dummy command; if it succeeds, battery is present. */ret = sbs_read_word_data(client, sbs_data[REG_STATUS].addr);if (ret < 0)val->intval = 0; /* battery disconnected */elseval->intval = 1; /* battery present */- } else { /* POWER_SUPPLY_PROP_HEALTH */
- /* Dummy command; if it succeeds, battery is present. */
- ret = sbs_read_word_data(client, sbs_data[REG_STATUS].addr);
- if (ret < 0) { /* battery not present*/
if (psp == POWER_SUPPLY_PROP_PRESENT) {val->intval = 0;return 0;Technically, you don't need the 'return 0' (and if we care about symmetry: the TI version doesn't), since the caller knows that "not present" will yield errors. I'm not sure which version makes more sense.
}return ret;- }
- if (psp == POWER_SUPPLY_PROP_PRESENT)
val->intval = 1; /* battery present */- else /* POWER_SUPPLY_PROP_HEALTH */ /* SBS spec doesn't have a general health command. */ val->intval = POWER_SUPPLY_HEALTH_UNKNOWN;
- }
return 0; } @@ -626,6 +631,8 @@ static int sbs_get_property(struct power_supply *psy, else ret = sbs_get_battery_presence_and_health(client, psp, val);
/* this can only be true if no gpio is used */-- 2.20.1