usb/160299: MicroSDHC-to-USB adapters do not work in FreeBSD 8.x

Hans Petter Selasky hselasky at c2i.net
Tue Aug 30 09:10:14 UTC 2011


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

From: Hans Petter Selasky <hselasky at c2i.net>
To: freebsd-usb at freebsd.org
Cc: Brett Glass <freebsd-prs at brettglass.com>,
 freebsd-gnats-submit at 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--


More information about the freebsd-usb mailing list