Bug 2884 - /proc/acpi/battery/BAT1 not created on hot-add - IBM T40/T41/R50p
Summary: /proc/acpi/battery/BAT1 not created on hot-add - IBM T40/T41/R50p
Status: CLOSED CODE_FIX
Alias: None
Product: ACPI
Classification: Unclassified
Component: Power-Battery (show other bugs)
Hardware: i386 Linux
: P2 normal
Assignee: Shaohua
URL:
Keywords:
: 4728 9731 (view as bug list)
Depends on:
Blocks:
 
Reported: 2004-06-13 13:00 UTC by Paul Ionescu
Modified: 2008-10-27 19:20 UTC (History)
5 users (show)

See Also:
Kernel Version: 2.6.7-rc3
Subsystem:
Regression: ---
Bisected commit-id:


Attachments
Test patch (3.88 KB, patch)
2004-12-21 22:44 UTC, Shaohua
Details | Diff
Patch for battery hotplug (7.31 KB, patch)
2005-07-04 23:06 UTC, Shaohua
Details | Diff
patch-battery-hotplug-1 (606 bytes, patch)
2007-10-29 19:31 UTC, Zhang Rui
Details | Diff
patch-battery-hotplug-2 (1.96 KB, application/octet-stream)
2007-10-29 19:32 UTC, Zhang Rui
Details
patch-battery-hotplug-3 (8.77 KB, patch)
2007-10-29 19:33 UTC, Zhang Rui
Details | Diff
patch-battery-hotplug-4 (1.60 KB, patch)
2007-10-29 19:33 UTC, Zhang Rui
Details | Diff
t43 acpidump (257.45 KB, application/octet-stream)
2008-01-10 19:37 UTC, Zhang Rui
Details
patch-battery-hotplug-1 (606 bytes, patch)
2008-01-16 00:07 UTC, Zhang Rui
Details | Diff
patch-battery-hotplug-2 (1.96 KB, patch)
2008-01-16 00:07 UTC, Zhang Rui
Details | Diff
patch-battery-hotplug-3 (8.69 KB, patch)
2008-01-16 00:08 UTC, Zhang Rui
Details | Diff
patch-battery-hotplug-4 (1.60 KB, patch)
2008-01-16 00:08 UTC, Zhang Rui
Details | Diff

Description Paul Ionescu 2004-06-13 13:00:13 UTC
Distribution:
FC2

Hardware Environment:
IBM T40/T41/R50p

Software Environment:
FC2 full install

Problem Description:
when hotplug the second battery,  /proc/acpi/battery/BAT1 is not created
if, the battery is present at boot time, the forementioned directory is created,
and everything is fine if later on I pull/plug the battery.

Steps to reproduce:
boot without second battery.
put second battery in bay.
/proc/acpi/battery/BAT1 is not created, but the battery is active because the
first battery is not discharging anymore.

boot with second battery in bay.
/proc/acpi/battery/BAT1 is created, battery is present, everything is fine if I
plug/unplug whatever battery I want.

With previous kernels, like 2.6.3 or something, the /proc/acpi/battery/BAT1
directory was created at boot time, even if there was not the second battery
iserted.
So it was not like this all the time, something changed recently in battery code.
Comment 1 Shaohua 2004-06-13 23:20:26 UTC
Ok, we have understood this problem and will provide fix later. Now could you 
please try add boot option 'acpi_gbl_create_osi_method'? Thanks.
Comment 2 Paul Ionescu 2004-06-14 10:25:53 UTC
I put that argument to the kernel, but still does not work:
cat /proc/cmdline 
ro root=LABEL=/ rhgb quiet hda=7752,240,63 acpi_sleep=s3_bios
acpi_gbl_create_osi_method

ls /proc/acpi/battery/
BAT0/

with both my batteries in. but the second one was hotplugged.
Comment 3 Shaohua 2004-06-14 19:37:23 UTC
Hmm, there is another bug (It has been merged into ACPICA, but still not in 
Linux). Please apply this small patch and try again 
(with 'acpi_gbl_create_osi_method')
===== utglobal.c 1.36 vs edited =====
--- 1.36/drivers/acpi/utilities/utglobal.c	Thu Apr  1 10:18:12 2004
+++ edited/drivers/acpi/utilities/utglobal.c	Tue Jun 15 10:31:05 2004
@@ -774,6 +774,7 @@
  *
  ***************************************************************************/
 
