Re: kern/46696: uhub disables port where USB keyboard attached

[Nick Hudson <nick.hudson%gmx.co.uk@localhost>]

On 03/25/13 02:30, kato%wide.ad.jp@localhost wrote:
>   Dear Nick,
Hi,

>   Sorry for a late response. I applied your patch to my NetBSD-6 (6.1_RC2,
>   sync'ed with CVS tree just now). The patch was applied successfully with
>   30 lines offset.
>   
>   I rebooted the machine with the patched kernel 8 times, and in 5 trials
>   out of 8, the USB keyboard worked fine. But unfortunately in 3 trials,
>   the USB keyboard didn't work at all. When I unplug the USB keyboard,
>   and re-plug it with the same USB port, it started to work well. So the
>   patch improves the situation but doesn't solve the issue, at least with
>   NetBSD-6.1_RC2.

Here's something else to try. I've provided patches against -current and 
netbsd-6.

>   The alternate patch
>         http://www.gson.org/netbsd/bugs/46696/usb.patch
>   works well even with NetBSD-6.

I don't think reverting the previous patch is the right thing to do. A 
quick poll of how other
OSes handle this suggest that the initial query of 64 bytes is very common.

Thanks,
Nick

Index: sys/dev/usb/usb_subr.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/usb_subr.c,v
retrieving revision 1.180
diff -u -p -u -r1.180 usb_subr.c
--- sys/dev/usb/usb_subr.c      9 Jun 2011 19:08:33 -0000       1.180
+++ sys/dev/usb/usb_subr.c      25 Mar 2013 07:41:51 -0000
@@ -1033,6 +1033,9 @@ usbd_reattach_device(device_t parent, us
  * Do as Windows does: try to read 64 bytes -- there are devices which
  * recognize the initial descriptor fetch (before the control endpoint's
  * MaxPacketSize is known by the host) by exactly this length.
+ *
+ * If the 64 bytes request fails try for just the 8 that the USB 2.0
+ * specification says should be requested.
  */
 static usbd_status
 usbd_get_initial_ddesc(usbd_device_handle dev, usb_device_descriptor_t *desc)
@@ -1046,13 +1049,23 @@ usbd_get_initial_ddesc(usbd_device_handl
        USETW2(req.wValue, UDESC_DEVICE, 0);
        USETW(req.wIndex, 0);
        USETW(req.wLength, 64);
+       DPRINTF(("%s asking for 64 bytes\n", __func__));
+
        res = usbd_do_request_flags(dev, &req, buf, USBD_SHORT_XFER_OK,
                &actlen, USBD_DEFAULT_TIMEOUT);
-       if (res)
-               return res;
+       if (res) {
+               DPRINTF(("%s asking for 8 bytes\n", __func__));
+               USETW(req.wLength, 8);
+               res = usbd_do_request_flags(dev, &req, buf, USBD_SHORT_XFER_OK,
+                       &actlen, USBD_DEFAULT_TIMEOUT);
+               if (res)
+                       return res;
+       }
        if (actlen < 8)
                return USBD_SHORT_XFER;
        memcpy(desc, buf, 8);
+       DPRINTF(("%s got info\n", __func__));
+
        return USBD_NORMAL_COMPLETION;
 }
 
@@ -1103,7 +1116,11 @@ usbd_new_device(device_t parent, usbd_bu
         * (which uses 64 bytes so it shouldn't be less),
         * highspeed devices must support 64 byte packets anyway
         */
-       USETW(dev->def_ep_desc.wMaxPacketSize, 64);
+       if (speed == USB_SPEED_HIGH || speed == USB_SPEED_FULL)
+               USETW(dev->def_ep_desc.wMaxPacketSize, 64);
+       else
+               USETW(dev->def_ep_desc.wMaxPacketSize, USB_MAX_IPACKET);
+
        dev->def_ep_desc.bInterval = 0;
 
        /* doesn't matter, just don't let it uninitialized */


Index: sys/dev/usb/usb_subr.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/usb_subr.c,v
retrieving revision 1.190
diff -u -p -u -r1.190 usb_subr.c
--- sys/dev/usb/usb_subr.c      20 Mar 2013 11:14:51 -0000      1.190
+++ sys/dev/usb/usb_subr.c      25 Mar 2013 07:38:41 -0000
@@ -1063,6 +1063,9 @@ usbd_reattach_device(device_t parent, us
  * Do as Windows does: try to read 64 bytes -- there are devices which
  * recognize the initial descriptor fetch (before the control endpoint's
  * MaxPacketSize is known by the host) by exactly this length.
+ *
+ * If the 64 bytes request fails try for just the 8 that the USB 2.0
+ * specification says should be requested.
  */
 static usbd_status
 usbd_get_initial_ddesc(usbd_device_handle dev, usb_device_descriptor_t *desc)
@@ -1076,13 +1079,23 @@ usbd_get_initial_ddesc(usbd_device_handl
        USETW2(req.wValue, UDESC_DEVICE, 0);
        USETW(req.wIndex, 0);
        USETW(req.wLength, 64);
+       DPRINTF(("%s asking for 64 bytes\n", __func__));
+
        res = usbd_do_request_flags(dev, &req, buf, USBD_SHORT_XFER_OK,
                &actlen, USBD_DEFAULT_TIMEOUT);
-       if (res)
-               return res;
+       if (res) {
+               DPRINTF(("%s asking for 8 bytes\n", __func__));
+               USETW(req.wLength, 8);
+               res = usbd_do_request_flags(dev, &req, buf, USBD_SHORT_XFER_OK,
+                       &actlen, USBD_DEFAULT_TIMEOUT);
+               if (res)
+                       return res;
+       }
        if (actlen < 8)
                return USBD_SHORT_XFER;
        memcpy(desc, buf, 8);
+       DPRINTF(("%s got info\n", __func__));
+
        return USBD_NORMAL_COMPLETION;
 }