Skip to contentSkip to conversation

Set manage_start_stop for some USB storage devices by default

PATCH In review Thadeu Lima de Souza Cascardo 4 participants 5 messages Mar 2, 2009 2 messages not in the archive

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Set manage_start_stop for some USB storage devices by default

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(-)
drivers/usb/storage/scsiglue.c +6 -0
index 8daaace..b7ed288 100644
@@ -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).
drivers/usb/storage/unusual_devs.h +5 -0
index 729a256..7332c4d 100644
@@ -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",
include/linux/usb_usual.h +4 -1
index bd414ec..8c56127 100644
@@ -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 };
Signature · 1 line
1.6.2.rc0

A message that is not in the archive (sent off-list or not yet received): 20090302174240.GA22206-StA3NKSgq9XYLCA8tIpKU1aTQe2KTcn/@public.gmane.org

#2 lore raw
Quoted 15 lines · 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.

Quoted 3 lines

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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#3 lore raw
Quoted 17 lines · Alan Stern
Quoted 15 lines · 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.

Quoted 7 lines
Quoted 3 lines

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?

Quoted 5 lines

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.

Quoted 5 lines

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?

Quoted 2 lines

Alan Stern

Thank you very much for your prompt answer and advice.

Regards,
Cascardo.

maintainer
#4 lore raw
Quoted 6 lines · Thadeu Lima de Souza Cascardo
Quoted 3 lines

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.

Quoted 8 lines
Quoted 5 lines

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.

Quoted 9 lines
Quoted 5 lines

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

A message that is not in the archive (sent off-list or not yet received): Pine.LNX.4.44L0.0903021452570.12723-100000-pYrvlCTfrz9XsRXLowluHWD2FQJk+8+b@public.gmane.org

#6 lore raw
Quoted 7 lines · Alan Stern
Quoted 6 lines · Thadeu Lima de Souza Cascardo
Quoted 5 lines

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.

Quoted 8 lines
Quoted 3 lines

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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html