ACPI / dock: ThinkPad X220: \_SB_.PCI0.LPC_.EC__.BAT1: Unable to dock!

PATCH Paul Bolle 3 participants 8 messages Jul 29, 2014 – Aug 13, 2014 ACPI

Share & tools

Paul Bolle

author

2014-07-29 21:51 UTC #0 lore raw

0) A ThinkPad X220 I use prints this error at the _first_ resume from suspend (to ram):

    ACPI: \_SB_.PCI0.LPC_.EC__.BAT1: Unable to dock!

1) BAT1 is apparently a "battery bay". Rubin suggested that it is a "slice" battery. I do not use a "slice" battery. (There are two other possible docks according to the ACPI printks is get. An "ata bay" and a "dock station". I don't use those either, but the kernel at least prints no errors about them.)

2) I've peeked a bit at the DSDT of this machine. Its _WAK method contains this line:

    \_SB.PCI0.LPC.EC.BATW (Arg0)

That BATW method is about 12 lines long. (Can I quote that in full?) It ends with this snippet:

    If (XOr (Local0, Local1))
    {
        Store (Local1, [...])
        Notify (\_SB.PCI0.LPC.EC.BAT1, 0x01) // Device Check
    }

3) That ACPI_NOTIFY_DEVICE_CHECK for BAT1 apparently triggers this call chain:
acpi_bus_notify()
acpi_hotplug_schedule()
acpi_hotplug_work_fn()
acpi_device_hotplug()
dock_notify()

4) Perhaps Lenovo needs to update the ACPI part of this laptop's firmware. (I'm running the latest firmware.) Does Lenovo listen to suggestions to update its firmware?

5) A workaround in the kernel _might_ be something utterly untested like this:

drivers/acpi/dock.c +9 -4

index d9339b442a4e..9e5856a6adfb 100644
@@ -333,7 +333,7 @@ static void dock_event(struct dock_station *ds, u32 event, int num)
  *
  * Execute the _DCK method in response to an acpi event
  */
-static void handle_dock(struct dock_station *ds, int dock)
+static acpi_status handle_dock(struct dock_station *ds, int dock)
 {
 	acpi_status status;
 	struct acpi_object_list arg_list;

@@ -351,11 +351,12 @@ static void handle_dock(struct dock_station *ds, int dock)
 	if (ACPI_FAILURE(status) && status != AE_NOT_FOUND)
 		acpi_handle_err(ds->handle, "Failed to execute _DCK (0x%x)\n",
 				status);
+	return status;
 }
 
-static inline void dock(struct dock_station *ds)
+static inline acpi_status dock(struct dock_station *ds)
 {
-	handle_dock(ds, 1);
+	return handle_dock(ds, 1);
 }
 
 static inline void undock(struct dock_station *ds)

@@ -471,8 +472,12 @@ int dock_notify(struct acpi_device *adev, u32 event)
 	case ACPI_NOTIFY_BUS_CHECK:
 	case ACPI_NOTIFY_DEVICE_CHECK:
 		if (!dock_in_progress(ds) && !acpi_device_enumerated(adev)) {
+			acpi_status status;
+
 			begin_dock(ds);
-			dock(ds);
+			status = dock(ds);
+			if (status == AE_NOT_FOUND)
+				break;
 			if (!dock_present(ds)) {
 				acpi_handle_err(handle, "Unable to dock!\n");
 				complete_dock(ds);

Please note that BAT1 does have a _EJ0 method but doesn't have a _DCK method.

6) Any suggestions?

Paul Bolle

author

2014-07-29 22:02 UTC #1 lore raw

Shaohua,

Could you please update your email address in MAINTAINERS?

Thanks,

Paul Bolle

Quoted 87 lines · Paul Bolle

0) A ThinkPad X220 I use prints this error at the _first_ resume from suspend (to ram):
    ACPI: \_SB_.PCI0.LPC_.EC__.BAT1: Unable to dock!
