On Sun, 2003-06-22 at 14:56, Matthew Dharm wrote:
> > But (as I read it -- remember I'm not an expert), the old sr.c code didn't
> > set the DBD bit, just like the new code. So whatever formula applied to
> > the old code should apply to the new code, yes?
Hmm, the diff I sent was an older one which had the dbd meaning
inverted. The attached is the one I'm actually testing.
> The code James sent sets DBD to 0 -- I like that, as many usb-storage
> devices choke when DBD is set to 1. I believe in avoiding the DBD bit as
> much as possible, and James seems to have eliminated it.
>
> However, DBD==0 means that a block descriptor is likely to be returned --
> so we need to add in the size of the block descriptor header.
OK, now I'm confused. the write caching determination code that you and
Andries spent a while getting to work has DBD==1, and I thought we'd
eliminated all the USB problems with that code...
Since the BD skip code is quite a wodge of complexity, I'd like to know
what devices fail on DBD==1 before trying to add it all in, especially
as we (fortunatly) have nothing in the kernel that actually wants to see
the BDs.
James
===== drivers/scsi/sd.c 1.123 vs edited =====
--- 1.123/drivers/scsi/sd.c Tue Jun 17 04:17:15 2003
+++ edited/drivers/scsi/sd.c Sun Jun 22 09:21:16 2003
@@ -1062,40 +1062,12 @@
}
/* called with buffer of length 512 */
-static int
+static inline int
sd_do_mode_sense(struct scsi_request *SRpnt, int dbd, int modepage,
- unsigned char *buffer, int len) {
- unsigned char cmd[12];
-
- memset((void *) &cmd[0], 0, 12);
- cmd[1] = dbd;
- cmd[2] = modepage;
-
- if (SRpnt->sr_device->use_10_for_ms) {
- if (len < 8)
- len = 8;
-
- cmd[0] = MODE_SENSE_10;
- cmd[8] = len;
- } else {
- if (len < 4)
- len = 4;
-
- cmd[0] = MODE_SENSE;
- cmd[4] = len;
- }
-
- SRpnt->sr_cmd_len = 0;
- SRpnt->sr_sense_buffer[0] = 0;
- SRpnt->sr_sense_buffer[2] = 0;
- SRpnt->sr_data_direction = SCSI_DATA_READ;
-
- memset((void *) buffer, 0, len);
-
- scsi_wait_req(SRpnt, (void *) cmd, (void *) buffer,
- len, SD_TIMEOUT, SD_MAX_RETRIES);
-
- return SRpnt->sr_result;
+ unsigned char *buffer, int len)
+{
+ return scsi_do_mode_sense(SRpnt, dbd, modepage, buffer, len,
+ SD_TIMEOUT, SD_MAX_RETRIES);
}
/*
@@ -1119,20 +1091,20 @@
* When only page 0 is implemented, a request for page 3F may return
* Sense Key 5: Illegal Request, Sense Code 24: Invalid field in CDB.
*/
- if (res)
+ if (res == 0)
res = sd_do_mode_sense(SRpnt, 0, 0, buffer, 4);
/*
* Third attempt: ask 255 bytes, as we did earlier.
*/
- if (res)
+ if (res == 0)
res = sd_do_mode_sense(SRpnt, 0, 0x3F, buffer, 255);
- if (res) {
+ if (res == 0) {
printk(KERN_WARNING
"%s: test WP failed, assume Write Enabled\n", diskname);
} else {
- sdkp->write_prot = ((buffer[2] & 0x80) != 0);
+ sdkp->write_prot = ((buffer[res == 4 ? 2 : 3] & 0x80) != 0);
printk(KERN_NOTICE "%s: Write Protect is %s\n", diskname,
sdkp->write_prot ? "on" : "off");
printk(KERN_DEBUG "%s: Mode Sense: %02x %02x %02x %02x\n",
@@ -1155,37 +1127,37 @@
/* cautiously ask */
res = sd_do_mode_sense(SRpnt, dbd, modepage, buffer, 4);
- if (res == 0) {
+ if (res != 0) {
/* that went OK, now ask for the proper length */
- len = buffer[0] + 1;
+ if(res == 4)
+ len = buffer[0] + 1;
+ else
+ len = buffer[0]*256 + buffer[1] + 2;
if (len > 128)
len = 128;
res = sd_do_mode_sense(SRpnt, dbd, modepage, buffer, len);
}
- if (res == 0 && buffer[3] + 6 < len) {
+ if (res != 0) {
const char *types[] = {
"write through", "none", "write back",
"write back, no read (daft)"
};
int ct = 0;
- int offset = buffer[3] + 4; /* start of mode page */
- sdkp->WCE = ((buffer[offset + 2] & 0x04) != 0);
- sdkp->RCD = ((buffer[offset + 2] & 0x01) != 0);
+ sdkp->WCE = ((buffer[res + 2] & 0x04) != 0);
+ sdkp->RCD = ((buffer[res + 2] & 0x01) != 0);
ct = sdkp->RCD + 2*sdkp->WCE;
printk(KERN_NOTICE "SCSI device %s: drive cache: %s\n",
diskname, types[ct]);
} else {
- if (res == 0 ||
- (status_byte(res) == CHECK_CONDITION
- && (SRpnt->sr_sense_buffer[0] & 0x70) == 0x70
+ if ((SRpnt->sr_sense_buffer[0] & 0x70) == 0x70
&& (SRpnt->sr_sense_buffer[2] & 0x0f) == ILLEGAL_REQUEST
/* ASC 0x24 ASCQ 0x00: Invalid field in CDB */
&& SRpnt->sr_sense_buffer[12] == 0x24
- && SRpnt->sr_sense_buffer[13] == 0x00)) {
+ && SRpnt->sr_sense_buffer[13] == 0x00) {
printk(KERN_NOTICE "%s: cache data unavailable\n",
diskname);
} else {
===== drivers/scsi/sr.c 1.82 vs edited =====
--- 1.82/drivers/scsi/sr.c Mon Jun 16 19:22:18 2003
+++ edited/drivers/scsi/sr.c Sun Jun 22 09:06:02 2003
@@ -660,7 +660,6 @@
static void get_capabilities(struct scsi_cd *cd)
{
- struct cdrom_generic_command cgc;
unsigned char *buffer;
int rc, n;
@@ -681,18 +680,10 @@
printk(KERN_ERR "sr: out of memory.\n");
return;
}
- memset(&cgc, 0, sizeof(struct cdrom_generic_command));
- cgc.cmd[0] = MODE_SENSE;
- cgc.cmd[2] = 0x2a;
- cgc.cmd[4] = 128;
- cgc.buffer = buffer;
- cgc.buflen = 128;
- cgc.quiet = 1;
- cgc.data_direction = SCSI_DATA_READ;
- cgc.timeout = SR_TIMEOUT;
- rc = sr_do_ioctl(cd, &cgc);
+ rc = scsi_mode_sense(cd->device, 0x08, 0x2a, buffer, 128,
+ SR_TIMEOUT, 3);
- if (rc) {
+ if (rc == 0) {
/* failed, drive doesn't have capabilities mode page */
cd->cdi.speed = 1;
cd->cdi.mask |= (CDC_CD_R | CDC_CD_RW | CDC_DVD_R |
@@ -702,7 +693,7 @@
printk("%s: scsi-1 drive\n", cd->cdi.name);
return;
}
- n = buffer[3] + 4;
+ n = rc;
cd->cdi.speed = ((buffer[n + 8] << 8) + buffer[n + 9]) / 176;
cd->readcd_known = 1;
cd->readcd_cdda = buffer[n + 5] & 0x01;
===== drivers/scsi/scsi_lib.c 1.95 vs edited =====
--- 1.95/drivers/scsi/scsi_lib.c Wed Jun 4 06:43:01 2003
+++ edited/drivers/scsi/scsi_lib.c Sun Jun 22 15:26:15 2003
@@ -1336,3 +1336,105 @@
kmem_cache_destroy(sgp->slab);
}
}
+/** scsi_do_mode_sense - issue a mode sense, falling back from 10 to
+ * six bytes if necessary.
+ * @SRpnt: SCSI request to fill in with the MODE_SENSE
+ * @dbd: set if mode sense will allow block descriptors to be returned
+ * @modepage: mode page being requested
+ * @buffer: request buffer (may not be smaller than eight bytes)
+ * @len: length of request buffer.
+ * @timeout: command timeout
+ * @retries: number of retries before failing
+ *
+ * Returns zero if unsuccessful, or the header offset (either 4
+ * or 8 depending on whether a six or ten byte command was
+ * issued) if successful.
+ **/
+int
+scsi_do_mode_sense(struct scsi_request *SRpnt, int dbd, int modepage,
+ unsigned char *buffer, int len, int timeout, int retries) {
+ unsigned char cmd[12];
+ int use_10_for_ms;
+ int header_offset;
+
+ memset((void *) &cmd[0], 0, 12);
+ cmd[1] = dbd ? 0x08: 0;
+ cmd[2] = modepage;
+
+ retry:
+ use_10_for_ms = SRpnt->sr_device->use_10_for_ms;
+
+ if (use_10_for_ms) {
+ if (len < 8)
+ len = 8;
+
+ cmd[0] = MODE_SENSE_10;
+ cmd[8] = len;
+ header_offset = 8;
+ } else {
+ if (len < 4)
+ len = 4;
+
+ cmd[0] = MODE_SENSE;
+ cmd[4] = len;
+ header_offset = 4;
+ }
+
+ SRpnt->sr_cmd_len = 0;
+ SRpnt->sr_sense_buffer[0] = 0;
+ SRpnt->sr_sense_buffer[2] = 0;
+ SRpnt->sr_data_direction = SCSI_DATA_READ;
+
+ memset((void *) buffer, 0, len);
+
+ scsi_wait_req(SRpnt, (void *) cmd, (void *) buffer,
+ len, timeout, retries);
+
+ /* This code looks awful: what it's doing is making sure an
+ * ILLEGAL REQUEST sense return identifies the actual command
+ * byte as the problem. MODE_SENSE commands can return
+ * ILLEGAL REQUEST if the code page isn't supported */
+ if (use_10_for_ms && ! scsi_status_is_good(SRpnt->sr_result) &&
+ (driver_byte(SRpnt->sr_result) & DRIVER_SENSE) &&
+ SRpnt->sr_sense_buffer[2] == ILLEGAL_REQUEST &&
+ (SRpnt->sr_sense_buffer[4] & 0x40) == 0x40 &&
+ SRpnt->sr_sense_buffer[5] == 0 &&
+ SRpnt->sr_sense_buffer[6] == 0 ) {
+ SRpnt->sr_device->use_10_for_ms = 0;
+ goto retry;
+ }
+
+ return scsi_status_is_good(SRpnt->sr_result) ? header_offset : 0;
+}
+
+/** scsi_mode_sense - issue a mode sense, falling back from 10 to
+ * six bytes if necessary.
+ * @sdev: scsi device to send command to.
+ * @dbd: set if mode sense will allow block descriptors to be returned
+ * @modepage: mode page being requested
+ * @buffer: request buffer (may not be smaller than eight bytes)
+ * @len: length of request buffer.
+ * @timeout: command timeout
+ * @retries: number of retries before failing
+ *
+ * Returns zero if unsuccessful, or the header offset (either 4
+ * or 8 depending on whether a six or ten byte command was
+ * issued) if successful.
+ **/
+int
+scsi_mode_sense(struct scsi_device *sdev, int dbd, int modepage,
+ unsigned char *buffer, int len, int timeout, int retries)
+{
+ struct scsi_request *sreq = scsi_allocate_request(sdev);
+ int ret;
+
+ if (!sreq)
+ return 0;
+
+ ret = scsi_do_mode_sense(sreq, dbd, modepage, buffer, len,
+ timeout, retries);
+
+ scsi_release_request(sreq);
+
+ return ret;
+}
===== drivers/scsi/scsi_syms.c 1.40 vs edited =====
--- 1.40/drivers/scsi/scsi_syms.c Thu Jun 5 06:26:04 2003
+++ edited/drivers/scsi/scsi_syms.c Sat Jun 21 20:52:12 2003
@@ -86,6 +86,9 @@
EXPORT_SYMBOL(scsi_remove_device);
EXPORT_SYMBOL(scsi_set_device_offline);
+EXPORT_SYMBOL(scsi_do_mode_sense);
+EXPORT_SYMBOL(scsi_mode_sense);
+
/*
* This symbol is for the highlevel drivers (e.g. sg) only.
*/
