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.
Tried it in Fedora 16. It seems to work, except I get two warnings for HDIO_GET_IDENTITY:
Jan 6 11:04:58 comonad kernel: [ 154.451055] usb 2-6: new high speed USB device number 3 using ehci_hcd
Jan 6 11:04:58 comonad kernel: [ 154.566106] usb 2-6: New USB device found, idVendor=152d, idProduct=0539
Jan 6 11:04:58 comonad kernel: [ 154.566116] usb 2-6: New USB device strings: Mfr=10, Product=11, SerialNumber=5
Jan 6 11:04:58 comonad kernel: [ 154.566124] usb 2-6: Product: USB to ATA/ATAPI Bridge
Jan 6 11:04:58 comonad kernel: [ 154.566130] usb 2-6: Manufacturer: JMicron
Jan 6 11:04:58 comonad kernel: [ 154.566135] usb 2-6: SerialNumber: 000000000000
Jan 6 11:04:58 comonad mtp-probe: checking bus 2, device 3: "/sys/devices/pci0000:00/0000:00:1d.7/usb2/2-6"
Jan 6 11:04:59 comonad mtp-probe: bus: 2, device: 3 was not an MTP device
Jan 6 11:04:59 comonad kernel: [ 154.846105] Initializing USB Mass Storage driver...
Jan 6 11:04:59 comonad kernel: [ 154.846943] scsi6 : usb-storage 2-6:1.0
Jan 6 11:04:59 comonad kernel: [ 154.847189] usbcore: registered new interface driver usb-storage
Jan 6 11:04:59 comonad kernel: [ 154.847192] USB Mass Storage support registered.
Jan 6 11:05:00 comonad kernel: [ 155.848091] scsi 6:0:0:0: Direct-Access WDC WD50 00AAKX-001CA0 0X00 PQ: 0 ANSI: 6
Jan 6 11:05:00 comonad kernel: [ 155.848686] scsi 6:0:0:1: Direct-Access WDC WD50 00AAKX-001CA0 0X00 PQ: 0 ANSI: 6
Jan 6 11:05:00 comonad kernel: [ 155.850750] sd 6:0:0:0: Attached scsi generic sg2 type 0
Jan 6 11:05:00 comonad kernel: [ 155.851131] sd 6:0:0:0: [sdb] 976773168 512-byte logical blocks: (500 GB/465 GiB)
Jan 6 11:05:00 comonad kernel: [ 155.851139] sd 6:0:0:0: [sdb] 4096-byte physical blocks
Jan 6 11:05:00 comonad kernel: [ 155.851525] sd 6:0:0:1: Attached scsi generic sg3 type 0
Jan 6 11:05:00 comonad kernel: [ 155.851872] sd 6:0:0:0: [sdb] Write Protect is off
Jan 6 11:05:00 comonad kernel: [ 155.852995] sd 6:0:0:0: [sdb] No Caching mode page present
Jan 6 11:05:00 comonad kernel: [ 155.853045] sd 6:0:0:0: [sdb] Assuming drive cache: write through
Jan 6 11:05:00 comonad kernel: [ 155.853877] sd 6:0:0:1: [sdc] 976773168 512-byte logical blocks: (500 GB/465 GiB)
Jan 6 11:05:00 comonad kernel: [ 155.853885] sd 6:0:0:1: [sdc] 4096-byte physical blocks
Jan 6 11:05:00 comonad kernel: [ 155.854993] sd 6:0:0:1: [sdc] Write Protect is off
Jan 6 11:05:00 comonad kernel: [ 155.857748] sd 6:0:0:1: [sdc] No Caching mode page present
Jan 6 11:05:00 comonad kernel: [ 155.857756] sd 6:0:0:1: [sdc] Assuming drive cache: write through
Jan 6 11:05:00 comonad kernel: [ 155.859755] sd 6:0:0:0: [sdb] No Caching mode page present
Jan 6 11:05:00 comonad kernel: [ 155.859764] sd 6:0:0:0: [sdb] Assuming drive cache: write through
Jan 6 11:05:00 comonad kernel: [ 155.873873] sd 6:0:0:1: [sdc] No Caching mode page present
Jan 6 11:05:00 comonad kernel: [ 155.873878] sd 6:0:0:1: [sdc] Assuming drive cache: write through
Jan 6 11:05:00 comonad kernel: [ 155.983567] sdb: sdb1
Jan 6 11:05:00 comonad kernel: [ 155.985650] sdc: sdc1
Jan 6 11:05:00 comonad kernel: [ 155.988102] sd 6:0:0:0: [sdb] No Caching mode page present
Jan 6 11:05:00 comonad kernel: [ 155.988106] sd 6:0:0:0: [sdb] Assuming drive cache: write through
Jan 6 11:05:00 comonad kernel: [ 155.988109] sd 6:0:0:0: [sdb] Attached SCSI disk
Jan 6 11:05:00 comonad kernel: [ 155.990395] sd 6:0:0:1: [sdc] No Caching mode page present
Jan 6 11:05:00 comonad kernel: [ 155.990399] sd 6:0:0:1: [sdc] Assuming drive cache: write through
Jan 6 11:05:00 comonad kernel: [ 155.990402] sd 6:0:0:1: [sdc] Attached SCSI disk
Jan 6 11:05:00 comonad ata_id[1980]: HDIO_GET_IDENTITY failed for '/dev/sdb': Invalid argument
Jan 6 11:05:00 comonad ata_id[1981]: HDIO_GET_IDENTITY failed for '/dev/sdc': Invalid argument
Jan 6 11:05:00 comonad kernel: [ 156.347271] EXT4-fs (sdb1): mounting ext3 file system using the ext4 subsystem
Jan 6 11:05:00 comonad kernel: [ 156.376664] EXT4-fs (sdb1): warning: maximal mount count reached, running e2fsck is recommended
Jan 6 11:05:00 comonad kernel: [ 156.378037] EXT4-fs (sdb1): mounted filesystem with ordered data mode. Opts: (null)
Jan 6 11:05:00 comonad kernel: [ 156.410618] EXT4-fs (sdc1): mounting ext3 file system using the ext4 subsystem
Jan 6 11:05:00 comonad kernel: [ 156.527192] EXT4-fs (sdc1): mounted filesystem with ordered data mode. Opts: (null)
>
> Alan Stern
>
>
>
> 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
