Handling erroneous READ CAPACITY response in sd.c

Handling erroneous READ CAPACITY response in sd.c

[Alan Stern]

A number of USB mass storage devices incorrectly return the total number 
of blocks rather than the largest block number in response to READ 
CAPACITY.  They then go on to compound the problem by not returning 
"Logical block address out of range" sense when asked to read the "last" 
block; instead they mess up the protocol and don't send any status phase 
information.

The usb-storage driver has been dealing with this by adding blacklist 
entries for these devices, but this isn't a very good solution.  After 
all, it's not a transport problem -- and it's not a good idea for a 
low-level driver to change the data being sent by a device.

Instead the adjustment should be made in sd.c, in the sd_read_capacity() 
routine.  One possibility is to have a SCSI blacklist flag for the bad 
devices.  A better choice might be to correct the mistake at runtime.  
Using the heuristic that the total number of blocks is almost always even 
(as far as I know it is _always_ even for USB disk-like devices), we could 
try to read the last block whenever READ CAPACITY reports an odd number of 
blocks.  If the read fails then we would know to decrement the number.

Does either of these sound like a good idea?  And if the second choice 
sounds better, is there anyone who could help me to write such a patch?

Alan Stern

Re: Handling erroneous READ CAPACITY response in sd.c

[Luben Tuikov]

[-- Attachment #1: Type: text/plain, Size: 6374 bytes --]

Alan Stern wrote:
> A number of USB mass storage devices incorrectly return the total number
> of blocks rather than the largest block number in response to READ
> CAPACITY.  They then go on to compound the problem by not returning
> "Logical block address out of range" sense when asked to read the "last"
> block; instead they mess up the protocol and don't send any status phase
> information.
> 
> The usb-storage driver has been dealing with this by adding blacklist
> entries for these devices, but this isn't a very good solution.  After
> all, it's not a transport problem -- and it's not a good idea for a
> low-level driver to change the data being sent by a device.
> 
> Instead the adjustment should be made in sd.c, in the sd_read_capacity()
> routine.  One possibility is to have a SCSI blacklist flag for the bad
> devices.  A better choice might be to correct the mistake at runtime. 
> Using the heuristic that the total number of blocks is almost always even
> (as far as I know it is _always_ even for USB disk-like devices), we could
> try to read the last block whenever READ CAPACITY reports an odd number of
> blocks.  If the read fails then we would know to decrement the number.
> 
> Does either of these sound like a good idea?  And if the second choice
> sounds better, is there anyone who could help me to write such a patch?

Alan, do you have such a USB device handy?

A third possibility is to use the PMI bit to get the proper value
of the LBA of the last logical block.

If you want to move this into sd, then do you know if those
devices support the use of PMI bit in READ CAPACITY CDB?
As block devices they should.

See the appended/attached patch.  Can you test it against such a USB
device?

Signed-off-by: Luben Tuikov <luben_tuikov@adaptec.com>

===== drivers/scsi/sd.c 1.161 vs edited =====
--- 1.161/drivers/scsi/sd.c	2004-10-15 10:46:07 -04:00
+++ edited/drivers/scsi/sd.c	2004-10-19 16:51:46 -04:00
@@ -987,6 +987,112 @@
  	}
  }

+/* sd_read_true_cap: Some device servers incorrectly return the
+ * capacity as opposed to the LBA of the last logical block of the
+ * block device.
+ *
+ * We try to fix this as follows: Let x = Returned LBA from the last
+ * READ CAPACITY command issued (result in "buffer").  Reissue the
+ * READ CAPACITY command as follows: set the partial medium indicator
+ * (PMI) bit to one; set the LBA to x - 1. Fire off that READ CAPACITY
+ * command.
+ *
+ * If we get success,
+ *       If Returned LBA > x - 1, then capacity is x+1, spec behavior.
+ *       Else Returned LBA <= x - 1, then capacity is x, broken device server.
+ * Else error, nothing can be assumed, capacity is x+1.
+ */
+#define GET_RLBA_READ_CAP16(_buffer) (((u64)(_buffer)[0] << 56) | \
+				     ((u64)(_buffer)[1] << 48) |  \
+				     ((u64)(_buffer)[2] << 40) |  \
+				     ((u64)(_buffer)[3] << 32) |  \
+				     ((sector_t)(_buffer)[4] << 24) | \
+				     ((sector_t)(_buffer)[5] << 16) | \
+				     ((sector_t)(_buffer)[6] << 8)  | \
+				     (sector_t)(_buffer)[7])
+#define GET_RLBA_READ_CAP10(_buffer) (((sector_t)(_buffer)[0] << 24) | \
+				      ((_buffer)[1] << 16) |           \
+				      ((_buffer)[2] << 8) |            \
+				      (_buffer)[3])
+static void sd_read_true_cap(struct scsi_disk *sd, char *diskname,
+			     struct scsi_request *SRpnt, unsigned char *buffer,
+			     int longrc)
+{
+	unsigned char cmd[16];
+	unsigned char buf[12];
+
+	/* save the old buffer contents here */
+	memcpy(buf, buffer, 12);
+
+	if (longrc) {
+		u64 *lba = (u64 *) (cmd+2);
+		u64 rlba;
+
+		memset((void *) cmd, 0, 16);
+		cmd[0] = SERVICE_ACTION_IN;
+		cmd[1] = SAI_READ_CAPACITY_16;
+		cmd[13] = 12;
+
+		rlba = GET_RLBA_READ_CAP16(buffer);
+		rlba -= 1;
+		*lba = cpu_to_be64(rlba);
+		/* turn on the PMI bit */
+		cmd[14] |= 1;
+		memset((void *) buffer, 0, 12);
+	} else {
+		u32 *lba = (u32 *) (cmd+2);
+		u32 rlba;
+
+		cmd[0] = READ_CAPACITY;
+		memset((void *) &cmd[1], 0, 9);
+		
+		rlba = GET_RLBA_READ_CAP10(buffer);
+		rlba -= 1;
+		*lba = cpu_to_be32(rlba);
+		/* turn on the PMI bit */
+		cmd[8] |= 1;
+		memset((void *) buffer, 0, 8);
+	}
+
+	SRpnt->sr_cmd_len = 0;
+	SRpnt->sr_sense_buffer[0] = 0;
+	SRpnt->sr_sense_buffer[2] = 0;
+	SRpnt->sr_data_direction = DMA_FROM_DEVICE;
+	
+	scsi_wait_req(SRpnt, (void *) cmd, (void *) buffer,
+		      longrc ? 12 : 8, SD_TIMEOUT, SD_MAX_RETRIES);
+
+	if (SRpnt->sr_result) {
+		/* Nothing can be assumed. */
+		printk(KERN_NOTICE "%s: %s: PMI not supported\n",
+		       __FUNCTION__, diskname);
+		memcpy(buffer, buf, 12);
+		return;
+	}
+
+	if (longrc) {
+		u64 rlba = GET_RLBA_READ_CAP16(buffer);
+		u64 x    = GET_RLBA_READ_CAP16(buf);
+		if (rlba > x - 1) {
+			goto out_spec;
+		}
+	} else {
+		u32 rlba = GET_RLBA_READ_CAP10(buffer);
+		u32 x    = GET_RLBA_READ_CAP10(buf);
+		if (rlba > x - 1) {
+			goto out_spec;
+		}
+	}
+	printk(KERN_NOTICE "%s: %s: broken device server\n", __FUNCTION__,
+	       diskname);
+	return;
+	
+ out_spec:
+	/* Capacity is x+1, spec behavior. */
+	printk(KERN_NOTICE "%s: %s: spec behavior\n", __FUNCTION__, diskname);
+	memcpy(buffer, buf, 12);
+} /* end sd_read_true_cap() */
+
  /*
   * read disk capacity
   */
@@ -1070,7 +1176,12 @@
  		
  		sdkp->capacity = 1 + (sector_t) 0xffffffff;		
  		goto got_data;
-	}	
+	}
+
+	/* Check if the device reported CAPACITY as opposed to
+	 * the maxumum LBA (as per the SBC spec).
+	 */
+	sd_read_true_cap(sdkp, diskname, SRpnt, buffer, longrc);
  	
  	if (!longrc) {
  		sector_size = (buffer[4] << 24) |
@@ -1078,12 +1189,14 @@
  		if (buffer[0] == 0xff && buffer[1] == 0xff &&
  		    buffer[2] == 0xff && buffer[3] == 0xff) {
  			if(sizeof(sdkp->capacity) > 4) {
-				printk(KERN_NOTICE "%s : very big device. try to use"
-				       " READ CAPACITY(16).\n", diskname);
+				printk(KERN_NOTICE "%s : very big device. "
+				       "try to use READ CAPACITY(16).\n",
+				       diskname);
  				longrc = 1;
  				goto repeat;
  			} else {
-				printk(KERN_ERR "%s: too big for kernel.  Assuming maximum 2Tb\n", diskname);
+				printk(KERN_ERR "%s: too big for kernel. "
+				       "Assuming maximum 2Tb\n", diskname);
  			}
  		}
  		sdkp->capacity = 1 + (((sector_t)buffer[0] << 24) |
@@ -1102,7 +1215,7 @@
  			
  		sector_size = (buffer[8] << 24) |
  			(buffer[9] << 16) | (buffer[10] << 8) | buffer[11];
-	}	
+	}

  got_data:
  	if (sector_size == 0) {

[-- Attachment #2: sd_read_cap.patch --]
[-- Type: application/octet-stream, Size: 4565 bytes --]

===== drivers/scsi/sd.c 1.161 vs edited =====
--- 1.161/drivers/scsi/sd.c	2004-10-15 10:46:07 -04:00
+++ edited/drivers/scsi/sd.c	2004-10-19 16:51:46 -04:00
@@ -987,6 +987,112 @@
 	}
 }
 
+/* sd_read_true_cap: Some device servers incorrectly return the
+ * capacity as opposed to the LBA of the last logical block of the
+ * block device.
+ *
+ * We try to fix this as follows: Let x = Returned LBA from the last
+ * READ CAPACITY command issued (result in "buffer").  Reissue the
+ * READ CAPACITY command as follows: set the partial medium indicator
+ * (PMI) bit to one; set the LBA to x - 1. Fire off that READ CAPACITY
+ * command.
+ *
+ * If we get success,
+ *       If Returned LBA > x - 1, then capacity is x+1, spec behavior.
+ *       Else Returned LBA <= x - 1, then capacity is x, broken device server.
+ * Else error, nothing can be assumed, capacity is x+1.
+ */
+#define GET_RLBA_READ_CAP16(_buffer) (((u64)(_buffer)[0] << 56) | \
+				     ((u64)(_buffer)[1] << 48) |  \
+				     ((u64)(_buffer)[2] << 40) |  \
+				     ((u64)(_buffer)[3] << 32) |  \
+				     ((sector_t)(_buffer)[4] << 24) | \
+				     ((sector_t)(_buffer)[5] << 16) | \
+				     ((sector_t)(_buffer)[6] << 8)  | \
+				     (sector_t)(_buffer)[7])
+#define GET_RLBA_READ_CAP10(_buffer) (((sector_t)(_buffer)[0] << 24) | \
+				      ((_buffer)[1] << 16) |           \
+				      ((_buffer)[2] << 8) |            \
+				      (_buffer)[3])
+static void sd_read_true_cap(struct scsi_disk *sd, char *diskname,
+			     struct scsi_request *SRpnt, unsigned char *buffer,
+			     int longrc)
+{
+	unsigned char cmd[16];
+	unsigned char buf[12];
+
+	/* save the old buffer contents here */
+	memcpy(buf, buffer, 12);
+
+	if (longrc) {
+		u64 *lba = (u64 *) (cmd+2);
+		u64 rlba;
+
+		memset((void *) cmd, 0, 16);
+		cmd[0] = SERVICE_ACTION_IN;
+		cmd[1] = SAI_READ_CAPACITY_16;
+		cmd[13] = 12;
+
+		rlba = GET_RLBA_READ_CAP16(buffer);
+		rlba -= 1;
+		*lba = cpu_to_be64(rlba);
+		/* turn on the PMI bit */
+		cmd[14] |= 1;
+		memset((void *) buffer, 0, 12);
+	} else {
+		u32 *lba = (u32 *) (cmd+2);
+		u32 rlba;
+
+		cmd[0] = READ_CAPACITY;
+		memset((void *) &cmd[1], 0, 9);
+		
+		rlba = GET_RLBA_READ_CAP10(buffer);
+		rlba -= 1;
+		*lba = cpu_to_be32(rlba);
+		/* turn on the PMI bit */
+		cmd[8] |= 1;
+		memset((void *) buffer, 0, 8);
+	}
+
+	SRpnt->sr_cmd_len = 0;
+	SRpnt->sr_sense_buffer[0] = 0;
+	SRpnt->sr_sense_buffer[2] = 0;
+	SRpnt->sr_data_direction = DMA_FROM_DEVICE;
+	
+	scsi_wait_req(SRpnt, (void *) cmd, (void *) buffer,
+		      longrc ? 12 : 8, SD_TIMEOUT, SD_MAX_RETRIES);
+
+	if (SRpnt->sr_result) {
+		/* Nothing can be assumed. */
+		printk(KERN_NOTICE "%s: %s: PMI not supported\n",
+		       __FUNCTION__, diskname);
+		memcpy(buffer, buf, 12);
+		return;
+	}
+
+	if (longrc) {
+		u64 rlba = GET_RLBA_READ_CAP16(buffer);
+		u64 x    = GET_RLBA_READ_CAP16(buf);
+		if (rlba > x - 1) {
+			goto out_spec;
+		}
+	} else {
+		u32 rlba = GET_RLBA_READ_CAP10(buffer);
+		u32 x    = GET_RLBA_READ_CAP10(buf);
+		if (rlba > x - 1) {
+			goto out_spec;
+		}
+	}
+	printk(KERN_NOTICE "%s: %s: broken device server\n", __FUNCTION__,
+	       diskname);
+	return;
+	
+ out_spec:
+	/* Capacity is x+1, spec behavior. */
+	printk(KERN_NOTICE "%s: %s: spec behavior\n", __FUNCTION__, diskname);
+	memcpy(buffer, buf, 12);
+} /* end sd_read_true_cap() */
+
 /*
  * read disk capacity
  */