+acpi_gbl_create_osi_method = TRUE;
 void
 acpi_ut_init_globals (
 	void)
@@ -785,7 +786,6 @@
 
 	/* Runtime configuration */
 
-	acpi_gbl_create_osi_method = TRUE;
 	acpi_gbl_all_methods_serialized = FALSE;
 
 	/* Memory allocation and cache lists */
Comment 4 Paul Ionescu 2004-07-25 14:08:09 UTC
Hi David,

I tried linux kernel 2.6.8-rc2 with latest ACPICA patch
(acpi-20040715-2.6.8.diff.bz2), and I still have this problem.

I tried with and without the boot option 'acpi_gbl_create_osi_method', and the
same result.
Do I need any extra patch for this to work ?

Comment 5 Paul Ionescu 2004-07-25 14:09:19 UTC
P.S. The above extra patch is not working for kernel 2.6.8rc1 with latest acpi
patch.
Comment 6 Shaohua 2004-07-26 18:27:59 UTC
Sorry Paul, I ponitered out a wrong boot option. It should be 'acpi_osi'. 
Actually this problem is not a bug. Currently Linux pretend itself as Windows, 
since most ACPI BIOS only are tested under windows. But current battery driver 
isn't compatible with windows' driver. One workaround is to disable the 
compatiblity with windows disabling OSI. 
Comment 7 Paul Ionescu 2004-07-27 05:15:15 UTC
Hi David,

I tried the new boot option to disable OSI, but is still not working:

 cat /proc/cmdline 
ro root=/dev/hda2 rhgb quiet hda=7752,240,63 acpi_sleep=s3_bios acpi_osi
 ls /proc/acpi/battery
BAT0/

Then I tried with the kernel parameter acpi_osi= ,as specified in
Documentation/kernel-parameters.txt, and still not working:

 cat /proc/cmdline 
ro root=/dev/hda2 rhgb quiet hda=7752,240,63 acpi_sleep=s3_bios acpi_osi=
  ls /proc/acpi/battery
BAT0/
Comment 8 Shaohua 2004-07-27 18:53:18 UTC
Looks like we must change os name also. Please add boot 
options 'acpi_os_name=Linux' and 'acpi_osi='. I'm sure this will work.

I will fix the battery driver soon, so no boot option is needed. Before this 
please use the boot options to work around. 

Thanks,
David
Comment 9 Paul Ionescu 2004-07-28 02:10:22 UTC
Hi David,

I put 'acpi_os_name=Linux' and 'acpi_osi=' as boot options and now it works.

Thanks,
Paul
Comment 10 Len Brown 2004-11-15 22:01:57 UTC
David, 
what do we need to do to make Linux's battery driver compatible with that this DSDT expects? 
Comment 11 Shaohua 2004-11-15 22:07:51 UTC
The fix is just like Anil's physical CPU hotplug process module (Sure this one 
it's more easy.). I planed to fix the problem after Bob export some hotplug 
APIs, but it seems Bob is busy. Ok, I'll provide a patch using old APIs and 
replace it when Bob release new APIs.
Comment 12 Shaohua 2004-12-21 22:44:06 UTC
Created attachment 4289 [details]
Test patch

