lkml.org 
[lkml]   [2009]   [Feb]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: 2.6.29-rc libata sff 32bit PIO regression


    Hugh Dickins wrote:
    > On Mon, 26 Jan 2009, Alan Cox wrote:
    >
    >>> [PATCH] libata sff: 32bit PIO use 16bit on slop
    >>>
    >>> 871af1210f13966ab911ed2166e4ab2ce775b99d libata: Add 32bit PIO support
    >>> causes errors on a four-year-old ata_piix Dell Precision 670. Using
    >>> 16bit PIO instead of 32bit PIO on the odd 1, 2 or 3 chars fixes that.
    >>>
    >>> Signed-off-by: Hugh Dickins <hugh@veritas.com>
    >>>
    >> For the 3 bytes of slop it should use a single iowrite32 but otherwise
    >> that seems ok. We do need to handle the FIFO setup on the AMD differently
    >> if we do this ...
    >>
    >
    > Sorry, I believe you were waiting on me for this, to accompany your
    > AMD and VLB patches. I'm afraid I don't have any such AMD devices
    > to test this along with yours, and the only non-0 slop that I've seen
    > in testing has been 2 (about 25% of ops, so I removed the "unlikely").
    > But this patch works as well for me as the patch I posted before
    > (though much more verbose: please simplify if you see a better way).
    >
    >
    > [PATCH] libata sff: 32bit PIO use 16bit on slop
    >
    > 871af1210f13966ab911ed2166e4ab2ce775b99d libata: Add 32bit PIO support
    > causes errors on a four-year-old ata_piix Dell Precision 670. Using
    > 16bit PIO instead of 32bit PIO on the odd 1 or 2 chars fixes that,
    > but Alan Cox indicates that we should still use 32bit for 3 chars.
    >
    > Signed-off-by: Hugh Dickins <hugh@veritas.com>
    > ---
    >
    > drivers/ata/libata-sff.c | 33 ++++++++++++++++++++++++---------
    > 1 file changed, 24 insertions(+), 9 deletions(-)
    >
    > --- 2.6.29-rc3/drivers/ata/libata-sff.c 2009-01-29 12:33:28.000000000 +0000
    > +++ linux/drivers/ata/libata-sff.c 2009-02-01 20:21:13.000000000 +0000
    > @@ -773,18 +773,33 @@ unsigned int ata_sff_data_xfer32(struct
    > else
    > iowrite32_rep(data_addr, buf, words);
    >
    > - if (unlikely(slop)) {
    > - __le32 pad;
    > - if (rw == READ) {
    > - pad = cpu_to_le32(ioread32(ap->ioaddr.data_addr));
    > - memcpy(buf + buflen - slop, &pad, slop);
    > + if (slop) {
    > + unsigned char *trailing_buf = buf + buflen - slop;
    >
    > +
    >
    > + if (slop <= 2) {
    > + __le16 slop_word;
    > + if (rw == READ) {
    > + slop_word = cpu_to_le16(ioread16(data_addr));
    > + memcpy(trailing_buf, &slop_word, slop);
    > + } else {
    > + slop_word = 0;
    > + memcpy(&slop_word, trailing_buf, slop);
    > + iowrite16(le16_to_cpu(slop_word), data_addr);
    > + }
    > } else {
    > - memcpy(&pad, buf + buflen - slop, slop);
    > - iowrite32(le32_to_cpu(pad), ap->ioaddr.data_addr);
    > + __le32 slop_word;
    > + if (rw == READ) {
    > + slop_word = cpu_to_le32(ioread32(data_addr));
    > + memcpy(trailing_buf, &slop_word, slop);
    > + } else {
    > + slop_word = 0;
    > + memcpy(&slop_word, trailing_buf, slop);
    > + iowrite32(le32_to_cpu(slop_word), data_addr);
    > + }
    >

    How about the following?

    unsigned char *tail = buf + buflen - slop;
    unsigned char pad[4];

    if (rw == READ) {
    if (slop <= 2)
    ioread16_rep(data_addr, pad, 1);
    else
    ioread32_rep(data_addr, pad, 1);
    memcpy(tail, pad, slop);
    } else {
    memcpy(pad, tail, slop);
    memset(pad + slop, 0, 4 - slop);
    if (slop <= 2)
    iowrite16_rep(data_addr, pad, 1);
    else
    iowrite32_rep(data_addr, pad, 1);
    }

    > }
    > - return words << 2;
    > +
    > + return buflen + (buflen & 1);
    >

    return (buflen + 1) & ~1;

    Well, I guess I could just have posted my own patch... :-)

    MBR, Sergei




    \
     
     \ /
      Last update: 2009-02-02 12:51    [W:0.045 / U:30.984 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site