* 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.
So you have repeatedly the same results on one xhci host?
And by different xhci-hosts you mean different IP-vendors i.e. Nec vs
AMD or something like that?
>> >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.
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?
>Thanks,
>Andiry
Sebastian
--
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
