> +Driver implications
> +===================
> +
> +int usb_alloc_streams(struct usb_driver *driver,
> + struct usb_device *dev,
This is very problematic. We never pass the usb_driver. Subsystems
like usb serial might not even have it available. Is this needed for anything
but debugging?
[..]
> +Clean up
> +========
> +
> +If a driver wishes to stop using streams to communicate with the device,it
> +should call
> +
> +int usb_free_streams(struct usb_driver *driver,
What is a driver supposed to do if this returns an error? This will
have to be used in disconnect(), which cannot fail.
> + struct usb_device *dev,
> + unsigned int *epaddr,
> + unsigned int num_eps,
> + gfp_t mem_flags);
[..]
> + */
> +int usb_alloc_streams(struct usb_driver *driver, struct usb_device *dev,
> + unsigned int *epaddr, unsigned int num_eps,
> + unsigned int num_streams, gfp_t mem_flags)
> +{
> + struct usb_hcd *hcd;
> + struct usb_host_endpoint **eps;
> + int i;
> +
> + hcd = bus_to_hcd(dev->bus);
> + if (!hcd->driver->alloc_streams)
> + return -EINVAL;
> + if (dev->speed != USB_SPEED_SUPER)
> + return -EINVAL;
> + eps = usb_make_endpoint_array(dev, epaddr, num_eps, mem_flags);
> + if (!eps)
> + return -ENOMEM;
> +
> + /* Streams only apply to bulk endpoints. */
> + for (i = 0; i < num_eps; i++) {
> + if (!usb_endpoint_xfer_bulk(&eps[i]->desc)) {
> + kfree(eps);
> + return -EINVAL;
> + }
> + }
> +
> + i = hcd->driver->alloc_streams(hcd, dev, eps, num_eps,
> + num_streams, mem_flags);
Locking?
> + kfree(eps);
> + return i;
> +}
> +EXPORT_SYMBOL_GPL(usb_alloc_streams);
> +
> +/* Reverts a group of bulk endpoints back to not using stream IDs.
> + * Can fail if we run out of memory, or are given bad arguments.
> + */
> +int usb_free_streams(struct usb_driver *driver, struct usb_device *dev,
> + unsigned int *epaddr, unsigned int num_eps, gfp_t mem_flags)
> +{
> + struct usb_hcd *hcd;
> + struct usb_host_endpoint **eps;
> + int i;
> +
> + hcd = bus_to_hcd(dev->bus);
> + if (!hcd->driver->free_streams)
> + return -EINVAL;
Too late. You must check this on allocation, or you'll have streams that
cannot be freed.
> + eps = usb_make_endpoint_array(dev, epaddr, num_eps, mem_flags);
> + if (!eps)
> + return -ENOMEM;
This is very problematic. You cannot deal with -ENOMEM in disconnect().
It would be better to allocate this in advance, or from a mempool.
Regards
Oliver
--
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
