Re: Windows and non-lockable devices (fwd)

Re: Windows and non-lockable devices (fwd)

[Alan Stern]

James and Mike:

At Matt Dharm's request I'm adding you into this thread.  Maybe you can 
help solve our problem.  Here's the quick overview:

The Panasonic DMC-LC40 USB camera causes difficulties, because whenever it
receives an ALLOW MEDIUM REMOVAL command the next few TEST UNIT READY
commands encounter "Medium not present" failures.  So after sd.c initially
probes the device and reads the partition sector, when the user tries to
open the device for mounting sd_spinup_disk() returns an error because it 
believes that the medium has been removed.

A similar problem exists with the iRiver MP3 USB player.  In this case the 
medium is not removable but the device sets RMB anyway.  And then it 
obligingly crashes when it receives PREVENT-ALLOW MEDIUM REMOVAL.

We've discussed a few possible workarounds:

	The usb-storage driver could turn off the RMB flag in the INQUIRY 
	data.  But that's more invasive than we like, and anyway the
	camera's medium really _is_ removable.

	usb-storage could set sdev->lockable to 0.  That's easy to do and
	solves the problem; the question then becomes: For which devices
	should we do this?  One answer is to have a blacklist of non-
	lockable devices.  Another answer is always to do it for devices
	that aren't CDs, maybe with a whitelist for the relatively small
	number of known good USB disk devices that can lock their medium.

Out of curiosity I asked Matthew Wheeler (who has a DMC digital camera) to 
capture a USB trace showing how Windows 2000 deals with the problem.  The 
summary of the result is below.

What do you advise?

Alan Stern


---------- Forwarded message ----------

On Tue, Jun 22, 2004 at 03:25:09PM -0400, Alan Stern wrote:
> 
> Okay, this log has lots of good information.  It's mixed in with tons of 
> sickening information, though... the Windows implementation of the 
> SCSI-over-USB protocols is not pretty.  So for the sake of readers with 
> weak stomachs I won't give a detailed listing of all the commands and 
> responses.
> 
> The log does show the write starting, and about 15 seconds later the 
> filesystem buffers are flushed and then there's an ALLOW MEDIUM REMOVAL.  
> When the next write begins, the first command is TEST UNIT READY which 
> does indeed encounter a "Medium not present" failure.
> 
> As near as I can tell, Windows then revalidates the disk.  It does a bunch 
> of READ CAPACITY, TEST UNIT READY, MODE SENSE, and READ(10) commands (for 
> things like the partition sector, the boot sector, the FAT, and the root 
> directory).  By the time that's all finished up whatever error condition 
> existed inside the camera has been cleared, so the write can proceed 
> normally.
> 
> The question is, how should we use this information?  Adopting a similar 
> approach would mean making some non-trivial changes to sd.c, and I'm not 
> sure that it's such a good approach to take in the first place.

I'm not sure where else you would put the fix.....

> Maybe some sort of compromise is in order.  If sd_spinup_disk() would
> try issuing both TEST UNIT READY and READ CAPACITY commands, that might
> be sufficient.

That might work.  We definatly want to bring someone from the SCSI side of
the equasion into this discussion.  Perhaps James Bottomley or Mike
Anderson?

Matt

Re: Windows and non-lockable devices (fwd)

[Mike Anderson]

Alan Stern [stern@rowland.harvard.edu] wrote:
> The Panasonic DMC-LC40 USB camera causes difficulties, because whenever it
> receives an ALLOW MEDIUM REMOVAL command the next few TEST UNIT READY
> commands encounter "Medium not present" failures.  So after sd.c initially
> probes the device and reads the partition sector, when the user tries to
> open the device for mounting sd_spinup_disk() returns an error because it 
> believes that the medium has been removed.
> 
> A similar problem exists with the iRiver MP3 USB player.  In this case the 
> medium is not removable but the device sets RMB anyway.  And then it 
> obligingly crashes when it receives PREVENT-ALLOW MEDIUM REMOVAL.
> 
> We've discussed a few possible workarounds:
> 
> 	The usb-storage driver could turn off the RMB flag in the INQUIRY 
> 	data.  But that's more invasive than we like, and anyway the
> 	camera's medium really _is_ removable.