@@ -1070,7 +1176,12 @@
 		
 		sdkp->capacity = 1 + (sector_t) 0xffffffff;		
 		goto got_data;
-	}	
+	}
+
+	/* Check if the device reported CAPACITY as opposed to
+	 * the maxumum LBA (as per the SBC spec).
+	 */
+	sd_read_true_cap(sdkp, diskname, SRpnt, buffer, longrc);
 	
 	if (!longrc) {
 		sector_size = (buffer[4] << 24) |
@@ -1078,12 +1189,14 @@
 		if (buffer[0] == 0xff && buffer[1] == 0xff &&
 		    buffer[2] == 0xff && buffer[3] == 0xff) {
 			if(sizeof(sdkp->capacity) > 4) {
-				printk(KERN_NOTICE "%s : very big device. try to use"
-				       " READ CAPACITY(16).\n", diskname);
+				printk(KERN_NOTICE "%s : very big device. "
+				       "try to use READ CAPACITY(16).\n",
+				       diskname);
 				longrc = 1;
 				goto repeat;
 			} else {
-				printk(KERN_ERR "%s: too big for kernel.  Assuming maximum 2Tb\n", diskname);
+				printk(KERN_ERR "%s: too big for kernel. "
+				       "Assuming maximum 2Tb\n", diskname);
 			}
 		}
 		sdkp->capacity = 1 + (((sector_t)buffer[0] << 24) |
@@ -1102,7 +1215,7 @@
 			
 		sector_size = (buffer[8] << 24) |
 			(buffer[9] << 16) | (buffer[10] << 8) | buffer[11];
-	}	
+	}
 
 got_data:
 	if (sector_size == 0) {

Re: Handling erroneous READ CAPACITY response in sd.c

[Alan Stern]

On Tue, 19 Oct 2004, Luben Tuikov wrote:

> Alan, do you have such a USB device handy?

I don't have one myself, but I know several users who do.

> A third possibility is to use the PMI bit to get the proper value
> of the LBA of the last logical block.
> 
> If you want to move this into sd, then do you know if those
> devices support the use of PMI bit in READ CAPACITY CDB?
> As block devices they should.

I don't know.  Yes, they should -- but considering that the READ CAPACITY 
support is wrong it wouldn't be surprising if the PMI support is also 
wrong.  There's an excellent chance the devices will return the same value 
whether PMI is set or not.

> See the appended/attached patch.  Can you test it against such a USB
> device?

I'll ask some people to try.  Do you think that READ FORMAT CAPACITIES (op
0x23) might be a better approach?

Alan Stern

Re: Handling erroneous READ CAPACITY response in sd.c

[Luben Tuikov]

Alan Stern wrote:
>>A third possibility is to use the PMI bit to get the proper value
>>of the LBA of the last logical block.
>>
>>If you want to move this into sd, then do you know if those
>>devices support the use of PMI bit in READ CAPACITY CDB?
>>As block devices they should.
> 
> 
> I don't know.  Yes, they should -- but considering that the READ CAPACITY 
> support is wrong it wouldn't be surprising if the PMI support is also 
> wrong.  There's an excellent chance the devices will return the same value 
> whether PMI is set or not.

Yeah, I agree.  It's worth a try though.  If the device implements
SBC, then it should implement PMI in READ CAPACITY.  Unless it implements
RBC (Reduced Block Commands) in which case PMI is not present, neither
is READ CAPACITY(16), only the 10 byte version.

>>See the appended/attached patch.  Can you test it against such a USB
>>device?
> 
> 
> I'll ask some people to try.  Do you think that READ FORMAT CAPACITIES (op
> 0x23) might be a better approach?

READ FORMAT CAPACITIES is an _optional_ command for MMC (cd/dvd devices),
so I don't think block devices would implement it.

Given that USB block devices already implement READ CAPACITY, there's a
chance that they implement the PMI bit.  Other than that, reading a "random"
sector trying to guess the capacity might set the device into an unknown
state or hang, so this is risky.

Anyone else with such a device willing to try this patch?

	Luben

Re: Handling erroneous READ CAPACITY response in sd.c

[Luben Tuikov]

[-- Attachment #1: Type: text/plain, Size: 93 bytes --]

(patch attached -- got lost somehow)

Signed-off-by: Luben Tuikov <luben_tuikov@adaptec.com>

[-- Attachment #2: sd_read_cap.patch --]
[-- Type: application/octet-stream, Size: 4565 bytes --]

===== drivers/scsi/sd.c 1.161 vs edited =====
--- 1.161/drivers/scsi/sd.c	2004-10-15 10:46:07 -04:00
+++ edited/drivers/scsi/sd.c	2004-10-19 16:51:46 -04:00
@@ -987,6 +987,112 @@
 	}
 }
 
+/* sd_read_true_cap: Some device servers incorrectly return the
+ * capacity as opposed to the LBA of the last logical block of the
+ * block device.
+ *
+ * We try to fix this as follows: Let x = Returned LBA from the last
+ * READ CAPACITY command issued (result in "buffer").  Reissue the
+ * READ CAPACITY command as follows: set the partial medium indicator
+ * (PMI) bit to one; set the LBA to x - 1. Fire off that READ CAPACITY
+ * command.
+ *
+ * If we get success,
+ *       If Returned LBA > x - 1, then capacity is x+1, spec behavior.
+ *       Else Returned LBA <= x - 1, then capacity is x, broken device server.
+ * Else error, nothing can be assumed, capacity is x+1.
+ */
+#define GET_RLBA_READ_CAP16(_buffer) (((u64)(_buffer)[0] << 56) | \
+				     ((u64)(_buffer)[1] << 48) |  \
+				     ((u64)(_buffer)[2] << 40) |  \
+				     ((u64)(_buffer)[3] << 32) |  \
+				     ((sector_t)(_buffer)[4] << 24) | \
+				     ((sector_t)(_buffer)[5] << 16) | \
+				     ((sector_t)(_buffer)[6] << 8)  | \
+				     (sector_t)(_buffer)[7])
+#define GET_RLBA_READ_CAP10(_buffer) (((sector_t)(_buffer)[0] << 24) | \
+				      ((_buffer)[1] << 16) |           \
+				      ((_buffer)[2] << 8) |            \
+				      (_buffer)[3])
+static void sd_read_true_cap(struct scsi_disk *sd, char *diskname,
+			     struct scsi_request *SRpnt, unsigned char *buffer,
+			     int longrc)
+{
+	unsigned char cmd[16];
+	unsigned char buf[12];
+
+	/* save the old buffer contents here */
+	memcpy(buf, buffer, 12);
+
+	if (longrc) {
+		u64 *lba = (u64 *) (cmd+2);
+		u64 rlba;
+
+		memset((void *) cmd, 0, 16);
+		cmd[0] = SERVICE_ACTION_IN;
+		cmd[1] = SAI_READ_CAPACITY_16;
+		cmd[13] = 12;
+
+		rlba = GET_RLBA_READ_CAP16(buffer);
+		rlba -= 1;
+		*lba = cpu_to_be64(rlba);
+		/* turn on the PMI bit */
+		cmd[14] |= 1;
+		memset((void *) buffer, 0, 12);
+	} else {
+		u32 *lba = (u32 *) (cmd+2);
+		u32 rlba;
+
+		cmd[0] = READ_CAPACITY;
+		memset((void *) &cmd[1], 0, 9);
+		
+		rlba = GET_RLBA_READ_CAP10(buffer);
+		rlba -= 1;
+		*lba = cpu_to_be32(rlba);
+		/* turn on the PMI bit */
+		cmd[8] |= 1;
+		memset((void *) buffer, 0, 8);
+	}
+
+	SRpnt->sr_cmd_len = 0;
+	SRpnt->sr_sense_buffer[0] = 0;
+	SRpnt->sr_sense_buffer[2] = 0;
+	SRpnt->sr_data_direction = DMA_FROM_DEVICE;
+	
+	scsi_wait_req(SRpnt, (void *) cmd, (void *) buffer,
+		      longrc ? 12 : 8, SD_TIMEOUT, SD_MAX_RETRIES);
+
+	if (SRpnt->sr_result) {
+		/* Nothing can be assumed. */
+		printk(KERN_NOTICE "%s: %s: PMI not supported\n",
+		       __FUNCTION__, diskname);
+		memcpy(buffer, buf, 12);
+		return;
+	}
+
+	if (longrc) {
+		u64 rlba = GET_RLBA_READ_CAP16(buffer);
+		u64 x    = GET_RLBA_READ_CAP16(buf);
+		if (rlba > x - 1) {
+			goto out_spec;
+		}
+	} else {
+		u32 rlba = GET_RLBA_READ_CAP10(buffer);
+		u32 x    = GET_RLBA_READ_CAP10(buf);
+		if (rlba > x - 1) {
+			goto out_spec;
+		}
+	}
+	printk(KERN_NOTICE "%s: %s: broken device server\n", __FUNCTION__,
+	       diskname);
+	return;
+	
+ out_spec:
+	/* Capacity is x+1, spec behavior. */
+	printk(KERN_NOTICE "%s: %s: spec behavior\n", __FUNCTION__, diskname);
+	memcpy(buffer, buf, 12);
+} /* end sd_read_true_cap() */
+
 /*
  * read disk capacity
  */
@@ -1070,7 +1176,12 @@
 		
 		sdkp->capacity = 1 + (sector_t) 0xffffffff;		
 		goto got_data;
-	}	
+	}
+
+	/* Check if the device reported CAPACITY as opposed to
+	 * the maxumum LBA (as per the SBC spec).
+	 */
+	sd_read_true_cap(sdkp, diskname, SRpnt, buffer, longrc);
 	
 	if (!longrc) {
 		sector_size = (buffer[4] << 24) |
@@ -1078,12 +1189,14 @@
 		if (buffer[0] == 0xff && buffer[1] == 0xff &&
 		    buffer[2] == 0xff && buffer[3] == 0xff) {
 			if(sizeof(sdkp->capacity) > 4) {
-				printk(KERN_NOTICE "%s : very big device. try to use"
-				       " READ CAPACITY(16).\n", diskname);
+				printk(KERN_NOTICE "%s : very big device. "
+				       "try to use READ CAPACITY(16).\n",
+				       diskname);
 				longrc = 1;
 				goto repeat;
 			} else {
-				printk(KERN_ERR "%s: too big for kernel.  Assuming maximum 2Tb\n", diskname);
+				printk(KERN_ERR "%s: too big for kernel. "
+				       "Assuming maximum 2Tb\n", diskname);
 			}
 		}
 		sdkp->capacity = 1 + (((sector_t)buffer[0] << 24) |
@@ -1102,7 +1215,7 @@
 			
 		sector_size = (buffer[8] << 24) |
 			(buffer[9] << 16) | (buffer[10] << 8) | buffer[11];
-	}	
+	}
 
 got_data:
 	if (sector_size == 0) {

Re: Handling erroneous READ CAPACITY response in sd.c

[Alan Stern]

People:

We would like to test a method for determining automatically when the 
capacity value returned by a drive is too high.  If this works it means 
that the unusual_devs.h entries for your devices can be removed, and we'll 
never be troubled by similarly broken devices again.

Please try applying the patches below.  The first one removes the existing
unusual_devs entries and the second one includes the code we want to test.  
Test results are written to the system log, and you don't need to turn on
the usb-storage verbose debugging.  In your replies, include the dmesg 
output showing what happens when you plug in or turn on your drives.

Thanks for your help,

Alan Stern

P.S.: Luben, I took the liberty of adding a routine to try out the 
READ FORMAT CAPACITIES command just to see how well it works.  Probably a 
lot of USB disk-like devices support the command since it is used by 
Windows.


First patch:

===== drivers/usb/storage/unusual_devs.h 1.160 vs edited =====
--- 1.160/drivers/usb/storage/unusual_devs.h	2004-10-16 04:56:53 -04:00
+++ edited/drivers/usb/storage/unusual_devs.h	2004-10-20 11:34:13 -04:00
@@ -171,16 +171,6 @@
 		"CD-R/RW Drive",
 		US_SC_8070, US_PR_CB, NULL, 0),
 
-/* Reported by Adriaan Penning <a.penning@luon.net>
- * Note that these cameras report "Medium not present" after
- * ALLOW_MEDIUM_REMOVAL, so they also need to be marked
- * NOT_LOCKABLE in the SCSI blacklist (and the vendor is MATSHITA). */
-UNUSUAL_DEV(  0x04da, 0x2372, 0x0000, 0x9999,
-		"Panasonic",
-		"DMC-LCx Camera",
-		US_SC_DEVICE, US_PR_DEVICE, NULL,
-		US_FL_FIX_CAPACITY ),
-
 /* Most of the following entries were developed with the help of
  * Shuttle/SCM directly.
  */
@@ -483,13 +473,6 @@
 		US_FL_SINGLE_LUN ),
 #endif
 
-/* Reported by Darsen Lu <darsen@micro.ee.nthu.edu.tw> */
-UNUSUAL_DEV( 0x066f, 0x8000, 0x0001, 0x0001,
-		"SigmaTel",
-		"USBMSC Audio Player",
-		US_SC_DEVICE, US_PR_DEVICE, NULL,
-		US_FL_FIX_CAPACITY ),
-
 /* Submitted by Benny Sjostrand <benny@hostmobility.com> */
 UNUSUAL_DEV( 0x0686, 0x4011, 0x0001, 0x0001,
 		"Minolta",
@@ -546,13 +529,6 @@
 		"USB-IDE",
 		US_SC_QIC, US_PR_FREECOM, freecom_init, 0),
 #endif
-
-/* Reported by Eero Volotinen <eero@ping-viini.org> */
-UNUSUAL_DEV(  0x07ab, 0xfccd, 0x0406, 0x0406,
-		"Freecom Technologies",
-		"FHD-Classic",
-		US_SC_DEVICE, US_PR_DEVICE, NULL,
-		US_FL_FIX_CAPACITY),
 
 UNUSUAL_DEV(  0x07af, 0x0004, 0x0100, 0x0133, 
 		"Microtech",



Second patch:

===== drivers/scsi/sd.c 1.72 vs edited =====
--- 1.72/drivers/scsi/sd.c	2004-09-13 12:46:37 -04:00
+++ edited/drivers/scsi/sd.c	2004-10-20 11:13:26 -04:00
@@ -990,6 +990,171 @@
 	}
 }
 
+/* sd_read_true_cap: Some device servers incorrectly return the
+ * capacity as opposed to the LBA of the last logical block of the
+ * block device.
+ *
+ * We try to fix this as follows: Let x = Returned LBA from the last
+ * READ CAPACITY command issued (result in "buffer").  Reissue the
+ * READ CAPACITY command as follows: set the partial medium indicator
+ * (PMI) bit to one; set the LBA to x - 1. Fire off that READ CAPACITY
+ * command.
+ *
+ * If we get success,
+ *       If Returned LBA > x - 1, then capacity is x+1, spec behavior.
+ *       Else Returned LBA <= x - 1, then capacity is x, broken device server.
+ * Else error, nothing can be assumed, capacity is x+1.
+ */
+#define GET_RLBA_READ_CAP16(_buffer) (((u64)(_buffer)[0] << 56) | \
+				     ((u64)(_buffer)[1] << 48) |  \
+				     ((u64)(_buffer)[2] << 40) |  \
+				     ((u64)(_buffer)[3] << 32) |  \
+				     ((sector_t)(_buffer)[4] << 24) | \
+				     ((sector_t)(_buffer)[5] << 16) | \
+				     ((sector_t)(_buffer)[6] << 8)  | \
+				     (sector_t)(_buffer)[7])
+#define GET_RLBA_READ_CAP10(_buffer) (((sector_t)(_buffer)[0] << 24) | \
+				      ((_buffer)[1] << 16) |           \
+				      ((_buffer)[2] << 8) |            \
+				      (_buffer)[3])
+static void sd_read_true_cap(struct scsi_disk *sd, char *diskname,
+			     struct scsi_request *SRpnt, unsigned char *buffer,
+			     int longrc)
+{
+	unsigned char cmd[16];
+	unsigned char buf[12];
+
+	/* save the old buffer contents here */
+	memcpy(buf, buffer, 12);
+
+	if (longrc) {
+		u64 *lba = (u64 *) (cmd+2);
+		u64 rlba;
+
+		memset((void *) cmd, 0, 16);
+		cmd[0] = SERVICE_ACTION_IN;
+		cmd[1] = SAI_READ_CAPACITY_16;
+		cmd[13] = 12;
+
+		rlba = GET_RLBA_READ_CAP16(buffer);
+		rlba -= 1;
+		*lba = cpu_to_be64(rlba);
+		/* turn on the PMI bit */
+		cmd[14] |= 1;
+		memset((void *) buffer, 0, 12);
+	} else {
+		u32 *lba = (u32 *) (cmd+2);
+		u32 rlba;
+
+		cmd[0] = READ_CAPACITY;
+		memset((void *) &cmd[1], 0, 9);
+		
+		rlba = GET_RLBA_READ_CAP10(buffer);
+		rlba -= 1;
+		*lba = cpu_to_be32(rlba);
+		/* turn on the PMI bit */
+		cmd[8] |= 1;
+		memset((void *) buffer, 0, 8);
+	}
+
+	SRpnt->sr_cmd_len = 0;
+	SRpnt->sr_sense_buffer[0] = 0;
+	SRpnt->sr_sense_buffer[2] = 0;
+	SRpnt->sr_data_direction = DMA_FROM_DEVICE;
+	
+	scsi_wait_req(SRpnt, (void *) cmd, (void *) buffer,
+		      longrc ? 12 : 8, SD_TIMEOUT, SD_MAX_RETRIES);
+
+	if (SRpnt->sr_result) {
+		/* Nothing can be assumed. */
+		printk(KERN_NOTICE "%s: %s: PMI not supported\n",
+		       __FUNCTION__, diskname);
+		memcpy(buffer, buf, 12);
+		return;
+	}
+
+	if (longrc) {
+		u64 rlba = GET_RLBA_READ_CAP16(buffer);
+		u64 x    = GET_RLBA_READ_CAP16(buf);
+		if (rlba > x - 1) {
+			goto out_spec;
+		}
+	} else {
+		u32 rlba = GET_RLBA_READ_CAP10(buffer);
+		u32 x    = GET_RLBA_READ_CAP10(buf);
+		if (rlba > x - 1) {
+			goto out_spec;
+		}
+	}
+	printk(KERN_NOTICE "%s: %s: broken device server\n", __FUNCTION__,
+	       diskname);
+	return;
+	
+ out_spec:
+	/* Capacity is x+1, spec behavior. */
+	printk(KERN_NOTICE "%s: %s: spec behavior\n", __FUNCTION__, diskname);
+	memcpy(buffer, buf, 12);
+} /* end sd_read_true_cap() */
+
+/* Try to use the READ FORMAT CAPACITIES command to check whether the
+ * READ CAPACITY result is correct.
+ */
+static void sd_read_format_cap(struct scsi_disk *sd, char *diskname,
+			     struct scsi_request *SRpnt, unsigned char *buffer)
+{
+	unsigned char cmd[12];
+	unsigned char buf[12];
+	u32 rlba;
+	u32 x;
+
+	/* save the old buffer contents here */
+	memcpy(buf, buffer, 12);
+
+	rlba = GET_RLBA_READ_CAP10(buffer);
+
+	cmd[0] = 0x23;		/* READ_FORMAT_CAPACITIES */
+	memset(&cmd[1], 0, 11);
+	cmd[8] = 12;
+	memset(buffer, 0, 12);
+
+	SRpnt->sr_cmd_len = 0;
+	SRpnt->sr_sense_buffer[0] = 0;
+	SRpnt->sr_sense_buffer[2] = 0;
+	SRpnt->sr_data_direction = DMA_FROM_DEVICE;
+
+	scsi_wait_req(SRpnt, (void *) cmd, (void *) buffer,
+		      12, SD_TIMEOUT, SD_MAX_RETRIES);
+
+	if (SRpnt->sr_result) {
+		/* Nothing can be assumed. */
+		printk(KERN_NOTICE "%s: %s: READ FORMAT CAPACITIES not supported\n",
+		       __FUNCTION__, diskname);
+		goto done;
+	}
+
+	if ((buffer[8] & 3) != 2) {
+		printk(KERN_NOTICE "%s: %s: no media or unformatted media\n",
+		       __FUNCTION__, diskname);
+		goto done;
+	}
+
+	x = GET_RLBA_READ_CAP10(buffer + 4);
+	printk(KERN_NOTICE "%s: %s: %u vs. %u\n",
+		       __FUNCTION__, diskname, rlba, x);
+	if (x > 0 && x <= rlba) {
+		printk(KERN_NOTICE "%s: %s: using lower capacity\n",
+		       __FUNCTION__, diskname);
+		--x;
+		buf[0] = (unsigned char) (x >> 24);
+		buf[1] = (unsigned char) (x >> 16);
+		buf[2] = (unsigned char) (x >> 8);
+		buf[3] = (unsigned char) x;
+	}
+
+done:
+	memcpy(buffer, buf, 12);
+}
+
 /*
  * read disk capacity
  */
@@ -1073,7 +1238,14 @@
 		
 		sdkp->capacity = 1 + (sector_t) 0xffffffff;		
 		goto got_data;
