Re: [PATCH 11/68] usb-core: Track if an endpoint has streams

2013-11-16 Thread Hans de Goede 

Hi,

On 11/16/2013 08:45 AM, Oliver Neukum wrote:

On Fri, 2013-11-15 at 23:26 +0100, Hans de Goede wrote:

Hi,

On 11/15/2013 11:16 PM, Oliver Neukum wrote:

On Fri, 2013-11-15 at 16:06 +0100, Hans de Goede wrote:

This is a preparation patch for adding support for bulk streams to usbfs.



@@ -2086,19 +2097,30 @@ int usb_free_streams(struct usb_interface *interface,
   {
struct usb_hcd *hcd;
struct usb_device *dev;
-   int i;
+   int i, ret;

dev = interface_to_usbdev(interface);
hcd = bus_to_hcd(dev-bus);
if (dev-speed != USB_SPEED_SUPER)
return -EINVAL;

-   /* Streams only apply to bulk endpoints. */
-   for (i = 0; i  num_eps; i++)
-   if (!eps[i] || !usb_endpoint_xfer_bulk(eps[i]-desc))


Why bother checking for the endpoint's type?


Not sure what you're trying to say here, the check gets re-added a couple of 
lines
later in the patch. Are you trying to say the ep type check can be removed 
entirely?


Yes, the check makes sense on allocation only.


Now that we check that a stream has endpoints, indeed the type check does not 
make
a whole lot of sense, since only bulk eps will have streams. I'll respin the 
patch
with the unnecessary check removed.

Regards,

Hans
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 11/68] usb-core: Track if an endpoint has streams

2013-11-15 Thread Hans de Goede 
This is a preparation patch for adding support for bulk streams to usbfs.

Signed-off-by: Hans de Goede hdego...@redhat.com
Signed-off-by: Sarah Sharp sarah.a.sh...@linux.intel.com
---
 drivers/usb/core/hcd.c | 40 +++-
 include/linux/usb.h|  2 ++
 2 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index e60e08d..b080696 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2049,7 +2049,7 @@ int usb_alloc_streams(struct usb_interface *interface,
 {
struct usb_hcd *hcd;
struct usb_device *dev;
-   int i;
+   int i, ret;
 
dev = interface_to_usbdev(interface);
hcd = bus_to_hcd(dev-bus);
@@ -2058,13 +2058,24 @@ int usb_alloc_streams(struct usb_interface *interface,
if (dev-speed != USB_SPEED_SUPER)
return -EINVAL;
 
-   /* Streams only apply to bulk endpoints. */
-   for (i = 0; i  num_eps; i++)
+   for (i = 0; i  num_eps; i++) {
+   /* Streams only apply to bulk endpoints. */
if (!usb_endpoint_xfer_bulk(eps[i]-desc))
return -EINVAL;
+   /* Re-alloc is not allowed */
+   if (eps[i]-streams)
+   return -EINVAL;
+   }
 
-   return hcd-driver-alloc_streams(hcd, dev, eps, num_eps,
+   ret = hcd-driver-alloc_streams(hcd, dev, eps, num_eps,
num_streams, mem_flags);
+   if (ret  0)
+   return ret;
+
+   for (i = 0; i  num_eps; i++)
+   eps[i]-streams = ret;
+
+   return ret;
 }
 EXPORT_SYMBOL_GPL(usb_alloc_streams);
 
@@ -2086,19 +2097,30 @@ int usb_free_streams(struct usb_interface *interface,
 {
struct usb_hcd *hcd;
struct usb_device *dev;
-   int i;
+   int i, ret;
 
dev = interface_to_usbdev(interface);
hcd = bus_to_hcd(dev-bus);
if (dev-speed != USB_SPEED_SUPER)
return -EINVAL;
 
-   /* Streams only apply to bulk endpoints. */
-   for (i = 0; i  num_eps; i++)
-   if (!eps[i] || !usb_endpoint_xfer_bulk(eps[i]-desc))
+   for (i = 0; i  num_eps; i++) {
+   /* Streams only apply to bulk endpoints. */
+   if (!usb_endpoint_xfer_bulk(eps[i]-desc))
+   return -EINVAL;
+   /* Double-free is not allowed */
+   if (!eps[i]-streams)
return -EINVAL;
+   }
+
+   ret = hcd-driver-free_streams(hcd, dev, eps, num_eps, mem_flags);
+   if (ret  0)
+   return ret;
+
+   for (i = 0; i  num_eps; i++)
+   eps[i]-streams = 0;
 
-   return hcd-driver-free_streams(hcd, dev, eps, num_eps, mem_flags);
+   return ret;
 }
 EXPORT_SYMBOL_GPL(usb_free_streams);
 
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 4c53d54..a4c8406 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -57,6 +57,7 @@ struct ep_device;
  * @extra: descriptors following this endpoint in the configuration
  * @extralen: how many bytes of extra are valid
  * @enabled: URBs may be submitted to this endpoint
+ * @streams: number of USB-3 streams allocated on the endpoint
  *
  * USB requests are always queued to a given endpoint, identified by a
  * descriptor within an active interface in a given USB configuration.
@@ -71,6 +72,7 @@ struct usb_host_endpoint {
unsigned char *extra;   /* Extra descriptors */
int extralen;
int enabled;
+   int streams;
 };
 
 /* host-side wrapper for one interface setting's parsed descriptors */
-- 
1.8.4.2

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 11/68] usb-core: Track if an endpoint has streams

2013-11-15 Thread Oliver Neukum 
On Fri, 2013-11-15 at 16:06 +0100, Hans de Goede wrote:
 This is a preparation patch for adding support for bulk streams to usbfs.

 @@ -2086,19 +2097,30 @@ int usb_free_streams(struct usb_interface *interface,
  {
   struct usb_hcd *hcd;
   struct usb_device *dev;
 - int i;
 + int i, ret;
  
   dev = interface_to_usbdev(interface);
   hcd = bus_to_hcd(dev-bus);
   if (dev-speed != USB_SPEED_SUPER)
   return -EINVAL;
  
 - /* Streams only apply to bulk endpoints. */
 - for (i = 0; i  num_eps; i++)
 - if (!eps[i] || !usb_endpoint_xfer_bulk(eps[i]-desc))

Why bother checking for the endpoint's type?

Regards
Oliver


--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 11/68] usb-core: Track if an endpoint has streams

2013-11-15 Thread Hans de Goede 

Hi,

On 11/15/2013 11:16 PM, Oliver Neukum wrote:

On Fri, 2013-11-15 at 16:06 +0100, Hans de Goede wrote:

This is a preparation patch for adding support for bulk streams to usbfs.



@@ -2086,19 +2097,30 @@ int usb_free_streams(struct usb_interface *interface,
  {
struct usb_hcd *hcd;
struct usb_device *dev;
-   int i;
+   int i, ret;

dev = interface_to_usbdev(interface);
hcd = bus_to_hcd(dev-bus);
if (dev-speed != USB_SPEED_SUPER)
return -EINVAL;

-   /* Streams only apply to bulk endpoints. */
-   for (i = 0; i  num_eps; i++)
-   if (!eps[i] || !usb_endpoint_xfer_bulk(eps[i]-desc))


Why bother checking for the endpoint's type?


Not sure what you're trying to say here, the check gets re-added a couple of 
lines
later in the patch. Are you trying to say the ep type check can be removed 
entirely?

Regards,

Hans
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 11/68] usb-core: Track if an endpoint has streams

2013-11-15 Thread Oliver Neukum 
On Fri, 2013-11-15 at 23:26 +0100, Hans de Goede wrote:
 Hi,
 
 On 11/15/2013 11:16 PM, Oliver Neukum wrote:
  On Fri, 2013-11-15 at 16:06 +0100, Hans de Goede wrote:
  This is a preparation patch for adding support for bulk streams to usbfs.
 
  @@ -2086,19 +2097,30 @@ int usb_free_streams(struct usb_interface 
  *interface,
{
 struct usb_hcd *hcd;
 struct usb_device *dev;
  -  int i;
  +  int i, ret;
 
 dev = interface_to_usbdev(interface);
 hcd = bus_to_hcd(dev-bus);
 if (dev-speed != USB_SPEED_SUPER)
 return -EINVAL;
 
  -  /* Streams only apply to bulk endpoints. */
  -  for (i = 0; i  num_eps; i++)
  -  if (!eps[i] || !usb_endpoint_xfer_bulk(eps[i]-desc))
 
  Why bother checking for the endpoint's type?
 
 Not sure what you're trying to say here, the check gets re-added a couple of 
 lines
 later in the patch. Are you trying to say the ep type check can be removed 
 entirely?

Yes, the check makes sense on allocation only.

Regards
Oliver


--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html