I would agree that this does not sound like the best idea.

> 
> 	usb-storage could set sdev->lockable to 0.  That's easy to do and
> 	solves the problem; the question then becomes: For which devices
> 	should we do this?  One answer is to have a blacklist of non-
> 	lockable devices.  Another answer is always to do it for devices
> 	that aren't CDs, maybe with a whitelist for the relatively small
> 	number of known good USB disk devices that can lock their medium.

Since SCSI already has the device list it would seem like we would
possibly add a new flag like was done for mode sense. As this is not a
transport issue I would assume we would not want to add flags in
usb/storage, but handle it in the mid-layer as a SCSI protocol
non-compliance.

James does this sound ok to you?

> > Maybe some sort of compromise is in order.  If sd_spinup_disk() would
> > try issuing both TEST UNIT READY and READ CAPACITY commands, that might
> > be sufficient.

This sounds like we would try to the dial in the timing of sd's commands
sequences to cover one devices internal timing issues and hope it solves
the problem. This sounds like a fragile solution and it would not solve
the issue for the "iRiver" unless I read the error case wrong.

-andmike
--
Michael Anderson
andmike@us.ibm.com

PATCH: (as333) BLIST flag for non-lockable devices

[Alan Stern]

On Wed, 23 Jun 2004, Mike Anderson wrote:

> Since SCSI already has the device list it would seem like we would
> possibly add a new flag like was done for mode sense. As this is not a
> transport issue I would assume we would not want to add flags in
> usb/storage, but handle it in the mid-layer as a SCSI protocol
> non-compliance.

Here is a patch that implements this suggestion.  It's rather similar to 
the one that Javier Marcet wrote back in January, in

http://marc.theaimsgroup.com/?l=linux-usb-users&m=107345268526718&w=2

Matthew, does this patch work for your digital camera?

Alan Stern



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

===== include/scsi/scsi_devinfo.h 1.5 vs edited =====
--- 1.5/include/scsi/scsi_devinfo.h	Sun May 30 04:51:19 2004
+++ edited/include/scsi/scsi_devinfo.h	Fri Jun 25 11:53:20 2004
@@ -24,4 +24,5 @@
 #define BLIST_REPORTLUN2	0x20000	/* try REPORT_LUNS even for SCSI-2 devs
  					   (if HBA supports more than 8 LUNs) */
 #define BLIST_NOREPORTLUN	0x40000	/* don't try REPORT_LUNS scan (SCSI-3 devs) */
+#define BLIST_NOT_LOCKABLE	0x80000	/* don't use PREVENT-ALLOW commands */
 #endif
===== drivers/scsi/scsi_devinfo.c 1.9 vs edited =====
--- 1.9/drivers/scsi/scsi_devinfo.c	Sat Jun 19 07:49:23 2004
+++ edited/drivers/scsi/scsi_devinfo.c	Fri Jun 25 12:07:06 2004
@@ -158,8 +158,11 @@
 	{"IOMEGA", "Io20S         *F", NULL, BLIST_KEY},
 	{"INSITE", "Floptical   F*8I", NULL, BLIST_KEY},
 	{"INSITE", "I325VM", NULL, BLIST_KEY},
+	{"iRiver", "iFP Mass Driver", NULL, BLIST_NOT_LOCKABLE | BLIST_INQUIRY_36},
 	{"LASOUND", "CDX7405", "3.10", BLIST_MAX5LUN | BLIST_SINGLELUN},
 	{"MATSHITA", "PD-1", NULL, BLIST_FORCELUN | BLIST_SINGLELUN},
+	{"MATSHITA", "DMC-LC5", NULL, BLIST_NOT_LOCKABLE | BLIST_INQUIRY_36},
+	{"MATSHITA", "DMC-LC40", NULL, BLIST_NOT_LOCKABLE | BLIST_INQUIRY_36},
 	{"Medion", "Flash XL  MMC/SD", "2.6D", BLIST_FORCELUN},
 	{"MegaRAID", "LD", NULL, BLIST_FORCELUN},
 	{"MICROP", "4110", NULL, BLIST_NOTQ},
