Re: [usb-storage] USB storage devices and SAT

[Douglas Gilbert <dougg@torque.net>]

Alan Stern wrote:
> On Tue, 5 Aug 2008, Boaz Harrosh wrote:
> 
>> There was a correct and simple patch proposed that fixes this problem
>> the right way:
>> http://marc.info/?l=linux-usb&m=121762869915609&w=2
>>
>> Doug could you please test this patch to see if it fixes your device?
>>
>> scsi-core already gives drivers complete control on what sense_size to
>> fetch. It is a parameter to the scsi_eh_prep_cmnd() call. So no need
>> for slave_configure(), default value, and all that loop.
> 
>>> http://thread.gmane.org/gmane.linux.utilities.smartmontools/5721
>>> Index: linux-2.6/drivers/usb/storage/scsiglue.c
>>> ===================================================================
>>> --- linux-2.6.orig/drivers/usb/storage/scsiglue.c	2008-08-02 00:07:24.000000000 +0200
>>> +++ linux-2.6/drivers/usb/storage/scsiglue.c	2008-08-02 00:07:37.000000000 +0200
>>> @@ -170,6 +170,10 @@
>>>  		if (us->fflags & US_FL_CAPACITY_HEURISTICS)
>>>  			sdev->guess_capacity = 1;
>>>  
>>> +		/* assume SPC3 or latter device support sense size different of 18 */
>>> +		if (sdev->scsi_level > SCSI_SPC_2)
>>> +			us->fflags |= US_FL_SANE_SENSE;
>>> +
>>>  		/* Some devices report a SCSI revision level above 2 but are
>>>  		 * unable to handle the REPORT LUNS command (for which
>>>  		 * support is mandatory at level 3).  Since we already have
>>> Index: linux-2.6/drivers/usb/storage/transport.c
>>> ===================================================================
>>> --- linux-2.6.orig/drivers/usb/storage/transport.c	2008-08-02 00:07:24.000000000 \
>>>                 +0200
>>> +++ linux-2.6/drivers/usb/storage/transport.c	2008-08-02 00:07:37.000000000 +0200
>>> @@ -595,10 +595,15 @@
>>>  	if (need_auto_sense) {
>>>  		int temp_result;
>>>  		struct scsi_eh_save ses;
>>> +		int sense_size = US_SENSE_SIZE;
>>> +
>>> +		/* device support and need bigger sense buffer */
>>> +		if (us->fflags & US_FL_SANE_SENSE)
>>> +			sense_size = ~0;
> 
> This looks highly suspicious at best.  Shouldn't sense_size be set to a
> real value, like 22 or 255?

The "correct" maximum value (SPC-3 and draft SPC-4) is 252.
Since SPC-3 the recommended maximum length for the basic
SCSI commands that have a 1 byte allocation length field was
altered from 255 to 252. This is to be a little friendlier
to transports that move data in 4 byte units across their
transport. Guessing a bit here but SATA, SAS and FCP fall
into that group of transports.

At some stage the allocation field length of an INQUIRY
command was changed from 1 to 2 bytes. So to pick up
VPD pages on older devices an INQUIRY's maximum allocation
length of 252 may be prudent. For example, choosing a value
of 260 (0x104) may give a surprising result if the device
only expects a 1 byte allocation length field.

>> I recommend this patch. It does exactly what's needed with minimum risk
>> it is even maybe over protective, with the white list. Perhaps it should
>> be turned to a black list instead. The old broken devices been an extincting
>> exception.
> 
> Changing it to a blacklist would be bad -- in fact it would be a 
> regression, because all the deficient devices which used to work would 
> stop working until they were identified and added to the blacklist.
> 
> Apart from the one nit above, I agree that this patch looks sensible.

Boaz,
I didn't test the above patch (as I don't have the external
USB devices that need it) but a gentleman who did, tells me
that he used a very similar patch and it solved his problem.

Doug Gilbert