Re: [PATCH] ACPI / EC: Add fully functioning ECDT EC

[Luya Tshimbalanga]

Below is the patch


From: Lv Zheng <lv.zheng(a)intel.com&gt;
Subject: [PATCH] ACPI / EC: Add fully functioning ECDT EC

It is possible to register _Qxx from namespace and use the ECDT EC to
do further handling. The reported bug reveals that Windows is using
ECDT in this way.
This patch extends Linux to support ECDT in this way.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=115021
Reported-by: Peter Wu <peter(a)lekensteyn.nl&gt;
Reported-by: Luya Tshimbalanga <luya(a)fedoraproject.org&gt;
Signed-off-by: Lv Zheng <lv.zheng(a)intel.com&gt;
---
Index: linux-acpica/drivers/acpi/ec.c
===================================================================
--- linux-acpica.orig/drivers/acpi/ec.c
+++ linux-acpica/drivers/acpi/ec.c
@@ -112,6 +112,8 @@ enum {
 	EC_FLAGS_STOPPED,		/* Driver is stopped */
 	EC_FLAGS_COMMAND_STORM,		/* GPE storms occurred to the
 					 * current command processing */
+	EC_FLAGS_QUERY_HANDLER_INSTALLED,
+					/* _Qxx handlers installed */
 };
 
 #define ACPI_EC_COMMAND_POLL		0x01 /* Available for command byte */
