public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
* adding flag for 192 byte mode sense
@ 2004-03-14  1:46 Patrick Mansfield
  2004-03-14  1:49 ` [PATCH] Replace scsi_host flags with scsi_device sdev_bflags Patrick Mansfield
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Patrick Mansfield @ 2004-03-14  1:46 UTC (permalink / raw)
  To: stern, mdharm-usb, James Bottomley, linux-scsi, usb-storage

Following are patches to support a flag for a 192 byte MODE SENSE, for use
by USB.

First patch replaces the scsi_host flags with a per sdev bflag, and sets
values in a new USB slave_alloc rather than the scsi_host template. I
grepped code for other users of scsi_host flags, and did not find any, and
do not recall seeing any added (flags is a bad name). slave_alloc is used
so that if needed the devinfo code can overwrite any settings.

Second patch adds the 192 flag.

With both patches we should get the exact behaviour we have now.

USB patches are still needed to set the BLIST_MS_192_BYTES_FOR_3F flag,
and to conditionally use the BLIST_USE_10_BYTE_MS based on the transparent
scsi versus all others. Alan or Matthew - can you send patches for that?

The scsi core usage of use_10_for_ms might need to change to behave the
way usb wants, though it probably will not matter. It currently demotes
from 10 byte to 6 byte if there is an illegal request on the 10 byte
command. Based on Alan's information, it sounds like USB wants the
commands to always be 6 or 10 bytes, with no change on an illegal request.

I tested with these usb mass storage devices: sony camera, olympus camera,
and dazzle card reader with no problems. I thought the sony used to choke
on the MODE SENSE, but it worked for both the short 8 byte and long 192
byte requests (did get sense key Illegal Request but no hang on allow
medium removal).

Some log output with command logging (sysctl -w dev.scsi.logging_level=0x2400)
on as follows.

Default USB behaviour, no MODE SENSE is sent:

kernel: scsi <1:0:0:0> send                  Inquiry 00 00 00 24 00 
kernel: scsi <1:0:0:0> done SUCCESS        0 Inquiry 00 00 00 24 00 
kernel:   Vendor: OLYMPUS   Model: C750UZ            Rev: 1.00
kernel:   Type:   Direct-Access                      ANSI SCSI revision: 02
kernel: scsi <1:0:0:0> send                  Test Unit Ready 00 00 00 00 00 
kernel: scsi <1:0:0:0> done SUCCESS        0 Test Unit Ready 00 00 00 00 00 
kernel: scsi <1:0:0:0> send                  Read Capacity 00 00 00 00 00 00 00 00 00 
kernel: scsi <1:0:0:0> done SUCCESS        0 Read Capacity 00 00 00 00 00 00 00 00 00 
kernel: SCSI device sda: 512000 512-byte hdwr sectors (262 MB)
kernel: sda: assuming Write Enabled
kernel: sda: assuming drive cache: write through
kernel: scsi <1:0:0:0> send                  Test Unit Ready 00 00 00 00 00 
kernel: scsi <1:0:0:0> done SUCCESS        0 Test Unit Ready 00 00 00 00 00 
kernel: scsi <1:0:0:0> send                  Prevent/Allow Medium Removal 00 00 00 01 00 
kernel: scsi <1:0:0:0> done SUCCESS        0 Prevent/Allow Medium Removal 00 00 00 01 00 
kernel:  sda:<6>scsi <1:0:0:0> send                  Read (10) 00 00 00 00 00 00 00 08 00 
kernel: scsi <1:0:0:0> done SUCCESS        0 Read (10) 00 00 00 00 00 00 00 08 00 
kernel:  sda1
kernel: scsi <1:0:0:0> send                  Prevent/Allow Medium Removal 00 00 00 00 00 
kernel: scsi <1:0:0:0> done SUCCESS        0 Prevent/Allow Medium Removal 00 00 00 00 00 

Command logs using 192 (0xc0) byte MODE SENSE - tested via addition of
a devinfo entry using:

	echo "OLYMPUS:C750UZ:0x1a000" > /proc/scsi/device_info


kernel: scsi <1:0:0:0> send                  Inquiry 00 00 00 24 00 
kernel: scsi <1:0:0:0> done SUCCESS        0 Inquiry 00 00 00 24 00 
kernel:   Vendor: OLYMPUS   Model: C750UZ            Rev: 1.00
kernel:   Type:   Direct-Access                      ANSI SCSI revision: 02
kernel: scsi <1:0:0:0> send                  Test Unit Ready 00 00 00 00 00 
kernel: scsi <1:0:0:0> done SUCCESS        0 Test Unit Ready 00 00 00 00 00 
kernel: scsi <1:0:0:0> send                  Read Capacity 00 00 00 00 00 00 00 00 00 
kernel: scsi <1:0:0:0> done SUCCESS        0 Read Capacity 00 00 00 00 00 00 00 00 00 
kernel: SCSI device sda: 512000 512-byte hdwr sectors (262 MB)
kernel: scsi <1:0:0:0> send                  Mode Sense (10) 00 3f 00 00 00 00 00 c0 00 
kernel: scsi <1:0:0:0> done SUCCESS        0 Mode Sense (10) 00 3f 00 00 00 00 00 c0 00 
kernel: sda: Write Protect is off
kernel: sda: assuming drive cache: write through
kernel: scsi <1:0:0:0> send                  Test Unit Ready 00 00 00 00 00 
kernel: scsi <1:0:0:0> done SUCCESS        0 Test Unit Ready 00 00 00 00 00 
kernel: scsi <1:0:0:0> send                  Prevent/Allow Medium Removal 00 00 00 01 00 
kernel: scsi <1:0:0:0> done SUCCESS        0 Prevent/Allow Medium Removal 00 00 00 01 00 
kernel:  sda:<6>scsi <1:0:0:0> send                  Read (10) 00 00 00 00 00 00 00 08 00 
kernel: scsi <1:0:0:0> done SUCCESS        0 Read (10) 00 00 00 00 00 00 00 08 00 
kernel:  sda1
kernel: scsi <1:0:0:0> send                  Prevent/Allow Medium Removal 00 00 00 00 00 
kernel: scsi <1:0:0:0> done SUCCESS        0 Prevent/Allow Medium Removal 00 00 00 00 00 

-- Patrick Mansfield

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] Replace scsi_host flags with scsi_device sdev_bflags
  2004-03-14  1:46 adding flag for 192 byte mode sense Patrick Mansfield
@ 2004-03-14  1:49 ` Patrick Mansfield
  2004-03-14  1:50   ` [PATCH] Add 192 byte MODE SENSE flag Patrick Mansfield
  2004-03-16 17:25 ` adding flag for 192 byte mode sense Alan Stern
  2004-03-22 16:16 ` Alan Stern
  2 siblings, 1 reply; 10+ messages in thread
From: Patrick Mansfield @ 2004-03-14  1:49 UTC (permalink / raw)
  To: stern, mdharm-usb, James Bottomley, linux-scsi, usb-storage

Replace the scsi_host flags with scsi_device sdev_bflags. Change USB to
set sdev_bflags in its new slave_alloc function.

diff -uprN -X /home/patman/dontdiff base-2.6/drivers/scsi/scsi_devinfo.c add_sdev_bflags-2.6/drivers/scsi/scsi_devinfo.c
--- base-2.6/drivers/scsi/scsi_devinfo.c	Fri Mar 12 16:07:10 2004
+++ add_sdev_bflags-2.6/drivers/scsi/scsi_devinfo.c	Fri Mar 12 16:16:47 2004
@@ -329,7 +329,7 @@ int scsi_get_device_flags(struct scsi_de
 	struct scsi_dev_info_list *devinfo;
 	unsigned int bflags;
 
-	bflags = sdev->host->hostt->flags;
+	bflags = sdev->sdev_bflags;
 	if (!bflags)
 		bflags = scsi_default_dev_flags;
 
diff -uprN -X /home/patman/dontdiff base-2.6/drivers/usb/storage/scsiglue.c add_sdev_bflags-2.6/drivers/usb/storage/scsiglue.c
--- base-2.6/drivers/usb/storage/scsiglue.c	Thu Mar 11 11:23:05 2004
+++ add_sdev_bflags-2.6/drivers/usb/storage/scsiglue.c	Fri Mar 12 16:16:47 2004
@@ -64,6 +64,17 @@ static const char* host_info(struct Scsi
 	return "SCSI emulation for USB Mass Storage devices";
 }
 
+static int slave_alloc (struct scsi_device *sdev)
+{
+	/*
+	 * Set default bflags. These can be overridden for individual
+	 * models and vendors via the scsi devinfo mechanism.
+	 */
+	sdev->sdev_bflags = (BLIST_MS_SKIP_PAGE_08 | BLIST_MS_SKIP_PAGE_3F |
+			     BLIST_USE_10_BYTE_MS);
+	return 0;
+}
+
 static int slave_configure (struct scsi_device *sdev)
 {
 	/* Scatter-gather buffers (all but the last) must have a length
@@ -365,6 +376,7 @@ struct scsi_host_template usb_stor_host_
 	/* unknown initiator id */
 	.this_id =			-1,
 
+	.slave_alloc =			slave_alloc,
 	.slave_configure =		slave_configure,
 
 	/* lots of sg segments can be handled */
@@ -384,10 +396,6 @@ struct scsi_host_template usb_stor_host_
 
 	/* sysfs device attributes */
 	.sdev_attrs =			sysfs_device_attr_list,
-
-	/* modify scsi_device bits on probe */
-	.flags = (BLIST_MS_SKIP_PAGE_08 | BLIST_MS_SKIP_PAGE_3F |
-		  BLIST_USE_10_BYTE_MS),
 
 	/* module management */
 	.module =			THIS_MODULE
diff -uprN -X /home/patman/dontdiff base-2.6/include/scsi/scsi_device.h add_sdev_bflags-2.6/include/scsi/scsi_device.h
--- base-2.6/include/scsi/scsi_device.h	Fri Mar 12 16:07:11 2004
+++ add_sdev_bflags-2.6/include/scsi/scsi_device.h	Fri Mar 12 16:16:47 2004
@@ -64,6 +64,10 @@ struct scsi_device {
 	unsigned char current_tag;	/* current tag */
 	struct scsi_target      *sdev_target;   /* used only for single_lun */
 
+	unsigned int	sdev_bflags; /* black/white flags as also found in
+				 * scsi_devinfo.[hc]. For now used only to
+				 * pass settings from slave_alloc to scsi
+				 * core. */
 	unsigned online:1;
 
 	unsigned writeable:1;
diff -uprN -X /home/patman/dontdiff base-2.6/include/scsi/scsi_host.h add_sdev_bflags-2.6/include/scsi/scsi_host.h
--- base-2.6/include/scsi/scsi_host.h	Fri Mar 12 16:07:11 2004
+++ add_sdev_bflags-2.6/include/scsi/scsi_host.h	Fri Mar 12 16:16:47 2004
@@ -345,12 +345,6 @@ struct scsi_host_template {
 	 * module_init/module_exit.
 	 */
 	struct list_head legacy_hosts;
-
-	/*
-	 * Default flags settings, these modify the setting of scsi_device
-	 * bits.
-	 */
-	unsigned int flags;
 };
 
 /*

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] Add 192 byte MODE SENSE flag
  2004-03-14  1:49 ` [PATCH] Replace scsi_host flags with scsi_device sdev_bflags Patrick Mansfield
