[PATCH 1/2] ACPI: EC: make boot_ec fully operational

[PATCH 1/2] ACPI: EC: make boot_ec fully operational

[Alexey Starikovskiy]

From: Alexey Starikovskiy <astarikovskiy@suse.de>

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

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

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

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index 56bee9e..33afcd1 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -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,

[PATCH 2/2] ACPI: EC: Fall back to DSDT scan if ECDT is broken

[Alexey Starikovskiy]

From: Alexey Starikovskiy <astarikovskiy@suse.de>

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

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

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

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index 33afcd1..ce9e97a 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -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;

Re: [PATCH 2/2] ACPI: EC: Fall back to DSDT scan if ECDT is broken

[Len Brown]

On Tuesday 21 August 2007 09:22, Alexey Starikovskiy wrote:
> +            printk(KERN_ERR PREFIX "ECDT is broken\n");

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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] ACPI: EC: Fall back to DSDT scan if ECDT is broken

[Alexey Starikovskiy]

Len Brown wrote:
> On Tuesday 21 August 2007 09:22, Alexey Starikovskiy wrote:
>   
>> +            printk(KERN_ERR PREFIX "ECDT is broken\n");
>>     
>
> I think this is
> 1. guaranteed to happen
>   
No, it does not happen on machine with correct ECDT, e.g. my TP  is ok.
> 2. guaranteed to alarm users and be viewed as a regression.
>   
There is a message, that we are trying to get boot EC from just found
ECDT. If it fails, we tell why.
>    They have no clue what an ECDT is, and this warning
>    doesn't tell them what to do about it, if anything.
>
> -Len
>   

Re: [PATCH 2/2] ACPI: EC: Fall back to DSDT scan if ECDT is broken

[Len Brown]

On Thursday 23 August 2007 23:42, Alexey Starikovskiy wrote:
> Len Brown wrote:
> > On Tuesday 21 August 2007 09:22, Alexey Starikovskiy wrote:
> >   
> >> +            printk(KERN_ERR PREFIX "ECDT is broken\n");
> >>     
> >
> > I think this is
> > 1. guaranteed to happen
> >   
> No, it does not happen on machine with correct ECDT, e.g. my TP  is ok.

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?

> > 2. guaranteed to alarm users and be viewed as a regression.
> >   
> There is a message, that we are trying to get boot EC from just found
> ECDT. If it fails, we tell why.

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

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