On Fri, Aug 16, 2013 at 11:17:41AM +0200, Hans de Goede wrote:
> 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.
We probably need to pass the error code up to the driver. The xHCI
driver can return an error code from xhci_free_streams, so it would be
good to pass that up to userspace as well.
Sarah Sharp
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
