All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hans de Goede <j.w.r.degoede@hhs.nl>
To: Boaz Harrosh <bharrosh@panasas.com>
Cc: Guillaume Bedot <littletux@zarb.org>,
	USB development list <linux-usb-devel@lists.sourceforge.net>,
	linux-scsi@vger.kernel.org,
	USB Storage list <usb-storage@lists.one-eyed-alien.net>,
	linux-usb@vger.kernel.org, David Brown <usb-storage2@davidb.org>,
	fedora-kernel-list@redhat.com
Subject: Re: Linux scsi / usb-mass-storage and HP printer cardreader bug + fix
Date: Thu, 10 Jan 2008 11:52:56 +0100	[thread overview]
Message-ID: <4785F908.3080307@hhs.nl> (raw)
In-Reply-To: <4785F6CD.6050907@panasas.com>

Boaz Harrosh wrote:
> ----- Original Message -----
> *From:* Hans de Goede <j.w.r.degoede@hhs.nl>
> *To:* USB Storage list <usb-storage@lists.one-eyed-alien.net>
> *CC:* fedora-kernel-list@redhat.com, USB development list
> <linux-usb-devel@lists.sourceforge.net>, David Brown
> <usb-storage2@davidb.org>, Guillaume Bedot <littletux@zarb.org>,
> linux-scsi@vger.kernel.org, linux-usb@vger.kernel.org
> *Sent:* Wed, Jan 09 2008 at 23:44 +0200
> *Subject:* Linux scsi / usb-mass-storage and HP printer cardreader bug + fix
> 
> 
>> Hi All,
>>
>> First of all sorry for the somewhat massive cross-posting, I've spend a 
>> significant amount of time hunting down this bug, and so far the response has 
>> been less the overwhelming.
>>
>> The problem is with the HP PSC 1350 (my printer and confirmed by 2 others) and 
>> atleast also the HP PSC 1610 (confirmed by Guillaume Bedot, in the CC).
>>
>> The cardreader of the multi function printers will "crash" and from that moment 
>> on no longer communicate in any sane way, if you try to read the last sector of 
>> an sdcard* in a read that is more then 1 sector, so trying to read 8 sectors 
>> starting at sector capicity-8 will crash it, as will reading 2 sectors starting 
>> at sector capicity-2, however reading the last sector in a one 1 sector read 
>> will succeed! (* xdcards seem to be fine).
>>
>> I haven't tried if it will crash on larger then 1 sector writes which include 
>> the last sector too, I immediately added code to not do that in both the read 
>> and write paths. I have tested reading and writing the end of the disk with 
>> this kludge in and it works.
>>
>> I currently have a somewhat ugly proof of concept patch for this, which adds 
>> another type of usb-massstorage quirk. When this quirk flag is set, the 
>> usb-massstorage driver modifies READ_10 and WRITE_10 commands of more then 1 
>> sector which includes the last sector to become one sector less. I've been told 
>> by scsi subsystem developers that doing a shorter read / write then requested 
>> is not a problem, the scsi subsystem is designed to handle getting less then it 
>> asked for and will send a seperate request for the last sector.
>>
>> I and 3 others (2 on a PSC 1350 too, one on a PSC1610) have tested this patch 
>> with success. I'm not asking for this patch to be included to the kernel as is, 
>> I'm asking for the now known workaround for this to be added to the kernel in 
>> someway!
>>
>> Perhaps its an idea to add the posibility to have a scsi command filter 
>> function / callback to the scsi or usb-massstorage subsystem, and then add a 
>> mechanism to set this filter depending on usb id's and if added to the scsi 
>> layer, a mechanism to set it based on scsi device and manufacturer 
>> identification strings. Such a mechanism might be usefull in the future to work 
>> around other broken hardware too, and has the added advantage of not having 
>> todo much changes to the normal code path, keep that readable.
>>
>> I'm willing to come up with a patch for such a filter mechanism, provided I get 
>> some pointers where this is best added.
>>
>> Thanks & Regards,
>>
>> Hans
>>
>>
>> p.s.
>>
>> I've also included the fedora-kernel list in the addressee's because I was 
>> hoping that maybe someone can take one of these printers to the kernel hackfest 
>>    in the weekend's fudcon and take a look at this.
>>   
>> +		if ((offset + num) == sdkp->capacity && num > 1) {
>> +			if (srb->cmnd[8] == 0)
>> +				srb->cmnd[7]--;
>> +			srb->cmnd[8]--;
>> +			srb->request_bufflen -= 512;
>> +			srb->underflow -= 512;
>> +		}
>>   
> This will no longer compile on top of latest scsi-misc, and
> LLDs are not suppose to modify request_bufflen anymore.
> 
> I'm not sure what the proper solution should be?
> 

I guess the proper solution would be to add a special case to the scsi layer 
where the read10 / write10 command is issued, and split the request in 2 there 
when it involves the last sector.

There was another reply in this thread stating that problems reading the last 
sector with sd / mmc cards happen quite often, and that this is most likely not 
an isolated case.

Regards,

Hans


-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
_______________________________________________
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:[~2008-01-10 10:52 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-01-09 21:44 Linux scsi / usb-mass-storage and HP printer cardreader bug + fix Hans de Goede
2008-01-09 22:10 ` Matthew Dharm
2008-01-10 10:43 ` Boaz Harrosh
2008-01-10 10:52   ` Hans de Goede [this message]
2008-01-10 11:27     ` Boaz Harrosh
2008-01-11 12:48       ` Hans de Goede
2008-01-11 13:57         ` Guillaume Bedot
     [not found]       ` <47860106.3090509-C4P08NqkoRlBDgjK7y7TUQ@public.gmane.org>
2008-01-11 20:14         ` PATCH: usb-storage-psc1350-v4.patch (was Linux scsi / usb-mass-storage and HP printer cardreader bug + fix) Hans de Goede
2008-01-11 20:34           ` [usb-storage] " Matthew Dharm
2008-01-14  9:40           ` Guillaume Bedot
2008-01-14  9:46             ` Hans de Goede
2008-01-14 16:03               ` Matthew Dharm
     [not found]                 ` <20080114160310.GH14375-JGfshJpz5UybPZpvUQj5UqxOck334EZe@public.gmane.org>
2008-01-14 16:33                   ` James Bottomley
2008-01-14 18:37                     ` Hans de Goede
2008-01-14 19:09                       ` James Bottomley
2008-01-14 19:27                         ` Hans de Goede
2008-01-14 20:20                           ` James Bottomley
     [not found]                             ` <1200342046.3159.64.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2008-01-20 10:12                               ` Hans de Goede
     [not found]                     ` <1200328388.3159.20.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2008-01-14 18:40                       ` Stefan Richter
2008-01-14 19:01                     ` Matthew Dharm
2008-01-14 19:10                       ` Hans de Goede
2008-04-25  5:43             ` (unknown), wangzhilei-pcEzf3JNfARxfCqBhyfcug

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=4785F908.3080307@hhs.nl \
    --to=j.w.r.degoede@hhs.nl \
    --cc=bharrosh@panasas.com \
    --cc=fedora-kernel-list@redhat.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=linux-usb-devel@lists.sourceforge.net \
    --cc=linux-usb@vger.kernel.org \
    --cc=littletux@zarb.org \
    --cc=usb-storage2@davidb.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 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.