On Tue, 18 Aug 2009, Sarah Sharp wrote:
> +Device drivers will call this API to request that the host controller driver
> +allocate memory so the driver can use up to num_requested_streams stream IDs.
> +They must pass an array of endpoint addresses that need to be setup with similar
> +stream IDs. This is to insure that a UASP driver will be able to use the same
> +stream ID for the bulk IN and OUT endpoints used in a Bi-directional command
> +sequence.
I don't understand this. Let's suppose the driver made two different
calls to usb_alloc_streams(), one for the bulk IN and one for the bulk
OUT endpoint, and let's say that each call allocated 8 streams. Why
wouldn't the driver then be able to use the same stream ID (between 1
and 8) for both endpoints in a bi-directional command?
> +static struct usb_host_endpoint **usb_make_endpoint_array(
> + struct usb_device *dev, unsigned int *epaddr,
> + unsigned int num_eps, gfp_t mem_flags)
> +{
> + struct usb_host_endpoint **eps;
> + unsigned int epnum;
> + int i;
> +
> + eps = kmalloc(sizeof(struct usb_host_endpoint *)*num_eps, mem_flags);
> + if (!eps)
> + return NULL;
> +
> + for (i = 0; i < num_eps; i++) {
> + epnum = epaddr[i] & USB_ENDPOINT_NUMBER_MASK;
> + if (usb_endpoint_out(epaddr[i]))
> + eps[i] = dev->ep_out[epnum];
> + else
> + eps[i] = dev->ep_out[epnum];
> + if (!eps[i]) {
> + kfree(eps);
> + return NULL;
> + }
> + }
> + return eps;
> +}
This seems like unnecessary overhead. Why not ask drivers to pass an
array of pointers in the first place? That's just as natural as an
array of endpoint addresses.
> @@ -1199,6 +1208,7 @@ struct urb {
> struct usb_device *dev; /* (in) pointer to associated device */
> struct usb_host_endpoint *ep; /* (internal) pointer to endpoint */
> unsigned int pipe; /* (in) pipe information */
> + unsigned int stream_id; /* (in) stream ID */
> int status; /* (return) non-ISO status */
> unsigned int transfer_flags; /* (in) URB_SHORT_NOT_OK | ...*/
> void *transfer_buffer; /* (in) associated data buffer */
Keeping track of the number of streams in the usb_host_endpoint
structure seems like a reasonable thing to do. Then automatically
re-enabling the streams after a device reset would be fairly
straightforward.
You could then add some error checking to usb_submit_urb(). Verify
that urb->stream_id <= the number of streams allocated for urb->ep.
Alan Stern
--
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
