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

From: Hans Petter Selasky <hsela...@c2i.net>
To: freebsd-usb@freebsd.org
Cc: Brett Glass <freebsd-...@brettglass.com>,
 freebsd-gnats-sub...@freebsd.org
Subject: Re: usb/160299: MicroSDHC-to-USB adapters do not work in FreeBSD 8.x
Date: Tue, 30 Aug 2011 10:59:52 +0200

 --Boundary-00=_IaKXO97fubXM8EQ
 Content-Type: Text/Plain;
   charset="iso-8859-15"
 Content-Transfer-Encoding: 7bit
 
 On Monday 29 August 2011 23:05:30 Brett Glass wrote:
 > >Number:         160299
 > >Category:       usb
 > >Synopsis:       MicroSDHC-to-USB adapters do not work in FreeBSD 8.x
 > >Confidential:   no
 > >Severity:       serious
 > >Priority:       high
 > >Responsible:    freebsd-usb
 > >State:          open
 > >Quarter:
 > >Keywords:
 > >Date-Required:
 > >Class:          sw-bug
 > >Submitter-Id:   current-users
 > >Arrival-Date:   Mon Aug 29 21:10:08 UTC 2011
 > >Closed-Date:
 > >Last-Modified:
 > >Originator:     Brett Glass
 > >Release:        FreeBSD 8.1-RELEASE
 > 
 > >Organization:
 > LARIAT
 > 
 > >Environment:
 > 
 > >Description:
 > I have tried MicroSDHC cards from several different vendors (Kingston,
 > Sandisk, etc.), with different MicroSDHC-to-USB adapters (also Kingston
 > and Sandisk), in FreeBSD 8.x systems. All cause SCSI errors such as
 > 
 > (da1:umass-sim1:1:0:0): SYNCHRONIZE CACHE(10). CDB: 35 0 0 0 0 0 0 0 0 0
 > (da1:umass-sim1:1:0:0): SCSI sense: Error code 0x52
 > 
 > Some USB flash memory sticks also produce similar errors. In all cases the
 > system sometimes hangs in the driver and the memory card or stick gets
 > quite warm, as if the system is trying the failed operation again and
 > again.
 > 
 > It appears that the problem, which has existed since FreeBSD 4.x, is that
 > the system expects to be able to issue SCSI commands to flash drives
 > (which are not SCSI drives). As a search of recent PRs reveals, this
 > problem has been addressed as a "quirk" on a per-device basis for many
 > individual devices (including memory sticks and cell phones that emulate
 > them), but keeps recurring as new ones are released. A more general fix is
 > needed.
 > 
 > >How-To-Repeat:
 > Place a MicroSDHC card in a USB adapter and insert in a FreeBSD 8.x
 > machine. Try to read and write it.
 > 
 > >Fix:
 > This behavior is so common that it should not be characterized as a quirk
 > but as a general property of USB flash devices. All USB flash storage
 > devices should have SYNCHRONIZE CACHE and similar SCSI commands disabled
 > by default. These commands should, of course, be enabled for USB-attached
 > ATAPI rotating media, which supports them.
 > 
 > >Release-Note:
 > >Audit-Trail:
 > 
 > >Unformatted:
 
 Hi,
 
 Can you try the attached patch and report back. Should work on 8-stable and 9-
 current.
 
 --HPS
 
 --Boundary-00=_IaKXO97fubXM8EQ
 Content-Type: text/x-patch;
   charset="iso-8859-15";
   name="msc_auto_quirk.patch"
 Content-Transfer-Encoding: 7bit
 Content-Disposition: inline;
        filename="msc_auto_quirk.patch"
 
 === sys/dev/usb/quirk/usb_quirk.c
 ==================================================================
 --- sys/dev/usb/quirk/usb_quirk.c      (revision 225095)
 +++ sys/dev/usb/quirk/usb_quirk.c      (local)
 @@ -567,9 +567,9 @@
        uint16_t x;
        uint16_t y;
  
 -      if (quirk == UQ_NONE) {
 -              return (0);
 -      }
 +      if (quirk == UQ_NONE)
 +              goto done;
 +
        mtx_lock(&usb_quirk_mtx);
  
        for (x = 0; x != USB_DEV_QUIRKS_MAX; x++) {
 @@ -603,7 +603,8 @@
                break;
        }
        mtx_unlock(&usb_quirk_mtx);
 -      return (0);
 +done:
 +      return (usb_test_quirk_w(info, quirk));
  }
  
  static struct usb_quirk_entry *
 === sys/dev/usb/usb_device.c
 ==================================================================
 --- sys/dev/usb/usb_device.c   (revision 225095)
 +++ sys/dev/usb/usb_device.c   (local)
 @@ -1239,8 +1239,10 @@
  usb_init_attach_arg(struct usb_device *udev,
      struct usb_attach_arg *uaa)
  {
 -      bzero(uaa, sizeof(*uaa));
 +      uint8_t x;
  
 +      memset(uaa, 0, sizeof(*uaa));
 +
        uaa->device = udev;
        uaa->usb_mode = udev->flags.usb_mode;
        uaa->port = udev->port_no;
 @@ -1254,6 +1256,9 @@
        uaa->info.bDeviceProtocol = udev->ddesc.bDeviceProtocol;
        uaa->info.bConfigIndex = udev->curr_config_index;
        uaa->info.bConfigNum = udev->curr_config_no;
 +
 +      for (x = 0; x != USB_MAX_AUTO_QUIRK; x++)
 +              uaa->info.autoQuirk[x] = udev->autoQuirk[x];
  }
  
  /*------------------------------------------------------------------------*
 @@ -1850,7 +1855,23 @@
                        }
                }
        }
 +      if (set_config_failed == 0 && config_index == 0 &&
 +          usb_test_quirk(&uaa, UQ_MSC_NO_SYNC_CACHE) == 0) {
  
 +              /*
 +               * Try to figure out if there are any MSC quirks we
 +               * should apply automatically:
 +               */
 +              err = usb_msc_auto_quirk(udev, 0);
 +
 +              if (err != 0) {
 +                      usbd_add_dynamic_quirk(udev, UQ_MSC_NO_SYNC_CACHE);
 +
 +                      set_config_failed = 1;
 +                      goto repeat_set_config;
 +              }
 +      }
 +
  config_done:
        DPRINTF("new dev (addr %d), udev=%p, parent_hub=%p\n",
            udev->address, udev, udev->parent_hub);
 @@ -2698,3 +2719,16 @@
        return (0);                     /* success */
  }
  
 +usb_error_t
 +usbd_add_dynamic_quirk(struct usb_device *udev, uint16_t quirk)
 +{
 +      uint8_t x;
 +
 +      for (x = 0; x != USB_MAX_AUTO_QUIRK; x++) {
 +              if (udev->autoQuirk[x] == 0) {
 +                      udev->autoQuirk[x] = quirk;
 +                      return (0);     /* success */
 +              }
 +      }
 +      return (USB_ERR_NOMEM);
 +}
 === sys/dev/usb/usb_device.h
 ==================================================================
 --- sys/dev/usb/usb_device.h   (revision 225095)
 +++ sys/dev/usb/usb_device.h   (local)
 @@ -149,6 +149,7 @@
  
        uint16_t power;                 /* mA the device uses */
        uint16_t langid;                /* language for strings */
 +      uint16_t autoQuirk[USB_MAX_AUTO_QUIRK];         /* dynamic quirks */
  
        uint8_t address;                /* device addess */
        uint8_t device_index;           /* device index in "bus->devices" */
 === sys/dev/usb/usb_dynamic.c
 ==================================================================
 --- sys/dev/usb/usb_dynamic.c  (revision 225095)
 +++ sys/dev/usb/usb_dynamic.c  (local)
 @@ -50,12 +50,12 @@
  #include <dev/usb/usb_process.h>
  #include <dev/usb/usb_device.h>
  #include <dev/usb/usb_dynamic.h>
 +#include <dev/usb/quirk/usb_quirk.h>
  
  /* function prototypes */
  static usb_handle_req_t usb_temp_get_desc_w;
  static usb_temp_setup_by_index_t usb_temp_setup_by_index_w;
  static usb_temp_unsetup_t usb_temp_unsetup_w;
 -static usb_test_quirk_t usb_test_quirk_w;
  static usb_quirk_ioctl_t usb_quirk_ioctl_w;
  
  /* global variables */
 @@ -72,9 +72,19 @@
        return (USB_ERR_INVAL);
  }
  
 -static uint8_t
 +uint8_t
  usb_test_quirk_w(const struct usbd_lookup_info *info, uint16_t quirk)
  {
 +      uint8_t x;
 +
 +      if (quirk == UQ_NONE)
 +              return (0);     /* no match */
 +
 +      for (x = 0; x != USB_MAX_AUTO_QUIRK; x++) {
 +              if (info->autoQuirk[x] == quirk)
 +                      return (1);     /* match */
 +      }
 +
        return (0);                     /* no match */
  }
  
 === sys/dev/usb/usb_dynamic.h
 ==================================================================
 --- sys/dev/usb/usb_dynamic.h  (revision 225095)
 +++ sys/dev/usb/usb_dynamic.h  (local)
 @@ -57,5 +57,6 @@
  void  usb_temp_unload(void *);
  void  usb_quirk_unload(void *);
  void  usb_bus_unload(void *);
 +usb_test_quirk_t usb_test_quirk_w;
  
  #endif                                        /* _USB_DYNAMIC_H_ */
 === sys/dev/usb/usb_freebsd.h
 ==================================================================
 --- sys/dev/usb/usb_freebsd.h  (revision 225095)
 +++ sys/dev/usb/usb_freebsd.h  (local)
 @@ -68,6 +68,8 @@
  #define       USB_EP0_BUFSIZE         1024    /* bytes */
  #define       USB_CS_RESET_LIMIT      20      /* failures = 20 * 50 ms = 1sec 
*/
  
 +#define       USB_MAX_AUTO_QUIRK      4       /* maximum number of dynamic 
quirks */
 +
  typedef uint32_t usb_timeout_t;               /* milliseconds */
  typedef uint32_t usb_frlength_t;      /* bytes */
  typedef uint32_t usb_frcount_t;               /* units */
 === sys/dev/usb/usb_msctest.c
 ==================================================================
 --- sys/dev/usb/usb_msctest.c  (revision 225095)
 +++ sys/dev/usb/usb_msctest.c  (local)
 @@ -96,6 +96,8 @@
                                          0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
                                          0x00, 0x00, 0x00, 0x00 };
  static uint8_t scsi_tct_eject[] =     { 0x06, 0xf5, 0x04, 0x02, 0x52, 0x70 };
 +static uint8_t scsi_sync_cache[] =    { 0x35, 0x00, 0x00, 0x00, 0x00, 0x00,
 +                                        0x00, 0x00, 0x00, 0x00 };
  
  #define       BULK_SIZE               64      /* dummy */
  #define       ERR_CSW_FAILED          -1
 @@ -163,7 +165,7 @@
  static void   bbb_transfer_start(struct bbb_transfer *, uint8_t);
  static void   bbb_data_clear_stall_callback(struct usb_xfer *, uint8_t,
                    uint8_t);
 -static uint8_t bbb_command_start(struct bbb_transfer *, uint8_t, uint8_t,
 +static int    bbb_command_start(struct bbb_transfer *, uint8_t, uint8_t,
                    void *, size_t, void *, size_t, usb_timeout_t);
  static struct bbb_transfer *bbb_attach(struct usb_device *, uint8_t);
  static void   bbb_detach(struct bbb_transfer *);
 @@ -454,7 +456,7 @@
   * 0: Success
   * Else: Failure
   *------------------------------------------------------------------------*/
 -static uint8_t
 +static int
  bbb_command_start(struct bbb_transfer *sc, uint8_t dir, uint8_t lun,
      void *data_ptr, size_t data_len, void *cmd_ptr, size_t cmd_len,
      usb_timeout_t data_timeout)
 @@ -566,9 +568,10 @@
  usb_iface_is_cdrom(struct usb_device *udev, uint8_t iface_index)
  {
        struct bbb_transfer *sc;
 -      usb_error_t err;
 -      uint8_t timeout, is_cdrom;
 +      uint8_t timeout;
 +      uint8_t is_cdrom;
        uint8_t sid_type;
 +      int err;
  
        sc = bbb_attach(udev, iface_index);
        if (sc == NULL)
 @@ -595,6 +598,61 @@
  }
  
  usb_error_t
 +usb_msc_auto_quirk(struct usb_device *udev, uint8_t iface_index)
 +{
 +      struct bbb_transfer *sc;
 +      uint8_t timeout;
 +      uint8_t is_no_direct;
 +      uint8_t sid_type;
 +      int err;
 +
 +      sc = bbb_attach(udev, iface_index);
 +      if (sc == NULL)
 +              return (0);
 +
 +      is_no_direct = 1;
 +      timeout = 4;    /* tries */
 +      while (--timeout) {
 +              err = bbb_command_start(sc, DIR_IN, 0, sc->buffer,
 +                  SCSI_INQ_LEN, &scsi_inquiry, sizeof(scsi_inquiry),
 +                  USB_MS_HZ);
 +
 +              if (err == 0 && sc->actlen > 0) {
 +                      sid_type = sc->buffer[0] & 0x1F;
 +                      if (sid_type == 0x00)
 +                              is_no_direct = 0;
 +                      break;
 +              } else if (err != ERR_CSW_FAILED)
 +                      break;  /* non retryable error */
 +              usb_pause_mtx(NULL, hz);
 +      }
 +
 +      if (is_no_direct) {
 +              DPRINTF("Device is not direct access.\n");
 +              goto done;
 +      }
 +
 +      err = bbb_command_start(sc, DIR_IN, 0, NULL, 0,
 +          &scsi_sync_cache, sizeof(scsi_sync_cache),
 +          USB_MS_HZ);
 +
 +      if ((err != 0) && (err != ERR_CSW_FAILED)) {
 +
 +              DPRINTF("Device doesn't handle synchronize cache\n");
 +
 +              /* Need to re-enumerate the device */
 +              usbd_req_re_enumerate(udev, NULL);
 +
 +              bbb_detach(sc);
 +              return (USB_ERR_STALLED);
 +      }
 +
 + done:
 +      bbb_detach(sc);
 +      return (0);
 +}
 +
 +usb_error_t
  usb_msc_eject(struct usb_device *udev, uint8_t iface_index, int method)
  {
        struct bbb_transfer *sc;
 === sys/dev/usb/usb_msctest.h
 ==================================================================
 --- sys/dev/usb/usb_msctest.h  (revision 225095)
 +++ sys/dev/usb/usb_msctest.h  (local)
 @@ -40,5 +40,7 @@
            uint8_t iface_index);
  usb_error_t usb_msc_eject(struct usb_device *udev,
            uint8_t iface_index, int method);
 +usb_error_t usb_msc_auto_quirk(struct usb_device *udev,
 +          uint8_t iface_index);
  
  #endif                                        /* _USB_MSCTEST_H_ */
 === sys/dev/usb/usbdi.h
 ==================================================================
 --- sys/dev/usb/usbdi.h        (revision 225095)
 +++ sys/dev/usb/usbdi.h        (local)
 @@ -353,6 +353,7 @@
        uint16_t idVendor;
        uint16_t idProduct;
        uint16_t bcdDevice;
 +      uint16_t autoQuirk[USB_MAX_AUTO_QUIRK];
        uint8_t bDeviceClass;
        uint8_t bDeviceSubClass;
        uint8_t bDeviceProtocol;
 @@ -475,6 +476,8 @@
  void  usb_pause_mtx(struct mtx *mtx, int _ticks);
  usb_error_t   usbd_set_pnpinfo(struct usb_device *udev,
                        uint8_t iface_index, const char *pnpinfo);
 +usb_error_t   usbd_add_dynamic_quirk(struct usb_device *udev,
 +                      uint16_t quirk);
  
  const struct usb_device_id *usbd_lookup_id_by_info(
            const struct usb_device_id *id, usb_size_t sizeof_id,
 
 --Boundary-00=_IaKXO97fubXM8EQ--
_______________________________________________
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to "freebsd-usb-unsubscr...@freebsd.org"

Reply via email to