Re: [RFC PATCH] thinkpad_acpi: Add support for controlling charge thresholds

[Julian Andres Klode <jak@jak-linux.org>]

On Tue, Nov 05, 2013 at 11:18:02AM +0100, Bjørn Mork wrote:
> Julian Andres Klode <jak@jak-linux.org> writes:
> 
> >  
> > +TPACPI_HANDLE(battery, root, "\\_SB.PCI0.LPC.EC.HKEY",
> > +	   "\\_SB.PCI0.LPCB.EC.HKEY",		/* X121e, T430u */
> > +	   "\\_SB.PCI0.LPCB.H_EC.HKEY",		/* L430 */
> > +	   "\\_SB.PCI0.LPCB.EC0.HKEY",		/* Edge/S series */
> > +	   );
> > +
> 
> Isn't this just the full patch to the existing "hkey_handle" for those
> models?  Why not just use that handle, like e.g the rfkill driver does?

I did not notice it, thanks for pointing that out.

> 
> Supported models could probably be autodetected by checking whether the
> methods exist?

Yes, this makes more sense. I modified it locally to check for existence
of BCTG (get start threshold) and set tp_features.battery accordingly. If
it exists, all features are enabled (I think we can safely assume their
existence, I don't know if there are really thinkpads where you can get
a start threshold but don't have one of the other functions).


> 
> > +static struct attribute_group bat##_BAT##_attribute_group = { \
> > +	.name  = "BAT" #_BAT, \
> > +	.attrs = bat##_BAT##_attributes \
> > +};
> 
> Are these names guaranteed to match the ACPI battery device(s)?

At least on the Sandy Bridge series and older, the first battery
(BAT0 here) always refers to the internal battery, and BAT1 to
the external one. I think this should match the ACPI battery
devices. 

On the Haswell ones, I don't know, because they have one non-removable
built-in and one removable.

> 
> > +DEFINE_BATTERY(0);
> > +DEFINE_BATTERY(1);
> 
> Are there always two batteries?

As far as I can tell, the controller supports up to 2 batteries. And
they can be configured while they are not plugged in. So, exporting
both of them (all the time) makes sense.

I don't know if the W520 or W530 support 3 batteries, as I don't
have access to them. If they do, I don't know whether they will be
two separate entries or controlled by the same one.

-- 
Julian Andres Klode  - Debian Developer, Ubuntu Member

See http://wiki.debian.org/JulianAndresKlode and http://jak-linux.org/.

Please do not top-post if possible.