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

Message ID 1406670374.2406.27.camel@x220
State Not Applicable, archived
Headers show

Commit Message

Paul Bolle July 29, 2014, 9:46 p.m. UTC
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:


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

6) Any suggestions?


Paul Bolle

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Paul Bolle July 29, 2014, 10:02 p.m. UTC | #1
Shaohua,

Could you please update your email address in MAINTAINERS?

Thanks,


Paul Bolle

On Tue, 2014-07-29 at 23:46 +0200, Paul Bolle wrote:
> 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:
> 
> diff --git a/drivers/acpi/dock.c b/drivers/acpi/dock.c
> index d9339b442a4e..9e5856a6adfb 100644
> --- a/drivers/acpi/dock.c
> +++ b/drivers/acpi/dock.c
> @@ -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


--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paul Bolle July 29, 2014, 10:09 p.m. UTC | #2
On Wed, 2014-07-30 at 00:23 +0200, Rafael J. Wysocki wrote:
> 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).

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

Off list, I guess? And in what format?


Paul Bolle

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rafael J. Wysocki July 29, 2014, 10:23 p.m. UTC | #3
On Tuesday, July 29, 2014 11:46:14 PM Paul Bolle wrote:
> 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:
> 
> diff --git a/drivers/acpi/dock.c b/drivers/acpi/dock.c
> index d9339b442a4e..9e5856a6adfb 100644
> --- a/drivers/acpi/dock.c
> +++ b/drivers/acpi/dock.c
> @@ -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?

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

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rafael J. Wysocki July 29, 2014, 10:36 p.m. UTC | #4
On Wednesday, July 30, 2014 12:09:29 AM Paul Bolle wrote:
> On Wed, 2014-07-30 at 00:23 +0200, Rafael J. Wysocki wrote:
> > 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).
> 
> > 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

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Henrique de Moraes Holschuh July 29, 2014, 11:39 p.m. UTC | #5
On Tue, 29 Jul 2014, Paul Bolle wrote:
> 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.
Paul Bolle Aug. 13, 2014, 9:48 a.m. UTC | #6
On Wed, 2014-07-30 at 00:23 +0200, Rafael J. Wysocki wrote:
> 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

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paul Bolle Aug. 13, 2014, 11:49 a.m. UTC | #7
On Wed, 2014-08-13 at 11:48 +0200, Paul Bolle wrote:
> - 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

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/drivers/acpi/dock.c b/drivers/acpi/dock.c
index d9339b442a4e..9e5856a6adfb 100644
--- a/drivers/acpi/dock.c
+++ b/drivers/acpi/dock.c
@@ -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);