===== drivers/scsi/scsi_scan.c 1.61 vs edited =====
--- 1.61/drivers/scsi/scsi_scan.c	Mon Jun 21 12:58:41 2004
+++ edited/drivers/scsi/scsi_scan.c	Fri Jun 25 11:56:28 2004
@@ -661,6 +661,9 @@
 	if (*bflags & BLIST_MS_192_BYTES_FOR_3F)
 		sdev->use_192_bytes_for_3f = 1;
 
+	if (*bflags & BLIST_NOT_LOCKABLE)
+		sdev->lockable = 0;
+
 	if(sdev->host->hostt->slave_configure)
 		sdev->host->hostt->slave_configure(sdev);
 

Re: PATCH: (as333) BLIST flag for non-lockable devices

[Matthew Wheeler]

Alan,
    This patch worked without any problems that I could see. I was able 
to mount, copy files to and from and eject the camera. Let me know if 
you would like me to run any more tests or send debugging logs etc.

    Matt

Alan Stern wrote:

>On Wed, 23 Jun 2004, Mike Anderson wrote:
>
>  
>
>>Since SCSI already has the device list it would seem like we would
>>possibly add a new flag like was done for mode sense. As this is not a
>>transport issue I would assume we would not want to add flags in
>>usb/storage, but handle it in the mid-layer as a SCSI protocol
>>non-compliance.
>>    
>>
>
>Here is a patch that implements this suggestion.  It's rather similar to 
>the one that Javier Marcet wrote back in January, in
>
>http://marc.theaimsgroup.com/?l=linux-usb-users&m=107345268526718&w=2
>
>Matthew, does this patch work for your digital camera?
>
>Alan Stern
>
>
>
>Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
>
>===== include/scsi/scsi_devinfo.h 1.5 vs edited =====
>--- 1.5/include/scsi/scsi_devinfo.h	Sun May 30 04:51:19 2004
>+++ edited/include/scsi/scsi_devinfo.h	Fri Jun 25 11:53:20 2004
>@@ -24,4 +24,5 @@
> #define BLIST_REPORTLUN2	0x20000	/* try REPORT_LUNS even for SCSI-2 devs
>  					   (if HBA supports more than 8 LUNs) */
> #define BLIST_NOREPORTLUN	0x40000	/* don't try REPORT_LUNS scan (SCSI-3 devs) */
>+#define BLIST_NOT_LOCKABLE	0x80000	/* don't use PREVENT-ALLOW commands */
> #endif
>===== drivers/scsi/scsi_devinfo.c 1.9 vs edited =====
>--- 1.9/drivers/scsi/scsi_devinfo.c	Sat Jun 19 07:49:23 2004
>+++ edited/drivers/scsi/scsi_devinfo.c	Fri Jun 25 12:07:06 2004
>@@ -158,8 +158,11 @@
> 	{"IOMEGA", "Io20S         *F", NULL, BLIST_KEY},
> 	{"INSITE", "Floptical   F*8I", NULL, BLIST_KEY},
> 	{"INSITE", "I325VM", NULL, BLIST_KEY},
>+	{"iRiver", "iFP Mass Driver", NULL, BLIST_NOT_LOCKABLE | BLIST_INQUIRY_36},
> 	{"LASOUND", "CDX7405", "3.10", BLIST_MAX5LUN | BLIST_SINGLELUN},
> 	{"MATSHITA", "PD-1", NULL, BLIST_FORCELUN | BLIST_SINGLELUN},
>+	{"MATSHITA", "DMC-LC5", NULL, BLIST_NOT_LOCKABLE | BLIST_INQUIRY_36},
>+	{"MATSHITA", "DMC-LC40", NULL, BLIST_NOT_LOCKABLE | BLIST_INQUIRY_36},
> 	{"Medion", "Flash XL  MMC/SD", "2.6D", BLIST_FORCELUN},
> 	{"MegaRAID", "LD", NULL, BLIST_FORCELUN},
> 	{"MICROP", "4110", NULL, BLIST_NOTQ},
>===== drivers/scsi/scsi_scan.c 1.61 vs edited =====
>--- 1.61/drivers/scsi/scsi_scan.c	Mon Jun 21 12:58:41 2004
>+++ edited/drivers/scsi/scsi_scan.c	Fri Jun 25 11:56:28 2004
>@@ -661,6 +661,9 @@
> 	if (*bflags & BLIST_MS_192_BYTES_FOR_3F)
> 		sdev->use_192_bytes_for_3f = 1;
> 
>+	if (*bflags & BLIST_NOT_LOCKABLE)
>+		sdev->lockable = 0;
>+
> 	if(sdev->host->hostt->slave_configure)
> 		sdev->host->hostt->slave_configure(sdev);
> 
>
>  
>

