Re: Linux scsi / usb-mass-storage and HP printer cardreader bug + fix
Hello, Le vendredi 11 janvier 2008 à 13:48 +0100, Hans de Goede a écrit : > That will work nicely, I'll write an updated patch this evening (when I have > access to the printer to test again). > Great news, i am impatient to test this new patch. I may face an other bug with the Transcend 1GB SD card, would be possible that the patch would be available for latests kernels ? Thanks in advance, Best regards, Guillaume B. - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Linux scsi / usb-mass-storage and HP printer cardreader bug + fix
Boaz Harrosh wrote: On Thu, Jan 10 2008 at 12:52 +0200, Hans de Goede <[EMAIL PROTECTED]> wrote: 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 Yes, you're right. in ULDs it is a much proper way to do this. So I guess you'll have to do that special host flag or device flag, and add a check for it in sd.c. You'll see that sd.c is already doing bufflen truncation at sd_prep_fn(), just add one more case. Yes, That will work nicely, I'll write an updated patch this evening (when I have access to the printer to test again). Regards, Hans - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Linux scsi / usb-mass-storage and HP printer cardreader bug + fix
On Thu, Jan 10 2008 at 12:52 +0200, Hans de Goede <[EMAIL PROTECTED]> wrote:
> Boaz Harrosh wrote:
>> - Original Message -
>> *From:* Hans de Goede <[EMAIL PROTECTED]>
>> *To:* USB Storage list <[EMAIL PROTECTED]>
>> *CC:* [EMAIL PROTECTED], USB development list
>> <[EMAIL PROTECTED]>, David Brown
>> <[EMAIL PROTECTED]>, Guillaume Bedot <[EMAIL PROTECTED]>,
>> linux-scsi@vger.kernel.org, [EMAIL PROTECTED]
>> *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
Yes, you're right. in ULDs it is a much proper way to do this.
So I guess you'll have to do that special host flag or device
flag, and add a check for it in sd.c. You'll see that sd.c is
already doing bufflen truncation at sd_prep_fn(), just add one
more case.
Boaz
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Linux scsi / usb-mass-storage and HP printer cardreader bug + fix
Boaz Harrosh wrote:
- Original Message -
*From:* Hans de Goede <[EMAIL PROTECTED]>
*To:* USB Storage list <[EMAIL PROTECTED]>
*CC:* [EMAIL PROTECTED], USB development list
<[EMAIL PROTECTED]>, David Brown
<[EMAIL PROTECTED]>, Guillaume Bedot <[EMAIL PROTECTED]>,
linux-scsi@vger.kernel.org, [EMAIL PROTECTED]
*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
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Linux scsi / usb-mass-storage and HP printer cardreader bug + fix
- Original Message -
*From:* Hans de Goede <[EMAIL PROTECTED]>
*To:* USB Storage list <[EMAIL PROTECTED]>
*CC:* [EMAIL PROTECTED], USB development list
<[EMAIL PROTECTED]>, David Brown
<[EMAIL PROTECTED]>, Guillaume Bedot <[EMAIL PROTECTED]>,
linux-scsi@vger.kernel.org, [EMAIL PROTECTED]
*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?
Boaz
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
