2012/3/15 Xu, Andiry <Andiry.Xu@xxxxxxx>: >> -----Original Message----- >> From: linux-usb-owner@xxxxxxxxxxxxxxx [mailto:linux-usb- >> owner@xxxxxxxxxxxxxxx] On Behalf Of Elric Fu >> Sent: Thursday, March 15, 2012 1:46 PM >> To: Sarah Sharp >> Cc: Elric Fu; linux-usb@xxxxxxxxxxxxxxx >> Subject: [PATCH 2/2] xHCI: fix bug of resume for VIA xHCI host >> >> The Save/Restore state funtion of VIA xHCI host is unstable. So add >> the XHCI_RESET_ON_RESUME quirk for it. >> > > Can you provide more background of the issue? What's the unstable > behavior of VIA host after resume? The issue of using the Save/Restore state operation will cause the device attached to the port of host is disconnected. > >> When driver free Event Ring in the resume procedure, driver can't >> write zero to Event Ring Registers in VIA xHCI host, otherwise the >> VIA host can't be resumed. >> > > Strange. What do you mean by "can't resume"? Does xhci_resume() return > failure, or it returns success but host does not work after resume? The issue is the host will hang up. The reason is that the host has a hardware state machine, in the reset-on-resume procedure driver will reset the host first then free event ring and re-initialize the host. But if driver set 0 to event ring registers in xhci_mem_cleanup() after reset the host, the host will think the "0" is a vaild value, and fetch it. it result in the host hang up. > > If the host needs a reset-on-resume, it will perform a complete > re-initialization. After xhci_mem_cleanup() called, xhci_resume() will > call xhci_init(), in which the ERST registers are re-written. Have you > checked the value, see if it's different form before suspend? I don't think in reset-on-resume write 0 to event Ring registers is necessary. As driver already reset the host before xhci_mem_cleanup(), the value of event ring registers will be 0. > > Also, note xhci_mem_cleanup is called not only by xhci_resume(), but also > by xhci_stop(). This means you may find the same issue by rmmod/modprobe > xhci-hcd. rmmod/modprobe xhci-hcd is ok. I have already tested it. I think it is ok if we write a valid value except for 0 to event ring registers in VIA xHCI host after reset the host. Best Regards, Elric Fu > > Thanks, > Andiry > >> Signed-off-by: Elric Fu <elricfu1@xxxxxxxxx> >> --- >> drivers/usb/host/xhci-mem.c | 2 +- >> drivers/usb/host/xhci-pci.c | 1 + >> 2 files changed, 2 insertions(+), 1 deletions(-) >> >> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c >> index 383fc85..9321837 100644 >> --- a/drivers/usb/host/xhci-mem.c >> +++ b/drivers/usb/host/xhci-mem.c >> @@ -1690,7 +1690,7 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci) >> int i; >> >> /* Free the Event Ring Segment Table and the actual Event Ring */ >> - if (xhci->ir_set) { >> + if (xhci->ir_set && !(xhci->quirks & XHCI_VIA_HOST)) { >> xhci_writel(xhci, 0, &xhci->ir_set->erst_size); >> xhci_write_64(xhci, 0, &xhci->ir_set->erst_base); >> xhci_write_64(xhci, 0, &xhci->ir_set->erst_dequeue); >> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c >> index 872d024..75350ea 100644 >> --- a/drivers/usb/host/xhci-pci.c >> +++ b/drivers/usb/host/xhci-pci.c >> @@ -106,6 +106,7 @@ static void xhci_pci_quirks(struct device *dev, struct >> xhci_hcd *xhci) >> } >> if (pdev->vendor == PCI_VENDOR_ID_VIA) { >> xhci->quirks |= XHCI_VIA_HOST; >> + xhci->quirks |= XHCI_RESET_ON_RESUME; >> xhci_via_get_fw(xhci, pdev); >> } >> } >> -- >> 1.7.1 >> >> -- >> 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 > > -- 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