@@ -1228,6 +1230,15 @@ acpi_ec_space_handler(u32 function, acpi
 static acpi_status
 ec_parse_io_ports(struct acpi_resource *resource, void *context);
 
+static void free_acpi_ec(struct acpi_ec *ec)
+{
+	if (first_ec == ec)
+		first_ec = NULL;
+	if (boot_ec == ec)
+		boot_ec = NULL;
+	kfree(ec);
+}
+
 static struct acpi_ec *make_acpi_ec(void)
 {
 	struct acpi_ec *ec = kzalloc(sizeof(struct acpi_ec), GFP_KERNEL);
@@ -1290,7 +1301,7 @@ ec_parse_device(acpi_handle handle, u32 
 	return AE_CTRL_TERMINATE;
 }
 
-static int ec_install_handlers(struct acpi_ec *ec)
+static int ec_install_handlers(struct acpi_ec *ec, bool handle_events)
 {
 	acpi_status status;
 
@@ -1319,6 +1330,9 @@ static int ec_install_handlers(struct ac
 		set_bit(EC_FLAGS_EC_HANDLER_INSTALLED, &ec->flags);
 	}
 
+	if (!handle_events)
+		return 0;
+
 	if (!test_bit(EC_FLAGS_GPE_HANDLER_INSTALLED, &ec->flags)) {
 		status = acpi_install_gpe_raw_handler(NULL, ec->gpe,
 					  ACPI_GPE_EDGE_TRIGGERED,
@@ -1331,7 +1345,16 @@ static int ec_install_handlers(struct ac
 				acpi_ec_enable_gpe(ec, true);
 		}
 	}
+	if (!test_bit(EC_FLAGS_QUERY_HANDLER_INSTALLED, &ec->flags)) {
+		/* Find and register all query methods */
+		acpi_walk_namespace(ACPI_TYPE_METHOD, ec->handle, 1,
+				    acpi_ec_register_query_methods,
+				    NULL, ec, NULL);
+		set_bit(EC_FLAGS_QUERY_HANDLER_INSTALLED, &ec->flags);
+	}
 
+	/* EC is fully operational, allow queries */
+	clear_bit(EC_FLAGS_QUERY_PENDING, &ec->flags);
 	return 0;
 }
 
@@ -1365,21 +1388,35 @@ static void ec_remove_handlers(struct ac
 	}
 }
 
-static struct acpi_ec *acpi_ec_alloc(void)
+static int acpi_set_boot_ec(struct acpi_ec *ec, bool refresh_globals,
+			    bool handle_events)
 {
-	struct acpi_ec *ec;
+	int ret;
 
-	/* Check for boot EC */
-	if (boot_ec) {
-		ec = boot_ec;
-		boot_ec = NULL;
-		ec_remove_handlers(ec);
-		if (first_ec == ec)
-			first_ec = NULL;
-	} else {
-		ec = make_acpi_ec();
+	/* Release old boot EC */
+	if (refresh_globals) {
+		if (boot_ec) {
+			ec_remove_handlers(boot_ec);
+			free_acpi_ec(boot_ec);
+		}
 	}
-	return ec;
+
+	/* Switch boot EC to use new EC */
+	ret = ec_install_handlers(ec, handle_events);
+	if (!ret) {
+		if (refresh_globals) {
+			boot_ec = ec;
+			if (!first_ec)
+				first_ec = ec;
+		}
+		acpi_handle_info(ec->handle, "used as boot EC to handle %s\n",
+				 handle_events ?
+				 "events and transactions" : "transactions");
+		acpi_handle_info(ec->handle,
+				 "GPE=0x%lx, I/O: CMD/SC=0x%lx, DATA=0x%lx\n",
+				 ec->gpe, ec->command_addr, ec->data_addr);
+	}
+	return ret;
 }
 
 static int acpi_ec_add(struct acpi_device *device)
@@ -1390,21 +1427,15 @@ static int acpi_ec_add(struct acpi_devic
 	strcpy(acpi_device_name(device), ACPI_EC_DEVICE_NAME);
 	strcpy(acpi_device_class(device), ACPI_EC_CLASS);
 
-	ec = acpi_ec_alloc();
+	ec = make_acpi_ec();
 	if (!ec)
 		return -ENOMEM;
 	if (ec_parse_device(device->handle, 0, ec, NULL) !=
 		AE_CTRL_TERMINATE) {
-			kfree(ec);
-			return -EINVAL;
+		free_acpi_ec(ec);
+		return -EINVAL;
 	}
 
-	/* Find and register all query methods */
-	acpi_walk_namespace(ACPI_TYPE_METHOD, ec->handle, 1,
-			    acpi_ec_register_query_methods, NULL, ec, NULL);
-
-	if (!first_ec)
-		first_ec = ec;
 	device->driver_data = ec;
 
 	ret = !!request_region(ec->data_addr, 1, "EC data");
@@ -1412,17 +1443,11 @@ static int acpi_ec_add(struct acpi_devic
 	ret = !!request_region(ec->command_addr, 1, "EC cmd");
 	WARN(!ret, "Could not request EC cmd io port 0x%lx", ec->command_addr);
 
-	pr_info("GPE = 0x%lx, I/O: command/status = 0x%lx, data = 0x%lx\n",
-			  ec->gpe, ec->command_addr, ec->data_addr);
-
-	ret = ec_install_handlers(ec);
+	ret = acpi_set_boot_ec(ec, true, true);
 
 	/* Reprobe devices depending on the EC */
 	acpi_walk_dep_device_list(ec->handle);
 
-	/* EC is fully operational, allow queries */
-	clear_bit(EC_FLAGS_QUERY_PENDING, &ec->flags);
-
 	/* Clear stale _Q events if hardware might require that */
 	if (EC_FLAGS_CLEAR_ON_RESUME)
 		acpi_ec_clear(ec);
@@ -1442,9 +1467,7 @@ static int acpi_ec_remove(struct acpi_de
 	release_region(ec->data_addr, 1);
 	release_region(ec->command_addr, 1);
 	device->driver_data = NULL;
-	if (ec == first_ec)
-		first_ec = NULL;
-	kfree(ec);
+	free_acpi_ec(ec);
 	return 0;
 }
 
@@ -1476,13 +1499,45 @@ static const struct acpi_device_id ec_de
 	{"", 0},
 };
 
+static int __init acpi_ec_ecdt_start(void)
+{
+	struct acpi_table_ecdt *ecdt_ptr;
+	acpi_status status;
+	acpi_handle handle;
+	int ret = 0;
+
+	if (!boot_ec)
+		return -ENODEV;
+
+	status = acpi_get_table(ACPI_SIG_ECDT, 1,
+				(struct acpi_table_header **)&ecdt_ptr);
+	if (ACPI_FAILURE(status)) {
+		ret = -ENODEV;
+		goto error;
+	}
+
+	/* At this point, the namespace is initialized */
+	status = acpi_get_handle(NULL, ecdt_ptr->id, &handle);
+	if (ACPI_FAILURE(status)) {
+		ret = -EINVAL;
+		goto error;
+	}
+	boot_ec->handle = handle;
+	ret = acpi_set_boot_ec(boot_ec, false, true);
+
+error:
+	if (ret)
+		acpi_handle_warn(boot_ec->handle, "cannot handle events\n");
+	return ret;
+}
+
 int __init acpi_ec_dsdt_probe(void)
 {
 	acpi_status status;
 	struct acpi_ec *ec;
 	int ret;
 
-	ec = acpi_ec_alloc();
+	ec = make_acpi_ec();
 	if (!ec)
 		return -ENOMEM;
 	/*
@@ -1496,13 +1551,12 @@ int __init acpi_ec_dsdt_probe(void)
 		ret = -ENODEV;
 		goto error;
 	}
-	ret = ec_install_handlers(ec);
-
+	ret = acpi_set_boot_ec(ec, true, true);
 error:
-	if (ret)
-		kfree(ec);
-	else
-		first_ec = boot_ec = ec;
+	if (ret) {
+		free_acpi_ec(ec);
+		ret = acpi_ec_ecdt_start();
+	}
 	return ret;
 }
 
@@ -1577,7 +1631,7 @@ int __init acpi_ec_ecdt_probe(void)
 	struct acpi_table_ecdt *ecdt_ptr;
 	struct acpi_ec *ec;
 
-	ec = acpi_ec_alloc();
+	ec = make_acpi_ec();
 	if (!ec)
 		return -ENOMEM;
 	/*
@@ -1600,7 +1654,6 @@ int __init acpi_ec_ecdt_probe(void)
 		goto error;
 	}
 
-	pr_info("EC description table is found, configuring boot EC\n");
 	if (EC_FLAGS_CORRECT_ECDT) {
 		ec->command_addr = ecdt_ptr->data.address;
 		ec->data_addr = ecdt_ptr->control.address;
@@ -1609,13 +1662,16 @@ int __init acpi_ec_ecdt_probe(void)
 		ec->data_addr = ecdt_ptr->data.address;
 	}
 	ec->gpe = ecdt_ptr->gpe;
+
+	/*
+	 * At this point, the namespace is not initialized, so it is not
+	 * possible to find the namespace object, and handle events.
+	 */
 	ec->handle = ACPI_ROOT_OBJECT;
-	ret = ec_install_handlers(ec);
+	ret = acpi_set_boot_ec(ec, true, false);
 error:
 	if (ret)
-		kfree(ec);
-	else
-		first_ec = boot_ec = ec;
+		free_acpi_ec(ec);
 	return ret;
 }
 


-- 
Luya Tshimbalanga
Graphic & Web Designer
E: luya(a)fedoraproject.org
W: http://www.coolest-storm.net