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@xxxxxxxxxxx>
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@xxxxxxxxxxx>
---
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
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
