> -----Original Message----- > From: Sebastian Andrzej Siewior [mailto:bigeasy@xxxxxxxxxxxxx] > Sent: Friday, July 29, 2011 4:54 PM > To: Xu, Andiry > Cc: sarah.a.sharp@xxxxxxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx; > gregkh@xxxxxxx; stern@xxxxxxxxxxxxxxxxxxx > Subject: Re: [PATCH 7/8] xHCI: test USB2 software LPM > > * Xu, Andiry | 2011-07-27 09:33:53 [+0800]: > > >> Would it make more sense to hold a in-kernel list with such devices > >and > >> avoid future tests (after a reboot) for this device like it is done > >> with > >> unusual devices for usb-storage? However the vendor could supply a > >> firmware update which fixes this LPM issue. > >> > > > >I'm not sure about the failure reasons. I'm using USB3 devices to test > >LPM, and connect them via a USB2 cable to force them working in USB2 > >mode. > >Some devices passed the test, some failed to suspend and some failed > to > >resume back into normal state. The behavior is also different on > >different > >xHCI hosts: a device pass the test on one host but fails to resume on > >another. So I'd like to keep the devices list inside xHCI before we > know > >that certain devices truly can not perform LPM. > > This is truly awesome. Please note this observation in your patch > description. OK. > So you have repeatedly the same results on one xhci host? Yes. The result is consistent on the same xHCI host. > And by different xhci-hosts you mean different IP-vendors i.e. Nec vs > AMD or something like that? > Yes, different xhci hosts means hosts from different IP vendors. > >> >diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > >> >index 95c6d91..fb636ed 100644 > >> >--- a/drivers/usb/host/xhci.c > >> >+++ b/drivers/usb/host/xhci.c > >> >@@ -2900,6 +2900,126 @@ int xhci_address_device(struct usb_hcd *hcd, > >> struct usb_device *udev) > >> > return 0; > >> > } > >> > > >> >+static int xhci_usb2_software_lpm_test(struct xhci_hcd *xhci, > >> >+ struct usb_device *udev, unsigned long > >flags) > >> >+{ > >> ... > >> >+ > >> >+ /* Set port link state to U2(L1) */ > >> >+ addr = port_array[port_num]; > >> >+ xhci_set_link_state(xhci, port_array, port_num, XDEV_U2); > >> >+ > >> >+ /* wait for ACK */ > >> >+ spin_unlock_irqrestore(&xhci->lock, flags); > >> >+ msleep(10); > >> >+ spin_lock_irqsave(&xhci->lock, flags); > >> > >> are you dropping the lock because you are waiting for the interrupt? > >> > > > >There may be an interrupt, maybe not, depends on the result of L1 > >suspend. > >If suspend fails, there will be an interrupt and a Port Status Change > >Event is occurred. If the device successfully enter L1 state, there'll > >be > >no interrupts or events. > > I see. > > > > >> .... > >> > >> >+ if (ret) { > >> >+ /* Insert dev to lpm_failed_devs list */ > >> >+ xhci_warn(xhci, "device LPM test failed, may disconnect > >and > >> " > >> >+ "re-enumerate\n"); > >> >+ dev_info = kzalloc(sizeof(struct dev_info), GFP_ATOMIC); > >> > >> 20 bytes on 64bit for one entry with GFP_ATOMIC. What about an array > >> with 3 entries for instance? Once the last last slot is used you > clean > >> the first one. There should not be that much of "faulty" devices in > >one > >> system or is likely? > >> > > > >I've found several LPM failed devices already, so there are "faulty" > >devices > >exist. Your advice saves memory and lookup time, but devices may be > >tested > >several times during the xHCI driver running period. > > They are anyway as you re-test the good devices on re-connect. Is it > possible that a device fails the test behind a hub but passes it if it > is directly connected? Too bad the device can't tell why it failed. > The test is for devices connected to root hub only. I think re-test The good devices is OK, as long as it does not affect devices's normal function. > On USB3.0 you have to send a "Set Feature" request U2_ENBALE I think. I > don't see that this is done in your USB2.0 case. Is this automaticaly > done by the xhci controller in xhci_set_link_state? > I assume SetFeature(U2_ENABLE) is applied to USB3 devices only. USB2.0 does not has this feature selector. Thanks, Andiry -- 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