Re: PATCH: BLIST flag for non-lockable devices

[Javier Marcet]

* Alan Stern <stern@rowland.harvard.edu> [040625 18:14]:

>> Since SCSI already has the device list it would seem like we would
>> possibly add a new flag like was done for mode sense. As this is not a
>> transport issue I would assume we would not want to add flags in
>> usb/storage, but handle it in the mid-layer as a SCSI protocol
>> non-compliance.

>Here is a patch that implements this suggestion.  It's rather similar to 
>the one that Javier Marcet wrote back in January, in

>http://marc.theaimsgroup.com/?l=linux-usb-users&m=107345268526718&w=2

>Matthew, does this patch work for your digital camera?

For me, as you probably guessed, this is fine. It works very well with
my iRiver.

Yet I wonder why the INQUIRY_36 flag? Isn't it the default? Or you just
try to skip the try & failure over longer inquiries since you already
know these devices' length?

No complaints no my side anyway. Push it upstream when you see it fit :)


-- 
The entire CHINESE WOMEN'S VOLLEYBALL TEAM all share ONE personality --
and have since BIRTH!!

Javier Marcet <javier@marcet.info>

Re: PATCH: BLIST flag for non-lockable devices

[Alan Stern]

On Mon, 28 Jun 2004, Javier Marcet wrote:

> For me, as you probably guessed, this is fine. It works very well with
> my iRiver.

Good.

> Yet I wonder why the INQUIRY_36 flag? Isn't it the default? Or you just
> try to skip the try & failure over longer inquiries since you already
> know these devices' length?

It's true that using a 36-byte INQUIRY command is the default.  However
the BLIST_INQUIRY_36 flag means more than that: It means also that there 
should _not_ be an INQUIRY command longer than 36 bytes, even if the 
device says it has extra data available.

I've been adding this flag to all the blacklist entries for USB devices,
just for safety.  With most of them it's not needed.  But I have seen an
example of a device which said that it had 37 bytes of INQUIRY data and
then crashed when it received a command asking for all 37 bytes.

Alan Stern

Re: PATCH: BLIST flag for non-lockable devices

[Javier Marcet]

* Alan Stern <stern@rowland.harvard.edu> [040628 17:54]:

>> Yet I wonder why the INQUIRY_36 flag? Isn't it the default? Or you just
>> try to skip the try & failure over longer inquiries since you already
>> know these devices' length?

>It's true that using a 36-byte INQUIRY command is the default.  However
>the BLIST_INQUIRY_36 flag means more than that: It means also that there 
>should _not_ be an INQUIRY command longer than 36 bytes, even if the 
>device says it has extra data available.

I see.

>I've been adding this flag to all the blacklist entries for USB devices,

Yes, I realized that upon first look at the blacklist.

>just for safety.  With most of them it's not needed.  But I have seen an
>example of a device which said that it had 37 bytes of INQUIRY data and
>then crashed when it received a command asking for all 37 bytes.

Ditto then, it works without a hitch -I only wish USB 1.1 was faster
though, but it's not something at hour hands, sigh-, better being safe
with that flag anyway.

Thanks for your help and time Alan (et all other who helped) :)


-- 
The problem with the gene pool is that there is no lifeguard.

Javier Marcet <javier@marcet.info>