[ltp] tp_smapi does not report status correctly when using battery charge control features

[Henrique de Moraes Holschuh]

On Mon, 06 Jul 2009, Darren Hoo wrote:
> >> I've got a thinkpad T60 and have battery charge control features enabled:
> >>    # echo 40 > /sys/devices/platform/smapi/BAT0/start_charge_thresh
> >>    # echo 100 > /sys/devices/platform/smapi/BAT0/stop_charge_thresh

So the battery will stop charging before it is full...

BTW, /sys/class/power_supply doesn't come from tp_smapi, it comes from ACPI.

Well, we all know just how beatifully complete the ACPI battery state
reporting is, don't we?  Detecting FULL is possible ONLY if you can trust
the battery reporting...

I think the kernel nowadays tries its best to do it properly, and any quirks
for broken platforms should be added there...  Please take a look on 2.6.29
or 2.6.30's drivers/acpi/battery.c to check what it does to try to give
userspace some decent state reports.

> >> line 131 of  src/gpm-devicekit.c says:
> >>                } else if (state == DKP_DEVICE_STATE_UNKNOWN && percentage > 95.0f) {

Well, that just plain can't work if the laptop stopped charging the battery
before it was full, like almost every thinkpad made in the last 10 years can
do (and it is not just thinkpads that can do it, either).

Yes, I know there are shitty laptops that have very broken battery status
reporting, but IMHO we should just cripple them instead of crippling the
support for platforms that do it right.

> > Well, the battery state is unknown, so g-p-m can't really do anything
> > sensible here. I think part of the problem is that ACPI exposes two
> > values, is_charging and is_discharging, and so it's tricky to map them
> > to a sane number of states.

AFAIK:

charge  discharge       state
  0        0            idle
  0        1            discharging
  1        0            charging
  1        1            invalid

If it is invalid, refuse to give battery readouts.  You may want to check
the table above, but that's what I recall from the ACPI and SBS
specifications.

> > What's the value of state in the correct device in
> > /sys/class/power_supply? If this is fixed in the kernel, it'll just
> > work in g-p-m. Just lowering the threshold is not a good idea at all,
> > and I'll probably remove that hack from g-p-m altogether.

Ah, good.  Thanks for cleaning up the mess.

We had a good thread in LKML (or linux-acpi? I forget) when fixing the
battery.c drivers.  You might want to hunt it down in the archives,
gmane.org probably has it.

> So the status is unknown.The kernel module I use is tp_smapi which is
> not in the  mainstream kernel yet .

And it will _never_ be.  But that doesn't matter much, you're getting stuff
from the generic ACPI battery module, as I said ;-)

If you want to use tp_smapi's battery readout instead of ACPI's, you have to
check for /sys/bus/platform/devices/smapi/* and use stuff from there when it
is present, instead of using the power_supply class.

If you're going to support tp_smapi, please remember that reading/writing
values through SMAPI is *expensive*, so don't do it for frivoulous reasons
(i.e. there is no point in reading battery state at more than 0.5Hz, don't
reprogram the battery charge thresholds at every step in a slider but rather
give the user a "apply" button, etc).   In fact, that holds true for ACPI as
well.

Maybe someday tp_smapi will be ported to the power_supply class, but that
has its own problems as well (one of which is the fact that you will get two
power_supply instances for everything in a thinkpad...)

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh