On Sun, 5 Jul 2009, Adam Nielsen wrote:
> >> In this particular case I know that VirtualBox isn't to blame, because if I
> >> plug in the real hardware device then it works perfectly (this is how I was
> >> able to initially reverse engineer the communication protocol.) It's only my
> >> gadgetfs device that isn't communicating properly.
> >
> > It may be that the Windows app behaves differently because of the
> > differences between your gadget and the real thing. Have you compared
> > usbmon traces for the two cases?
>
> Okay, I've been through the whole HID initialisation sequence (which resulted
> in some new dissector code for Wireshark) and here are the only differences I
> can see:
>
> - In the device descriptor:
> - Hardware is USB1.1, gadgetfs is USB2.0
> - Hardware bcdDevice is 0x101, gadgetfs overrides this as 0x100
I don't see anything in the gadgetfs source code to override bcdDevice.
Are you sure it happens?
> - Hardware bMaxPacketSize0 is 8, gadgetfs overrides this as 64
> - In the config descriptor:
> - Hardware bmAttributes is bus powered, remote wakeup. Gadgetfs device
> won't be detected under Linux unless it's listed as self-powered, no remote
> wakeup.
I don't understand that. What do you mean, it won't be detected? What
will happen?
> - After returning the HID descriptor, the host sends the hardware an empty
> write on its interrupt output endpoint. This message never appears in the
> usbmon trace for the gadgetfs driver. (But only under win32, the interrupt
> does appear when Linux is accessing the device.)
This is probably the result of a bug in usbfs (and possibly a bug in
libusb as well); 0-length transfers aren't accepted.
> I think the last point is the most significant - interrupt messages don't seem
> to be passed through regardless of which direction they're in.
Are you sure the problem is interrupt transfers and not 0-length
transfers?
> I'm not sure
> why the host is trying to write zero bytes to the read-only interrupt
> endpoint, but my gadgetfs code gets an error if I try to read from the
> interrupt endpoint (which is normally used for writing data out to the host.)
Above you wrote that the host sends a 0-length transfer to the
"interrupt output endpoint". Here you say the "read-only interrupt
endpoint". Clearly "output" != "read-only". Which is it?
> > "arrives on an interrupt endpoint" is a little misleading. The data
> > may be sitting in the device's buffer for that endpoint, but it won't
> > arrive at the host until the host asks for it.
>
> So in the case of a keyboard sending keypresses over an interrupt endpoint,
> how does the host know when keys are pressed? Does it need to poll the
> interrupt endpoint just in case some data arrives?
That's right.
> Why not then use a bulk
> endpoint?
Three reasons. First, interrupt endpoints have guaranteed maximum
latency (important for real-time response to input events) and bulk
endpoints don't. Second, polling bulk endpoints uses up a lot more
bandwidth than polling interrupt endpoints. Third, low-speed devices
(which includes the majority of USB mice) aren't allowed to have bulk
endpoints.
> I always assumed the host would be notified when there is data to
> read from an interrupt endpoint.
Your assumption was wrong. This is all explained in the USB 2.0
specification, freely available from www.usb.org. Much of it is quite
readable.
> > Comparison of the usbmon logs should show the important differences.
> > In fact, you've got four cases to look at, by varying the application
> > (Windows app vs. your test program) and the device (real thing vs. your
> > gadget).
>
> Well apart from the minor issues listed above, the usbmon traces look
> identical for all four cases, with one of them (Windows to gadgetfs) having no
> interrupt messages. The HID initialisation code (which tells the host there
> is an interrupt endpoint and where it is) is identical.
>
> Any other ideas? I'm happy to post my code if it would help.
Here is a patch to make usbfs accept 0-length transfers. You may need
to tweak it somewhat to make it apply to your kernel. And you should
check libusb to see if it rejects 0-length transfers as well.
Alan Stern
Index: usb-2.6/drivers/usb/core/devio.c
===================================================================
--- usb-2.6.orig/drivers/usb/core/devio.c
+++ usb-2.6/drivers/usb/core/devio.c
@@ -984,7 +984,7 @@ static int proc_do_submiturb(struct dev_
USBDEVFS_URB_ZERO_PACKET |
USBDEVFS_URB_NO_INTERRUPT))
return -EINVAL;
- if (!uurb->buffer)
+ if (uurb->buffer_length > 0 && !uurb->buffer)
return -EINVAL;
if (!(uurb->type == USBDEVFS_URB_TYPE_CONTROL &&
(uurb->endpoint & ~USB_ENDPOINT_DIR_MASK) == 0)) {
@@ -1040,11 +1040,6 @@ static int proc_do_submiturb(struct dev_
is_in = 0;
uurb->endpoint &= ~USB_DIR_IN;
}
- if (!access_ok(is_in ? VERIFY_WRITE : VERIFY_READ,
- uurb->buffer, uurb->buffer_length)) {
- kfree(dr);
- return -EFAULT;
- }
break;
case USBDEVFS_URB_TYPE_BULK:
@@ -1057,9 +1052,6 @@ static int proc_do_submiturb(struct dev_
uurb->number_of_packets = 0;
if (uurb->buffer_length > MAX_USBFS_BUFFER_SIZE)
return -EINVAL;
- if (!access_ok(is_in ? VERIFY_WRITE : VERIFY_READ,
- uurb->buffer, uurb->buffer_length))
- return -EFAULT;
break;
case USBDEVFS_URB_TYPE_ISO:
@@ -1099,27 +1091,34 @@ static int proc_do_submiturb(struct dev_
return -EINVAL;
if (uurb->buffer_length > MAX_USBFS_BUFFER_SIZE)
return -EINVAL;
- if (!access_ok(is_in ? VERIFY_WRITE : VERIFY_READ,
- uurb->buffer, uurb->buffer_length))
- return -EFAULT;
break;
default:
return -EINVAL;
}
- as = alloc_async(uurb->number_of_packets);
- if (!as) {
+ if (uurb->buffer_length > 0 &&
+ !access_ok(is_in ? VERIFY_WRITE : VERIFY_READ,
+ uurb->buffer, uurb->buffer_length)) {
kfree(isopkt);
kfree(dr);
- return -ENOMEM;
+ return -EFAULT;
}
- as->urb->transfer_buffer = kmalloc(uurb->buffer_length, GFP_KERNEL);
- if (!as->urb->transfer_buffer) {
+ as = alloc_async(uurb->number_of_packets);
+ if (!as) {
kfree(isopkt);
kfree(dr);
- free_async(as);
return -ENOMEM;
}
+ if (uurb->buffer_length > 0) {
+ as->urb->transfer_buffer = kmalloc(uurb->buffer_length,
+ GFP_KERNEL);
+ if (!as->urb->transfer_buffer) {
+ kfree(isopkt);
+ kfree(dr);
+ free_async(as);
+ return -ENOMEM;
+ }
+ }
as->urb->dev = ps->dev;
as->urb->pipe = (uurb->type << 30) |
__create_pipe(ps->dev, uurb->endpoint & 0xf) |
@@ -1161,7 +1160,7 @@ static int proc_do_submiturb(struct dev_
kfree(isopkt);
as->ps = ps;
as->userurb = arg;
- if (uurb->endpoint & USB_DIR_IN)
+ if (is_in && uurb->buffer_length > 0)
as->userbuffer = uurb->buffer;
else
as->userbuffer = NULL;
@@ -1171,9 +1170,9 @@ static int proc_do_submiturb(struct dev_
as->uid = cred->uid;
as->euid = cred->euid;
security_task_getsecid(current, &as->secid);
- if (!is_in) {
+ if (!is_in && uurb->buffer_length > 0) {
if (copy_from_user(as->urb->transfer_buffer, uurb->buffer,
- as->urb->transfer_buffer_length)) {
+ uurb->buffer_length)) {
free_async(as);
return -EFAULT;
}
--
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
