apm.c and multiple battery slots

apm.c and multiple battery slots

[Stevie O]

I have an Inspiron 8100 and 2.4.13-ac5, with apmd-final. (I'm holding off 
upgrading until I stop seeing so much stuff about VM problems on lkml). It 
has two battery slots, left and right.

If I put the battery in the left slot and run 'apm' I get

AC on-line, battery charging: 84% (4:12)

If I take out the battery and run apm I get

AC on-line, no system battery

If I put it into the right slot I get

AC on-line

and /proc/apm says I have -1% battery free and -1 minutes remaining ;)

--

The battery was designed to go into the right slot, not the left (left is 
where the floppy drive goes).

--

I went to apm.c to look into patching it to support multiple batteries.

I found this function:
static int apm_get_battery_status(which, status, bat, life, nbat <- battery #)

but it's #if 0'd out, and isn't referred to anywhere in the code. I looked 
at the changelog in the file to try to determine when it stopped being 
used, and why, but I found no useful information, and I can't even ask the 
person who did it, since they didn't tell me they did...


--
Stevie-O

Real programmers use COPY CON PROGRAM.EXE

Re: apm.c and multiple battery slots

[Thomas Hood]

> I went to apm.c to look into patching it to support
> multiple batteries.
> I found this function:
>   static int apm_get_battery_status(which, status, bat,
>                                     life, nbat <- battery #)
> but it's #if 0'd out, and isn't referred to anywhere in the code.
> I looked at the changelog in the file to try to determine when
> it stopped being used, and why, but I found no useful information,
> and I can't even ask the person who did it, since they didn't
> tell me they did...

I am not the author, so the following is speculation.
My guess is that apm_get_battery_status() was written to
support multiple batteries (supported by APM 1.2 only) but
that the authors never got around to providing a user
interface to this functionality; so it remains ifdeffed out.
(Hence the function never "stopped" being used.)

The current official maintainer of the driver is Stephen Rothwell.

Stephen: How do you think the info about the second battery
might be furnished to the user?

--
Thomas

Re: apm.c and multiple battery slots

[Stevie O]

At 08:30 AM 2/2/2002 -0500, you wrote:
>I am not the author, so the following is speculation.
>My guess is that apm_get_battery_status() was written to
>support multiple batteries (supported by APM 1.2 only) but
>that the authors never got around to providing a user
>interface to this functionality; so it remains ifdeffed out.
>(Hence the function never "stopped" being used.)
>
>The current official maintainer of the driver is Stephen Rothwell.
>
>Stephen: How do you think the info about the second battery
>might be furnished to the user?

I already thought about that a bit... what if we changed /proc/apm again, 
adding one line for each possible battery slot?

The current format is:

<driver version> <BIOS version> <APM flags> <AC status> <battery status> 
<battery flag> <battery left (%)> <remaining time> <remaining time units 
(min/sec)>

I suggest we change the first line to reflect an overall battery status 
(i.e. average of all slots in system). Then we could add one line for each 
battery slot, indicating
<battery status> <battery flag> <battery left % > <remaining time in seconds>

I suggest we always show the new stuff in seconds because I like easily 
parsed things, and even shell scripts can divide by 60 if they want to ;)

If there's no battery in this slot, we can show the usual of -1's, or just 
put "? ? ? ?".

So on my system, I would see this if I did

$ cat /proc/apm
1.16 1.2 0x03 0x01 0x03 0x09 94% 283 min
0x04 0x80 -1 -1
0x03 0x09 94 16980

Whoever manages apmd also probably has some suggestions...

(btw, "Stephen" is a really great name ;)


--
Stevie-O

Real programmers use COPY CON VMLINUZ then run LoadLin

Re: apm.c and multiple battery slots

[Thomas Hood]

Stevie O <stevie@qrpff.net> wrote:
> I suggest we change the first line to reflect an
> overall battery status (i.e. average of all slots
> in system).

Sounds like a good idea.

> Then we could add one line for each battery slot,
> indicating <battery status> <battery flag> <battery left % >
> <remaining time in seconds>

How about putting each of these lines in a separate proc
file?  This would avoid changing the format of /proc/apm,
which would break things.

--
Thomas

Re: apm.c and multiple battery slots

[Stevie O]

At 09:58 PM 2/2/2002 -0500, Thomas Hood wrote:
>Stevie O <stevie@qrpff.net> wrote:
> > I suggest we change the first line to reflect an
> > overall battery status (i.e. average of all slots
> > in system).
>
>Sounds like a good idea.
>
> > Then we could add one line for each battery slot,
> > indicating <battery status> <battery flag> <battery left % >
> > <remaining time in seconds>
>
>How about putting each of these lines in a separate proc
>file?  This would avoid changing the format of /proc/apm,
>which would break things.

Or we could do something similar to what the IDE subsystem does, and have

/proc/apmbat/0 <- battery [slot] 0
/proc/apmbat/1 <- battery [slot] 1
...
/proc/apmbat/  <- battery [slot] N

I'd actually prefer to call it /proc/apm/ but obviously that won't work :)





--
Stevie-O

Real programmers use COPY CON PROGRAM.EXE

Re: apm.c and multiple battery slots

[David Woodhouse]

jdthood@mail.com said:
> > Then we could add one line for each battery slot,
> > indicating <battery status> <battery flag> <battery left % >
> > <remaining time in seconds>

> How about putting each of these lines in a separate proc file?  This
> would avoid changing the format of /proc/apm, which would break
> things.

Please don't add an APM-specific interface for batteries. Look at the other
code which talks to batteries - the ACPI code, SMBus and numerous other
drivers for embedded systems - and design an interface to userspace which
can be used by all of them.

Preferably not in /proc.

--
dwmw2

APM and APIC -- multiple batteries (was: apm.c and multiple battery slots)

[Stevie O]

At 12:01 PM 2/3/2002 +0000, David Woodhouse wrote:
>Please don't add an APM-specific interface for batteries. Look at the other
>code which talks to batteries - the ACPI code, SMBus and numerous other
>drivers for embedded systems - and design an interface to userspace which
>can be used by all of them.
>
>Preferably not in /proc.

Well, the only other interface I know of is device files, and from what 
I've seen, that seems to be a place where we autoload drivers when needed...

Besides, pretty much everything else in /proc is for transmitting (normally 
unchangeable, except for the sysctls) information from kernel to userspace. 
Why not /proc?

--

Now that you mention it, yes, I agree that an APM-specific interface for 
batteries would make code reuse harder (needing stuff for both APM and ACPI 
stuff). So we could have /proc/batteries/ maybe?

How does the ACPI stuff handle this? *Does* the ACPI stuff handle this 
(i.e. multiple batteries)?
If so:
   Is it a generic interface?
     If so, we should let APM use it too.
     If not, could we perhaps change it to one?
If not:
   I'm ready for suggestions about my original idea :)


--
Stevie-O

Real programmers use COPY CON PROGRAM.EXE

Re: APM and APIC -- multiple batteries (was: apm.c and multiple battery slots)

[Pavel Machek]

Hi!

> How does the ACPI stuff handle this? *Does* the ACPI stuff handle this 
> (i.e. multiple batteries)?

Yes.

> If so:
>    Is it a generic interface?
>      If so, we should let APM use it too.

Should be generic enough...
								Pavel
-- 
Philips Velo 1: 1"x4"x8", 300gram, 60, 12MB, 40bogomips, linux, mutt,
details at http://atrey.karlin.mff.cuni.cz/~pavel/velo/index.html.