Paul,
Could you please test this patch? It doesn't require any boot option to support
battery hotplug.
The patch is based on the ACPI test tree. you can get the tree from:
bk clone http://linux-acpi.bkbits.net:8080/linux-acpi-test-mm
Then apply the patch in the tree. Thanks.
Comment 13 Shaohua 2004-12-29 22:09:28 UTC
The patch can be applied into latest base kernel, could you please try? We 
can't do further, if no test report. Thanks.
Comment 14 Len Brown 2005-01-03 21:19:07 UTC
test result?
Comment 15 Shaohua 2005-06-10 01:09:03 UTC
*** Bug 4728 has been marked as a duplicate of this bug. ***
Comment 16 Eugene Pavlovsky 2005-06-16 03:29:45 UTC
These kernel parameters make it work for me. Will the battery driver be fixed so
that there would be no need for them?
Comment 17 Shaohua 2005-06-16 17:34:34 UTC
Yes, the boot option is just a workaround, we must fix the driver, so please 
try the driver!
Comment 18 Eugene Pavlovsky 2005-06-17 00:13:21 UTC
do you mean "try fixing the driver"? if so, sorry, I'm not keen for kernel dev.
Comment 19 Shaohua 2005-07-04 23:06:28 UTC
Created attachment 5265 [details]
Patch for battery hotplug

This patch works for me for both hot add and hot remove battery, please give it
a try. Thanks!
Comment 20 Len Brown 2007-03-08 21:26:17 UTC
patch is obsolete as of the oldest tree i have -- 2.6.12
re-opening.
Comment 21 Len Brown 2007-03-09 14:53:48 UTC
is this issue still present in a recent kernel, say 2.6.20?
Comment 22 Paul Ionescu 2007-03-24 13:56:35 UTC
This issue is still present in kernel 2.6.20 from FC6
Comment 23 Fu Michael 2007-09-12 01:04:29 UTC
rui will refresh the patch in comment# 19. 

Paul, would you please test the refreshed patch and update if it works for you?
Comment 24 Zhang Rui 2007-10-29 19:31:50 UTC
Created attachment 13330 [details]
patch-battery-hotplug-1
Comment 25 Zhang Rui 2007-10-29 19:32:33 UTC
Created attachment 13331 [details]
patch-battery-hotplug-2
Comment 26 Zhang Rui 2007-10-29 19:33:05 UTC
Created attachment 13332 [details]
patch-battery-hotplug-3
Comment 27 Zhang Rui 2007-10-29 19:33:31 UTC
Created attachment 13333 [details]
patch-battery-hotplug-4
Comment 28 Zhang Rui 2007-10-29 19:36:26 UTC
Hi, Paul,