-	}	
+	}
+
+	/* Check if the device reported CAPACITY as opposed to
+	 * the maxumum LBA (as per the SBC spec).
+	 */
+	sd_read_true_cap(sdkp, diskname, SRpnt, buffer, longrc);
+	if (!longrc)
+		sd_read_format_cap(sdkp, diskname, SRpnt, buffer);
 	
 	if (!longrc) {
 		sector_size = (buffer[4] << 24) |
@@ -1081,12 +1253,14 @@
 		if (buffer[0] == 0xff && buffer[1] == 0xff &&
 		    buffer[2] == 0xff && buffer[3] == 0xff) {
 			if(sizeof(sdkp->capacity) > 4) {
-				printk(KERN_NOTICE "%s : very big device. try to use"
-				       " READ CAPACITY(16).\n", diskname);
+				printk(KERN_NOTICE "%s : very big device. "
+				       "try to use READ CAPACITY(16).\n",
+				       diskname);
 				longrc = 1;
 				goto repeat;
 			} else {
-				printk(KERN_ERR "%s: too big for kernel.  Assuming maximum 2Tb\n", diskname);
+				printk(KERN_ERR "%s: too big for kernel. "
+				       "Assuming maximum 2Tb\n", diskname);
 			}
 		}
 		sdkp->capacity = 1 + (((sector_t)buffer[0] << 24) |
@@ -1105,7 +1279,7 @@
 			
 		sector_size = (buffer[8] << 24) |
 			(buffer[9] << 16) | (buffer[10] << 8) | buffer[11];
-	}	
+	}
 
 got_data:
 	if (sector_size == 0) {

Re: Handling erroneous READ CAPACITY response in sd.c

[Eero Volotinen]

Alan Stern wrote:
> People:
> 
> We would like to test a method for determining automatically when the 
> capacity value returned by a drive is too high.  If this works it means 
> that the unusual_devs.h entries for your devices can be removed, and we'll 
> never be troubled by similarly broken devices again.
> 
> Please try applying the patches below.  The first one removes the existing
> unusual_devs entries and the second one includes the code we want to test.  
> Test results are written to the system log, and you don't need to turn on
> the usb-storage verbose debugging.  In your replies, include the dmesg 
> output showing what happens when you plug in or turn on your drives.

Does not work on my machine. It detects disk and then it hangs.

usb 1-4: new high speed USB device using address 3
Initializing USB Mass Storage driver...
usb 1-4: control timeout on ep0in
usb 1-4: string descriptor 0 read error: -71
usb 1-4: string descriptor 0 read error: -71
usb 1-4: string descriptor 0 read error: -71
usb 1-4: string descriptor 0 read error: -71
scsi0 : SCSI emulation for USB Mass Storage devices
usbcore: registered new driver usb-storage
USB Mass Storage support registered.
usb 1-4: string descriptor 0 read error: -71
usb-storage: device found at 3
usb 1-4: string descriptor 0 read error: -71
usb-storage: waiting for device to settle before scanning
inserting floppy driver for 2.6.10-rc1alan
floppy0: no floppy controllers found
   Vendor: HDS72808  Model: 0PLAT20           Rev: PF2O
   Type:   Direct-Access                      ANSI SCSI revision: 00
sd_read_true_cap: sda: spec behavior
sd_read_format_cap: sda: no media or unformatted media
SCSI device sda: 160836481 512-byte hdwr sectors (82348 MB)
sda: assuming drive cache: write through
  sda:SCSI error : <0 0 0 0> return code = 0x70000
end_request: I/O error, dev sda, sector 160836480
Buffer I/O error on device sda, logical block 160836480
SCSI error : <0 0 0 0> return code = 0x70000
end_request: I/O error, dev sda, sector 160836480
Buffer I/O error on device sda, logical block 160836480
  sda1
Attached scsi disk sda at scsi0, channel 0, id 0, lun 0
usb-storage: device scan complete
SCSI error : <0 0 0 0> return code = 0x70000
end_request: I/O error, dev sda, sector 0
Buffer I/O error on device sda, logical block 0
SCSI error : <0 0 0 0> return code = 0x70000
end_request: I/O error, dev sda, sector 1
Buffer I/O error on device sda, logical block 1
SCSI error : <0 0 0 0> return code = 0x70000
end_request: I/O error, dev sda, sector 2
Buffer I/O error on device sda, logical block 2
SCSI error : <0 0 0 0> return code = 0x70000
end_request: I/O error, dev sda, sector 3
Buffer I/O error on device sda, logical block 3
SCSI error : <0 0 0 0> return code = 0x70000
end_request: I/O error, dev sda, sector 4
Buffer I/O error on device sda, logical block 4
SCSI error : <0 0 0 0> return code = 0x70000
end_request: I/O error, dev sda, sector 5
Buffer I/O error on device sda, logical block 5
SCSI error : <0 0 0 0> return code = 0x70000
end_request: I/O error, dev sda, sector 6
Buffer I/O error on device sda, logical block 6
usb 1-4: USB disconnect, address 3
SCSI error : <0 0 0 0> return code = 0x70000
end_request: I/O error, dev sda, sector 7
Buffer I/O error on device sda, logical block 7
SCSI error : <0 0 0 0> return code = 0x70000
end_request: I/O error, dev sda, sector 8
Buffer I/O error on device sda, logical block 8
  target0:0:0: Illegal state transition <NULL>->cancel
Badness in scsi_device_set_state at drivers/scsi/scsi_lib.c:1713

--
Eero

Re: Handling erroneous READ CAPACITY response in sd.c

[Alan Stern]

On Sun, 24 Oct 2004, Eero Volotinen wrote:

> Does not work on my machine. It detects disk and then it hangs.

>    Vendor: HDS72808  Model: 0PLAT20           Rev: PF2O
>    Type:   Direct-Access                      ANSI SCSI revision: 00
> sd_read_true_cap: sda: spec behavior
> sd_read_format_cap: sda: no media or unformatted media
> SCSI device sda: 160836481 512-byte hdwr sectors (82348 MB)
> sda: assuming drive cache: write through
>   sda:SCSI error : <0 0 0 0> return code = 0x70000
> end_request: I/O error, dev sda, sector 160836480
> Buffer I/O error on device sda, logical block 160836480

Okay, so it looks like neither of the earlier suggestions will work.

Here's one that I feel certain will work, although it takes a slightly 
different approach.  Please try it out and see what happens.

Luben and Andries, what do you think of this patch?

Alan Stern



Signed-off-by: Alan Stern <stern@rowland.harvard.edu>

===== include/scsi/scsi_device.h 1.14 vs edited =====
--- 1.14/include/scsi/scsi_device.h	2004-10-05 11:47:13 -04:00
+++ edited/include/scsi/scsi_device.h	2004-10-25 15:10:48 -04:00
@@ -111,6 +111,7 @@
 	unsigned allow_restart:1; /* issue START_UNIT in error handler */
 	unsigned no_uld_attach:1; /* disable connecting to upper level drivers */
 	unsigned select_no_atn:1;
+	unsigned even_capacity:1; /* must have an even number of sectors */
 
 	unsigned int device_blocked;	/* Device returned QUEUE_FULL. */
 
===== drivers/scsi/sd.c 1.73 vs edited =====
--- 1.73/drivers/scsi/sd.c	2004-10-20 12:24:44 -04:00
+++ edited/drivers/scsi/sd.c	2004-10-25 15:19:33 -04:00
@@ -1133,6 +1133,16 @@
 		 */
 		sector_size = 512;
 	}
+
+	/* Handle broken devices that return the total number of sectors
+	 * rather than the highest sector number, by forcing the total
+	 * number to be even. */
+	if (sdp->even_capacity && (sdkp->capacity & 1)) {
+		printk(KERN_NOTICE "%s : odd number of sectors reported, "
+			"decreasing by one.\n", diskname);
+		--sdkp->capacity;
+	}
+
 	{
 		/*
 		 * The msdos fs needs to know the hardware sector size
===== drivers/usb/storage/protocol.c 1.25 vs edited =====
--- 1.25/drivers/usb/storage/protocol.c	2004-10-20 12:38:15 -04:00
+++ edited/drivers/usb/storage/protocol.c	2004-10-25 15:23:32 -04:00
@@ -57,35 +57,6 @@
  * Helper routines
  ***********************************************************************/
 
-/*
- * Fix-up the return data from a READ CAPACITY command. My Feiya reader
- * returns a value that is 1 too large.
- */
-static void fix_read_capacity(struct scsi_cmnd *srb)
-{
-	unsigned int index, offset;
-	__be32 c;
-	unsigned long capacity;
-
-	/* verify that it's a READ CAPACITY command */
-	if (srb->cmnd[0] != READ_CAPACITY)
-		return;
-
-	index = offset = 0;
-	if (usb_stor_access_xfer_buf((unsigned char *) &c, 4, srb,
-			&index, &offset, FROM_XFER_BUF) != 4)
-		return;
-
-	capacity = be32_to_cpu(c);
-	US_DEBUGP("US: Fixing capacity: from %ld to %ld\n",
-	       capacity+1, capacity);
-	c = cpu_to_be32(capacity - 1);
-
-	index = offset = 0;
-	usb_stor_access_xfer_buf((unsigned char *) &c, 4, srb,
-			&index, &offset, TO_XFER_BUF);
-}
-
 /***********************************************************************
  * Protocol routines
  ***********************************************************************/
@@ -174,12 +145,6 @@
 {
 	/* send the command to the transport layer */
 	usb_stor_invoke_transport(srb, us);
-
-	if (srb->result == SAM_STAT_GOOD) {
-		/* Fix the READ CAPACITY result if necessary */
-		if (us->flags & US_FL_FIX_CAPACITY)
-			fix_read_capacity(srb);
-	}
 }
 
 /***********************************************************************
===== drivers/usb/storage/scsiglue.c 1.86 vs edited =====
--- 1.86/drivers/usb/storage/scsiglue.c	2004-09-30 16:07:33 -04:00
+++ edited/drivers/usb/storage/scsiglue.c	2004-10-25 15:25:44 -04:00
@@ -152,6 +152,12 @@
 		sdev->skip_ms_page_3f = 1;
 #endif
 
+		/* Some devices return the total number of blocks in
+		 * response to READ CAPACITY instead of the highest
+		 * block number.  Deal with this by forcing the number
+		 * of blocks to be even.  Fortunately no existing USB
+		 * drive has an odd number of blocks, so far as I know! */
+		sdev->even_capacity = 1;
 	} else {
 
 		/* Non-disk-type devices don't need to blacklist any pages
@@ -390,7 +396,7 @@
 		DO_FLAG(SINGLE_LUN);
 		DO_FLAG(SCM_MULT_TARG);
 		DO_FLAG(FIX_INQUIRY);
-		DO_FLAG(FIX_CAPACITY);
+		DO_FLAG(IGNORE_RESIDUE);
 
 		*(pos++) = '\n';
 	}
===== drivers/usb/storage/unusual_devs.h 1.161 vs edited =====
--- 1.161/drivers/usb/storage/unusual_devs.h	2004-10-20 12:38:15 -04:00
+++ edited/drivers/usb/storage/unusual_devs.h	2004-10-25 15:24:29 -04:00
@@ -171,16 +171,6 @@
 		"CD-R/RW Drive",
 		US_SC_8070, US_PR_CB, NULL, 0),
 
-/* Reported by Adriaan Penning <a.penning@luon.net>
- * Note that these cameras report "Medium not present" after
- * ALLOW_MEDIUM_REMOVAL, so they also need to be marked
- * NOT_LOCKABLE in the SCSI blacklist (and the vendor is MATSHITA). */
-UNUSUAL_DEV(  0x04da, 0x2372, 0x0000, 0x9999,
-		"Panasonic",
-		"DMC-LCx Camera",
-		US_SC_DEVICE, US_PR_DEVICE, NULL,
-		US_FL_FIX_CAPACITY ),
-
 /* Most of the following entries were developed with the help of
  * Shuttle/SCM directly.
  */
@@ -483,13 +473,6 @@
 		US_FL_SINGLE_LUN ),
 #endif
 
-/* Reported by Darsen Lu <darsen@micro.ee.nthu.edu.tw> */
-UNUSUAL_DEV( 0x066f, 0x8000, 0x0001, 0x0001,
-		"SigmaTel",
-		"USBMSC Audio Player",
-		US_SC_DEVICE, US_PR_DEVICE, NULL,
-		US_FL_FIX_CAPACITY ),
-
 /* Submitted by Benny Sjostrand <benny@hostmobility.com> */
 UNUSUAL_DEV( 0x0686, 0x4011, 0x0001, 0x0001,
 		"Minolta",
@@ -547,13 +530,6 @@
 		US_SC_QIC, US_PR_FREECOM, freecom_init, 0),
 #endif
 
-/* Reported by Eero Volotinen <eero@ping-viini.org> */
-UNUSUAL_DEV(  0x07ab, 0xfccd, 0x0406, 0x0406,
-		"Freecom Technologies",
-		"FHD-Classic",
-		US_SC_DEVICE, US_PR_DEVICE, NULL,
-		US_FL_FIX_CAPACITY),
-
 UNUSUAL_DEV(  0x07af, 0x0004, 0x0100, 0x0133, 
 		"Microtech",
 		"USB-SCSI-DB25",
@@ -710,13 +686,6 @@
 		"MP3 player",
 		US_SC_RBC, US_PR_BULK, NULL,
 		US_FL_MODE_XLATE),
