Re: [PATCH 8/9 v2] xHCI: test USB2 software LPM

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



* Andiry Xu | 2011-08-18 14:50:20 [+0800]:

>I only observe one device shows different behaviors on different IP
>hosts. Like:
>
>IP	A	B
>DevA	pass	pass
>DevB	fail	pass
>DevC	fail	fail
>
>I only have two hosts supporting LPM, so I'm not sure what behavior
>other IP hosts will have. 

I see so i tis more or less random.

>> >diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
>> >index 3a0f695..fce413a 100644
>> >--- a/drivers/usb/host/xhci.c
>> >+++ b/drivers/usb/host/xhci.c
>> >@@ -2957,6 +2961,166 @@ int xhci_address_device(struct usb_hcd *hcd, struct usb_device *udev)
>> > 	return 0;
>> > }
>> > 
>> >+/* Calculate HIRD/BESL for USB2 PORTPMSC*/
>> >+static int xhci_calculate_hird_besl(int u2del, bool use_besl)
>> >+{
>> >+	int hird;
>> >+
>> >+	if (use_besl) {
>> >+		for (hird = 0; hird < 16; hird++) {
>> 
>> ARRAY_SIZE_OF
>
>>From the spec, the besl encoding array is fixed with 16 elements.

yes, I know. Therefore your array has 16 elements.

>> >+	/* Look for devices in lpm_failed_devs list */
>> >+	dev_id = le16_to_cpu(udev->descriptor.idVendor) << 16 |
>> >+			le16_to_cpu(udev->descriptor.idProduct);
>> >+	list_for_each_entry(dev_info, &xhci->lpm_failed_devs, list) {
>> >+		if (dev_info->dev_id == dev_id)
>> >+			return -EINVAL;
>> >+	}
>> 
>> I'm still not really convinced about the list for each device. If the
>> device passes the test you re-test it on the next plug-in. So why not
>> re-test it in the fail case if your static array is full?
>> Can you quote some numbers about the time a successfull and a failed
>> test needs? According to the code would say about 20ms in both cases
>> (unless it is already in the list).
>
>>From my observation, the test result is constant: a device always pass
>or always fail on the same host. Re-test a passed device does not
>involve much effort, but re-test a failed device may involve transfer
>error, disconnection and re-initialization. 

Ah. By disconnection you mean physical interaction?

>Actually LPM suspend and resume does not need 20ms, it's designed to
>suspend/resume very quickly. Here I just use msleep to make sure it
>enters the right link state.

Sure but your mdelay() is never skipped, it is always executed so you
always spend this time.

Would it make sense to make this test only when LPM is requested which
is only the case when the user requests it? One could add a message in
powertop which would warn the user that this 2.1 device could fail to
suspend.
What do you think?

>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


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux