On Thu, Aug 01 2013, Felipe Balbi wrote:
> Hi folks,
>
> as we all know naming conventions are fragile and easy to break. We've
> had weird endpoint naming conventions for far too long in the gadget
> framework.
>
> I'm trying to come up with means to get rid of that and, one of the
> ideas, was to add transfer support flags to our struct usb_ep which gets
> initialized by the UDC driver. Then ep_matches() can use those flags to
> check if it should return that endpoint or not.
>
> The ***UNFINISHED*** patch below does just that and shows an example of
> how to initialize such flags on dwc3. Please go over it and let me know
> what you guys think.

> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index f168eae..0bc3621 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -1655,6 +1655,9 @@ static int dwc3_gadget_init_hw_endpoints(struct dwc3 
> *dwc,
>               if (epnum == 0 || epnum == 1) {
>                       dep->endpoint.maxpacket = 512;
>                       dep->endpoint.maxburst = 1;
> +
> +                     dep->endpoint.supports_control = true;
> +

Personally I'd stick to “1”, but no strong feelings.

>                       dep->endpoint.ops = &dwc3_gadget_ep0_ops;
>                       if (!epnum)
>                               dwc->gadget.ep0 = &dep->endpoint;

> @@ -177,6 +184,11 @@ struct usb_ep {
>       u8                      address;
>       const struct usb_endpoint_descriptor    *desc;
>       const struct usb_ss_ep_comp_descriptor  *comp_desc;
> +
> +     unsigned                supports_isochronous:1;
> +     unsigned                supports_interrupt:1;
> +     unsigned                supports_control:1;
> +     unsigned                supports_bulk:1;
>  };

Perhaps:

+       unsigned                supported_transfer_modes:4;

And later:

+       ep->supported_transfer_modes =
+               (1 << USB_ENDPOINT_XFER_CONTROL) |
+               (1 << USB_ENDPOINT_XFER_BULK)

Naturally, we can have macros for those constants.

The advantage might be that the switch would be replaced by a single if:

+       type = desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK;
+       if (!(ep->supported_transfer_modes & (1 << type))) {
+               return 0;

or something.

Again, just throwing out ideas.

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +----<email/xmpp: [email protected]>--------------ooO--(_)--Ooo--

Attachment: signature.asc
Description: PGP signature

Reply via email to