On Tue, Aug 17, 2010 at 02:39:27PM -0700, Sarah Sharp wrote:
> Hi John,
>
> I've been testing your patch with a VIA USB 3.0 hub prototype.
>
> I've tested with a mouse, USB 3.0 hard drive, and high speed webcam.
> All devices seem to work fine under the hub with your patch. The only
> slightly concerning thing is that I see a bunch of stalls when I try to
> run `lsusb -v` for the USB 3.0 hub portion. The lsusb output says:
>
> can't get hub descriptor: Broken pipe
> Device Status: 0x0001
> Self Powered
>
> The USB 3.0 hub descriptor is a different descriptor code (and format)
> than USB 1.1 or 2.0 hubs. It looks like your patch changed the kernel
> code in get_hub_descriptor() to ask for the right descriptor, but I'm
> not sure if userspace is issuing a control transfer through usbfs and
> bypassing the kernel.
>
> Full USBmon, lsusb and dmesg output attached. I'm running lsusb
> (usbutils) version 0.82.
>
> The first failed transfer is
> ffff88012c161cc0 3629591912 S Ci:9:002:0 s a0 06 2900 0000 000d 13 <
> ffff88012c161cc0 3629595966 C Ci:9:002:0 -32 0
>
> bmRequestType = a0, bRequest = 06, wValue = 2900, wIndex = 0000, wLength
> = 000d
>
> That looks like a USB 2.0 GetHubDescriptor request to me.
>
> Alan, Daniel, or Greg, do you know if lsusb is calling through libusb to
> get the hub descriptor, or is it just getting cached descriptors from
> the kernel?
Should be easy to strace to see :)
Anyway, it's lsusb doing a usbfs call through libusb to get the hub
descriptor with the following code:
ret = usb_control_msg(fd,
USB_ENDPOINT_IN | USB_TYPE_CLASS | USB_RECIP_DEVICE,
USB_REQ_GET_DESCRIPTOR,
0x29 << 8, 0,
buf, sizeof buf, CTRL_TIMEOUT);
I think that looks like the above request to me.
> John, I'm concerned that you're blindly assigning the USB 3.0 hub
> descriptor to a usb_hub_descriptor. The current structure is:
>
> struct usb_hub_descriptor {
> __u8 bDescLength;
> __u8 bDescriptorType;
> __u8 bNbrPorts;
> __le16 wHubCharacteristics;
> __u8 bPwrOn2PwrGood;
> __u8 bHubContrCurrent;
> /* add 1 bit for hub status change; round to bytes */
> __u8 DeviceRemovable[(USB_MAXCHILDREN + 1 + 7) / 8];
> __u8 PortPwrCtrlMask[(USB_MAXCHILDREN + 1 + 7) / 8];
> } __attribute__ ((packed));
>
> The USB 3.0 hub descriptor should look something like this:
>
> struct usb_hub_usb3_descriptor {
> __u8 bDescLength;
> __u8 bDescriptorType;
> __u8 bNbrPorts;
> __le16 wHubCharacteristics;
> __u8 bPwrOn2PwrGood;
> __u8 bHubContrCurrent;
> /* add 1 bit for hub status change; round to bytes */
> __u8 bHubHdrDecLat;
> __u16 wHubDelay;
> __u8 DeviceRemovable[2];
> } __attribute__ ((packed));
>
> The descriptor is longer, and the fields after bHubContrCurrent don't
> align properly. So I don't think your current approach of just
> assigning the new hub descriptor to a pointer of the old hub descriptor
> type is going to work very well.
No, that wouldn't, nice catch :)
Hm, I guess this means that lsusb needs to be able to handle USB 3.0
hubs properly, which it doesn't seem to be doing here, right?
thanks,
greg k-h
--
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