@ 2004-03-14  1:50   ` Patrick Mansfield
  0 siblings, 0 replies; 10+ messages in thread
From: Patrick Mansfield @ 2004-03-14  1:50 UTC (permalink / raw)
  To: stern, mdharm-usb, James Bottomley, linux-scsi, usb-storage

Add a BLIST_MS_192_BYTES_FOR_3F flag. If it is set, sends a 192 byte MODE
SENSE in sd.c.

diff -uprN -X /home/patman/dontdiff add_sdev_bflags-2.6/drivers/scsi/scsi_scan.c 192_byte_ms-2.6/drivers/scsi/scsi_scan.c
--- add_sdev_bflags-2.6/drivers/scsi/scsi_scan.c	Fri Mar 12 16:07:10 2004
+++ 192_byte_ms-2.6/drivers/scsi/scsi_scan.c	Fri Mar 12 16:17:55 2004
@@ -643,6 +643,9 @@ static int scsi_add_lun(struct scsi_devi
 	if (*bflags & BLIST_USE_10_BYTE_MS)
 		sdev->use_10_for_ms = 1;
 
+	if (*bflags & BLIST_MS_192_BYTES_FOR_3F)
+		sdev->use_192_bytes_for_3f = 1;
+
 	if(sdev->host->hostt->slave_configure)
 		sdev->host->hostt->slave_configure(sdev);
 
diff -uprN -X /home/patman/dontdiff add_sdev_bflags-2.6/drivers/scsi/sd.c 192_byte_ms-2.6/drivers/scsi/sd.c
--- add_sdev_bflags-2.6/drivers/scsi/sd.c	Fri Mar 12 16:07:10 2004
+++ 192_byte_ms-2.6/drivers/scsi/sd.c	Fri Mar 12 16:17:55 2004
@@ -1122,26 +1122,32 @@ sd_read_write_protect_flag(struct scsi_d
 		return;
 	}
 
-	/*
-	 * First attempt: ask for all pages (0x3F), but only 4 bytes.
-	 * We have to start carefully: some devices hang if we ask
-	 * for more than is available.
-	 */
-	res = sd_do_mode_sense(SRpnt, 0, 0x3F, buffer, 4, &data);
+	if (sdkp->device->use_192_bytes_for_3f) {
+		res = sd_do_mode_sense(SRpnt, 0, 0x3F, buffer, 192, &data);
+	} else {
+		/*
+		 * First attempt: ask for all pages (0x3F), but only 4 bytes.
+		 * We have to start carefully: some devices hang if we ask
+		 * for more than is available.
+		 */
+		res = sd_do_mode_sense(SRpnt, 0, 0x3F, buffer, 4, &data);
 
-	/*
-	 * Second attempt: ask for page 0
-	 * When only page 0 is implemented, a request for page 3F may return
-	 * Sense Key 5: Illegal Request, Sense Code 24: Invalid field in CDB.
-	 */
-	if (!scsi_status_is_good(res))
-		res = sd_do_mode_sense(SRpnt, 0, 0, buffer, 4, &data);
+		/*
+		 * Second attempt: ask for page 0 When only page 0 is
+		 * implemented, a request for page 3F may return Sense Key
+		 * 5: Illegal Request, Sense Code 24: Invalid field in
+		 * CDB.
+		 */
+		if (!scsi_status_is_good(res))
+			res = sd_do_mode_sense(SRpnt, 0, 0, buffer, 4, &data);
 
-	/*
-	 * Third attempt: ask 255 bytes, as we did earlier.
-	 */
-	if (!scsi_status_is_good(res))
-		res = sd_do_mode_sense(SRpnt, 0, 0x3F, buffer, 255, &data);
+		/*
+		 * Third attempt: ask 255 bytes, as we did earlier.
+		 */
+		if (!scsi_status_is_good(res))
+			res = sd_do_mode_sense(SRpnt, 0, 0x3F, buffer, 255,
+					       &data);
+	}
 
 	if (!scsi_status_is_good(res)) {
 		printk(KERN_WARNING
diff -uprN -X /home/patman/dontdiff add_sdev_bflags-2.6/include/scsi/scsi_device.h 192_byte_ms-2.6/include/scsi/scsi_device.h
--- add_sdev_bflags-2.6/include/scsi/scsi_device.h	Fri Mar 12 16:16:47 2004
+++ 192_byte_ms-2.6/include/scsi/scsi_device.h	Fri Mar 12 16:17:55 2004
@@ -97,6 +97,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 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 */
 
diff -uprN -X /home/patman/dontdiff add_sdev_bflags-2.6/include/scsi/scsi_devinfo.h 192_byte_ms-2.6/include/scsi/scsi_devinfo.h
--- add_sdev_bflags-2.6/include/scsi/scsi_devinfo.h	Thu Mar 11 11:23:27 2004
+++ 192_byte_ms-2.6/include/scsi/scsi_devinfo.h	Fri Mar 12 16:17:55 2004
@@ -19,4 +19,5 @@
 #define BLIST_MS_SKIP_PAGE_08	0x2000	/* do not send ms page 0x08 */
 #define BLIST_MS_SKIP_PAGE_3F	0x4000	/* do not send ms page 0x3f */
 #define BLIST_USE_10_BYTE_MS	0x8000	/* use 10 byte ms before 6 byte ms */
+#define BLIST_MS_192_BYTES_FOR_3F	0x10000	/*  192 byte ms page 0x3f request */
 #endif

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: adding flag for 192 byte mode sense
  2004-03-14  1:46 adding flag for 192 byte mode sense Patrick Mansfield
  2004-03-14  1:49 ` [PATCH] Replace scsi_host flags with scsi_device sdev_bflags Patrick Mansfield
