Re: [RFC/PATCH v2] usb: Add streams support to epautoconf.

Re: [RFC/PATCH v2] usb: Add streams support to epautoconf.

[merez]

> On Thu, Dec 16, 2010 at 10:15AM, Greg KH wrote:
> What do you exactly mean by "proprietary search algorithm"?
Our implementation for finding an EP with the required number of streams
may not fit the needs and platform definitions of all controllers.  For
example, having the minimum number of streams as a parameter will allow
controllers to compromise on the number of required streams in case there
is no EP that answers their first request.
At the beginning of usb_ep_autoconfig we could see many if statements for
finding the fitting EP for several controllers (for example
gadget_is_net2280). The new gadget op will prevent having additional if
statements in the usb_ep_autoconfig in case a different search is needed. 
Thanks,
Maya Erez
Consultant for Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

Re: [RFC/PATCH v2] usb: Add streams support to epautoconf.

[Greg KH]

On Mon, Dec 20, 2010 at 11:33:44AM -0800, merez@codeaurora.org wrote:
> > On Thu, Dec 16, 2010 at 10:15AM, Greg KH wrote:
> > What do you exactly mean by "proprietary search algorithm"?
> Our implementation for finding an EP with the required number of streams
> may not fit the needs and platform definitions of all controllers.  For
> example, having the minimum number of streams as a parameter will allow
> controllers to compromise on the number of required streams in case there
> is no EP that answers their first request.
> At the beginning of usb_ep_autoconfig we could see many if statements for
> finding the fitting EP for several controllers (for example
> gadget_is_net2280). The new gadget op will prevent having additional if
> statements in the usb_ep_autoconfig in case a different search is needed. 

Ok, but please note the word of "proprietary" usually means something
else when talking about kernel code (i.e. licensing issues.)

I'll be glad to take this type of patch when you also provide a patch
that uses the callback, but not before then.

thanks,

greg k-h

Re: [RFC/PATCH v2] usb: Add streams support to epautoconf.

[merez]

>> On Wed, Dec 22, 2010 at 03:58AM, Maya Erez wrote:
> We beleive that the chip-specific code for net2280 and goku at the
> beginning of usb_ep_autoconfig can be moved to this new gadget op,
> therefore it is also needed for today's implementation. I will do the
> cahnge and send a new patch for review
Since the unique implementation of net2280 and goku don't cover all EPs
types and since we don't have the ability to fully test it, we didn't feel
confident to do the changes in their code. Therefore, we decided to remove
the new gadget_op.
Thanks,
Maya Erez
Consultant for Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

Re: [RFC/PATCH v2] usb: Add streams support to epautoconf.

[merez]

> On Tue, Dec 21, 2010 at 09:58AM, Greg KH wrote:
> First rule of kernel development, "don't add infrastructure that you do
> not need today."
We beleive that the chip-specific code for net2280 and goku at the
beginning of usb_ep_autoconfig can be moved to this new gadget op,
therefore it is also needed for today's implementation. I will do the
cahnge and send a new patch for review
Thanks,
Maya Erez
Consultant for Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

Re: [RFC/PATCH v2] usb: Add streams support to epautoconf.

[Greg KH]

On Tue, Dec 21, 2010 at 05:46:34AM -0800, merez@codeaurora.org wrote:
> > On Mon, Dec 20, 2010 at 11:58AM, Greg KH wrote:
> > Ok, but please note the word of "proprietary" usually means something 
> else when talking about kernel code (i.e. licensing issues.)
> >
> > I'll be glad to take this type of patch when you also provide a patch
> that uses the callback, but not before then.
> 
> What I meant by "proprietary" is not proprietary in the terms of kernel
> world. I meant a unique algorithm, different from the default. Sorry for
> the misunderstanding.
> Currently we don't have a unique algorithm. When implementing this change
> we thought it may be useful for other controllers but for us the default
> implementation was good enough.

First rule of kernel development, "don't add infrastructure that you do
not need today."

thanks,

greg k-h

Re: [RFC/PATCH v2] usb: Add streams support to epautoconf.

[merez]

> On Mon, Dec 20, 2010 at 11:58AM, Greg KH wrote:
> Ok, but please note the word of "proprietary" usually means something 
else when talking about kernel code (i.e. licensing issues.)
>
> I'll be glad to take this type of patch when you also provide a patch
that uses the callback, but not before then.

What I meant by "proprietary" is not proprietary in the terms of kernel
world. I meant a unique algorithm, different from the default. Sorry for
the misunderstanding.
Currently we don't have a unique algorithm. When implementing this change
we thought it may be useful for other controllers but for us the default
implementation was good enough.
Maya Erez
Consultant for Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

Re: [RFC/PATCH v2] usb: Add streams support to epautoconf.

[Greg KH]

On Thu, Dec 16, 2010 at 09:34:19AM +0200, Maya Erez wrote:
> - Add the capability to search for an EP with a required number of streams.
> - Add a gadget_op to allow DCDs to use a proprietary search algorithm
> for required and minimum number of streams.

What do you exactly mean by "proprietary search algorithm"?

thanks,

greg k-h

[RFC/PATCH v2] usb: Add streams support to epautoconf.

[Maya Erez]

- Add the capability to search for an EP with a required number of streams.
- Add a gadget_op to allow DCDs to use a proprietary search algorithm
for required and minimum number of streams.

Signed-off-by: Maya Erez <merez@codeaurora.org>
---
 drivers/usb/gadget/epautoconf.c |  143 ++++++++++++++++++++++++++++++++-------
 include/linux/usb/gadget.h      |   14 ++++-
 2 files changed, 131 insertions(+), 26 deletions(-)

diff --git a/drivers/usb/gadget/epautoconf.c b/drivers/usb/gadget/epautoconf.c
index 8a83248..1c77a10 100644
--- a/drivers/usb/gadget/epautoconf.c
+++ b/drivers/usb/gadget/epautoconf.c
@@ -63,13 +63,16 @@ static int
 ep_matches (
 	struct usb_gadget		*gadget,
 	struct usb_ep			*ep,
-	struct usb_endpoint_descriptor	*desc
+	struct usb_endpoint_descriptor	*desc,
+	struct usb_ss_ep_comp_descriptor *ep_comp
 )
 {
 	u8		type;
 	const char	*tmp;
 	u16		max;

+	int		num_req_streams = 0;
+
 	/* endpoint already claimed? */
 	if (NULL != ep->driver_data)
 		return 0;
@@ -128,6 +131,23 @@ ep_matches (
 		}
 	}

+
+	/*
+	 * Get the number of required streams from the EP companion
+	 * descriptor and see if the EP matches it
+	 */
+	if (usb_endpoint_xfer_bulk(desc)) {
+		if (ep_comp) {
+			num_req_streams = ep_comp->bmAttributes & 0x1f;
+			if (num_req_streams > ep->num_supported_strms)
+				return 0;
+			/* Update the ep_comp descriptor if needed */
+			if (num_req_streams != ep->num_supported_strms)
+				ep_comp->bmAttributes = ep->num_supported_strms;
+		}
+
+	}
+
 	/* endpoint maxpacket size is an input parameter, except for bulk
 	 * where it's an output parameter representing the full speed limit.
 	 * the usb spec fixes high speed bulk maxpacket at 512 bytes.
@@ -200,43 +220,75 @@ find_ep (struct usb_gadget *gadget, const char *name)
 }

 /**
- * usb_ep_autoconfig - choose an endpoint matching the descriptor
+ * usb_ep_autoconfig_ss() - choose an endpoint matching the ep
+ * descriptor and ep companion descriptor
  * @gadget: The device to which the endpoint must belong.
  * @desc: Endpoint descriptor, with endpoint direction and transfer mode
- *	initialized.  For periodic transfers, the maximum packet
- *	size must also be initialized.  This is modified on success.
+ *    initialized.  For periodic transfers, the maximum packet
+ *    size must also be initialized.  This is modified on
+ *    success.
+ * @ep_comp: Endpoint companion descriptor, with the required
+ *    number of streams. Will be modified when the chosen EP
+ *    supports a different number of streams.
+ * @min_num_of_req_strms: A minimum number of streams that allow
+ *    the FD to be functional
  *
- * By choosing an endpoint to use with the specified descriptor, this
- * routine simplifies writing gadget drivers that work with multiple
- * USB device controllers.  The endpoint would be passed later to
- * usb_ep_enable(), along with some descriptor.
+ * This routine replaces the usb_ep_autoconfig when needed
+ * superspeed enhancments. If such enhancemnets are required,
+ * the FD should call usb_ep_autoconfig_ss directly and provide
+ * the additional ep_comp and min_num_of_req_strms parameters.
+ *
+ * By choosing an endpoint to use with the specified descriptor,
+ * this routine simplifies writing gadget drivers that work with
+ * multiple USB device controllers.  The endpoint would be
+ * passed later to usb_ep_enable(), along with some descriptor.
  *
  * That second descriptor won't always be the same as the first one.
  * For example, isochronous endpoints can be autoconfigured for high
  * bandwidth, and then used in several lower bandwidth altsettings.
  * Also, high and full speed descriptors will be different.
  *
- * Be sure to examine and test the results of autoconfiguration on your
- * hardware.  This code may not make the best choices about how to use the
- * USB controller, and it can't know all the restrictions that may apply.
- * Some combinations of driver and hardware won't be able to autoconfigure.
+ * To allow controller special algorithms, we added a new gadget
+ * op for assigning an ep.
+ *
+ * Be sure to examine and test the results of autoconfiguration
+ * on your hardware.  This code may not make the best choices
+ * about how to use the USB controller, and it can't know all
+ * the restrictions that may apply. Some combinations of driver
+ * and hardware won't be able to autoconfigure.
  *
  * On success, this returns an un-claimed usb_ep, and modifies the endpoint
  * descriptor bEndpointAddress.  For bulk endpoints, the wMaxPacket value
- * is initialized as if the endpoint were used at full speed.  To prevent
- * the endpoint from being returned by a later autoconfig call, claim it
- * by assigning ep->driver_data to some non-null value.
+ * is initialized as if the endpoint were used at full speed and
+ * the bmAttribute field in the ep companion descriptor is
+ * updated with the assigned number of streams if it is
+ * different from the original value. To prevent the endpoint
+ * from being returned by a later autoconfig call, claim it by
+ * assigning ep->driver_data to some non-null value.
  *
  * On failure, this returns a null endpoint descriptor.
  */
-struct usb_ep *usb_ep_autoconfig (
+struct usb_ep *usb_ep_autoconfig_ss(
 	struct usb_gadget		*gadget,
-	struct usb_endpoint_descriptor	*desc
+	struct usb_endpoint_descriptor	*desc,
+	struct usb_ss_ep_comp_descriptor *ep_comp,
+	int min_num_of_req_strms
 )
 {
 	struct usb_ep	*ep;
 	u8		type;

+	/*
+	 * Call the controller specific function for assigning an EP
+	 * if it is defined
+	 */
+	if (gadget->ops->ep_assign) {
+		struct usb_ep *assigned_ep = NULL;
+		gadget->ops->ep_assign(gadget, desc, ep_comp,
+				    min_num_of_req_strms, &assigned_ep);
+		return assigned_ep;
+	}
+
 	type = desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK;

 	/* First, apply chip-specific "best usage" knowledge.
@@ -245,23 +297,24 @@ struct usb_ep *usb_ep_autoconfig (
 	if (gadget_is_net2280 (gadget) && type == USB_ENDPOINT_XFER_INT) {
 		/* ep-e, ep-f are PIO with only 64 byte fifos */
 		ep = find_ep (gadget, "ep-e");
-		if (ep && ep_matches (gadget, ep, desc))
+		if (ep && ep_matches(gadget, ep, desc, ep_comp))
 			return ep;
 		ep = find_ep (gadget, "ep-f");
-		if (ep && ep_matches (gadget, ep, desc))
+		if (ep && ep_matches(gadget, ep, desc, ep_comp))
 			return ep;

 	} else if (gadget_is_goku (gadget)) {
 		if (USB_ENDPOINT_XFER_INT == type) {
 			/* single buffering is enough */
-			ep = find_ep (gadget, "ep3-bulk");
-			if (ep && ep_matches (gadget, ep, desc))
+			ep = find_ep(gadget, "ep3-bulk");
+			if (ep && ep_matches(gadget, ep, desc, ep_comp))
 				return ep;
 		} else if (USB_ENDPOINT_XFER_BULK == type
 				&& (USB_DIR_IN & desc->bEndpointAddress)) {
 			/* DMA may be available */
-			ep = find_ep (gadget, "ep2-bulk");
-			if (ep && ep_matches (gadget, ep, desc))
+			ep = find_ep(gadget, "ep2-bulk");
+			if (ep && ep_matches(gadget, ep, desc,
+					      ep_comp))
 				return ep;
 		}

@@ -280,14 +333,14 @@ struct usb_ep *usb_ep_autoconfig (
 				ep = find_ep(gadget, "ep2out");
 		} else
 			ep = NULL;
-		if (ep && ep_matches (gadget, ep, desc))
+		if (ep && ep_matches(gadget, ep, desc, ep_comp))
 			return ep;
 #endif
 	}

 	/* Second, look at endpoints until an unclaimed one looks usable */
 	list_for_each_entry (ep, &gadget->ep_list, ep_list) {
-		if (ep_matches (gadget, ep, desc))
+		if (ep_matches(gadget, ep, desc, ep_comp))
 			return ep;
 	}

@@ -296,6 +349,46 @@ struct usb_ep *usb_ep_autoconfig (
 }

 /**
+ * usb_ep_autoconfig() - choose an endpoint matching the
+ * descriptor
+ * @gadget: The device to which the endpoint must belong.
+ * @desc: Endpoint descriptor, with endpoint direction and transfer mode
+ *	initialized.  For periodic transfers, the maximum packet
+ *	size must also be initialized.  This is modified on success.
+ *
+ * By choosing an endpoint to use with the specified descriptor, this
+ * routine simplifies writing gadget drivers that work with multiple
+ * USB device controllers.  The endpoint would be passed later to
+ * usb_ep_enable(), along with some descriptor.
+ *
+ * That second descriptor won't always be the same as the first one.
+ * For example, isochronous endpoints can be autoconfigured for high
+ * bandwidth, and then used in several lower bandwidth altsettings.
+ * Also, high and full speed descriptors will be different.
+ *
+ * Be sure to examine and test the results of autoconfiguration on your
+ * hardware.  This code may not make the best choices about how to use the
+ * USB controller, and it can't know all the restrictions that may apply.
+ * Some combinations of driver and hardware won't be able to autoconfigure.
+ *
+ * On success, this returns an un-claimed usb_ep, and modifies the endpoint
+ * descriptor bEndpointAddress.  For bulk endpoints, the wMaxPacket value
+ * is initialized as if the endpoint were used at full speed.  To prevent
+ * the endpoint from being returned by a later autoconfig call, claim it
+ * by assigning ep->driver_data to some non-null value.
+ *
+ * On failure, this returns a null endpoint descriptor.
+ */
+struct usb_ep *usb_ep_autoconfig(
+	struct usb_gadget		*gadget,
+	struct usb_endpoint_descriptor	*desc
+)
+{
+	return usb_ep_autoconfig_ss(gadget, desc, NULL, 0);
+}
+
+
+/**
  * usb_ep_autoconfig_reset - reset endpoint autoconfig state
  * @gadget: device for which autoconfig state will be reset
  *
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 006412c..1012afc 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -419,7 +419,7 @@ static inline void usb_ep_fifo_flush(struct usb_ep *ep)
 struct usb_gadget;

 /* the rest of the api to the controller hardware: device operations,
- * which don't involve endpoints (or i/o).
+ * which don't involve i/o.
  */
 struct usb_gadget_ops {
 	int	(*get_frame)(struct usb_gadget *);
@@ -430,6 +430,12 @@ struct usb_gadget_ops {
 	int	(*pullup) (struct usb_gadget *, int is_on);
 	int	(*ioctl)(struct usb_gadget *,
 				unsigned code, unsigned long param);
+	int	(*ep_assign) (struct usb_gadget *,
+		struct usb_endpoint_descriptor	*,
+		struct usb_ss_ep_comp_descriptor *,
+		int min_num_of_req_strms,
+		struct usb_ep **returned_ep);
+
 };

 /**
@@ -892,6 +898,12 @@ static inline void usb_free_descriptors(struct usb_descriptor_header **v)
 extern struct usb_ep *usb_ep_autoconfig(struct usb_gadget *,
 			struct usb_endpoint_descriptor *) __devinit;

+
+extern struct usb_ep *usb_ep_autoconfig_ss(struct usb_gadget *,
+			struct usb_endpoint_descriptor *,
+			struct usb_ss_ep_comp_descriptor *,
+			int min_num_of_req_strms) __devinit;
+
 extern void usb_ep_autoconfig_reset(struct usb_gadget *) __devinit;

 #endif /* __LINUX_USB_GADGET_H */
--
1.6.3.3
--
Seny by a Consultant for Qualcomm innovation center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum