linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/1] libata-dev: ata_check_atapi_dma() fix for ATA_FLAG_PIO_POLLING LLDDs
@ 2006-03-13  7:57 Albert Lee
  2006-03-13  8:09 ` Jeff Garzik
  0 siblings, 1 reply; 10+ messages in thread
From: Albert Lee @ 2006-03-13  7:57 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: IDE Linux, Tejun Heo, Doug Maxey

ata_check_atapi_dma() fix for LLDDs with the ATA_FLAG_PIO_POLLING flag.

Signed-off-by: Albert Lee <albertcc@tw.ibm.com>
---

We don't support polling DMA.
If the LLDD handles only interrupts in the HSM_ST_LAST state 
(indicated by the ATA_FLAG_PIO_POLLING flag) and the ATAPI device
generates CDB interrupts, we have to use the polling PIO protocol.
(Otherwise, the CDB interrupts might confuse the LLDD.)

We just set the protocol to PIO here in ata_check_atapi_dma().
Later in ata_qc_issue_prot(), the PIO protocol will be set as "polling"
for those LLDDs.

Patch against the irq-pio branch +
(1) irq-pio minor fixes (respin) +
(2) integrate polling pio with irq-pio (respin)

For your review, thanks,

Albert

--- 05_cleanup/drivers/scsi/libata-core.c	2006-03-13 13:29:02.000000000 +0800
+++ 06_check_atapi_dma/drivers/scsi/libata-core.c	2006-03-13 13:29:25.000000000 +0800
@@ -2850,6 +2850,15 @@ int ata_check_atapi_dma(struct ata_queue
 	if (ap->ops->check_atapi_dma)
 		rc = ap->ops->check_atapi_dma(qc);
 
+	/* We don't support polling DMA.
+	 * Use PIO if the LLDD handles only interrupts in
+	 * the HSM_ST_LAST state and the ATAPI device
+	 * generates CDB interrupts.
+	 */
+	if ((ap->flags & ATA_FLAG_PIO_POLLING) &&
+	    (qc->dev->flags & ATA_DFLAG_CDB_INTR))
+		rc = 1;
+
 	return rc;
 }
 /**
@@ -4004,6 +4013,7 @@ unsigned int ata_qc_issue_prot(struct at
 			break;
 		case ATA_PROT_ATAPI_DMA:
 			if (qc->dev->flags & ATA_DFLAG_CDB_INTR)
+				/* see ata_check_atapi_dma() */
 				BUG();
 			break;
 		default:


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/1] libata-dev: ata_check_atapi_dma() fix for ATA_FLAG_PIO_POLLING LLDDs
  2006-03-13  7:57 [PATCH 1/1] libata-dev: ata_check_atapi_dma() fix for ATA_FLAG_PIO_POLLING LLDDs Albert Lee
@ 2006-03-13  8:09 ` Jeff Garzik
  2006-03-14  4:52   ` Albert Lee
  0 siblings, 1 reply; 10+ messages in thread
From: Jeff Garzik @ 2006-03-13  8:09 UTC (permalink / raw)
  To: albertl; +Cc: IDE Linux, Tejun Heo, Doug Maxey

Albert Lee wrote:
> ata_check_atapi_dma() fix for LLDDs with the ATA_FLAG_PIO_POLLING flag.
> 
> Signed-off-by: Albert Lee <albertcc@tw.ibm.com>
> ---
> 
> We don't support polling DMA.
> If the LLDD handles only interrupts in the HSM_ST_LAST state 
> (indicated by the ATA_FLAG_PIO_POLLING flag) and the ATAPI device
> generates CDB interrupts, we have to use the polling PIO protocol.
> (Otherwise, the CDB interrupts might confuse the LLDD.)
> 
> We just set the protocol to PIO here in ata_check_atapi_dma().
> Later in ata_qc_issue_prot(), the PIO protocol will be set as "polling"
> for those LLDDs.
> 
> Patch against the irq-pio branch +
> (1) irq-pio minor fixes (respin) +
> (2) integrate polling pio with irq-pio (respin)

Applied, though I think its an open question what happens with 
CDB-interrupt ATAPI devices want to do DMA.  Perhaps we just get an 
interrupt event that we clear, then life proceeds as normal.

Does anybody actually have such a device anywhere?  :)

	Jeff




^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/1] libata-dev: ata_check_atapi_dma() fix for ATA_FLAG_PIO_POLLING LLDDs
  2006-03-13  8:09 ` Jeff Garzik
@ 2006-03-14  4:52   ` Albert Lee
  2006-03-15 15:54     ` Mark Lord
  0 siblings, 1 reply; 10+ messages in thread
From: Albert Lee @ 2006-03-14  4:52 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: IDE Linux, Tejun Heo, Doug Maxey

Jeff Garzik wrote:

