public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Robert Hancock <hancockr@shaw.ca>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Tomas Styblo <tripie@cpan.org>,
	linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
	usb-storage@lists.one-eyed-alien.net
Subject: Re: [PATCH] JMicron JM20337 USB-SATA data corruption bugfix - device 152d:2338
Date: Wed, 23 Jul 2008 17:20:25 -0600	[thread overview]
Message-ID: <4887BCB9.2000103@shaw.ca> (raw)
In-Reply-To: <Pine.LNX.4.44L0.0807222238060.12003-100000@netrider.rowland.org>

Alan Stern wrote:
> The system doesn't detect the first problem for two reasons: The
> scsi_eh_prep_cmnd routine fails to preserve scmd->underflow, and
> usb-storage fails to check for underflow when the device returns Check
> Condition status.  The Bugzilla report contains fixes for both of
> these, and they solve the problem.
> 
> The second problem is harder.  The device is supposed to send a STALL
> to cut the WRITE short -- and a USB trace under Windows seems to
> indicate that it does -- but under Linux no STALL is received.  I
> don't know why not.  But since Linux does not respond in the way the
> device expects, the device crashes.
> 
>> Anyway, I'll send the patch to this person so he can try it. 
>> I guess it won't fix his problem. This patch is much simpler and doesn't 
>> need any delays - I really think this is a different situation.
> 
> It isn't.  And your patch is an ad-hoc correction that doesn't address
> the true underlying reasons for the errors.

Tomas, you should try the patch listed in that bug report (well, except 
maybe the part that's actually adding the JMicron unusual devices entry) 
and see if that fixes the problem.

It remains an issue, though, that if there's no underflow, if the device 
reports an error in the CSW but doesn't provide sense data, we assume 
nothing bad happened and don't retry. That definitely does not seem 
correct. The device is not supposed to do this, but with how crappily 
some of these devices are designed we should be more defensive.

It's pretty gross to add random delays in without some good evidence or 
input from the manufacturer that it will really fix the problem. (We do 
have one GO_SLOW flag with a delay in there already for some Genesys 
chips, but I believe that was based on some actual input from Genesys.)

> 
> You should also try adding the delay mentioned in the bug report.
> There's an excellent chance it will also prevent your problems.
> 
>> I sometimes experience the problems described by this person, as I
>> noted in the first message with the patch. When these "reset high
>> speed USB device" messages appear, it is usually necessary to
>> disconnect and power off the device.
> 
> Because the device's firmware has crashed.  That's why the reset fails.
> 
> Alan Stern
> 
> 

  reply	other threads:[~2008-07-23 23:20 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <fa.eWQqoO1e4Mdhu8SK5jzMFAI/3NU@ifi.uio.no>
2008-07-21 19:37 ` [PATCH] JMicron JM20337 USB-SATA data corruption bugfix - device 152d:2338 Robert Hancock
2008-07-22  2:37   ` Alan Stern
2008-07-22  9:03     ` Tomas Styblo
2008-07-23  2:38       ` Alan Stern
2008-07-23 23:20         ` Robert Hancock [this message]
2008-07-24  3:42           ` Alan Stern
2008-07-30 19:55             ` Robert Hancock
2008-07-30 20:00               ` [usb-storage] " Matthew Dharm
2008-07-30 20:46                 ` Robert Hancock
2008-07-30 21:18                 ` Alan Stern
2008-07-30 21:07               ` Alan Stern
2008-08-01 21:15                 ` Alex Buell
2008-08-01 22:22                   ` Alex Buell
2008-08-02  2:32                     ` Robert Hancock
2008-08-02 23:49                     ` Alan Stern
2008-08-03  9:07                       ` Alex Buell
2008-08-04 16:48                         ` Alan Stern
2008-08-04 20:17                           ` Alex Buell
2008-08-04 20:45                             ` Alan Stern
2008-09-02 12:10                               ` Thiago Galesi
2008-09-02 15:02                                 ` Alan Stern
2008-09-02 16:07                                   ` Thiago Galesi
2008-09-02 19:19                                     ` Alan Stern
2008-09-02 20:16                                       ` Thiago Galesi
2008-09-02 21:06                                         ` Alan Stern
2008-09-04 12:09                                           ` Thiago Galesi
2008-09-04 14:03                                             ` Alan Stern
2008-09-04 15:17                                               ` Thiago Galesi
2008-09-04 15:26                                                 ` Alan Stern
2008-07-25  8:44           ` Tomas Styblo
2008-07-25  8:54             ` Robert Hancock
2008-07-22  5:11   ` Tomas Styblo
2008-07-22  5:31     ` Robert Hancock
2008-07-22  6:11       ` Tomas Styblo
2008-07-22  8:45         ` Robert Hancock
2008-07-24  6:15           ` Alex Buell
2008-07-29 21:09           ` Alex Buell
2008-07-29 22:33             ` [usb-storage] " Matthew Dharm
2008-07-20  5:13 Tomas Styblo

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=4887BCB9.2000103@shaw.ca \
    --to=hancockr@shaw.ca \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=stern@rowland.harvard.edu \
    --cc=tripie@cpan.org \
    --cc=usb-storage@lists.one-eyed-alien.net \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox