[v5,02/46] usb: gadget: add endpoint capabilities flags
diff mbox

Message ID 1438351258-31578-3-git-send-email-r.baldyga@samsung.com
State New
Headers show

Commit Message

Robert Baldyga July 31, 2015, 2 p.m. UTC
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(+)

Comments

David Laight July 31, 2015, 3:51 p.m. UTC | #1
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
Felipe Balbi July 31, 2015, 3:58 p.m. UTC | #2
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.

Patch
diff mbox

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;