Hi,
On 11/15/2013 12:08 AM, Sarah Sharp wrote:
All right, this patchset looks sane other than the one small bug I
mentioned, and the couple comments I have below.
Have you talked to the other libusb or libusbX developers about adding
streams support to libusb? ISTR that you said you needed libusb changes
in order to get the virtualized UAS device to work in qemu.
Yes, this has all been coordinated with the other libusb devs, I've
a libusb tree with patches adding support (and docs) here:
https://github.com/jwrdegoede/libusbx/commits/master
The patches have been reviewed by the Mac OS X port maintainer, and
he also has patches ready to add bulk-stream support for Mac OS X.
On Wed, Oct 09, 2013 at 05:19:31PM +0200, Hans de Goede wrote:
Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
You need a commit message here, so we can tell usbfs users what bulk
streams are (reference the documentation), and how to use this new
interface (e.g. they need to allocate streams before using the new
uurb->stream_id field).
Agreed, will fix in my next version. Note I've been working on uas
for the last 3 weeks running a large battery of tests, and adding
small fixes left and right. I was about to send out a pull-req for
you with everything from your "fun-streams-fixes" branch + later
uas patches I've send + new uas patches from the last weeks, rebased
on top of current usb-next, when I received your review mails.
I'll incorporate your review comments in my tree, run a final set
of tests and then send the pull-req.
---
drivers/usb/core/devio.c | 118 ++++++++++++++++++++++++++++++++++++++
include/uapi/linux/usbdevice_fs.h | 7 +++
2 files changed, 125 insertions(+)
diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index bfb2821..4ca7e86 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -778,6 +778,79 @@ static struct usb_host_endpoint *ep_to_host_endpoint(struct usb_device *dev,
return dev->ep_out[ep & USB_ENDPOINT_NUMBER_MASK];
}
+static int parse_usbdevfs_streams(struct dev_state *ps,
+ struct usbdevfs_streams __user *streams,
+ unsigned int *num_streams_ret,
+ unsigned int *num_eps_ret,
+ struct usb_host_endpoint ***eps_ret,
+ struct usb_interface **intf_ret)
+{
+ unsigned int i, num_streams, num_eps;
+ struct usb_host_endpoint **eps;
+ struct usb_interface *intf = NULL;
+ unsigned char ep;
+ int ifnum, ret;
+
+ if (get_user(num_streams, &streams->num_streams) ||
+ get_user(num_eps, &streams->num_eps))
+ return -EFAULT;
+
+ if (num_eps < 1 || num_eps > USB_MAXENDPOINTS)
+ return -EINVAL;
+
+ /* The XHCI controller allows max 1024 streams */
+ if (num_streams_ret && (num_streams < 2 || num_streams > 1024))
+ return -EINVAL;
We really shouldn't hard-code the 1024 streams value. I'm actually not
sure where you got that value.
I don't remember, I think I got this value from misreading the
HCC_MAX_PSA macro. Upon second reading that macro leads to a max value
of 2 ^ 16.
Each xHCI host can indicate the max
primary stream array size (basically the number of streams) it supports,
see MaxPSASize in section 5.3.6 of the xHCI spec. An xHCI host can
choose to indicate it doesn't support streams at all, by setting that
value to zero.
Ideally, we should have something like bus->sg_tablesize that allows the
xHCI host to indicate the number of streams it supports. Perhaps
usb_bus->max_streams?
I don't think that is necessary. The usb_alloc_streams API says that it can
return less then requested. The check for > 1024 is only there to detect
that what userspace is asking for is utter non-sense. I'll raise it to 2 ^ 16,
anything below 2 ^ 16 is fair game even if the controller supports less,
the application will simply get less streams then requested.
Something which we do need to handle, which the xhci driver currently does not
seem to handle is MaxPSASize being 0. But that should be handled at the xhci
level IMHO. I'll add a patch for this to my tree.
Regards,
Hans
--
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
