Re: PATCH: usb-storage-psc1350-v4.patch (was Linux scsi / usb-mass-storage and HP printer cardreader bug + fix)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 2008-01-14 at 08:03 -0800, Matthew Dharm wrote:
> On Mon, Jan 14, 2008 at 10:46:56AM +0100, Hans de Goede wrote:
> > Guillaume Bedot wrote:
> > >But it fixes only two models.
> > >Do you think other devices (hp or not) can be impacted ?
> > >There are hundreds of models with card readers only for hp :
> > >http://hplip.sourceforge.net/supported_devices/combined.html
> > >
> > >Will this be possible to use "LAST_SECTOR_BUG" quirk for testing without
> > >recompiling a kernel ?
> > >
> > 
> > This is not possible AFAIK, I've already wrote a blog post about this 
> > asking for people to test this, but got no responses.
> 
> Once the patches are accepted by the SCSI people, one of the things we can
> consider doing is enabling this quirk for all USB devices.  It should be
> pretty harmless to all properly working devices, and the performance hit
> should be pretty minimal.

The SCSI patches look OK, with the stylistic points fixed below ...
we'll need two separate patches as well (one for SCSI, one for USB).

> We may be able to convince the SCSI people to enable it for all devices,
> regardless of HCD.

No ... I'm not particularly keen to have enterprise vendors after my
blood ...


> +       /* Some devices (some sdcards for one) don't like it if the last sector
> +          gets read in a larger then 1 sector read */

The comment style in sd is

/*
 * comment
 */

> +       if (sdp->last_sector_bug && rq->nr_sectors > sdp->sector_size / 512 &&

An unlikely() here, please to force the compiler to optimise for the
non-buggy case.  Plus what is the rq->nr_sectors > sdp->sector_size /
512 test supposed to be doing?  that being true is supposed to be a
guarantee of the block layer (and if something goes wrong there's a
check for this lower down).

> +           block + rq->nr_sectors == get_capacity(disk)) {

rq->nr_sectors should be this_count

> +               this_count -= sdp->sector_size / 512;

If you relocate this code to after the sector_size/this_count adjustment
code (i.e. about line 442) you can just do --this_count;

James


_______________________________________________
Fedora-kernel-list mailing list
Fedora-kernel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/fedora-kernel-list

[Index of Archives]     [Fedora General Discussion]     [Older Fedora Users Archive]     [Fedora Advisory Board]     [Fedora Security]     [Fedora Devel Java]     [Fedora Legacy]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Mentors]     [Fedora Package Announce]     [Fedora Package Review]     [Fedora Music]     [Fedora Packaging]     [Centos]     [Fedora SELinux]     [Coolkey]     [Yum Users]     [Tux]     [Yosemite News]     [KDE Users]     [Fedora Art]     [Fedora Docs]     [USB]     [Asterisk PBX]

  Powered by Linux