Le mercredi 31 mai 2017 à 21:28 +0200, Paul Kocialkowski a écrit :

Hi,

Le mercredi 31 mai 2017 à 19:32 +0200, Pavel Machek a écrit :

quoted

The status reported directly by the battery controller is not always

quoted

quoted

quoted

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

Oh, I had skipped that comment, sorry about that. Yeah I understand your
concern. However, this line was mostly inspired by another part of the code,
below the following comment: /* Other gauges return signed value */

I think we should fix the first occurence first and then used the fixed syntax
in v2 of this patch. What do you think?

quoted

quoted

quoted

quoted

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

Ah right, I had forgotten about the USB current limitation thing. In this
case,
I guess the battery is never actually full and IMO, it should be reported as
such.

quoted

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:~$

quoted

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.

The controller is known, from my experience, to not be reliable in that
regard,
so I don't think it makes sense to pass a state that doesn't reflect the
actual
state of charging just because the chip tells us so.

Worst case, we could also have a dt property to enable that kind of fixup
workaround and let every device maintainer decide whether it is relevant for
their device.

Actually, since a similar fix[0] was accepted in sbs-battery, I'd rather not
make this optional but rather make it the default and perhaps have a dt prop to
disable it.

What do you think?

[0]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?
h=v4.12-rc4&id=7f93e1fa032bb5ee19b868b9649bc98c82553003
-- 
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/