> 
> Applied, though I think its an open question what happens with
> CDB-interrupt ATAPI devices want to do DMA.  Perhaps we just get an
> interrupt event that we clear, then life proceeds as normal.

Not sure. I have no such device at hand for test; currently relying
on the SFF-8020i spec (http://www.bswd.com/sff8020i.pdf) as reference.
According to p.31, we have to send out the CDB after receiving
the CDB interrupt. So, just clearing the irq looks not enough.

> 
> Does anybody actually have such a device anywhere?  :)
> 

Maybe Alan or Mark knows? 

We really need tester for such device.
How about the attached patch?

---
Albert

--- 07_followup/drivers/scsi/libata-core.c	2006-03-13 16:25:52.000000000 +0800
+++ 08_need_tester/drivers/scsi/libata-core.c	2006-03-14 12:45:09.000000000 +0800
@@ -1294,8 +1294,12 @@ static int ata_dev_configure(struct ata_
 		}
 		dev->cdb_len = (unsigned int) rc;
 
-		if (ata_id_cdb_intr(dev->id))
+		if (ata_id_cdb_intr(dev->id)) {
 			dev->flags |= ATA_DFLAG_CDB_INTR;
+			printk(KERN_INFO "This ATAPI device generates CDB intr.\n"
+			       "Please send an email to linux-ide@vger.kernel.org\n"
+			       "if you are interested to help testing such device.\n");
+		}
 
 		/* print device info to dmesg */
 		if (print_info)



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/1] libata-dev: ata_check_atapi_dma() fix for ATA_FLAG_PIO_POLLING LLDDs
  2006-03-14  4:52   ` Albert Lee
@ 2006-03-15 15:54     ` Mark Lord
  2006-03-15 16:12       ` Mark Lord
  0 siblings, 1 reply; 10+ messages in thread
From: Mark Lord @ 2006-03-15 15:54 UTC (permalink / raw)
  To: albertl; +Cc: Jeff Garzik, IDE Linux, Tejun Heo, Doug Maxey

Albert Lee wrote:
> Jeff Garzik wrote:
> 
>> Applied, though I think its an open question what happens with
>> CDB-interrupt ATAPI devices want to do DMA.  Perhaps we just get an
>> interrupt event that we clear, then life proceeds as normal.
> 
> Not sure. I have no such device at hand for test; currently relying
> on the SFF-8020i spec (http://www.bswd.com/sff8020i.pdf) as reference.
> According to p.31, we have to send out the CDB after receiving
> the CDB interrupt. So, just clearing the irq looks not enough.
> 
>> Does anybody actually have such a device anywhere?  :)
> 
> Maybe Alan or Mark knows? 
> 
> We really need tester for such device.

hdparm identifies them with "-I", but libata doesn't allow HDIO_DRIVE_CMD
on ATAPI (yet?), and I've not yet updated hdparm to use SGIO directly.

But I did have such devices here once, like this one below,
from my library of IDENTIFY dumps:

 >ATAPI CD-ROM, with removable media
 >        Model Number:       CD-ROM 36X/AKU
 >        Serial Number:
 >        Firmware Revision:  U10I
 >Standards:
 >        Used: ATAPI for CD-ROMs, SFF-8020i, r2.5
 >        Supported: CD-ROM ATAPI-1
 >Configuration:
 >        DRQ response: <=10ms with INTRQ
 >        Packet size: 12 bytes
...

MMmm.. there's even a drive on the shelf here that might
be that exact unit.  I'll plug it in and see if it is.

-ml


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/1] libata-dev: ata_check_atapi_dma() fix for ATA_FLAG_PIO_POLLING LLDDs
  2006-03-15 15:54     ` Mark Lord
