On Thu, Jun 05, 2008 at 12:21:29PM +0200, Nick Piggin wrote: > On Thu, Jun 05, 2008 at 10:24:24AM +0100, Alan Cox wrote: > > > And when booting with the commit applied, I instead get a whole lot of > > > messages like this (this is the first one, copied by hand): > > > > > > ata2.00: exception Emask 0x0 SAct 0x0 Serr 0x10000000 action 0x6 frozen > > > ata2: SError: { } > > > ata2.00: cmd c8/00:02:42:08:20/00:00:00:00:00/e0 tag 0 dma 1024 in > > > res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout) > > > ata2.00: status: { DRDY } > > > ata2: hard resetting link > > > ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300) > > > ata2.00: configured for UDMA/133 > > > ata2: EH complete > > > > Well I've been over the patch twice now and I cannot see a single point > > at which the sequence of code that *should* be executed is any different. > > If it is of any help to you, doing this: > diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c > index 90d20c6..f6cb8d7 100644 > --- a/drivers/ata/libata-sff.c > +++ b/drivers/ata/libata-sff.c > @@ -1583,6 +1583,12 @@ inline unsigned int ata_sff_host_intr(struct ata_port *ap > if (status & ATA_BUSY) > goto idle_irq; > > + /* check main status, clearing INTRQ */ > + status = ap->ops->sff_check_status(ap); > + if (unlikely(status & ATA_BUSY)) > + goto idle_irq; > + > + > /* ack bmdma irq events */ > ap->ops->sff_irq_clear(ap); > > Gets it working again... > > > > Stick wmb();rmb(); (or similar barriers to compiler optimisation and I/O > > fencing) at the start and end of your ata_sff_altstatus() and see what > > happens, if it suddenly decides to behave or forcing it no inline makes > > it behave then that would be useful info. > > Will give that a try next And doing this made no difference diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c index 90d20c6..db6be15 100644 --- a/drivers/ata/libata-sff.c +++ b/drivers/ata/libata-sff.c @@ -249,10 +249,18 @@ u8 ata_sff_check_status(struct ata_port *ap) */ static u8 ata_sff_altstatus(struct ata_port *ap) { + u8 ret; + + mb(); + if (ap->ops->sff_check_altstatus) - return ap->ops->sff_check_altstatus(ap); + ret = ap->ops->sff_check_altstatus(ap); + + ret = ioread8(ap->ioaddr.altstatus_addr); - return ioread8(ap->ioaddr.altstatus_addr); + mb(); + + return ret; } /** -- 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