ACPI: EC: make boot_ec fully operational

From: Alexey Starikovskiy <[email protected]>

Initialize all fields of boot_ec, so it does not require switch to DSDT found one (fix for #8909).

---

 drivers/acpi/ec.c |   52
++++++++++++++++++++++++++--------------------------
 1 files changed, 26 insertions(+), 26 deletions(-)

index 56bee9e..33afcd1 100644
@@ -664,6 +664,16 @@ acpi_ec_register_query_methods(acpi_handle handle,

u32 level,

     return AE_OK;

}

+static void
+ec_complete_parse_device(struct acpi_ec *ec)
+{
+ /* Find and register all query methods */
+ acpi_walk_namespace(ACPI_TYPE_METHOD, ec->handle, 1,
+ acpi_ec_register_query_methods, ec, NULL);
+ /* Use the global lock for all EC transactions? */
+ acpi_evaluate_integer(ec->handle, "_GLK", NULL, &ec->global_lock);
+}
+
static acpi_status
ec_parse_device(acpi_handle handle, u32 Level, void *context, void
**retval)
{
@@ -680,19 +690,10 @@ ec_parse_device(acpi_handle handle, u32 Level, void *context, void **retval)

     status = acpi_evaluate_integer(handle, "_GPE", NULL, &ec->gpe);
     if (ACPI_FAILURE(status))
         return status;

-
- /* Find and register all query methods */
- acpi_walk_namespace(ACPI_TYPE_METHOD, handle, 1,
- acpi_ec_register_query_methods, ec, NULL);
-
- /* Use the global lock for all EC transactions? */
- acpi_evaluate_integer(handle, "_GLK", NULL, &ec->global_lock);
-

     ec->handle = handle;
-
+    ec_complete_parse_device(ec);
     printk(KERN_INFO PREFIX "GPE = 0x%lx, I/O: command/status = 0x%lx,
data = 0x%lx\n",
               ec->gpe, ec->command_addr, ec->data_addr);
-
     return AE_CTRL_TERMINATE;

}

@@ -710,10 +711,15 @@ static int acpi_ec_add(struct acpi_device *device)

     if (!device)
         return -EINVAL;
-
     strcpy(acpi_device_name(device), ACPI_EC_DEVICE_NAME);
     strcpy(acpi_device_class(device), ACPI_EC_CLASS);

+ /* Check if we found the boot EC */
+ if (boot_ec && boot_ec->handle == device->handle) {
+ ec = boot_ec;
+ goto end;
+ }
+

     ec = make_acpi_ec();
     if (!ec)
         return -ENOMEM;
@@ -723,21 +729,11 @@ static int acpi_ec_add(struct acpi_device *device)
         kfree(ec);
         return -EINVAL;
     }

-
- /* Check if we found the boot EC */
- if (boot_ec) {
- if (boot_ec->gpe == ec->gpe) {

-            ec_remove_handlers(boot_ec);
-            mutex_destroy(&boot_ec->lock);
-            kfree(boot_ec);
-            first_ec = boot_ec = NULL;

- }
- }
+ ec->handle = device->handle;
+ end:

     if (!first_ec)
         first_ec = ec;
-    ec->handle = device->handle;
     acpi_driver_data(device) = ec;
-
     acpi_ec_add_fs(device);
     return 0;

}
@@ -824,8 +820,9 @@ static int acpi_ec_start(struct acpi_device *device)

     if (!ec)
         return -EINVAL;

-
- ret = ec_install_handlers(ec);
+
+ if (ec != boot_ec)
+ ret = ec_install_handlers(ec);

     /* EC is fully operational, allow queries */
     atomic_set(&ec->query_pending, 0);
@@ -866,7 +863,10 @@ int __init acpi_ec_ecdt_probe(void)
         boot_ec->command_addr = ecdt_ptr->control.address;
         boot_ec->data_addr = ecdt_ptr->data.address;
         boot_ec->gpe = ecdt_ptr->gpe;
-        boot_ec->handle = ACPI_ROOT_OBJECT;
+        status = acpi_get_handle(NULL, ecdt_ptr->id, &boot_ec->handle);
+        if (ACPI_FAILURE(status))
+            goto error;
+        ec_complete_parse_device(boot_ec);
     } else {
         printk(KERN_DEBUG PREFIX "Look up EC in DSDT\n");
         status = acpi_get_devices(ec_device_ids[0].id, ec_parse_device,

From: Alexey Starikovskiy <[email protected]>

Try to recover boot EC init in case of broken ECDT (bug #8651).

---

 drivers/acpi/ec.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

index 33afcd1..ce9e97a 100644
@@ -864,17 +864,20 @@ int __init acpi_ec_ecdt_probe(void)
         boot_ec->data_addr = ecdt_ptr->data.address;
         boot_ec->gpe = ecdt_ptr->gpe;
         status = acpi_get_handle(NULL, ecdt_ptr->id, &boot_ec->handle);
-        if (ACPI_FAILURE(status))
+        if (ACPI_FAILURE(status)) {
+            printk(KERN_ERR PREFIX "ECDT is broken\n");
             goto error;
+        }
         ec_complete_parse_device(boot_ec);
-    } else {
+    }
+    if (ACPI_FAILURE(status)) {
+        /* Try to recover boot_ec init */
         printk(KERN_DEBUG PREFIX "Look up EC in DSDT\n");
         status = acpi_get_devices(ec_device_ids[0].id, ec_parse_device,
-                        boot_ec, NULL);
+                      boot_ec, NULL);
         if (ACPI_FAILURE(status))
             goto error;
     }
-
     ret = ec_install_handlers(boot_ec);
     if (!ret) {
         first_ec = boot_ec;

I think this is
1. guaranteed to happen
2. guaranteed to alarm users and be viewed as a regression.

They have no clue what an ECDT is, and this warning doesn't tell them what to do about it, if anything.

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

No, it does not happen on machine with correct ECDT, e.g. my TP is ok.

There is a message, that we are trying to get boot EC from just found ECDT. If it fails, we tell why.

Let me clarify...
It is guaranteed to happen on _some_ system out in the field that belongs to somebody who doesn't know what an ECDT is, yes?

It tells _you_ why, but the user hasn't a clue.
If it said something like

ACPI: ec: BIOS bug: mumble mumble abc is corrupt, ignoring it

then you'd learn what you want, the the user would know what they need to know -- that Linux thinks their BIOS has a bug, but is continuing on. The user then can check for a new BIOS if they like, but will not send their distro mail about the "broken ECDT" which they think might be related to the Extra Critical Data Transfer feature that
if broken is surely important and worth reporting...

does this make sense?

thanks,
-Len