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

Hi Bernhard,

First of all, sorry for the very late reply. Your patch somehow felt
off my radar.

On Mon, 1 Oct 2007 20:13:12 +0200 (CEST), Bernhard Kaindl wrote:
> 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?

Yes, my drive is a 320 GB model too.

> 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

Also see:
http://www.nslu2-linux.org/wiki/FAQ/DealWithAutoSpinDownOnSeagateFreeAgent

> 
> 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

Your patch fixes my problems, thank you! Please submit it for upstream
inclusion. The only change I would suggest is to check that the product
name starts with "FreeAgent" rather than testing for specific strings.
This is both more efficient (two string comparisons instead of three)
and more future-proof (if Seagate later extend the product line.)

-- 
Jean Delvare

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel