On Thu, 2011-08-25 at 18:29 +0200, Sebastian Andrzej Siewior wrote:
> * 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?
>
No, usbcore will disconnect it and initialize it again.
> >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?
>
You mean module parameter, or have some switch
in /sys/bus/usb/devices/x-x/power/ ?
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
