[2.624-rc1 regression] lost battery information

[2.624-rc1 regression] lost battery information

[Andrey Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 6694 bytes --]

I have lost battery in 2.6.24-rc1. Without CONFIG_ACPI_PROCFS I have 
no /proc/acpi/battery and cannot test netlink interface because right now 
there is no consumer of this.

With CONFIG_ACPI_PROCFS I get

{pts/1}% LC_ALL=C ll /proc/acpi/battery/BAT1
total 0
-rw-r--r-- 1 root root 0 Oct 26 20:18 alarm
-r--r--r-- 1 root root 0 Oct 26 20:18 info
-r--r--r-- 1 root root 0 Oct 26 20:18 state
{pts/1}% LC_ALL=C  cat /proc/acpi/battery/BAT1/*
cat: /proc/acpi/battery/BAT1/alarm: Bad address
cat: /proc/acpi/battery/BAT1/info: Bad address
cat: /proc/acpi/battery/BAT1/state: Bad address
{pts/1}% LC_ALL=C ll /proc/acpi/battery/BAT2
total 0
-rw-r--r-- 1 root root 0 Oct 26 20:18 alarm
-r--r--r-- 1 root root 0 Oct 26 20:18 info
-r--r--r-- 1 root root 0 Oct 26 20:18 state
{pts/1}% LC_ALL=C cat /proc/acpi/battery/BAT2/*
present:                 no
present:                 no
present:                 no

BAT2 is correct - it is not present. BAT1 is lying. There is nothing in dmesg. 
battery is loaded (obviously)

ACPI related stuff from dmesg:

{pts/1}% dmesg |grep ACPI
[    0.000000]  BIOS-e820: 00000000000eee00 - 00000000000ef000 (ACPI NVS)
[    0.000000]  BIOS-e820: 000000001ef60000 - 000000001ef70000 (ACPI data)
[    0.000000] ACPI: RSDP 000F0090, 0014 (r0 TOSHIB)
[    0.000000] ACPI: RSDT 1EF60000, 0028 (r1 TOSHIB 750        970814 TASM  
4010000)
[    0.000000] ACPI: FACP 1EF60054, 0084 (r2 TOSHIB 750        970814 TASM  
4010000)
[    0.000000] ACPI: DSDT 1EF600D8, 68DA (r1 TOSHIB 4000     20020417 MSFT  
100000A)
[    0.000000] ACPI: FACS 000EEE00, 0040
[    0.000000] ACPI: PM-Timer IO Port: 0xee08
[  896.112009] ACPI: Core revision 20070126
[  896.112775] ACPI: Looking for DSDT in initramfs... error, file /DSDT.aml 
not found.
[  896.123590]  tbxface-0598 [00] tb_load_namespace     : ACPI Tables 
successfully acquired
[  896.123631] ACPI: setting ELCR to 0200 (from 0a00)
[  896.124208] evxfevnt-0091 [00] enable                : Transition to ACPI 
mode successful
[  896.131744] ACPI: bus type pci registered
[  896.149165] ACPI: EC: Look up EC in DSDT
[  896.163343] ACPI: Interpreter enabled
[  896.163362] ACPI: (supports S0 S3 S4 S5)
[  896.163510] ACPI: Using PIC for interrupt routing
[  896.195892] ACPI: PCI Root Bridge [PCI0] (0000:00)
[  896.197650] PCI quirk: region ee00-ee3f claimed by ali7101 ACPI
[  896.200015] ACPI: PCI Interrupt Routing Table [\_SB_.PCI0._PRT]
[  896.200588] ACPI: PCI Interrupt Routing Table [\_SB_.PCI0.PCI1._PRT]
[  896.227797] ACPI: PCI Interrupt Link [LNKA] (IRQs 3 4 5 6 7 10 *11)
[  896.228562] ACPI: PCI Interrupt Link [LNKB] (IRQs 3 4 5 6 7 10 *11)
[  896.229271] ACPI: PCI Interrupt Link [LNKC] (IRQs 3 4 5 6 7 10 *11)
[  896.230101] ACPI: PCI Interrupt Link [LNKD] (IRQs 3 4 5 6 7 10 *11)
[  896.230818] ACPI: PCI Interrupt Link [LNKG] (IRQs 3 4 5 6 7 10 *11)
[  896.231527] ACPI: PCI Interrupt Link [LNKH] (IRQs 3 4 5 6 7 10 *11)
[  896.232912] ACPI: Power Resource [PFAN] (off)
[  896.233622] pnp: PnP ACPI init
[  896.233766] ACPI: bus type pnp registered
[  896.257679] pnp: PnP ACPI: found 12 devices
[  896.257737] ACPI: ACPI bus type pnp unregistered
[  896.258820] PCI: Using ACPI for IRQ routing
[  896.325763] ACPI: PCI Interrupt Link [LNKC] enabled at IRQ 11
[  896.325805] ACPI: PCI Interrupt 0000:00:10.0[A] -> Link [LNKC] -> GSI 11 
(level, low) -> IRQ 11
[  896.327116] ACPI: PCI Interrupt Link [LNKA] enabled at IRQ 11
[  896.327143] ACPI: PCI Interrupt 0000:00:11.0[A] -> Link [LNKA] -> GSI 11 
(level, low) -> IRQ 11
[  896.328392] ACPI: PCI Interrupt Link [LNKB] enabled at IRQ 11
[  896.328416] ACPI: PCI Interrupt 0000:00:11.1[B] -> Link [LNKB] -> GSI 11 
(level, low) -> IRQ 11
[  896.978962] ACPI: PCI Interrupt Link [LNKD] enabled at IRQ 11
[  896.980097] ACPI: PCI Interrupt 0000:00:0a.0[A] -> Link [LNKD] -> GSI 11 
(level, low) -> IRQ 11
[  902.378588] ACPI: Unable to derive IRQ for device 0000:00:04.0
[  902.406719] ACPI: PCI Interrupt 0000:00:04.0[A]: no GSI
[  919.051426] ACPI: PCI Interrupt Link [LNKG] enabled at IRQ 11
[  919.051451] ACPI: PCI Interrupt 0000:00:02.0[A] -> Link [LNKG] -> GSI 11 
(level, low) -> IRQ 11
[  920.132284] ACPI: PCI Interrupt Link [LNKH] enabled at IRQ 11
[  920.132307] ACPI: PCI Interrupt 0000:00:06.0[A] -> Link [LNKH] -> GSI 11 
(level, low) -> IRQ 11
[  927.120073] ACPI: AC Adapter [ADP1] (on-line)
[  927.195942] ACPI: Battery Slot [BAT1] (battery present)
[  927.200475] ACPI: Battery Slot [BAT2] (battery absent)
[  927.277564] ACPI: Power Button (FF) [PWRF]
[  927.290786] ACPI: Lid Switch [LID]
[  927.324850] ACPI: Transitioning device [FAN] to D3
[  927.324867] ACPI: Transitioning device [FAN] to D3
[  927.324891] ACPI: Fan [FAN] (off)
[  927.535960] ACPI: CPU0 (power states: C1[C1] C2[C2])
[  927.638487] ACPI: Thermal Zone [THRM] (55 C)
[  927.770100] toshiba_acpi: Toshiba Laptop ACPI Extras version 0.18
[  927.920519] ACPI: Video Device [VGA] (multi-head: yes  rom: yes  post: no)
[ 1055.552624] ACPI: PCI interrupt for device 0000:00:0a.0 disabled
[ 1055.554812] ACPI: PCI interrupt for device 0000:00:06.0 disabled
[ 1055.555479] ACPI: PCI interrupt for device 0000:00:02.0 disabled
[    0.901020] ACPI: PCI Interrupt 0000:00:02.0[A] -> Link [LNKG] -> GSI 11 
(level, low) -> IRQ 11
[    0.901271] ACPI: Unable to derive IRQ for device 0000:00:04.0
[    0.901278] ACPI: PCI Interrupt 0000:00:04.0[A]: no GSI
[    0.904594] ACPI: PCI Interrupt 0000:00:06.0[A] -> Link [LNKH] -> GSI 11 
(level, low) -> IRQ 11


As for the case without ACPI_PROCFS ... well, I do not have it in /proc - 
which is expected - but neither I do have it in /sys. And Kconfig help reads

          The deprecated files (and their replacements) include:

          /proc/acpi/sleep (/sys/power/state)
          /proc/acpi/info (/sys/modules/acpi/parameters/acpica_version)
          /proc/acpi/dsdt (/sys/firmware/acpi/tables/DSDT)
          /proc/acpi/fadt (/sys/firmware/acpi/tables/FACP)
          /proc/acpi/debug_layer (/sys/module/acpi/parameters/debug_layer)
          /proc/acpi/debug_level (/sys/module/acpi/parameters/debug_level)

neither does it mention /proc/acpi/battery not do I actually have any battery 
information in /sys.

Personally I do not like it (if it is intentional). Leaving only netlink 
interface means user has no way to query for actual state. We need something 
running all the time and hope, it never loses any event and thus reflects 
actual state. But it also means we are not allowed to restart it (whatever it 
is) as it will have no way to query for actual state on restart ...

-andrey

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [2.624-rc1 regression] lost battery information

[Frans Pop]

Andrey Borzenkov wrote:
> I have lost battery in 2.6.24-rc1. Without CONFIG_ACPI_PROCFS I have
> no /proc/acpi/battery and cannot test netlink interface because right now
> there is no consumer of this.

This is a known issue. Please see:
http://lkml.org/lkml/2007/10/22/110

Cheers,
Frans Pop

Re: [2.624-rc1 regression] lost battery information

[Alexey Starikovskiy]

[-- Attachment #1: Type: text/plain, Size: 7080 bytes --]

Andrey Borzenkov wrote:
> I have lost battery in 2.6.24-rc1. Without CONFIG_ACPI_PROCFS I have 
> no /proc/acpi/battery and cannot test netlink interface because right now 
> there is no consumer of this.
for /sysfs interface you need to enable power_supply driver.
or you could apply this patch and power_supply will be selected by battery itself.
> 
> With CONFIG_ACPI_PROCFS I get
> 
> {pts/1}% LC_ALL=C ll /proc/acpi/battery/BAT1
> total 0
> -rw-r--r-- 1 root root 0 Oct 26 20:18 alarm
> -r--r--r-- 1 root root 0 Oct 26 20:18 info
> -r--r--r-- 1 root root 0 Oct 26 20:18 state
> {pts/1}% LC_ALL=C  cat /proc/acpi/battery/BAT1/*
> cat: /proc/acpi/battery/BAT1/alarm: Bad address
> cat: /proc/acpi/battery/BAT1/info: Bad address
> cat: /proc/acpi/battery/BAT1/state: Bad address
Could you make sure it's a clean build with all drivers updated/kernel included?
> {pts/1}% LC_ALL=C ll /proc/acpi/battery/BAT2
> total 0
> -rw-r--r-- 1 root root 0 Oct 26 20:18 alarm
> -r--r--r-- 1 root root 0 Oct 26 20:18 info
> -r--r--r-- 1 root root 0 Oct 26 20:18 state
> {pts/1}% LC_ALL=C cat /proc/acpi/battery/BAT2/*
> present:                 no
> present:                 no
> present:                 no
> 
> BAT2 is correct - it is not present. BAT1 is lying. There is nothing in dmesg. 
> battery is loaded (obviously)
> 
> ACPI related stuff from dmesg:
> 
> {pts/1}% dmesg |grep ACPI
> [    0.000000]  BIOS-e820: 00000000000eee00 - 00000000000ef000 (ACPI NVS)
> [    0.000000]  BIOS-e820: 000000001ef60000 - 000000001ef70000 (ACPI data)
> [    0.000000] ACPI: RSDP 000F0090, 0014 (r0 TOSHIB)
> [    0.000000] ACPI: RSDT 1EF60000, 0028 (r1 TOSHIB 750        970814 TASM  
> 4010000)
> [    0.000000] ACPI: FACP 1EF60054, 0084 (r2 TOSHIB 750        970814 TASM  
> 4010000)
> [    0.000000] ACPI: DSDT 1EF600D8, 68DA (r1 TOSHIB 4000     20020417 MSFT  
> 100000A)
> [    0.000000] ACPI: FACS 000EEE00, 0040
> [    0.000000] ACPI: PM-Timer IO Port: 0xee08
> [  896.112009] ACPI: Core revision 20070126
> [  896.112775] ACPI: Looking for DSDT in initramfs... error, file /DSDT.aml 
> not found.
> [  896.123590]  tbxface-0598 [00] tb_load_namespace     : ACPI Tables 
> successfully acquired
> [  896.123631] ACPI: setting ELCR to 0200 (from 0a00)
> [  896.124208] evxfevnt-0091 [00] enable                : Transition to ACPI 
> mode successful
> [  896.131744] ACPI: bus type pci registered
> [  896.149165] ACPI: EC: Look up EC in DSDT
> [  896.163343] ACPI: Interpreter enabled
> [  896.163362] ACPI: (supports S0 S3 S4 S5)
> [  896.163510] ACPI: Using PIC for interrupt routing
> [  896.195892] ACPI: PCI Root Bridge [PCI0] (0000:00)
> [  896.197650] PCI quirk: region ee00-ee3f claimed by ali7101 ACPI
> [  896.200015] ACPI: PCI Interrupt Routing Table [\_SB_.PCI0._PRT]
> [  896.200588] ACPI: PCI Interrupt Routing Table [\_SB_.PCI0.PCI1._PRT]
> [  896.227797] ACPI: PCI Interrupt Link [LNKA] (IRQs 3 4 5 6 7 10 *11)
> [  896.228562] ACPI: PCI Interrupt Link [LNKB] (IRQs 3 4 5 6 7 10 *11)
> [  896.229271] ACPI: PCI Interrupt Link [LNKC] (IRQs 3 4 5 6 7 10 *11)
> [  896.230101] ACPI: PCI Interrupt Link [LNKD] (IRQs 3 4 5 6 7 10 *11)
> [  896.230818] ACPI: PCI Interrupt Link [LNKG] (IRQs 3 4 5 6 7 10 *11)
> [  896.231527] ACPI: PCI Interrupt Link [LNKH] (IRQs 3 4 5 6 7 10 *11)
> [  896.232912] ACPI: Power Resource [PFAN] (off)
> [  896.233622] pnp: PnP ACPI init
> [  896.233766] ACPI: bus type pnp registered
> [  896.257679] pnp: PnP ACPI: found 12 devices
> [  896.257737] ACPI: ACPI bus type pnp unregistered
> [  896.258820] PCI: Using ACPI for IRQ routing
> [  896.325763] ACPI: PCI Interrupt Link [LNKC] enabled at IRQ 11
> [  896.325805] ACPI: PCI Interrupt 0000:00:10.0[A] -> Link [LNKC] -> GSI 11 
> (level, low) -> IRQ 11
> [  896.327116] ACPI: PCI Interrupt Link [LNKA] enabled at IRQ 11
> [  896.327143] ACPI: PCI Interrupt 0000:00:11.0[A] -> Link [LNKA] -> GSI 11 
> (level, low) -> IRQ 11
> [  896.328392] ACPI: PCI Interrupt Link [LNKB] enabled at IRQ 11
> [  896.328416] ACPI: PCI Interrupt 0000:00:11.1[B] -> Link [LNKB] -> GSI 11 
> (level, low) -> IRQ 11
> [  896.978962] ACPI: PCI Interrupt Link [LNKD] enabled at IRQ 11
> [  896.980097] ACPI: PCI Interrupt 0000:00:0a.0[A] -> Link [LNKD] -> GSI 11 
> (level, low) -> IRQ 11
> [  902.378588] ACPI: Unable to derive IRQ for device 0000:00:04.0
> [  902.406719] ACPI: PCI Interrupt 0000:00:04.0[A]: no GSI
> [  919.051426] ACPI: PCI Interrupt Link [LNKG] enabled at IRQ 11
> [  919.051451] ACPI: PCI Interrupt 0000:00:02.0[A] -> Link [LNKG] -> GSI 11 
> (level, low) -> IRQ 11
> [  920.132284] ACPI: PCI Interrupt Link [LNKH] enabled at IRQ 11
> [  920.132307] ACPI: PCI Interrupt 0000:00:06.0[A] -> Link [LNKH] -> GSI 11 
> (level, low) -> IRQ 11
> [  927.120073] ACPI: AC Adapter [ADP1] (on-line)
> [  927.195942] ACPI: Battery Slot [BAT1] (battery present)
> [  927.200475] ACPI: Battery Slot [BAT2] (battery absent)
> [  927.277564] ACPI: Power Button (FF) [PWRF]
> [  927.290786] ACPI: Lid Switch [LID]
> [  927.324850] ACPI: Transitioning device [FAN] to D3
> [  927.324867] ACPI: Transitioning device [FAN] to D3
> [  927.324891] ACPI: Fan [FAN] (off)
> [  927.535960] ACPI: CPU0 (power states: C1[C1] C2[C2])
> [  927.638487] ACPI: Thermal Zone [THRM] (55 C)
> [  927.770100] toshiba_acpi: Toshiba Laptop ACPI Extras version 0.18
> [  927.920519] ACPI: Video Device [VGA] (multi-head: yes  rom: yes  post: no)
> [ 1055.552624] ACPI: PCI interrupt for device 0000:00:0a.0 disabled
> [ 1055.554812] ACPI: PCI interrupt for device 0000:00:06.0 disabled
> [ 1055.555479] ACPI: PCI interrupt for device 0000:00:02.0 disabled
> [    0.901020] ACPI: PCI Interrupt 0000:00:02.0[A] -> Link [LNKG] -> GSI 11 
> (level, low) -> IRQ 11
> [    0.901271] ACPI: Unable to derive IRQ for device 0000:00:04.0
> [    0.901278] ACPI: PCI Interrupt 0000:00:04.0[A]: no GSI
> [    0.904594] ACPI: PCI Interrupt 0000:00:06.0[A] -> Link [LNKH] -> GSI 11 
> (level, low) -> IRQ 11
> 
> 
> As for the case without ACPI_PROCFS ... well, I do not have it in /proc - 
> which is expected - but neither I do have it in /sys. And Kconfig help reads
> 
>           The deprecated files (and their replacements) include:
> 
>           /proc/acpi/sleep (/sys/power/state)
>           /proc/acpi/info (/sys/modules/acpi/parameters/acpica_version)
>           /proc/acpi/dsdt (/sys/firmware/acpi/tables/DSDT)
>           /proc/acpi/fadt (/sys/firmware/acpi/tables/FACP)
>           /proc/acpi/debug_layer (/sys/module/acpi/parameters/debug_layer)
>           /proc/acpi/debug_level (/sys/module/acpi/parameters/debug_level)
> 
> neither does it mention /proc/acpi/battery not do I actually have any battery 
> information in /sys.
> 
> Personally I do not like it (if it is intentional). Leaving only netlink 
> interface means user has no way to query for actual state. We need something 
> running all the time and hope, it never loses any event and thus reflects 
> actual state. But it also means we are not allowed to restart it (whatever it 
> is) as it will have no way to query for actual state on restart ...
> 
> -andrey


[-- Attachment #2: use_select_for_power_class.patch --]
[-- Type: text/x-diff, Size: 1350 bytes --]

ACPI: use select POWER_SUPPLY for AC, BATTERY and SBS

From: Alexey Starikovskiy <astarikovskiy@suse.de>

POWER_SUPPLY is needed for AC, battery, and SBS sysfs support.
Use 'select' instead of 'depends on', as it is will not be selected
by anything else, leading to confusion.

Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de>
---

 drivers/acpi/Kconfig |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 5d0e26a..ecd87d7 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -88,7 +88,8 @@ config ACPI_PROC_EVENT
 
 config ACPI_AC
 	tristate "AC Adapter"
-	depends on X86 && POWER_SUPPLY
+	depends on X86
+	select POWER_SUPPLY
 	default y
 	help
 	  This driver adds support for the AC Adapter object, which indicates
@@ -97,7 +98,8 @@ config ACPI_AC
 
 config ACPI_BATTERY
 	tristate "Battery"
-	depends on X86 && POWER_SUPPLY
+	depends on X86
+	select POWER_SUPPLY
 	default y
 	help
 	  This driver adds support for battery information through
@@ -352,7 +354,7 @@ config ACPI_HOTPLUG_MEMORY
 config ACPI_SBS
 	tristate "Smart Battery System"
 	depends on X86
-	depends on POWER_SUPPLY
+	select POWER_SUPPLY
 	help
 	  This driver adds support for the Smart Battery System, another
 	  type of access to battery information, found on some laptops.

Re: [2.624-rc1 regression] lost battery information

[Andrey Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 10299 bytes --]

On Friday 26 October 2007, Alexey Starikovskiy wrote:
> Andrey Borzenkov wrote:
> > I have lost battery in 2.6.24-rc1. Without CONFIG_ACPI_PROCFS I have
> > no /proc/acpi/battery and cannot test netlink interface because right now
> > there is no consumer of this.
>
> for /sysfs interface you need to enable power_supply driver.
> or you could apply this patch and power_supply will be selected by battery
> itself.
>

I already have power_supply module, battery depends on it and it is autloaded. 
But I fail to see where I can get battery info in /sys

Here is what I get in 2.6.23 from /proc/acpi/battery:

sh-3.2# ls -l /proc/acpi/battery/BAT1
total 0
-rw-r--r-- 1 root root 0 Oct 26 21:00 alarm
-r--r--r-- 1 root root 0 Oct 26 21:00 info
-r--r--r-- 1 root root 0 Oct 26 21:00 state
sh-3.2# cat /proc/acpi/battery/BAT1/alarm
alarm:                   756 mWh
sh-3.2# cat /proc/acpi/battery/BAT1/info
present:                 yes
design capacity:         38880 mWh
last full capacity:      37530 mWh
battery technology:      rechargeable
design voltage:          10800 mV
design capacity warning: 756 mWh
design capacity low:     0 mWh
capacity granularity 1:  10 mWh
capacity granularity 2:  10 mWh
model number:            XM2038P04   
serial number:           2000009244
battery type:            Li-ION  
OEM info:                
sh-3.2# cat /proc/acpi/battery/BAT1/state
present:                 yes
capacity state:          ok
charging state:          charged
present rate:            0 mW
remaining capacity:      37530 mWh
present voltage:         11340 mV

and here is what I can find in 2.6.24-rc1 in /sys/class/power_supply:

sh-3.2# ls -l /sys/class/power_supply/BAT1/
total 0
-rw-r--r-- 1 root root 4096 2007-10-26 20:55 alarm
lrwxrwxrwx 1 root root    0 2007-10-26 20:55 
device -> ../../../../../../devices/LNXSYSTM:00/device:00/PNP0C0A:00
drwxr-xr-x 2 root root    0 2007-10-26 20:55 power
lrwxrwxrwx 1 root root    0 2007-10-26 20:55 
subsystem -> ../../../../../../class/power_supply
-r--r--r-- 1 root root 4096 2007-10-26 20:55 type
-rw-r--r-- 1 root root 4096 2007-10-26 20:55 uevent
sh-3.2# ls -l /sys/class/power_supply/BAT1/device/
total 0
lrwxrwxrwx 1 root root    0 2007-10-26 20:54 
driver -> ../../../../bus/acpi/drivers/battery
-r--r--r-- 1 root root 4096 2007-10-26 20:56 hid
-r--r--r-- 1 root root 4096 2007-10-26 20:54 modalias
-r--r--r-- 1 root root 4096 2007-10-26 20:56 path
drwxr-xr-x 2 root root    0 2007-10-26 20:56 power
drwxr-xr-x 3 root root    0 2007-10-26 20:54 power_supply
lrwxrwxrwx 1 root root    0 2007-10-26 20:54 subsystem -> ../../../../bus/acpi
-rw-r--r-- 1 root root 4096 2007-10-26 20:54 uevent
sh-3.2# cat /sys/class/power_supply/BAT1/alarm 
0
sh-3.2# cat /sys/class/power_supply/BAT1/device/hid 
PNP0C0A
sh-3.2# cat /sys/class/power_supply/BAT1/device/path
\_SB_.BAT1

so the only useful information I get is that it is Battery (type attribute) 
and PNP/ACPI IDs. 

> > With CONFIG_ACPI_PROCFS I get
> >
> > {pts/1}% LC_ALL=C ll /proc/acpi/battery/BAT1
> > total 0
> > -rw-r--r-- 1 root root 0 Oct 26 20:18 alarm
> > -r--r--r-- 1 root root 0 Oct 26 20:18 info
> > -r--r--r-- 1 root root 0 Oct 26 20:18 state
> > {pts/1}% LC_ALL=C  cat /proc/acpi/battery/BAT1/*
> > cat: /proc/acpi/battery/BAT1/alarm: Bad address
> > cat: /proc/acpi/battery/BAT1/info: Bad address
> > cat: /proc/acpi/battery/BAT1/state: Bad address
>

And even if it worked with /sys you still have regression in /proc/acpi

> Could you make sure it's a clean build with all drivers updated/kernel
> included?
>

It is clean build of -rc1. As long as can trust git pull (I build every 
version in clean O directory).

> > {pts/1}% LC_ALL=C ll /proc/acpi/battery/BAT2
> > total 0
> > -rw-r--r-- 1 root root 0 Oct 26 20:18 alarm
> > -r--r--r-- 1 root root 0 Oct 26 20:18 info
> > -r--r--r-- 1 root root 0 Oct 26 20:18 state
> > {pts/1}% LC_ALL=C cat /proc/acpi/battery/BAT2/*
> > present:                 no
> > present:                 no
> > present:                 no
> >
> > BAT2 is correct - it is not present. BAT1 is lying. There is nothing in
> > dmesg. battery is loaded (obviously)
> >
> > ACPI related stuff from dmesg:
> >
> > {pts/1}% dmesg |grep ACPI
> > [    0.000000]  BIOS-e820: 00000000000eee00 - 00000000000ef000 (ACPI NVS)
> > [    0.000000]  BIOS-e820: 000000001ef60000 - 000000001ef70000 (ACPI
> > data) [    0.000000] ACPI: RSDP 000F0090, 0014 (r0 TOSHIB)
> > [    0.000000] ACPI: RSDT 1EF60000, 0028 (r1 TOSHIB 750        970814
> > TASM 4010000)
> > [    0.000000] ACPI: FACP 1EF60054, 0084 (r2 TOSHIB 750        970814
> > TASM 4010000)
> > [    0.000000] ACPI: DSDT 1EF600D8, 68DA (r1 TOSHIB 4000     20020417
> > MSFT 100000A)
> > [    0.000000] ACPI: FACS 000EEE00, 0040
> > [    0.000000] ACPI: PM-Timer IO Port: 0xee08
> > [  896.112009] ACPI: Core revision 20070126
> > [  896.112775] ACPI: Looking for DSDT in initramfs... error, file
> > /DSDT.aml not found.
> > [  896.123590]  tbxface-0598 [00] tb_load_namespace     : ACPI Tables
> > successfully acquired
> > [  896.123631] ACPI: setting ELCR to 0200 (from 0a00)
> > [  896.124208] evxfevnt-0091 [00] enable                : Transition to
> > ACPI mode successful
> > [  896.131744] ACPI: bus type pci registered
> > [  896.149165] ACPI: EC: Look up EC in DSDT
> > [  896.163343] ACPI: Interpreter enabled
> > [  896.163362] ACPI: (supports S0 S3 S4 S5)
> > [  896.163510] ACPI: Using PIC for interrupt routing
> > [  896.195892] ACPI: PCI Root Bridge [PCI0] (0000:00)
> > [  896.197650] PCI quirk: region ee00-ee3f claimed by ali7101 ACPI
> > [  896.200015] ACPI: PCI Interrupt Routing Table [\_SB_.PCI0._PRT]
> > [  896.200588] ACPI: PCI Interrupt Routing Table [\_SB_.PCI0.PCI1._PRT]
> > [  896.227797] ACPI: PCI Interrupt Link [LNKA] (IRQs 3 4 5 6 7 10 *11)
> > [  896.228562] ACPI: PCI Interrupt Link [LNKB] (IRQs 3 4 5 6 7 10 *11)
> > [  896.229271] ACPI: PCI Interrupt Link [LNKC] (IRQs 3 4 5 6 7 10 *11)
> > [  896.230101] ACPI: PCI Interrupt Link [LNKD] (IRQs 3 4 5 6 7 10 *11)
> > [  896.230818] ACPI: PCI Interrupt Link [LNKG] (IRQs 3 4 5 6 7 10 *11)
> > [  896.231527] ACPI: PCI Interrupt Link [LNKH] (IRQs 3 4 5 6 7 10 *11)
> > [  896.232912] ACPI: Power Resource [PFAN] (off)
> > [  896.233622] pnp: PnP ACPI init
> > [  896.233766] ACPI: bus type pnp registered
> > [  896.257679] pnp: PnP ACPI: found 12 devices
> > [  896.257737] ACPI: ACPI bus type pnp unregistered
> > [  896.258820] PCI: Using ACPI for IRQ routing
> > [  896.325763] ACPI: PCI Interrupt Link [LNKC] enabled at IRQ 11
> > [  896.325805] ACPI: PCI Interrupt 0000:00:10.0[A] -> Link [LNKC] -> GSI
> > 11 (level, low) -> IRQ 11
> > [  896.327116] ACPI: PCI Interrupt Link [LNKA] enabled at IRQ 11
> > [  896.327143] ACPI: PCI Interrupt 0000:00:11.0[A] -> Link [LNKA] -> GSI
> > 11 (level, low) -> IRQ 11
> > [  896.328392] ACPI: PCI Interrupt Link [LNKB] enabled at IRQ 11
> > [  896.328416] ACPI: PCI Interrupt 0000:00:11.1[B] -> Link [LNKB] -> GSI
> > 11 (level, low) -> IRQ 11
> > [  896.978962] ACPI: PCI Interrupt Link [LNKD] enabled at IRQ 11
> > [  896.980097] ACPI: PCI Interrupt 0000:00:0a.0[A] -> Link [LNKD] -> GSI
> > 11 (level, low) -> IRQ 11
> > [  902.378588] ACPI: Unable to derive IRQ for device 0000:00:04.0
> > [  902.406719] ACPI: PCI Interrupt 0000:00:04.0[A]: no GSI
> > [  919.051426] ACPI: PCI Interrupt Link [LNKG] enabled at IRQ 11
> > [  919.051451] ACPI: PCI Interrupt 0000:00:02.0[A] -> Link [LNKG] -> GSI
> > 11 (level, low) -> IRQ 11
> > [  920.132284] ACPI: PCI Interrupt Link [LNKH] enabled at IRQ 11
> > [  920.132307] ACPI: PCI Interrupt 0000:00:06.0[A] -> Link [LNKH] -> GSI
> > 11 (level, low) -> IRQ 11
> > [  927.120073] ACPI: AC Adapter [ADP1] (on-line)
> > [  927.195942] ACPI: Battery Slot [BAT1] (battery present)
> > [  927.200475] ACPI: Battery Slot [BAT2] (battery absent)
> > [  927.277564] ACPI: Power Button (FF) [PWRF]
> > [  927.290786] ACPI: Lid Switch [LID]
> > [  927.324850] ACPI: Transitioning device [FAN] to D3
> > [  927.324867] ACPI: Transitioning device [FAN] to D3
> > [  927.324891] ACPI: Fan [FAN] (off)
> > [  927.535960] ACPI: CPU0 (power states: C1[C1] C2[C2])
> > [  927.638487] ACPI: Thermal Zone [THRM] (55 C)
> > [  927.770100] toshiba_acpi: Toshiba Laptop ACPI Extras version 0.18
> > [  927.920519] ACPI: Video Device [VGA] (multi-head: yes  rom: yes  post:
> > no) [ 1055.552624] ACPI: PCI interrupt for device 0000:00:0a.0 disabled [
> > 1055.554812] ACPI: PCI interrupt for device 0000:00:06.0 disabled [
> > 1055.555479] ACPI: PCI interrupt for device 0000:00:02.0 disabled [   
> > 0.901020] ACPI: PCI Interrupt 0000:00:02.0[A] -> Link [LNKG] -> GSI 11
> > (level, low) -> IRQ 11
> > [    0.901271] ACPI: Unable to derive IRQ for device 0000:00:04.0
> > [    0.901278] ACPI: PCI Interrupt 0000:00:04.0[A]: no GSI
> > [    0.904594] ACPI: PCI Interrupt 0000:00:06.0[A] -> Link [LNKH] -> GSI
> > 11 (level, low) -> IRQ 11
> >
> >
> > As for the case without ACPI_PROCFS ... well, I do not have it in /proc -
> > which is expected - but neither I do have it in /sys. And Kconfig help
> > reads
> >
> >           The deprecated files (and their replacements) include:
> >
> >           /proc/acpi/sleep (/sys/power/state)
> >           /proc/acpi/info (/sys/modules/acpi/parameters/acpica_version)
> >           /proc/acpi/dsdt (/sys/firmware/acpi/tables/DSDT)
> >           /proc/acpi/fadt (/sys/firmware/acpi/tables/FACP)
> >           /proc/acpi/debug_layer
> > (/sys/module/acpi/parameters/debug_layer) /proc/acpi/debug_level
> > (/sys/module/acpi/parameters/debug_level)
> >
> > neither does it mention /proc/acpi/battery not do I actually have any
> > battery information in /sys.
> >
> > Personally I do not like it (if it is intentional). Leaving only netlink
> > interface means user has no way to query for actual state. We need
> > something running all the time and hope, it never loses any event and
> > thus reflects actual state. But it also means we are not allowed to
> > restart it (whatever it is) as it will have no way to query for actual
> > state on restart ...
> >
> > -andrey



[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [2.624-rc1 regression] lost battery information

[Frans Pop]

Andrey Borzenkov wrote:
> I already have power_supply module, battery depends on it and it is
> autloaded. But I fail to see where I can get battery info in /sys

Ah, yes. I see what you mean now and I can confirm the same regression wrt
missing battery data in /proc for my laptop.

$ cat /proc/acpi/battery/BAT1/*
cat: /proc/acpi/battery/BAT1/alarm: Bad address
cat: /proc/acpi/battery/BAT1/info: Bad address
cat: /proc/acpi/battery/BAT1/state: Bad address

Sorry for the earlier misunderstanding.

Re: [2.624-rc1 regression] lost battery information

[Alexey Starikovskiy]

Andrey Borzenkov wrote:
> On Friday 26 October 2007, Alexey Starikovskiy wrote:
>> Andrey Borzenkov wrote:
>>> I have lost battery in 2.6.24-rc1. Without CONFIG_ACPI_PROCFS I have
>>> no /proc/acpi/battery and cannot test netlink interface because right now
>>> there is no consumer of this.
>> for /sysfs interface you need to enable power_supply driver.
>> or you could apply this patch and power_supply will be selected by battery
>> itself.
>>
> 
> I already have power_supply module, battery depends on it and it is autloaded. 
> But I fail to see where I can get battery info in /sys
Intent was to put into /sysfs same information:
aystarik@samsung:~/client_conf$ ls /sys/class/power_supply/BAT1/
alarm  charge_full  charge_full_design  charge_now  current_now  device  manufacturer  model_name  present  status  subsystem  technology  type  uevent  voltage_min_design  voltage_now

aystarik@samsung:~/client_conf$ cat /sys/class/power_supply/BAT1/*
0
4172000
4300000
4172000
0
cat: /sys/class/power_supply/BAT1/device: Is a directory
Pacifi
Bat1
1
Full
cat: /sys/class/power_supply/BAT1/subsystem: Is a directory
Li-ion
Battery
PHYSDEVPATH=/devices/LNXSYSTM:00/device:00/PNP0C0A:00
PHYSDEVBUS=acpi
PHYSDEVDRIVER=battery
POWER_SUPPLY_NAME=BAT1
POWER_SUPPLY_TYPE=Battery
POWER_SUPPLY_STATUS=Full
POWER_SUPPLY_PRESENT=1
POWER_SUPPLY_TECHNOLOGY=Li-ion
POWER_SUPPLY_VOLTAGE_MIN_DESIGN=14800000
POWER_SUPPLY_VOLTAGE_NOW=16522000
POWER_SUPPLY_CURRENT_NOW=0
POWER_SUPPLY_CHARGE_FULL_DESIGN=4300000
POWER_SUPPLY_CHARGE_FULL=4172000
POWER_SUPPLY_CHARGE_NOW=4172000
POWER_SUPPLY_MODEL_NAME=Bat1
POWER_SUPPLY_MANUFACTURER=Pacifi
14800000
16522000

aystarik@samsung:~/client_conf$ cat /proc/acpi/battery/BAT1/*
alarm:                   unsupported
present:                 yes
design capacity:         4300 mAh
last full capacity:      4172 mAh
battery technology:      rechargeable
design voltage:          14800 mV
design capacity warning: 208 mAh
design capacity low:     41 mAh
capacity granularity 1:  41 mAh
capacity granularity 2:  41 mAh
model number:            Bat1
serial number:           001
battery type:            LION
OEM info:                Pacifi
present:                 yes
capacity state:          ok
charging state:          charged
present rate:            0 mA
remaining capacity:      4172 mAh
present voltage:         16518 mV

Re: [2.624-rc1 regression] lost battery information

[Andrey Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 2823 bytes --]

On Friday 26 October 2007, Alexey Starikovskiy wrote:
> Andrey Borzenkov wrote:
> > On Friday 26 October 2007, Alexey Starikovskiy wrote:
> >> Andrey Borzenkov wrote:
> >>> I have lost battery in 2.6.24-rc1. Without CONFIG_ACPI_PROCFS I have
> >>> no /proc/acpi/battery and cannot test netlink interface because right
> >>> now there is no consumer of this.
> >>
> >> for /sysfs interface you need to enable power_supply driver.
> >> or you could apply this patch and power_supply will be selected by
> >> battery itself.
> >
> > I already have power_supply module, battery depends on it and it is
> > autloaded. But I fail to see where I can get battery info in /sys
>
> Intent was to put into /sysfs same information:
> aystarik@samsung:~/client_conf$ ls /sys/class/power_supply/BAT1/
> alarm  charge_full  charge_full_design  charge_now  current_now  device 
> manufacturer  model_name  present  status  subsystem  technology  type 
> uevent  voltage_min_design  voltage_now
>

is it in -rc1 or can you point me to the patch (I'd rather avoid having to 
pull from different git trees). Thank you.

And what about ACPI_PROCFS case? It still needs attention I believe.

> aystarik@samsung:~/client_conf$ cat /sys/class/power_supply/BAT1/*
> 0
> 4172000
> 4300000
> 4172000
> 0
> cat: /sys/class/power_supply/BAT1/device: Is a directory
> Pacifi
> Bat1
> 1
> Full
> cat: /sys/class/power_supply/BAT1/subsystem: Is a directory
> Li-ion
> Battery
> PHYSDEVPATH=/devices/LNXSYSTM:00/device:00/PNP0C0A:00
> PHYSDEVBUS=acpi
> PHYSDEVDRIVER=battery
> POWER_SUPPLY_NAME=BAT1
> POWER_SUPPLY_TYPE=Battery
> POWER_SUPPLY_STATUS=Full
> POWER_SUPPLY_PRESENT=1
> POWER_SUPPLY_TECHNOLOGY=Li-ion
> POWER_SUPPLY_VOLTAGE_MIN_DESIGN=14800000
> POWER_SUPPLY_VOLTAGE_NOW=16522000
> POWER_SUPPLY_CURRENT_NOW=0
> POWER_SUPPLY_CHARGE_FULL_DESIGN=4300000
> POWER_SUPPLY_CHARGE_FULL=4172000
> POWER_SUPPLY_CHARGE_NOW=4172000
> POWER_SUPPLY_MODEL_NAME=Bat1
> POWER_SUPPLY_MANUFACTURER=Pacifi
> 14800000
> 16522000
>
> aystarik@samsung:~/client_conf$ cat /proc/acpi/battery/BAT1/*
> alarm:                   unsupported
> present:                 yes
> design capacity:         4300 mAh
> last full capacity:      4172 mAh
> battery technology:      rechargeable
> design voltage:          14800 mV
> design capacity warning: 208 mAh
> design capacity low:     41 mAh
> capacity granularity 1:  41 mAh
> capacity granularity 2:  41 mAh
> model number:            Bat1
> serial number:           001
> battery type:            LION
> OEM info:                Pacifi
> present:                 yes
> capacity state:          ok
> charging state:          charged
> present rate:            0 mA
> remaining capacity:      4172 mAh
> present voltage:         16518 mV



[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [2.624-rc1 regression] lost battery information

[Alexey Starikovskiy]

[-- Attachment #1: Type: text/plain, Size: 2159 bytes --]

Andrey Borzenkov wrote:
> On Friday 26 October 2007, Alexey Starikovskiy wrote:
>> Andrey Borzenkov wrote:
>>> On Friday 26 October 2007, Alexey Starikovskiy wrote:
>>>> Andrey Borzenkov wrote:
>>>>> I have lost battery in 2.6.24-rc1. Without CONFIG_ACPI_PROCFS I have
>>>>> no /proc/acpi/battery and cannot test netlink interface because right
>>>>> now there is no consumer of this.
>>>> for /sysfs interface you need to enable power_supply driver.
>>>> or you could apply this patch and power_supply will be selected by
>>>> battery itself.
>>> I already have power_supply module, battery depends on it and it is
>>> autloaded. But I fail to see where I can get battery info in /sys
>> Intent was to put into /sysfs same information:
>> aystarik@samsung:~/client_conf$ ls /sys/class/power_supply/BAT1/
>> alarm  charge_full  charge_full_design  charge_now  current_now  device 
>> manufacturer  model_name  present  status  subsystem  technology  type 
>> uevent  voltage_min_design  voltage_now
>>
> 
> is it in -rc1 or can you point me to the patch (I'd rather avoid having to 
> pull from different git trees). Thank you.
No, it should be rc1.
> 
> And what about ACPI_PROCFS case? It still needs attention I believe.
As you can see, there is info in /proc too:
>> aystarik@samsung:~/client_conf$ cat /proc/acpi/battery/BAT1/*
>> alarm:                   unsupported
>> present:                 yes
>> design capacity:         4300 mAh
>> last full capacity:      4172 mAh
>> battery technology:      rechargeable
>> design voltage:          14800 mV
>> design capacity warning: 208 mAh
>> design capacity low:     41 mAh
>> capacity granularity 1:  41 mAh
>> capacity granularity 2:  41 mAh
>> model number:            Bat1
>> serial number:           001
>> battery type:            LION
>> OEM info:                Pacifi
>> present:                 yes
>> capacity state:          ok
>> charging state:          charged
>> present rate:            0 mA
>> remaining capacity:      4172 mAh
>> present voltage:         16518 mV

Your cat's "Bad address" means -EFAULT, according to "man errno".
Please apply this patch to see what exactly failed...


[-- Attachment #2: x.patch --]
[-- Type: text/x-diff, Size: 1372 bytes --]

diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 02a396d..55e9a8e 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -262,21 +262,29 @@ static int extract_package(struct acpi_battery *battery,
 {
 	int i, *x;
 	union acpi_object *element;
-	if (package->type != ACPI_TYPE_PACKAGE)
+	if (package->type != ACPI_TYPE_PACKAGE) {
+printk (KERN_ERR PREFIX "package->type = %x\n", package->type);
 		return -EFAULT;
+	}
 	for (i = 0; i < num; ++i) {
-		if (package->package.count <= i)
+		if (package->package.count <= i) {
+printk (KERN_ERR PREFIX "package.count = %d, i = %d\n", package->package.count, i);
 			return -EFAULT;
+		}
 		element = &package->package.elements[i];
 		if (offsets[i].mode) {
 			if (element->type != ACPI_TYPE_STRING &&
-			    element->type != ACPI_TYPE_BUFFER)
+			    element->type != ACPI_TYPE_BUFFER) {
+printk (KERN_ERR PREFIX "element[%d]->type = %x, expected string\n", i, element->type);
 				return -EFAULT;
+			}
 			strncpy((u8 *)battery + offsets[i].offset,
 				element->string.pointer, 32);
 		} else {
-			if (element->type != ACPI_TYPE_INTEGER)
+			if (element->type != ACPI_TYPE_INTEGER) {
+printk (KERN_ERR PREFIX "element[%d]->type = %x, expected integer\n", i, element->type);
 				return -EFAULT;
+			}
 			x = (int *)((u8 *)battery + offsets[i].offset);
 			*x = element->integer.value;
 		}

Re: [2.624-rc1 regression] lost battery information

[Frans Pop]

Alexey Starikovskiy wrote:
> Andrey Borzenkov wrote:
>> is it in -rc1 or can you point me to the patch (I'd rather avoid having
>> to pull from different git trees). Thank you.
> No, it should be rc1.
>> 
>> And what about ACPI_PROCFS case? It still needs attention I believe.
> As you can see, there is info in /proc too:

I'm missing the info in /sys too, so it looks like the error in proc and
the missing info in /sys are related.

Alexey: do you have POWER_SUPPLY and/or AC/BATTERY compiled in or modular?
I wonder if that could make the difference.

Re: [2.624-rc1 regression] lost battery information

[Alexey Starikovskiy]

Frans Pop wrote:
> Alexey Starikovskiy wrote:
>> Andrey Borzenkov wrote:
>>> is it in -rc1 or can you point me to the patch (I'd rather avoid having
>>> to pull from different git trees). Thank you.
>> No, it should be rc1.
>>> And what about ACPI_PROCFS case? It still needs attention I believe.
>> As you can see, there is info in /proc too:
> 
> I'm missing the info in /sys too, so it looks like the error in proc and
> the missing info in /sys are related.
> 
> Alexey: do you have POWER_SUPPLY and/or AC/BATTERY compiled in or modular?
> I wonder if that could make the difference.
> 
Currently everything is module.

Re: [2.624-rc1 regression] lost battery information

[Andrey Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 3502 bytes --]

On Friday 26 October 2007, Alexey Starikovskiy wrote:
>
> Your cat's "Bad address" means -EFAULT, according to "man errno".
> Please apply this patch to see what exactly failed...



[ 1191.471572] ACPI: element[12]->type = 1, expected string
[ 1196.640065] ACPI: element[12]->type = 1, expected string
[ 1199.479773] ACPI: element[12]->type = 1, expected string
[ 1199.745435] ACPI: element[12]->type = 1, expected string

it is "OEM type". For reference here is _BIF from my DSDT:

             Method (_BIF, 0, NotSerialized)
            {
                Name (BUFF, Package (0x0D) {})
                Store (0x00, Index (BUFF, 0x00))
                Store (\_SB.MEM.BDV2, Local2)
                Multiply (\_SB.MEM.BDC2, Local2, Local0)
                Divide (Local0, 0x03E8, Local1, Local0)
                Store (Local0, Index (BUFF, 0x01))
                Multiply (\_SB.MEM.BLF2, Local2, Local0)
                Divide (Local0, 0x03E8, Local1, Local0)
                Store (Local0, Index (BUFF, 0x02))
                Store (\_SB.MEM.BTC2, Index (BUFF, 0x03))
                Store (\_SB.MEM.BDV2, Index (BUFF, 0x04))
                Multiply (\_SB.MEM.BCW2, Local2, Local0)
                Divide (Local0, 0x03E8, Local1, Local0)
                Store (Local0, Index (BUFF, 0x05))
                Multiply (\_SB.MEM.BCL2, Local2, Local0)
                Divide (Local0, 0x03E8, Local1, Local0)
                Store (Local0, Index (BUFF, 0x06))
                Multiply (\_SB.MEM.BG12, Local2, Local0)
                Divide (Local0, 0x03E8, Local1, Local0)
                Store (Local0, Index (BUFF, 0x07))
                Multiply (\_SB.MEM.BG22, Local2, Local0)
                Divide (Local0, 0x03E8, Local1, Local0)
                Store (Local0, Index (BUFF, 0x08))
                Store (\_SB.MEM.BMN2, Index (BUFF, 0x09))
                Store (\_SB.MEM.BSN2, Index (BUFF, 0x0A))
                Store (\_SB.MEM.BTP2, Index (BUFF, 0x0B))
                Store (\_SB.MEM.BOI2, Index (BUFF, 0x0C))
                Return (BUFF)
            }

This is behaviour change. Previous battery.c used generic acpi_extract_package 
which allowed (allows) for object of type integer when string is requested:

                case ACPI_TYPE_INTEGER:
                        switch (format_string[i]) {
                        case 'N':
                                size_required += sizeof(acpi_integer);
                                tail_offset += sizeof(acpi_integer);
                                break;
                        case 'S':
                                size_required +=
                                    sizeof(char *) + sizeof(acpi_integer) +
                                    sizeof(char);
                                tail_offset += sizeof(char *);
                                break;

while current battery.c:extract_package fails:

                if (offsets[i].mode) {
                        if (element->type != ACPI_TYPE_STRING &&
                            element->type != ACPI_TYPE_BUFFER) {
printk (KERN_ERR PREFIX "element[%d]->type = %x, expected string\n", i, 
element->type);
                                return -EFAULT;
                        }

well, while it could be BIOS fault this happily worked before ... This is 
obviously also the reason why I do not have anything in /sys

Fans, could you check whether you have the same issue using test patch?

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [2.624-rc1 regression] lost battery information

[Alexey Starikovskiy]

[-- Attachment #1: Type: text/plain, Size: 3749 bytes --]

Andrey,
Please try the attached patch. I choose to do snprintf() instead of direct copy,
as your previous message showed empty OEM type.

Thanks,
Alex.
Andrey Borzenkov wrote:
> On Friday 26 October 2007, Alexey Starikovskiy wrote:
>> Your cat's "Bad address" means -EFAULT, according to "man errno".
>> Please apply this patch to see what exactly failed...
> 
> 
> 
> [ 1191.471572] ACPI: element[12]->type = 1, expected string
> [ 1196.640065] ACPI: element[12]->type = 1, expected string
> [ 1199.479773] ACPI: element[12]->type = 1, expected string
> [ 1199.745435] ACPI: element[12]->type = 1, expected string
> 
> it is "OEM type". For reference here is _BIF from my DSDT:
> 
>              Method (_BIF, 0, NotSerialized)
>             {
>                 Name (BUFF, Package (0x0D) {})
>                 Store (0x00, Index (BUFF, 0x00))
>                 Store (\_SB.MEM.BDV2, Local2)
>                 Multiply (\_SB.MEM.BDC2, Local2, Local0)
>                 Divide (Local0, 0x03E8, Local1, Local0)
>                 Store (Local0, Index (BUFF, 0x01))
>                 Multiply (\_SB.MEM.BLF2, Local2, Local0)
>                 Divide (Local0, 0x03E8, Local1, Local0)
>                 Store (Local0, Index (BUFF, 0x02))
>                 Store (\_SB.MEM.BTC2, Index (BUFF, 0x03))
>                 Store (\_SB.MEM.BDV2, Index (BUFF, 0x04))
>                 Multiply (\_SB.MEM.BCW2, Local2, Local0)
>                 Divide (Local0, 0x03E8, Local1, Local0)
>                 Store (Local0, Index (BUFF, 0x05))
>                 Multiply (\_SB.MEM.BCL2, Local2, Local0)
>                 Divide (Local0, 0x03E8, Local1, Local0)
>                 Store (Local0, Index (BUFF, 0x06))
>                 Multiply (\_SB.MEM.BG12, Local2, Local0)
>                 Divide (Local0, 0x03E8, Local1, Local0)
>                 Store (Local0, Index (BUFF, 0x07))
>                 Multiply (\_SB.MEM.BG22, Local2, Local0)
>                 Divide (Local0, 0x03E8, Local1, Local0)
>                 Store (Local0, Index (BUFF, 0x08))
>                 Store (\_SB.MEM.BMN2, Index (BUFF, 0x09))
>                 Store (\_SB.MEM.BSN2, Index (BUFF, 0x0A))
>                 Store (\_SB.MEM.BTP2, Index (BUFF, 0x0B))
>                 Store (\_SB.MEM.BOI2, Index (BUFF, 0x0C))
>                 Return (BUFF)
>             }
> 
> This is behaviour change. Previous battery.c used generic acpi_extract_package 
> which allowed (allows) for object of type integer when string is requested:
> 
>                 case ACPI_TYPE_INTEGER:
>                         switch (format_string[i]) {
>                         case 'N':
>                                 size_required += sizeof(acpi_integer);
>                                 tail_offset += sizeof(acpi_integer);
>                                 break;
>                         case 'S':
>                                 size_required +=
>                                     sizeof(char *) + sizeof(acpi_integer) +
>                                     sizeof(char);
>                                 tail_offset += sizeof(char *);
>                                 break;
> 
> while current battery.c:extract_package fails:
> 
>                 if (offsets[i].mode) {
>                         if (element->type != ACPI_TYPE_STRING &&
>                             element->type != ACPI_TYPE_BUFFER) {
> printk (KERN_ERR PREFIX "element[%d]->type = %x, expected string\n", i, 
> element->type);
>                                 return -EFAULT;
>                         }
> 
> well, while it could be BIOS fault this happily worked before ... This is 
> obviously also the reason why I do not have anything in /sys
> 
> Fans, could you check whether you have the same issue using test patch?


[-- Attachment #2: battery_allow_extract_string_from_integer.patch --]
[-- Type: text/x-diff, Size: 1795 bytes --]

ACPI: Battery: Allow extract string from integer

From: Alexey Starikovskiy <astarikovskiy@suse.de>

Some machines return integer instead of expected string.

Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de>
---

 drivers/acpi/battery.c |   24 ++++++++++++++----------
 1 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 02a396d..6841358 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -260,7 +260,7 @@ static int extract_package(struct acpi_battery *battery,
 			   union acpi_object *package,
 			   struct acpi_offsets *offsets, int num)
 {
-	int i, *x;
+	int i;
 	union acpi_object *element;
 	if (package->type != ACPI_TYPE_PACKAGE)
 		return -EFAULT;
@@ -269,16 +269,20 @@ static int extract_package(struct acpi_battery *battery,
 			return -EFAULT;
 		element = &package->package.elements[i];
 		if (offsets[i].mode) {
-			if (element->type != ACPI_TYPE_STRING &&
-			    element->type != ACPI_TYPE_BUFFER)
-				return -EFAULT;
-			strncpy((u8 *)battery + offsets[i].offset,
-				element->string.pointer, 32);
+			u8 *ptr = (u8 *)battery + offsets[i].offset;
+			if (element->type == ACPI_TYPE_STRING ||
+			    element->type == ACPI_TYPE_BUFFER)
+				strncpy(ptr, element->string.pointer, 32);
+			else if (element->type == ACPI_TYPE_INTEGER) {
+				snprintf(ptr, 32, "%lld",
+					element->integer.value);
+			} else return -EFAULT;
 		} else {
-			if (element->type != ACPI_TYPE_INTEGER)
-				return -EFAULT;
-			x = (int *)((u8 *)battery + offsets[i].offset);
-			*x = element->integer.value;
+			if (element->type == ACPI_TYPE_INTEGER) {
+				int *x = (int *)((u8 *)battery +
+						offsets[i].offset);
+				*x = element->integer.value;
+			} else return -EFAULT;
 		}
 	}
 	return 0;

Re: [2.624-rc1 regression] lost battery information

[Andrey Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 4425 bytes --]

On Saturday 27 October 2007, Alexey Starikovskiy wrote:
> Andrey,
> Please try the attached patch. I choose to do snprintf() instead of direct
> copy, as your previous message showed empty OEM type.
>

Not quite. Now I get

OEM info:                0

while before I got empty string. If I read acpi_extract_package correctly, it 
actually interpreted integer as string without any conversion. Which in this 
case obviously gave us empty string (integer being 0). I'd prefer to remain 
compatible.

also

{pts/1}% cat /sys/class/power_supply/BAT1/manufacturer
0

which is rather weird manufacturer name :)

> Thanks,
> Alex.
>
> Andrey Borzenkov wrote:
> > On Friday 26 October 2007, Alexey Starikovskiy wrote:
> >> Your cat's "Bad address" means -EFAULT, according to "man errno".
> >> Please apply this patch to see what exactly failed...
> >
> > [ 1191.471572] ACPI: element[12]->type = 1, expected string
> > [ 1196.640065] ACPI: element[12]->type = 1, expected string
> > [ 1199.479773] ACPI: element[12]->type = 1, expected string
> > [ 1199.745435] ACPI: element[12]->type = 1, expected string
> >
> > it is "OEM type". For reference here is _BIF from my DSDT:
> >
> >              Method (_BIF, 0, NotSerialized)
> >             {
> >                 Name (BUFF, Package (0x0D) {})
> >                 Store (0x00, Index (BUFF, 0x00))
> >                 Store (\_SB.MEM.BDV2, Local2)
> >                 Multiply (\_SB.MEM.BDC2, Local2, Local0)
> >                 Divide (Local0, 0x03E8, Local1, Local0)
> >                 Store (Local0, Index (BUFF, 0x01))
> >                 Multiply (\_SB.MEM.BLF2, Local2, Local0)
> >                 Divide (Local0, 0x03E8, Local1, Local0)
> >                 Store (Local0, Index (BUFF, 0x02))
> >                 Store (\_SB.MEM.BTC2, Index (BUFF, 0x03))
> >                 Store (\_SB.MEM.BDV2, Index (BUFF, 0x04))
> >                 Multiply (\_SB.MEM.BCW2, Local2, Local0)
> >                 Divide (Local0, 0x03E8, Local1, Local0)
> >                 Store (Local0, Index (BUFF, 0x05))
> >                 Multiply (\_SB.MEM.BCL2, Local2, Local0)
> >                 Divide (Local0, 0x03E8, Local1, Local0)
> >                 Store (Local0, Index (BUFF, 0x06))
> >                 Multiply (\_SB.MEM.BG12, Local2, Local0)
> >                 Divide (Local0, 0x03E8, Local1, Local0)
> >                 Store (Local0, Index (BUFF, 0x07))
> >                 Multiply (\_SB.MEM.BG22, Local2, Local0)
> >                 Divide (Local0, 0x03E8, Local1, Local0)
> >                 Store (Local0, Index (BUFF, 0x08))
> >                 Store (\_SB.MEM.BMN2, Index (BUFF, 0x09))
> >                 Store (\_SB.MEM.BSN2, Index (BUFF, 0x0A))
> >                 Store (\_SB.MEM.BTP2, Index (BUFF, 0x0B))
> >                 Store (\_SB.MEM.BOI2, Index (BUFF, 0x0C))
> >                 Return (BUFF)
> >             }
> >
> > This is behaviour change. Previous battery.c used generic
> > acpi_extract_package which allowed (allows) for object of type integer
> > when string is requested:
> >
> >                 case ACPI_TYPE_INTEGER:
> >                         switch (format_string[i]) {
> >                         case 'N':
> >                                 size_required += sizeof(acpi_integer);
> >                                 tail_offset += sizeof(acpi_integer);
> >                                 break;
> >                         case 'S':
> >                                 size_required +=
> >                                     sizeof(char *) + sizeof(acpi_integer)
> > + sizeof(char);
> >                                 tail_offset += sizeof(char *);
> >                                 break;
> >
> > while current battery.c:extract_package fails:
> >
> >                 if (offsets[i].mode) {
> >                         if (element->type != ACPI_TYPE_STRING &&
> >                             element->type != ACPI_TYPE_BUFFER) {
> > printk (KERN_ERR PREFIX "element[%d]->type = %x, expected string\n", i,
> > element->type);
> >                                 return -EFAULT;
> >                         }
> >
> > well, while it could be BIOS fault this happily worked before ... This is
> > obviously also the reason why I do not have anything in /sys
> >
> > Fans, could you check whether you have the same issue using test patch?



[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [2.624-rc1 regression] lost battery information

[Alexey Starikovskiy]

[-- Attachment #1: Type: text/plain, Size: 4540 bytes --]

Andrey Borzenkov wrote:
> On Saturday 27 October 2007, Alexey Starikovskiy wrote:
>> Andrey,
>> Please try the attached patch. I choose to do snprintf() instead of direct
>> copy, as your previous message showed empty OEM type.
>>
> 
> Not quite. Now I get
> 
> OEM info:                0
Ok, I was hoping to see some number starting with 0, which would be printed 
as empty string... 
> 
> while before I got empty string. If I read acpi_extract_package correctly, it 
> actually interpreted integer as string without any conversion. Which in this 
> case obviously gave us empty string (integer being 0). I'd prefer to remain 
> compatible.
As you wish... :) Please check the attached patch.
> 
> also
> 
> {pts/1}% cat /sys/class/power_supply/BAT1/manufacturer
> 0
> 
> which is rather weird manufacturer name :)
> 
>> Thanks,
>> Alex.
>>
>> Andrey Borzenkov wrote:
>>> On Friday 26 October 2007, Alexey Starikovskiy wrote:
>>>> Your cat's "Bad address" means -EFAULT, according to "man errno".
>>>> Please apply this patch to see what exactly failed...
>>> [ 1191.471572] ACPI: element[12]->type = 1, expected string
>>> [ 1196.640065] ACPI: element[12]->type = 1, expected string
>>> [ 1199.479773] ACPI: element[12]->type = 1, expected string
>>> [ 1199.745435] ACPI: element[12]->type = 1, expected string
>>>
>>> it is "OEM type". For reference here is _BIF from my DSDT:
>>>
>>>              Method (_BIF, 0, NotSerialized)
>>>             {
>>>                 Name (BUFF, Package (0x0D) {})
>>>                 Store (0x00, Index (BUFF, 0x00))
>>>                 Store (\_SB.MEM.BDV2, Local2)
>>>                 Multiply (\_SB.MEM.BDC2, Local2, Local0)
>>>                 Divide (Local0, 0x03E8, Local1, Local0)
>>>                 Store (Local0, Index (BUFF, 0x01))
>>>                 Multiply (\_SB.MEM.BLF2, Local2, Local0)
>>>                 Divide (Local0, 0x03E8, Local1, Local0)
>>>                 Store (Local0, Index (BUFF, 0x02))
>>>                 Store (\_SB.MEM.BTC2, Index (BUFF, 0x03))
>>>                 Store (\_SB.MEM.BDV2, Index (BUFF, 0x04))
>>>                 Multiply (\_SB.MEM.BCW2, Local2, Local0)
>>>                 Divide (Local0, 0x03E8, Local1, Local0)
>>>                 Store (Local0, Index (BUFF, 0x05))
>>>                 Multiply (\_SB.MEM.BCL2, Local2, Local0)
>>>                 Divide (Local0, 0x03E8, Local1, Local0)
>>>                 Store (Local0, Index (BUFF, 0x06))
>>>                 Multiply (\_SB.MEM.BG12, Local2, Local0)
>>>                 Divide (Local0, 0x03E8, Local1, Local0)
>>>                 Store (Local0, Index (BUFF, 0x07))
>>>                 Multiply (\_SB.MEM.BG22, Local2, Local0)
>>>                 Divide (Local0, 0x03E8, Local1, Local0)
>>>                 Store (Local0, Index (BUFF, 0x08))
>>>                 Store (\_SB.MEM.BMN2, Index (BUFF, 0x09))
>>>                 Store (\_SB.MEM.BSN2, Index (BUFF, 0x0A))
>>>                 Store (\_SB.MEM.BTP2, Index (BUFF, 0x0B))
>>>                 Store (\_SB.MEM.BOI2, Index (BUFF, 0x0C))
>>>                 Return (BUFF)
>>>             }
>>>
>>> This is behaviour change. Previous battery.c used generic
>>> acpi_extract_package which allowed (allows) for object of type integer
>>> when string is requested:
>>>
>>>                 case ACPI_TYPE_INTEGER:
>>>                         switch (format_string[i]) {
>>>                         case 'N':
>>>                                 size_required += sizeof(acpi_integer);
>>>                                 tail_offset += sizeof(acpi_integer);
>>>                                 break;
>>>                         case 'S':
>>>                                 size_required +=
>>>                                     sizeof(char *) + sizeof(acpi_integer)
>>> + sizeof(char);
>>>                                 tail_offset += sizeof(char *);
>>>                                 break;
>>>
>>> while current battery.c:extract_package fails:
>>>
>>>                 if (offsets[i].mode) {
>>>                         if (element->type != ACPI_TYPE_STRING &&
>>>                             element->type != ACPI_TYPE_BUFFER) {
>>> printk (KERN_ERR PREFIX "element[%d]->type = %x, expected string\n", i,
>>> element->type);
>>>                                 return -EFAULT;
>>>                         }
>>>
>>> well, while it could be BIOS fault this happily worked before ... This is
>>> obviously also the reason why I do not have anything in /sys
>>>
>>> Fans, could you check whether you have the same issue using test patch?
> 
> 


[-- Attachment #2: battery_allow_extract_string_from_integer.patch --]
[-- Type: text/x-diff, Size: 1848 bytes --]

ACPI: Battery: Allow extract string from integer

From: Alexey Starikovskiy <astarikovskiy@suse.de>

Some machines return integer instead of expected string.

Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de>
---

 drivers/acpi/battery.c |   25 +++++++++++++++----------
 1 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 02a396d..6c06879 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -260,7 +260,7 @@ static int extract_package(struct acpi_battery *battery,
 			   union acpi_object *package,
 			   struct acpi_offsets *offsets, int num)
 {
-	int i, *x;
+	int i;
 	union acpi_object *element;
 	if (package->type != ACPI_TYPE_PACKAGE)
 		return -EFAULT;
@@ -269,16 +269,21 @@ static int extract_package(struct acpi_battery *battery,
 			return -EFAULT;
 		element = &package->package.elements[i];
 		if (offsets[i].mode) {
-			if (element->type != ACPI_TYPE_STRING &&
-			    element->type != ACPI_TYPE_BUFFER)
-				return -EFAULT;
-			strncpy((u8 *)battery + offsets[i].offset,
-				element->string.pointer, 32);
+			u8 *ptr = (u8 *)battery + offsets[i].offset;
+			if (element->type == ACPI_TYPE_STRING ||
+			    element->type == ACPI_TYPE_BUFFER)
+				strncpy(ptr, element->string.pointer, 32);
+			else if (element->type == ACPI_TYPE_INTEGER) {
+				strncpy(ptr, (u8 *)&element->integer.value,
+					sizeof(acpi_integer));
+				ptr[sizeof(acpi_integer)] = 0;
+			} else return -EFAULT;
 		} else {
-			if (element->type != ACPI_TYPE_INTEGER)
-				return -EFAULT;
-			x = (int *)((u8 *)battery + offsets[i].offset);
-			*x = element->integer.value;
+			if (element->type == ACPI_TYPE_INTEGER) {
+				int *x = (int *)((u8 *)battery +
+						offsets[i].offset);
+				*x = element->integer.value;
+			} else return -EFAULT;
 		}
 	}
 	return 0;

Re: [2.624-rc1 regression] lost battery information

[Frans Pop]

Hmm. Things seem to have progressed since I was last online :-)

With Alexey's original patch I also get a number of times:
ACPI: element[12]->type = 1, expected string

On Saturday 27 October 2007, Alexey Starikovskiy wrote:
> As you wish... :) Please check the attached patch.

With 'battery_allow_extract_string_from_integer.patch' all info in /proc is 
back and I now also see the new files in /sys/class/power_supply.

The "OEM info" field (line 13 in BAT1/info) is empty, just as it was empty 
in 2.6.23 too.

Tested-by: Frans Pop <elendil@planet.nl>

Cheers,
Frans Pop

Re: [2.624-rc1 regression] lost battery information

[Andrey Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 228 bytes --]

On Saturday 27 October 2007, Alexey Starikovskiy wrote:
> As you wish... :) Please check the attached patch.
>

Not sure why you need to reimplement acpi_extract_package, but ...

Tested-by: Andrey Borzenkov <arvidjaar@mail.ru>

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [2.624-rc1 regression] lost battery information

[Alexey Starikovskiy]

Andrey Borzenkov wrote:
> On Saturday 27 October 2007, Alexey Starikovskiy wrote:
>> As you wish... :) Please check the attached patch.
>>
> 
> Not sure why you need to reimplement acpi_extract_package, but ...
Take a look on memory allocations around it... :)
> 
> Tested-by: Andrey Borzenkov <arvidjaar@mail.ru>

Re: ACPI: use select POWER_SUPPLY for AC, BATTERY and SBS (was: [2.624-rc1 regression] lost battery information)

[Matej Laitl]

Alexey Starikovskiy wrote:
> ACPI: use select POWER_SUPPLY for AC, BATTERY and SBS
>
> From: Alexey Starikovskiy <astarikovskiy@suse.de>
>
> POWER_SUPPLY is needed for AC, battery, and SBS sysfs support.
> Use 'select' instead of 'depends on', as it is will not be selected
> by anything else, leading to confusion.
>
> Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de>
> ---
>
>  drivers/acpi/Kconfig |    8 +++++---
>  1 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 5d0e26a..ecd87d7 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -88,7 +88,8 @@ config ACPI_PROC_EVENT
>
>  config ACPI_AC
>         tristate "AC Adapter"
> -       depends on X86 && POWER_SUPPLY
> +       depends on X86
> +       select POWER_SUPPLY
>         default y
>         help
>           This driver adds support for the AC Adapter object, which
> indicates @@ -97,7 +98,8 @@ config ACPI_AC
>
>  config ACPI_BATTERY
>         tristate "Battery"
> -       depends on X86 && POWER_SUPPLY
> +       depends on X86
> +       select POWER_SUPPLY
>         default y
>         help
>           This driver adds support for battery information through
> @@ -352,7 +354,7 @@ config ACPI_HOTPLUG_MEMORY
>  config ACPI_SBS
>         tristate "Smart Battery System"
>         depends on X86
> -       depends on POWER_SUPPLY
> +       select POWER_SUPPLY
>         help
>           This driver adds support for the Smart Battery System, another
>           type of access to battery information, found on some laptops.

I'd love if this got merged, as I also lost my battery information by 
not-enabling POWER_SUPPLY (which looks like something unrelated to ACPI). (I 
know "select" is evil, but this use-case is appropriate, IMO)

Matej