On Jan 9, 2012, at 7:29 PM, Perry Wagle wrote:
>
> On Jan 6, 2012, at 7:29 AM, Alan Stern wrote:
>
>> On Thu, 5 Jan 2012, Alan Stern wrote:
>>
>>> Sure, that ought to work. I'll update the patch.
>>>
>>>> The flip side is to go forward with your patch as planned, and if the
>>>> VPD issue becomes a problem, then add in a separate "don't get VPD
>>>> pages" flag later. Unless this VPD data is *really* important, I
>>>> would be inclined to not issue the command. A userspace tool could
>>>> always issue the command if the user wanted the info, and thus they
>>>> accept the responsibility for killing their device (until reset).
>>>
>>> I don't know how important that information is. It might matter for
>>> things like releasing regions on SSDs. If necessary, we can always add
>>> a new flag like you suggest.
>>
>> Here's the updated patch. I decided to add a separate flag for the VPD
>> pages now; that way we can easily change the logic for setting that
>> flag in usb-storage. Together the two new flags should cover all the
>> important things that forcing the level to SCSI_2 did.
>>
>> (Except for one thing: User programs that look at the ANSI SCSI
>> revision field in /proc/scsi/scsi will now see the correct value. I
>> doubt that any programs rely on that, however -- it's not easy to do --
>> they are much more likely to issue their own INQUIRY command to get the
>> value directly.)
>>
>> When Perry verifies that this still works, I'll split it up into about
>> three separate patches and submit it via the SCSI tree, without marking
>> it for -stable.
>
> When do you think you will be able to split it into three pieces?
>
> One of my problems (my last!) is that I'm wanting to backport it to 2.6.31 for
> an embedded ARM NAS device, and the USB_SC_UFI definition (and all its
> semantics) isn't defined way back then. I'm not sure what to include and what
Ok, it was US_SC_UFI back then. I discovered another cool way to use gitk.
Seems to compile and make sense to me at least now.
> not to in the below patch. This is your third proposed patch, the second seems
> to work fine, but this one doesn't compile due to the missing definition from 2010.
> Unfortunately, I'm stuck back in 2008 with this custom-optimized 2.6.31. Sigh.
>
> What do I need to figure out? Seems like I need to excise the part that worries
> about the floppy (USB_SC_UFI), but maybe I should leave in the VPD stuff?
>
> What's going to bite me, most likely?
>
>> Alan Stern
>
> Thanks!
>
> Perry Wagle
>
>>
>>
>>
>> Index: usb-3.2/include/scsi/scsi_device.h
>> ===================================================================
>> --- usb-3.2.orig/include/scsi/scsi_device.h
>> +++ usb-3.2/include/scsi/scsi_device.h
>> @@ -136,6 +136,7 @@ struct scsi_device {
>> unsigned use_10_for_ms:1; /* first try 10-byte mode sense/select */
>> unsigned skip_ms_page_8:1; /* do not use MODE SENSE page 0x08 */
>> unsigned skip_ms_page_3f:1; /* do not use MODE SENSE page 0x3f */
>> + unsigned skip_vpd_pages:1; /* do not read VPD pages */
>> unsigned use_192_bytes_for_3f:1; /* ask for 192 bytes from page 0x3f */
>> unsigned no_start_on_add:1; /* do not issue start on add */
>> unsigned allow_restart:1; /* issue START_UNIT in error handler */
>> @@ -247,8 +248,10 @@ struct scsi_target {
>> unsigned int single_lun:1; /* Indicates we should only
>> * allow I/O to one of the luns
>> * for the device at a time. */
>> - unsigned int pdt_1f_for_no_lun; /* PDT = 0x1f */
>> - /* means no lun present */
>> + unsigned int pdt_1f_for_no_lun:1; /* PDT = 0x1f
>> + * means no lun present. */
>> + unsigned int no_report_luns:1; /* Don't use
>> + * REPORT LUNS for scanning. */
>> /* commands actually active on LLD. protected by host lock. */
>> unsigned int target_busy;
>> /*
>> Index: usb-3.2/drivers/scsi/scsi_scan.c
>> ===================================================================
>> --- usb-3.2.orig/drivers/scsi/scsi_scan.c
>> +++ usb-3.2/drivers/scsi/scsi_scan.c
>> @@ -1295,6 +1295,7 @@ EXPORT_SYMBOL(int_to_scsilun);
>> * LUNs even if it's older than SCSI-3.
>> * If BLIST_NOREPORTLUN is set, return 1 always.
>> * If BLIST_NOLUN is set, return 0 always.
>> + * If starget->no_report_luns is set, return 1 always.
>> *
>> * Return:
>> * 0: scan completed (or no memory, so further scanning is futile)
>> @@ -1321,6 +1322,7 @@ static int scsi_report_lun_scan(struct s
>> * Only support SCSI-3 and up devices if BLIST_NOREPORTLUN is not set.
>> * Also allow SCSI-2 if BLIST_REPORTLUN2 is set and host adapter does
>> * support more than 8 LUNs.
>> + * Don't attempt if the target doesn't support REPORT LUNS.
>> */
>> if (bflags & BLIST_NOREPORTLUN)
>> return 1;
>> @@ -1332,6 +1334,8 @@ static int scsi_report_lun_scan(struct s
>> return 1;
>> if (bflags & BLIST_NOLUN)
>> return 0;
>> + if (starget->no_report_luns)
>> + return 1;
>>
>> if (!(sdev = scsi_device_lookup_by_target(starget, 0))) {
>> sdev = scsi_alloc_sdev(starget, 0, NULL);
>> Index: usb-3.2/drivers/scsi/sd.c
>> ===================================================================
>> --- usb-3.2.orig/drivers/scsi/sd.c
>> +++ usb-3.2/drivers/scsi/sd.c
>> @@ -2341,7 +2341,7 @@ static int sd_try_extended_inquiry(struc
>> * some USB ones crash on receiving them, and the pages
>> * we currently ask for are for SPC-3 and beyond
>> */
>> - if (sdp->scsi_level > SCSI_SPC_2)
>> + if (sdp->scsi_level > SCSI_SPC_2 && !sdp->skip_vpd_pages)
>> return 1;
>> return 0;
>> }
>> Index: usb-3.2/drivers/usb/storage/scsiglue.c
>> ===================================================================
>> --- usb-3.2.orig/drivers/usb/storage/scsiglue.c
>> +++ usb-3.2/drivers/usb/storage/scsiglue.c
>> @@ -78,8 +78,6 @@ static const char* host_info(struct Scsi
>>
>> static int slave_alloc (struct scsi_device *sdev)
>> {
>> - struct us_data *us = host_to_us(sdev->host);
>> -
>> /*
>> * Set the INQUIRY transfer length to 36. We don't use any of
>> * the extra data and many devices choke if asked for more or
>> @@ -104,18 +102,6 @@ static int slave_alloc (struct scsi_devi
>> */
>> blk_queue_update_dma_alignment(sdev->request_queue, (512 - 1));
>>
>> - /*
>> - * The UFI spec treates the Peripheral Qualifier bits in an
>> - * INQUIRY result as reserved and requires devices to set them
>> - * to 0. However the SCSI spec requires these bits to be set
>> - * to 3 to indicate when a LUN is not present.
>> - *
>> - * Let the scanning code know if this target merely sets
>> - * Peripheral Device Type to 0x1f to indicate no LUN.
>> - */
>> - if (us->subclass == USB_SC_UFI)
>> - sdev->sdev_target->pdt_1f_for_no_lun = 1;
>> -
>> return 0;
>> }
>>
>> @@ -197,6 +183,9 @@ static int slave_configure(struct scsi_d
>> * page x08, so we will skip it. */
>> sdev->skip_ms_page_8 = 1;
>>
>> + /* Some devices don't handle VPD pages correctly */
>> + sdev->skip_vpd_pages = 1;
>> +
>> /* Some disks return the total number of blocks in response
>> * to READ CAPACITY rather than the highest block number.
>> * If this device makes that mistake, tell the sd driver. */
>> @@ -217,16 +206,6 @@ static int slave_configure(struct scsi_d
>> 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
>> - * a Get-Max-LUN request, we won't lose much by setting the
>> - * revision level down to 2. The only devices that would be
>> - * affected are those with sparse LUNs. */
>> - if (sdev->scsi_level > SCSI_2)
>> - sdev->sdev_target->scsi_level =
>> - sdev->scsi_level = SCSI_2;
>> -
>> /* USB-IDE bridges tend to report SK = 0x04 (Non-recoverable
>> * Hardware Error) when any low-level error occurs,
>> * recoverable or not. Setting this flag tells the SCSI
>> @@ -283,6 +262,33 @@ static int slave_configure(struct scsi_d
>> return 0;
>> }
>>
>> +static int target_alloc(struct scsi_target *starget)
>> +{
>> + struct us_data *us = host_to_us(dev_to_shost(starget->dev.parent));
>> +
>> + /*
>> + * Some USB drives don't support REPORT LUNS, even though they
>> + * report a SCSI revision level above 2. Tell the SCSI layer
>> + * not to issue that command; it will perform a normal sequential
>> + * scan instead.
>> + */
>> + starget->no_report_luns = 1;
>> +
>> + /*
>> + * The UFI spec treats the Peripheral Qualifier bits in an
>> + * INQUIRY result as reserved and requires devices to set them
>> + * to 0. However the SCSI spec requires these bits to be set
>> + * to 3 to indicate when a LUN is not present.
>> + *
>> + * Let the scanning code know if this target merely sets
>> + * Peripheral Device Type to 0x1f to indicate no LUN.
>> + */
>> + if (us->subclass == USB_SC_UFI)
>> + starget->pdt_1f_for_no_lun = 1;
>> +
>> + return 0;
>> +}
>> +
>> /* queue a command */
>> /* This is always called with scsi_lock(host) held */
>> static int queuecommand_lck(struct scsi_cmnd *srb,
>> @@ -546,6 +552,7 @@ struct scsi_host_template usb_stor_host_
>>
>> .slave_alloc = slave_alloc,
>> .slave_configure = slave_configure,
>> + .target_alloc = target_alloc,
>>
>> /* lots of sg segments can be handled */
>> .sg_tablesize = SCSI_MAX_SG_CHAIN_SEGMENTS,
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
