final set of EC init patches

Hi,

Here is the final set of EC init patches.
Renamed ec_ecdt into boot_ec, hope that's ok...
Regards,
Alex.

Subject: [PATCH 1/9] ACPI: EC: Make EC to initialize first in ACPI
To: Alexey Starikovskiy <[email protected]>
Date: Thu, 01 Mar 2007 01:31:38 +0300
Message-ID: <[email protected]>
User-Agent: StGIT/0.11
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit

From: Alexey Starikovskiy <[email protected]>

---

 drivers/acpi/Makefile |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

index 5956e9f..9623aac 100644
@@ -1,6 +1,6 @@
 #
 # Makefile for the Linux ACPI interpreter
-# 
+#
 
 export ACPI_CFLAGS
 

@@ -32,16 +32,17 @@ #
 processor-objs	+= processor_core.o processor_throttling.o \
 				processor_idle.o processor_thermal.o
 ifdef CONFIG_CPU_FREQ
-processor-objs	+= processor_perflib.o			
+processor-objs	+= processor_perflib.o
 endif
 
 obj-y				+= sleep/
 obj-y				+= bus.o glue.o
 obj-y				+= scan.o
+# Keep EC driver first. Initialization of others depend on it.
+obj-$(CONFIG_ACPI_EC)		+= ec.o
 obj-$(CONFIG_ACPI_AC) 		+= ac.o
 obj-$(CONFIG_ACPI_BATTERY)	+= battery.o
 obj-$(CONFIG_ACPI_BUTTON)	+= button.o
-obj-$(CONFIG_ACPI_EC)		+= ec.o
 obj-$(CONFIG_ACPI_FAN)		+= fan.o
 obj-$(CONFIG_ACPI_DOCK)		+= dock.o
 obj-$(CONFIG_ACPI_BAY)		+= bay.o

Subject: [PATCH 2/9] ACPI: EC: "Fake ECDT" workaround is not needed any longer.
To: Alexey Starikovskiy <[email protected]>
Date: Thu, 01 Mar 2007 01:31:39 +0300
Message-ID: <[email protected]>
In-Reply-To: <[email protected]>
References: <[email protected]>
User-Agent: StGIT/0.11
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit

From: Alexey Starikovskiy <[email protected]>

---

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

Subject: [PATCH 3/9] ACPI: EC: enable burst functionality in EC.
To: Alexey Starikovskiy <[email protected]>
Date: Thu, 01 Mar 2007 01:31:39 +0300
Message-ID: <[email protected]>
In-Reply-To: <[email protected]>
References: <[email protected]>
User-Agent: StGIT/0.11
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit

From: Alexey Starikovskiy <[email protected]>

---

 drivers/acpi/ec.c |   96 ++++++++++++++++++++++-------------------------------
 1 files changed, 40 insertions(+), 56 deletions(-)

Subject: [PATCH 4/9] ACPI: EC: Remove casts to/from void* from ec.c
To: Alexey Starikovskiy <[email protected]>
Date: Thu, 01 Mar 2007 01:31:39 +0300
Message-ID: <[email protected]>
In-Reply-To: <[email protected]>
References: <[email protected]>
User-Agent: StGIT/0.11
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit

From: Alexey Starikovskiy <[email protected]>

---

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

