* [PATCH v4 1/7] power: bq24190_charger: Fix irq trigger to IRQF_TRIGGER_FALLING
2017-01-18 17:26 ` [PATCH v4 0/7] " Liam Breck
@ 2017-01-18 17:26 ` Liam Breck
2017-01-18 17:26 ` [PATCH v4 2/7] power: bq24190_charger: Call set_mode_host() on pm_resume() Liam Breck
` (6 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Liam Breck @ 2017-01-18 17:26 UTC (permalink / raw)
To: Sebastian Reichel; +Cc: linux-pm, Mark Greer, Tony Lindgren, Liam Breck
The interrupt signal is TRIGGER_FALLING. This is is specified in the
data sheet PIN FUNCTIONS: "The INT pin sends active low, 256us
pulse to host to report charger device status and fault."
Also the direction can be seen in the data sheet Figure 37 "BQ24190
with D+/D- Detection and USB On-The-Go (OTG)" which shows a 10k
pull-up resistor installed for the sample configurations.
Fixes: d7bf353fd0aa3 ("bq24190_charger: Add support for TI BQ24190 Battery Charger")
Signed-off-by: Liam Breck <kernel@networkimprov.net>
Acked-by: Mark Greer <mgreer@animalcreek.com>
Acked-by: Tony Lindgren <tony@atomide.com>
---
drivers/power/supply/bq24190_charger.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/power/supply/bq24190_charger.c b/drivers/power/supply/bq24190_charger.c
index 694c088..f5746b9 100644
--- a/drivers/power/supply/bq24190_charger.c
+++ b/drivers/power/supply/bq24190_charger.c
@@ -1394,7 +1394,7 @@ static int bq24190_probe(struct i2c_client *client,
ret = devm_request_threaded_irq(dev, bdi->irq, NULL,
bq24190_irq_handler_thread,
- IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+ IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
"bq24190-charger", bdi);
if (ret < 0) {
dev_err(dev, "Can't set up irq handler\n");
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v4 2/7] power: bq24190_charger: Call set_mode_host() on pm_resume()
2017-01-18 17:26 ` [PATCH v4 0/7] " Liam Breck
2017-01-18 17:26 ` [PATCH v4 1/7] power: bq24190_charger: Fix irq trigger to IRQF_TRIGGER_FALLING Liam Breck
@ 2017-01-18 17:26 ` Liam Breck
2017-01-18 17:26 ` [PATCH v4 3/7] power: bq24190_charger: Install irq_handler_thread() at end of probe() Liam Breck
` (5 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Liam Breck @ 2017-01-18 17:26 UTC (permalink / raw)
To: Sebastian Reichel; +Cc: linux-pm, Mark Greer, Tony Lindgren, Liam Breck
pm_resume() does a register_reset() which clears charger host mode.
Fix by calling set_mode_host() after the reset.
Fixes: d7bf353fd0aa3 ("bq24190_charger: Add support for TI BQ24190 Battery Charger")
Signed-off-by: Liam Breck <kernel@networkimprov.net>
Acked-by: Mark Greer <mgreer@animalcreek.com>
Acked-by: Tony Lindgren <tony@atomide.com>
---
drivers/power/supply/bq24190_charger.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/power/supply/bq24190_charger.c b/drivers/power/supply/bq24190_charger.c
index f5746b9..b51eac1 100644
--- a/drivers/power/supply/bq24190_charger.c
+++ b/drivers/power/supply/bq24190_charger.c
@@ -1494,6 +1494,7 @@ static int bq24190_pm_resume(struct device *dev)
pm_runtime_get_sync(bdi->dev);
bq24190_register_reset(bdi);
+ bq24190_set_mode_host(bdi);
pm_runtime_put_sync(bdi->dev);
/* Things may have changed while suspended so alert upper layer */
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v4 3/7] power: bq24190_charger: Install irq_handler_thread() at end of probe()
2017-01-18 17:26 ` [PATCH v4 0/7] " Liam Breck
2017-01-18 17:26 ` [PATCH v4 1/7] power: bq24190_charger: Fix irq trigger to IRQF_TRIGGER_FALLING Liam Breck
2017-01-18 17:26 ` [PATCH v4 2/7] power: bq24190_charger: Call set_mode_host() on pm_resume() Liam Breck
@ 2017-01-18 17:26 ` Liam Breck
2017-01-18 17:26 ` [PATCH v4 4/7] power: bq24190_charger: Adjust formatting Liam Breck
` (4 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Liam Breck @ 2017-01-18 17:26 UTC (permalink / raw)
To: Sebastian Reichel; +Cc: linux-pm, Mark Greer, Tony Lindgren, Liam Breck
The device specific data is not fully initialized on
request_threaded_irq(). This may cause a crash when the IRQ handler
tries to reference them.
Fix the issue by installing IRQ handler at the end of the probe.
Fixes: d7bf353fd0aa3 ("bq24190_charger: Add support for TI BQ24190 Battery Charger")
Signed-off-by: Liam Breck <kernel@networkimprov.net>
Acked-by: Mark Greer <mgreer@animalcreek.com>
Acked-by: Tony Lindgren <tony@atomide.com>
---
drivers/power/supply/bq24190_charger.c | 35 ++++++++++++++++---------------
1 file changed, 19 insertions(+), 16 deletions(-)
diff --git a/drivers/power/supply/bq24190_charger.c b/drivers/power/supply/bq24190_charger.c
index b51eac1..54c8952 100644
--- a/drivers/power/supply/bq24190_charger.c
+++ b/drivers/power/supply/bq24190_charger.c
@@ -1392,22 +1392,13 @@ static int bq24190_probe(struct i2c_client *client,
return -EINVAL;
}
- ret = devm_request_threaded_irq(dev, bdi->irq, NULL,
- bq24190_irq_handler_thread,
- IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
- "bq24190-charger", bdi);
- if (ret < 0) {
- dev_err(dev, "Can't set up irq handler\n");
- goto out1;
- }
-
pm_runtime_enable(dev);
pm_runtime_resume(dev);
ret = bq24190_hw_init(bdi);
if (ret < 0) {
dev_err(dev, "Hardware init failed\n");
- goto out2;
+ goto out1;
}
charger_cfg.drv_data = bdi;
@@ -1418,7 +1409,7 @@ static int bq24190_probe(struct i2c_client *client,
if (IS_ERR(bdi->charger)) {
dev_err(dev, "Can't register charger\n");
ret = PTR_ERR(bdi->charger);
- goto out2;
+ goto out1;
}
battery_cfg.drv_data = bdi;
@@ -1427,24 +1418,34 @@ static int bq24190_probe(struct i2c_client *client,
if (IS_ERR(bdi->battery)) {
dev_err(dev, "Can't register battery\n");
ret = PTR_ERR(bdi->battery);
- goto out3;
+ goto out2;
}
ret = bq24190_sysfs_create_group(bdi);
if (ret) {
dev_err(dev, "Can't create sysfs entries\n");
+ goto out3;
+ }
+
+ ret = devm_request_threaded_irq(dev, bdi->irq, NULL,
+ bq24190_irq_handler_thread,
+ IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+ "bq24190-charger", bdi);
+ if (ret < 0) {
+ dev_err(dev, "Can't set up irq handler\n");
goto out4;
}
return 0;
out4:
- power_supply_unregister(bdi->battery);
+ bq24190_sysfs_remove_group(bdi);
out3:
- power_supply_unregister(bdi->charger);
+ power_supply_unregister(bdi->battery);
out2:
- pm_runtime_disable(dev);
+ power_supply_unregister(bdi->charger);
out1:
+ pm_runtime_disable(dev);
if (bdi->gpio_int)
gpio_free(bdi->gpio_int);
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v4 4/7] power: bq24190_charger: Adjust formatting
2017-01-18 17:26 ` [PATCH v4 0/7] " Liam Breck
` (2 preceding siblings ...)
2017-01-18 17:26 ` [PATCH v4 3/7] power: bq24190_charger: Install irq_handler_thread() at end of probe() Liam Breck
@ 2017-01-18 17:26 ` Liam Breck
2017-01-18 17:26 ` [PATCH v4 5/7] power: bq24190_charger: Call power_supply_changed() for relevant component Liam Breck
` (3 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Liam Breck @ 2017-01-18 17:26 UTC (permalink / raw)
To: Sebastian Reichel; +Cc: linux-pm, Mark Greer, Tony Lindgren, Liam Breck
Add breathing room in probe() out* section.
Fixes: d7bf353fd0aa3 ("bq24190_charger: Add support for TI BQ24190 Battery Charger")
Signed-off-by: Liam Breck <kernel@networkimprov.net>
Acked-by: Mark A. Greer <mgreer@animalcreek.com>
Acked-by: Tony Lindgren <tony@atomide.com>
---
drivers/power/supply/bq24190_charger.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/power/supply/bq24190_charger.c b/drivers/power/supply/bq24190_charger.c
index 54c8952..62194d8 100644
--- a/drivers/power/supply/bq24190_charger.c
+++ b/drivers/power/supply/bq24190_charger.c
@@ -1440,15 +1440,17 @@ static int bq24190_probe(struct i2c_client *client,
out4:
bq24190_sysfs_remove_group(bdi);
+
out3:
power_supply_unregister(bdi->battery);
+
out2:
power_supply_unregister(bdi->charger);
+
out1:
pm_runtime_disable(dev);
if (bdi->gpio_int)
gpio_free(bdi->gpio_int);
-
return ret;
}
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v4 5/7] power: bq24190_charger: Call power_supply_changed() for relevant component
2017-01-18 17:26 ` [PATCH v4 0/7] " Liam Breck
` (3 preceding siblings ...)
2017-01-18 17:26 ` [PATCH v4 4/7] power: bq24190_charger: Adjust formatting Liam Breck
@ 2017-01-18 17:26 ` Liam Breck
2017-01-18 17:26 ` [PATCH v4 6/7] power: bq24190_charger: Don't read fault register outside irq_handle_thread() Liam Breck
` (2 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Liam Breck @ 2017-01-18 17:26 UTC (permalink / raw)
To: Sebastian Reichel; +Cc: linux-pm, Mark Greer, Tony Lindgren, Liam Breck
We wrongly get uevents for bq24190-charger and bq24190-battery on every
register change.
Fix by checking the association with charger and battery before
emitting uevent(s).
Fixes: d7bf353fd0aa3 ("bq24190_charger: Add support for TI BQ24190 Battery Charger")
Signed-off-by: Liam Breck <kernel@networkimprov.net>
Acked-by: Mark Greer <mgreer@animalcreek.com>
Acked-by: Tony Lindgren <tony@atomide.com>
---
drivers/power/supply/bq24190_charger.c | 50 +++++++++++++++++++---------------
1 file changed, 27 insertions(+), 23 deletions(-)
diff --git a/drivers/power/supply/bq24190_charger.c b/drivers/power/supply/bq24190_charger.c
index 62194d8..ba5a5b2 100644
--- a/drivers/power/supply/bq24190_charger.c
+++ b/drivers/power/supply/bq24190_charger.c
@@ -159,7 +159,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;
@@ -1197,7 +1196,10 @@ static const struct power_supply_desc bq24190_battery_desc = {
static irqreturn_t bq24190_irq_handler_thread(int irq, void *data)
{
struct bq24190_dev_info *bdi = data;
- bool alert_userspace = false;
+ 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;
@@ -1225,8 +1227,12 @@ static irqreturn_t bq24190_irq_handler_thread(int irq, void *data)
ret);
}
+ if ((bdi->ss_reg & battery_mask_ss) != (ss_reg & battery_mask_ss))
+ alert_battery = true;
+ if ((bdi->ss_reg & ~battery_mask_ss) != (ss_reg & ~battery_mask_ss))
+ alert_charger = true;
+
bdi->ss_reg = ss_reg;
- alert_userspace = true;
}
mutex_lock(&bdi->f_reg_lock);
@@ -1239,33 +1245,23 @@ static irqreturn_t bq24190_irq_handler_thread(int irq, void *data)
}
if (f_reg != bdi->f_reg) {
+ if ((bdi->f_reg & battery_mask_f) != (f_reg & battery_mask_f))
+ alert_battery = true;
+ if ((bdi->f_reg & ~battery_mask_f) != (f_reg & ~battery_mask_f))
+ 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 +1296,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 +1375,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; /* impossible state */
bdi->charger_health_valid = false;
bdi->battery_health_valid = false;
bdi->battery_status_valid = false;
@@ -1491,13 +1492,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; /* impossible state */
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 */
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v4 6/7] power: bq24190_charger: Don't read fault register outside irq_handle_thread()
2017-01-18 17:26 ` [PATCH v4 0/7] " Liam Breck
` (4 preceding siblings ...)
2017-01-18 17:26 ` [PATCH v4 5/7] power: bq24190_charger: Call power_supply_changed() for relevant component Liam Breck
@ 2017-01-18 17:26 ` Liam Breck
2017-01-18 17:26 ` [PATCH v4 7/7] power: bq24190_charger: Handle fault before status on interrupt Liam Breck
2017-01-20 11:51 ` [PATCH v4 0/7] BQ24190 charger fixes Sebastian Reichel
7 siblings, 0 replies; 13+ messages in thread
From: Liam Breck @ 2017-01-18 17:26 UTC (permalink / raw)
To: Sebastian Reichel; +Cc: linux-pm, Mark Greer, Tony Lindgren, Liam Breck
Caching the fault register after a single I2C read may not keep an accurate
value.
Fix by doing two reads in irq_handle_thread() and using the cached value
elsewhere. If a safety timer fault later clears itself, we apparently don't get
an interrupt (INT), however other interrupts would refresh the register cache.
>From the data sheet: "When a fault occurs, the charger device sends out INT
and keeps the fault state in REG09 until the host reads the fault register.
Before the host reads REG09 and all the faults are cleared, the charger
device would not send any INT upon new faults. In order to read the
current fault status, the host has to read REG09 two times consecutively.
The 1st reads fault register status from the last read [1] and the 2nd reads
the current fault register status."
[1] presumably a typo; should be "last fault"
Fixes: d7bf353fd0aa3 ("bq24190_charger: Add support for TI BQ24190 Battery Charger")
Signed-off-by: Liam Breck <kernel@networkimprov.net>
Acked-by: Mark Greer <mgreer@animalcreek.com>
Acked-by: Tony Lindgren <tony@atomide.com>
---
drivers/power/supply/bq24190_charger.c | 93 ++++++++++------------------------
1 file changed, 26 insertions(+), 67 deletions(-)
diff --git a/drivers/power/supply/bq24190_charger.c b/drivers/power/supply/bq24190_charger.c
index ba5a5b2..a36788c 100644
--- a/drivers/power/supply/bq24190_charger.c
+++ b/drivers/power/supply/bq24190_charger.c
@@ -144,10 +144,7 @@
* so the first read after a fault returns the latched value and subsequent
* reads return the current value. In order to return the fault status
* to the user, have the interrupt handler save the reg's value and retrieve
- * it in the appropriate health/status routine. Each routine has its own
- * flag indicating whether it should use the value stored by the last run
- * of the interrupt handler or do an actual reg read. That way each routine
- * can report back whatever fault may have occured.
+ * it in the appropriate health/status routine.
*/
struct bq24190_dev_info {
struct i2c_client *client;
@@ -159,9 +156,6 @@ struct bq24190_dev_info {
unsigned int gpio_int;
unsigned int irq;
struct mutex f_reg_lock;
- bool charger_health_valid;
- bool battery_health_valid;
- bool battery_status_valid;
u8 f_reg;
u8 ss_reg;
u8 watchdog;
@@ -635,21 +629,11 @@ static int bq24190_charger_get_health(struct bq24190_dev_info *bdi,
union power_supply_propval *val)
{
u8 v;
- int health, ret;
+ int health;
mutex_lock(&bdi->f_reg_lock);
-
- if (bdi->charger_health_valid) {
- v = bdi->f_reg;
- bdi->charger_health_valid = false;
- mutex_unlock(&bdi->f_reg_lock);
- } else {
- mutex_unlock(&bdi->f_reg_lock);
-
- ret = bq24190_read(bdi, BQ24190_REG_F, &v);
- if (ret < 0)
- return ret;
- }
+ v = bdi->f_reg;
+ mutex_unlock(&bdi->f_reg_lock);
if (v & BQ24190_REG_F_BOOST_FAULT_MASK) {
/*
@@ -936,18 +920,8 @@ static int bq24190_battery_get_status(struct bq24190_dev_info *bdi,
int status, ret;
mutex_lock(&bdi->f_reg_lock);
-
- if (bdi->battery_status_valid) {
- chrg_fault = bdi->f_reg;
- bdi->battery_status_valid = false;
- mutex_unlock(&bdi->f_reg_lock);
- } else {
- mutex_unlock(&bdi->f_reg_lock);
-
- ret = bq24190_read(bdi, BQ24190_REG_F, &chrg_fault);
- if (ret < 0)
- return ret;
- }
+ chrg_fault = bdi->f_reg;
+ mutex_unlock(&bdi->f_reg_lock);
chrg_fault &= BQ24190_REG_F_CHRG_FAULT_MASK;
chrg_fault >>= BQ24190_REG_F_CHRG_FAULT_SHIFT;
@@ -995,21 +969,11 @@ static int bq24190_battery_get_health(struct bq24190_dev_info *bdi,
union power_supply_propval *val)
{
u8 v;
- int health, ret;
+ int health;
mutex_lock(&bdi->f_reg_lock);
-
- if (bdi->battery_health_valid) {
- v = bdi->f_reg;
- bdi->battery_health_valid = false;
- mutex_unlock(&bdi->f_reg_lock);
- } else {
- mutex_unlock(&bdi->f_reg_lock);
-
- ret = bq24190_read(bdi, BQ24190_REG_F, &v);
- if (ret < 0)
- return ret;
- }
+ v = bdi->f_reg;
+ mutex_unlock(&bdi->f_reg_lock);
if (v & BQ24190_REG_F_BAT_FAULT_MASK) {
health = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
@@ -1201,7 +1165,7 @@ static irqreturn_t bq24190_irq_handler_thread(int irq, void *data)
| BQ24190_REG_F_NTC_FAULT_MASK;
bool alert_charger = false, alert_battery = false;
u8 ss_reg = 0, f_reg = 0;
- int ret;
+ int i, ret;
pm_runtime_get_sync(bdi->dev);
@@ -1231,33 +1195,35 @@ static irqreturn_t bq24190_irq_handler_thread(int irq, void *data)
alert_battery = true;
if ((bdi->ss_reg & ~battery_mask_ss) != (ss_reg & ~battery_mask_ss))
alert_charger = true;
-
bdi->ss_reg = ss_reg;
}
- mutex_lock(&bdi->f_reg_lock);
-
- ret = bq24190_read(bdi, BQ24190_REG_F, &f_reg);
- if (ret < 0) {
- mutex_unlock(&bdi->f_reg_lock);
- dev_err(bdi->dev, "Can't read F reg: %d\n", ret);
- goto out;
- }
+ i = 0;
+ do {
+ ret = bq24190_read(bdi, BQ24190_REG_F, &f_reg);
+ if (ret < 0) {
+ dev_err(bdi->dev, "Can't read F reg: %d\n", ret);
+ goto out;
+ }
+ } while (f_reg && ++i < 2);
if (f_reg != bdi->f_reg) {
+ dev_info(bdi->dev,
+ "Fault: boost %d, charge %d, battery %d, ntc %d\n",
+ !!(f_reg & BQ24190_REG_F_BOOST_FAULT_MASK),
+ !!(f_reg & BQ24190_REG_F_CHRG_FAULT_MASK),
+ !!(f_reg & BQ24190_REG_F_BAT_FAULT_MASK),
+ !!(f_reg & BQ24190_REG_F_NTC_FAULT_MASK));
+
+ mutex_lock(&bdi->f_reg_lock);
if ((bdi->f_reg & battery_mask_f) != (f_reg & battery_mask_f))
alert_battery = true;
if ((bdi->f_reg & ~battery_mask_f) != (f_reg & ~battery_mask_f))
alert_charger = true;
-
bdi->f_reg = f_reg;
- bdi->charger_health_valid = true;
- bdi->battery_health_valid = true;
- bdi->battery_status_valid = true;
+ mutex_unlock(&bdi->f_reg_lock);
}
- mutex_unlock(&bdi->f_reg_lock);
-
if (alert_charger)
power_supply_changed(bdi->charger);
if (alert_battery)
@@ -1377,9 +1343,6 @@ static int bq24190_probe(struct i2c_client *client,
mutex_init(&bdi->f_reg_lock);
bdi->f_reg = 0;
bdi->ss_reg = BQ24190_REG_SS_VBUS_STAT_MASK; /* impossible state */
- bdi->charger_health_valid = false;
- bdi->battery_health_valid = false;
- bdi->battery_status_valid = false;
i2c_set_clientdata(client, bdi);
@@ -1494,9 +1457,6 @@ static int bq24190_pm_resume(struct device *dev)
bdi->f_reg = 0;
bdi->ss_reg = BQ24190_REG_SS_VBUS_STAT_MASK; /* impossible state */
- 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);
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v4 7/7] power: bq24190_charger: Handle fault before status on interrupt
2017-01-18 17:26 ` [PATCH v4 0/7] " Liam Breck
` (5 preceding siblings ...)
2017-01-18 17:26 ` [PATCH v4 6/7] power: bq24190_charger: Don't read fault register outside irq_handle_thread() Liam Breck
@ 2017-01-18 17:26 ` Liam Breck
2017-01-20 11:51 ` [PATCH v4 0/7] BQ24190 charger fixes Sebastian Reichel
7 siblings, 0 replies; 13+ messages in thread
From: Liam Breck @ 2017-01-18 17:26 UTC (permalink / raw)
To: Sebastian Reichel; +Cc: linux-pm, Mark Greer, Tony Lindgren, Liam Breck
Reading both fault and status registers and logging any fault should
take priority over handling status register update.
Fix by moving the status handling to later in interrupt routine.
Fixes: d7bf353fd0aa3 ("bq24190_charger: Add support for TI BQ24190 Battery Charger")
Signed-off-by: Liam Breck <kernel@networkimprov.net>
Acked-by: Mark Greer <mgreer@animalcreek.com>
Acked-by: Tony Lindgren <tony@atomide.com>
---
drivers/power/supply/bq24190_charger.c | 46 +++++++++++++++++-----------------
1 file changed, 23 insertions(+), 23 deletions(-)
diff --git a/drivers/power/supply/bq24190_charger.c b/drivers/power/supply/bq24190_charger.c
index a36788c..1cb7d14 100644
--- a/drivers/power/supply/bq24190_charger.c
+++ b/drivers/power/supply/bq24190_charger.c
@@ -1175,29 +1175,6 @@ static irqreturn_t bq24190_irq_handler_thread(int irq, void *data)
goto out;
}
- if (ss_reg != bdi->ss_reg) {
- /*
- * The device is in host mode so when PG_STAT goes from 1->0
- * (i.e., power removed) HIZ needs to be disabled.
- */
- if ((bdi->ss_reg & BQ24190_REG_SS_PG_STAT_MASK) &&
- !(ss_reg & BQ24190_REG_SS_PG_STAT_MASK)) {
- ret = bq24190_write_mask(bdi, BQ24190_REG_ISC,
- BQ24190_REG_ISC_EN_HIZ_MASK,
- BQ24190_REG_ISC_EN_HIZ_SHIFT,
- 0);
- if (ret < 0)
- dev_err(bdi->dev, "Can't access ISC reg: %d\n",
- ret);
- }
-
- if ((bdi->ss_reg & battery_mask_ss) != (ss_reg & battery_mask_ss))
- alert_battery = true;
- if ((bdi->ss_reg & ~battery_mask_ss) != (ss_reg & ~battery_mask_ss))
- alert_charger = true;
- bdi->ss_reg = ss_reg;
- }
-
i = 0;
do {
ret = bq24190_read(bdi, BQ24190_REG_F, &f_reg);
@@ -1224,6 +1201,29 @@ static irqreturn_t bq24190_irq_handler_thread(int irq, void *data)
mutex_unlock(&bdi->f_reg_lock);
}
+ if (ss_reg != bdi->ss_reg) {
+ /*
+ * The device is in host mode so when PG_STAT goes from 1->0
+ * (i.e., power removed) HIZ needs to be disabled.
+ */
+ if ((bdi->ss_reg & BQ24190_REG_SS_PG_STAT_MASK) &&
+ !(ss_reg & BQ24190_REG_SS_PG_STAT_MASK)) {
+ ret = bq24190_write_mask(bdi, BQ24190_REG_ISC,
+ BQ24190_REG_ISC_EN_HIZ_MASK,
+ BQ24190_REG_ISC_EN_HIZ_SHIFT,
+ 0);
+ if (ret < 0)
+ dev_err(bdi->dev, "Can't access ISC reg: %d\n",
+ ret);
+ }
+
+ if ((bdi->ss_reg & battery_mask_ss) != (ss_reg & battery_mask_ss))
+ alert_battery = true;
+ if ((bdi->ss_reg & ~battery_mask_ss) != (ss_reg & ~battery_mask_ss))
+ alert_charger = true;
+ bdi->ss_reg = ss_reg;
+ }
+
if (alert_charger)
power_supply_changed(bdi->charger);
if (alert_battery)
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v4 0/7] BQ24190 charger fixes
2017-01-18 17:26 ` [PATCH v4 0/7] " Liam Breck
` (6 preceding siblings ...)
2017-01-18 17:26 ` [PATCH v4 7/7] power: bq24190_charger: Handle fault before status on interrupt Liam Breck
@ 2017-01-20 11:51 ` Sebastian Reichel
7 siblings, 0 replies; 13+ messages in thread
From: Sebastian Reichel @ 2017-01-20 11:51 UTC (permalink / raw)
To: Liam Breck; +Cc: linux-pm, Mark Greer, Tony Lindgren
[-- Attachment #1: Type: text/plain, Size: 690 bytes --]
Hi,
On Wed, Jan 18, 2017 at 09:26:47AM -0800, Liam Breck wrote:
> Hi Sebastian, thanks for your patience :-)
>
> These are misc bq24190_charger fixes, tested on a custom OMAP3 board.
>
> Changes in v4
>
> #4 factors out whitespace change in "Install irq_handler_thread() at end of probe()"
>
> Note: The other whitespace issue Mark raised is related to the purpose of that patch "Don't read
> fault register outside irq_handle_thread()"
I queued the patches fixing the subject to contain "supply: ".
Apart from that I dropped Liam's Acked-By from the whitespace
patch, as he never Acked those changes and dropped the Fixes
line from the same patch.
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread