Bug 9494

Could you please append acpidump and .config?
also, please do a 'ls /proc/acpi/battery/*' and /sys/class/power*/

donald:~ # ls /proc/acpi/battery/*
alarm  info  state
donald:~ # ls /sys/class/power*/
BAT1
donald:~ # ls /sys/class/power_supply/BAT1/
charge_full         charge_now   device        model_name  present  subsystem   type    voltage_min_design
charge_full_design  current_now  manufacturer  power       status   technology  uevent  voltage_now

Created attachment 13840 [details]
acpidump

Created attachment 13841 [details]
config

Rolf,
could you please check the latest git tree? 2.6.24-rc4 is good starting point.
Please try to set CONFIG_ACPI_PROCFS_POWER to No, if it does not work.

CONFIG_ACPI_PROCFS_POWER=n solves it

Danny works on it. But yes, this *is* user space breakage after all. Personally I think we need conditional compiling of power_supply and automatically disable procfs interface if power_supply is compiled in.

It would be interesting to see what does not work as expected in sysfs code.

On Samstag, 27. Oktober 2007, Danny Kukawka wrote:
> On Samstag, 27. Oktober 2007, Andrey Borzenkov wrote:
> > In kernel 2.6.24-rc1 if I turn on ACPI_PROCFS number of batteries in HAL
> > is doubled; it gets them once via sysfs and once via /proc. Because there
> > may be legal reasons to enable ACPI_PROCFS for compatibility and because
> > there is no way to disable sysfs entries it would be nice if HAL could
> > detect those duplicates. What I am not exactly sure, how. After all we
> > may have non-ACPI batteries in sysfs and ACPI ones in procfs ...
> >
> > -andrey
>
> Okay, I take a look at this. Could you send me your lshal output directly?

Update: found the problems with 2.6.24 and wrote a first patch to fix it, but 
this need much more changes in the acpi addon and on several other places, 
since the sysfs battery/AC code don't work as expected atm. I send a patch as 
soon as it work.

Andrey,
all entries in /sys/class/power_supply have parent, which would be some acpi_device in case you mention.

It looks like charge_now in sysfs does not decrease when laptop is on battery. At least I doubt it will stay on the same value for half an hour when I was using the laptop. After plugging power in it decreased by a big amount (one third or something like that).

Created attachment 13942 [details]
Update state on access

Please check if this patch helps.

Does not change anything

Created attachment 13970 [details]
Update state on access #2

Please check this one...

I think it's the completely wrong place to fix as I don't have SBS enabled at all.

right... I was confused by your bisect log, and the fact that rc4 has patch for CM battery to update on read in sysfs... 

Rolf, could you add printk to acpi_battery_get_state() near the evaluate_object and check that you see it in dmesg after read of current from /sys?

evaluate_object() returns 0. I see the message only when I cat that file by hand so hal is not looking at it at all. That means that it became pretty useless now. hal can't even detect if I remove the battery now.

so hal looks at sysfs battery to enumerate it and then ignores it completely?

seems to be that way.

It looks into charge_now once every time you plug or unplug the AC connector.

this means hal looks at it only then gets notify from power_supply...

(In reply to comment #20)
> this means hal looks at it only then gets notify from power_supply...
> 

that's correct; when using power_supply HAL reacts only on explicit KOBJ_{ADD,REMOVE,CHANGE} events. I do not have charge_now at all so I cannot test.

Created attachment 14131 [details]
Make sysfs interface to ACPI power devices optional

Please check if this patch (and disabling ACPI_SYSFS_POWER) helps...

Hi. I also experience this problem, although with gnome-power-manager instead of kpowersave

I just applied this last patch to 2.6.24-rc5-gda8cadb3 and it partially helps.

I don't have the two batteries anymore, only one is detected. The problem is that now /proc/acpi/battery/BAT0/info looks like this:

present:                 yes
design capacity:         0 mWh
last full capacity:      0 mWh
battery technology:      non-rechargeable
design voltage:          0 mV
design capacity warning: 0 mWh
design capacity low:     0 mWh
capacity granularity 1:  0 mWh
capacity granularity 2:  0 mWh
model number:            
serial number:           
battery type:            
OEM info: 

whilst /proc/acpi/battery/BAT0/state looks fine.

/sys/class/power_supply is now empty (as expected, I think)

Please let me know if you need more details.

Same here. This is probably just HAL getting confused by what the kernel prints out.

Oh, by the way, the "state" file is just fine:

# cat /proc/acpi/battery/BAT1/state
present:                 yes
capacity state:          ok
charging state:          discharging
present rate:            0 mW
remaining capacity:      3526 mWh
present voltage:         11100 mV

Created attachment 14137 [details]
Make sysfs interface to ACPI power devices optional #2

Please check this patch

Yes, that one helps.

Confirmed. This patch fixes the issue for me too. Many thanks.

Is this going to be pushed for .24-final?

Hope so.
Adding Len to be sure.

bugme-daemon@bugzilla.kernel.org wrote:
> http://bugzilla.kernel.org/show_bug.cgi?id=9494
> ------- Comment #29 from fabio.comolli@gmail.com  2007-12-24 03:18 -------
> Is this going to be pushed for .24-final?
> 
> 

patch in comment #26 added to acpi test branch

it is unfortunate to require yet another kernel build option
it isn't clear from the Kconfig test when to say Y and when N
(has the "Future" arrived yet?)
it is also unfortunate to need all those ifdefs in the driver --
will we ever be able to get rid of them?

With ACPI_PROCFS being deprecated we could remove all these ifdefs with all other ACPI procfs entries.

patch in comment #26
shipped in linux-2.6.24-rc7-git5

closed.

It is not clear how it fixes this regression - whoever updates to 2.6.24 still gets both old and new interfaces.

Is it possible to express XOR in Kconfig? Either PROCFS_POWER or SYSFS_POWER?

Having battery exported by both interfaces does not brake every setup, so there is no need to have them exclude each other.