Hi,
On 08/10/2017 06:00 PM, Mathias Nyman wrote:
> 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
>
Okay, I will submit a v3 to include this change.
Best regards,
Lu Baolu
--
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
