Re: 2.6.29-rc libata sff 32bit PIO regression

[Mikael Pettersson]

Hugh Dickins writes:
 > I've a Dell Precision 670 here (four-year-old EM64T Xeon with ata_piix)
 > which doesn't like your commit 871af1210f13966ab911ed2166e4ab2ce775b99d
 > libata: Add 32bit PIO support.  Full dmesg (and .config) attached, but
 > here's an extract showing the start of the error messages on ata2:
 > 
 > ata1: PATA max UDMA/100 cmd 0x1f0 ctl 0x3f6 bmdma 0xffa0 irq 14
 > ata2: PATA max UDMA/100 cmd 0x170 ctl 0x376 bmdma 0xffa8 irq 15
 > ata1: port disabled. ignoring.
 > ata2.00: ATAPI: GCR-8483B, 1.07, max UDMA/33
 > ata2.00: configured for UDMA/33
 > scsi 1:0:0:0: CD-ROM            HL-DT-ST CD-ROM GCR-8483B 1.07 PQ: 0 ANSI: 5
 > ata2.00: qc timeout (cmd 0xa0)
 > ata2.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
 > ata2.00: cmd a0/00:00:00:00:00/00:00:00:00:00/a0 tag 0
 >          cdb 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
 >          res 51/20:03:00:00:00/00:00:00:00:00/a0 Emask 0x5 (timeout)
 > ata2.00: status: { DRDY ERR }
 > ata2: soft resetting link
 > ata2.00: configured for UDMA/33
 > ata2: EH complete
 > 
 > linux-next behaved the same, so I played around, and the patch below seems
 > to fix it fine (though I've not yet tried it on other, working machines);
 > but I can well imagine you may prefer to do it differently.  Or was I just
 > wrong to move that box's CD handling from ide to libata a few months ago?
 > 
 > [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>
 > ---
 > 
 >  drivers/ata/libata-sff.c |   18 +++++++++++-------
 >  1 file changed, 11 insertions(+), 7 deletions(-)
 > 
 > --- 2.6.29-rc2/drivers/ata/libata-sff.c	2009-01-17 10:40:12.000000000 +0000
 > +++ linux/drivers/ata/libata-sff.c	2009-01-21 12:32:01.000000000 +0000
 > @@ -774,17 +774,21 @@ unsigned int ata_sff_data_xfer32(struct 
 >  		iowrite32_rep(data_addr, buf, words);
 >  
 >  	if (unlikely(slop)) {
 > -		__le32 pad;
 > +		unsigned char slop_buf[4];
 > +		unsigned char *trailing_buf = buf + buflen - slop;
 > +
 > +		words = (slop + 1) >> 1;
 >  		if (rw == READ) {
 > -			pad = cpu_to_le32(ioread32(ap->ioaddr.data_addr));
 > -			memcpy(buf + buflen - slop, &pad, slop);
 > +			ioread16_rep(data_addr, slop_buf, words);
 > +			memcpy(trailing_buf, slop_buf, slop);
 >  		} else {
 > -			memcpy(&pad, buf + buflen - slop, slop);
 > -			iowrite32(le32_to_cpu(pad), ap->ioaddr.data_addr);
 > +			memcpy(slop_buf, trailing_buf, slop);
 > +			slop_buf[slop] = 0;
 > +			iowrite16_rep(data_addr, slop_buf, words);
 >  		}
 > -		words++;
 >  	}
 > -	return words << 2;
 > +
 > +	return buflen + (slop & 1);
 >  }
 >  EXPORT_SYMBOL_GPL(ata_sff_data_xfer32);

Thanks, this patch also fixes the 2.6.29-rc bmdma32-related ata_piix regression
I reported, where having a specific CD drive connected causes a complete system
hang when init starts udev <http://marc.info/?l=linux-ide&m=123244042628229&w=2>.

Tested-by: Mikael Pettersson <mikpe@xxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/