On Friday 03 July 2009 11:43:02 Laurent Pinchart wrote:
> On Thursday 02 July 2009 17:37:08 Laurent Pinchart wrote:
> > Hi David,
> >
> > On Friday 19 June 2009 14:28:14 David Brownell wrote:
> > > On Friday 08 May 2009, Laurent Pinchart wrote:
> > > > Hi everybody,
> > > >
> > > > the USB composite gadget framework currently handles EP0 control
> > > > requests sent to endpoints (bRequestType[4..0] == USB_RECIP_ENDPOINT)
> > > > down to the current configuration EP0 request handler. Composite
> > > > drivers thus need to parse the request and call the appropriate
> > > > function handlers, making functions not self- contained.
> > > >
> > > > It should be possible to maintain an association between functions
> > > > and endpoints for each configuration, much like we already maintain
> > > > an association between functions and interfaces.
> > >
> > > Ideally something less pricey than an array of function pointers,
> > > indexed by endpoint ... :)
> > >
> > > A per-function bitmap would suffice, and it's not like those calls
> > > are on critical paths. Though I suppose the code to walk those
> > > bitmaps is likely to take as much space as the array-of-pointers. ;)
> >
> > As the USB specification limits the number of endpoints to 32 (16 IN and
> > 16 OUT), this would cost 128 bytes on a 32 bit architecture (256 bytes on
> > a 64 bit architecture). The bitmap would take 4 (8) bytes, but would need
> > to be stored in the function instead of the configuration. If we assume
> > the device has two functions, function pointers have a data overhead of
> > 120 (240) bytes. Given the amount of code required to walk the bitmaps,
> > this might indeed be an overkill, but here you are :-) Could you please
> > review the following patch (based on 2.6.31-rc1) ?
>
> [snip]
>
> And obviously I forgot to include a very important part of the patch, which
> is the update of the per-function bitmap when allocating/deallocating an
> endpoint
>
> :-/ I'll resubmit a new patch soon.
soon = (schedule == AS_TIME_PERMITS) ? REALLY_LATE : NOT_SO_LATE;
:-S
Anyway, here's the new patch version (based on 2.6.31-rc5). I plan to submit a
fix to the audio gadget driver after getting this patch committed (and in this
case "soon" should keep it's normal meaning, as the patch is ready).
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 59e8523..897c2ed 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -688,6 +688,7 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
u16 w_value = le16_to_cpu(ctrl->wValue);
u16 w_length = le16_to_cpu(ctrl->wLength);
struct usb_function *f = NULL;
+ u8 endp;
/* partial re-init of the response message; the function or the
* gadget might need to intercept e.g. a control-OUT completion
@@ -800,23 +801,33 @@ unknown:
ctrl->bRequestType, ctrl->bRequest,
w_value, w_index, w_length);
- /* functions always handle their interfaces ... punt other
- * recipients (endpoint, other, WUSB, ...) to the current
+ /* functions always handle their interfaces and endpoints...
+ * punt other recipients (other, WUSB, ...) to the current
* configuration code.
*
* REVISIT it could make sense to let the composite device
* take such requests too, if that's ever needed: to work
* in config 0, etc.
*/
- if ((ctrl->bRequestType & USB_RECIP_MASK)
- == USB_RECIP_INTERFACE) {
+ switch (ctrl->bRequestType & USB_RECIP_MASK) {
+ case USB_RECIP_INTERFACE:
f = cdev->config->interface[intf];
- if (f && f->setup)
- value = f->setup(f, ctrl);
- else
+ break;
+
+ case USB_RECIP_ENDPOINT:
+ endp = ((w_index & 0x80) >> 3) | (w_index & 0x0f);
+ list_for_each_entry(f, &cdev->config->functions, list) {
+ if (test_bit(endp, f->endpoints))
+ break;
+ }
+ if (&f->list == &cdev->config->functions)
f = NULL;
+ break;
}
- if (value < 0 && !f) {
+
+ if (f && f->setup)
+ value = f->setup(f, ctrl);
+ else {
struct usb_configuration *c;
c = cdev->config;
diff --git a/drivers/usb/gadget/epautoconf.c b/drivers/usb/gadget/epautoconf.c
index cd0914e..8640d1a 100644
--- a/drivers/usb/gadget/epautoconf.c
+++ b/drivers/usb/gadget/epautoconf.c
@@ -174,6 +174,7 @@ ep_matches (
return 0;
desc->bEndpointAddress |= epnum;
}
+ ep->addr = desc->bEndpointAddress;
/* report (variable) full speed bulk maxpacket */
if (USB_ENDPOINT_XFER_BULK == type) {
diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index 4f6bb3d..e7e8ab5 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -127,6 +127,7 @@ struct usb_function {
/* private: */
/* internals */
struct list_head list;
+ DECLARE_BITMAP(endpoints, 32);
};
int usb_add_function(struct usb_configuration *, struct usb_function *);
@@ -134,6 +135,41 @@ int usb_add_function(struct usb_configuration *, struct usb_function *);
int usb_function_deactivate(struct usb_function *);
int usb_function_activate(struct usb_function *);
+/**
+ * usb_function_claim_endpoint - bind a function to an endpoint
+ * @f: the function to be bound
+ * @ep: the endpoint to which it will be bound
+ *
+ * This is used by usb function drivers to claim usage of an endpoint. Once
+ * the endpoint is claimed, all control requests targetted at that endpoint
+ * will be dispatched to the function's setup callback instead of the current
+ * configuration's setup callback.
+ */
+static inline void
+usb_function_claim_endpoint(struct usb_function *f, struct usb_ep *ep)
+{
+ int endp = ((ep->addr & 0x80) >> 3) | (ep->addr & 0x0f);
+ set_bit(endp, f->endpoints);
+}
+
+/**
+ * usb_function_release_endpoint - unbind a function from an endpoint
+ * @f: the function to be unbound
+ * @ep: the endpoint from which it will be unbound
+ *
+ * This is used by usb function drivers to release usage of an endpoint. After
+ * the endpoint is released, all control requests targetted at that endpoint
+ * will be dispatched back to current configuration's setup callback.
+ */
+static inline void
+usb_function_release_endpoint(struct usb_function *f, struct usb_ep *ep)
+{
+ if (ep) {
+ int endp = ((ep->addr & 0x80) >> 3) | (ep->addr & 0x0f);
+ clear_bit(endp, f->endpoints);
+ }
+}
+
int usb_interface_id(struct usb_configuration *, struct usb_function *);
/**
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index bbf45d5..3b74a6f 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -124,6 +124,7 @@ struct usb_ep_ops {
/**
* struct usb_ep - device side representation of USB endpoint
* @name:identifier for the endpoint, such as "ep-a" or "ep9in-bulk"
+ * @addr:endpoint's address (direction and number)
* @ops: Function pointers used to access hardware-specific operations.
* @ep_list:the gadget's ep_list holds all of its endpoints
* @maxpacket:The maximum packet size used on this endpoint. The initial
@@ -140,6 +141,7 @@ struct usb_ep {
void *driver_data;
const char *name;
+ u8 addr;
const struct usb_ep_ops *ops;
struct list_head ep_list;
unsigned maxpacket:16;
Best regards,
Laurent Pinchart
--
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
