All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ruud Linders <rkmp@xs4all.nl>
To: Linus Torvalds <torvalds@osdl.org>
Cc: James Bottomley <James.Bottomley@SteelEye.com>,
	Patrick Mansfield <patmans@us.ibm.com>,
	Christoph Hellwig <hch@infradead.org>,
	Alan Stern <stern@rowland.harvard.edu>,
	Matthew Dharm <mdharm-usb@one-eyed-alien.net>,
	David Brownell <david-b@pacbell.net>, Greg KH <greg@kroah.com>,
	USB development list <linux-usb-devel@lists.sourceforge.net>,
	SCSI development list <linux-scsi@vger.kernel.org>,
	Andries.Brouwer@cwi.nl
Subject: Re: [linux-usb-devel] Re: USB storage problems on OHCI..
Date: Tue, 23 Sep 2003 19:47:04 +0200	[thread overview]
Message-ID: <3F708718.4050401@xs4all.nl> (raw)
In-Reply-To: <Pine.LNX.4.44.0309221250470.1673-100000@home.osdl.org>



I tried the patch but it doesn't work for me using an USB-2 Memory stick
"DiskonKey" on an USB-2 port (with uhci_hcd & ehci_hcd loaded).

After a 3 minute time-out I get
     "SCSI device sda: drive cache: write through"
and the device starts working just fine.

Unloading the ehci_hcd module doesn't make any difference.
Only when ripping out the whole mode-sense call it works immediately :-)
so it appears the device is just unhappy about the sd_do_mode_sense.

_
Regards,
	Ruud Linders

Linus Torvalds wrote:
> On 22 Sep 2003, James Bottomley wrote:
> 
>>I think we could try 4 bytes for this (even to avoid wide residue
>>problems) and see how it goes.
> 
> 
> How about just trusting the size (and as far as I can tell from the SCSI 
> specs, the size is the size _without_ the header and block descriptors), 
> and capping it at 20?
> 
> If the device reports a size that is smaller than the required one (3), we 
> just fail the request - instead of doing a small read and then making 
> decisions on data that isn't even there, like we used to do!
> 
> (The old code would use the "size" as the read length argument, which
> really looks fundamentally wrong. At the very least it should add in the
> header length etc).
> 
> Can people try this attached patch? It looks bigger than it is - in order 
> to do sane error handling for the "size is too small" case I re-organized 
> the thing a bit. The real change is just
> 
>  - capping size at 20 (real value) instead of 128 (random crud)
>  - properly add in the header and block descriptor sizes
> 
> Comments?
> 
> 			Linus
> 
> -----
> ===== drivers/scsi/sd.c 1.134 vs edited =====
> --- 1.134/drivers/scsi/sd.c	Wed Jun 25 04:47:26 2003
> +++ edited/drivers/scsi/sd.c	Mon Sep 22 12:42:32 2003
> @@ -1108,14 +1108,26 @@
>  	/* cautiously ask */
>  	res = sd_do_mode_sense(SRpnt, dbd, modepage, buffer, 4, &data);
>  
> -	if (scsi_status_is_good(res)) {
> -		/* that went OK, now ask for the proper length */
> -		len = data.length;
> -		if (len > 128)
> -			len = 128;
> -		res = sd_do_mode_sense(SRpnt, dbd, modepage, buffer,
> -				       len, &data);
> -	}
> +	if (!scsi_status_is_good(res))
> +		goto bad_sense;
> +
> +	/* that went OK, now ask for the proper length */
> +	len = data.length;
> +
> +	/*
> +	 * We're only interested in the first three bytes, actually.
> +	 * But the data cache page is defined for the first 20.
> +	 */
> +	if (len < 3)
> +		goto bad_sense;
> +	if (len > 20)
> +		len = 20;
> +
> +	/* Take headers and block descriptors into account */
> +	len += data.header_length + data.block_descriptor_length;
> +
> +	/* Get the data */
> +	res = sd_do_mode_sense(SRpnt, dbd, modepage, buffer, len, &data);
>  
>  	if (scsi_status_is_good(res)) {
>  		const char *types[] = {
> @@ -1133,23 +1145,26 @@
>  
>  		printk(KERN_NOTICE "SCSI device %s: drive cache: %s\n",
>  		       diskname, types[ct]);
> +
> +		return;
> +	}
> +
> +bad_sense:
> +	if ((SRpnt->sr_sense_buffer[0] & 0x70) == 0x70
> +	     && (SRpnt->sr_sense_buffer[2] & 0x0f) == ILLEGAL_REQUEST
> +	     /* ASC 0x24 ASCQ 0x00: Invalid field in CDB */
> +	     && SRpnt->sr_sense_buffer[12] == 0x24
> +	     && SRpnt->sr_sense_buffer[13] == 0x00) {
> +		printk(KERN_NOTICE "%s: cache data unavailable\n",
> +		       diskname);
>  	} else {
> -		if ((SRpnt->sr_sense_buffer[0] & 0x70) == 0x70
> -		     && (SRpnt->sr_sense_buffer[2] & 0x0f) == ILLEGAL_REQUEST
> -		     /* ASC 0x24 ASCQ 0x00: Invalid field in CDB */
> -		     && SRpnt->sr_sense_buffer[12] == 0x24
> -		     && SRpnt->sr_sense_buffer[13] == 0x00) {
> -			printk(KERN_NOTICE "%s: cache data unavailable\n",
> -			       diskname);
> -		} else {
> -			printk(KERN_ERR "%s: asking for cache data failed\n",
> -			       diskname);
> -		}
> -		printk(KERN_ERR "%s: assuming drive cache: write through\n",
> +		printk(KERN_ERR "%s: asking for cache data failed\n",
>  		       diskname);
> -		sdkp->WCE = 0;
> -		sdkp->RCD = 0;
>  	}
> +	printk(KERN_ERR "%s: assuming drive cache: write through\n",
> +	       diskname);
> +	sdkp->WCE = 0;
> +	sdkp->RCD = 0;
>  }
>  
>  /**
> 
> 
> 
> -------------------------------------------------------
> This sf.net email is sponsored by:ThinkGeek
> Welcome to geek heaven.
> http://thinkgeek.com/sf
> _______________________________________________
> linux-usb-devel@lists.sourceforge.net
> To unsubscribe, use the last form field at:
> https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
> 


  reply	other threads:[~2003-09-23 17:47 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20030922004943.E32009@one-eyed-alien.net>
2003-09-22 14:25 ` [linux-usb-devel] Re: USB storage problems on OHCI Alan Stern
2003-09-22 14:31   ` Christoph Hellwig
2003-09-22 15:11     ` Christoph Hellwig
2003-09-22 15:49       ` Patrick Mansfield
2003-09-22 16:09         ` [linux-usb-devel] " Linus Torvalds
2003-09-22 16:42           ` Alan Stern
2003-09-22 17:23           ` Patrick Mansfield
2003-09-22 17:41             ` [linux-usb-devel] " Linus Torvalds
2003-09-22 17:55               ` James Bottomley
2003-09-22 19:55                 ` [linux-usb-devel] " Linus Torvalds
2003-09-23 17:47                   ` Ruud Linders [this message]
2003-09-23 18:16                     ` Linus Torvalds
2003-09-24 16:40                       ` Ruud Linders
2003-09-24 16:53                         ` Linus Torvalds
2003-09-26 18:43                           ` Alan Stern
2003-10-03 14:18                             ` Alan Stern
2003-10-03 15:05                               ` James Bottomley
2003-10-03 21:35                                 ` Patrick Mansfield
2003-09-22 18:46           ` Alan Cox
2003-09-22 16:37         ` [linux-usb-devel] " Christoph Hellwig
2003-09-22 16:44           ` Patrick Mansfield
2003-09-22 17:21             ` Christoph Hellwig
2003-09-22 19:01               ` Alan Stern
2003-09-22 15:50       ` Alan Stern
2003-09-22 15:58         ` Patrick Mansfield
2003-09-22 16:36           ` [linux-usb-devel] " Alan Stern
2003-09-22 16:38         ` Christoph Hellwig
2003-09-22 15:29   ` [linux-usb-devel] " Linus Torvalds
2003-09-22 16:22     ` David Brownell
2003-09-22 16:31       ` [linux-usb-devel] " Linus Torvalds
2003-09-22 17:13         ` David Brownell
2003-09-22 17:29           ` Linus Torvalds
2003-09-22 17:49             ` [linux-usb-devel] " David Brownell
2003-09-22 18:40               ` Linus Torvalds
2003-09-22 19:14                 ` Alan Stern
2003-09-22 16:58       ` [linux-usb-devel] " Martin Diehl
2003-09-22 17:19         ` David Brownell
2003-09-22 18:55 Andries.Brouwer
2003-09-22 19:28 ` [linux-usb-devel] " James Bottomley
  -- strict thread matches above, loose matches on Subject: below --
2003-09-22 19:56 Andries.Brouwer
2003-09-22 20:48 ` Alan Stern
2003-09-22 20:51 ` Matthew Dharm
2003-09-23 14:04 ` James Bottomley
2003-09-22 22:55 Pat LaVarre
2003-09-29 14:54 ` Pat LaVarre
2003-09-22 23:07 Andries.Brouwer
2003-09-23 14:37 Andries.Brouwer
2003-09-23 14:51 ` James Bottomley
2003-09-23 15:46 ` Linus Torvalds
2003-09-23 15:23 Alan Stern
2003-09-24 14:10 ` [linux-usb-devel] " James Bottomley

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3F708718.4050401@xs4all.nl \
    --to=rkmp@xs4all.nl \
    --cc=Andries.Brouwer@cwi.nl \
    --cc=James.Bottomley@SteelEye.com \
    --cc=david-b@pacbell.net \
    --cc=greg@kroah.com \
    --cc=hch@infradead.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=linux-usb-devel@lists.sourceforge.net \
    --cc=mdharm-usb@one-eyed-alien.net \
    --cc=patmans@us.ibm.com \
    --cc=stern@rowland.harvard.edu \
    --cc=torvalds@osdl.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.