The patches are tested on my T43 laptop and they work well.
can you please test the patch set in kernel 2.6.23?
Comment 29 Len Brown 2007-12-05 18:53:32 UTC
Rui,
Since you reproduced this on your T43...
Please attach the acpidump output for that box.
it is key that we understand why the workaround of
'acpi_os_name=Linux' and 'acpi_osi=' works
(and why we'll not need it anymore after your patch series).

<marking RESOLVED b/c there is a patch series proposed for review & testing>
Comment 30 Zhang Rui 2008-01-10 19:37:20 UTC
Created attachment 14409 [details]
t43 acpidump
Comment 31 Len Brown 2008-01-12 16:00:12 UTC
The DSDT in comment #30 looks this way,

            If (LEqual (\SCMP (\_OS, "Microsoft Windows"), Zero))
            {
                Store (0x01, \W98F) // Windows 98
            }
            Else
            {
                If (CondRefOf (\_OSI, Local0))
                {
                    If (\_OSI ("Windows 2001.1"))
                    {
                        Store (0x01, \WDNF)
                        Store (0x01, \WXPF)
                        Store (0x01, \WNTF)
                    }
                    Else
                    {
                        If (\_OSI ("Windows 2001 SP1"))
                        {
                            Store (0x01, \WSPV)
                            Store (0x01, \WXPF)
                            Store (0x01, \WNTF)
                            // XP SP1 -- Linux w/o cmdline comes here
                        }
                        Else
                        {
                            If (\_OSI ("Windows 2001"))
                            {
                                Store (0x01, \WXPF)
                                Store (0x01, \WNTF)
                                // Windows 2001 is Windows XP
                            }
                        }
                    }
                }
                Else
                {
                    If (LEqual (\SCMP (\_OS, "Microsoft Windows NT"), Zero))
                    {
                        Store (0x01, \WNTF)
                    }
                    Else
                    {
                        If (LEqual (\SCMP (\_OS, "Microsoft WindowsME: Millennium Edition"), Zero))
                        {
                            Store (0x01, \WMEF)
                            Store (0x01, \W98F)
                        }
                    }
                }
            }

Without any bootparams, Linux would take the Windows 2001.1 path.
With acpi_os_name=Linux acpi_osi= it looks like we take a path
through the AML where no OS bits are set -- probably a path
that has never been tested...  As the DSDT never checks the
string "Linux", you'd get the same result if you replaced
it with any random string that doesn't begin with "Microsoft".

It is possible to test all of the paths above using some
cmdline options to see which one matters:

acpi_os_name="Microsoft Windows"
   This will take the 1st path above, and so no acpi_osi= params
   would have an effect here, since they'd not be tested.

acpi_osi=
   This will keep our default _OS name and thus end up at the 
   "Microsoft Windows NT" case

acpi_osi= acpi_os_name="Microsoft WindowsME: Millennium Edition"
    this will take us to the ME case

acpi_osi="!Windows 2001.1"
    this will take us to the Windows 2001 SP1 case

acpi_osi="!Windows 2001.1" acpi_osi="!Windows 2001 SP1"
    this will take us to the Windows 2001 case
Comment 32 Zhang Rui 2008-01-14 22:01:48 UTC
If WNTF is set, the _STA method of BAT1 will return 0, which means the ACPI device will not be added to the Linux device tree. And we need the battery hotplug support at this time.
So IMO,
acpi_os_name="Microsoft Windows"
acpi_osi= , acpi_os_name="Microsoft WindowsME: Millennium Edition"
should have the same result as acpi_osi= , and acpi_os_name="Linux".
Comment 33 Len Brown 2008-01-14 22:11:45 UTC
Yep, the ASL reads that way to me too.  (I'm impressed
with how many OS version checks are in this DSDT)

So I think you just established that battery hot-add support
was added to windows at "Microsoft Windows NT" and newer; and
older OS's are assumed to not support that.  Since Linux
claims to support what NT and newer support, we have some
catching up to do...

I'll remove the workaround from the description,
since for a lot of reasons it can't be widely used in practice.
Comment 34 Zhang Rui 2008-01-16 00:07:09 UTC
Created attachment 14472 [details]
patch-battery-hotplug-1
Comment 35 Zhang Rui 2008-01-16 00:07:38 UTC
Created attachment 14473 [details]
patch-battery-hotplug-2
Comment 36 Zhang Rui 2008-01-16 00:08:03 UTC
Created attachment 14474 [details]
patch-battery-hotplug-3
Comment 37 Zhang Rui 2008-01-16 00:08:33 UTC
Created attachment 14475 [details]
patch-battery-hotplug-4
Comment 38 Zhang Rui 2008-01-16 00:12:27 UTC
These are the refreshed battery hotplug patches against 2.6.24-rc7

Patch 01 flushes kacpi_notify_wq before removing the notify handler
         as well as the kacpid_wq.
Patch 02 adds a new acpi execute type, OSL_DEVICE_HOTPLUG_HANDLER,
         This is used for the device hotplug notify handler which will be
         queued in keventd_wq so that it can flush the kacpi_notify_wq.
Patch 03 support device hotplug in generic code.
         Only battery is supported for now.
Patch 04 enable the "eject" sysfs attribute which is used to evaluate _EJx
         method and eject a device in user space.
Comment 39 Mike Kelly 2008-02-16 05:37:08 UTC
I have two questions. First, has a version of this been committed somewhere? And, if not, should this bug be reopened?

Secondly, I know that many battery monitoring apps are able to read the "present" key from proc or sysfs. But, I don't think many of them will check for new battery devices added after they are started.

I've confirmed that at least xfce4-battery-plugin doesn't work properly if I insert a new battery after it has started when using this patch. I have to reload it.

Should all the programs that rely on just the present attribute be changed? Or would your patch need to change somehow?
Comment 40 Len Brown 2008-03-25 17:21:16 UTC
this patch series was in the acpi test tree,
but were dropped due to merge conflicts
and also a proposal for a simpler solution.
Comment 41 Shaohua 2008-05-06 18:37:56 UTC
I'm thinking of handling this in acpi dock driver in a unified way for sata/battery/dock.
Comment 42 Andi Kleen 2008-08-15 22:55:59 UTC
Shaohua, what's the status of this? Thanks
Comment 43 Shaohua 2008-08-17 19:25:55 UTC
I sent a patch set for bay/battery/dock hotplug several weeks ago, but it appears lost. Andi, should I repost them?
Comment 44 Andi Kleen 2008-08-17 19:27:26 UTC
Yes please do.
Comment 45 Shaohua 2008-09-23 22:36:15 UTC
Len said the patches are in his -test tree now.
Comment 46 Len Brown 2008-10-16 23:19:22 UTC
*** Bug 9731 has been marked as a duplicate of this bug. ***
Comment 47 Len Brown 2008-10-17 00:05:32 UTC
the "dock" branch is queued for linux-2.6.28:

commit 0a918a9432cc30aede10f904253b66ea6ab485ac
Author: Thomas Renninger <trenn@suse.de>
Date:   Sat Oct 11 00:15:04 2008 -0400

    Subject: ACPI dock: Use ACPI_EXCEPTION instead of printk(KERN_ERR
    
    lenb: stripped patch down to what still applied to new dock.c
    
    Signed-off-by: Thomas Renninger <trenn@suse.de>
    Signed-off-by: Len Brown <len.brown@intel.com>

commit c5d191b8e531e33b823242f3d2c6b81d765e96dd
Author: Len Brown <len.brown@intel.com>
Date:   Wed Sep 24 02:53:25 2008 -0400

    dock: Shaohua Li is new maintainer
    
    Signed-off-by: Len Brown <len.brown@intel.com>

commit 8652b00fd6416773f113dab3dfa0d4509def825b
Author: Shaohua Li <shaohua.li@intel.com>
Date:   Thu Aug 28 10:07:45 2008 +0800

    dock: add 'type' sysfs file
    
    add a sysfs file to present dock type. Suggested by Holger.
    
    Signed-off-by: Shaohua Li <shaohua.li@intel.com>
    Signed-off-by: Len Brown <len.brown@intel.com>

commit 61b836958371c717d1e6d4fea1d2c512969ad20b
Author: Shaohua Li <shaohua.li@intel.com>
Date:   Thu Aug 28 10:07:14 2008 +0800

    dock: fix for ATA bay in a dock station
    
    an ATA bay can be in a dock and itself can be ejected separately.
    This patch handles such eject bay. Found by Holger.
    
    Signed-off-by: Shaohua Li <shaohua.li@intel.com>
    Signed-off-by: Len Brown <len.brown@intel.com>

commit 4be9309d15e88e4a1e4a78deb52eb88c7da38c99
Author: Shaohua Li <shaohua.li@intel.com>
Date:   Thu Aug 28 10:06:44 2008 +0800

    bay: remove driver, all functions now handled by dock driver
    
    Signed-off-by: Shaohua Li <shaohua.li@intel.com>
    Signed-off-by: Len Brown <len.brown@intel.com>

commit 1253f7aabfebc51446dbec5c8895c5c8846dfe06
Author: Shaohua Li <shaohua.li@intel.com>
Date:   Thu Aug 28 10:06:16 2008 +0800

    dock: introduce .uevent for devices in dock, eg libata
    
    dock's uevent reported itself, not ata. It might be difficult to find an
    ata device just according to a dock. This patch introduces docking ops
    for each device in a dock. when docking, dock driver can send device
    specific uevent. This should help dock station too (not just bay)
    
    Signed-off-by: Shaohua Li <shaohua.li@intel.com>
    Acked-by: Tejun Heo <htejun@gmail.com>
    Signed-off-by: Len Brown <len.brown@intel.com>

commit f730ae1838635a02aa60834762c61566911d004c
Author: Shaohua Li <shaohua.li@intel.com>
Date:   Thu Aug 28 10:05:45 2008 +0800

    libata: remove functions now handed by ACPI dock driver
    
    dock driver can handle ata(bay) hotplug now. dock driver already handles
    _EJ0 and _STA, so remove them. Also libata doesn't need register
    notification handler anymore.
    
    Signed-off-by: Shaohua Li <shaohua.li@intel.com>
    Acked-by: Tejun Heo <htejun@gmail.com>
    Signed-off-by: Len Brown <len.brown@intel.com>

commit 19cd847ab24fefe9e50101ec94479e0400a08650
Author: Zhang Rui <rui.zhang@intel.com>
Date:   Thu Aug 28 10:05:06 2008 +0800

    ACPI: fix hotplug race
    
    The hotplug notification handler and drivers' notification handler all
    run in one workqueue.  Before hotplug removes an acpi device, the
    device driver's notification handler is already be recorded to run just
    after global notification handler.  After hotplug notification handler
    runs, acpica will notice a NULL notification handler and crash.
    
    So now we run run hotplug in another workqueue and wait
    for all acpi notication handlers finish.
    This was found in battery hotplug, but actually all
    hotplug can be affected.
    
    Signed-off-by: Zhang Rui <rui.zhang@intel.com>
    Signed-off-by: Shaohua Li <shaohua.li@intel.com>
    Signed-off-by: Len Brown <len.brown@intel.com>

commit 6bd00a61ab63d4ceb635ae0316353c11c900b8d8
Author: Shaohua Li <shaohua.li@intel.com>
Date:   Thu Aug 28 10:04:29 2008 +0800

    ACPI: introduce notifier change to avoid duplicates
    
    The battery driver already registers notification handler.
    To avoid registering notification handler again,
    introduce a notifier chain in global system notifier handler
    and use it in dock driver.
    
    Signed-off-by: Shaohua Li <shaohua.li@intel.com>
    Signed-off-by: Len Brown <len.brown@intel.com>

commit db350b084dc2cf816288643861ce07b0562dd723
Author: Shaohua Li <shaohua.li@intel.com>
Date:   Thu Aug 28 10:03:58 2008 +0800

    dock: add bay and battery hotplug support
    
    Make the dock driver support bay and battery hotplug.
    They are all regarded as dock, so handling can be unified.
    
    Signed-off-by: Shaohua Li <shaohua.li@intel.com>
    Signed-off-by: Len Brown <len.brown@intel.com>

commit 406f692d0803d73acd3984c1e11719d3a913fd5e
Author: Shaohua Li <shaohua.li@intel.com>
Date:   Thu Aug 28 10:03:26 2008 +0800

    dock: add _LCK support
    
    support _LCK method, which is a optional method for hotplug
    
    lenb: we have not seen _LCK used in the field yet
    
    Signed-off-by: Shaohua Li <shaohua.li@intel.com>
    Signed-off-by: Len Brown <len.brown@intel.com>

commit 82545394e0690aaef446cb262aa5dac0f9c7156e
Author: Shaohua Li <shaohua.li@intel.com>
Date:   Thu Aug 28 10:02:41 2008 +0800

    dock: fix eject request process (2.6.27-rc1 regression)
    
    commit 2a7feab28d3fc060d320eaba192e49dad1079b7e introduces a bug.
    My thinkpad actually will send an eject_request and we should follow the
    eject process to finish the eject, otherwise system still thinks the bay
    is present.
    
    Signed-off-by: Shaohua Li <shaohua.li@intel.com>
    Signed-off-by: Len Brown <len.brown@intel.com>
commit 8b59560a3baf2e7c24e0fb92ea5d09eca92805db
Author: Shaohua Li <shaohua.li@intel.com>
Date:   Thu Aug 28 10:02:03 2008 +0800

    ACPI: dock: avoid check _STA method
    
    In some BIOSes, every _STA method call will send a notification again,
    this cause freeze. And in some BIOSes, it appears _STA should be called
    after _DCK. This tries to avoid calls _STA, and still keep the device
    present check.
    
    http://bugzilla.kernel.org/show_bug.cgi?id=10431
    
    Signed-off-by: Shaohua Li <shaohua.li@intel.com>
    Signed-off-by: Len Brown <len.brown@intel.com>
Comment 48 Len Brown 2008-10-27 19:20:34 UTC
shipped in linux-2.6.28-rc1
closed

Note You need to log in before you can comment on or make changes to this bug.