Skip to contentSkip to conversation

usb/gadget: Add an EP dispose() callback for EP lifetime tracking

PATCH New Felipe Balbi 1 participant 1 message Mar 19, 2018 USB

lore
Share & tools
lore
#0 lore raw

PATCH v5 1/2 usb/gadget: Add an EP dispose() callback for EP lifetime tracking

Hi,

Quoted 52 lines · Benjamin Herrenschmidt
Quoted 34 lines · Felipe Balbi

Hi,

Quoted 31 lines · Benjamin Herrenschmidt

Some UDC may want to allocate endpoints dynamically, either because the HW supports an arbitrary large number or because (like the Aspeed BMC SoCs), the pool of HW endpoints is shared between multiple gadgets.

The allocation side can be done rather easily using the existing match_ep() UDC hook.

However we have no good place to "free" them.

This implements a "simple" variant of this, which calls an EP dispose callback on all EPs associated with a gadget when the composite device gets unbound.

This is required by my upcoming Aspeed vHub driver.

---
 drivers/usb/gadget/composite.c | 8 ++++++++
 include/linux/usb/gadget.h     | 1 +
 2 files changed, 9 insertions(+)
drivers/usb/gadget/composite.c +1 -0
index 77c7ecca816a..ec99c9cfc1a2 100644
@@ -2172,6 +2172,7 @@ int composite_os_desc_req_prepare(struct usb_composite_dev *cdev,
 void composite_dev_cleanup(struct usb_composite_dev *cdev)
 {
 	struct usb_gadget_string_container *uc, *tmp;
+	struct usb_ep			   *ep, *tmp_ep;
 
Quoted 13 lines

drivers/usb/gadget/composite.c void composite_dev_cleanup(struct usb_composite_dev *cdev) · line 2200

Quoted 10 lines
drivers/usb/gadget/composite.c +4 -0
 	list_for_each_entry_safe(uc, tmp, &cdev->gstrings, list) {
 		list_del(&uc->list);
@@ -2193,6 +2194,13 @@ void composite_dev_cleanup(struct usb_composite_dev *cdev)
 	}
 	cdev->next_string_id = 0;
 	device_remove_file(&cdev->gadget->dev, &dev_attr_suspended);
+
+	/* Dispose EPs if the UDC driver tracks lifetime */
+	list_for_each_entry_safe(ep, tmp_ep,
+				 &cdev->gadget->ep_list, ep_list) {

do you really need this to be safe? You don't seem to be modifying ep_list here.

Yes, ep->dispose() may do just that. In my Aspeed implementation in fact that's pretty much the first thing it does.

Quoted 7 lines

IE, I'm returning all the endpoints to the common pool, thus taking them out of the gadget EP list.

That said, there could be other reasons why a driver might want to know about disposal without actually taking all the EPs back (for example a driver could have some dedicated EPs and some in a pool) so I prefer the list_del remaining in the back-end.

That seems rather obfuscated. There's no indication that ep_list is modified from within that iteration block. Seems like it would be best to make the list_del() explicit and ->dispose() just adds the ep to its own internal list. No?