@ 2004-03-16 17:25 ` Alan Stern
  2004-03-16 18:02   ` Matthew Dharm
  2004-03-22 16:16 ` Alan Stern
  2 siblings, 1 reply; 10+ messages in thread
From: Alan Stern @ 2004-03-16 17:25 UTC (permalink / raw)
  To: Patrick Mansfield; +Cc: mdharm-usb, James Bottomley, linux-scsi, usb-storage

On Sat, 13 Mar 2004, Patrick Mansfield wrote:

> Following are patches to support a flag for a 192 byte MODE SENSE, for use
> by USB.
> 
> First patch replaces the scsi_host flags with a per sdev bflag, and sets
> values in a new USB slave_alloc rather than the scsi_host template. I
> grepped code for other users of scsi_host flags, and did not find any, and
> do not recall seeing any added (flags is a bad name). slave_alloc is used
> so that if needed the devinfo code can overwrite any settings.
> 
> Second patch adds the 192 flag.
> 
> With both patches we should get the exact behaviour we have now.

Great work!  Thanks, Patrick.

> USB patches are still needed to set the BLIST_MS_192_BYTES_FOR_3F flag,
> and to conditionally use the BLIST_USE_10_BYTE_MS based on the transparent
> scsi versus all others. Alan or Matthew - can you send patches for that?

Patch below.  It's not ideal because these things depend on the device 
type, which isn't known until after slave_alloc().  Hence the scsi devinfo 
mechanism can't be used for overrides.  It probably won't matter though.
We might want to keep using MODE SENSE(10) always even though Windows 
doesn't.  Nobody has complained about it so far.

> The scsi core usage of use_10_for_ms might need to change to behave the
> way usb wants, though it probably will not matter. It currently demotes
> from 10 byte to 6 byte if there is an illegal request on the 10 byte
> command. Based on Alan's information, it sounds like USB wants the
> commands to always be 6 or 10 bytes, with no change on an illegal request.

There are devices that like MS(10) and dislike MS(6).  I haven't seen the 
reverse, but maybe Matt has.

> I tested with these usb mass storage devices: sony camera, olympus camera,
> and dazzle card reader with no problems. I thought the sony used to choke
> on the MODE SENSE, but it worked for both the short 8 byte and long 192
> byte requests (did get sense key Illegal Request but no hang on allow
> medium removal).

What we really need is testing by the few people whose devices couldn't 
handle page 3f at all, before.

Alan Stern

P.S.:  A separate question has come up recently.  There are error
conditions in which the USB Mass Storage protocols require a device reset.
Currently usb-storage has been doing its own resetting, but maybe it would
be better to let the SCSI error handler take care of things.  Is there a
result code the driver can return to indicate that error recovery should
commence immediately (no command retries)?  Better yet, is there a code
to indicate that the initial TEST UNIT READY should be skipped and the
error handler should go straight into a device reset?


--- 2.6/drivers/usb/storage/scsiglue.c1	Tue Mar 16 10:38:08 2004
+++ 2.6/drivers/usb/storage/scsiglue.c	Tue Mar 16 12:12:25 2004
@@ -48,6 +48,7 @@
 #include "usb.h"
 #include "debug.h"
 #include "transport.h"
+#include "protocol.h"
 
 #include <linux/slab.h>
 #include <linux/module.h>
@@ -70,8 +71,15 @@
 	 * Set default bflags. These can be overridden for individual
 	 * models and vendors via the scsi devinfo mechanism.
 	 */
-	sdev->sdev_bflags = (BLIST_MS_SKIP_PAGE_08 | BLIST_MS_SKIP_PAGE_3F |
-			     BLIST_USE_10_BYTE_MS);
+
+	/* By default, USB mass storage devices use MODE SENSE(10).
+	 * Use 192 byte transfers for MODE SENSE page x3f.
+	 * We don't know yet whether MODE SENSE page x08 is reliable. */
+	sdev->sdev_bflags = (BLIST_USE_10_BYTE_MS |
+			BLIST_MS_192_BYTES_FOR_3F |
+			BLIST_MS_SKIP_PAGE_08);
+	/* Maybe add an unusual_devs flag for BLIST_MS_SKIP_PAGE_3F */
+
 	return 0;
 }
 
@@ -98,6 +106,23 @@
 	if (us->pusb_dev->descriptor.idVendor == USB_VENDOR_ID_GENESYS &&
 			us->pusb_dev->speed == USB_SPEED_HIGH)
 		blk_queue_max_sectors(sdev->request_queue, 128);
+
+	/* We can't put these settings in slave_alloc() because that gets
+	 * called before the device type is known.  Consequently these
+	 * settings can't be overridden via the scsi devinfo mechanism. */
+	if (sdev->type == TYPE_DISK) {
+
+		/* Disk-type devices use MODE SENSE(6) if the protocol
+		 * (SubClass) is Transparent SCSI */
+		if (us->subclass == US_SC_SCSI)
+			sdev->use_10_for_ms = 0;
+	} else {
+
+		/* Non-disk-type devices don't need to blacklist MS page x08
+		 * or to force 192 byte transfer lengths for MODE SENSE */
+		sdev->skip_ms_page_8 = 0;
+		sdev->use_192_bytes_for_3f = 0;
+	}
 
 	/* this is to satisify the compiler, tho I don't think the 
 	 * return code is ever checked anywhere. */


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: adding flag for 192 byte mode sense
  2004-03-16 17:25 ` adding flag for 192 byte mode sense Alan Stern
