public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Robert Hancock <hancockr@shaw.ca>
To: Tomas Styblo <tripie@cpan.org>
Cc: linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
	usb-storage@lists.one-eyed-alien.net,
	Alan Stern <stern@rowland.harvard.edu>
Subject: Re: [PATCH] JMicron JM20337 USB-SATA data corruption bugfix -	device 152d:2338
Date: Tue, 22 Jul 2008 02:45:32 -0600	[thread overview]
Message-ID: <48859E2C.4070805@shaw.ca> (raw)
In-Reply-To: <20080722061147.GB8303@notebook.homenet.local>

Tomas Styblo wrote:
> * Robert Hancock <hancockr@shaw.ca> [Tue, 22 Jul 2008]:
>> In any case, given that your code apparently fixes the corruption it seems 
>> that srb->result is being set to SAM_STAT_CHECK_CONDITION, but the 
>> DID_ERROR and SUGGEST_RETRY flags are not being set. Presumably then the  
>> SCSI layer looks at the sense data and says "hmm, nothing to worry about 
>> here" and carries on.
> 
> That's exactly what I thought was happening, after a cursory 
> look at the SCSI code.
> 
>> I think we do need something like your patch, though it should likely be 
>> moved inside the if (need_auto_sense) check, and I don't see a reason to 
>> limit to this device ID only.
> 
> Thank you. This is a very insidious bug as it doesn't manifest
> itself very often, months of data corruption may pass before you
> notice it.
> 
> So is there a bug in the chipset, or does the error handling code 
> not follow specifications?

It looks clear to me that it's a bug in the chipset. It's supposed to 
set some valid sense data if an error occurs, not just set the "failed" 
flag in the USB storage status word. (Presumably the fact that these 
errors are occurring in the first place is a bug in itself.. though that 
could be a problem with the enclosure or drive as well.)

However the kernel should be more robust and not ignore the error 
indication that it is giving.

> I wonder if the company that makes the chipset should be notified
> about this problem?

I suppose it wouldn't hurt to let JMicron know about this. I doubt they 
could do anything for existing chipsets, but it might help them avoid 
this bug in future designs.

  reply	other threads:[~2008-07-22  8:45 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
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 [this message]
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=48859E2C.4070805@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