Hi,

On 08/15/2013 06:00 PM, Sarah Sharp wrote:
On Wed, Aug 14, 2013 at 02:32:03PM +0200, Hans de Goede wrote:
-       /* 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]->has_streams)
+                       return -EINVAL;
+       }

[snip]

  }
  EXPORT_SYMBOL_GPL(usb_alloc_streams);

@@ -2075,7 +2086,7 @@ void 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);
@@ -2087,7 +2098,12 @@ void usb_free_streams(struct usb_interface *interface,
                if (!eps[i] || !usb_endpoint_xfer_bulk(&eps[i]->desc))
                        return;

Shouldn't you also check if all the endpoints have streams enabled here?
If you're going to return an error in the allocation function if any
endpoint has streams enabled, I think it makes sense to return an error
in the free function if one of the endpoints doesn't have streams
enabled.

Well there is not error return, as this function is void, so I thought
it would be better to leave this up to the host driver. Note I'm open to
changing this.

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

Reply via email to