lkml.org 
[lkml]   [2009]   [Feb]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] libata-sff: fix 32-bit PIO regression
    On Sun, 8 Feb 2009, Sergei Shtylyov wrote:

    > Commit 871af1210f13966ab911ed2166e4ab2ce775b99d (libata: Add 32bit PIO support)
    > caused all kind of errors on the ATAPI devices, so it's been empirically proven
    > that one shouldn't read/write an extra data word when a device isn't expecting
    > it already. "Don't do it then"; however still taking a chance to use 32-bit I/O
    > one last time when there are exactly 3 trailing bytes. Oh, and stop pointless
    > swapping bytes to and fro as well by using io*_rep() which shouldn't byte-swap.
    >
    > This should fix the kernel.org bug #12609.
    >
    > ---
    > This is hopefully better replacement for Hugh Dickins most recent patch
    > (http://marc.info/?l=linux-ide&m=123352294619281)...

    Yes, looks nice, and works for me. My only criticism would be,
    minor issue unchanged by your patch, that actually "slop" isn't
    unlikely enough to deserve an "unlikely" - slop of 1 or 3 is
    unlikely, but slop of 2 is quite common.

    Hugh

    >
    > drivers/ata/libata-sff.c | 23 ++++++++++++++++-------
    > 1 files changed, 16 insertions(+), 7 deletions(-)
    >
    > Index: linux-2.6/drivers/ata/libata-sff.c
    > ===================================================================
    > --- linux-2.6.orig/drivers/ata/libata-sff.c
    > +++ linux-2.6/drivers/ata/libata-sff.c
    > @@ -773,18 +773,27 @@ unsigned int ata_sff_data_xfer32(struct
    > else
    > iowrite32_rep(data_addr, buf, words);
    >
    > + /* Transfer trailing bytes, if any */
    > if (unlikely(slop)) {
    > - __le32 pad;
    > + unsigned char pad[4];
    > +
    > + /* Point buf to the tail of buffer */
    > + buf += buflen - slop;
    > if (rw == READ) {
    > - pad = cpu_to_le32(ioread32(ap->ioaddr.data_addr));
    > - memcpy(buf + buflen - slop, &pad, slop);
    > + if (slop < 3)
    > + ioread16_rep(data_addr, pad, 1);
    > + else
    > + ioread32_rep(data_addr, pad, 1);
    > + memcpy(buf, pad, slop);
    > } else {
    > - memcpy(&pad, buf + buflen - slop, slop);
    > - iowrite32(le32_to_cpu(pad), ap->ioaddr.data_addr);
    > + memcpy(pad, buf, slop);
    > + if (slop < 3)
    > + iowrite16_rep(data_addr, pad, 1);
    > + else
    > + iowrite32_rep(data_addr, pad, 1);
    > }
    > - words++;
    > }
    > - return words << 2;
    > + return (buflen + 1) & ~1;
    > }
    > EXPORT_SYMBOL_GPL(ata_sff_data_xfer32);
    >


    \
     
     \ /
      Last update: 2009-02-08 22:43    [W:4.124 / U:0.120 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site