> -----Original Message----- > From: Elric Fu [mailto:elricfu1@xxxxxxxxx] > Sent: Thursday, March 15, 2012 3:41 PM > To: Xu, Andiry > Cc: Sarah Sharp; linux-usb@xxxxxxxxxxxxxxx > Subject: Re: [PATCH 2/2] xHCI: fix bug of resume for VIA xHCI host > > 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. > Hmm. I think xHCI suspend path may need to be save DCBAA and all related data structures before remove core well power. It's specified by xHCI 1.0 errata. > > > >> 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. > xhci_halt() is called before xhci_reset(). Why does the host fetch event ring registers after it's halted? > > > > 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. > But xhci_stop() also calls xhci_reset(), then xhci_mem_cleanup(). It also writes 0 to ERST registers after reset the host. 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