index 816caec..7f50ea1 100644
@@ -402,7 +402,7 @@ static int acpi_ec_query(struct acpi_ec 
 
 static void acpi_ec_gpe_query(void *ec_cxt)
 {
-	struct acpi_ec *ec = (struct acpi_ec *)ec_cxt;
+	struct acpi_ec *ec = ec_cxt;
 	u8 value = 0;
 	char object_name[8];
 

@@ -420,7 +420,7 @@ static u32 acpi_ec_gpe_handler(void *dat
 {
 	acpi_status status = AE_OK;
 	u8 value;
-	struct acpi_ec *ec = (struct acpi_ec *)data;
+	struct acpi_ec *ec = data;
 
 	if (acpi_ec_mode == EC_INTR) {
 		wake_up(&ec->wait);

@@ -464,7 +464,7 @@ acpi_ec_space_handler(u32 function,
 		      void *handler_context, void *region_context)
 {
 	int result = 0;
-	struct acpi_ec *ec = NULL;
+	struct acpi_ec *ec = handler_context;
 	u64 temp = *value;
 	acpi_integer f_v = 0;
 	int i = 0;

@@ -476,8 +476,6 @@ acpi_ec_space_handler(u32 function,
 		return AE_BAD_PARAMETER;
 	}
 
-	ec = (struct acpi_ec *)handler_context;
-
       next_byte:
 	switch (function) {
 	case ACPI_READ:

@@ -533,7 +531,7 @@ static struct proc_dir_entry *acpi_ec_di
 
 static int acpi_ec_read_info(struct seq_file *seq, void *offset)
 {
-	struct acpi_ec *ec = (struct acpi_ec *)seq->private;
+	struct acpi_ec *ec = seq->private;
 
 	if (!ec)
 		goto end;

@@ -689,7 +687,7 @@ static int acpi_ec_remove(struct acpi_de
 static acpi_status
 acpi_ec_io_ports(struct acpi_resource *resource, void *context)
 {
-	struct acpi_ec *ec = (struct acpi_ec *)context;
+	struct acpi_ec *ec = context;
 
 	if (resource->type != ACPI_RESOURCE_TYPE_IO) {
 		return AE_OK;

Subject: [PATCH 5/9] ACPI: EC: Put install handlers into separate function.
To: Alexey Starikovskiy <[email protected]>
Date: Thu, 01 Mar 2007 01:31:40 +0300
Message-ID: <[email protected]>
In-Reply-To: <[email protected]>
References: <[email protected]>
User-Agent: StGIT/0.11
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit

From: Alexey Starikovskiy <[email protected]>

---

 drivers/acpi/ec.c |   87 ++++++++++++++++-------------------------------------
 1 files changed, 27 insertions(+), 60 deletions(-)

Subject: [PATCH 6/9] ACPI: EC: Clean ECDT and namespace parsing.
To: Alexey Starikovskiy <[email protected]>
Date: Thu, 01 Mar 2007 01:31:40 +0300
Message-ID: <[email protected]>
In-Reply-To: <[email protected]>
References: <[email protected]>
User-Agent: StGIT/0.11
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit

From: Alexey Starikovskiy <[email protected]>

---

 drivers/acpi/ec.c |  203 ++++++++++++++++++++++++++++-------------------------
 1 files changed, 108 insertions(+), 95 deletions(-)

Subject: [PATCH 7/9] ACPI: EC: Rename ec_ecdt to more informative boot_ec
To: Alexey Starikovskiy <[email protected]>
Date: Thu, 01 Mar 2007 01:31:40 +0300
Message-ID: <[email protected]>
In-Reply-To: <[email protected]>
References: <[email protected]>
User-Agent: StGIT/0.11
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit

From: Alexey Starikovskiy <[email protected]>

---

 drivers/acpi/ec.c |   45 +++++++++++++++++++++------------------------
 1 files changed, 21 insertions(+), 24 deletions(-)

Subject: [PATCH 8/9] ACPI: EC: first_ec is better to be acpi_ec than
acpi_device.
To: Alexey Starikovskiy <[email protected]>
Date: Thu, 01 Mar 2007 01:31:40 +0300
Message-ID: <[email protected]>
In-Reply-To: <[email protected]>
References: <[email protected]>
User-Agent: StGIT/0.11
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit

From: Alexey Starikovskiy <[email protected]>

---

 drivers/acpi/ec.c |   41 +++++++++++++----------------------------
 1 files changed, 13 insertions(+), 28 deletions(-)

Subject: [PATCH 9/9] ACPI: EC: Cleanup of EC initialization
To: Alexey Starikovskiy <[email protected]>
Date: Thu, 01 Mar 2007 01:31:41 +0300
Message-ID: <[email protected]>
In-Reply-To: <[email protected]>
References: <[email protected]>
User-Agent: StGIT/0.11
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit

From: Alexey Starikovskiy <[email protected]>

---

 drivers/acpi/ec.c |   32 +++++++++++++-------------------
 1 files changed, 13 insertions(+), 19 deletions(-)

index fbfb256..fce38fe 100644
@@ -1,6 +1,8 @@
 /*
- *  acpi_ec.c - ACPI Embedded Controller Driver ($Revision: 38 $)
+ *  ec.c - ACPI Embedded Controller Driver (v2.0)
  *
+ *  Copyright (C) 2006, 2007 Alexey Starikovskiy <[email protected]>
+ *  Copyright (C) 2006 Denis Sadykov <[email protected]>
  *  Copyright (C) 2004 Luming Yu <[email protected]>
  *  Copyright (C) 2001, 2002 Andy Grover <[email protected]>
  *  Copyright (C) 2001, 2002 Paul Diefenbaugh <[email protected]>

@@ -94,7 +96,6 @@ static struct acpi_driver acpi_ec_driver
 /* External interfaces use first EC only, so remember */
 static struct acpi_ec {
 	acpi_handle handle;
-	unsigned long uid;
 	unsigned long gpe;
 	unsigned long command_addr;
 	unsigned long data_addr;

@@ -520,13 +521,11 @@ static int acpi_ec_read_info(struct seq_
 	if (!ec)
 		goto end;
 
-	seq_printf(seq, "gpe:                 0x%02x\n", (u32) ec->gpe);
-	seq_printf(seq, "ports:                   0x%02x, 0x%02x\n",
-		   (u32) ec->command_addr, (u32) ec->data_addr);
-	seq_printf(seq, "use global lock:         %s\n",
+	seq_printf(seq, "gpe:\t\t\t0x%02x\n", (u32) ec->gpe);
+	seq_printf(seq, "ports:\t\t\t0x%02x, 0x%02x\n",
+		   (unsigned)ec->command_addr, (unsigned)ec->data_addr);
+	seq_printf(seq, "use global lock:\t%s\n",
 		   ec->global_lock ? "yes" : "no");
-	acpi_enable_gpe(NULL, ec->gpe, ACPI_NOT_ISR);
-
       end:
 	return 0;
 }

@@ -649,15 +648,13 @@ static int acpi_ec_add(struct acpi_devic
 
 static int acpi_ec_remove(struct acpi_device *device, int type)
 {
-	struct acpi_ec *ec = NULL;
+	struct acpi_ec *ec;
 
 	if (!device)
 		return -EINVAL;
 
 	ec = acpi_driver_data(device);
-
 	acpi_ec_remove_fs(device);
-
 	acpi_driver_data(device) = NULL;
 	if (ec == first_ec)
 		first_ec = NULL;

@@ -665,7 +662,6 @@ static int acpi_ec_remove(struct acpi_de
 	/* Don't touch boot EC */
 	if (boot_ec != ec)
 		kfree(ec);
-
 	return 0;
 }
 

@@ -674,22 +670,20 @@ ec_parse_io_ports(struct acpi_resource *
 {
 	struct acpi_ec *ec = context;
 
-	if (resource->type != ACPI_RESOURCE_TYPE_IO) {
+	if (resource->type != ACPI_RESOURCE_TYPE_IO)
 		return AE_OK;
-	}
 
 	/*
 	 * The first address region returned is the data port, and
 	 * the second address region returned is the status/command
 	 * port.
 	 */
-	if (ec->data_addr == 0) {
+	if (ec->data_addr == 0)
 		ec->data_addr = resource->data.io.minimum;
-	} else if (ec->command_addr == 0) {
+	else if (ec->command_addr == 0)
 		ec->command_addr = resource->data.io.minimum;
-	} else {
+	else
 		return AE_CTRL_TERMINATE;
-	}
 
 	return AE_OK;
 }

@@ -702,6 +696,7 @@ static int ec_install_handlers(struct ac
 					  &acpi_ec_gpe_handler, ec);
 	if (ACPI_FAILURE(status))
 		return -ENODEV;
+
 	acpi_set_gpe_type(NULL, ec->gpe, ACPI_GPE_TYPE_RUNTIME);
 	acpi_enable_gpe(NULL, ec->gpe, ACPI_NOT_ISR);
 

@@ -821,7 +816,6 @@ int __init acpi_ec_ecdt_probe(void)
 	boot_ec->gpe = ecdt_ptr->gpe;
 	/* use the GL just to be safe */
 	boot_ec->global_lock = TRUE;
-	boot_ec->uid = ecdt_ptr->uid;
 	boot_ec->handle = ACPI_ROOT_OBJECT;
 
 	ret = ec_install_handlers(boot_ec);

why is fake_ecdt no longer necessary -- because ec initializes first now?

while I'm glad to see the ec_burst_enable exported -- it is currently not called and we continue to always operate in normal mode, yes?

it seems that burst mode is intended for when we want to push a series of commands to the ec, so we'd
1. enable burst mode
2. do a bunch of ec transactions
2. disable burst mode
right?

Anybody got any candidates for a series of transactions that would benefit from this mode? If yes, is it possible to detect an efficiency benefit from enabling burst mode for that series?

yes, i like "boot_ec" better than "ec_ecdt".

as usual, they patch series is "light" on check-in comments -- in particular, there are none at all explaining why these changes are necessary.

thanks,
-Len

ps. mbox applies better if "From" is not replaced by ">From"

right

yes

right

the one obvious candidate is smart battery, it reads all of its data as separate read/writes through EC.

glad to hear that :)

make code cleaner...

ibm-acpi would like a lot to have burst-mode for EC dumps (read of 256 EC registers), and for thermal reading (two bursts of eight register reads, or one of sixteen non-contiguous reads if that is possible -- I need to read the ACPI spec on the burst mode).

  "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