1) BAT1 is apparently a "battery bay". Rubin suggested that it is a "slice" battery. I do not use a "slice" battery. (There are two other possible docks according to the ACPI printks is get. An "ata bay" and a "dock station". I don't use those either, but the kernel at least prints no errors about them.)
2) I've peeked a bit at the DSDT of this machine. Its _WAK method contains this line:
    \_SB.PCI0.LPC.EC.BATW (Arg0)
That BATW method is about 12 lines long. (Can I quote that in full?) It ends with this snippet:
    If (XOr (Local0, Local1))
    {
        Store (Local1, [...])
        Notify (\_SB.PCI0.LPC.EC.BAT1, 0x01) // Device Check
    }
3) That ACPI_NOTIFY_DEVICE_CHECK for BAT1 apparently triggers this call chain:
acpi_bus_notify()
acpi_hotplug_schedule()
acpi_hotplug_work_fn()
acpi_device_hotplug()
dock_notify()
4) Perhaps Lenovo needs to update the ACPI part of this laptop's firmware. (I'm running the latest firmware.) Does Lenovo listen to suggestions to update its firmware?
5) A workaround in the kernel _might_ be something utterly untested like this:
drivers/acpi/dock.c +9 -4
index d9339b442a4e..9e5856a6adfb 100644
@@ -333,7 +333,7 @@ static void dock_event(struct dock_station *ds, u32 event, int num)
  *
  * Execute the _DCK method in response to an acpi event
  */
-static void handle_dock(struct dock_station *ds, int dock)
+static acpi_status handle_dock(struct dock_station *ds, int dock)
 {
 	acpi_status status;
 	struct acpi_object_list arg_list;
@@ -351,11 +351,12 @@ static void handle_dock(struct dock_station *ds, int dock)
 	if (ACPI_FAILURE(status) && status != AE_NOT_FOUND)
 		acpi_handle_err(ds->handle, "Failed to execute _DCK (0x%x)\n",
 				status);
+	return status;
 }
 
-static inline void dock(struct dock_station *ds)
+static inline acpi_status dock(struct dock_station *ds)
 {
-	handle_dock(ds, 1);
+	return handle_dock(ds, 1);
 }
 
 static inline void undock(struct dock_station *ds)
@@ -471,8 +472,12 @@ int dock_notify(struct acpi_device *adev, u32 event)
 	case ACPI_NOTIFY_BUS_CHECK:
 	case ACPI_NOTIFY_DEVICE_CHECK:
 		if (!dock_in_progress(ds) && !acpi_device_enumerated(adev)) {
+			acpi_status status;
+
 			begin_dock(ds);
-			dock(ds);
+			status = dock(ds);
+			if (status == AE_NOT_FOUND)
+				break;
 			if (!dock_present(ds)) {
 				acpi_handle_err(handle, "Unable to dock!\n");
 				complete_dock(ds);
Please note that BAT1 does have a _EJ0 method but doesn't have a _DCK method.
6) Any suggestions?
Paul Bolle

Rafael J. Wysocki

2014-07-29 22:04 UTC #2 lore raw

In reply to Paul Bolle

Quoted 80 lines · Paul Bolle

0) A ThinkPad X220 I use prints this error at the _first_ resume from suspend (to ram):
    ACPI: \_SB_.PCI0.LPC_.EC__.BAT1: Unable to dock!
1) BAT1 is apparently a "battery bay". Rubin suggested that it is a "slice" battery. I do not use a "slice" battery. (There are two other possible docks according to the ACPI printks is get. An "ata bay" and a "dock station". I don't use those either, but the kernel at least prints no errors about them.)
2) I've peeked a bit at the DSDT of this machine. Its _WAK method contains this line:
    \_SB.PCI0.LPC.EC.BATW (Arg0)
That BATW method is about 12 lines long. (Can I quote that in full?) It ends with this snippet:
    If (XOr (Local0, Local1))
    {
        Store (Local1, [...])
        Notify (\_SB.PCI0.LPC.EC.BAT1, 0x01) // Device Check
    }
