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

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

The following reply was made to PR kern/46696; it has been noted by GNATS.

From: Nick Hudson <nick.hudson%gmx.co.uk@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: kato%wide.ad.jp@localhost, kern-bug-people%netbsd.org@localhost, 
gnats-admin%netbsd.org@localhost, 
 netbsd-bugs%netbsd.org@localhost, mishka%NetBSD.org@localhost
Subject: Re: kern/46696: uhub disables port where USB keyboard attached
Date: Mon, 25 Mar 2013 07:46:40 +0000

 This is a multi-part message in MIME format.
 --------------080504070601060102070209
 Content-Type: text/plain; charset=ISO-8859-1; format=flowed
 Content-Transfer-Encoding: 7bit
 
 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
 
 
 --------------080504070601060102070209
 Content-Type: text/plain; charset=us-ascii;
  name="netbsd-6.usb_subr.c.diff"
 Content-Transfer-Encoding: 7bit
 Content-Disposition: attachment;
  filename="netbsd-6.usb_subr.c.diff"
 
 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 */
 
 
 
 --------------080504070601060102070209
 Content-Type: text/plain; charset=us-ascii;
  name="usb_subr.c.diff"
 Content-Transfer-Encoding: 7bit
 Content-Disposition: attachment;
  filename="usb_subr.c.diff"
 
 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;
  }
  
 
 
 --------------080504070601060102070209--