[PATCH] Set manage_start_stop for some USB storage devices by default

[PATCH] Set manage_start_stop for some USB storage devices by default

[Thadeu Lima de Souza Cascardo]

Hello,

Some time ago, I've worked with a Sandisk storage device, which could
not resume properly. I've found out that it was timing out when
receiving requests. I could mount it right after resuming, but using a
mounted device was not possible. Since this device is the only storage
present in some mobile, it is used as the root device and cannot be
unmounted.

I've thought this was a bug in the device itself and not a problem in
the bus or host controller, since other devices worked pretty well.
After doing some tests, I've realized that doing a rescan in the device
before accessing it worked. And setting manage_start_stop as true does
this automatically when resuming for us.

Thus, the patch below. I would like some comments about this particular
issue and, if the solution is acceptable, I will send a proper commit
log.

Best regards,
Cascardo.

---
 drivers/usb/storage/scsiglue.c     |    6 ++++++
 drivers/usb/storage/unusual_devs.h |    5 +++++
 include/linux/usb_usual.h          |    5 ++++-
 3 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c
index 8daaace..b7ed288 100644
--- a/drivers/usb/storage/scsiglue.c
+++ b/drivers/usb/storage/scsiglue.c
@@ -123,6 +123,12 @@ static int slave_configure(struct scsi_device *sdev)
 {
 	struct us_data *us = host_to_us(sdev->host);
 
+	/* Some devices require that manage_start_stop be set up so they
+	 * can resume properly
+	 */
+	if (us->fflags & US_FL_MANAGE_START_STOP)
+		sdev->manage_start_stop = 1;
+
 	/* Many devices have trouble transfering more than 32KB at a time,
 	 * while others have trouble with more than 64K. At this time we
 	 * are limiting both to 32K (64 sectores).
diff --git a/drivers/usb/storage/unusual_devs.h b/drivers/usb/storage/unusual_devs.h
index 729a256..7332c4d 100644
--- a/drivers/usb/storage/unusual_devs.h
+++ b/drivers/usb/storage/unusual_devs.h
@@ -1015,6 +1015,11 @@ UNUSUAL_DEV(  0x0781, 0x0200, 0x0000, 0x9999,
 		0),
 #endif
 
+UNUSUAL_DEV(  0x0781, 0x6500, 0x0000, 0xffff,
+		"Sandisk",
+		"uSSD 5000",
+		US_SC_DEVICE, US_PR_DEVICE, NULL, US_FL_MANAGE_START_STOP),
+
 #ifdef CONFIG_USB_STORAGE_FREECOM
 UNUSUAL_DEV(  0x07ab, 0xfc01, 0x0000, 0x9999,
 		"Freecom",
diff --git a/include/linux/usb_usual.h b/include/linux/usb_usual.h
index bd414ec..8c56127 100644
--- a/include/linux/usb_usual.h
+++ b/include/linux/usb_usual.h
@@ -54,7 +54,10 @@
 	US_FLAG(BULK_IGNORE_TAG,0x00004000)			\
 		/* Ignore tag mismatch in bulk operations */	\
 	US_FLAG(CAPACITY_OK,	0x00010000)			\
-		/* READ CAPACITY response is correct */
+		/* READ CAPACITY response is correct */		\
+	US_FLAG(MANAGE_START_STOP,	0x00020000)		\
+		/* manage_start_stop for the SCSI device */
+
 
 #define US_FLAG(name, value)	US_FL_##name = value ,
 enum { US_DO_ALL_FLAGS };
-- 
1.6.2.rc0

Re: [PATCH] Set manage_start_stop for some USB storage devices by default

[Alan Stern]

On Mon, 2 Mar 2009, Thadeu Lima de Souza Cascardo wrote:

> Hello,
> 
> Some time ago, I've worked with a Sandisk storage device, which could
> not resume properly. I've found out that it was timing out when
> receiving requests. I could mount it right after resuming, but using a
> mounted device was not possible. Since this device is the only storage
> present in some mobile, it is used as the root device and cannot be
> unmounted.
> 
> I've thought this was a bug in the device itself and not a problem in
> the bus or host controller, since other devices worked pretty well.
> After doing some tests, I've realized that doing a rescan in the device
> before accessing it worked. And setting manage_start_stop as true does
> this automatically when resuming for us.
> 
> Thus, the patch below. I would like some comments about this particular
> issue and, if the solution is acceptable, I will send a proper commit
> log.

Maybe there doesn't need to be a patch for this at all.  Can't you get 
the same result by writing a udev rule?  In fact, isn't that how the 
manage_start_stop flag is intended to be set?

As for the patch itself...  You are setting the flag in the wrong 
place.  The manage_start_stop flag applies only to SCSI disk devices 
AFAICT.  (The sysfs interface isn't present if the device isn't a disk, 
at any rate.)  So you should set the flag in the code that applies to 
disks.

Also, this patch won't apply to the current development tree, because
there have been several changes to usb-storage's recently.  In
particular there is new code allowing quirk flags to be set by a module
parameter, and your new flag (if it is accepted) should be added to
this mechanism.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Set manage_start_stop for some USB storage devices by default

[Thadeu Lima de Souza Cascardo]

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

On Mon, Mar 02, 2009 at 02:42:15PM -0500, Alan Stern wrote:
> On Mon, 2 Mar 2009, Thadeu Lima de Souza Cascardo wrote:
> 
> > Hello,
> > 
> > Some time ago, I've worked with a Sandisk storage device, which could
> > not resume properly. I've found out that it was timing out when
> > receiving requests. I could mount it right after resuming, but using a
> > mounted device was not possible. Since this device is the only storage
> > present in some mobile, it is used as the root device and cannot be
> > unmounted.
> > 
> > I've thought this was a bug in the device itself and not a problem in
> > the bus or host controller, since other devices worked pretty well.
> > After doing some tests, I've realized that doing a rescan in the device
> > before accessing it worked. And setting manage_start_stop as true does
> > this automatically when resuming for us.
> > 
> > Thus, the patch below. I would like some comments about this particular
> > issue and, if the solution is acceptable, I will send a proper commit
> > log.
> 
> Maybe there doesn't need to be a patch for this at all.  Can't you get 
> the same result by writing a udev rule?  In fact, isn't that how the 
> manage_start_stop flag is intended to be set?

I did expect that doing it in user space would be the proper way. Any
tips on how to find out the USB id's given the disk?

> As for the patch itself...  You are setting the flag in the wrong 
> place.  The manage_start_stop flag applies only to SCSI disk devices 
> AFAICT.  (The sysfs interface isn't present if the device isn't a disk, 
> at any rate.)  So you should set the flag in the code that applies to 
> disks.

I thought on adding a quirk flag to the SCSI host and, then, testing
that in the SCSI code itself. Didn't think SCSI guys would like that.

> Also, this patch won't apply to the current development tree, because
> there have been several changes to usb-storage's recently.  In
> particular there is new code allowing quirk flags to be set by a module
> parameter, and your new flag (if it is accepted) should be added to
> this mechanism.

Also expected. But I didn't know about the new changes in the quirk
flags code. Which tree should I refer to regarding usb-storage or USB,
besides next and rc trees?

> Alan Stern
> 

Thank you very much for your prompt answer and advice.

Regards,
Cascardo.

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

Re: [PATCH] Set manage_start_stop for some USB storage devices by default

[Alan Stern]

On Mon, 2 Mar 2009, Thadeu Lima de Souza Cascardo wrote:

> > Maybe there doesn't need to be a patch for this at all.  Can't you get 
> > the same result by writing a udev rule?  In fact, isn't that how the 
> > manage_start_stop flag is intended to be set?
> 
> I did expect that doing it in user space would be the proper way. Any
> tips on how to find out the USB id's given the disk?

Look in /proc/bus/usb/devices or run lsusb.

> > As for the patch itself...  You are setting the flag in the wrong 
> > place.  The manage_start_stop flag applies only to SCSI disk devices 
> > AFAICT.  (The sysfs interface isn't present if the device isn't a disk, 
> > at any rate.)  So you should set the flag in the code that applies to 
> > disks.
> 
> I thought on adding a quirk flag to the SCSI host and, then, testing
> that in the SCSI code itself. Didn't think SCSI guys would like that.

Even if you keep the quirk in usb-storage, it should still go in the 
appropriate place, somewhat farther down than where you put it.

> > Also, this patch won't apply to the current development tree, because
> > there have been several changes to usb-storage's recently.  In
> > particular there is new code allowing quirk flags to be set by a module
> > parameter, and your new flag (if it is accepted) should be added to
> > this mechanism.
> 
> Also expected. But I didn't know about the new changes in the quirk
> flags code. Which tree should I refer to regarding usb-storage or USB,
> besides next and rc trees?

http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/

I tend to use the gregkh-all-*.patch files.  However, I think 
everything in there also goes into -next.

Alan Stern

Re: [PATCH] Set manage_start_stop for some USB storage devices by default

[Greg KH]

On Mon, Mar 02, 2009 at 02:57:03PM -0500, Alan Stern wrote:
> On Mon, 2 Mar 2009, Thadeu Lima de Souza Cascardo wrote:
> > > Also, this patch won't apply to the current development tree, because
> > > there have been several changes to usb-storage's recently.  In
> > > particular there is new code allowing quirk flags to be set by a module
> > > parameter, and your new flag (if it is accepted) should be added to
> > > this mechanism.
> > 
> > Also expected. But I didn't know about the new changes in the quirk
> > flags code. Which tree should I refer to regarding usb-storage or USB,
> > besides next and rc trees?
> 
> http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/
> 
> I tend to use the gregkh-all-*.patch files.  However, I think 
> everything in there also goes into -next.

Yes, it all is in the linux-next releases, you can use those if it's
easier than using my trees.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html