On Wed, 22 Sep 2004, Kyle Harris wrote:
> This is against 2.6.8.1
>
> Changes -
>
> Change the way that a device is enumerated since some devices fail if the
> initial get_device_descriptor does not request the actual descriptor length.
I don't like many of the details in your patch:
It unconditionally changes the way enumeration works. This make break
some devices that work okay now. It's okay to add a new scheme, but we
should try the old scheme first and if that fails then fallback to the new
scheme.
You neglected to call usb_disable_endpoint(). This should be done
whenever the maxpacket length of an endpoint (in this case, endpoint 0 for
device 0) changes.
You allocate your 64-byte buffer using GFP_KERNEL. That's wrong since
this is on one of the code paths for usb-storage; you have to use GFP_NOIO
or GFP_ATOMIC.
I don't like the way you changed usb_submit_urb to allow destinations
with address 0. It's possible to send the Get-Descriptor request without
making that change. (Maybe that indicates a bug in usb_submit_urb...)
I don't like the way you repeatedly set udev->devnum to -1. This should
be done in only one spot.
If you really want to imitate Windows, you should issue the 64-byte
Get-Descriptor request to every device regardless of its speed. Then
skip the existing 8-byte request, as it would no longer be needed.
Below is a patch I worked out a month or two ago. It may be slightly
stale now, and it probably could use a little improvement, but take a
look.
Alan Stern
===== drivers/usb/core/hub.c 1.188 vs edited =====
--- 1.188/drivers/usb/core/hub.c Thu Jul 29 06:00:59 2004
+++ edited/drivers/usb/core/hub.c Wed Aug 4 15:02:30 2004
@@ -1924,6 +1924,7 @@
int i, j, retval;
unsigned delay = HUB_SHORT_RESET_TIME;
enum usb_device_speed oldspeed = udev->speed;
+ int get_descriptor_first = 0;
/* root hub ports have a slightly longer reset period
* (from USB 2.0 spec, section 7.1.7.5)
@@ -1961,19 +1962,18 @@
i = 64;
break;
case USB_SPEED_FULL: /* 8, 16, 32, or 64 */
- /* to determine the ep0 maxpacket size, read the first 8
- * bytes from the device descriptor to get bMaxPacketSize0;
- * then correct our initial (small) guess.
+ /* to determine the ep0 maxpacket size, try to read
+ * the device descriptor to get bMaxPacketSize0 and
+ * then correct our initial guess.
*/
- // FALLTHROUGH
+ i = 64;
+ break;
case USB_SPEED_LOW: /* fixed at 8 */
i = 8;
break;
default:
goto fail;
}
- udev->epmaxpacketin [0] = i;
- udev->epmaxpacketout[0] = i;
dev_info (&udev->dev,
"%s %s speed USB device using address %d\n",
@@ -1998,17 +1998,58 @@
udev->tt = &hub->tt;
udev->ttport = port + 1;
}
+
+ /* Clear out any stale HC state pertaining to device 0 endpoint 0 */
+ j = udev->devnum;
+ udev->devnum = 0;
+ usb_disable_endpoint(udev, 0 + USB_DIR_IN);
+ usb_disable_endpoint(udev, 0 + USB_DIR_OUT);
+ udev->devnum = j;
+ udev->epmaxpacketin [0] = i;
+ udev->epmaxpacketout[0] = i;
/* Why interleave GET_DESCRIPTOR and SET_ADDRESS this way?
* Because device hardware and firmware is sometimes buggy in
* this area, and this is how Linux has done it for ages.
* Change it cautiously.
*
- * NOTE: Windows gets the descriptor first, seemingly to help
- * work around device bugs like "can't use addresses with bit 3
- * set in certain configurations". Yes, really.
+ * NOTE: For full-speed devices, if the bMaxPacketSize0 field
+ * is initially set to 1 we will get the descriptor first,
+ * using a 64-byte transfer request. This is what Windows does,
+ * so it may help with some non-standards-compliant devices.
*/
- for (i = 0; i < GET_DESCRIPTOR_TRIES; ++i) {
+ if (udev->descriptor.bMaxPacketSize0 == 1)
+ get_descriptor_first = 1;
+
+ for (i = 0; i < GET_DESCRIPTOR_TRIES; (++i, msleep(100))) {
+ if (get_descriptor_first) {
+ struct usb_device_descriptor *buf;
+
+ buf = kmalloc(64, GFP_NOIO);
+ if (!buf) {
+ retval = -ENOMEM;
+ continue;
+ }
+ memset(buf, 0, 64);
+ (void) usb_control_msg(udev,
+ /* Address 0 */
+ (PIPE_CONTROL << 30) | USB_DIR_IN,
+ USB_REQ_GET_DESCRIPTOR, USB_DIR_IN,
+ USB_DT_DEVICE << 8, 0,
+ buf, 64,
+ HZ * USB_CTRL_GET_TIMEOUT);
+ udev->descriptor = *buf;
+ kfree(buf);
+
+ retval = hub_port_reset(hdev, port, udev, delay);
+ if (retval < 0) /* error or disconnect */
+ goto fail;
+ if (udev->descriptor.bMaxPacketSize0 < 8) {
+ retval = -EMSGSIZE;
+ continue;
+ }
+ }
+
for (j = 0; j < SET_ADDRESS_TRIES; ++j) {
retval = hub_set_address(udev);
if (retval >= 0)
@@ -2027,10 +2070,12 @@
* - read ep0 maxpacket even for high and low speed,
*/
msleep(10);
- retval = usb_get_device_descriptor(udev, 8);
+ if (get_descriptor_first)
+ retval = 8;
+ else
+ retval = usb_get_device_descriptor(udev, 8);
if (retval >= 8)
break;
- msleep(100);
}
if (retval != 8) {
dev_err(&udev->dev, "device descriptor read/%s, error %d\n",
@@ -2211,6 +2256,7 @@
}
/* reset and get descriptor */
+ udev->descriptor.bMaxPacketSize0 = (i & 1);
status = hub_port_init(hdev, udev, port);
if (status < 0)
goto loop;
-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170
Project Admins to receive an Apple iPod Mini FREE for your judgement on
who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
