On Thu, 17 Nov 2011, Geoff Levand wrote: > Hi Alan, > > I tried to give some verbose comments to help you understand the problem. I > would greatly appreciate any advice or test suggestions you have. > > On Tue, 2011-11-15 at 11:35 -0500, Alan Stern wrote: > On Tue, 15 Nov 2011, Geoff Levand wrote: > > > > > The PS3 EHCI HC stops the root hub after both root hub ports are > > > suspended. The current EHCI driver can't handle this behavior. > > > Three workarounds are needed in the PS3 bus glue. > > > > Can you be more specific? What exactly do you mean by "stops the root > > hub"? > > Maybe it is not the root hub, but the HC that is 'stopped'. Here is what I > observe: > > This is 100% repeatable. With a kernel build with PM support and USB debug > messages enabled, and with no external USB devices attached to the system soon > after startup messages like these are seen: > > ps3-ehci-driver sb_08: port 1 high speed > ps3-ehci-driver sb_08: GetStatus port:1 status 001005 0 ACK POWER sig=se0 PE CONNECT > ps3-ehci-driver sb_08: port 2 high speed > ps3-ehci-driver sb_08: GetStatus port:2 status 001005 0 ACK POWER sig=se0 PE CONNECT > usb usb3: bus auto-suspend, wakeup 1 > usb usb4: bus auto-suspend, wakeup 1 > usb 1-1: unlink qh256-0001/c000000006238180 start 1 [1/0 us] > usb 1-2: unlink qh256-0001/c000000006229580 start 2 [1/0 us] > usb usb1: bus auto-suspend, wakeup 1 > ps3-ehci-driver sb_06: suspend root hub > ps3-ehci-driver sb_06: force halt; handshake d000080080020014 0000c000 00000000 -> -110 > > Adding a dump_stack() call to handshake_on_error_set_halt() gives this: > > ps3-ehci-driver sb_06: suspend root hub > Call Trace: > [c00000000610b610] [c000000000011248] .show_stack+0x6c/0x16c (unreliable) > [c00000000610b6c0] [c0000000002e5fdc] .handshake_on_error_set_halt+0x48/0xa8 > [c00000000610b760] [c0000000002e60c4] .ehci_quiesce+0x88/0x148 > [c00000000610b7e0] [c0000000002e9810] .ehci_bus_suspend+0x11c/0x514 > [c00000000610b890] [c0000000002cecd0] .hcd_bus_suspend+0xec/0x1a8 > [c00000000610b930] [c0000000002df390] .generic_suspend+0x20/0x58 > [c00000000610b9a0] [c0000000002d6110] .usb_suspend_both+0x1d0/0x2c4 > [c00000000610ba60] [c0000000002d62dc] .usb_runtime_suspend+0xd8/0x120 > [c00000000610bae0] [c00000000029b304] .__rpm_callback+0x5c/0xa4 > [c00000000610bb70] [c00000000029b9a8] .rpm_suspend+0x318/0x5a4 > [c00000000610bc80] [c00000000029cda4] .pm_runtime_work+0xa8/0xdc > [c00000000610bd10] [c000000000069edc] .process_one_work+0x2cc/0x4ac > [c00000000610bdf0] [c00000000006a53c] .worker_thread+0x268/0x3f4 > [c00000000610bea0] [c000000000071dcc] .kthread+0xb0/0xbc > [c00000000610bf90] [c00000000001c138] .kernel_thread+0x54/0x70 > ps3-ehci-driver sb_06: force halt; handshake d000080080020014 0000c000 00000000 -> -110 > > The first call to handshake_on_error_set_halt() in ehci_quiesce() is failing > with ETIMEDOUT (-100). After a bit of searching I found what seemed like odd > behavior that was related to the handshake failure. If I add this routine to > ehci-hcd.c: > > +#define ps3_frame_index_check(_a) _ps3_frame_index_check(_a, __func__, __LINE__) > +static int _ps3_frame_index_check(struct ehci_hcd *ehci, const char *func, int line) > +{ > + unsigned int old_stamp = ehci_read_frame_index(ehci); > + unsigned int i; > + > + udelay(125); > + > + for (i = 20; i; i--) { > + if (ehci_read_frame_index(ehci) == old_stamp) > + printk("."); > + else > + break; > + udelay(125); > + } > + > + if (i) { > + printk("%s:%d: frame_index: ok\n", func, line); > + return 0; > + } > + > + printk("\n%s:%d: frame_index: stopped\n", func, line); > + > + return -1; > +} > > Then add these to the SetPortFeature + USB_PORT_FEAT_SUSPEND case of > ehci_hub_control(): > > case USB_PORT_FEAT_SUSPEND: > + ehci_dbg(ehci, "port %d SUSPEND\n", wIndex + 1); > + ps3_frame_index_check(ehci); > if (ehci->no_selective_suspend) > break; > if ((temp & PORT_PE) == 0 > || (temp & PORT_RESET) != 0) > goto error; > > /* After above check the port must be connected. > * Set appropriate bit thus could put phy into low power > * mode if we have hostpc feature > */ > temp &= ~PORT_WKCONN_E; > temp |= PORT_WKDISC_E | PORT_WKOC_E; > ehci_writel(ehci, temp | PORT_SUSPEND, status_reg); > if (hostpc_reg) { > spin_unlock_irqrestore(&ehci->lock, flags); > msleep(5);/* 5ms for HCD enter low pwr mode */ > spin_lock_irqsave(&ehci->lock, flags); > temp1 = ehci_readl(ehci, hostpc_reg); > ehci_writel(ehci, temp1 | HOSTPC_PHCD, > hostpc_reg); > temp1 = ehci_readl(ehci, hostpc_reg); > ehci_dbg(ehci, "Port%d phy low pwr mode %s\n", > wIndex, (temp1 & HOSTPC_PHCD) ? > "succeeded" : "failed"); > } > set_bit(wIndex, &ehci->suspended_ports); > + udelay(3000); > + ps3_frame_index_check(ehci); > break; > > I get this output: > > ps3-ehci-driver sb_06: port 1 SUSPEND > ehci_hub_control:976: frame_index: ok > ehci_hub_control:1004: frame_index: ok > ps3-ehci-driver sb_06: port 2 SUSPEND > ehci_hub_control:976: frame_index: ok > .................... > ehci_hub_control:1004: frame_index: stopped > usb usb1: bus auto-suspend, wakeup 1 > ps3-ehci-driver sb_06: suspend root hub > ps3-ehci-driver sb_06: force halt; handshake d000080080020014 0000c000 00000000 -> -110 > > If I change the delay from udelay(3000) to udelay(2500) the final > ps3_frame_index_check() will succeed, but the ehci_quiesce handshake will > fail. > > I don't think the frame index should stop incrementing after the second port is > suspended, and I think the cause is the reason the ehci_quiesce handshake > fails. Yes indeed. So it appears the root hub really does get stopped when all the ports are suspended. Can you add a call to dbg_status() in _ps3_frame_index_check()? I'd like to know if the STS_HALT bit is set. > > Does the controller do this if only one port is suspended and the other > > is not enabled? What if neither port is enabled? > > I have not tested these. It would be nice to know. Is there any way you can test it? If those hubs are internal and permanently connected then I guess you can't. Do the controllers really have only two ports? > > > kernel BUG at drivers/usb/host/ehci-mem.c:74! > > > > This means some qh was still linked (or something similar) when it was > > removed. I don't see how these are connected to the root hub stopping. > > I think it is because when the HC frame index stops incrementing the qh > processing does not finish up, so it is still linked. I would expect the IAA watchdog timer to catch something like that. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html