[PATCH] SCSI error handling on some USB disks needs allow_restartcp

On Sun, 30 Sep 2007, Jean Delvare wrote:
> On Thu, 27 Sep 2007 20:00:36 +0200 (CEST), Bernhard Kaindl wrote:
>> There is a family of USB disks from Seagate (FreeAgent Desktop, Pro and Go
>> models) which actually need allow_restart by default, even without autosuspend:
>
> This is the exact model of my USB disk drive, for which this thread
> started.

Your USB device ID differs a bit from mine:

yours is 0bc2:3000 Seagate RSS LLC
mine  is 0bc2:3010 Seagate RSS LLC

Very similar.

I have found that USB device two times on the web so far and both times it was
a FreeAgent Desktop 320GB, is yours also 320GB?


That's from the FreeAgent Pro, but I assume that the USB electronics are the same
and only the built-in harddisk drive should differ from the FreeAgent Desktop

>
>> These disks suspend themselfes after 15 minutes of inactivity but need START_UNIT
>> for restart. I have two Pro models and tested this and have a patch ready which
>> fixes those selectively. Sending it as followup mail.
>
> I did notice the auto-suspend behavior too. Can you please point me to
> your patch, or send it to me in private? I'm not subscribed to the
> linux-usb-devel list, but I'd like to try it.

I'll keep you in Cc. Please find the patch attached and below.

A patch for older Debian kernels (which do not have the support for allow_restart):

http://bugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=419175#25

This patch uses the slave_alloc function of the SCSI host as described
in one of the mails which said where to enable allow_restart and
the ieee1394/spb2.c does it in the same way.

This version uses string matches against manufacturer and product
strings because I do not yet know all the USB device IDs of those
devices, but I know that the manufacturer / product strings look
from looking at the relevant Gentoo bug report:

https://bugs.launchpad.net/ubuntu/+source/udev/+bug/61235

Best Regards everyone,
Bernhard Kaindl

Tested with Seagate FreeAgent Pro:

Signed-Off-By: Bernhard Kaindl <[email protected]>

--- drivers/usb/storage/scsiglue.c
+++ drivers/usb/storage/scsiglue.c
@@ -73,6 +73,7 @@
  static int slave_alloc (struct scsi_device *sdev)
  {
  	struct us_data *us = host_to_us(sdev->host);
+	const char *vendor, *product;

  	/*
  	 * Set the INQUIRY transfer length to 36.  We don't use any of
@@ -81,6 +82,15 @@
  	 */
  	sdev->inquiry_len = 36;

+	vendor = us->pusb_dev->manufacturer;
+	product = us->pusb_dev->product;
+
+	if (vendor && product &&
+	    !strcmp(vendor, "Seagate") &&
+	    (!strcmp(product, "FreeAgent Pro") ||
+	     !strcmp(product, "FreeAgentDesktop")))
+		sdev->allow_restart = 1;
+
  	/*
  	 * The UFI spec treates the Peripheral Qualifier bits in an
  	 * INQUIRY result as reserved and requires devices to set them

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

--- drivers/usb/storage/scsiglue.c
+++ drivers/usb/storage/scsiglue.c
@@ -73,6 +73,7 @@
 static int slave_alloc (struct scsi_device *sdev)
 {
 	struct us_data *us = host_to_us(sdev->host);
+	const char *vendor, *product;
 
 	/*
 	 * Set the INQUIRY transfer length to 36.  We don't use any of
@@ -81,6 +82,15 @@
 	 */
 	sdev->inquiry_len = 36;
 
+	vendor = us->pusb_dev->manufacturer;
+	product = us->pusb_dev->product;
+
+	if (vendor && product &&
+	    !strcmp(vendor, "Seagate") &&
+	    (!strcmp(product, "FreeAgent Pro") ||
+	     !strcmp(product, "FreeAgentDesktop")))
+		sdev->allow_restart = 1;
+		
 	/*
 	 * The UFI spec treates the Peripheral Qualifier bits in an
 	 * INQUIRY result as reserved and requires devices to set them