@ 2006-03-15 16:12       ` Mark Lord
  2006-03-15 16:24         ` Mark Lord
  2006-03-15 16:49         ` Mark Lord
  0 siblings, 2 replies; 10+ messages in thread
From: Mark Lord @ 2006-03-15 16:12 UTC (permalink / raw)
  To: albertl; +Cc: Jeff Garzik, IDE Linux, Tejun Heo, Doug Maxey

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

Albert Lee wrote:
> Jeff Garzik wrote:
> 
>> Applied, though I think its an open question what happens with
>> CDB-interrupt ATAPI devices want to do DMA.  Perhaps we just get an
>> interrupt event that we clear, then life proceeds as normal.
> 
> Not sure. I have no such device at hand for test; currently relying
> on the SFF-8020i spec (http://www.bswd.com/sff8020i.pdf) as reference.
> According to p.31, we have to send out the CDB after receiving
> the CDB interrupt. So, just clearing the irq looks not enough.
> 
>> Does anybody actually have such a device anywhere?  :)

Yup, I have two such drives here.

One of them has a grease-pencil "NFG" marking on it,
and other other one doesn't.  They both powered up and
worked just now, and their identify blobs are attached
(you can interpret them with "hdparm --Istdin <blob").

I'm late paying attention on this thread, so what is it
that we wanted done with them?

Or even better, who wants one of them?  Albert, Jeff, or Tejun?

Cheers

[-- Attachment #2: aopen_936e_aku.identify --]
[-- Type: text/plain, Size: 1451 bytes --]


/dev/hdd:
 IO_support   =  0 (default 16-bit)
 unmaskirq    =  0 (off)
 using_dma    =  1 (on)
 keepsettings =  0 (off)
 readonly     =  0 (off)
 readahead    = 256 (on)
85a0 0000 0000 0000 0000 0000 0000 0000
0000 0000 2020 2020 2020 2020 2020 2020
2020 2020 2020 2020 0000 0000 0000 5531
3049 2020 2020 4344 2d52 4f4d 2033 3658
2f41 4b55 2020 2020 2020 2020 2020 2020
2020 2020 2020 2020 2020 2020 2020 0000
0000 0f00 0000 0400 0200 0006 0000 0000
0000 0000 0000 0000 0000 0000 0000 0107
0003 0078 0096 00e3 0078 0000 0000 0000
0000 0002 0009 3030 3120 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0107 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000

[-- Attachment #3: aopen_940e_aku.identify --]
[-- Type: text/plain, Size: 1451 bytes --]


/dev/hdc:
 IO_support   =  0 (default 16-bit)
 unmaskirq    =  0 (off)
 using_dma    =  1 (on)
 keepsettings =  0 (off)
 readonly     =  0 (off)
 readahead    = 256 (on)
85a0 0000 0000 0000 0000 0000 0000 0000
0000 0000 2020 2020 2020 2020 2020 2020
2020 2020 2020 2020 0000 0000 0000 5533
3120 2020 2020 4344 2d52 4f4d 2034 3058
2f41 4b55 2020 2020 2020 2020 2020 2020
2020 2020 2020 2020 2020 2020 2020 0000
0000 0f00 0000 0400 0200 0006 0000 0000
0000 0000 0000 0000 0000 0000 0000 0107
0003 0078 0096 00e3 0078 0000 0000 0000
0000 0002 0009 3030 3330 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0107 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0041
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000
0000 0000 0000 0000 0000 0000 0000 0000

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/1] libata-dev: ata_check_atapi_dma() fix for ATA_FLAG_PIO_POLLING LLDDs
  2006-03-15 16:12       ` Mark Lord
@ 2006-03-15 16:24         ` Mark Lord
  2006-03-16  7:22           ` Albert Lee
  2006-03-15 16:49         ` Mark Lord
  1 sibling, 1 reply; 10+ messages in thread
From: Mark Lord @ 2006-03-15 16:24 UTC (permalink / raw)
  To: albertl; +Cc: Jeff Garzik, IDE Linux, Tejun Heo, Doug Maxey

Mark Lord wrote:
> Albert Lee wrote:
>> Jeff Garzik wrote:
>>
>>> Applied, though I think its an open question what happens with
>>> CDB-interrupt ATAPI devices want to do DMA.  Perhaps we just get an
>>> interrupt event that we clear, then life proceeds as normal.

Oh yeah, from experience I know that the interrupt and DRQ bit
are *NOT* synchronized with these devices.. I believe that the IRQ
sometimes arrives early, and sometimes late.

Reading the ATA status reg after the IRQ always clears the IRQ,
and is usually necessary for the transfer to continue.

The last time I did a full ATAPI driver, it used both periodic polling
and an IRQ handler to wait for (DRQ && !BUSY).

Cheers

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/1] libata-dev: ata_check_atapi_dma() fix for ATA_FLAG_PIO_POLLING LLDDs
  2006-03-15 16:12       ` Mark Lord
  2006-03-15 16:24         ` Mark Lord
@ 2006-03-15 16:49         ` Mark Lord
  2006-04-01 13:24           ` Albert Lee
  1 sibling, 1 reply; 10+ messages in thread
From: Mark Lord @ 2006-03-15 16:49 UTC (permalink / raw)
  To: albertl; +Cc: Jeff Garzik, IDE Linux, Tejun Heo, Doug Maxey

Mark Lord wrote:
>
> Yup, I have two such drives here.
> 
> One of them has a grease-pencil "NFG" marking on it,
> and other other one doesn't.  They both powered up and
> worked just now, and their identify blobs are attached
> (you can interpret them with "hdparm --Istdin <blob").
..

Oh, another note:  neither of these two drives works
with DMA enabled, at least not in 2.6.12 using the IDE driver.

PIO mode appears to be functioning fine on both of them.

Cheers

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/1] libata-dev: ata_check_atapi_dma() fix for ATA_FLAG_PIO_POLLING LLDDs
  2006-03-15 16:24         ` Mark Lord
