Re: [PATCH 5/5] power: supply: bq27xxx: Correct supply status with current draw

From: Paul Kocialkowski <contact@paulk.fr>
To: Pavel Machek <pavel@ucw.cz>
Cc: linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
	"Pali Rohár" <pali.rohar@gmail.com>,
	"Andrew F . Davis" <afd@ti.com>,
	"Sebastian Reichel" <sre@kernel.org>,
	"Chris Lapa" <chris@lapa.com.au>,
	"Matt Ranostay" <mranostay@gmail.com>
Subject: Re: [PATCH 5/5] power: supply: bq27xxx: Correct supply status with current draw
Date: Wed, 31 May 2017 18:55:19 +0200	[thread overview]
Message-ID: <1496249719.1774.1.camel@paulk.fr> (raw)
In-Reply-To: <20170528191619.GA20159@xo-6d-61-c0.localdomain>

[-- Attachment #1: Type: text/plain, Size: 2138 bytes --]

Hi,

Le dimanche 28 mai 2017 à 21:16 +0200, Pavel Machek a écrit :
> Hi!
> 
> > The status reported directly by the battery controller is not always
> > reliable and should be corrected based on the current draw information.
> > 
> > This implements such a correction with a dedicated function, called
> > when retrieving the supply status.
> > 
> > @@ -1182,6 +1196,8 @@ static int bq27xxx_battery_status(struct
> > bq27xxx_device_info *di,
> >  		else
> >  			status = POWER_SUPPLY_STATUS_DISCHARGING;
> >  	} else {
> > +		curr = (int)((s16)curr) * 1000;
> 
> Umm.
> 
> > @@ -1190,6 +1206,18 @@ static int bq27xxx_battery_status(struct
> > bq27xxx_device_info *di,
> >  			status = POWER_SUPPLY_STATUS_CHARGING;
> >  	}
> >  
> > +
> > +	if (curr == 0 && status != POWER_SUPPLY_STATUS_NOT_CHARGING)
> > +		status = POWER_SUPPLY_STATUS_FULL;
> > +
> > +	if (status == POWER_SUPPLY_STATUS_FULL) {
> > +		/* Drawing or providing current when full */
> > +		if (curr > 0)
> > +			status = POWER_SUPPLY_STATUS_CHARGING;
> > +		else if (curr < 0)
> > +			status = POWER_SUPPLY_STATUS_DISCHARGING;
> > +	}
> 
> Are you sure this works? On N900, we normally see small currents to/from
> "full" battery.

In my case, this works perfectly and I am quite surprised of what you're
describing. Is it the case when the battery has a PSU connected?

I guess I would consider this a hardware issue (leak currents) and we could
definitely set some range (in device-tree) to distinguish between full + leak
currents and bad reporting from the fuel gauge. That would work well in my case
too.

> Should the test be for absolute_value(curr) < something rather than for == 0?
> 
> What hw did you test it on?

I tested this on nyan Chromebooks (Acer Chromebook 13 and HP Chromebook 11) as
well as veyron Chromebooks (Chromebook C201PA) that use a bq27xxx fuel gauge.

Cheers!

-- 
Paul Kocialkowski, developer of free digital technology and hardware support

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]