Re: [PATCH] ata_piix: fix pio/mwdma programming (for testing, don't apply)

[Tejun Heo <htejun@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1230 bytes --]

Mark Lord wrote:
> Tejun Heo wrote:
>> ..
>> Then, PACKET_IDENTIFY after configuring transfer mode fails with
>> -ENOENT.  Meaning it saw (status & (ATA_BUSY|ATA_DRQ|ATA_ERR|ATA_DF)) ==
>> 0 in HSM_ST.
> ..
>> So, PATA gurus, can you bless us with enlightenment?  :-)
> 
> Heh.. guaranteeing detection of all the strange implementations out there
> is part black magic.
> 
> But the simple thing to do here is, just for fun, hack the code
> to do the infamous 50 millisecond hard wait before issuing the
> PACKET_IDENTIFY.
> If that fixes it, then it's just a matter of tuning to discover the real
> amount of delay required, and a nicer way of doing the delay.

Okay.

> Also, zero out the features register before issuing PACKET_IDENTIFY,
> if the code isn't already doing that.

Okay.

> After the drive asserts BUSY, and later deasserts BUSY,
> there might be a slight delay before the drive asserts DRQ.
> So, it is possible for the status to read zeros in the important bits.
> 
> My suggestion is to wait up to the infamous 50 milliseconds again here,
> if needed.

Okay, the attached patch does what Mark suggested.  Art, can you please
give it a shot and report dmesg?  My thanks for sticking around till now.

-- 
tejun

[-- Attachment #2: wait-for-drq.patch --]
[-- Type: text/x-patch, Size: 1089 bytes --]

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 667acd2..4c81433 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -1478,7 +1478,8 @@ int ata_dev_read_id(struct ata_device *dev, unsigned int *p_class,
 	}
 
 	tf.protocol = ATA_PROT_PIO;
-	tf.flags |= ATA_TFLAG_POLLING; /* for polling presence detection */
+	/* POLLING for polling detection, ISADDR|DEVICE to clear TF */
+	tf.flags |= ATA_TFLAG_POLLING | ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
 
 	err_mask = ata_exec_internal(dev, &tf, NULL, DMA_FROM_DEVICE,
 				     id, sizeof(id[0]) * ATA_ID_WORDS);
@@ -4477,6 +4478,18 @@ fsm_start:
 				goto fsm_start;
 
 		} else {
+			unsigned long timeout = jiffies + msecs_to_jiffies(50);
+
+			while (time_before(jiffies, timeout)) {
+				if (status & ATA_DRQ)
+					break;
+				ata_dev_printk(qc->dev, KERN_INFO,
+					"XXX: waiting for DRQ, status=0x%x\n",
+					status);
+				msleep(10);
+				status = ata_chk_status(ap);
+			}
+
 			/* ATA PIO protocol */
 			if (unlikely((status & ATA_DRQ) == 0)) {
 				/* handle BSY=0, DRQ=0 as error */