@ 2006-03-16  7:22           ` Albert Lee
  2006-03-17  5:51             ` Mark Lord
  0 siblings, 1 reply; 10+ messages in thread
From: Albert Lee @ 2006-03-16  7:22 UTC (permalink / raw)
  To: Mark Lord; +Cc: Jeff Garzik, IDE Linux, Tejun Heo, Doug Maxey

Mark Lord wrote:
> 
>> Albert Lee wrote:
>>
>>> Jeff Garzik wrote:
>>>
>>>> Applied, though I think its an open question what happens with
>>>> CDB-interrupt ATAPI devices want to do DMA.  Perhaps we just get an
>>>> interrupt event that we clear, then life proceeds as normal.
> 
> 
> Oh yeah, from experience I know that the interrupt and DRQ bit
> are *NOT* synchronized with these devices.. I believe that the IRQ
> sometimes arrives early, and sometimes late.

Hmm, this will be a headache to handle.
The cd-rom vendors don't follow the sff-8020i spec. :(

1. Maybe adding some read status loops to the CDB-interrupt handler
when checking the DRQ bit can workaround this problem?
Hopefully the DRQ bit will be 1 after some loops,
otherwise the HSM will consider it as AC_ERR_HSM error.
(Need some testing to determine the actual loops time needed.)

> 
> Reading the ATA status reg after the IRQ always clears the IRQ,
> and is usually necessary for the transfer to continue.
> 
> The last time I did a full ATAPI driver, it used both periodic polling
> and an IRQ handler to wait for (DRQ && !BUSY).
> 

2. Similar to this idea is always use polling to send the ATAPI CDB
(as if there is no CDB interrupt). Then, in the irq handler,
whenever we see an CDB-irq in the HSM_ST_FIRST state,
we just check whether the device may assert CDB irq.
If yes, irq handler only acks the irq and let the polling thread do the
sending CDB work. (Please see the attached patch.)

Either #1 or #2 should workaround the DRQ/INTRQ not synchronized problem.
(#2 is more intrusive and not 100% pure irq driven.)

Thanks for the valuable AOpen CD-936E/CD-940E information.
Will try to get some of the drives from the computer components
store this weekend. More test to follow and see how current mainline
and irq-pio branch work with these drives.

--
Albert

Patch for #2: always use polling to send CDB.
There is a minor problem in the patch: In the HSM_ST_FIRST state, 
we no longer move the HSM forward (and make the device BSY) in the irq handler
=> So, the irq handler may be entered more than once in HSM_ST_FIRST state
(and status register read/irq acked more than once) until the polling thread
moves the HSM forward. But this seems doesn't hurt.

--- 07_followup/drivers/scsi/libata-core.c	2006-03-13 16:25:52.000000000 +0800
+++ 09_cdb_intr/drivers/scsi/libata-core.c	2006-03-16 14:17:06.000000000 +0800
@@ -3519,8 +3519,7 @@ static inline int ata_hsm_ok_in_wq(struc
 		    (qc->tf.flags & ATA_TFLAG_WRITE))
 		    return 1;
 
-		if (is_atapi_taskfile(&qc->tf) &&
-		    !(qc->dev->flags & ATA_DFLAG_CDB_INTR))
+		if (is_atapi_taskfile(&qc->tf))
 			return 1;
 	}
 
