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: - : 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