[PATCH] x86, olpc: Wait for last byte of EC command to be accepted

[PATCH] x86, olpc: Wait for last byte of EC command to be accepted

[Daniel Drake]

From: Paul Fox <pgf@laptop.org>

When executing EC commands, only waiting when there are still more bytes to
write is usually fine. However, if the system suspends very quickly after
a call to olpc_ec_cmd(), the last data byte may not yet be transferred to
the EC, and the command will not complete.

This solves a bug where the SCI wakeup mask was not correctly written
when going into suspend.

Signed-off-by: Paul Fox <pgf@laptop.org>
Signed-off-by: Daniel Drake <dsd@laptop.org>
---
 arch/x86/platform/olpc/olpc.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/platform/olpc/olpc.c b/arch/x86/platform/olpc/olpc.c
index 8b9940e..7cce722 100644
--- a/arch/x86/platform/olpc/olpc.c
+++ b/arch/x86/platform/olpc/olpc.c
@@ -161,13 +161,13 @@ restart:
 	if (inbuf && inlen) {
 		/* write data to EC */
 		for (i = 0; i < inlen; i++) {
+			pr_devel("olpc-ec:  sending cmd arg 0x%x\n", inbuf[i]);
+			outb(inbuf[i], 0x68);
 			if (wait_on_ibf(0x6c, 0)) {
 				printk(KERN_ERR "olpc-ec:  timeout waiting for"
 						" EC accept data!\n");
 				goto err;
 			}
-			pr_devel("olpc-ec:  sending cmd arg 0x%x\n", inbuf[i]);
-			outb(inbuf[i], 0x68);
 		}
 	}
 	if (outbuf && outlen) {
-- 
1.7.6

Re: [PATCH] x86, olpc: Wait for last byte of EC command to be accepted

[Andres Salomon]

Acked-by: Andres Salomon <dilinger@queued.net>

After some squinting.. This patch looks correct.  This is a
long-standing issue, we should probably add a Cc: stable@kernel.org so
it gets picked up for stable kernels.

On Tue, 26 Jul 2011
16:42:26 +0100 (BST) Daniel Drake <dsd@laptop.org> wrote:

> From: Paul Fox <pgf@laptop.org>
> 
> When executing EC commands, only waiting when there are still more
> bytes to write is usually fine. However, if the system suspends very
> quickly after a call to olpc_ec_cmd(), the last data byte may not yet
> be transferred to the EC, and the command will not complete.
> 
> This solves a bug where the SCI wakeup mask was not correctly written
> when going into suspend.
> 
> Signed-off-by: Paul Fox <pgf@laptop.org>
> Signed-off-by: Daniel Drake <dsd@laptop.org>
> ---
>  arch/x86/platform/olpc/olpc.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/platform/olpc/olpc.c
> b/arch/x86/platform/olpc/olpc.c index 8b9940e..7cce722 100644
> --- a/arch/x86/platform/olpc/olpc.c
> +++ b/arch/x86/platform/olpc/olpc.c
> @@ -161,13 +161,13 @@ restart:
>  	if (inbuf && inlen) {
>  		/* write data to EC */
>  		for (i = 0; i < inlen; i++) {
> +			pr_devel("olpc-ec:  sending cmd arg 0x%x\n",
> inbuf[i]);
> +			outb(inbuf[i], 0x68);
>  			if (wait_on_ibf(0x6c, 0)) {
>  				printk(KERN_ERR "olpc-ec:  timeout
> waiting for" " EC accept data!\n");
>  				goto err;
>  			}
> -			pr_devel("olpc-ec:  sending cmd arg 0x%x\n",
> inbuf[i]);
> -			outb(inbuf[i], 0x68);
>  		}
>  	}
>  	if (outbuf && outlen) {

Re: [PATCH] x86, olpc: Wait for last byte of EC command to be accepted

[Ingo Molnar]

* Daniel Drake <dsd@laptop.org> wrote:

> From: Paul Fox <pgf@laptop.org>
> 
> When executing EC commands, only waiting when there are still more bytes to
> write is usually fine. However, if the system suspends very quickly after
> a call to olpc_ec_cmd(), the last data byte may not yet be transferred to
> the EC, and the command will not complete.
> 
> This solves a bug where the SCI wakeup mask was not correctly written
> when going into suspend.

What did that result in - a resume hang? (i.e. how urgent is this fix 
- v3.1 material or v3.2 material?)

Thanks,

	Ingo

Re: [PATCH] x86, olpc: Wait for last byte of EC command to be accepted

[Daniel Drake]

On Fri, Aug 5, 2011 at 11:43 AM, Ingo Molnar <mingo@elte.hu> wrote:
> What did that result in - a resume hang? (i.e. how urgent is this fix
> - v3.1 material or v3.2 material?)

Sorry - the explanation is indeed lacking in that area!

It means that sometimes, on XO-1.5 (but not XO-1), the devices that
were marked as wakeup sources can't wake up the system. e.g. you ask
for wifi wakeups, suspend, but then incoming wifi frames don't wake up
the system as they should.

In my opinion, as a simple bugfix (verified by 2 of our EC firmware
developers) it is 3.1 material.

Thanks,
Daniel