@ 2004-03-16 18:02   ` Matthew Dharm
  2004-03-16 20:08     ` Alan Stern
  0 siblings, 1 reply; 10+ messages in thread
From: Matthew Dharm @ 2004-03-16 18:02 UTC (permalink / raw)
  To: Alan Stern; +Cc: Patrick Mansfield, James Bottomley, linux-scsi, usb-storage

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

On Tue, Mar 16, 2004 at 12:25:03PM -0500, Alan Stern wrote:
> +
> +	/* We can't put these settings in slave_alloc() because that gets
> +	 * called before the device type is known.  Consequently these
> +	 * settings can't be overridden via the scsi devinfo mechanism. */
> +	if (sdev->type == TYPE_DISK) {
> +
> +		/* Disk-type devices use MODE SENSE(6) if the protocol
> +		 * (SubClass) is Transparent SCSI */
> +		if (us->subclass == US_SC_SCSI)
> +			sdev->use_10_for_ms = 0;
> +	} else {
> +
> +		/* Non-disk-type devices don't need to blacklist MS page x08
> +		 * or to force 192 byte transfer lengths for MODE SENSE */
> +		sdev->skip_ms_page_8 = 0;
> +		sdev->use_192_bytes_for_3f = 0;
> +	}
>  

Do we know what non-disk type devices use for MODE_SENSE?  That would
(primarily) be CD-ROMs, I think...