-
-/* aeb */
-UNUSUAL_DEV( 0x090c, 0x1132, 0x0000, 0xffff,
-		"Feiya",
-		"5-in-1 Card Reader",
-		US_SC_DEVICE, US_PR_DEVICE, NULL,
-		US_FL_FIX_CAPACITY ),
 
 UNUSUAL_DEV(  0x097a, 0x0001, 0x0000, 0x0001,
 		"Minds@Work",
===== drivers/usb/storage/usb.h 1.64 vs edited =====
--- 1.64/drivers/usb/storage/usb.h	2004-10-20 12:38:15 -04:00
+++ edited/drivers/usb/storage/usb.h	2004-10-25 15:23:01 -04:00
@@ -72,7 +72,7 @@
 #define US_FL_IGNORE_SER      0		 /* [no longer used]		    */
 #define US_FL_SCM_MULT_TARG   0x00000020 /* supports multiple targets	    */
 #define US_FL_FIX_INQUIRY     0x00000040 /* INQUIRY response needs faking   */
-#define US_FL_FIX_CAPACITY    0x00000080 /* READ CAPACITY response too big  */
+#define US_FL_FIX_CAPACITY    0		 /* [no longer used]		    */
 #define US_FL_IGNORE_RESIDUE  0x00000100 /* reported residue is wrong	    */
 
 /* Dynamic flag definitions: used in set_bit() etc. */

Re: Handling erroneous READ CAPACITY response in sd.c

[Luben Tuikov]

[-- Attachment #1: Type: text/plain, Size: 4697 bytes --]

Eero Volotinen wrote:
> Alan Stern wrote:
> 
>> People:
>>
>> We would like to test a method for determining automatically when the 
>> capacity value returned by a drive is too high.  If this works it 
>> means that the unusual_devs.h entries for your devices can be removed, 
>> and we'll never be troubled by similarly broken devices again.
>>
>> Please try applying the patches below.  The first one removes the 
>> existing
>> unusual_devs entries and the second one includes the code we want to 
>> test.  Test results are written to the system log, and you don't need 
>> to turn on
>> the usb-storage verbose debugging.  In your replies, include the dmesg 
>> output showing what happens when you plug in or turn on your drives.
> 
> 
> Does not work on my machine. It detects disk and then it hangs.
> 
> usb 1-4: new high speed USB device using address 3
> Initializing USB Mass Storage driver...
> usb 1-4: control timeout on ep0in
> usb 1-4: string descriptor 0 read error: -71
> usb 1-4: string descriptor 0 read error: -71
> usb 1-4: string descriptor 0 read error: -71
> usb 1-4: string descriptor 0 read error: -71
> scsi0 : SCSI emulation for USB Mass Storage devices
> usbcore: registered new driver usb-storage
> USB Mass Storage support registered.
> usb 1-4: string descriptor 0 read error: -71
> usb-storage: device found at 3
> usb 1-4: string descriptor 0 read error: -71

Alan, is this error message something to be concerned with?

> usb-storage: waiting for device to settle before scanning
> inserting floppy driver for 2.6.10-rc1alan
> floppy0: no floppy controllers found
>   Vendor: HDS72808  Model: 0PLAT20           Rev: PF2O
>   Type:   Direct-Access                      ANSI SCSI revision: 00
> sd_read_true_cap: sda: spec behavior

Ok, the above line tells that the device _does_ support
the PMI bit, and testing with the PMI bit on returned
values as _intended_ by the spec.

> sd_read_format_cap: sda: no media or unformatted media

Alan, let's weed out _only_ Descriptor Type 0 (reserved).
All others seem to be reporting _some_ capacity...
(see attached patch, I've also removed the macro defs)

The attached patch ignore the "No Media Present",
and trusts the "The reported value is for the maximum
capacity of a media that the Logical Unit is capable
of reading." -- MMC4r00, table 422.

> SCSI device sda: 160836481 512-byte hdwr sectors (82348 MB)
> sda: assuming drive cache: write through
>  sda:SCSI error : <0 0 0 0> return code = 0x70000
> end_request: I/O error, dev sda, sector 160836480
> Buffer I/O error on device sda, logical block 160836480
> SCSI error : <0 0 0 0> return code = 0x70000
> end_request: I/O error, dev sda, sector 160836480
> Buffer I/O error on device sda, logical block 160836480
>  sda1
> Attached scsi disk sda at scsi0, channel 0, id 0, lun 0
> usb-storage: device scan complete
> SCSI error : <0 0 0 0> return code = 0x70000
> end_request: I/O error, dev sda, sector 0
> Buffer I/O error on device sda, logical block 0
> SCSI error : <0 0 0 0> return code = 0x70000
> end_request: I/O error, dev sda, sector 1
> Buffer I/O error on device sda, logical block 1
> SCSI error : <0 0 0 0> return code = 0x70000
> end_request: I/O error, dev sda, sector 2
> Buffer I/O error on device sda, logical block 2
> SCSI error : <0 0 0 0> return code = 0x70000
> end_request: I/O error, dev sda, sector 3
> Buffer I/O error on device sda, logical block 3
> SCSI error : <0 0 0 0> return code = 0x70000
> end_request: I/O error, dev sda, sector 4
> Buffer I/O error on device sda, logical block 4
> SCSI error : <0 0 0 0> return code = 0x70000
> end_request: I/O error, dev sda, sector 5
> Buffer I/O error on device sda, logical block 5
> SCSI error : <0 0 0 0> return code = 0x70000
> end_request: I/O error, dev sda, sector 6
> Buffer I/O error on device sda, logical block 6
> usb 1-4: USB disconnect, address 3
> SCSI error : <0 0 0 0> return code = 0x70000
> end_request: I/O error, dev sda, sector 7
> Buffer I/O error on device sda, logical block 7
> SCSI error : <0 0 0 0> return code = 0x70000
> end_request: I/O error, dev sda, sector 8
> Buffer I/O error on device sda, logical block 8

Ok, so reading the last sector and the first 9 sectors
fails.

Either the device got disrupted earlier, or reading
the "last" LBA confused it.

>  target0:0:0: Illegal state transition <NULL>->cancel
> Badness in scsi_device_set_state at drivers/scsi/scsi_lib.c:1713

How did we get here...?

Eero, can you try the attached patch?  I wonder if we
can take the maximum capacity media which could be supported?
(This is "patch 2", you still have to apply "patch 1" which
removes the broken devices from unusual devs.)

Thanks,
	Luben


[-- Attachment #2: cap.patch --]
[-- Type: application/octet-stream, Size: 5425 bytes --]

===== drivers/scsi/sd.c 1.161 vs edited =====
--- 1.161/drivers/scsi/sd.c	2004-10-15 10:46:07 -04:00
+++ edited/drivers/scsi/sd.c	2004-10-25 13:28:23 -04:00
@@ -987,6 +987,152 @@
 	}
 }
 
+/* sd_read_true_cap: Some device servers incorrectly return the
+ * capacity as opposed to the LBA of the last logical block of the
+ * block device.
+ *
+ * We try to fix this as follows: Let x = Returned LBA from the last
+ * READ CAPACITY command issued (result in "buffer").  Reissue the
+ * READ CAPACITY command as follows: set the partial medium indicator
+ * (PMI) bit to one; set the LBA to x - 1. Fire off that READ CAPACITY
+ * command.
+ *
+ * If we get success,
+ *       If Returned LBA > x - 1, then capacity is x+1, spec behavior.
+ *       Else Returned LBA <= x - 1, then capacity is x, broken device server.
+ * Else error, nothing can be assumed, capacity is x+1.
+ */
+static void sd_read_true_cap(struct scsi_disk *sd, char *diskname,
+			     struct scsi_request *SRpnt, unsigned char *buffer,
+			     int longrc)
+{
+	unsigned char cmd[16];
+	unsigned char buf[12];
+
+	/* save the old buffer contents here */
+	memcpy(buf, buffer, 12);
+
+	if (longrc) {
+		u64 *lba = (u64 *) (cmd+2);
+		u64 rlba;
+
+		memset((void *) cmd, 0, 16);
+		cmd[0] = SERVICE_ACTION_IN;
+		cmd[1] = SAI_READ_CAPACITY_16;
+		cmd[13] = 12;
+
+		rlba = be64_to_cpu(*(u64 *)buffer);
+		rlba -= 1;
+		*lba = cpu_to_be64(rlba);
+		/* turn on the PMI bit */
+		cmd[14] |= 1;
+		memset((void *) buffer, 0, 12);
+	} else {
+		u32 *lba = (u32 *) (cmd+2);
+		u32 rlba;
+
+		cmd[0] = READ_CAPACITY;
+		memset((void *) &cmd[1], 0, 9);
+		
+		rlba = be32_to_cpu(*(u32 *)buffer);
+		rlba -= 1;
+		*lba = cpu_to_be32(rlba);
+		/* turn on the PMI bit */
+		cmd[8] |= 1;
+		memset((void *) buffer, 0, 8);
+	}
+
+	SRpnt->sr_cmd_len = 0;
+	SRpnt->sr_sense_buffer[0] = 0;
+	SRpnt->sr_sense_buffer[2] = 0;
+	SRpnt->sr_data_direction = DMA_FROM_DEVICE;
+	
+	scsi_wait_req(SRpnt, (void *) cmd, (void *) buffer,
+		      longrc ? 12 : 8, SD_TIMEOUT, SD_MAX_RETRIES);
+
+	if (SRpnt->sr_result) {
+		/* Nothing can be assumed. */
+		printk(KERN_NOTICE "%s: %s: PMI not supported\n",
+		       __FUNCTION__, diskname);
+		memcpy(buffer, buf, 12);
+		return;
+	}
+
+	if (longrc) {
+		u64 rlba = be64_to_cpu(*(u64 *)buffer);
+		u64 x    = be64_to_cpu(*(u64 *)buf);
+		if (rlba > x - 1) {
+			goto out_spec;
+		}
+	} else {
+		u32 rlba = be32_to_cpu(*(u32 *)buffer);
+		u32 x    = be32_to_cpu(*(u32 *)buf);
+		if (rlba > x - 1) {
+			goto out_spec;
+		}
+	}
+	printk(KERN_NOTICE "%s: %s: broken device server\n", __FUNCTION__,
+	       diskname);
+	return;
+
+ out_spec:
+	/* Capacity is x+1, spec behavior. */
+	printk(KERN_NOTICE "%s: %s: spec behavior\n", __FUNCTION__, diskname);
+	memcpy(buffer, buf, 12);
+} /* end sd_read_true_cap() */
+
+
+/* Try to use the READ FORMAT CAPACITIES command to check whether the
+ * READ CAPACITY result is correct.
+ */
+static void sd_read_format_cap(struct scsi_disk *sd, char *diskname,
+		       struct scsi_request *SRpnt, unsigned char *buffer)
+{
+	unsigned char cmd[12];
+	unsigned char buf[12];
+	u32 rlba;
+	u32 x;
+
+	/* save the old buffer contents here */
+	memcpy(buf, buffer, 12);
+
+	rlba = be32_to_cpu(*(u32 *)buffer);
+
+	cmd[0] = 0x23;          /* READ_FORMAT_CAPACITIES */
+	memset(&cmd[1], 0, 11);
+	cmd[8] = 12;
+	memset(buffer, 0, 12);
+
+	SRpnt->sr_cmd_len = 0;
+	SRpnt->sr_sense_buffer[0] = 0;
+	SRpnt->sr_sense_buffer[2] = 0;
+	SRpnt->sr_data_direction = DMA_FROM_DEVICE;
+
+	scsi_wait_req(SRpnt, (void *) cmd, (void *) buffer,
+		      12, SD_TIMEOUT, SD_MAX_RETRIES);
+
+	if (SRpnt->sr_result) {
+		/* Nothing can be assumed. */
+		printk(KERN_NOTICE "%s: %s: READ FORMAT CAPACITIES "
+		       "not supported\n", __FUNCTION__, diskname);
+		goto done;
+	}
+
+	x = be32_to_cpu(*(u32 *)(buffer + 4));
+	printk(KERN_NOTICE "%s: %s: %u vs. %u\n",
+	       __FUNCTION__, diskname, rlba, x);
+	if (0 < x && x <= rlba) {
+		u32 *lba = (u32 *) buf;
+		
+		printk(KERN_NOTICE "%s: %s: using lower capacity\n",
+		       __FUNCTION__, diskname);
+		x -= 1;
+		*lba = cpu_to_be32(x);
+	}
+ done:
+ 	memcpy(buffer, buf, 12);
+}
+
 /*
  * read disk capacity
  */
@@ -1070,7 +1216,14 @@
 		
 		sdkp->capacity = 1 + (sector_t) 0xffffffff;		
 		goto got_data;
-	}	
+	}
+
+	/* Check if the device reported CAPACITY as opposed to
+	 * the maxumum LBA (as per the SBC spec).
+	 */
+	sd_read_true_cap(sdkp, diskname, SRpnt, buffer, longrc);
+ 	if (!longrc)
+ 		sd_read_format_cap(sdkp, diskname, SRpnt, buffer);
 	
 	if (!longrc) {
 		sector_size = (buffer[4] << 24) |
@@ -1078,12 +1231,14 @@
 		if (buffer[0] == 0xff && buffer[1] == 0xff &&
 		    buffer[2] == 0xff && buffer[3] == 0xff) {
 			if(sizeof(sdkp->capacity) > 4) {
-				printk(KERN_NOTICE "%s : very big device. try to use"
-				       " READ CAPACITY(16).\n", diskname);
+				printk(KERN_NOTICE "%s : very big device. "
+				       "try to use READ CAPACITY(16).\n",
+				       diskname);
 				longrc = 1;
 				goto repeat;
 			} else {
-				printk(KERN_ERR "%s: too big for kernel.  Assuming maximum 2Tb\n", diskname);
+				printk(KERN_ERR "%s: too big for kernel. "
+				       "Assuming maximum 2Tb\n", diskname);
 			}
 		}
 		sdkp->capacity = 1 + (((sector_t)buffer[0] << 24) |
@@ -1102,7 +1257,7 @@
 			
 		sector_size = (buffer[8] << 24) |
 			(buffer[9] << 16) | (buffer[10] << 8) | buffer[11];
-	}	
+	}
 
 got_data:
 	if (sector_size == 0) {

Re: Handling erroneous READ CAPACITY response in sd.c

[Luben Tuikov]

Alan Stern wrote:
> On Sun, 24 Oct 2004, Eero Volotinen wrote:
> 
> 
>>Does not work on my machine. It detects disk and then it hangs.
> 
> 
>>   Vendor: HDS72808  Model: 0PLAT20           Rev: PF2O
>>   Type:   Direct-Access                      ANSI SCSI revision: 00
>>sd_read_true_cap: sda: spec behavior
>>sd_read_format_cap: sda: no media or unformatted media
>>SCSI device sda: 160836481 512-byte hdwr sectors (82348 MB)
>>sda: assuming drive cache: write through
>>  sda:SCSI error : <0 0 0 0> return code = 0x70000
>>end_request: I/O error, dev sda, sector 160836480
>>Buffer I/O error on device sda, logical block 160836480
> 
> 
> Okay, so it looks like neither of the earlier suggestions will work.

Well, see my earlier email.  Let's not weed out descriptor type 3,
only 0 (which has no meaning as it is "reserved").  Looks like all
descriptors return _some_ kind of capacity -- let's see what it is
and possibly use it.  That is, since as you said Windows uses it...

> Here's one that I feel certain will work, although it takes a slightly 
> different approach.  Please try it out and see what happens.
> 
> Luben and Andries, what do you think of this patch?

Patch itself looks good.  But...

Brandishing all USB storage disk devices to have even number
capacity isn't something I think we can have in a kernel.  Or can we?

	Luben



> Alan Stern
> 
> 
> 
> Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> 
> ===== include/scsi/scsi_device.h 1.14 vs edited =====
> --- 1.14/include/scsi/scsi_device.h	2004-10-05 11:47:13 -04:00
> +++ edited/include/scsi/scsi_device.h	2004-10-25 15:10:48 -04:00
> @@ -111,6 +111,7 @@
>  	unsigned allow_restart:1; /* issue START_UNIT in error handler */
>  	unsigned no_uld_attach:1; /* disable connecting to upper level drivers */
>  	unsigned select_no_atn:1;
> +	unsigned even_capacity:1; /* must have an even number of sectors */
>  
>  	unsigned int device_blocked;	/* Device returned QUEUE_FULL. */
>  
> ===== drivers/scsi/sd.c 1.73 vs edited =====
> --- 1.73/drivers/scsi/sd.c	2004-10-20 12:24:44 -04:00
> +++ edited/drivers/scsi/sd.c	2004-10-25 15:19:33 -04:00
> @@ -1133,6 +1133,16 @@
>  		 */
>  		sector_size = 512;
>  	}
> +
> +	/* Handle broken devices that return the total number of sectors
> +	 * rather than the highest sector number, by forcing the total
> +	 * number to be even. */
> +	if (sdp->even_capacity && (sdkp->capacity & 1)) {
> +		printk(KERN_NOTICE "%s : odd number of sectors reported, "
> +			"decreasing by one.\n", diskname);
> +		--sdkp->capacity;
> +	}
> +
>  	{
>  		/*
>  		 * The msdos fs needs to know the hardware sector size
> ===== drivers/usb/storage/protocol.c 1.25 vs edited =====
> --- 1.25/drivers/usb/storage/protocol.c	2004-10-20 12:38:15 -04:00
> +++ edited/drivers/usb/storage/protocol.c	2004-10-25 15:23:32 -04:00
> @@ -57,35 +57,6 @@
>   * Helper routines
>   ***********************************************************************/
>  
> -/*
> - * Fix-up the return data from a READ CAPACITY command. My Feiya reader
> - * returns a value that is 1 too large.
> - */
> -static void fix_read_capacity(struct scsi_cmnd *srb)
> -{
> -	unsigned int index, offset;
> -	__be32 c;
> -	unsigned long capacity;
> -
> -	/* verify that it's a READ CAPACITY command */
> -	if (srb->cmnd[0] != READ_CAPACITY)
> -		return;
> -
> -	index = offset = 0;
> -	if (usb_stor_access_xfer_buf((unsigned char *) &c, 4, srb,
> -			&index, &offset, FROM_XFER_BUF) != 4)
> -		return;
> -
> -	capacity = be32_to_cpu(c);
> -	US_DEBUGP("US: Fixing capacity: from %ld to %ld\n",
> -	       capacity+1, capacity);
> -	c = cpu_to_be32(capacity - 1);
> -
> -	index = offset = 0;
> -	usb_stor_access_xfer_buf((unsigned char *) &c, 4, srb,
> -			&index, &offset, TO_XFER_BUF);
> -}
> -
>  /***********************************************************************
>   * Protocol routines
>   ***********************************************************************/
> @@ -174,12 +145,6 @@
>  {
>  	/* send the command to the transport layer */
>  	usb_stor_invoke_transport(srb, us);
> -
> -	if (srb->result == SAM_STAT_GOOD) {
> -		/* Fix the READ CAPACITY result if necessary */
> -		if (us->flags & US_FL_FIX_CAPACITY)
> -			fix_read_capacity(srb);
> -	}
>  }
>  
>  /***********************************************************************
> ===== drivers/usb/storage/scsiglue.c 1.86 vs edited =====
> --- 1.86/drivers/usb/storage/scsiglue.c	2004-09-30 16:07:33 -04:00
> +++ edited/drivers/usb/storage/scsiglue.c	2004-10-25 15:25:44 -04:00
> @@ -152,6 +152,12 @@
>  		sdev->skip_ms_page_3f = 1;
>  #endif
>  
> +		/* Some devices return the total number of blocks in
> +		 * response to READ CAPACITY instead of the highest
> +		 * block number.  Deal with this by forcing the number
> +		 * of blocks to be even.  Fortunately no existing USB
> +		 * drive has an odd number of blocks, so far as I know! */
> +		sdev->even_capacity = 1;
>  	} else {
>  
>  		/* Non-disk-type devices don't need to blacklist any pages
> @@ -390,7 +396,7 @@
>  		DO_FLAG(SINGLE_LUN);
>  		DO_FLAG(SCM_MULT_TARG);
>  		DO_FLAG(FIX_INQUIRY);
> -		DO_FLAG(FIX_CAPACITY);
> +		DO_FLAG(IGNORE_RESIDUE);
>  
>  		*(pos++) = '\n';
>  	}
> ===== drivers/usb/storage/unusual_devs.h 1.161 vs edited =====
> --- 1.161/drivers/usb/storage/unusual_devs.h	2004-10-20 12:38:15 -04:00
> +++ edited/drivers/usb/storage/unusual_devs.h	2004-10-25 15:24:29 -04:00
> @@ -171,16 +171,6 @@
>  		"CD-R/RW Drive",
>  		US_SC_8070, US_PR_CB, NULL, 0),
>  
> -/* Reported by Adriaan Penning <a.penning@luon.net>
> - * Note that these cameras report "Medium not present" after
> - * ALLOW_MEDIUM_REMOVAL, so they also need to be marked
> - * NOT_LOCKABLE in the SCSI blacklist (and the vendor is MATSHITA). */
> -UNUSUAL_DEV(  0x04da, 0x2372, 0x0000, 0x9999,
> -		"Panasonic",
> -		"DMC-LCx Camera",
> -		US_SC_DEVICE, US_PR_DEVICE, NULL,
> -		US_FL_FIX_CAPACITY ),
> -
>  /* Most of the following entries were developed with the help of
>   * Shuttle/SCM directly.
>   */
> @@ -483,13 +473,6 @@
>  		US_FL_SINGLE_LUN ),
>  #endif
>  
> -/* Reported by Darsen Lu <darsen@micro.ee.nthu.edu.tw> */
> -UNUSUAL_DEV( 0x066f, 0x8000, 0x0001, 0x0001,
> -		"SigmaTel",
> -		"USBMSC Audio Player",
> -		US_SC_DEVICE, US_PR_DEVICE, NULL,
> -		US_FL_FIX_CAPACITY ),
> -
>  /* Submitted by Benny Sjostrand <benny@hostmobility.com> */
>  UNUSUAL_DEV( 0x0686, 0x4011, 0x0001, 0x0001,
>  		"Minolta",
> @@ -547,13 +530,6 @@
>  		US_SC_QIC, US_PR_FREECOM, freecom_init, 0),
>  #endif
>  
> -/* Reported by Eero Volotinen <eero@ping-viini.org> */
> -UNUSUAL_DEV(  0x07ab, 0xfccd, 0x0406, 0x0406,
> -		"Freecom Technologies",
> -		"FHD-Classic",
> -		US_SC_DEVICE, US_PR_DEVICE, NULL,
> -		US_FL_FIX_CAPACITY),
> -
>  UNUSUAL_DEV(  0x07af, 0x0004, 0x0100, 0x0133, 
>  		"Microtech",
>  		"USB-SCSI-DB25",
> @@ -710,13 +686,6 @@
>  		"MP3 player",
>  		US_SC_RBC, US_PR_BULK, NULL,
>  		US_FL_MODE_XLATE),
> -
> -/* aeb */
> -UNUSUAL_DEV( 0x090c, 0x1132, 0x0000, 0xffff,
> -		"Feiya",
> -		"5-in-1 Card Reader",
> -		US_SC_DEVICE, US_PR_DEVICE, NULL,
> -		US_FL_FIX_CAPACITY ),
>  
>  UNUSUAL_DEV(  0x097a, 0x0001, 0x0000, 0x0001,
>  		"Minds@Work",
> ===== drivers/usb/storage/usb.h 1.64 vs edited =====
> --- 1.64/drivers/usb/storage/usb.h	2004-10-20 12:38:15 -04:00
> +++ edited/drivers/usb/storage/usb.h	2004-10-25 15:23:01 -04:00
> @@ -72,7 +72,7 @@
>  #define US_FL_IGNORE_SER      0		 /* [no longer used]		    */
>  #define US_FL_SCM_MULT_TARG   0x00000020 /* supports multiple targets	    */
>  #define US_FL_FIX_INQUIRY     0x00000040 /* INQUIRY response needs faking   */
> -#define US_FL_FIX_CAPACITY    0x00000080 /* READ CAPACITY response too big  */
> +#define US_FL_FIX_CAPACITY    0		 /* [no longer used]		    */
>  #define US_FL_IGNORE_RESIDUE  0x00000100 /* reported residue is wrong	    */
>  
>  /* Dynamic flag definitions: used in set_bit() etc. */
> 

Re: Handling erroneous READ CAPACITY response in sd.c

[Luben Tuikov]

[-- Attachment #1: Type: text/plain, Size: 495 bytes --]

Eero Volotinen wrote:
> 
>> Eero, can you try the attached patch?  I wonder if we
>> can take the maximum capacity media which could be supported?
>> (This is "patch 2", you still have to apply "patch 1" which
>> removes the broken devices from unusual devs.)
>>
>> Thanks,
>>     Luben
>>
> 
> Ok. I test your patch in a few days and report results then.

Forgot to add code to weed out the descriptor type 0 (reserved)
case.  Please try this (attached) version of the patch.

Thanks,
	Luben



[-- Attachment #2: cap.patch --]
[-- Type: application/octet-stream, Size: 5469 bytes --]

===== drivers/scsi/sd.c 1.161 vs edited =====
--- 1.161/drivers/scsi/sd.c	2004-10-15 10:46:07 -04:00
+++ edited/drivers/scsi/sd.c	2004-10-25 17:01:03 -04:00
@@ -987,6 +987,155 @@
 	}
 }
 
+/* sd_read_true_cap: Some device servers incorrectly return the
+ * capacity as opposed to the LBA of the last logical block of the
+ * block device.
+ *
+ * We try to fix this as follows: Let x = Returned LBA from the last
+ * READ CAPACITY command issued (result in "buffer").  Reissue the
+ * READ CAPACITY command as follows: set the partial medium indicator
+ * (PMI) bit to one; set the LBA to x - 1. Fire off that READ CAPACITY
+ * command.
+ *
+ * If we get success,
+ *       If Returned LBA > x - 1, then capacity is x+1, spec behavior.
+ *       Else Returned LBA <= x - 1, then capacity is x, broken device server.
+ * Else error, nothing can be assumed, capacity is x+1.
+ */
+static void sd_read_true_cap(struct scsi_disk *sd, char *diskname,
+			     struct scsi_request *SRpnt, unsigned char *buffer,
+			     int longrc)
+{
+	unsigned char cmd[16];
+	unsigned char buf[12];
+
+	/* save the old buffer contents here */
+	memcpy(buf, buffer, 12);
+
+	if (longrc) {
+		u64 *lba = (u64 *) (cmd+2);
+		u64 rlba;
+
+		memset((void *) cmd, 0, 16);
+		cmd[0] = SERVICE_ACTION_IN;
+		cmd[1] = SAI_READ_CAPACITY_16;
+		cmd[13] = 12;
+
+		rlba = be64_to_cpu(*(u64 *)buffer);
+		rlba -= 1;
+		*lba = cpu_to_be64(rlba);
+		/* turn on the PMI bit */
+		cmd[14] |= 1;
+		memset((void *) buffer, 0, 12);
+	} else {
+		u32 *lba = (u32 *) (cmd+2);
+		u32 rlba;
+
+		cmd[0] = READ_CAPACITY;
+		memset((void *) &cmd[1], 0, 9);
+		
+		rlba = be32_to_cpu(*(u32 *)buffer);
+		rlba -= 1;
+		*lba = cpu_to_be32(rlba);
+		/* turn on the PMI bit */
+		cmd[8] |= 1;
+		memset((void *) buffer, 0, 8);
+	}
+
+	SRpnt->sr_cmd_len = 0;
+	SRpnt->sr_sense_buffer[0] = 0;
+	SRpnt->sr_sense_buffer[2] = 0;
+	SRpnt->sr_data_direction = DMA_FROM_DEVICE;
+	
+	scsi_wait_req(SRpnt, (void *) cmd, (void *) buffer,
+		      longrc ? 12 : 8, SD_TIMEOUT, SD_MAX_RETRIES);
+
+	if (SRpnt->sr_result) {
+		/* Nothing can be assumed. */
+		printk(KERN_NOTICE "%s: %s: PMI not supported\n",
+		       __FUNCTION__, diskname);
+		memcpy(buffer, buf, 12);
+		return;
+	}
+
+	if (longrc) {
+		u64 rlba = be64_to_cpu(*(u64 *)buffer);
+		u64 x    = be64_to_cpu(*(u64 *)buf);
+		if (rlba > x - 1) {
+			goto out_spec;
+		}
+	} else {
+		u32 rlba = be32_to_cpu(*(u32 *)buffer);
+		u32 x    = be32_to_cpu(*(u32 *)buf);
+		if (rlba > x - 1) {
+			goto out_spec;
+		}
+	}
+	printk(KERN_NOTICE "%s: %s: broken device server\n", __FUNCTION__,
+	       diskname);
+	return;
+
+ out_spec:
+	/* Capacity is x+1, spec behavior. */
+	printk(KERN_NOTICE "%s: %s: spec behavior\n", __FUNCTION__, diskname);
+	memcpy(buffer, buf, 12);
+} /* end sd_read_true_cap() */
+
+
+/* Try to use the READ FORMAT CAPACITIES command to check whether the
+ * READ CAPACITY result is correct.
+ */
+static void sd_read_format_cap(struct scsi_disk *sd, char *diskname,
+		       struct scsi_request *SRpnt, unsigned char *buffer)
+{
+	unsigned char cmd[12];
+	unsigned char buf[12];
+	u32 rlba;
+	u32 x;
+
+	/* save the old buffer contents here */
+	memcpy(buf, buffer, 12);
+
+	rlba = be32_to_cpu(*(u32 *)buffer);
+
+	cmd[0] = 0x23;          /* READ_FORMAT_CAPACITIES */
+	memset(&cmd[1], 0, 11);
+	cmd[8] = 12;
+	memset(buffer, 0, 12);
+
+	SRpnt->sr_cmd_len = 0;
+	SRpnt->sr_sense_buffer[0] = 0;
+	SRpnt->sr_sense_buffer[2] = 0;
+	SRpnt->sr_data_direction = DMA_FROM_DEVICE;
+
+	scsi_wait_req(SRpnt, (void *) cmd, (void *) buffer,
+		      12, SD_TIMEOUT, SD_MAX_RETRIES);
+
+	if (SRpnt->sr_result) {
+		/* Nothing can be assumed. */
+		printk(KERN_NOTICE "%s: %s: READ FORMAT CAPACITIES "
+		       "not supported\n", __FUNCTION__, diskname);
+		goto done;
+	}
+
+	if ((buffer[8] & 3) == 0)
+		goto done;
+
+	x = be32_to_cpu(*(u32 *)(buffer + 4));
+	printk(KERN_NOTICE "%s: %s: %u vs. %u\n",
+	       __FUNCTION__, diskname, rlba, x);
+	if (0 < x && x <= rlba) {
+		u32 *lba = (u32 *) buf;
+		
+		printk(KERN_NOTICE "%s: %s: using lower capacity\n",
+		       __FUNCTION__, diskname);
+		x -= 1;
+		*lba = cpu_to_be32(x);
+	}
+ done:
+ 	memcpy(buffer, buf, 12);
+}
+
 /*
  * read disk capacity
  */
@@ -1070,7 +1219,14 @@
 		
 		sdkp->capacity = 1 + (sector_t) 0xffffffff;		
 		goto got_data;
-	}	
+	}
+
+	/* Check if the device reported CAPACITY as opposed to
+	 * the maxumum LBA (as per the SBC spec).
+	 */
+	sd_read_true_cap(sdkp, diskname, SRpnt, buffer, longrc);
+ 	if (!longrc)
+ 		sd_read_format_cap(sdkp, diskname, SRpnt, buffer);
 	
 	if (!longrc) {
 		sector_size = (buffer[4] << 24) |
@@ -1078,12 +1234,14 @@
 		if (buffer[0] == 0xff && buffer[1] == 0xff &&
 		    buffer[2] == 0xff && buffer[3] == 0xff) {
 			if(sizeof(sdkp->capacity) > 4) {
-				printk(KERN_NOTICE "%s : very big device. try to use"
-				       " READ CAPACITY(16).\n", diskname);
+				printk(KERN_NOTICE "%s : very big device. "
+				       "try to use READ CAPACITY(16).\n",
+				       diskname);
 				longrc = 1;
 				goto repeat;
 			} else {
-				printk(KERN_ERR "%s: too big for kernel.  Assuming maximum 2Tb\n", diskname);
+				printk(KERN_ERR "%s: too big for kernel. "
+				       "Assuming maximum 2Tb\n", diskname);
 			}
 		}
 		sdkp->capacity = 1 + (((sector_t)buffer[0] << 24) |
@@ -1102,7 +1260,7 @@
 			
 		sector_size = (buffer[8] << 24) |
 			(buffer[9] << 16) | (buffer[10] << 8) | buffer[11];
-	}	
+	}
 
 got_data:
 	if (sector_size == 0) {

Re: Handling erroneous READ CAPACITY response in sd.c

[Alan Stern]

Everything we've tried for detecting too-high-by-one READ CAPACITY
responses has failed on one device or another.  I can't think of any way
to detect the error at runtime, short of actually trying to read the last
sector.  And that is not a good tactic because it will quite likely leave
the device is some non-resettable hung-up state.  Even the heuristic that
USB devices will always have an even number of sectors turns out to be
wrong for the Apple iPod.

There doesn't seem to be anything left but blacklisting.  This could be
done at the SCSI level or the usb-storage level.  It turns out there's
actually an advantage to putting the entries in the usb-storage list:  In
many cases a vendor will use a USB interface chip from another company,
changing the vendor name stored in the INQUIRY data but leaving the USB
vendor ID alone (so it reflects the chip designer, not the drive seller).  
This means that a single USB blacklist could cover a range of devices that
would require multiple SCSI blacklist entries.  I've actually seen
something like this happen in a different context (the Panasonic DMC-LCx
cameras with internal interfaces by Matsushita have different Product
strings in their INQUIRY data but the same Product ID in their USB
descriptors).

On the other hand, the adjustment of the READ CAPACITY value should still 
be moved to sd.c from usb-storage.  This can be implemented easily enough 
by adding a bitflag to the scsi_device structure; the flag could be set 
either by a SCSI blist entry or by usb-storage in its slave_configure 
routine.  The flag would tell sd_read_capacity to decrement the number of 
sectors it receives from the device.

Can anyone come up with a better suggestion?

Alan Stern

Re: Handling erroneous READ CAPACITY response in sd.c

[Matthew Dharm]

[-- Attachment #1: Type: text/plain, Size: 957 bytes --]

On Fri, Nov 05, 2004 at 11:18:34AM -0500, Alan Stern wrote:
> On the other hand, the adjustment of the READ CAPACITY value should still 
> be moved to sd.c from usb-storage.  This can be implemented easily enough 
> by adding a bitflag to the scsi_device structure; the flag could be set 
> either by a SCSI blist entry or by usb-storage in its slave_configure 
> routine.  The flag would tell sd_read_capacity to decrement the number of 
> sectors it receives from the device.

If I read this right, you're saying we should flag the device, but not
actually tweak the READ_CAPACITY data?  In other words we get rid of the
code which tries to munge the data in-flight?

Matt

-- 
Matthew Dharm                              Home: mdharm-usb@one-eyed-alien.net 
Maintainer, Linux USB Mass Storage Driver

It's not that hard.  No matter what the problem is, tell the customer 
to reinstall Windows.
					-- Nurse
User Friendly, 3/22/1998

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: Handling erroneous READ CAPACITY response in sd.c

[Alan Stern]

On Fri, 5 Nov 2004, Matthew Dharm wrote:

> On Fri, Nov 05, 2004 at 11:18:34AM -0500, Alan Stern wrote:
> > On the other hand, the adjustment of the READ CAPACITY value should still 
> > be moved to sd.c from usb-storage.  This can be implemented easily enough 
> > by adding a bitflag to the scsi_device structure; the flag could be set 
> > either by a SCSI blist entry or by usb-storage in its slave_configure 
> > routine.  The flag would tell sd_read_capacity to decrement the number of 
> > sectors it receives from the device.
> 
> If I read this right, you're saying we should flag the device, but not
> actually tweak the READ_CAPACITY data?  In other words we get rid of the
> code which tries to munge the data in-flight?

Right.  Let usb-storage transport the data unchanged, and let the user of 
the data (sd.c in this case) decide what to do about it.

Alan Stern

Re: [usb-storage] Re: Handling erroneous READ CAPACITY response in sd.c

[Andries Brouwer]

> On the other hand, the adjustment of the READ CAPACITY value should still 
> be moved to sd.c from usb-storage.  This can be implemented easily enough 
> by adding a bitflag to the scsi_device structure; the flag could be set 
> either by a SCSI blist entry or by usb-storage in its slave_configure 
> routine.  The flag would tell sd_read_capacity to decrement the number of 
> sectors it receives from the device.

I have not been following the discussion but happen to see this
fragment. Probably the below is irrelevant.

We used to have entries in unusual_devices - the above sounds
as if it is proposed to move such entries to a SCSI blist.
I don't see why that is progress.

Concerning the size of a disk: if one claims that the disk is one
sector shorter than it really is, in most cases no great harm is done.
However, certain partition tables and RAID configuration info
lives on the last sector or the last cylinder of a disk.
Even worse, sometimes this information is read or written by
the hardware.
Thus, there exist situations where it is really bad when
the last sector of a disk disappears from sight.

Andries

Re: [usb-storage] Re: Handling erroneous READ CAPACITY response in sd.c

[Alan Stern]

On Fri, 5 Nov 2004, Andries Brouwer wrote:

> I have not been following the discussion but happen to see this
> fragment. Probably the below is irrelevant.

As you may have surmised, this discussion concerns how we should handle 
devices that return the total number of sectors for READ CAPACITY rather 
than the highest sector number.

> We used to have entries in unusual_devices - the above sounds
> as if it is proposed to move such entries to a SCSI blist.
> I don't see why that is progress.

No.  I said that it _could_ be moved to a SCSI blist but would probably be
better off remaining in unusual_devices.  What should be changed is the
way we handle these devices.  Right now usb-storage alters the data sent
by the device, before anyone else can see it.  My proposal is that
usb-storage pass the data unchanged and sd_read_capacity() change the
value it gets if the device is known to be bad.

> Concerning the size of a disk: if one claims that the disk is one
> sector shorter than it really is, in most cases no great harm is done.
> However, certain partition tables and RAID configuration info
> lives on the last sector or the last cylinder of a disk.
> Even worse, sometimes this information is read or written by
> the hardware.
> Thus, there exist situations where it is really bad when
> the last sector of a disk disappears from sight.

That is precisely why we need to recognize which devices do this wrongly 
and handle them specially while leaving everything else unchanged.

Alan Stern

Re: [usb-storage] Re: Handling erroneous READ CAPACITY response in sd.c

[Andries Brouwer]

On Fri, Nov 05, 2004 at 04:38:07PM -0500, Alan Stern wrote:

> No.  I said that it _could_ be moved to a SCSI blist but would probably be
> better off remaining in unusual_devices.  What should be changed is the
> way we handle these devices.  Right now usb-storage alters the data sent
> by the device, before anyone else can see it.  My proposal is that
> usb-storage pass the data unchanged and sd_read_capacity() change the
> value it gets if the device is known to be bad.

OK - yes, I agree.

Andries

Re: Handling erroneous READ CAPACITY response in sd.c

[Luben Tuikov]

Alan Stern wrote:
> Everything we've tried for detecting too-high-by-one READ CAPACITY
> responses has failed on one device or another.  I can't think of any way
> to detect the error at runtime, short of actually trying to read the last
> sector.  And that is not a good tactic because it will quite likely leave
> the device is some non-resettable hung-up state.  Even the heuristic that
> USB devices will always have an even number of sectors turns out to be
> wrong for the Apple iPod.
> 
> There doesn't seem to be anything left but blacklisting.  This could be
> done at the SCSI level or the usb-storage level.  It turns out there's
> actually an advantage to putting the entries in the usb-storage list:  In
> many cases a vendor will use a USB interface chip from another company,
> changing the vendor name stored in the INQUIRY data but leaving the USB
> vendor ID alone (so it reflects the chip designer, not the drive seller).  
> This means that a single USB blacklist could cover a range of devices that
> would require multiple SCSI blacklist entries.  I've actually seen
> something like this happen in a different context (the Panasonic DMC-LCx
> cameras with internal interfaces by Matsushita have different Product
> strings in their INQUIRY data but the same Product ID in their USB
> descriptors).
> 
> On the other hand, the adjustment of the READ CAPACITY value should still 
> be moved to sd.c from usb-storage.  This can be implemented easily enough 
> by adding a bitflag to the scsi_device structure; the flag could be set 
> either by a SCSI blist entry or by usb-storage in its slave_configure 
> routine.  The flag would tell sd_read_capacity to decrement the number of 
> sectors it receives from the device.

This sounds good.  Just to clarify: if we blacklist in usb-storage, then
I think there's no need to also blacklist in SCSI Core as well.  Let sd
use the flag to decrement the Returned LBA (effectively assign Returned
LBA to capacity rather than increment it and then assign it.)

	Luben

Re: Handling erroneous READ CAPACITY response in sd.c

[Alan Stern]

On Mon, 8 Nov 2004, Luben Tuikov wrote:

> This sounds good.  Just to clarify: if we blacklist in usb-storage, then
> I think there's no need to also blacklist in SCSI Core as well.  Let sd
> use the flag to decrement the Returned LBA (effectively assign Returned
> LBA to capacity rather than increment it and then assign it.)

That's right.  Below is the patch.  I'm never quite sure who these things 
should be submitted to because they apply to both the USB and SCSI 
subsystems.  But if it looks good to both of you, I can send it on to 
James Bottomley.

Alan Stern



Signed-off-by: Alan Stern <stern@rowland.harvard.edu>

===== include/scsi/scsi_device.h 1.14 vs edited =====
--- 1.14/include/scsi/scsi_device.h	2004-10-05 11:47:13 -04:00
+++ edited/include/scsi/scsi_device.h	2004-11-08 15:42:52 -05:00
@@ -111,6 +111,7 @@
 	unsigned allow_restart:1; /* issue START_UNIT in error handler */
 	unsigned no_uld_attach:1; /* disable connecting to upper level drivers */
 	unsigned select_no_atn:1;
+	unsigned fix_capacity:1;	/* READ_CAPACITY is too high by 1 */
 
 	unsigned int device_blocked;	/* Device returned QUEUE_FULL. */
 
===== drivers/scsi/sd.c 1.73 vs edited =====
--- 1.73/drivers/scsi/sd.c	2004-10-20 12:24:44 -04:00
+++ edited/drivers/scsi/sd.c	2004-11-08 15:42:37 -05:00
@@ -1104,6 +1104,11 @@
 			(buffer[9] << 16) | (buffer[10] << 8) | buffer[11];
 	}	
 
+	/* Some devices return the total number of sectors, not the
+	 * highest sector number.  Make the necessary adjustment. */
+	if (sdp->fix_capacity)
+		--sdkp->capacity;
+
 got_data:
 	if (sector_size == 0) {
 		sector_size = 512;
===== drivers/usb/storage/protocol.c 1.25 vs edited =====
--- 1.25/drivers/usb/storage/protocol.c	2004-10-20 12:38:15 -04:00
+++ edited/drivers/usb/storage/protocol.c	2004-11-08 15:37:22 -05:00
@@ -54,39 +54,6 @@
 #include "transport.h"
 
 /***********************************************************************
- * Helper routines
- ***********************************************************************/
-
-/*
- * Fix-up the return data from a READ CAPACITY command. My Feiya reader
- * returns a value that is 1 too large.
- */
-static void fix_read_capacity(struct scsi_cmnd *srb)
-{
-	unsigned int index, offset;
-	__be32 c;
-	unsigned long capacity;
-
-	/* verify that it's a READ CAPACITY command */
-	if (srb->cmnd[0] != READ_CAPACITY)
-		return;
-
-	index = offset = 0;
-	if (usb_stor_access_xfer_buf((unsigned char *) &c, 4, srb,
-			&index, &offset, FROM_XFER_BUF) != 4)
-		return;
-
-	capacity = be32_to_cpu(c);
-	US_DEBUGP("US: Fixing capacity: from %ld to %ld\n",
-	       capacity+1, capacity);
-	c = cpu_to_be32(capacity - 1);
-
-	index = offset = 0;
-	usb_stor_access_xfer_buf((unsigned char *) &c, 4, srb,
-			&index, &offset, TO_XFER_BUF);
-}
-
-/***********************************************************************
  * Protocol routines
  ***********************************************************************/
 
@@ -174,12 +141,6 @@
 {
 	/* send the command to the transport layer */
 	usb_stor_invoke_transport(srb, us);
-
-	if (srb->result == SAM_STAT_GOOD) {
-		/* Fix the READ CAPACITY result if necessary */
-		if (us->flags & US_FL_FIX_CAPACITY)
-			fix_read_capacity(srb);
-	}
 }
 
 /***********************************************************************
===== drivers/usb/storage/scsiglue.c 1.87 vs edited =====
--- 1.87/drivers/usb/storage/scsiglue.c	2004-11-01 13:59:21 -05:00
+++ edited/drivers/usb/storage/scsiglue.c	2004-11-08 15:43:05 -05:00
@@ -152,6 +152,11 @@
 		sdev->skip_ms_page_3f = 1;
 #endif
 
+		/* Some disks return the total number blocks in response
+		 * to READ CAPACITY rather than the highest block number.
+		 * If this device makes that mistake, tell the sd driver. */
+		if (us->flags & US_FL_FIX_CAPACITY)
+			sdev->fix_capacity = 1;
 	} else {
 
 		/* Non-disk-type devices don't need to blacklist any pages

Re: Handling erroneous READ CAPACITY response in sd.c

[Luben Tuikov]

Alan Stern wrote:
> On Mon, 8 Nov 2004, Luben Tuikov wrote:
> 
> 
>>This sounds good.  Just to clarify: if we blacklist in usb-storage, then
>>I think there's no need to also blacklist in SCSI Core as well.  Let sd
>>use the flag to decrement the Returned LBA (effectively assign Returned
>>LBA to capacity rather than increment it and then assign it.)
> 
> 
> That's right.  Below is the patch.  I'm never quite sure who these things 

Looks good to me.

	Luben

> should be submitted to because they apply to both the USB and SCSI 
> subsystems.  But if it looks good to both of you, I can send it on to 
> James Bottomley.
> 
> Alan Stern
> 
> 
> 
> Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> 
> ===== include/scsi/scsi_device.h 1.14 vs edited =====
> --- 1.14/include/scsi/scsi_device.h	2004-10-05 11:47:13 -04:00
> +++ edited/include/scsi/scsi_device.h	2004-11-08 15:42:52 -05:00
> @@ -111,6 +111,7 @@
>  	unsigned allow_restart:1; /* issue START_UNIT in error handler */
>  	unsigned no_uld_attach:1; /* disable connecting to upper level drivers */
>  	unsigned select_no_atn:1;
> +	unsigned fix_capacity:1;	/* READ_CAPACITY is too high by 1 */
>  
>  	unsigned int device_blocked;	/* Device returned QUEUE_FULL. */
>  
> ===== drivers/scsi/sd.c 1.73 vs edited =====
> --- 1.73/drivers/scsi/sd.c	2004-10-20 12:24:44 -04:00
> +++ edited/drivers/scsi/sd.c	2004-11-08 15:42:37 -05:00
> @@ -1104,6 +1104,11 @@
>  			(buffer[9] << 16) | (buffer[10] << 8) | buffer[11];
>  	}	
>  
> +	/* Some devices return the total number of sectors, not the
> +	 * highest sector number.  Make the necessary adjustment. */
> +	if (sdp->fix_capacity)
> +		--sdkp->capacity;
> +
>  got_data:
>  	if (sector_size == 0) {
>  		sector_size = 512;
> ===== drivers/usb/storage/protocol.c 1.25 vs edited =====
> --- 1.25/drivers/usb/storage/protocol.c	2004-10-20 12:38:15 -04:00
> +++ edited/drivers/usb/storage/protocol.c	2004-11-08 15:37:22 -05:00
> @@ -54,39 +54,6 @@
>  #include "transport.h"
>  
>  /***********************************************************************
> - * Helper routines
> - ***********************************************************************/
> -
> -/*
> - * Fix-up the return data from a READ CAPACITY command. My Feiya reader
> - * returns a value that is 1 too large.
> - */
> -static void fix_read_capacity(struct scsi_cmnd *srb)
> -{
> -	unsigned int index, offset;
> -	__be32 c;
> -	unsigned long capacity;
> -
> -	/* verify that it's a READ CAPACITY command */
> -	if (srb->cmnd[0] != READ_CAPACITY)
> -		return;
> -
> -	index = offset = 0;
> -	if (usb_stor_access_xfer_buf((unsigned char *) &c, 4, srb,
> -			&index, &offset, FROM_XFER_BUF) != 4)
> -		return;
> -
> -	capacity = be32_to_cpu(c);
> -	US_DEBUGP("US: Fixing capacity: from %ld to %ld\n",
> -	       capacity+1, capacity);
> -	c = cpu_to_be32(capacity - 1);
> -
> -	index = offset = 0;
> -	usb_stor_access_xfer_buf((unsigned char *) &c, 4, srb,
> -			&index, &offset, TO_XFER_BUF);
> -}
> -
> -/***********************************************************************
>   * Protocol routines
>   ***********************************************************************/
>  
> @@ -174,12 +141,6 @@
>  {
>  	/* send the command to the transport layer */
>  	usb_stor_invoke_transport(srb, us);
> -
> -	if (srb->result == SAM_STAT_GOOD) {
> -		/* Fix the READ CAPACITY result if necessary */
> -		if (us->flags & US_FL_FIX_CAPACITY)
> -			fix_read_capacity(srb);
> -	}
>  }
>  
>  /***********************************************************************
> ===== drivers/usb/storage/scsiglue.c 1.87 vs edited =====
> --- 1.87/drivers/usb/storage/scsiglue.c	2004-11-01 13:59:21 -05:00
> +++ edited/drivers/usb/storage/scsiglue.c	2004-11-08 15:43:05 -05:00
> @@ -152,6 +152,11 @@
>  		sdev->skip_ms_page_3f = 1;
>  #endif
>  
> +		/* Some disks return the total number blocks in response
> +		 * to READ CAPACITY rather than the highest block number.
> +		 * If this device makes that mistake, tell the sd driver. */
> +		if (us->flags & US_FL_FIX_CAPACITY)
> +			sdev->fix_capacity = 1;
>  	} else {
>  
>  		/* Non-disk-type devices don't need to blacklist any pages
> 

Re: Handling erroneous READ CAPACITY response in sd.c

[Matthew Dharm]

[-- Attachment #1: Type: text/plain, Size: 1110 bytes --]

On Mon, Nov 08, 2004 at 04:03:14PM -0500, Alan Stern wrote:
> On Mon, 8 Nov 2004, Luben Tuikov wrote:
> 
> > This sounds good.  Just to clarify: if we blacklist in usb-storage, then
> > I think there's no need to also blacklist in SCSI Core as well.  Let sd
> > use the flag to decrement the Returned LBA (effectively assign Returned
> > LBA to capacity rather than increment it and then assign it.)
> 
> That's right.  Below is the patch.  I'm never quite sure who these things 
> should be submitted to because they apply to both the USB and SCSI 
> subsystems.  But if it looks good to both of you, I can send it on to 
> James Bottomley.

Send the SCSI parts to James, I think.  Once they're in, we'll put in the
USB parts.

After this patch, are there any other users of usb_stor_access_xfer_buf()?

Matt

-- 
Matthew Dharm                              Home: mdharm-usb@one-eyed-alien.net 
Maintainer, Linux USB Mass Storage Driver

S:  Another stupid question?
G:  There's no such thing as a stupid question, only stupid people.
					-- Stef and Greg
User Friendly, 7/15/1998

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: Handling erroneous READ CAPACITY response in sd.c

[Alan Stern]

On Mon, 8 Nov 2004, Matthew Dharm wrote:

> Send the SCSI parts to James, I think.  Once they're in, we'll put in the
> USB parts.
> 
> After this patch, are there any other users of usb_stor_access_xfer_buf()?

Loads of 'em.  Just do a grep and you'll see.

Alan Stern