| Date | Fri, 28 Sep 2012 12:24:53 -0600 | From | Mathieu Poirier <> | Subject | Re: [PATCH 17/57] power: ab8500_bm: Added support for BATT_OVV |
| |
On 12-09-26 09:36 PM, Anton Vorontsov wrote: > On Tue, Sep 25, 2012 at 10:12:14AM -0600, mathieu.poirier@linaro.org wrote: >> From: Hakan Berg <hakan.berg@stericsson.com> >> >> Add support for the battery over-voltage situation >> >> Signed-off-by: Hakan Berg <hakan.berg@stericsson.com> >> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org> >> Reviewed-by: Karl KOMIEROWSKI <karl.komierowski@stericsson.com> >> --- >> drivers/power/ab8500_fg.c | 32 ++++++++++++++++---------------- >> 1 files changed, 16 insertions(+), 16 deletions(-) >> >> diff --git a/drivers/power/ab8500_fg.c b/drivers/power/ab8500_fg.c >> index c4d9307..8507254 100644 >> --- a/drivers/power/ab8500_fg.c >> +++ b/drivers/power/ab8500_fg.c >> @@ -1842,24 +1842,26 @@ static void ab8500_fg_check_hw_failure_work(struct work_struct *work) >> * If we have had a battery over-voltage situation, >> * check ovv-bit to see if it should be reset. >> */ >> - if (di->flags.bat_ovv) { >> - ret = abx500_get_register_interruptible(di->dev, >> - AB8500_CHARGER, AB8500_CH_STAT_REG, >> - ®_value); >> - if (ret < 0) { >> - dev_err(di->dev, "%s ab8500 read failed\n", __func__); >> - return; >> - } >> - if ((reg_value & BATT_OVV) != BATT_OVV) { >> - dev_dbg(di->dev, "Battery recovered from OVV\n"); >> - di->flags.bat_ovv = false; >> + ret = abx500_get_register_interruptible(di->dev, >> + AB8500_CHARGER, AB8500_CH_STAT_REG, >> + ®_value); >> + if (ret < 0) { >> + dev_err(di->dev, "%s ab8500 read failed\n", __func__); >> + return; >> + } >> + if ((reg_value & BATT_OVV) == BATT_OVV) { >> + if (!di->flags.bat_ovv) { >> + dev_dbg(di->dev, "Battery OVV\n"); >> + di->flags.bat_ovv = true; >> power_supply_changed(&di->fg_psy); >> - return; >> } >> - >> /* Not yet recovered from ovv, reschedule this test */ >> queue_delayed_work(di->fg_wq, &di->fg_check_hw_failure_work, >> - round_jiffies(HZ)); >> + HZ); > > Why this change? I.e. round_jiffies(HZ) -> HZ? > > Yes, it seems like round_jiffies(HZ) is not needed since HZ itself is a > full second.. But the change itself does not belong to this patch.
I agree with your point of view. How do we fix it now ? Do you think it's worth crafting a one-line patch ?
> >> + } else { >> + dev_dbg(di->dev, "Battery recovered from OVV\n"); >> + di->flags.bat_ovv = false; >> + power_supply_changed(&di->fg_psy); >> } >> } >> >> @@ -2039,8 +2041,6 @@ static irqreturn_t ab8500_fg_batt_ovv_handler(int irq, void *_di) >> struct ab8500_fg *di = _di; >> >> dev_dbg(di->dev, "Battery OVV\n"); >> - di->flags.bat_ovv = true; >> - power_supply_changed(&di->fg_psy); >> >> /* Schedule a new HW failure check */ >> queue_delayed_work(di->fg_wq, &di->fg_check_hw_failure_work, 0); >> -- >> 1.7.5.4
|