Matt

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

Somebody call an exorcist!
					-- Dust Puppy
User Friendly, 5/16/1998

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: adding flag for 192 byte mode sense
  2004-03-16 18:02   ` Matthew Dharm
@ 2004-03-16 20:08     ` Alan Stern
  2004-03-16 20:27       ` [usb-storage] " Pat LaVarre
  0 siblings, 1 reply; 10+ messages in thread
From: Alan Stern @ 2004-03-16 20:08 UTC (permalink / raw)
  To: Matthew Dharm; +Cc: Patrick Mansfield, James Bottomley, linux-scsi, usb-storage

On Tue, 16 Mar 2004, Matthew Dharm wrote:

> Do we know what non-disk type devices use for MODE_SENSE?  That would
> (primarily) be CD-ROMs, I think...

With my USB CDRW drive, Windows 2000 issued x5a = MODE SENSE(10).  It did
so even though the class/protocol/subclass are 08/50/06.  Furthermore,
testing with usb-storage showed that the drive doesn't accept MODE
SENSE(6).

Alan Stern


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [usb-storage] Re: adding flag for 192 byte mode sense
  2004-03-16 20:08     ` Alan Stern
@ 2004-03-16 20:27       ` Pat LaVarre
  0 siblings, 0 replies; 10+ messages in thread
From: Pat LaVarre @ 2004-03-16 20:27 UTC (permalink / raw)
  To: Alan Stern
  Cc: Matthew Dharm, James Bottomley, usb-storage, linux-scsi,
	Patrick Mansfield

> With my USB CDRW drive, Windows 2000 issued x5a = MODE SENSE(10).  It did
> so even though the class/protocol/subclass are 08/50/06.  Furthermore,
> testing with usb-storage showed that the drive doesn't accept MODE
> SENSE(6).

Yes, per spec.  That is:

t10.org mmc4r02h.pdf "Table 213" "Commands ... (Opcode order)" shows no
definition for the response of a PDT x05 DVD/ CD drive to an assault of
PDT x00 SBC ops x1A/ 5A Mode Sense/ Select (6).

t10.org spc2r20.pdf "Table 13" "Commands for all device types" via code
"Z" gives this rude freedom to MMC: "Command implementation is"
peripheral "device type specific".

Pat LaVarre



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: adding flag for 192 byte mode sense
  2004-03-14  1:46 adding flag for 192 byte mode sense Patrick Mansfield
  2004-03-14  1:49 ` [PATCH] Replace scsi_host flags with scsi_device sdev_bflags Patrick Mansfield
  2004-03-16 17:25 ` adding flag for 192 byte mode sense Alan Stern
@ 2004-03-22 16:16 ` Alan Stern
  2004-03-22 16:31   ` James Bottomley
  2 siblings, 1 reply; 10+ messages in thread
From: Alan Stern @ 2004-03-22 16:16 UTC (permalink / raw)
  To: Patrick Mansfield; +Cc: mdharm-usb, James Bottomley, linux-scsi, usb-storage

Three items:


	1. I haven't seen much feedback regarding the suggested patches that 
Patrick and I wrote:

http://marc.theaimsgroup.com/?l=linux-scsi&m=107922926816157&w=2
http://marc.theaimsgroup.com/?l=linux-scsi&m=107922905218137&w=2
http://marc.theaimsgroup.com/?l=linux-scsi&m=107945931705407&w=2

Do they look good?  Should they be applied?  One suggestion for a change: 
the flag could be renamed "use_192_bytes_for_ms" since we may end up 
wanting to use it for both page 0x3f and 0x08.

Also, why does sd.c check write-enable only for removable media?  Can't 
non-removable-media drives also have a write-protect switch?


	2. Some users have reported problems with USB digital cameras that 
don't support PREVENT-ALLOW MEDIUM REMOVAL and crash when they receive 
that command, even though they return the RMB flag set in their INQUIRY 
data.  Their medium really _is_ removable, in the sense that you can swap 
memory cards while the camera is turned on and plugged in to the computer.
How should we handle this?


	3. When a USB mass storage device enters a state that requires a
device reset, what command completion code should the usb-storage driver
return to invoke the SCSI error handler?  And is there any way to do this
so that the error handler skips the initial TEST UNIT READY step and goes
directly to the device reset?


Alan Stern


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: adding flag for 192 byte mode sense
  2004-03-22 16:16 ` Alan Stern
