Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 10 Sep 2007 22:12:52 -0600
From:      Scott Long <scottl@samsco.org>
To:        freebsd-scsi@FreeBSD.ORG, freebsd-current@FreeBSD.ORG
Cc:        njl@FreeBSD.ORG
Subject:   Retirement of CAM_QUIRK_NOSERIAL
Message-ID:  <46E615C4.1010605@samsco.org>

next in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format.
--------------070500040300070701010809
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

All,

The attached patch should make CAM behave properly with regard to
probing device serial numbers only when the device advertises that
it supports it.  It will hopefully eliminate the need for the 
CAM_QUIRK_NOSERIAL quirk (one instance is left because of an unrelated 
legacy problem that may or may not be possible to fix).  This should
especially benefit USB-UMASS devices, where the console output should
be less noisy.  It might even make more devices work out-of-the-box.
So please focus testing on USB, but I'd also ask that people test
the following devices as well as any firewire devices:

  * Western Digital My Book 250GB (USB)
  * Maxtor Personal Storage 3000XT (Firewire)

Thanks,

Scott

--------------070500040300070701010809
Content-Type: text/plain; x-mac-type="0"; x-mac-creator="0";
	name="cam_xpt.sn0.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="cam_xpt.sn0.diff"

Index: cam_xpt.c
===================================================================
RCS file: /usr/ncvs/src/sys/cam/cam_xpt.c,v
retrieving revision 1.190
diff -u -r1.190 cam_xpt.c
--- cam_xpt.c	30 Jun 2007 14:58:56 -0000	1.190
+++ cam_xpt.c	11 Sep 2007 04:00:19 -0000
@@ -464,7 +464,7 @@
 	{
 		/* I can't believe we need a quirk for DPT volumes. */
 		{ T_ANY, SIP_MEDIA_FIXED|SIP_MEDIA_REMOVABLE, "DPT", "*", "*" },
-		CAM_QUIRK_NOSERIAL|CAM_QUIRK_NOLUNS,
+		CAM_QUIRK_NOLUNS,
 		/*mintags*/0, /*maxtags*/255
 	},
 	{
@@ -495,7 +495,7 @@
 			T_SEQUENTIAL, SIP_MEDIA_REMOVABLE, "EXABYTE",
 			"EXB-8200*", "*"
 		},
-		CAM_QUIRK_NOSERIAL|CAM_QUIRK_NOLUNS, /*mintags*/0, /*maxtags*/0
+		CAM_QUIRK_NOLUNS, /*mintags*/0, /*maxtags*/0
 	},
 	{
 		/*
@@ -506,7 +506,7 @@
 			T_SEQUENTIAL, SIP_MEDIA_REMOVABLE, "EXABYTE",
 			"IPL-6860*", "*"
 		},
-		CAM_QUIRK_NOSERIAL|CAM_QUIRK_NOLUNS, /*mintags*/0, /*maxtags*/0
+		CAM_QUIRK_NOLUNS, /*mintags*/0, /*maxtags*/0
 	},
 	{
 		/*
@@ -551,17 +551,6 @@
 	},
 	{
 		/*
-		 * Maxtor Personal Storage 3000XT (Firewire)
-		 * hangs upon serial number probing.
-		 */
-		{
-			T_DIRECT, SIP_MEDIA_FIXED, "Maxtor",
-			"1394 storage", "*"
-		},
-		CAM_QUIRK_NOSERIAL, /*mintags*/0, /*maxtags*/0
-	},
-	{
-		/*
 		 * Would repond to all LUNs if asked for.
 		 */
 		{
@@ -620,18 +609,6 @@
 		CAM_QUIRK_NOLUNS, /*mintags*/0, /*maxtags*/0
 	},
 	{
-		/*
-		 * Western Digital My Book 250GB (USB)
-		 * hangs upon serial number probing.
-		 * PR: 107495
-		 */
-		{
-			T_DIRECT, SIP_MEDIA_FIXED, "WD",
-			"2500JB External", "*"
-		},
-		CAM_QUIRK_NOSERIAL, /*mintags*/0, /*maxtags*/0
-	},
-	{
 		/* Default tagged queuing parameters for all devices */
 		{
 		  T_ANY, SIP_MEDIA_REMOVABLE|SIP_MEDIA_FIXED,
@@ -5472,7 +5449,8 @@
 	PROBE_INQUIRY,	/* this counts as DV0 for Basic Domain Validation */
 	PROBE_FULL_INQUIRY,
 	PROBE_MODE_SENSE,
-	PROBE_SERIAL_NUM,
+	PROBE_SERIAL_NUM_0,
+	PROBE_SERIAL_NUM_1,
 	PROBE_TUR_FOR_NEGOTIATION,
 	PROBE_INQUIRY_BASIC_DV1,
 	PROBE_INQUIRY_BASIC_DV2,
@@ -5815,10 +5793,42 @@
 		}
 		xpt_print(periph->path, "Unable to mode sense control page - "
 		    "malloc failure\n");
-		softc->action = PROBE_SERIAL_NUM;
+		softc->action = PROBE_SERIAL_NUM_0;
 	}
 	/* FALLTHROUGH */
-	case PROBE_SERIAL_NUM:
+	case PROBE_SERIAL_NUM_0:
+	{
+		struct scsi_vpd_supported_page_list *vpd_list = NULL;
+		struct cam_ed *device;
+
+		device = periph->path->device;
+		if ((device->quirk->quirks & CAM_QUIRK_NOSERIAL) == 0) {
+			vpd_list = malloc(sizeof(*vpd_list), M_CAMXPT,
+			    M_NOWAIT | M_ZERO);
+		}
+
+		if (vpd_list != NULL) {
+			scsi_inquiry(csio,
+				     /*retries*/4,
+				     probedone,
+				     MSG_SIMPLE_Q_TAG,
+				     (u_int8_t *)vpd_list,
+				     sizeof(*vpd_list) + 28,
+				     /*evpd*/TRUE,
+				     SVPD_SUPPORTED_PAGE_LIST,
+				     SSD_MIN_SIZE,
+				     /*timeout*/60 * 1000);
+			break;
+		}
+		/*
+		 * We'll have to do without, let our probedone
+		 * routine finish up for us.
+		 */
+		start_ccb->csio.data_ptr = NULL;
+		probedone(periph, start_ccb);
+		return;
+	}
+	case PROBE_SERIAL_NUM_1:
 	{
 		struct scsi_vpd_unit_serial_number *serial_buf;
 		struct cam_ed* device;
@@ -5828,10 +5838,8 @@
 		device->serial_num = NULL;
 		device->serial_num_len = 0;
 
-		if ((device->quirk->quirks & CAM_QUIRK_NOSERIAL) == 0)
-			serial_buf = (struct scsi_vpd_unit_serial_number *)
-				malloc(sizeof(*serial_buf), M_CAMXPT,
-					M_NOWAIT | M_ZERO);
+		serial_buf = (struct scsi_vpd_unit_serial_number *)
+			malloc(sizeof(*serial_buf), M_CAMXPT, M_NOWAIT|M_ZERO);
 
 		if (serial_buf != NULL) {
 			scsi_inquiry(csio,
@@ -6056,7 +6064,7 @@
 				if (INQ_DATA_TQ_ENABLED(inq_buf))
 					softc->action = PROBE_MODE_SENSE;
 				else
-					softc->action = PROBE_SERIAL_NUM;
+					softc->action = PROBE_SERIAL_NUM_0;
 
 				path->device->flags &= ~CAM_DEV_UNCONFIGURED;
 
@@ -6121,11 +6129,61 @@
 		}
 		xpt_release_ccb(done_ccb);
 		free(mode_hdr, M_CAMXPT);
-		softc->action = PROBE_SERIAL_NUM;
+		softc->action = PROBE_SERIAL_NUM_0;
 		xpt_schedule(periph, priority);
 		return;
 	}
-	case PROBE_SERIAL_NUM:
+	case PROBE_SERIAL_NUM_0:
+	{
+		struct ccb_scsiio *csio;
+		struct scsi_vpd_supported_page_list *page_list;
+		int length, serialnum_supported, i;
+
+		serialnum_supported = 0;
+		csio = &done_ccb->csio;
+		page_list =
+		    (struct scsi_vpd_supported_page_list *)csio->data_ptr;
+
+		if (page_list == NULL) {
+			/*
+			 * Don't process the command as it was never sent
+			 */
+		} else if ((csio->ccb_h.status & CAM_STATUS_MASK) == CAM_REQ_CMP
+		    && (page_list->length > 0)) {
+			length = min(page_list->length, csio->dxfer_len - 4);
+			for (i = 0; i < length; i++) {
+				if (page_list->list[i] ==
+				    SVPD_UNIT_SERIAL_NUMBER) {
+					serialnum_supported = 1;
+					break;
+				}
+			}
+		} else if (cam_periph_error(done_ccb, 0,
+					    SF_RETRY_UA|SF_NO_PRINT,
+					    &softc->saved_ccb) == ERESTART) {
+			return;
+		} else if ((done_ccb->ccb_h.status & CAM_DEV_QFRZN) != 0) {
+			/* Don't wedge the queue */
+			xpt_release_devq(done_ccb->ccb_h.path, /*count*/1,
+					 /*run_queue*/TRUE);
+		}
+
+		if (page_list != NULL)
+			free(page_list, M_DEVBUF);
+
+		if (serialnum_supported) {
+			xpt_release_ccb(done_ccb);
+			softc->action = PROBE_SERIAL_NUM_1;
+			xpt_schedule(periph, priority);
+			return;
+		}
+		xpt_release_ccb(done_ccb);
+		softc->action = PROBE_TUR_FOR_NEGOTIATION;
+		xpt_schedule(periph, done_ccb->ccb_h.pinfo.priority);
+		return;
+	}
+			
+	case PROBE_SERIAL_NUM_1:
 	{
 		struct ccb_scsiio *csio;
 		struct scsi_vpd_unit_serial_number *serial_buf;
Index: scsi/scsi_all.h
===================================================================
RCS file: /usr/ncvs/src/sys/cam/scsi/scsi_all.h,v
retrieving revision 1.28
diff -u -r1.28 scsi_all.h
--- scsi/scsi_all.h	4 Dec 2006 23:04:13 -0000	1.28
+++ scsi/scsi_all.h	11 Sep 2007 03:10:52 -0000
@@ -663,6 +663,17 @@
 	u_int8_t vendor_specific1[SID_VENDOR_SPECIFIC_1_SIZE];
 };
 
+struct scsi_vpd_supported_page_list
+{
+	u_int8_t device;
+	u_int8_t page_code;
+#define SVPD_SUPPORTED_PAGE_LIST 0x00
+	u_int8_t reserved;
+	u_int8_t length;	/* number of VPD entries */
+#define SVPD_SUPPORTED_PAGES_SIZE	251
+	u_int8_t list[SVPD_SUPPORTED_PAGES_SIZE];
+};
+
 struct scsi_vpd_unit_serial_number
 {
 	u_int8_t device;

--------------070500040300070701010809--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?46E615C4.1010605>