xhci: xhci_handle_cmd_stop_ep might give back urb incorrectly
Hi Mathias, Alan and all,
During xhci handling a completion event for stop endpoint command ,
xhci_handle_cmd_stop_ep() always gives back a urb with status set to 0.
The comment says,
/* Doesn't matter what we pass for status, since the
core will
* just overwrite it (because the URB has been unlinked).
*/
I don't think this assumption is always correct. Stop endpoint command
is used in below two cases in xhci driver.
1. In xhci_urb_dequeue(). This is the hcd driver interface for canceling
a transfer.
2. During suspend process. This is to stop on-going transfers and cause
xHC to save various endpoint state into memory.
The assumption of URB has been unlinked is only the fact for case 1.
It's wrong in case 2. If my understanding is right, USB client drivers
(serial, mass storage ) might receive an incompletion urb transfer
with status set to success during suspend/resume.
Please correct me if there is any misunderstanding.
[below is the code segment in xhci_handle_cmd_stop_ep()]
/*
* Drop the lock and complete the URBs in the cancelled TD list.
* New TDs to be cancelled might be added to the end of the
list before
* we can complete all the URBs for the TDs we already unlinked.
* So stop when we've completed the URB for the last TD we
unlinked.
*/
do {
cur_td = list_entry(ep-cancelled_td_list.next,
struct xhci_td, cancelled_td_list);
list_del_init(cur_td-cancelled_td_list);
/* Clean up the cancelled URB */
/* Doesn't matter what we pass for status, since the
core will
* just overwrite it (because the URB has been unlinked).
*/
xhci_giveback_urb_in_irq(xhci, cur_td, 0);
/* Stop processing the cancelled list if the watchdog
timer is
* running.
*/
if (xhci-xhc_state XHCI_STATE_DYING)
return;
} while (cur_td != last_unlinked_td);
[end of code segment]
Beset Regards,
Lu Baolu
--
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
Re: xhci: xhci_handle_cmd_stop_ep might give back urb incorrectly
On Mon, 24 Nov 2014, Lu, Baolu wrote: Hi Mathias, Alan and all, During xhci handling a completion event for stop endpoint command , xhci_handle_cmd_stop_ep() always gives back a urb with status set to 0. The comment says, /* Doesn't matter what we pass for status, since the core will * just overwrite it (because the URB has been unlinked). */ I don't think this assumption is always correct. Stop endpoint command is used in below two cases in xhci driver. 1. In xhci_urb_dequeue(). This is the hcd driver interface for canceling a transfer. 2. During suspend process. This is to stop on-going transfers and cause xHC to save various endpoint state into memory. The assumption of URB has been unlinked is only the fact for case 1. It's wrong in case 2. If my understanding is right, USB client drivers (serial, mass storage ) might receive an incompletion urb transfer with status set to success during suspend/resume. Please correct me if there is any misunderstanding. When a suspend is about to occur, usbcore makes sure there are no outstanding URBs for the device. It tells the device's driver to go into suspend, unlinks any remaining URBs, and prevents new ones from being submitted. Alan Stern -- 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
Re: xhci: xhci_handle_cmd_stop_ep might give back urb incorrectly
On 2014年11月24日 23:33, Alan Stern wrote: On Mon, 24 Nov 2014, Lu, Baolu wrote: Hi Mathias, Alan and all, During xhci handling a completion event for stop endpoint command , xhci_handle_cmd_stop_ep() always gives back a urb with status set to 0. The comment says, /* Doesn't matter what we pass for status, since the core will * just overwrite it (because the URB has been unlinked). */ I don't think this assumption is always correct. Stop endpoint command is used in below two cases in xhci driver. 1. In xhci_urb_dequeue(). This is the hcd driver interface for canceling a transfer. 2. During suspend process. This is to stop on-going transfers and cause xHC to save various endpoint state into memory. The assumption of URB has been unlinked is only the fact for case 1. It's wrong in case 2. If my understanding is right, USB client drivers (serial, mass storage ) might receive an incompletion urb transfer with status set to success during suspend/resume. Please correct me if there is any misunderstanding. When a suspend is about to occur, usbcore makes sure there are no outstanding URBs for the device. It tells the device's driver to go into suspend, unlinks any remaining URBs, and prevents new ones from being submitted. That's clear now. Thank you, Alan. Alan Stern -- 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