@ 2004-03-22 16:31   ` James Bottomley
  2004-03-22 17:28     ` Alan Stern
  0 siblings, 1 reply; 10+ messages in thread
From: James Bottomley @ 2004-03-22 16:31 UTC (permalink / raw)
  To: Alan Stern; +Cc: Patrick Mansfield, mdharm-usb, SCSI Mailing List, usb-storage

On Mon, 2004-03-22 at 11:16, Alan Stern wrote:
> Do they look good?  Should they be applied?  One suggestion for a change: 
> the flag could be renamed "use_192_bytes_for_ms" since we may end up 
> wanting to use it for both page 0x3f and 0x08.

They already are in the scsi-misc-2.6 repository.

> 	3. When a USB mass storage device enters a state that requires a
> device reset, what command completion code should the usb-storage driver
> return to invoke the SCSI error handler?  And is there any way to do this
> so that the error handler skips the initial TEST UNIT READY step and goes
> directly to the device reset?

Anything that maps to FAILED in scsi_decide_disposition()

What TEST UNIT READY?  The error handler only issues those *after*
recovery, not before.

James



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: adding flag for 192 byte mode sense
  2004-03-22 16:31   ` James Bottomley
@ 2004-03-22 17:28     ` Alan Stern
  0 siblings, 0 replies; 10+ messages in thread
