On 10.08.2017 03:35, Lu Baolu wrote:
Hi,

On 08/09/2017 03:58 PM, Mathias Nyman wrote:
On 27.07.2017 05:21, Lu Baolu wrote:
xhci_disable_slot() is a helper for disabling a slot when a device
goes away or recovers from error situations. Currently, it checks
the corespoding virt-dev pointer and returns directly (w/o issuing
disable slot command) if it's null.

This is unnecessary and will cause problems in case where virt-dev
allocation fails and xhci_disable_slot() is called to roll back the
hardware state. Refer to the implementation of xhci_alloc_dev().


True, checking for xhci->devs[slot_id] doesn't work if xhci_alloc_dev()
failed xhci_alloc_virt_device() and calls xhci_disable_slot,

but the virt dev check is needed for test mode, which will just try
to disable all slots from 1 to HCS_MAX_SLOTS:

  xhci_enter_test_mode(..)
        /* Disable all Device Slots */
     for (i = 1; i <= HCS_MAX_SLOTS(xhci->hcs_params1); i++) {
                 retval = xhci_disable_slot(xhci, NULL, i);



So, how about checking virt dev before this invoking?

@@ -612,10 +612,12 @@ static int xhci_enter_test_mode(struct xhci_hcd *xhci,
         xhci_dbg(xhci, "Disable all slots\n");
         spin_unlock_irqrestore(&xhci->lock, *flags);
         for (i = 1; i <= HCS_MAX_SLOTS(xhci->hcs_params1); i++) {
-               retval = xhci_disable_slot(xhci, NULL, i);
-               if (retval)
-                       xhci_err(xhci, "Failed to disable slot %d, %d. Enter test 
mode anyway\n",
-                                i, retval);
+               if (xhci->devs[i]) {
+                       retval = xhci_disable_slot(xhci, NULL, i);
+                       if (retval)
+                               xhci_err(xhci, "Failed to disable slot %d, %d. Enter 
test mode anyway\n",
+                                        i, retval);
+               }


Yes, something like this will work

Thanks
Mathias

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to