[2.6.28-rc2] EeePC ACPI errors & exceptions

[2.6.28-rc2] EeePC ACPI errors & exceptions

[Darren Salt]

[-- Attachment #1: Type: text/plain, Size: 1314 bytes --]

EeePC 901, BIOS revision 1603, current Debian lenny; running on AC.

I've noticed the following errors & exceptions, apparently coinciding with
the startup of xfce4-sensors-plugin:

ACPI: EC: missing confirmations, switch off interrupt mode.
ACPI Exception (evregion-0419): AE_TIME, Returned by Handler for [EmbeddedControl] [20080926]
ACPI Error (psparse-0524): Method parse/execution failed [\_SB_.PCI0.SBRG.EC0_.BST2] (Node f7030d38), AE_TIME
ACPI Error (psparse-0524): Method parse/execution failed [\_SB_.PCI0.CBST] (Node f70336f8), AE_TIME
ACPI Error (psparse-0524): Method parse/execution failed [\_SB_.PCI0.BAT0._BST] (Node f7031c48), AE_TIME
ACPI Exception (battery-0360): AE_TIME, Evaluating _BST [20080926]

Also, this (which, unlike the above, is also present with 2.6.27.*):

ACPI: I/O resource 0000:00:1f.3 [0x400-0x41f] conflicts with ACPI region SMRG [0x400-0x40f]
ACPI: Device needs an ACPI driver

Full dmesg & config are attached.

(The Elantech touchpad patch from http://arjan.opmeer.net/elantech/ is
applied to this kernel.)

-- 
| Darren Salt    | linux or ds at              | nr. Ashington, | Toon
| RISC OS, Linux | youmustbejoking,demon,co,uk | Northumberland | Army
|   <URL:http://www.youmustbejoking.demon.co.uk/progs.packages.html>

Thou shalt not sleep within an interrupt handler.

[-- Attachment #2: dmesg.gz --]
[-- Type: application/x-gzip, Size: 8819 bytes --]

[-- Attachment #3: config-2.6.28-rc2.gz --]
[-- Type: application/x-gzip, Size: 13774 bytes --]

Re: [2.6.28-rc2] EeePC ACPI errors & exceptions

[Zhao Yakui]

On Mon, 2008-10-27 at 15:52 -0700, Darren Salt wrote:
Will you please attach the output of acpidump?
Of course you can file a bug report at
http://bugzilla.kernel.org/enter_bug.cgi?product=ACPI
and attach the output of acpidump, dmesg.

It will be better if you can add the boot option of "printk.time=1".
thanks.

> EeePC 901, BIOS revision 1603, current Debian lenny; running on AC.
> 
> I've noticed the following errors & exceptions, apparently coinciding with
> the startup of xfce4-sensors-plugin:
> 
> ACPI: EC: missing confirmations, switch off interrupt mode.
> ACPI Exception (evregion-0419): AE_TIME, Returned by Handler for [EmbeddedControl] [20080926]
> ACPI Error (psparse-0524): Method parse/execution failed [\_SB_.PCI0.SBRG.EC0_.BST2] (Node f7030d38), AE_TIME
> ACPI Error (psparse-0524): Method parse/execution failed [\_SB_.PCI0.CBST] (Node f70336f8), AE_TIME
> ACPI Error (psparse-0524): Method parse/execution failed [\_SB_.PCI0.BAT0._BST] (Node f7031c48), AE_TIME
> ACPI Exception (battery-0360): AE_TIME, Evaluating _BST [20080926]
> 
> Also, this (which, unlike the above, is also present with 2.6.27.*):
> 
> ACPI: I/O resource 0000:00:1f.3 [0x400-0x41f] conflicts with ACPI region SMRG [0x400-0x40f]
> ACPI: Device needs an ACPI driver
> 
> Full dmesg & config are attached.
> 
> (The Elantech touchpad patch from http://arjan.opmeer.net/elantech/ is
> applied to this kernel.)
> 

Re: [2.6.28-rc2] EeePC ACPI errors & exceptions

[Zhao Yakui]

On Mon, 2008-10-27 at 15:52 -0700, Darren Salt wrote: 
> EeePC 901, BIOS revision 1603, current Debian lenny; running on AC.
> 
> I've noticed the following errors & exceptions, apparently coinciding with
> the startup of xfce4-sensors-plugin:
Will you please confirm whether the same issue exists on the previous
kernel? For example: 2.6.27 or 2.6.27-rc6

After the following commit is merged, it seems that on the EEEPC laptop
the EC will switch off EC GPE interrupt mode when there is no EC GPE
interrupt confirm for some EC transactions. Then AE_TIME is returned by
the function of ec_poll, which causes that the _BST object of battery
can't be evaluated.
   >commit 7c6db4e050601f359081fde418ca6dc4fc2d0011
   >Author: Alexey Starikovskiy <astarikovskiy@suse.de>
   >Date:   Thu Sep 25 21:00:31 2008 +0400
     >ACPI: EC: do transaction from interrupt context

When there is no EC GPE interrupt confirm in some EC transaction, the
EC will switch off the EC GPE interrupt mode in the function of
acpi_ec_wait. 

But why is AE_TIME returned by the function of ec_poll?
>static int ec_poll(struct acpi_ec *ec)
{
        unsigned long delay = jiffies + msecs_to_jiffies(ACPI_EC_DELAY);
        msleep(1);
// Maybe the current jiffies is already after the predefined jiffies
after msleep(1). In such case the ETIME will be returned. Of course the
EC transaction can't be finished. If so, IMO this is not reasonable as
this is caused by that OS has no opportunity to issue the following EC
command sequence.
        while (time_before(jiffies, delay)) {
                gpe_transaction(ec, acpi_ec_read_status(ec));
                msleep(1);
                if (ec_transaction_done(ec))
                        return 0;
//Maybe there exists the following cases. EC transaction is not finished
after msleep(1),but the current jiffies is already after predefined
jiffies. So ETIME is returned. In such case, IMO this is also not
reasonable.
        }
        return -ETIME;
}
     At the same time msleep is realized by schedule_timeout. On linux
although one process is waked up by some events, it won't be scheduled
immediately. So maybe the current jiffies is already after the
predefined timeout jiffies  after msleep(1).

     Now some people suggest that msleep is replaced by udelay. Although
the above two cases can be avoided by that msleep is replaced by udelay,
maybe the above two cases still exist if the preempt schedule happens at
the corresponding place.
    At the same time when msleep is replaced by udelay, CPU will do
nothing but loop while executing udelay. If the 100 EC transactions are
done in one second, the CPU will do nothing in about 100*2*100
microseconds. IMO this is not reasonable. 

    IMO the better solution is that the EC transaction is divided into
the different phases. (Not do the EC transaction in one loop).
    For example:
    The EC read transaction can be done in the following sequence:
    a. Issue read command 
    b. wait until the EC input buffer is empty and then write the EC
internal read address
    c. wait until the EC output buffer is ready and read the data from
EC the Data port. Of course it indicates that EC read transaction is
finished.
    
    The EC  write transaction can be done in the following sequence:
    a. issue the write command
    b. wait until the EC input buffer is empty and write the EC internal
write address
    c. wait until the EC input buffer is empty and write the data to the
EC Data port
    d. wait until the EC input buffer is empty. After it becomes empty,
it indicates that the EC write transaction is finished.

Welcome the comments.
   Thanks.
    
    
> ACPI: EC: missing confirmations, switch off interrupt mode.
> ACPI Exception (evregion-0419): AE_TIME, Returned by Handler for [EmbeddedControl] [20080926]
> ACPI Error (psparse-0524): Method parse/execution failed [\_SB_.PCI0.SBRG.EC0_.BST2] (Node f7030d38), AE_TIME
> ACPI Error (psparse-0524): Method parse/execution failed [\_SB_.PCI0.CBST] (Node f70336f8), AE_TIME
> ACPI Error (psparse-0524): Method parse/execution failed [\_SB_.PCI0.BAT0._BST] (Node f7031c48), AE_TIME
> ACPI Exception (battery-0360): AE_TIME, Evaluating _BST [20080926]
> 
> Also, this (which, unlike the above, is also present with 2.6.27.*):
> 
> ACPI: I/O resource 0000:00:1f.3 [0x400-0x41f] conflicts with ACPI region SMRG [0x400-0x40f]
> ACPI: Device needs an ACPI driver
> 
> Full dmesg & config are attached.
> 
> (The Elantech touchpad patch from http://arjan.opmeer.net/elantech/ is
> applied to this kernel.)
> 

Re: [2.6.28-rc2] EeePC ACPI errors & exceptions

[Zhao Yakui]

[-- Attachment #1: Type: text/plain, Size: 2345 bytes --]

On Mon, 2008-10-27 at 15:52 -0700, Darren Salt wrote:
> EeePC 901, BIOS revision 1603, current Debian lenny; running on AC.
> 
> I've noticed the following errors & exceptions, apparently coinciding with
> the startup of xfce4-sensors-plugin:
> 
> ACPI: EC: missing confirmations, switch off interrupt mode.
> ACPI Exception (evregion-0419): AE_TIME, Returned by Handler for [EmbeddedControl] [20080926]
> ACPI Error (psparse-0524): Method parse/execution failed [\_SB_.PCI0.SBRG.EC0_.BST2] (Node f7030d38), AE_TIME
> ACPI Error (psparse-0524): Method parse/execution failed [\_SB_.PCI0.CBST] (Node f70336f8), AE_TIME
> ACPI Error (psparse-0524): Method parse/execution failed [\_SB_.PCI0.BAT0._BST] (Node f7031c48), AE_TIME
> ACPI Exception (battery-0360): AE_TIME, Evaluating _BST [20080926]
Will you please try the attached patch on the 2.6.28-rc2 and see whether
the issue still exists?
    After the test, please attach the output of dmesg.
    

The following is included by the attached patch.
   a. EC transaction is divided into several phases.
        For example: EC write transaction
        a. issue EC write command
        b. wait until the input is empty and then write the internal
address
        c. wait until the input is empty and write the data
        d. wait until the input is empty. If it is empty, it indicates
        that EC transaction is finished.
   At the same time EC driver will be started in EC GEP mode. And when
there is no EC GPE confirmation for some EC transaction on some broken
laptops,the EC driver will be switched to polling mode. But EC GPE is
still enabled.

b. Some delay is added in the EC GPE handler on some broken BIOS.
   The delay won't be added for most laptops.Only when more than five
interrupts happen in the same jiffies and EC status are the same, OS
will add some delay in the EC GPE handler. If the same issue still
happens after adding delay,the delay time will be increased.But the max
delay time is ten microseconds.

Thanks.

> 
> Also, this (which, unlike the above, is also present with 2.6.27.*):
> 
> ACPI: I/O resource 0000:00:1f.3 [0x400-0x41f] conflicts with ACPI region SMRG [0x400-0x40f]
> ACPI: Device needs an ACPI driver
> 
> Full dmesg & config are attached.
> 
> (The Elantech touchpad patch from http://arjan.opmeer.net/elantech/ is
> applied to this kernel.)
> 

[-- Attachment #2: ec_all.patch --]
[-- Type: text/x-patch, Size: 16358 bytes --]

a. EC transaction is divided into several phases.
        For example: EC write transaction
        a. issue EC write command
        b. wait until the input is empty and then write the internal address
        c. wait until the input is empty and write the data
        d. wait until the input is empty. If it is empty, it indicates
        that EC transaction is finished.
At the same time EC driver will be started in EC GEP mode. And when there is
no EC GPE confirmation for some EC transaction on some broken laptops,the EC
driver will be switched to polling mode. But EC GPE is still enabled.

b. Some delay is added in the EC GPE handler on some broken BIOS.
   The delay won't be added for most laptops.Only when more than five
interrupts happen in the same jiffies and EC status are the same, OS will add
some delay in the EC GPE handler. If the same issue still happens after adding
delay,the delay time will be increased.But the max delay time is ten
microseconds.

Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
---
 drivers/acpi/ec.c |  333 +++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 233 insertions(+), 100 deletions(-)

Index: linux-2.6/drivers/acpi/ec.c
===================================================================
--- linux-2.6.orig/drivers/acpi/ec.c
+++ linux-2.6/drivers/acpi/ec.c
@@ -65,15 +65,25 @@ enum ec_command {
 	ACPI_EC_BURST_DISABLE = 0x83,
 	ACPI_EC_COMMAND_QUERY = 0x84,
 };
+/* EC events */
+enum ec_event {
+	ACPI_EC_EVENT_OBF_1 = 1,	/* Output buffer full */
+	ACPI_EC_EVENT_IBF_0,		/* Input buffer empty */
+};
 
 #define ACPI_EC_DELAY		500	/* Wait 500ms max. during EC ops */
 #define ACPI_EC_UDELAY_GLK	1000	/* Wait 1ms max. to get global lock */
 #define ACPI_EC_UDELAY		100	/* Wait 100us before polling EC again */
 
+#define EC_IRQ_NUM 		5	/* the EC interrupt storm threshold */
+#define EC_MAX_UDELAY		10	/* the max udelay time */
+
 #define ACPI_EC_STORM_THRESHOLD 20	/* number of false interrupts
 					   per one transaction */
 
 enum {
+	EC_FLAGS_WAIT_GPE,		/* Don't check EC status
+					 * until GPE arrives*/
 	EC_FLAGS_QUERY_PENDING,		/* Query is pending */
 	EC_FLAGS_GPE_MODE,		/* Expect GPE to be sent
 					 * for status change */
@@ -111,11 +121,21 @@ static struct acpi_ec {
 	unsigned long data_addr;
 	unsigned long global_lock;
 	unsigned long flags;
+	unsigned long pre_jiffies;
+	/* record the jiffies when last EC interrupt happens */
 	struct mutex lock;
 	wait_queue_head_t wait;
 	struct list_head list;
 	struct transaction *curr;
 	spinlock_t curr_lock;
+	u8 pre_state;
+	/* record the EC status when last EC interrrupt happens */
+	atomic_t ec_irq_count;
+	unsigned long udelay;
+	/*
+	 * this is for the input parameter of udelay in EC GPE interrupt.
+	 * the max value is IRQ_MAX_UDELAY.(10)
+	 */
 } *boot_ec, *first_ec;
 
 /* 
@@ -210,18 +230,60 @@ unlock:
 	spin_unlock_irqrestore(&ec->curr_lock, flags);
 }
 
-static int acpi_ec_wait(struct acpi_ec *ec)
+static inline int acpi_ec_check_status(struct acpi_ec *ec,
+			enum ec_event event)
 {
-	if (wait_event_timeout(ec->wait, ec_transaction_done(ec),
-			       msecs_to_jiffies(ACPI_EC_DELAY)))
+	if (test_bit(EC_FLAGS_WAIT_GPE, &ec->flags))
 		return 0;
-	/* missing GPEs, switch back to poll mode */
-	if (printk_ratelimit())
-		pr_info(PREFIX "missing confirmations, "
-				"switch off interrupt mode.\n");
-	set_bit(EC_FLAGS_NO_GPE, &ec->flags);
-	clear_bit(EC_FLAGS_GPE_MODE, &ec->flags);
-	return 1;
+	if (event == ACPI_EC_EVENT_OBF_1) {
+		if (acpi_ec_read_status(ec) & ACPI_EC_FLAG_OBF)
+			return 1;
+	} else if (event == ACPI_EC_EVENT_IBF_0) {
+		if (!(acpi_ec_read_status(ec) & ACPI_EC_FLAG_IBF))
+			return 1;
+	}
+
+	return 0;
+}
+
+static int acpi_ec_wait(struct acpi_ec *ec, enum ec_event event, int force_poll)
+{
+	if (likely(test_bit(EC_FLAGS_GPE_MODE, &ec->flags)) &&
+	    likely(!force_poll)) {
+		if (wait_event_timeout(ec->wait,
+				acpi_ec_check_status(ec, event),
+					msecs_to_jiffies(ACPI_EC_DELAY)))
+			return 0;
+		/* Check whether the bogus timeout happens */
+		if (!test_bit(EC_FLAGS_WAIT_GPE, &ec->flags) &&
+			acpi_ec_check_status(ec, event))
+			return 0;
+
+		clear_bit(EC_FLAGS_WAIT_GPE, &ec->flags);
+		if (acpi_ec_check_status(ec, event)) {
+			/*
+			 * Clear GPE mode flags. When EC internal register is
+			 * accessed, EC driver will work in polling mode.
+			 * But GPE is still enabled.
+			 */
+			clear_bit(EC_FLAGS_GPE_MODE, &ec->flags);
+			return 0;
+		}
+	} else {
+		unsigned long delay = jiffies + msecs_to_jiffies(ACPI_EC_DELAY);
+		clear_bit(EC_FLAGS_WAIT_GPE, &ec->flags);
+		while (time_before(jiffies, delay)) {
+			if (acpi_ec_check_status(ec, event))
+				return 0;
+			msleep(1);
+		}
+		if (acpi_ec_check_status(ec, event))
+			return 0;
+	}
+	pr_err(PREFIX "acpi_ec_wait timeout, status = 0x%2.2x, event = %s\n",
+		acpi_ec_read_status(ec),
+		(event == ACPI_EC_EVENT_OBF_1) ? "\"b0=1\"" : "\"b1=0\"");
+	return -ETIME;
 }
 
 static void acpi_ec_gpe_query(void *ec_cxt);
@@ -248,48 +310,51 @@ static int ec_poll(struct acpi_ec *ec)
 	}
 	return -ETIME;
 }
-
-static int acpi_ec_transaction_unlocked(struct acpi_ec *ec,
-					struct transaction *t,
+static int acpi_ec_transaction_unlocked(struct acpi_ec *ec, u8 command,
+					const u8 *wdata, unsigned wdata_len,
+					u8 *rdata, unsigned rdata_len,
 					int force_poll)
 {
-	unsigned long tmp;
-	int ret = 0;
+	int result = 0;
+	set_bit(EC_FLAGS_WAIT_GPE, &ec->flags);
 	pr_debug(PREFIX "transaction start\n");
-	/* disable GPE during transaction if storm is detected */
-	if (test_bit(EC_FLAGS_GPE_STORM, &ec->flags)) {
-		clear_bit(EC_FLAGS_GPE_MODE, &ec->flags);
-		acpi_disable_gpe(NULL, ec->gpe, ACPI_NOT_ISR);
-	}
-	/* start transaction */
-	spin_lock_irqsave(&ec->curr_lock, tmp);
-	/* following two actions should be kept atomic */
-	t->irq_count = 0;
-	ec->curr = t;
-	acpi_ec_write_cmd(ec, ec->curr->command);
-	if (ec->curr->command == ACPI_EC_COMMAND_QUERY)
+	acpi_ec_write_cmd(ec, command);
+	for (; wdata_len > 0; --wdata_len) {
+		result = acpi_ec_wait(ec, ACPI_EC_EVENT_IBF_0, force_poll);
+		if (result) {
+			pr_err(PREFIX
+			       "write_cmd timeout, command = %d\n", command);
+			goto end;
+		}
+		set_bit(EC_FLAGS_WAIT_GPE, &ec->flags);
+		acpi_ec_write_data(ec, *(wdata++));
+	}
+
+	if (!rdata_len) {
+		result = acpi_ec_wait(ec, ACPI_EC_EVENT_IBF_0, force_poll);
+		if (result) {
+			pr_err(PREFIX
+			       "finish-write timeout, command = %d\n", command);
+			goto end;
+		}
+	} else if (command == ACPI_EC_COMMAND_QUERY)
 		clear_bit(EC_FLAGS_QUERY_PENDING, &ec->flags);
-	spin_unlock_irqrestore(&ec->curr_lock, tmp);
-	/* if we selected poll mode or failed in GPE-mode do a poll loop */
-	if (force_poll ||
-	    !test_bit(EC_FLAGS_GPE_MODE, &ec->flags) ||
-	    acpi_ec_wait(ec))
-		ret = ec_poll(ec);
-	pr_debug(PREFIX "transaction end\n");
-	spin_lock_irqsave(&ec->curr_lock, tmp);
-	ec->curr = NULL;
-	spin_unlock_irqrestore(&ec->curr_lock, tmp);
-	if (test_bit(EC_FLAGS_GPE_STORM, &ec->flags)) {
-		/* check if we received SCI during transaction */
-		ec_check_sci(ec, acpi_ec_read_status(ec));
-		/* it is safe to enable GPE outside of transaction */
-		acpi_enable_gpe(NULL, ec->gpe, ACPI_NOT_ISR);
-	} else if (test_bit(EC_FLAGS_GPE_MODE, &ec->flags) &&
-		   t->irq_count > ACPI_EC_STORM_THRESHOLD) {
-		pr_debug(PREFIX "GPE storm detected\n");
-		set_bit(EC_FLAGS_GPE_STORM, &ec->flags);
+
+	for (; rdata_len > 0; --rdata_len) {
+		result = acpi_ec_wait(ec, ACPI_EC_EVENT_OBF_1, force_poll);
+		if (result) {
+			pr_err(PREFIX "read timeout, command = %d\n", command);
+			goto end;
+		}
+		/* Don't expect GPE after last read */
+		if (rdata_len > 1)
+			set_bit(EC_FLAGS_WAIT_GPE, &ec->flags);
+
+		*(rdata++) = acpi_ec_read_data(ec);
 	}
-	return ret;
+end:
+	pr_debug(PREFIX "transaction end\n");
+	return result;
 }
 
 static int ec_check_ibf0(struct acpi_ec *ec)
@@ -310,35 +375,47 @@ static int ec_wait_ibf0(struct acpi_ec *
 	return -ETIME;
 }
 
-static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t,
+static int acpi_ec_transaction(struct acpi_ec *ec, u8 command,
+			       const u8 *wdata, unsigned wdata_len,
+			       u8 *rdata, unsigned rdata_len,
 			       int force_poll)
 {
 	int status;
 	u32 glk;
-	if (!ec || (!t) || (t->wlen && !t->wdata) || (t->rlen && !t->rdata))
+
+	if (!ec || (wdata_len && !wdata) || (rdata_len && !rdata))
 		return -EINVAL;
-	if (t->rdata)
-		memset(t->rdata, 0, t->rlen);
+
+	if (rdata)
+		memset(rdata, 0, rdata_len);
+
 	mutex_lock(&ec->lock);
 	if (ec->global_lock) {
 		status = acpi_acquire_global_lock(ACPI_EC_UDELAY_GLK, &glk);
 		if (ACPI_FAILURE(status)) {
-			status = -ENODEV;
-			goto unlock;
+			mutex_unlock(&ec->lock);
+			return -ENODEV;
 		}
 	}
-	if (ec_wait_ibf0(ec)) {
+
+	status = acpi_ec_wait(ec, ACPI_EC_EVENT_IBF_0, 0);
+	if (status) {
 		pr_err(PREFIX "input buffer is not empty, "
 				"aborting transaction\n");
-		status = -ETIME;
 		goto end;
 	}
-	status = acpi_ec_transaction_unlocked(ec, t, force_poll);
+
+	status = acpi_ec_transaction_unlocked(ec, command,
+					      wdata, wdata_len,
+					      rdata, rdata_len,
+					      force_poll);
+
 end:
+
 	if (ec->global_lock)
 		acpi_release_global_lock(glk);
-unlock:
 	mutex_unlock(&ec->lock);
+
 	return status;
 }
 
@@ -349,32 +426,35 @@ unlock:
 int acpi_ec_burst_enable(struct acpi_ec *ec)
 {
 	u8 d;
-	struct transaction t = {.command = ACPI_EC_BURST_ENABLE,
-				.wdata = NULL, .rdata = &d,
-				.wlen = 0, .rlen = 1};
-
-	return acpi_ec_transaction(ec, &t, 0);
+	/*
+	 * When burst enable command is issused, EC will put the EC burst
+	 * acknowledge byte (0x90) into output buffer.
+	 * Maybe it is worth checking the returned acknowledge bytes.
+	 */
+	return acpi_ec_transaction(ec, ACPI_EC_BURST_ENABLE, NULL, 0, &d, 1, 0);
 }
 
 int acpi_ec_burst_disable(struct acpi_ec *ec)
 {
-	struct transaction t = {.command = ACPI_EC_BURST_DISABLE,
-				.wdata = NULL, .rdata = NULL,
-				.wlen = 0, .rlen = 0};
+	u8 d = acpi_ec_read_status(ec);
+	if (d & ACPI_EC_FLAG_BURST) {
+		/*
+		 * Only when EC is in burst mode, it is necessary to disable.
+		 */
+		return acpi_ec_transaction(ec, ACPI_EC_BURST_DISABLE, NULL,
+						0, NULL, 0, 0);
+	}
+	return 0;
 
-	return (acpi_ec_read_status(ec) & ACPI_EC_FLAG_BURST) ?
-				acpi_ec_transaction(ec, &t, 0) : 0;
 }
 
 static int acpi_ec_read(struct acpi_ec *ec, u8 address, u8 * data)
 {
 	int result;
 	u8 d;
-	struct transaction t = {.command = ACPI_EC_COMMAND_READ,
-				.wdata = &address, .rdata = &d,
-				.wlen = 1, .rlen = 1};
 
-	result = acpi_ec_transaction(ec, &t, 0);
+	result = acpi_ec_transaction(ec, ACPI_EC_COMMAND_READ,
+				     &address, 1, &d, 1, 0);
 	*data = d;
 	return result;
 }
@@ -382,11 +462,8 @@ static int acpi_ec_read(struct acpi_ec *
 static int acpi_ec_write(struct acpi_ec *ec, u8 address, u8 data)
 {
 	u8 wdata[2] = { address, data };
-	struct transaction t = {.command = ACPI_EC_COMMAND_WRITE,
-				.wdata = wdata, .rdata = NULL,
-				.wlen = 2, .rlen = 0};
-
-	return acpi_ec_transaction(ec, &t, 0);
+	return acpi_ec_transaction(ec, ACPI_EC_COMMAND_WRITE,
+				   wdata, 2, NULL, 0, 0);
 }
 
 /*
@@ -448,13 +525,12 @@ int ec_transaction(u8 command,
 		   u8 * rdata, unsigned rdata_len,
 		   int force_poll)
 {
-	struct transaction t = {.command = command,
-				.wdata = wdata, .rdata = rdata,
-				.wlen = wdata_len, .rlen = rdata_len};
 	if (!first_ec)
 		return -ENODEV;
 
-	return acpi_ec_transaction(first_ec, &t, force_poll);
+	return acpi_ec_transaction(first_ec, command, wdata,
+				   wdata_len, rdata, rdata_len,
+				   force_poll);
 }
 
 EXPORT_SYMBOL(ec_transaction);
@@ -463,9 +539,7 @@ static int acpi_ec_query(struct acpi_ec 
 {
 	int result;
 	u8 d;
-	struct transaction t = {.command = ACPI_EC_COMMAND_QUERY,
-				.wdata = NULL, .rdata = &d,
-				.wlen = 0, .rlen = 1};
+
 	if (!ec || !data)
 		return -EINVAL;
 
@@ -475,7 +549,8 @@ static int acpi_ec_query(struct acpi_ec 
 	 * bit to be cleared (and thus clearing the interrupt source).
 	 */
 
-	result = acpi_ec_transaction(ec, &t, 0);
+	result = acpi_ec_transaction(ec, ACPI_EC_COMMAND_QUERY,
+						NULL, 0, &d, 1, 0);
 	if (result)
 		return result;
 
@@ -549,29 +624,77 @@ static void acpi_ec_gpe_query(void *ec_c
 	}
 	mutex_unlock(&ec->lock);
 }
+/*******************************************************************************
+*
+* FUNCTION:	acpi_ec_gpe_delay
+*
+* PARAMETERS:	struct acpi_ec *ec, the EC device
+		u8 state  - the EC status when EC GPE interrupt happens
+
+* RETURN:	No
+*
+* DESCRIPTION: detect whether there exists EC GPE interrupt storm. If yes, it
+		will try to add some delay to reduce the number of EC GPE
+		interrupts.
+******************************************************************************/
+static void acpi_ec_gpe_delay(struct acpi_ec *ec, u8 state)
+{
+	static int warn_done;
+	if ((jiffies == ec->pre_jiffies) && (state == ec->pre_state)) {
+		atomic_inc(&ec->ec_irq_count);
+		if (atomic_read(&ec->ec_irq_count) > EC_IRQ_NUM) {
+			/*
+			 * If the ec_irq_count is above EC_IRQ_NUM, it will
+			 * be regarded as EC GPE interrupt storm. We will
+			 * try to add some udelay in acpi_ec_gpe_delay.
+			 */
+			atomic_set(&ec->ec_irq_count, 0);
+			if (ec->udelay > EC_MAX_UDELAY) {
+				if (!warn_done) {
+					printk(KERN_WARNING "EC GPE interrupt"
+					" storm. And it is hardware issue\n");
+					warn_done++;
+				}
+			} else {
+				/*
+				 * the input parameter of udelay will be
+				 * increased.
+				 */
+				ec->udelay = ec->udelay + 1;
+			}
+		}
+	} else {
+		ec->pre_jiffies = jiffies;
+		ec->pre_state = state;
+		atomic_set(&ec->ec_irq_count, 0);
+	}
+	if (ec->udelay)
+		udelay(ec->udelay);
+}
 
 static u32 acpi_ec_gpe_handler(void *data)
 {
 	struct acpi_ec *ec = data;
-	u8 status;
+	u8 status ;
+	acpi_status ec_status = AE_OK;
 
 	pr_debug(PREFIX "~~~> interrupt\n");
 	status = acpi_ec_read_status(ec);
 
-	gpe_transaction(ec, status);
-	if (ec_transaction_done(ec) && (status & ACPI_EC_FLAG_IBF) == 0)
+	clear_bit(EC_FLAGS_WAIT_GPE, &ec->flags);
+	if (test_bit(EC_FLAGS_GPE_MODE, &ec->flags))
 		wake_up(&ec->wait);
 
-	ec_check_sci(ec, status);
-	if (!test_bit(EC_FLAGS_GPE_MODE, &ec->flags) &&
-	    !test_bit(EC_FLAGS_NO_GPE, &ec->flags)) {
-		/* this is non-query, must be confirmation */
-		if (printk_ratelimit())
-			pr_info(PREFIX "non-query interrupt received,"
-				" switching to interrupt mode\n");
-		set_bit(EC_FLAGS_GPE_MODE, &ec->flags);
+	if (status & ACPI_EC_FLAG_SCI) {
+		if (!test_and_set_bit(EC_FLAGS_QUERY_PENDING, &ec->flags)) {
+			ec_status = acpi_os_execute(OSL_EC_BURST_HANDLER,
+				acpi_ec_gpe_query, ec);
+		}
 	}
-	return ACPI_INTERRUPT_HANDLED;
+	acpi_ec_gpe_delay(ec, status);
+
+	return ACPI_SUCCESS(ec_status) ?
+			ACPI_INTERRUPT_HANDLED : ACPI_INTERRUPT_NOT_HANDLED;
 }
 
 /* --------------------------------------------------------------------------
@@ -712,10 +835,22 @@ static struct acpi_ec *make_acpi_ec(void
 	if (!ec)
 		return NULL;
 	ec->flags = 1 << EC_FLAGS_QUERY_PENDING;
+	/*
+	 * Start GPE mode.
+	 * When EC driver is started, it will work in GPE mode.
+	 * It means that the EC GPE interrupt is expected when EC status is
+	 * changed.
+	 * Of course if there is no EC GPE interrupt in some EC transaction,
+	 * it will be cleared and EC will work in polling mode when EC
+	 * internal register is accessed.
+	 */
+	set_bit(EC_FLAGS_GPE_MODE, &ec->flags);
 	mutex_init(&ec->lock);
 	init_waitqueue_head(&ec->wait);
 	INIT_LIST_HEAD(&ec->list);
 	spin_lock_init(&ec->curr_lock);
+	ec->pre_jiffies = jiffies;
+	atomic_set(&ec->ec_irq_count, 0);
 	return ec;
 }
 
@@ -1004,8 +1139,6 @@ int __init acpi_ec_ecdt_probe(void)
 static int acpi_ec_suspend(struct acpi_device *device, pm_message_t state)
 {
 	struct acpi_ec *ec = acpi_driver_data(device);
-	/* Stop using GPE */
-	set_bit(EC_FLAGS_NO_GPE, &ec->flags);
 	clear_bit(EC_FLAGS_GPE_MODE, &ec->flags);
 	acpi_disable_gpe(NULL, ec->gpe, ACPI_NOT_ISR);
 	return 0;
@@ -1014,9 +1147,9 @@ static int acpi_ec_suspend(struct acpi_d
 static int acpi_ec_resume(struct acpi_device *device)
 {
 	struct acpi_ec *ec = acpi_driver_data(device);
-	/* Enable use of GPE back */
-	clear_bit(EC_FLAGS_NO_GPE, &ec->flags);
 	acpi_enable_gpe(NULL, ec->gpe, ACPI_NOT_ISR);
+	/* Enable use of GPE back */
+	set_bit(EC_FLAGS_GPE_MODE, &ec->flags);
 	return 0;
 }
 

Re: [2.6.28-rc2] EeePC ACPI errors & exceptions

[Darren Salt]

[-- Attachment #1: Type: text/plain, Size: 1893 bytes --]

I demand that Zhao Yakui may or may not have written...

> On Mon, 2008-10-27 at 15:52 -0700, Darren Salt wrote:
>> EeePC 901, BIOS revision 1603, current Debian lenny; running on AC.
>> I've noticed the following errors & exceptions, apparently coinciding with
>> the startup of xfce4-sensors-plugin:

>> ACPI: EC: missing confirmations, switch off interrupt mode.
>> ACPI Exception (evregion-0419): AE_TIME, Returned by Handler for [EmbeddedControl] [20080926]
>> ACPI Error (psparse-0524): Method parse/execution failed [\_SB_.PCI0.SBRG.EC0_.BST2] (Node f7030d38), AE_TIME
>> ACPI Error (psparse-0524): Method parse/execution failed [\_SB_.PCI0.CBST] (Node f70336f8), AE_TIME
>> ACPI Error (psparse-0524): Method parse/execution failed [\_SB_.PCI0.BAT0._BST] (Node f7031c48), AE_TIME
>> ACPI Exception (battery-0360): AE_TIME, Evaluating _BST [20080926]

> Will you please try the attached patch on the 2.6.28-rc2 and see whether
> the issue still exists?

> After the test, please attach the output of dmesg.

I later noticed that ACPI functions (lid, hot keys etc.) were effectively
disabled by that bug – acpi_listen reported nothing at all.

With the patch, those ACPI errors are gone and the ACPI functions work again.
I see no other significant differences, but output is attached regardless
along with an ACPI dump. (rt2860sta is also present.)

(This leaves only the I/O resource conflict; but, unlike this, that's also
present in 2.6.27, doesn't seem to cause any harm, and looks like it's
specific to BIOS rev. 1603 and possibly others > 1301.)

[snip]
-- 
| Darren Salt    | linux or ds at              | nr. Ashington, | Toon
| RISC OS, Linux | youmustbejoking,demon,co,uk | Northumberland | Army
| + Use more efficient products. Use less.          BE MORE ENERGY EFFICIENT.

I have 240 airconditioning in my car; 2 windows down at 40mph.

[-- Attachment #2: eee901-1603.acpi.gz --]
[-- Type: application/x-gzip, Size: 10303 bytes --]

[-- Attachment #3: dmesg.gz --]
[-- Type: application/x-gzip, Size: 11106 bytes --]

Re: [2.6.28-rc2] EeePC ACPI errors & exceptions

[Alexey Starikovskiy]

Hi Darren,

Please check if the patch 
http://marc.info/?l=linux-acpi&m=122516784917952&w=4
helps.

Thanks,
Alex.

Re: [2.6.28-rc2] EeePC ACPI errors & exceptions

[Darren Salt]

[-- Attachment #1: Type: text/plain, Size: 744 bytes --]

I demand that Alexey Starikovskiy may or may not have written...

> Please check if the patch 
> http://marc.info/?l=linux-acpi&m=122516784917952&w=4
> helps.

With the other patch, this makes no difference.

With only your patch, ACPI EC is started in poll mode; later, several
"non-query interrupt received" messages are logged, followed by a "missing
confirmations" message; none of which I see with the other patch. Otherwise,
all seems well.

dmesg attached for the latter case.

-- 
| Darren Salt    | linux or ds at              | nr. Ashington, | Toon
| RISC OS, Linux | youmustbejoking,demon,co,uk | Northumberland | Army
| + Lobby friends, family, business, government.    WE'RE KILLING THE PLANET.

You will lose an important file.


[-- Attachment #2: dmesg.gz --]
[-- Type: application/x-gzip, Size: 11150 bytes --]

Re: [2.6.28-rc2] EeePC ACPI errors & exceptions

[Zhao Yakui]

On Tue, 2008-10-28 at 13:46 -0700, Alexey Starikovskiy wrote:
> Hi Darren,
> 
> Please check if the patch 
> http://marc.info/?l=linux-acpi&m=122516784917952&w=4
> helps.
In the attached patch the msleep is replaced by udelay gain. 
   In the following commit the udelay is replaced by msleep. 
   >commit 1b7fc5aae8867046f8d3d45808309d5b7f2e036a
   >Author: Alexey Starikovskiy <astarikovskiy@suse.de>
   >Date:   Fri Jun 6 11:49:33 2008 -0400
     >ACPI: EC: Use msleep instead of udelay while waiting for event
   
   After the problem happens again, the udelay is restored again before
getting the root cause. 
   Maybe we should find the root cause of the problem and change the
working flowchart about the EC driver. It is inappropriate that we make
some changes and it is reverted again when the problem happens.
   
   At the same time after mlseep is replaced by the udelay, the CPU will
do thing but loop while doing EC transaction on some laptops (In the
function of ec_poll). If 100 EC transactions are done, the CPU will do
nothing but loop at least for 100*2*100 microseconds. In such case maybe
the performance will be affected.

  After the following commit is merged, the EC transaction will be
executed in EC GPE interrupt context on most laptops.Maybe it is easier.
But for the some laptops it can't be done in EC GPE interrupt context.
So it falls back to the EC polling mode. (This is realized by the
function of ec_poll).
    >commit 7c6db4e050601f359081fde418ca6dc4fc2d0011
    >Author: Alexey Starikovskiy <astarikovskiy@suse.de>
    >Date:   Thu Sep 25 21:00:31 2008 +0400
       >ACPI: EC: do transaction from interrupt context
   
   Why is AE_TIME sometimes returned by the function of ec_poll?
>static int ec_poll(struct acpi_ec *ec)
{
        unsigned long delay = jiffies + msecs_to_jiffies(ACPI_EC_DELAY);
        msleep(1);
// Maybe the current jiffies is already after the predefined jiffies
after msleep(1). In such case the ETIME will be returned. Of course the
EC transaction can't be finished. If so, IMO this is not reasonable as
this is caused by that OS has no opportunity to issue the following EC
command sequence.
        while (time_before(jiffies, delay)) {
                gpe_transaction(ec, acpi_ec_read_status(ec));
                msleep(1);
                if (ec_transaction_done(ec))
                        return 0;
//Maybe there exists the following cases. EC transaction is not finished
after msleep(1),but the current jiffies is already after predefined
jiffies. So ETIME is returned. In such case, IMO this is also not
reasonable.
        }
        return -ETIME;
}
     At the same time msleep is realized by schedule_timeout. On linux
although one process is waked up by some events, it won't be scheduled
immediately. So maybe the current jiffies is already after the
predefined timeout jiffies  after msleep(1). 
    Although the possibility of this issue can be reduced by that msleep
is replaced by udelay,maybe the issue still exists if the preempt
schedule happens at the corresponding place.

    In the above case the ETIME will be returned by ec_poll. But the
reason is not that EC controller can't update its status in time.
Instead it is caused by that host has no opportunity to issue the
sequence operation in the current work flowchart. In current EC work
flowchart the EC transaction is done in a big loop. 
    
    Maybe the better solution is that the EC transaction is explicitly
divided into several different phases. 

    Maybe my analysis is not correct. If so, please correct me. 
Welcome the comments.

    thanks.
    
    
     
> Thanks,
> Alex.
> 
> 

Re: [2.6.28-rc2] EeePC ACPI errors & exceptions

[Alexey Starikovskiy]

Not a problem, just find the root cause. Or shut up.


Zhao Yakui wrote:
> On Tue, 2008-10-28 at 13:46 -0700, Alexey Starikovskiy wrote:
>   
>> Hi Darren,
>>
>> Please check if the patch 
>> http://marc.info/?l=linux-acpi&m=122516784917952&w=4
>> helps.
>>     
> In the attached patch the msleep is replaced by udelay gain. 
>    In the following commit the udelay is replaced by msleep. 
>    >commit 1b7fc5aae8867046f8d3d45808309d5b7f2e036a
>    >Author: Alexey Starikovskiy <astarikovskiy@suse.de>
>    >Date:   Fri Jun 6 11:49:33 2008 -0400
>      >ACPI: EC: Use msleep instead of udelay while waiting for event
>    
>    After the problem happens again, the udelay is restored again before
> getting the root cause. 
>    Maybe we should find the root cause of the problem and change the
> working flowchart about the EC driver. It is inappropriate that we make
> some changes and it is reverted again when the problem happens.
>    
>    At the same time after mlseep is replaced by the udelay, the CPU will
> do thing but loop while doing EC transaction on some laptops (In the
> function of ec_poll). If 100 EC transactions are done, the CPU will do
> nothing but loop at least for 100*2*100 microseconds. In such case maybe
> the performance will be affected.
>
>   After the following commit is merged, the EC transaction will be
> executed in EC GPE interrupt context on most laptops.Maybe it is easier.
> But for the some laptops it can't be done in EC GPE interrupt context.
> So it falls back to the EC polling mode. (This is realized by the
> function of ec_poll).
>     >commit 7c6db4e050601f359081fde418ca6dc4fc2d0011
>     >Author: Alexey Starikovskiy <astarikovskiy@suse.de>
>     >Date:   Thu Sep 25 21:00:31 2008 +0400
>        >ACPI: EC: do transaction from interrupt context
>    
>    Why is AE_TIME sometimes returned by the function of ec_poll?
>   
>> static int ec_poll(struct acpi_ec *ec)
>>     
> {
>         unsigned long delay = jiffies + msecs_to_jiffies(ACPI_EC_DELAY);
>         msleep(1);
> // Maybe the current jiffies is already after the predefined jiffies
> after msleep(1). In such case the ETIME will be returned. Of course the
> EC transaction can't be finished. If so, IMO this is not reasonable as
> this is caused by that OS has no opportunity to issue the following EC
> command sequence.
>         while (time_before(jiffies, delay)) {
>                 gpe_transaction(ec, acpi_ec_read_status(ec));
>                 msleep(1);
>                 if (ec_transaction_done(ec))
>                         return 0;
> //Maybe there exists the following cases. EC transaction is not finished
> after msleep(1),but the current jiffies is already after predefined
> jiffies. So ETIME is returned. In such case, IMO this is also not
> reasonable.
>         }
>         return -ETIME;
> }
>      At the same time msleep is realized by schedule_timeout. On linux
> although one process is waked up by some events, it won't be scheduled
> immediately. So maybe the current jiffies is already after the
> predefined timeout jiffies  after msleep(1). 
>     Although the possibility of this issue can be reduced by that msleep
> is replaced by udelay,maybe the issue still exists if the preempt
> schedule happens at the corresponding place.
>
>     In the above case the ETIME will be returned by ec_poll. But the
> reason is not that EC controller can't update its status in time.
> Instead it is caused by that host has no opportunity to issue the
> sequence operation in the current work flowchart. In current EC work
> flowchart the EC transaction is done in a big loop. 
>     
>     Maybe the better solution is that the EC transaction is explicitly
> divided into several different phases. 
>
>     Maybe my analysis is not correct. If so, please correct me. 
> Welcome the comments.
>
>     thanks.
>     
>     
>      
>   
>> Thanks,
>> Alex.
>>
>>
>>     
>
>   

Re: [2.6.28-rc2] EeePC ACPI errors & exceptions

[Zhao Yakui]

On Wed, 2008-10-29 at 17:29 +0800, Alexey Starikovskiy wrote:
> Not a problem, just find the root cause. Or shut up.
Maybe you don't read the explanation I have written.
> 
> Zhao Yakui wrote:
> > On Tue, 2008-10-28 at 13:46 -0700, Alexey Starikovskiy wrote:
> >   
> >> Hi Darren,
> >>
> >> Please check if the patch 
> >> http://marc.info/?l=linux-acpi&m=122516784917952&w=4
> >> helps.
> >>     
> > In the attached patch the msleep is replaced by udelay gain. 
> >    In the following commit the udelay is replaced by msleep. 
> >    >commit 1b7fc5aae8867046f8d3d45808309d5b7f2e036a
> >    >Author: Alexey Starikovskiy <astarikovskiy@suse.de>
> >    >Date:   Fri Jun 6 11:49:33 2008 -0400
> >      >ACPI: EC: Use msleep instead of udelay while waiting for event
> >    
> >    After the problem happens again, the udelay is restored again before
> > getting the root cause. 
> >    Maybe we should find the root cause of the problem and change the
> > working flowchart about the EC driver. It is inappropriate that we make
> > some changes and it is reverted again when the problem happens.
> >    
> >    At the same time after mlseep is replaced by the udelay, the CPU will
> > do thing but loop while doing EC transaction on some laptops (In the
> > function of ec_poll). If 100 EC transactions are done, the CPU will do
> > nothing but loop at least for 100*2*100 microseconds. In such case maybe
> > the performance will be affected.
> >
> >   After the following commit is merged, the EC transaction will be
> > executed in EC GPE interrupt context on most laptops.Maybe it is easier.
> > But for the some laptops it can't be done in EC GPE interrupt context.
> > So it falls back to the EC polling mode. (This is realized by the
> > function of ec_poll).
> >     >commit 7c6db4e050601f359081fde418ca6dc4fc2d0011
> >     >Author: Alexey Starikovskiy <astarikovskiy@suse.de>
> >     >Date:   Thu Sep 25 21:00:31 2008 +0400
> >        >ACPI: EC: do transaction from interrupt context
> >  
The following is the detailed explanation why this issue happens. In
fact after you sent your patch, I raise the issue about it. But it is
ignored. (Maybe the AE_TIME will be returned by EC driver. But the
reason is not caused by that EC controller can't update its status in
time. Instead it is caused by that host has no opportunity to issue the
sequence EC command.)
>   
> >    Why is AE_TIME sometimes returned by the function of ec_poll?
> >   
> >> static int ec_poll(struct acpi_ec *ec)
> >>     
> > {
> >         unsigned long delay = jiffies + msecs_to_jiffies(ACPI_EC_DELAY);
> >         msleep(1);
> > // Maybe the current jiffies is already after the predefined jiffies
> > after msleep(1). In such case the ETIME will be returned. Of course the
> > EC transaction can't be finished. If so, IMO this is not reasonable as
> > this is caused by that OS has no opportunity to issue the following EC
> > command sequence.
> >         while (time_before(jiffies, delay)) {
> >                 gpe_transaction(ec, acpi_ec_read_status(ec));
> >                 msleep(1);
> >                 if (ec_transaction_done(ec))
> >                         return 0;
> > //Maybe there exists the following cases. EC transaction is not finished
> > after msleep(1),but the current jiffies is already after predefined
> > jiffies. So ETIME is returned. In such case, IMO this is also not
> > reasonable.
> >         }
> >         return -ETIME;
> > }
> >      At the same time msleep is realized by schedule_timeout. On linux
> > although one process is waked up by some events, it won't be scheduled
> > immediately. So maybe the current jiffies is already after the
> > predefined timeout jiffies  after msleep(1). 
> >     Although the possibility of this issue can be reduced by that msleep
> > is replaced by udelay,maybe the issue still exists if the preempt
> > schedule happens at the corresponding place.
> >
> >     In the above case the ETIME will be returned by ec_poll. But the
> > reason is not that EC controller can't update its status in time.
> > Instead it is caused by that host has no opportunity to issue the
> > sequence operation in the current work flowchart. In current EC work
> > flowchart the EC transaction is done in a big loop. 
> >     
> >     Maybe the better solution is that the EC transaction is explicitly
> > divided into several different phases. 
> >
> >     Maybe my analysis is not correct. If so, please correct me. 
> > Welcome the comments.
> >
> >     thanks.
> >     
> >     
> >      
> >   
> >> Thanks,
> >> Alex.
> >>
> >>
> >>     
> >
> >   
> 

Re: [2.6.28-rc2] EeePC ACPI errors & exceptions

[Alexey Starikovskiy]

Zhao Yakui wrote:
> On Wed, 2008-10-29 at 17:29 +0800, Alexey Starikovskiy wrote:
>   
>> Not a problem, just find the root cause. Or shut up.
>>     
> Maybe you don't read the explanation I have written.
>   
Found root cause should not have "maybe" before it.