| Message ID | 1438351258-31578-3-git-send-email-r.baldyga@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
From: Robert Baldyga > Sent: 31 July 2015 15:00 > Introduce struct usb_ep_caps which contains information about capabilities > of usb endpoints - supported transfer types and directions. This structure > should be filled by UDC driver for each of its endpoints, and will be > used in epautoconf in new ep matching mechanism which will replace ugly > guessing of endpoint capabilities basing on its name. > > Signed-off-by: Robert Baldyga <r.baldyga@samsung.com> > --- > include/linux/usb/gadget.h | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h > index 68fb5e8..a9a4959 100644 > --- a/include/linux/usb/gadget.h > +++ b/include/linux/usb/gadget.h > @@ -141,10 +141,29 @@ struct usb_ep_ops { > }; ... > +struct usb_ep_caps { > + unsigned type_control:1; > + unsigned type_iso:1; > + unsigned type_bulk:1; > + unsigned type_int:1; > + unsigned dir_in:1; > + unsigned dir_out:1; > +}; With the way this is used (eg below from 13/46) + + if (i == 0) { + ep->ep.caps.type_control = true; + } else { + ep->ep.caps.type_iso = true; + ep->ep.caps.type_bulk = true; + ep->ep.caps.type_int = true; + } + + ep->ep.caps.dir_in = true; + ep->ep.caps.dir_out = true; I think it would be more obvious if you used a u8 and explicit bitmasks. The initialisation (as above) would the be explicitly assigning 'not supported' to the other fields. The compiler will also generate much better code... David
Hi, On Fri, Jul 31, 2015 at 03:51:52PM +0000, David Laight wrote: > From: Robert Baldyga > > Sent: 31 July 2015 15:00 > > Introduce struct usb_ep_caps which contains information about capabilities > > of usb endpoints - supported transfer types and directions. This structure > > should be filled by UDC driver for each of its endpoints, and will be > > used in epautoconf in new ep matching mechanism which will replace ugly > > guessing of endpoint capabilities basing on its name. > > > > Signed-off-by: Robert Baldyga <r.baldyga@samsung.com> > > --- > > include/linux/usb/gadget.h | 21 +++++++++++++++++++++ > > 1 file changed, 21 insertions(+) > > > > diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h > > index 68fb5e8..a9a4959 100644 > > --- a/include/linux/usb/gadget.h > > +++ b/include/linux/usb/gadget.h > > @@ -141,10 +141,29 @@ struct usb_ep_ops { > > }; > ... > > +struct usb_ep_caps { > > + unsigned type_control:1; > > + unsigned type_iso:1; > > + unsigned type_bulk:1; > > + unsigned type_int:1; > > + unsigned dir_in:1; > > + unsigned dir_out:1; > > +}; > > With the way this is used (eg below from 13/46) > > + > + if (i == 0) { > + ep->ep.caps.type_control = true; > + } else { > + ep->ep.caps.type_iso = true; > + ep->ep.caps.type_bulk = true; > + ep->ep.caps.type_int = true; > + } > + > + ep->ep.caps.dir_in = true; > + ep->ep.caps.dir_out = true; > > I think it would be more obvious if you used a u8 and explicit bitmasks. > The initialisation (as above) would the be explicitly assigning 'not supported' > to the other fields. > The compiler will also generate much better code... compiler should convert single bit flags into u32 just fine. It's all static data anyway. Besides, single bit flags allow us to have as many as we need without ending up with stuff like: u32 flags; u32 flags1; u32 flags2; etc. Just let the compiler do those conversions for us.
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h index 68fb5e8..a9a4959 100644 --- a/include/linux/usb/gadget.h +++ b/include/linux/usb/gadget.h @@ -141,10 +141,29 @@ struct usb_ep_ops { }; /** + * struct usb_ep_caps - endpoint capabilities description + * @type_control:Endpoint supports control type (reserved for ep0). + * @type_iso:Endpoint supports isochronous transfers. + * @type_bulk:Endpoint supports bulk transfers. + * @type_int:Endpoint supports interrupt transfers. + * @dir_in:Endpoint supports IN direction. + * @dir_out:Endpoint supports OUT direction. + */ +struct usb_ep_caps { + unsigned type_control:1; + unsigned type_iso:1; + unsigned type_bulk:1; + unsigned type_int:1; + unsigned dir_in:1; + unsigned dir_out:1; +}; + +/** * struct usb_ep - device side representation of USB endpoint * @name:identifier for the endpoint, such as "ep-a" or "ep9in-bulk" * @ops: Function pointers used to access hardware-specific operations. * @ep_list:the gadget's ep_list holds all of its endpoints + * @caps:The structure describing types and directions supported by endoint. * @maxpacket:The maximum packet size used on this endpoint. The initial * value can sometimes be reduced (hardware allowing), according to * the endpoint descriptor used to configure the endpoint. @@ -167,12 +186,14 @@ struct usb_ep_ops { * gadget->ep_list. the control endpoint (gadget->ep0) is not in that list, * and is accessed only in response to a driver setup() callback. */ + struct usb_ep { void *driver_data; const char *name; const struct usb_ep_ops *ops; struct list_head ep_list; + struct usb_ep_caps caps; bool claimed; unsigned maxpacket:16; unsigned maxpacket_limit:16;
Introduce struct usb_ep_caps which contains information about capabilities of usb endpoints - supported transfer types and directions. This structure should be filled by UDC driver for each of its endpoints, and will be used in epautoconf in new ep matching mechanism which will replace ugly guessing of endpoint capabilities basing on its name. Signed-off-by: Robert Baldyga <r.baldyga@samsung.com> --- include/linux/usb/gadget.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)