3) That ACPI_NOTIFY_DEVICE_CHECK for BAT1 apparently triggers this call chain:
acpi_bus_notify()
acpi_hotplug_schedule()
acpi_hotplug_work_fn()
acpi_device_hotplug()
dock_notify()
4) Perhaps Lenovo needs to update the ACPI part of this laptop's firmware. (I'm running the latest firmware.) Does Lenovo listen to suggestions to update its firmware?
5) A workaround in the kernel _might_ be something utterly untested like this:
drivers/acpi/dock.c +9 -4
index d9339b442a4e..9e5856a6adfb 100644
@@ -333,7 +333,7 @@ static void dock_event(struct dock_station *ds, u32 event, int num)
  *
  * Execute the _DCK method in response to an acpi event
  */
-static void handle_dock(struct dock_station *ds, int dock)
+static acpi_status handle_dock(struct dock_station *ds, int dock)
 {
 	acpi_status status;
 	struct acpi_object_list arg_list;
@@ -351,11 +351,12 @@ static void handle_dock(struct dock_station *ds, int dock)
 	if (ACPI_FAILURE(status) && status != AE_NOT_FOUND)
 		acpi_handle_err(ds->handle, "Failed to execute _DCK (0x%x)\n",
 				status);
+	return status;
 }
 
-static inline void dock(struct dock_station *ds)
+static inline acpi_status dock(struct dock_station *ds)
 {
-	handle_dock(ds, 1);
+	return handle_dock(ds, 1);
 }
 
 static inline void undock(struct dock_station *ds)
@@ -471,8 +472,12 @@ int dock_notify(struct acpi_device *adev, u32 event)
 	case ACPI_NOTIFY_BUS_CHECK:
 	case ACPI_NOTIFY_DEVICE_CHECK:
 		if (!dock_in_progress(ds) && !acpi_device_enumerated(adev)) {
+			acpi_status status;
+
 			begin_dock(ds);
-			dock(ds);
+			status = dock(ds);
+			if (status == AE_NOT_FOUND)
+				break;
 			if (!dock_present(ds)) {
 				acpi_handle_err(handle, "Unable to dock!\n");
 				complete_dock(ds);

Quoted 4 lines

Please note that BAT1 does have a _EJ0 method but doesn't have a _DCK method.
6) Any suggestions?

Let me take a look at that, but I'll need some time.

Can you send me the output of acpidump from the machine in question, please?

Rafael

Paul Bolle

author

2014-07-29 22:09 UTC #3 lore raw

Quoted 1 line · Rafael J. Wysocki

Let me take a look at that, but I'll need some time.

Do take your time. It's mostly a cosmetic issue (ie, an unneeded error) that I've tolerated ever since I use this machine (which is ten months).

Quoted 1 line

Can you send me the output of acpidump from the machine in question, please?

Off list, I guess? And in what format?

Paul Bolle

Rafael J. Wysocki

2014-07-29 22:17 UTC #4 lore raw

Quoted 9 lines · Paul Bolle

Quoted 1 line · Rafael J. Wysocki
Let me take a look at that, but I'll need some time.
Do take your time. It's mostly a cosmetic issue (ie, an unneeded error) that I've tolerated ever since I use this machine (which is ten months).
Quoted 1 line
Can you send me the output of acpidump from the machine in question, please?
Off list, I guess? And in what format?

It's not useful to send it to the list I think and please keep the original format.

Rafael

Paul Bolle

author

2014-08-13 09:48 UTC #5 lore raw

In reply to Rafael J. Wysocki

Quoted 1 line · Rafael J. Wysocki

Can you send me the output of acpidump from the machine in question, please?

0) That I've done some time ago. In the mean time I've decompiled the dsdt.dat that acpidump generates. Here are my (rather verbose) notes, that I mostly post to archive them somewhere public.

1) The interesting parts of dsdt.dsl are:
DefinitionBlock ("dsdt.aml", "DSDT", 1, "LENOVO", "TP-8D ", 0x00001390)