From: Alan Stern @ 2004-03-22 17:28 UTC (permalink / raw)
  To: James Bottomley
  Cc: Patrick Mansfield, mdharm-usb, SCSI Mailing List, usb-storage

On 22 Mar 2004, James Bottomley wrote:

> On Mon, 2004-03-22 at 11:16, Alan Stern wrote:
> > Do they look good?  Should they be applied?  One suggestion for a change: 
> > the flag could be renamed "use_192_bytes_for_ms" since we may end up 
> > wanting to use it for both page 0x3f and 0x08.
> 
> They already are in the scsi-misc-2.6 repository.

Great!  Were just the SCSI patches included or were the usb-storage 
changes put in as well?  When can I expect to see them in a distribution 
kernel?


> > 	3. When a USB mass storage device enters a state that requires a
> > device reset, what command completion code should the usb-storage driver
> > return to invoke the SCSI error handler?  And is there any way to do this
> > so that the error handler skips the initial TEST UNIT READY step and goes
> > directly to the device reset?
> 
> Anything that maps to FAILED in scsi_decide_disposition()

So for example, (DID_OK << 16) + (ABORT << 8) would do the trick, right?
Or (0xff << 16).  Is there any equivalent that expresses the intention
more clearly?


> What TEST UNIT READY?  The error handler only issues those *after*
> recovery, not before.

Hmm... apparently that disappeared while I wasn't looking.  I recall 
reading lots and lots of debugging logs in which the first thing that 
happened after a command timed out was a TEST UNIT READY.  But it doesn't 
matter.

Alan Stern


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2004-03-22 17:28 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-03-14  1:46 adding flag for 192 byte mode sense Patrick Mansfield
2004-03-14  1:49 ` [PATCH] Replace scsi_host flags with scsi_device sdev_bflags Patrick Mansfield
2004-03-14  1:50   ` [PATCH] Add 192 byte MODE SENSE flag Patrick Mansfield
2004-03-16 17:25 ` adding flag for 192 byte mode sense Alan Stern
2004-03-16 18:02   ` Matthew Dharm
2004-03-16 20:08     ` Alan Stern
2004-03-16 20:27       ` [usb-storage] " Pat LaVarre
2004-03-22 16:16 ` Alan Stern
2004-03-22 16:31   ` James Bottomley
2004-03-22 17:28     ` Alan Stern

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox