* PATCH: (as284c) Add BLIST_INQUIRY_36 to all USB blacklist entries
@ 2004-08-26 15:53 Alan Stern
2004-08-26 19:11 ` Christoph Hellwig
0 siblings, 1 reply; 6+ messages in thread
From: Alan Stern @ 2004-08-26 15:53 UTC (permalink / raw)
To: James Bottomley; +Cc: SCSI development list
James:
I submitted this patch earlier but received no response.
This patch adds the BLIST_INQUIRY_36 flag to all the SCSI blacklist
entries for USB devices. While it may not be strictly necessary for all
of them, it doesn't hurt: Since the usb-storage driver doesn't use any of
the INQUIRY data after the first 36 bytes, there's no reason to try
reading any more of it. And some devices crash when we try to read more,
even though they advertise that more bytes are available. The usb-storage
driver does try to set the flag automatically, but the blacklist entries
override that setting.
Please apply.
Alan Stern
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
===== drivers/scsi/scsi_devinfo.c 1.11 vs edited =====
--- 1.11/drivers/scsi/scsi_devinfo.c Thu Jul 1 07:54:11 2004
+++ edited/drivers/scsi/scsi_devinfo.c Thu Aug 26 11:51:19 2004
@@ -115,13 +115,14 @@
/*
* Other types of devices that have special flags.
+ * Note that all USB devices should have the BLIST_INQUIRY_36 flag.
*/
{"ADAPTEC", "AACRAID", NULL, BLIST_FORCELUN},
{"ADAPTEC", "Adaptec 5400S", NULL, BLIST_FORCELUN},
{"AFT PRO", "-IX CF", "0.0>", BLIST_FORCELUN},
- {"BELKIN", "USB 2 HS-CF", "1.95", BLIST_FORCELUN},
+ {"BELKIN", "USB 2 HS-CF", "1.95", BLIST_FORCELUN | BLIST_INQUIRY_36},
{"CANON", "IPUBJD", NULL, BLIST_SPARSELUN},
- {"CBOX3", "USB Storage-SMC", "300A", BLIST_FORCELUN},
+ {"CBOX3", "USB Storage-SMC", "300A", BLIST_FORCELUN | BLIST_INQUIRY_36},
{"CMD", "CRA-7280", NULL, BLIST_SPARSELUN}, /* CMD RAID Controller */
{"CNSI", "G7324", NULL, BLIST_SPARSELUN}, /* Chaparral G7324 RAID */
{"CNSi", "G8324", NULL, BLIST_SPARSELUN}, /* Chaparral G8324 RAID */
@@ -142,9 +143,9 @@
{"EMC", "SYMMETRIX", NULL, BLIST_SPARSELUN | BLIST_LARGELUN | BLIST_FORCELUN},
{"EMULEX", "MD21/S2 ESDI", NULL, BLIST_SINGLELUN},
{"FSC", "CentricStor", "*", BLIST_SPARSELUN | BLIST_LARGELUN},
- {"Generic", "USB SD Reader", "1.00", BLIST_FORCELUN},
- {"Generic", "USB Storage-SMC", "0180", BLIST_FORCELUN},
- {"Generic", "USB Storage-SMC", "0207", BLIST_FORCELUN},
+ {"Generic", "USB SD Reader", "1.00", BLIST_FORCELUN | BLIST_INQUIRY_36},
+ {"Generic", "USB Storage-SMC", "0180", BLIST_FORCELUN | BLIST_INQUIRY_36},
+ {"Generic", "USB Storage-SMC", "0207", BLIST_FORCELUN | BLIST_INQUIRY_36},
{"HITACHI", "DF400", "*", BLIST_SPARSELUN},
{"HITACHI", "DF500", "*", BLIST_SPARSELUN},
{"HITACHI", "DF600", "*", BLIST_SPARSELUN},
@@ -182,7 +183,7 @@
{"SGI", "RAID3", "*", BLIST_SPARSELUN},
{"SGI", "RAID5", "*", BLIST_SPARSELUN},
{"SGI", "TP9100", "*", BLIST_REPORTLUN2},
- {"SMSC", "USB 2 HS-CF", NULL, BLIST_SPARSELUN},
+ {"SMSC", "USB 2 HS-CF", NULL, BLIST_SPARSELUN | BLIST_INQUIRY_36},
{"SONY", "CD-ROM CDU-8001", NULL, BLIST_BORKEN},
{"SONY", "TSL", NULL, BLIST_FORCELUN}, /* DDS3 & DDS4 autoloaders */
{"SUN", "T300", "*", BLIST_SPARSELUN},
@@ -190,7 +191,7 @@
{"TEXEL", "CD-ROM", "1.06", BLIST_BORKEN},
{"TOSHIBA", "CDROM", NULL, BLIST_ISROM},
{"TOSHIBA", "CD-ROM", NULL, BLIST_ISROM},
- {"USB2.0", "SMARTMEDIA/XD", NULL, BLIST_FORCELUN},
+ {"USB2.0", "SMARTMEDIA/XD", NULL, BLIST_FORCELUN | BLIST_INQUIRY_36},
{"XYRATEX", "RS", "*", BLIST_SPARSELUN | BLIST_LARGELUN},
{"Zzyzx", "RocketStor 500S", NULL, BLIST_SPARSELUN},
{"Zzyzx", "RocketStor 2000", NULL, BLIST_SPARSELUN},
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: PATCH: (as284c) Add BLIST_INQUIRY_36 to all USB blacklist entries
2004-08-26 15:53 PATCH: (as284c) Add BLIST_INQUIRY_36 to all USB blacklist entries Alan Stern
@ 2004-08-26 19:11 ` Christoph Hellwig
2004-08-26 19:27 ` Alan Stern
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Christoph Hellwig @ 2004-08-26 19:11 UTC (permalink / raw)
To: Alan Stern; +Cc: James Bottomley, SCSI development list
On Thu, Aug 26, 2004 at 11:53:45AM -0400, Alan Stern wrote:
> James:
>
> I submitted this patch earlier but received no response.
>
> This patch adds the BLIST_INQUIRY_36 flag to all the SCSI blacklist
Maybe instead of adding gazillions of blacklists for different inquiry
lengthes we should just add an max_inquiry_length field to scsi_device
that can be set in slave_alloc?
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: PATCH: (as284c) Add BLIST_INQUIRY_36 to all USB blacklist entries
2004-08-26 19:11 ` Christoph Hellwig
@ 2004-08-26 19:27 ` Alan Stern
2004-09-07 20:47 ` PATCH: (as373) Let LLD specify INQUIRY length Alan Stern
2004-10-15 18:30 ` Alan Stern
2 siblings, 0 replies; 6+ messages in thread
From: Alan Stern @ 2004-08-26 19:27 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: James Bottomley, SCSI development list
On Thu, 26 Aug 2004, Christoph Hellwig wrote:
> On Thu, Aug 26, 2004 at 11:53:45AM -0400, Alan Stern wrote:
> > James:
> >
> > I submitted this patch earlier but received no response.
> >
> > This patch adds the BLIST_INQUIRY_36 flag to all the SCSI blacklist
>
> Maybe instead of adding gazillions of blacklists for different inquiry
> lengthes we should just add an max_inquiry_length field to scsi_device
> that can be set in slave_alloc?
That sounds good to me. The usb-storage driver sets BLIST_INQUIRY_36 in
slave_alloc(), but an existing blacklist entry will override it.
Alan Stern
^ permalink raw reply [flat|nested] 6+ messages in thread
* PATCH: (as373) Let LLD specify INQUIRY length
2004-08-26 19:11 ` Christoph Hellwig
2004-08-26 19:27 ` Alan Stern
@ 2004-09-07 20:47 ` Alan Stern
2004-10-15 18:30 ` Alan Stern
2 siblings, 0 replies; 6+ messages in thread
From: Alan Stern @ 2004-09-07 20:47 UTC (permalink / raw)
To: James Bottomley; +Cc: Christoph Hellwig, SCSI development list
On Thu, 26 Aug 2004, Christoph Hellwig wrote:
> Maybe instead of adding gazillions of blacklists for different inquiry
> lengthes we should just add an max_inquiry_length field to scsi_device
> that can be set in slave_alloc?
That sounds like a good suggestion. Even better, instead of adding a new
field we can simply use the existing inquiry_length.
This patch changes scsi_probe_lun() to use the value in
sdev->inquiry_length for the first INQUIRY attempt, if that value is
nonzero. Subsequent attempts are based, as before, on the blacklist flags
and the Additional Length field in the INQUIRY data.
The patch also contains a fairly extensive reorganization of the
subroutine. All the code that was duplicated for sending the INQUIRY
command twice has been consolidated. The routine now makes up to three
passes:
In the first pass, the transfer length is the value initially
found in sdev->inquiry_length if that has been set, otherwise
it is the current conservative 36 bytes.
If the first pass succeeds, the routine retrieves the blist flags
for the device and checks the Additional Length field. The blist
flags take precedence over sdev->inquiry_length, which in turn
takes precedence over the Additional Length. If it turns out
there is more data available than we transferred the first time,
a second pass tries to get it.
If the second pass succeeds the INQUIRY data may have changed,
so the blist flags are looked up again and the Additional Length
is checked again. If not, a third pass tries to get the data
back, using the same transfer length as the first pass.
Finally, the value stored in sdev->inquiry_length is set to the amount
actually transferred or the size computed from the Additional Length,
whichever is smaller.
Although the net change in the source file size is small, the new routine
has more comments and less code. Overall I think it's an improvement.
Alan Stern
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
===== drivers/scsi/scsi_scan.c 1.62 vs edited =====
--- 1.62/drivers/scsi/scsi_scan.c Mon Jun 28 10:44:56 2004
+++ edited/drivers/scsi/scsi_scan.c Tue Sep 7 15:38:10 2004
@@ -346,104 +346,113 @@
{
struct scsi_device *sdev = sreq->sr_device; /* a bit ugly */
unsigned char scsi_cmd[MAX_COMMAND_SIZE];
- int possible_inq_resp_len;
- int count = 0;
+ int first_inquiry_len, try_inquiry_len, next_inquiry_len;
+ int response_len = 0;
+ int pass, count;
*bflags = 0;
- repeat_inquiry:
- SCSI_LOG_SCAN_BUS(3, printk(KERN_INFO "scsi scan: INQUIRY to host %d"
- " channel %d id %d lun %d\n", sdev->host->host_no,
- sdev->channel, sdev->id, sdev->lun));
-
- memset(scsi_cmd, 0, 6);
- scsi_cmd[0] = INQUIRY;
- scsi_cmd[4] = 36; /* issue conservative alloc_length */
- sreq->sr_cmd_len = 0;
- sreq->sr_data_direction = DMA_FROM_DEVICE;
-
- memset(inq_result, 0, 36);
- scsi_wait_req(sreq, (void *) scsi_cmd, (void *) inq_result, 36,
- HZ/2 + HZ*scsi_inq_timeout, 3);
-
- SCSI_LOG_SCAN_BUS(3, printk(KERN_INFO "scsi scan: 1st INQUIRY %s with"
- " code 0x%x\n", sreq->sr_result ?
- "failed" : "successful", sreq->sr_result));
- ++count;
-
- if (sreq->sr_result) {
- if ((driver_byte(sreq->sr_result) & DRIVER_SENSE) != 0 &&
- (sreq->sr_sense_buffer[2] & 0xf) == UNIT_ATTENTION &&
- (sreq->sr_sense_buffer[12] == 0x28 ||
- sreq->sr_sense_buffer[12] == 0x29) &&
- sreq->sr_sense_buffer[13] == 0) {
- /* not-ready to ready transition or power-on - good */
- /* dpg: bogus? INQUIRY never returns UNIT_ATTENTION */
- /* Supposedly, but many buggy devices do so anyway */
- if (count < 3)
- goto repeat_inquiry;
- }
- /*
- * assume no peripheral if any other sort of error
- */
- return;
- }
- /*
- * Get any flags for this device.
- *
- * XXX add a bflags to Scsi_Device, and replace the corresponding
- * bit fields in Scsi_Device, so bflags need not be passed as an
- * argument.
- */
- *bflags |= scsi_get_device_flags(sdev, &inq_result[8], &inq_result[16]);
-
- possible_inq_resp_len = (unsigned char) inq_result[4] + 5;
- if (BLIST_INQUIRY_36 & *bflags)
- possible_inq_resp_len = 36;
- else if (BLIST_INQUIRY_58 & *bflags)
- possible_inq_resp_len = 58;
- else if (possible_inq_resp_len > 255)
- possible_inq_resp_len = 36; /* sanity */
+ /* Perform up to 3 passes. The first pass uses a conservative
+ * transfer length of 36 unless sdev->inquiry_len specifies a
+ * different value. */
+ first_inquiry_len = sdev->inquiry_len ? sdev->inquiry_len : 36;
+ try_inquiry_len = first_inquiry_len;
+ pass = 1;
+
+ next_pass:
+ SCSI_LOG_SCAN_BUS(3, printk(KERN_INFO "scsi scan: INQUIRY pass %d "
+ "to host %d channel %d id %d lun %d, length %d\n",
+ pass, sdev->host->host_no, sdev->channel,
+ sdev->id, sdev->lun, try_inquiry_len));
- if (possible_inq_resp_len > 36) { /* do additional INQUIRY */
+ /* Each pass gets up to three chances to ignore Unit Attention */
+ for (count = 0; count < 3; ++count) {
memset(scsi_cmd, 0, 6);
scsi_cmd[0] = INQUIRY;
- scsi_cmd[4] = (unsigned char) possible_inq_resp_len;
+ scsi_cmd[4] = (unsigned char) try_inquiry_len;
sreq->sr_cmd_len = 0;
sreq->sr_data_direction = DMA_FROM_DEVICE;
- /*
- * re-zero inq_result just to be safe.
- */
- memset(inq_result, 0, possible_inq_resp_len);
- scsi_wait_req(sreq, (void *) scsi_cmd,
- (void *) inq_result,
- possible_inq_resp_len, (1+scsi_inq_timeout)*(HZ/2), 3);
- SCSI_LOG_SCAN_BUS(3, printk(KERN_INFO "scsi scan: 2nd INQUIRY"
- " %s with code 0x%x\n", sreq->sr_result ?
- "failed" : "successful", sreq->sr_result));
+
+ memset(inq_result, 0, try_inquiry_len);
+ scsi_wait_req(sreq, (void *) scsi_cmd, (void *) inq_result,
+ try_inquiry_len,
+ HZ/2 + HZ*scsi_inq_timeout, 3);
+
+ SCSI_LOG_SCAN_BUS(3, printk(KERN_INFO "scsi scan: INQUIRY %s "
+ "with code 0x%x\n",
+ sreq->sr_result ? "failed" : "successful",
+ sreq->sr_result));
+
if (sreq->sr_result) {
- /* if the longer inquiry has failed, flag the device
- * as only accepting 36 byte inquiries and retry the
- * 36 byte inquiry */
- printk(KERN_INFO "scsi scan: %d byte inquiry failed"
- " with code %d. Consider BLIST_INQUIRY_36 for"
- " this device\n", possible_inq_resp_len,
- sreq->sr_result);
- *bflags = BLIST_INQUIRY_36;
- goto repeat_inquiry;
+
+ /* not-ready to ready transition or power-on - good */
+ /* dpg: bogus? INQUIRY never returns UNIT_ATTENTION */
+ /* Supposedly, but many buggy devices do so anyway. */
+ if ((driver_byte(sreq->sr_result) & DRIVER_SENSE) &&
+ (sreq->sr_sense_buffer[2] & 0xf) ==
+ UNIT_ATTENTION &&
+ (sreq->sr_sense_buffer[12] == 0x28 ||
+ sreq->sr_sense_buffer[12] == 0x29) &&
+ sreq->sr_sense_buffer[13] == 0)
+ continue;
}
+ break;
+ }
+
+ if (sreq->sr_result == 0) {
+ response_len = (unsigned char) inq_result[4] + 5;
+ if (response_len > 255)
+ response_len = first_inquiry_len; /* sanity */
/*
- * The INQUIRY can change, this means the length can change.
+ * Get any flags for this device.
+ *
+ * XXX add a bflags to Scsi_Device, and replace the
+ * corresponding bit fields in Scsi_Device, so bflags
+ * need not be passed as an argument.
*/
- possible_inq_resp_len = (unsigned char) inq_result[4] + 5;
- if (BLIST_INQUIRY_58 & *bflags)
- possible_inq_resp_len = 58;
- else if (possible_inq_resp_len > 255)
- possible_inq_resp_len = 36; /* sanity */
+ *bflags = scsi_get_device_flags(sdev, &inq_result[8],
+ &inq_result[16]);
+
+ /* When the first pass succeeds we gain information about
+ * what larger transfer lengths might work. */
+ if (pass == 1) {
+ if (BLIST_INQUIRY_36 & *bflags)
+ next_inquiry_len = 36;
+ else if (BLIST_INQUIRY_58 & *bflags)
+ next_inquiry_len = 58;
+ else if (sdev->inquiry_len)
+ next_inquiry_len = sdev->inquiry_len;
+ else
+ next_inquiry_len = response_len;
+
+ /* If more data is available perform the second pass */
+ if (next_inquiry_len > try_inquiry_len) {
+ try_inquiry_len = next_inquiry_len;
+ pass = 2;
+ goto next_pass;
+ }
+ }
+
+ } else if (pass == 2) {
+ printk(KERN_INFO "scsi scan: %d byte inquiry failed. "
+ "Consider BLIST_INQUIRY_36 for this device\n",
+ try_inquiry_len);
+
+ /* If this pass failed, the third pass goes back and transfers
+ * the same amount as we successfully got in the first pass. */
+ try_inquiry_len = first_inquiry_len;
+ pass = 3;
+ goto next_pass;
}
- sdev->inquiry_len = possible_inq_resp_len;
+ /* If the last transfer attempt got an error, assume the
+ * peripheral doesn't exist or is dead. */
+ if (sreq->sr_result)
+ return;
+
+ /* Don't report any more data than the device says is valid */
+ sdev->inquiry_len = min(try_inquiry_len, response_len);
/*
* XXX Abort if the response length is less than 36? If less than
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: PATCH: (as373) Let LLD specify INQUIRY length
2004-08-26 19:11 ` Christoph Hellwig
2004-08-26 19:27 ` Alan Stern
2004-09-07 20:47 ` PATCH: (as373) Let LLD specify INQUIRY length Alan Stern
@ 2004-10-15 18:30 ` Alan Stern
2004-10-15 19:28 ` James Bottomley
2 siblings, 1 reply; 6+ messages in thread
From: Alan Stern @ 2004-10-15 18:30 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: SCSI development list
On Thu, 26 Aug 2004, Christoph Hellwig wrote:
> Maybe instead of adding gazillions of blacklists for different inquiry
> lengthes we should just add an max_inquiry_length field to scsi_device
> that can be set in slave_alloc?
Christoph:
I sent in a patch to do this:
http://marc.theaimsgroup.com/?l=linux-scsi&m=109459028822709&w=2
Nobody has responded to it. Is there some particular person I should send
this to? Or should I change the patch, so that it merely addresses the
issue at hand without simplifying the entire subroutine?
Alan Stern
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: PATCH: (as373) Let LLD specify INQUIRY length
2004-10-15 18:30 ` Alan Stern
@ 2004-10-15 19:28 ` James Bottomley
0 siblings, 0 replies; 6+ messages in thread
From: James Bottomley @ 2004-10-15 19:28 UTC (permalink / raw)
To: Alan Stern; +Cc: Christoph Hellwig, SCSI development list
On Fri, 2004-10-15 at 13:30, Alan Stern wrote:
> Nobody has responded to it. Is there some particular person I should send
> this to? Or should I change the patch, so that it merely addresses the
> issue at hand without simplifying the entire subroutine?
OK, sorry, I have it now ... it was in a reply to a patch that I'd
previously applied, so I thought I already had it.
James
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2004-10-15 19:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-08-26 15:53 PATCH: (as284c) Add BLIST_INQUIRY_36 to all USB blacklist entries Alan Stern
2004-08-26 19:11 ` Christoph Hellwig
2004-08-26 19:27 ` Alan Stern
2004-09-07 20:47 ` PATCH: (as373) Let LLD specify INQUIRY length Alan Stern
2004-10-15 18:30 ` Alan Stern
2004-10-15 19:28 ` James Bottomley
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox