[PATCH] for acpi S1 power cycle resume problems

[PATCH] for acpi S1 power cycle resume problems

[William Morrow]

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

Hi
I was told that if I had a patch to submit for a baseline change that 
this was the place to do it.
If not, please let me know...

thanks,
morrow

Patched against 2.6.11 baseline
problems fixed:
1) OHCI_INTR_RD not being cleared in ohci interrupt handler
 results in interrupt storm and system hang on RD status.
 ohci spec indicates this should be done.
2) PORT_CSC not being cleared in ehci_hub_status_data
 code attempts to clear bit, but bit is write to clear.
 there are other errant clears, since the PORTSCn regs
 have 3 RWC bits, and the rest are RW. All stmts of the form:
   writel (v, &ehci->regs->port_status[i])
 should clear RWC bits if they do not intend to clear status,
 and should set the bits which should be cleared (this case).
3) loop control and subsequent port resume/reset not correct.
 unsigned index made detecting port1 active impossible, and
 OWNER/POWER status was being ignored on ports assigned
 to companion controller.

Signed-off-by: Jordan Crouse <jordan.crouse@amd.com>


[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 3145 bytes --]

diff -uprN linux-2.6.11.orig/drivers/usb/host/ehci.h linux-2.6.11/drivers/usb/host/ehci.h
--- linux-2.6.11.orig/drivers/usb/host/ehci.h	2005-03-02 00:38:25.000000000 -0700
+++ linux-2.6.11/drivers/usb/host/ehci.h	2005-08-17 08:15:36.000000000 -0600
@@ -262,6 +262,7 @@ struct ehci_regs {
 #define PORT_PE		(1<<2)		/* port enable */
 #define PORT_CSC	(1<<1)		/* connect status change */
 #define PORT_CONNECT	(1<<0)		/* device connected */
+#define PORT_RWC_BITS   (PORT_CSC | PORT_PEC | PORT_OCC)
 } __attribute__ ((packed));
 
 /* Appendix C, Debug port ... intended for use with special "debug devices"
diff -uprN linux-2.6.11.orig/drivers/usb/host/ehci-hcd.c linux-2.6.11/drivers/usb/host/ehci-hcd.c
--- linux-2.6.11.orig/drivers/usb/host/ehci-hcd.c	2005-03-02 00:38:38.000000000 -0700
+++ linux-2.6.11/drivers/usb/host/ehci-hcd.c	2005-08-17 08:15:36.000000000 -0600
@@ -722,7 +722,7 @@ static int ehci_suspend (struct usb_hcd 
 static int ehci_resume (struct usb_hcd *hcd)
 {
 	struct ehci_hcd		*ehci = hcd_to_ehci (hcd);
-	unsigned		port;
+	int			port;
 	struct usb_device	*root = hcd->self.root_hub;
 	int			retval = -EINVAL;
 	int			powerup = 0;
@@ -733,11 +733,11 @@ static int ehci_resume (struct usb_hcd *
 		msleep (100);
 
 	/* If any port is suspended, we know we can/must resume the HC. */
-	for (port = HCS_N_PORTS (ehci->hcs_params); port > 0; ) {
+	for (port = HCS_N_PORTS (ehci->hcs_params); --port >= 0; ) {
 		u32	status;
-		port--;
 		status = readl (&ehci->regs->port_status [port]);
-		if (status & PORT_SUSPEND) {
+		if ( (status & PORT_SUSPEND) != 0 ||
+		    ((status & PORT_OWNER) != 0 && (status & PORT_POWER) != 0) ) {
 			down (&hcd->self.root_hub->serialize);
 			retval = ehci_hub_resume (hcd);
 			up (&hcd->self.root_hub->serialize);
@@ -755,7 +755,7 @@ static int ehci_resume (struct usb_hcd *
 	/* Else reset, to cope with power loss or flush-to-storage
 	 * style "resume" having activated BIOS during reboot.
 	 */
-	if (port == 0) {
+	if (port < 0) {
 		(void) ehci_halt (ehci);
 		(void) ehci_reset (ehci);
 		(void) ehci_hc_reset (hcd);
diff -uprN linux-2.6.11.orig/drivers/usb/host/ehci-hub.c linux-2.6.11/drivers/usb/host/ehci-hub.c
--- linux-2.6.11.orig/drivers/usb/host/ehci-hub.c	2005-03-02 00:38:32.000000000 -0700
+++ linux-2.6.11/drivers/usb/host/ehci-hub.c	2005-08-17 08:15:36.000000000 -0600
@@ -232,7 +232,8 @@ ehci_hub_status_data (struct usb_hcd *hc
 		if (temp & PORT_OWNER) {
 			/* don't report this in GetPortStatus */
 			if (temp & PORT_CSC) {
-				temp &= ~PORT_CSC;
+				temp &= ~PORT_RWC_BITS;
+				temp |= PORT_CSC;
 				writel (temp, &ehci->regs->port_status [i]);
 			}
 			continue;
diff -uprN linux-2.6.11.orig/drivers/usb/host/ohci-hcd.c linux-2.6.11/drivers/usb/host/ohci-hcd.c
--- linux-2.6.11.orig/drivers/usb/host/ohci-hcd.c	2005-03-02 00:37:48.000000000 -0700
+++ linux-2.6.11/drivers/usb/host/ohci-hcd.c	2005-08-17 08:15:36.000000000 -0600
@@ -720,6 +720,7 @@ static irqreturn_t ohci_irq (struct usb_
 
 	if (ints & OHCI_INTR_RD) {
 		ohci_vdbg (ohci, "resume detect\n");
+		ohci_writel (ohci, OHCI_INTR_RD, &regs->intrstatus);
 		schedule_work(&ohci->rh_resume);
 	}
 

Re: [PATCH] for acpi S1 power cycle resume problems

[David Brownell]

> Date: Fri, 19 Aug 2005 08:39:25 -0600
> From: "William Morrow" <William.Morrow@amd.com>
> Subject: [PATCH] for acpi S1 power cycle resume problems
>
>
> Hi
> I was told that if I had a patch to submit for a baseline change that 
> this was the place to do it.

In this case that works fine.  Normally they should go to linux-usb-devel
for me (and others) to read there.

Thanks, these need a bit of cleaning up, finishing, and splitting out;
they should be in 2.6.14 though.  Comments below.  Were these patches
written by you, or by Jordan?

- Dave



> If not, please let me know...
>
> thanks,
> morrow
>
> Patched against 2.6.11 baseline
> problems fixed:
> 1) OHCI_INTR_RD not being cleared in ohci interrupt handler
>  results in interrupt storm and system hang on RD status.
>  ohci spec indicates this should be done.

Yeah, I noticed that one but didn't fix it yet.  It's not that
it was _never_ cleared ... only certain code paths missed it.
The systems I test with were clearly using those working paths!

Having this fixed should help get rid of the 1/4 second timer
this driver normally ties up.  That'll help make the dynamic
tick stuff work better, reducing power even when something like
"ACPI S1" doesn't exist (like say, on that one Zaurus).


> 2) PORT_CSC not being cleared in ehci_hub_status_data
>  code attempts to clear bit, but bit is write to clear.
>  there are other errant clears, since the PORTSCn regs
>  have 3 RWC bits, and the rest are RW. All stmts of the form:
>    writel (v, &ehci->regs->port_status[i])
>  should clear RWC bits if they do not intend to clear status,
>  and should set the bits which should be cleared (this case).

Yeah, whoever did that RWC patch for UHCI ports certainly should
have checked other HCDs for the same bug.  (Kicks self.)

In fact you didn't fix this issue comprehensively.  There are
other places that register is written; they need to change too.

This is clearly wrong, but did you notice any effects more
serious than "lsusb -v" output for EHCI root hubs looking
a bit strange?


> 3) loop control and subsequent port resume/reset not correct.
>  unsigned index made detecting port1 active impossible,

Odd, I've done that with some regularity.  Is that maybe
some kind of compiler bug?  (I heard even 4.1 isn't quite
there yet for kernels.)

The looping doesn't look incorrect to me; ports are numbered
from 1..N, and C code in the body must index them from 0..(N-1).


> and OWNER/POWER status was being ignored on ports assigned
>  to companion controller.

Well, in that one resume case anyway!

But OWNER and POWER are very different status bits ... if POWER
ever goes off, that port is by definition not resumable.  But
if a port's owned by the companion (OHCI or UHCI) controller,
then it surely ought not to be reset (even if the companion's
own SUSPEND bit doesn't show through EHCI).