From 7cc63b29528b33f485bf524eb9a036b2113924f5 Mon Sep 17 00:00:00 2001 From: Liam Breck Date: Sun, 18 Sep 2016 18:07:21 -0700 Subject: [PATCH 1/2] power: bq24190_charger: Call power_supply_changed() only for relevant component We wrongly get uevents for bq24190-charger and bq24190-battery on every register change. Fix the issue by checking the association with charger and/or battery before emitting uevent(s). Fixes: d7bf353fd0aa3 ("bq24190_charger: Add support for TI BQ24190 Battery Charger") Cc: Mark A. Greer Cc: Matt Ranostay Signed-off-by: Liam Breck [tony@atomide.com: set up helper functions for various alerts to keep interrupt code readable] Signed-off-by: Tony Lindgren --- drivers/power/supply/bq24190_charger.c | 87 +++++++++++++++++++------- 1 file changed, 64 insertions(+), 23 deletions(-) diff --git a/drivers/power/supply/bq24190_charger.c b/drivers/power/supply/bq24190_charger.c index 62194d85488809..a2a346ac67d152 100644 --- a/drivers/power/supply/bq24190_charger.c +++ b/drivers/power/supply/bq24190_charger.c @@ -127,6 +127,8 @@ #define BQ24190_REG_F_BAT_FAULT_SHIFT 3 #define BQ24190_REG_F_NTC_FAULT_MASK (BIT(2) | BIT(1) | BIT(0)) #define BQ24190_REG_F_NTC_FAULT_SHIFT 0 +#define BQ24190_REG_F_BAT_ALERT_MASK (BQ24190_REG_F_BAT_FAULT_MASK | \ + BQ24190_REG_F_NTC_FAULT_MASK) #define BQ24190_REG_VPRS 0x0A /* Vendor/Part/Revision Status */ #define BQ24190_REG_VPRS_PN_MASK (BIT(5) | BIT(4) | BIT(3)) @@ -159,7 +161,6 @@ struct bq24190_dev_info { unsigned int gpio_int; unsigned int irq; struct mutex f_reg_lock; - bool first_time; bool charger_health_valid; bool battery_health_valid; bool battery_status_valid; @@ -1194,10 +1195,46 @@ static const struct power_supply_desc bq24190_battery_desc = { .property_is_writeable = bq24190_battery_property_is_writeable, }; +static inline bool +bq24190_has_ss_reg_battery_alert(struct bq24190_dev_info *bdi, + const u8 val) +{ + u8 old, new; + + old = bdi->ss_reg & BQ24190_REG_SS_CHRG_STAT_MASK; + new = val & BQ24190_REG_SS_CHRG_STAT_MASK; + + return old != new; +} + +static inline bool +bq24190_has_ss_reg_charger_alert(struct bq24190_dev_info *bdi, + const u8 val) +{ + u8 old, new; + + old = bdi->ss_reg & ~BQ24190_REG_SS_CHRG_STAT_MASK; + new = val & ~BQ24190_REG_SS_CHRG_STAT_MASK; + + return old != new; +} + +static inline bool +bq24190_has_f_reg_battery_alert(struct bq24190_dev_info *bdi, + const u8 val) +{ + u8 old, new; + + old = bdi->f_reg & BQ24190_REG_F_BAT_ALERT_MASK; + new = val & BQ24190_REG_F_BAT_ALERT_MASK; + + return old != new; +} + static irqreturn_t bq24190_irq_handler_thread(int irq, void *data) { struct bq24190_dev_info *bdi = data; - bool alert_userspace = false; + bool alert_charger = false, alert_battery = false; u8 ss_reg = 0, f_reg = 0; int ret; @@ -1225,8 +1262,12 @@ static irqreturn_t bq24190_irq_handler_thread(int irq, void *data) ret); } + if (bq24190_has_ss_reg_battery_alert(bdi, ss_reg)) + alert_battery = true; + if (bq24190_has_ss_reg_charger_alert(bdi, ss_reg)) + alert_charger = true; + bdi->ss_reg = ss_reg; - alert_userspace = true; } mutex_lock(&bdi->f_reg_lock); @@ -1239,33 +1280,24 @@ static irqreturn_t bq24190_irq_handler_thread(int irq, void *data) } if (f_reg != bdi->f_reg) { + if (bq24190_has_f_reg_battery_alert(bdi, ss_reg)) + alert_battery = true; + + if (bdi->f_reg != f_reg) + alert_charger = true; + bdi->f_reg = f_reg; bdi->charger_health_valid = true; bdi->battery_health_valid = true; bdi->battery_status_valid = true; - - alert_userspace = true; } mutex_unlock(&bdi->f_reg_lock); - /* - * Sometimes bq24190 gives a steady trickle of interrupts even - * though the watchdog timer is turned off and neither the STATUS - * nor FAULT registers have changed. Weed out these sprurious - * interrupts so userspace isn't alerted for no reason. - * In addition, the chip always generates an interrupt after - * register reset so we should ignore that one (the very first - * interrupt received). - */ - if (alert_userspace) { - if (!bdi->first_time) { - power_supply_changed(bdi->charger); - power_supply_changed(bdi->battery); - } else { - bdi->first_time = false; - } - } + if (alert_charger) + power_supply_changed(bdi->charger); + if (alert_battery) + power_supply_changed(bdi->battery); out: pm_runtime_put_sync(bdi->dev); @@ -1300,6 +1332,10 @@ static int bq24190_hw_init(struct bq24190_dev_info *bdi) goto out; ret = bq24190_set_mode_host(bdi); + if (ret < 0) + goto out; + + ret = bq24190_read(bdi, BQ24190_REG_SS, &bdi->ss_reg); out: pm_runtime_put_sync(bdi->dev); return ret; @@ -1375,7 +1411,8 @@ static int bq24190_probe(struct i2c_client *client, bdi->model = id->driver_data; strncpy(bdi->model_name, id->name, I2C_NAME_SIZE); mutex_init(&bdi->f_reg_lock); - bdi->first_time = true; + bdi->f_reg = 0; + bdi->ss_reg = BQ24190_REG_SS_VBUS_STAT_MASK; bdi->charger_health_valid = false; bdi->battery_health_valid = false; bdi->battery_status_valid = false; @@ -1491,12 +1528,16 @@ static int bq24190_pm_resume(struct device *dev) struct i2c_client *client = to_i2c_client(dev); struct bq24190_dev_info *bdi = i2c_get_clientdata(client); + bdi->f_reg = 0; + bdi->ss_reg = BQ24190_REG_SS_VBUS_STAT_MASK; bdi->charger_health_valid = false; bdi->battery_health_valid = false; bdi->battery_status_valid = false; pm_runtime_get_sync(bdi->dev); bq24190_register_reset(bdi); + bq24190_set_mode_host(bdi); + bq24190_read(bdi, BQ24190_REG_SS, &bdi->ss_reg); pm_runtime_put_sync(bdi->dev); /* Things may have changed while suspended so alert upper layer */ From 90b972d7d17600a2fabdea6dcaeb9b4c3c983996 Mon Sep 17 00:00:00 2001 From: Liam Date: Wed, 11 Jan 2017 21:38:38 -0800 Subject: [PATCH 2/2] squash to prev --- drivers/power/supply/bq24190_charger.c | 50 ++++---------------------- 1 file changed, 7 insertions(+), 43 deletions(-) diff --git a/drivers/power/supply/bq24190_charger.c b/drivers/power/supply/bq24190_charger.c index a2a346ac67d152..ba5a5b2af63c71 100644 --- a/drivers/power/supply/bq24190_charger.c +++ b/drivers/power/supply/bq24190_charger.c @@ -127,8 +127,6 @@ #define BQ24190_REG_F_BAT_FAULT_SHIFT 3 #define BQ24190_REG_F_NTC_FAULT_MASK (BIT(2) | BIT(1) | BIT(0)) #define BQ24190_REG_F_NTC_FAULT_SHIFT 0 -#define BQ24190_REG_F_BAT_ALERT_MASK (BQ24190_REG_F_BAT_FAULT_MASK | \ - BQ24190_REG_F_NTC_FAULT_MASK) #define BQ24190_REG_VPRS 0x0A /* Vendor/Part/Revision Status */ #define BQ24190_REG_VPRS_PN_MASK (BIT(5) | BIT(4) | BIT(3)) @@ -1195,45 +1193,12 @@ static const struct power_supply_desc bq24190_battery_desc = { .property_is_writeable = bq24190_battery_property_is_writeable, }; -static inline bool -bq24190_has_ss_reg_battery_alert(struct bq24190_dev_info *bdi, - const u8 val) -{ - u8 old, new; - - old = bdi->ss_reg & BQ24190_REG_SS_CHRG_STAT_MASK; - new = val & BQ24190_REG_SS_CHRG_STAT_MASK; - - return old != new; -} - -static inline bool -bq24190_has_ss_reg_charger_alert(struct bq24190_dev_info *bdi, - const u8 val) -{ - u8 old, new; - - old = bdi->ss_reg & ~BQ24190_REG_SS_CHRG_STAT_MASK; - new = val & ~BQ24190_REG_SS_CHRG_STAT_MASK; - - return old != new; -} - -static inline bool -bq24190_has_f_reg_battery_alert(struct bq24190_dev_info *bdi, - const u8 val) -{ - u8 old, new; - - old = bdi->f_reg & BQ24190_REG_F_BAT_ALERT_MASK; - new = val & BQ24190_REG_F_BAT_ALERT_MASK; - - return old != new; -} - static irqreturn_t bq24190_irq_handler_thread(int irq, void *data) { struct bq24190_dev_info *bdi = data; + const u8 battery_mask_ss = BQ24190_REG_SS_CHRG_STAT_MASK; + const u8 battery_mask_f = BQ24190_REG_F_BAT_FAULT_MASK + | BQ24190_REG_F_NTC_FAULT_MASK; bool alert_charger = false, alert_battery = false; u8 ss_reg = 0, f_reg = 0; int ret; @@ -1262,9 +1227,9 @@ static irqreturn_t bq24190_irq_handler_thread(int irq, void *data) ret); } - if (bq24190_has_ss_reg_battery_alert(bdi, ss_reg)) + if ((bdi->ss_reg & battery_mask_ss) != (ss_reg & battery_mask_ss)) alert_battery = true; - if (bq24190_has_ss_reg_charger_alert(bdi, ss_reg)) + if ((bdi->ss_reg & ~battery_mask_ss) != (ss_reg & ~battery_mask_ss)) alert_charger = true; bdi->ss_reg = ss_reg; @@ -1280,10 +1245,9 @@ static irqreturn_t bq24190_irq_handler_thread(int irq, void *data) } if (f_reg != bdi->f_reg) { - if (bq24190_has_f_reg_battery_alert(bdi, ss_reg)) + if ((bdi->f_reg & battery_mask_f) != (f_reg & battery_mask_f)) alert_battery = true; - - if (bdi->f_reg != f_reg) + if ((bdi->f_reg & ~battery_mask_f) != (f_reg & ~battery_mask_f)) alert_charger = true; bdi->f_reg = f_reg;