{
    [...]
    Scope (\_SB)
    {
        Method (_INI, 0, NotSerialized)  // _INI: Initialize
        {
            [...]
            If (LGreaterEqual (\_REV, 0x02))
            {
                Store (0x01, \H8DR)
            }
            [...]
        }
        [...]
        Device (PCI0)
        {
            [...]
            Device (LPC)
            {
                [...]
                Device (EC)
                {
                    [...]
                    Field (ECOR, ByteAcc, NoLock, Preserve)
                    {
                        [...]
                        HB1A,   1, 
                        [...]
                    }
                    [...]
                    Method (BATW, 1, NotSerialized)
                    {
                        Store (\_SB.PCI0.LPC.EC.BAT1.XB1S, Local0)
                        If (\H8DR)
                        {
                            Store (HB1A, Local1)
                        }
                        Else
                        {
                            [...]
                        }

                        If (XOr (Local0, Local1))
                        {
                            Store (Local1, \_SB.PCI0.LPC.EC.BAT1.XB1S)
                            Notify (\_SB.PCI0.LPC.EC.BAT1, 0x01) // Device Check
                        }
                    }
                    [...]
                    Device (BAT1)
                    {
                        [...]
                        Name (XB1S, 0x01)
                        [...]
                        Method (_EJ0, 1, NotSerialized)  // _EJx: Eject Device
                        {
                            If (Arg0)
                            {
                                [...]
                                Store (0x00, XB1S) /* \_SB_.PCI0.LPC_.EC__.BAT1.XB1S */
                            }
                        }
                    }
                    [...]
                }
            }
            [...]
        }
        [...]
    }
    [...]
    Method (\_WAK, 1, NotSerialized)  // _WAK: Wake
    {
        [...]
        \_SB.PCI0.LPC.EC.BATW (Arg0)
        [...]
    }
    [...]
    Name (H8DR, 0x00)
    [...]
}

2) Some guesswork:
- HB1A is always 1;
- H8DR will be 0x01 (because Linux currently sets _REV at 5);
- XB1S is apparently 0x00 at first resume (I don't know how that happens);
- so "XOr ([HB1A], [XB1S])" evaluates to 1 at the first "Wake" and XB1S will then be set to 1 and a "Device Check" event is fired;
- it's this "Device Check" event that triggers the error I reported;
- because HB1A and XB1S are equal after the BATW method has run at first resume we will not get a "Device Check" event at subsequent resumes.

Paul Bolle

author

2014-08-13 11:49 UTC #6 lore raw

Quoted 2 lines · Paul Bolle

- XB1S is apparently 0x00 at first resume (I don't know how that happens);

Perhaps because this battery's _EJ0 method was evaluated previously (say, during suspend):
DefinitionBlock ("dsdt.aml", "DSDT", 1, "LENOVO", "TP-8D ", 0x00001390)

{
    Scope (\_SB)
    {
        Device (PCI0)
        {
            Device (LPC)
            {
                Device (EC)
                {
                    Device (BAT1)
                    {
                        Method (_EJ0, 1, NotSerialized)  // _EJx: Eject Device
                        {
                            If (Arg0)
                            {
                                [...]
                                Store (0x00, XB1S) /* \_SB_.PCI0.LPC_.EC__.BAT1.XB1S */
                            }
                        }
                    }
                }
            }
        }
    }
}

But I don't know whether that _EJ0 method is actually evaluated during the first boot and suspend cycle.

Paul Bolle

Henrique de Moraes Holschuh

2014-07-29 23:39 UTC #7 lore raw

In reply to Paul Bolle

Quoted 3 lines · Paul Bolle

4) Perhaps Lenovo needs to update the ACPI part of this laptop's firmware. (I'm running the latest firmware.) Does Lenovo listen to suggestions to update its firmware?

Yes, they do. Post a detailed report about this to the Linux-ThinkPad ML, with a suitable subject line. If you're lucky, one of the lurkers @lenovo that read that list will do something about it.

Signature · 4 lines

  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh