Re: [PATCH] for acpi S1 power cycle resume problems
> Date: Fri, 19 Aug 2005 08:39:25 -0600 > From: "William Morrow" <[EMAIL PROTECTED]> > 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, >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). - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] for acpi S1 power cycle resume problems
Date: Fri, 19 Aug 2005 08:39:25 -0600 From: William Morrow [EMAIL PROTECTED] 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). - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[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.
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, >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 <[EMAIL PROTECTED]>
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.0
-0700
+++ linux-2.6.11/drivers/usb/host/ehci.h2005-08-17 08:15:36.0
-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.0 -0700
+++ linux-2.6.11/drivers/usb/host/ehci-hcd.c2005-08-17 08:15:36.0
-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);
- unsignedport;
+ 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 (>regs->port_status [port]);
- if (status & PORT_SUSPEND) {
+ if ( (status & PORT_SUSPEND) != 0 ||
+ ((status & PORT_OWNER) != 0 && (status & PORT_POWER) != 0)
) {
down (>self.root_hub->serialize);
retval = ehci_hub_resume (hcd);
up (>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.0 -0700
+++ linux-2.6.11/drivers/usb/host/ehci-hub.c2005-08-17 08:15:36.0
-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, >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.0 -0700
+++ linux-2.6.11/drivers/usb/host/ohci-hcd.c2005-08-17 08:15:36.0
-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, >intrstatus);
schedule_work(>rh_resume);
}
[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.
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 [EMAIL PROTECTED]
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.0
-0700
+++ linux-2.6.11/drivers/usb/host/ehci.h2005-08-17 08:15:36.0
-0600
@@ -262,6 +262,7 @@ struct ehci_regs {
#define PORT_PE(12) /* port enable */
#define PORT_CSC (11) /* connect status change */
#define PORT_CONNECT (10) /* 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.0 -0700
+++ linux-2.6.11/drivers/usb/host/ehci-hcd.c2005-08-17 08:15:36.0
-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);
- unsignedport;
+ 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.0 -0700
+++ linux-2.6.11/drivers/usb/host/ehci-hub.c2005-08-17 08:15:36.0
-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.0 -0700
+++ linux-2.6.11/drivers/usb/host/ohci-hcd.c2005-08-17 08:15:36.0
-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);
}
