On Wed, Dec 28, 2022 at 12:31:16PM -0800, Wesley Cheng wrote:
Hi Alan,
On 12/28/2022 7:16 AM, Alan Stern wrote:
On Wed, Dec 28, 2022 at 09:59:03AM +0100, Oliver Neukum wrote:
On 27.12.22 22:07, Wesley Cheng wrote:
Hmmm...maybe I should change the name of the API then to avoid the confusion. Yes, usb_hcd_flush_endpoint() does ensure that URBs submitted to the EP are stopped. However, with this offloading concept, we aren't actually submitting URBs from the main processor, so the ep->urb_list will be empty.
This means the usb_hcd_flush_endpoint() API won't actually do anything. What we need is to ensure that we send a XHCI stop ep command to the controller.
That is a concept specific to XHCI, yet you are adding a generic API. The namin should reflect that. usb_quiesce_endpoint() ?
Or even xhci_send_stop_ep_cmd(), which is what the routine is intended to do.
Just to clarify, you're talking about renaming the API that was added in the XHCI driver, correct?
To be precise, we're talking about renaming your usb_hcd_stop_endpoint() function, although similar arguments probably apply to your usb_free_interrupter(), usb_set_interrupter(), and usb_hcd_get_transfer_resource() routines.
You wrote earlier:
The XHCI driver is the one that maintains the list of interrupters that are available, so the locking was placed in the XHCI driver versus adding it in the core hcd layer.
The "stop ep" functionality and other interrupter management things you want to add seem a lot like this locking stuff. Since you decided to put the locking in the xhci-hcd driver instead of the core HCD layer, it would be logical to do the same with the "stop ep" and other routines. Which means there shouldn't be any need to make changes to hcd.c or include/linux/usb/hcd.h.
Alan Stern