@@ -4106,10 +4105,9 @@ unsigned int ata_qc_issue_prot(struct at
 
 		ap->hsm_task_state = HSM_ST_FIRST;
 
-		/* send cdb by polling if no cdb interrupt */
-		if ((!(qc->dev->flags & ATA_DFLAG_CDB_INTR)) ||
-		    (qc->tf.flags & ATA_TFLAG_POLLING))
-			ata_port_queue_task(ap, ata_pio_task, ap, 0);
+		/* always send cdb by polling */
+		ata_port_queue_task(ap, ata_pio_task, ap, 0);
+
 		break;
 
 	case ATA_PROT_ATAPI_DMA:
@@ -4119,9 +4117,9 @@ unsigned int ata_qc_issue_prot(struct at
 		ap->ops->bmdma_setup(qc);	    /* set up bmdma */
 		ap->hsm_task_state = HSM_ST_FIRST;
 
-		/* send cdb by polling if no cdb interrupt */
-		if (!(qc->dev->flags & ATA_DFLAG_CDB_INTR))
-			ata_port_queue_task(ap, ata_pio_task, ap, 0);
+		/* always send cdb by polling */
+		ata_port_queue_task(ap, ata_pio_task, ap, 0);
+
 		break;
 
 	default:
@@ -4444,7 +4442,14 @@ inline unsigned int ata_host_intr (struc
 	/* ack bmdma irq events */
 	ap->ops->irq_clear(ap);
 
-	ata_hsm_move(ap, qc, status, 0);
+	switch (ap->hsm_task_state) {
+	case HSM_ST_FIRST:
+		/* ATAPI CDB interrupt. Let polling handle this. */
+		break;
+	default:
+		ata_hsm_move(ap, qc, status, 0);
+	}
+
 	return 1;	/* irq handled */
 
 idle_irq:


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/1] libata-dev: ata_check_atapi_dma() fix for ATA_FLAG_PIO_POLLING LLDDs
  2006-03-16  7:22           ` Albert Lee
@ 2006-03-17  5:51             ` Mark Lord
  0 siblings, 0 replies; 10+ messages in thread
From: Mark Lord @ 2006-03-17  5:51 UTC (permalink / raw)
  To: albertl; +Cc: Jeff Garzik, IDE Linux, Tejun Heo, Doug Maxey

Albert Lee wrote:
>
> Thanks for the valuable AOpen CD-936E/CD-940E information.
> Will try to get some of the drives from the computer components
> store this weekend. More test to follow and see how current mainline
> and irq-pio branch work with these drives.
..

Albert,

What continent are you on?  If you're in Canada or the USA,
then email me privately (mlord@pobox.com) and I'll just post
you one of these units.  They're pretty ancient, and not
exactly available in stores nowadays.

Cheers

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/1] libata-dev: ata_check_atapi_dma() fix for ATA_FLAG_PIO_POLLING LLDDs
  2006-03-15 16:49         ` Mark Lord
@ 2006-04-01 13:24           ` Albert Lee
  0 siblings, 0 replies; 10+ messages in thread
From: Albert Lee @ 2006-04-01 13:24 UTC (permalink / raw)
  To: Mark Lord, Jeff Garzik; +Cc: IDE Linux, Tejun Heo, Doug Maxey, Alan Cox

Mark Lord wrote:
> 
>>
>> Yup, I have two such drives here.
>>
>> One of them has a grease-pencil "NFG" marking on it,
>> and other other one doesn't.  They both powered up and
>> worked just now, and their identify blobs are attached
>> (you can interpret them with "hdparm --Istdin <blob").
> 
> ..
> 
> Oh, another note:  neither of these two drives works
> with DMA enabled, at least not in 2.6.12 using the IDE driver.
> 
> PIO mode appears to be functioning fine on both of them.
> 

Thanks for Mark's info, I have finally found two of such CD-ROM drives:
one is a 1998 AOpen CD-936E/AKW (36x), the other is a LITEON LTN-403 (40x), removed
from a 1999 IBM Aptiva PC.

Both of them generates CDB interrupts. The AOpen drive supports PIO4/MWDMA2 and
the LITEON drive supports UDMA/33. (So, CDB intr + UDMA drives do exist.)

I've tested the drives for about a week. The test result of the irq-pio code
looks good. DRQ is always set when CDB interrupt is generated. 
Both drives follow the behavior defined by the sff-8020i spec.
I did not hit the DRQ=0 during CDB interrupt problem mentioned by Mark.
(If we encounter such problem in the future, maybe we can add some 
waiting loops to the HSM_ST_FIRST state, waiting for DRQ to be set, something 
like what ata_wait_idle() does.)

Interrupt-driven PIO, polling PIO and (interrupt driven) DMA protocols are tested
with both drives. Results look good. (Sample logs attached below.)
Hard drives are also tested, result looks good, too.

Current upstream is also tested. Since CDB intr is not supported yet, the CDB intr
caused irq to be disabled.

Summary: 
The current known problem of the irq-pio branch is the DRQ=1 ERR=1 handling.
A revised patch has been sent per Jeff's IDE try_to_flush_leftover_data() pointer.
Otherwise the irq-pio branch looks ready for upstream (need your comments)
and integration with Tejun's EH and Alan's PATA works.

Thanks,

Albert

Sample transation logs of the LTN-403 drive (irq-pio and dma).
(Interrupt driven PIO)
Mar 23 19:30:26 p4ht-s kernel: ata_scsi_dump_cdb: CDB (1:0,0,0) 28 00 00 00 2c 30 00 00 32
Mar 23 19:30:26 p4ht-s kernel: ata_scsi_translate: ENTER
Mar 23 19:30:26 p4ht-s kernel: ata_dev_select: ENTER, ata1: device 0, wait 1
Mar 23 19:30:26 p4ht-s kernel: ata_tf_load_mmio: feat 0x0 nsect 0x0 lba 0x0 0x0 0x20
Mar 23 19:30:26 p4ht-s kernel: ata_tf_load_mmio: device 0xA0
Mar 23 19:30:26 p4ht-s kernel: ata_exec_command_mmio: ata1: cmd 0xA0
Mar 23 19:30:26 p4ht-s kernel: ata_scsi_translate: EXIT
Mar 23 19:30:26 p4ht-s kernel: ata_host_intr: ata1: protocol 5 task_state 5  <== CDB intr
Mar 23 19:30:26 p4ht-s kernel: ata_hsm_move: ata1: protocol 5 task_state 5 (dev_stat 0x58) <== DRQ=1, good.
Mar 23 19:30:26 p4ht-s kernel: atapi_send_cdb: send cdb
Mar 23 19:30:26 p4ht-s kernel: ata_host_intr: ata1: protocol 5 task_state 2
Mar 23 19:30:26 p4ht-s kernel: ata_hsm_move: ata1: protocol 5 task_state 2 (dev_stat 0x58)
Mar 23 19:30:26 p4ht-s kernel: atapi_pio_bytes: ata1: xfering 8192 bytes
Mar 23 19:30:26 p4ht-s kernel: __atapi_pio_bytes: data read
Mar 23 19:30:26 p4ht-s kernel: __atapi_pio_bytes: data read
Mar 23 19:30:27 p4ht-s kernel: ata_host_intr: ata1: protocol 5 task_state 2
Mar 23 19:30:27 p4ht-s kernel: ata_hsm_move: ata1: protocol 5 task_state 2 (dev_stat 0x58)
Mar 23 19:30:27 p4ht-s kernel: atapi_pio_bytes: ata1: xfering 8192 bytes
Mar 23 19:30:27 p4ht-s kernel: __atapi_pio_bytes: data read
Mar 23 19:30:27 p4ht-s kernel: __atapi_pio_bytes: data read
Mar 23 19:30:27 p4ht-s kernel: ata_host_intr: ata1: protocol 5 task_state 2
Mar 23 19:30:27 p4ht-s kernel: ata_hsm_move: ata1: protocol 5 task_state 2 (dev_stat 0x58)
Mar 23 19:30:27 p4ht-s kernel: atapi_pio_bytes: ata1: xfering 8192 bytes
Mar 23 19:30:27 p4ht-s kernel: __atapi_pio_bytes: data read
Mar 23 19:30:27 p4ht-s kernel: __atapi_pio_bytes: data read
Mar 23 19:30:27 p4ht-s kernel: ata_host_intr: ata1: protocol 5 task_state 2
Mar 23 19:30:27 p4ht-s kernel: ata_hsm_move: ata1: protocol 5 task_state 2 (dev_stat 0x58)
Mar 23 19:30:27 p4ht-s kernel: atapi_pio_bytes: ata1: xfering 8192 bytes
Mar 23 19:30:27 p4ht-s kernel: __atapi_pio_bytes: data read
Mar 23 19:30:27 p4ht-s kernel: __atapi_pio_bytes: data read
Mar 23 19:30:27 p4ht-s kernel: ata_host_intr: ata1: protocol 5 task_state 2
Mar 23 19:30:27 p4ht-s kernel: ata_hsm_move: ata1: protocol 5 task_state 2 (dev_stat 0x58)
Mar 23 19:30:27 p4ht-s kernel: atapi_pio_bytes: ata1: xfering 8192 bytes
Mar 23 19:30:27 p4ht-s kernel: __atapi_pio_bytes: data read
Mar 23 19:30:27 p4ht-s kernel: __atapi_pio_bytes: data read
Mar 23 19:30:27 p4ht-s kernel: ata_host_intr: ata1: protocol 5 task_state 2
Mar 23 19:30:27 p4ht-s kernel: ata_hsm_move: ata1: protocol 5 task_state 2 (dev_stat 0x58)
Mar 23 19:30:27 p4ht-s kernel: atapi_pio_bytes: ata1: xfering 8192 bytes
Mar 23 19:30:27 p4ht-s kernel: __atapi_pio_bytes: data read
Mar 23 19:30:27 p4ht-s kernel: __atapi_pio_bytes: data read
Mar 23 19:30:27 p4ht-s kernel: ata_host_intr: ata1: protocol 5 task_state 2
Mar 23 19:30:27 p4ht-s kernel: ata_hsm_move: ata1: protocol 5 task_state 2 (dev_stat 0x58)
Mar 23 19:30:27 p4ht-s kernel: atapi_pio_bytes: ata1: xfering 8192 bytes
Mar 23 19:30:27 p4ht-s kernel: __atapi_pio_bytes: data read
Mar 23 19:30:27 p4ht-s kernel: __atapi_pio_bytes: data read
Mar 23 19:30:27 p4ht-s kernel: ata_host_intr: ata1: protocol 5 task_state 2
Mar 23 19:30:27 p4ht-s kernel: ata_hsm_move: ata1: protocol 5 task_state 2 (dev_stat 0x58)
Mar 23 19:30:27 p4ht-s kernel: atapi_pio_bytes: ata1: xfering 8192 bytes
Mar 23 19:30:27 p4ht-s kernel: __atapi_pio_bytes: data read
Mar 23 19:30:27 p4ht-s kernel: __atapi_pio_bytes: data read
Mar 23 19:30:27 p4ht-s kernel: ata_host_intr: ata1: protocol 5 task_state 2
Mar 23 19:30:27 p4ht-s kernel: ata_hsm_move: ata1: protocol 5 task_state 2 (dev_stat 0x58)
Mar 23 19:30:27 p4ht-s kernel: atapi_pio_bytes: ata1: xfering 8192 bytes
Mar 23 19:30:27 p4ht-s kernel: __atapi_pio_bytes: data read
Mar 23 19:30:27 p4ht-s kernel: __atapi_pio_bytes: data read
Mar 23 19:30:27 p4ht-s kernel: ata_host_intr: ata1: protocol 5 task_state 2
Mar 23 19:30:27 p4ht-s kernel: ata_hsm_move: ata1: protocol 5 task_state 2 (dev_stat 0x58)
Mar 23 19:30:27 p4ht-s kernel: atapi_pio_bytes: ata1: xfering 8192 bytes
Mar 23 19:30:27 p4ht-s kernel: __atapi_pio_bytes: data read
Mar 23 19:30:27 p4ht-s kernel: __atapi_pio_bytes: data read
Mar 23 19:30:27 p4ht-s kernel: ata_host_intr: ata1: protocol 5 task_state 2
Mar 23 19:30:27 p4ht-s kernel: ata_hsm_move: ata1: protocol 5 task_state 2 (dev_stat 0x58)
Mar 23 19:30:27 p4ht-s kernel: atapi_pio_bytes: ata1: xfering 8192 bytes
Mar 23 19:30:27 p4ht-s kernel: __atapi_pio_bytes: data read
Mar 23 19:30:27 p4ht-s kernel: __atapi_pio_bytes: data read
Mar 23 19:30:27 p4ht-s kernel: ata_host_intr: ata1: protocol 5 task_state 2
Mar 23 19:30:27 p4ht-s kernel: ata_hsm_move: ata1: protocol 5 task_state 2 (dev_stat 0x58)
Mar 23 19:30:27 p4ht-s kernel: atapi_pio_bytes: ata1: xfering 8192 bytes
Mar 23 19:30:27 p4ht-s kernel: __atapi_pio_bytes: data read
Mar 23 19:30:27 p4ht-s kernel: __atapi_pio_bytes: data read
Mar 23 19:30:27 p4ht-s kernel: ata_host_intr: ata1: protocol 5 task_state 2
Mar 23 19:30:27 p4ht-s kernel: ata_hsm_move: ata1: protocol 5 task_state 2 (dev_stat 0x58)
Mar 23 19:30:27 p4ht-s kernel: atapi_pio_bytes: ata1: xfering 4096 bytes
Mar 23 19:30:27 p4ht-s kernel: __atapi_pio_bytes: data read
Mar 23 19:30:27 p4ht-s kernel: ata_host_intr: ata1: protocol 5 task_state 3
Mar 23 19:30:27 p4ht-s kernel: ata_hsm_move: ata1: protocol 5 task_state 3 (dev_stat 0x50)
Mar 23 19:30:27 p4ht-s kernel: ata_hsm_move: ata1: command complete, drv_stat 0x50
Mar 23 19:30:27 p4ht-s kernel: atapi_qc_complete: ENTER, err_mask 0x0

================

(DMA)

Mar 23 19:38:13 p4ht-s kernel: ata_scsi_dump_cdb: CDB (1:0,0,0) 28 00 00 00 0c 30 00 00 32
Mar 23 19:38:13 p4ht-s kernel: ata_scsi_translate: ENTER
Mar 23 19:38:13 p4ht-s kernel: ata_sg_setup: ENTER, ata1
Mar 23 19:38:13 p4ht-s kernel: ata_sg_setup: 25 sg elements mapped
Mar 23 19:38:13 p4ht-s kernel: ata_fill_sg: PRD[0] = (0x486F000, 0x1000)
Mar 23 19:38:13 p4ht-s kernel: ata_fill_sg: PRD[1] = (0x10A7C000, 0x1000)
Mar 23 19:38:13 p4ht-s kernel: ata_fill_sg: PRD[2] = (0x12BD6000, 0x1000)
Mar 23 19:38:13 p4ht-s kernel: ata_fill_sg: PRD[3] = (0xF3DF000, 0x1000)
Mar 23 19:38:13 p4ht-s kernel: ata_fill_sg: PRD[4] = (0xF242000, 0x1000)
Mar 23 19:38:13 p4ht-s kernel: ata_fill_sg: PRD[5] = (0x9A25000, 0x1000)
Mar 23 19:38:13 p4ht-s kernel: ata_fill_sg: PRD[6] = (0x4E9E000, 0x1000)
Mar 23 19:38:13 p4ht-s kernel: ata_fill_sg: PRD[7] = (0x1E32B000, 0x1000)
Mar 23 19:38:13 p4ht-s kernel: ata_fill_sg: PRD[8] = (0x7DC7000, 0x1000)
Mar 23 19:38:13 p4ht-s kernel: ata_fill_sg: PRD[9] = (0xB4F3000, 0x1000)
Mar 23 19:38:13 p4ht-s kernel: ata_fill_sg: PRD[10] = (0xDD63000, 0x1000)
Mar 23 19:38:13 p4ht-s kernel: ata_fill_sg: PRD[11] = (0x115EB000, 0x1000)
Mar 23 19:38:13 p4ht-s kernel: ata_fill_sg: PRD[12] = (0xDB6D000, 0x1000)
Mar 23 19:38:13 p4ht-s kernel: ata_fill_sg: PRD[13] = (0x4CEC000, 0x1000)
Mar 23 19:38:13 p4ht-s kernel: ata_fill_sg: PRD[14] = (0x100E1000, 0x1000)
Mar 23 19:38:13 p4ht-s kernel: ata_fill_sg: PRD[15] = (0x776E000, 0x1000)
Mar 23 19:38:13 p4ht-s kernel: ata_fill_sg: PRD[16] = (0xA947000, 0x1000)
Mar 23 19:38:13 p4ht-s kernel: ata_fill_sg: PRD[17] = (0xAD8A000, 0x1000)
Mar 23 19:38:13 p4ht-s kernel: ata_fill_sg: PRD[18] = (0x7036000, 0x1000)
Mar 23 19:38:13 p4ht-s kernel: ata_fill_sg: PRD[19] = (0x13FA9000, 0x1000)
Mar 23 19:38:13 p4ht-s kernel: ata_fill_sg: PRD[20] = (0xA5D5000, 0x1000)
Mar 23 19:38:13 p4ht-s kernel: ata_fill_sg: PRD[21] = (0xBE62000, 0x1000)
Mar 23 19:38:13 p4ht-s kernel: ata_fill_sg: PRD[22] = (0x2E8F000, 0x1000)
Mar 23 19:38:13 p4ht-s kernel: ata_fill_sg: PRD[23] = (0x10DF3000, 0x1000)
Mar 23 19:38:13 p4ht-s kernel: ata_fill_sg: PRD[24] = (0xA5D2000, 0x1000)
Mar 23 19:38:13 p4ht-s kernel: ata_dev_select: ENTER, ata1: device 0, wait 1
Mar 23 19:38:13 p4ht-s kernel: ata_tf_load_mmio: feat 0x1 nsect 0x0 lba 0x0 0x0 0x0
Mar 23 19:38:13 p4ht-s kernel: ata_tf_load_mmio: device 0xA0
Mar 23 19:38:13 p4ht-s kernel: ata_exec_command_mmio: ata1: cmd 0xA0
Mar 23 19:38:13 p4ht-s kernel: ata_scsi_translate: EXIT
Mar 23 19:38:13 p4ht-s kernel: ata_host_intr: ata1: protocol 7 task_state 5  <== CDB intr
Mar 23 19:38:13 p4ht-s kernel: ata_hsm_move: ata1: protocol 7 task_state 5 (dev_stat 0x58) <== DRQ=1, good.
Mar 23 19:38:13 p4ht-s kernel: atapi_send_cdb: send cdb
Mar 23 19:38:13 p4ht-s kernel: ata_host_intr: ata1: protocol 7 task_state 3
Mar 23 19:38:13 p4ht-s kernel: ata_host_intr: ata1: host_stat 0x4
Mar 23 19:38:13 p4ht-s kernel: ata_hsm_move: ata1: protocol 7 task_state 3 (dev_stat 0x50)
Mar 23 19:38:13 p4ht-s kernel: ata_hsm_move: ata1: command complete, drv_stat 0x50
Mar 23 19:38:13 p4ht-s kernel: ata_sg_clean: unmapping 25 sg elements
Mar 23 19:38:13 p4ht-s kernel: atapi_qc_complete: ENTER, err_mask 0x0


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2006-04-01 13:25 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-03-13  7:57 [PATCH 1/1] libata-dev: ata_check_atapi_dma() fix for ATA_FLAG_PIO_POLLING LLDDs Albert Lee
2006-03-13  8:09 ` Jeff Garzik
2006-03-14  4:52   ` Albert Lee
2006-03-15 15:54     ` Mark Lord
2006-03-15 16:12       ` Mark Lord
2006-03-15 16:24         ` Mark Lord
2006-03-16  7:22           ` Albert Lee
2006-03-17  5:51             ` Mark Lord
2006-03-15 16:49         ` Mark Lord
2006-04-01 13:24           ` Albert Lee

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).