Hi,

On Thu, Mar 23, 2017 at 03:24:15PM -0700, Liam Breck wrote:

On Thu, Mar 23, 2017 at 3:02 PM, Hans de Goede [off-list ref] wrote:

quoted

On 23-03-17 22:31, Liam Breck wrote:

quoted

On Wed, Mar 22, 2017 at 7:55 AM, Hans de Goede [off-list ref]
wrote:

quoted

If the charger gets unplugged before the battery is fully charged we will
get a one time Input fault. Ignore this rather then logging a message for
it. Likewise on the next interrupt after the one time Input fault all
fault flags will be 0, do not log a message when there are no faults.

This fixes messages like these getting logged on charger unplug + replug:
bq24190-charger 15-006b: Fault: boost 0, charge 1, battery 0, ntc 0
bq24190-charger 15-006b: Fault: boost 0, charge 0, battery 0, ntc 0

Cc: Liam Breck <redacted>
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Hans de Goede <redacted>
---
Changes in v2:
-This is a new patch in v2 of this patch-set
---
 drivers/power/supply/bq24190_charger.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/power/supply/bq24190_charger.c

b/drivers/power/supply/bq24190_charger.c
index b535f24..351e020 100644

--- a/drivers/power/supply/bq24190_charger.c
+++ b/drivers/power/supply/bq24190_charger.c

@@ -1189,12 +1189,17 @@ static void bq24190_check_status(struct

bq24190_dev_info *bdi)
        } 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));
+               /*
+                * Don't spam the logs if all faults are cleared, or when
the
+                * cable providing Vbus gets unplugged.
+                */
+               if  (f_reg && f_reg != (1 <<
BQ24190_REG_F_CHRG_FAULT_SHIFT))

if (f_reg && ((ss_reg & BQ24190_REG_SS_PG_STAT_MASK) ||
    f_reg != (1 << BQ24190_REG_F_CHRG_FAULT_SHIFT)) )

Sebastian has already merged the original patch into his for-next branch,
please
provide a patch on top.

I recall Sebastian merging and then dropping a commit from the other
series I'm working on.

Yeah, I thought it was a fix, but it was useless without the
remaining series.

I'll return to the charger once the dependency for my charger patchset
is merged. I just got a big change request for that.

Ok.

quoted

quoted

quoted

+                       dev_info(bdi->dev,

This should probably be dev_warn now.

makes sense to me.

FWIW I think a follow-up commit for this is enough, but I can also
swap the existing commit.

-- Sebastian

quoted

quoted

quoted

+                               "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))
--
2.9.3