> -----Original Message----- > From: Sebastian Andrzej Siewior [mailto:bigeasy@xxxxxxxxxxxxx] > Sent: Tuesday, July 26, 2011 7:24 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 > > * Andiry Xu | 2011-07-26 08:37:37 [+0800]: > > >This patch tests USB2 software LPM for a USB2 LPM-capable device. > > > >When a lpm-capable device is addressed, if the host also supports > software > >LPM, apply a test by putting the device into L1 state and resume it to > see > >if the device can do L1 suspend/resume successfully. > > > >If the device fails to enter L1 or resume from L1 state, it may not > >function normally and usbcore may disconnect and re-enumerate it. In > this > >case, store the device's Vid and Pid information, make sure the host > will > >not test LPM for it twice. > > So the device announces that it is capable of doing LPM but once you > tell > it to do so, the device doesn't work anymore. > Is it likely that this can be caused by a bad cable or something like > that or is it always a bad UDC (not implemented LPM feature). > > 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. > >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. > .... > > >+ 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. 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
