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

[Pavel Machek <pavel@ucw.cz>]

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

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.

As in "two casts in one expression -- too ugly to live".

> > > @@ -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?

"PSU"? This is cellphone. It has USB connection and charges from that.

It has been charging for long while now, and current_now fluctuates
between 20706 and -2856. USB has limitted current, so I guess "draw
current from battery if we need more than USB can provide" is quite common.

pavel@n900:~$ cat /sys/class/power_supply/bq27200-0/current_now
5355
pavel@n900:~$ cat /sys/class/power_supply/bq27200-0/current_now
5355
pavel@n900:~$ cat /sys/class/power_supply/bq27200-0/current_now
-4105
pavel@n900:~$ cat /sys/class/power_supply/bq27200-0/current_now
-4105
pavel@n900:~$ cat /sys/class/power_supply/bq27200-0/current_now
-7675
pavel@n900:~$ cat /sys/class/power_supply/bq27200-0/current_now
-5712
pavel@n900:~$ #screen on
pavel@n900:~$ cat /sys/class/power_supply/bq27200-0/current_now
4641
pavel@n900:~$ cat /sys/class/power_supply/bq27200-0/current_now
4641
pavel@n900:~$ cat /sys/class/power_supply/bq27200-0/current_now
37842
pavel@n900:~$ cat /sys/class/power_supply/bq27200-0/current_now
16600
pavel@n900:~$

> 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.

I'd pass to userspace what the controller reports. Yes, I seldom see
"STATUS_FULL" but that may be a problem we need to track down.



       	  	 	     		 	      	    	    	 Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]