* SATA ATAPI work in progress @ 2004-05-12 20:20 Pat LaVarre 2004-05-12 20:40 ` Jeff Garzik 0 siblings, 1 reply; 34+ messages in thread From: Pat LaVarre @ 2004-05-12 20:20 UTC (permalink / raw) To: jgarzik; +Cc: linux-ide Jeff G: > From: linux-2.6.6-bk1/drivers/scsi/ata_piix.c > ... > Maintained by: Jeff Garzik <jga...@pob...> > Please ALWAYS copy linux-ide@vger.kernel.org on emails. Ok, hi. > > From: [offline] > > 1) atapi_scsi_queuecmd() should be deleted, > > and instead a translation function should be > > supplied for ata_scsi_translate() for all > > ATAPI devices, resulting in a PACKET > > taskfile. For 2.6.6-bk1 I wrote the following trivial patch. Is my patch a step forward? Given ATA_ENABLE_ATAPI ATA_DEBUG ATA_VERBOSE_DEBUG, then now the initial op x12 Inquiry now instead chokes via: kernel: ata2: DMA timeout, stat 0x21 kernel: ata_dma_complete: ENTER kernel: ata_dma_complete: host 2, host_stat==0x21, drv_stat==0x58 kernel: ATA: abnormal status 0x58 on port 0xE007 Looks like I've asked to start the Data DMA In before my PIO Command Out? Left to myself, maybe next I'll go digging in ata_tf_load_pio and ata_exec_command_pio. > From: linux-2.6.6-bk1/ `modprobe -r ata-piix; modprobe ata-piix` now seemingly work without reboot, thank you. > > From: [offline] > > 2) Issue and process a REQUEST SENSE > > internally, based on the PACKET ... returns. This means "auto sense", I think. > > based on the PACKET sense key returns. This means auto sense when (x1F1 Error & xF0 SK) are nonzero, I think. This design choice surprises me. I vote we auto sense when (x3F6 AlternateStatus & x01 ERR) is nonzero, so that still we see sense even when SK ASC ASCQ is zero, in particular when ASC ASCQ is nonzero in combination with zero SK. Also we thus duck trusting the redundant copy of SK. Pat LaVarre diff -Nurp linux-2.6.6-bk1/drivers/scsi/libata-scsi.c linux-2.6.6-bk1-pel/drivers/scsi/libata-scsi.c --- linux-2.6.6-bk1/drivers/scsi/libata-scsi.c 2004-05-12 09:57:09.000000000 -0600 +++ linux-2.6.6-bk1-pel/drivers/scsi/libata-scsi.c 2004-05-12 14:01:27.530354416 -0600 @@ -215,6 +215,35 @@ int ata_scsi_error(struct Scsi_Host *hos DPRINTK("EXIT\n"); return 0; +} + +/** + * atapi_xlat - Pass SCSI r/w command thru to ATAPI + * @qc: Storage for translated ATA taskfile + * @scsicmd: SCSI command to translate + * + * Trust caller already cleared *qc->tf (often via ata_tf_init), + * deciding tf->device & (0x10 ATA_DEV1 | 0x07 SFF 8070i LUN), etc. + * + * RETURNS: + * Zero on success, non-zero on error. + */ + +static unsigned int atapi_xlat(struct ata_queued_cmd *qc, u8 *scsicmd) +{ + struct ata_taskfile *tf = &qc->tf; + tf->flags &= ~ATA_TFLAG_LBA48; + tf->flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE; + tf->protocol = qc->dev->xfer_protocol; +#ifdef ATA_FORCE_PIO +#else + tf->feature = ATAPI_PKT_DMA; /* often x01 */ +#endif + tf->lbam = 0xFE; /* PIO "byte count limit" */ + tf->lbah = 0xFF; + tf->command = ATA_CMD_PACKET; /* often 0xA0 */ + DPRINTK("ATAPI\n"); + return 0; } /** @@ -885,78 +914,6 @@ void ata_scsi_badcmd(struct scsi_cmnd *c } /** - * atapi_scsi_queuecmd - Send CDB to ATAPI device - * @ap: Port to which ATAPI device is attached. - * @dev: Target device for CDB. - * @cmd: SCSI command being sent to device. - * @done: SCSI command completion function. - * - * Sends CDB to ATAPI device. If the Linux SCSI layer sends a - * non-data command, then this function handles the command - * directly, via polling. Otherwise, the bmdma engine is started. - * - * LOCKING: - * spin_lock_irqsave(host_set lock) - */ - -static void atapi_scsi_queuecmd(struct ata_port *ap, struct ata_device *dev, - struct scsi_cmnd *cmd, void (*done)(struct scsi_cmnd *)) -{ - struct ata_queued_cmd *qc; - u8 *scsicmd = cmd->cmnd; - - VPRINTK("ENTER, drv_stat = 0x%x\n", ata_chk_status(ap)); - - if (cmd->sc_data_direction == SCSI_DATA_UNKNOWN) { - DPRINTK("unknown data, scsicmd 0x%x\n", scsicmd[0]); - ata_bad_cdb(cmd, done); - return; - } - - switch(scsicmd[0]) { - case READ_6: - case WRITE_6: - case MODE_SELECT: - case MODE_SENSE: - DPRINTK("read6/write6/modesel/modesense trap\n"); - ata_bad_scsiop(cmd, done); - return; - - default: - /* do nothing */ - break; - } - - qc = ata_scsi_qc_new(ap, dev, cmd, done); - if (!qc) { - printk(KERN_ERR "ata%u: command queue empty\n", ap->id); - return; - } - - qc->flags |= ATA_QCFLAG_ATAPI; - - qc->tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE; - if (cmd->sc_data_direction == SCSI_DATA_WRITE) { - qc->tf.flags |= ATA_TFLAG_WRITE; - DPRINTK("direction: write\n"); - } - - qc->tf.command = ATA_CMD_PACKET; - - if (cmd->sc_data_direction == SCSI_DATA_NONE) { - qc->tf.protocol = ATA_PROT_ATAPI; - qc->flags |= ATA_QCFLAG_POLL; - qc->tf.ctl |= ATA_NIEN; /* disable interrupts */ - } else { - qc->tf.protocol = ATA_PROT_ATAPI_DMA; - qc->flags |= ATA_QCFLAG_SG; /* data is present; dma-map it */ - qc->tf.feature |= ATAPI_PKT_DMA; - } - - atapi_start(qc); -} - -/** * ata_scsi_find_dev - lookup ata_device from scsi_cmnd * @ap: ATA port to which the device is attached * @cmd: SCSI command to be sent to the device @@ -1086,13 +1043,13 @@ int ata_scsi_queuecmd(struct scsi_cmnd * if (dev->class == ATA_DEV_ATA) { ata_xlat_func_t xlat_func = ata_get_xlat_func(cmd->cmnd[0]); - if (xlat_func) ata_scsi_translate(ap, dev, cmd, done, xlat_func); else ata_scsi_simulate(ap, dev, cmd, done); - } else - atapi_scsi_queuecmd(ap, dev, cmd, done); + } else { + ata_scsi_translate(ap, dev, cmd, done, atapi_xlat); + } out_unlock: return 0; ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: SATA ATAPI work in progress 2004-05-12 20:20 SATA ATAPI work in progress Pat LaVarre @ 2004-05-12 20:40 ` Jeff Garzik 2004-05-12 23:14 ` Pat LaVarre 0 siblings, 1 reply; 34+ messages in thread From: Jeff Garzik @ 2004-05-12 20:40 UTC (permalink / raw) To: Pat LaVarre; +Cc: linux-ide Pat LaVarre wrote: >>>From: [offline] >>>1) atapi_scsi_queuecmd() should be deleted, >>>and instead a translation function should be >>>supplied for ata_scsi_translate() for all >>>ATAPI devices, resulting in a PACKET >>>taskfile. > > > For 2.6.6-bk1 I wrote the following trivial patch. > > Is my patch a step forward? Yeah, it's a decent first step. > Given ATA_ENABLE_ATAPI ATA_DEBUG ATA_VERBOSE_DEBUG, then now the initial > op x12 Inquiry now instead chokes via: > > kernel: ata2: DMA timeout, stat 0x21 > kernel: ata_dma_complete: ENTER > kernel: ata_dma_complete: host 2, host_stat==0x21, drv_stat==0x58 > kernel: ATA: abnormal status 0x58 on port 0xE007 > > Looks like I've asked to start the Data DMA In before my PIO Command > Out? Left to myself, maybe next I'll go digging in ata_tf_load_pio and > ata_exec_command_pio. Well first, you need to make sure your ATAPI device supports ultra DMA, as you don't want to bother with PIO or [SM]WDMA right now. But yeah, after that, it's time to go digging. I generally use drivers/ide and Hale Landis's ATADRVR (http://www.ata-atapi.com/) for comparing IO write-for-write, to make sure I have the steps and the state machine correct. And of course the state machine docs itself, in ATA/ATAPI7. >>From: linux-2.6.6-bk1/ > > > `modprobe -r ata-piix; modprobe ata-piix` now seemingly work without > reboot, thank you. > > >>>From: [offline] >>>2) Issue and process a REQUEST SENSE >>>internally, based on the PACKET ... returns. > > > This means "auto sense", I think. > > >>>based on the PACKET sense key returns. > > > This means auto sense when (x1F1 Error & xF0 SK) are nonzero, I think. > > This design choice surprises me. I vote we auto sense when (x3F6 > AlternateStatus & x01 ERR) is nonzero, so that still we see sense even > when SK ASC ASCQ is zero, in particular when ASC ASCQ is nonzero in > combination with zero SK. Also we thus duck trusting the redundant copy > of SK. libata should provide the illusion that an ATAPI device always provides autosense on each command. For various reasons, we can't let the SCSI layer just call REQUEST SENSE as it does for scsi-1/2 devices during scsi_unjam_host(). > diff -Nurp linux-2.6.6-bk1/drivers/scsi/libata-scsi.c linux-2.6.6-bk1-pel/drivers/scsi/libata-scsi.c > --- linux-2.6.6-bk1/drivers/scsi/libata-scsi.c 2004-05-12 09:57:09.000000000 -0600 > +++ linux-2.6.6-bk1-pel/drivers/scsi/libata-scsi.c 2004-05-12 14:01:27.530354416 -0600 > @@ -215,6 +215,35 @@ int ata_scsi_error(struct Scsi_Host *hos > > DPRINTK("EXIT\n"); > return 0; > +} > + > +/** > + * atapi_xlat - Pass SCSI r/w command thru to ATAPI > + * @qc: Storage for translated ATA taskfile > + * @scsicmd: SCSI command to translate > + * > + * Trust caller already cleared *qc->tf (often via ata_tf_init), > + * deciding tf->device & (0x10 ATA_DEV1 | 0x07 SFF 8070i LUN), etc. > + * > + * RETURNS: > + * Zero on success, non-zero on error. > + */ > + > +static unsigned int atapi_xlat(struct ata_queued_cmd *qc, u8 *scsicmd) > +{ > + struct ata_taskfile *tf = &qc->tf; > + tf->flags &= ~ATA_TFLAG_LBA48; I'm not sure you should bother with this. > + tf->flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE; > + tf->protocol = qc->dev->xfer_protocol; > +#ifdef ATA_FORCE_PIO > +#else > + tf->feature = ATAPI_PKT_DMA; /* often x01 */ > +#endif don't use ifdef, test ATA_QCFLAG_DMA bit > + tf->lbam = 0xFE; /* PIO "byte count limit" */ > + tf->lbah = 0xFF; for DMA, set this to zero. for PIO, set this to 8K (one SATA FIS). > + tf->command = ATA_CMD_PACKET; /* often 0xA0 */ > + DPRINTK("ATAPI\n"); > + return 0; > } > > /** > @@ -885,78 +914,6 @@ void ata_scsi_badcmd(struct scsi_cmnd *c > } > > /** > - * atapi_scsi_queuecmd - Send CDB to ATAPI device > - * @ap: Port to which ATAPI device is attached. > - * @dev: Target device for CDB. > - * @cmd: SCSI command being sent to device. > - * @done: SCSI command completion function. > - * > - * Sends CDB to ATAPI device. If the Linux SCSI layer sends a > - * non-data command, then this function handles the command > - * directly, via polling. Otherwise, the bmdma engine is started. > - * > - * LOCKING: > - * spin_lock_irqsave(host_set lock) > - */ > - > -static void atapi_scsi_queuecmd(struct ata_port *ap, struct ata_device *dev, > - struct scsi_cmnd *cmd, void (*done)(struct scsi_cmnd *)) > -{ > - struct ata_queued_cmd *qc; > - u8 *scsicmd = cmd->cmnd; > - > - VPRINTK("ENTER, drv_stat = 0x%x\n", ata_chk_status(ap)); > - > - if (cmd->sc_data_direction == SCSI_DATA_UNKNOWN) { > - DPRINTK("unknown data, scsicmd 0x%x\n", scsicmd[0]); > - ata_bad_cdb(cmd, done); > - return; > - } > - > - switch(scsicmd[0]) { > - case READ_6: > - case WRITE_6: > - case MODE_SELECT: > - case MODE_SENSE: > - DPRINTK("read6/write6/modesel/modesense trap\n"); > - ata_bad_scsiop(cmd, done); > - return; > - > - default: > - /* do nothing */ > - break; > - } > - > - qc = ata_scsi_qc_new(ap, dev, cmd, done); > - if (!qc) { > - printk(KERN_ERR "ata%u: command queue empty\n", ap->id); > - return; > - } > - > - qc->flags |= ATA_QCFLAG_ATAPI; > - > - qc->tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE; > - if (cmd->sc_data_direction == SCSI_DATA_WRITE) { > - qc->tf.flags |= ATA_TFLAG_WRITE; > - DPRINTK("direction: write\n"); > - } > - > - qc->tf.command = ATA_CMD_PACKET; > - > - if (cmd->sc_data_direction == SCSI_DATA_NONE) { > - qc->tf.protocol = ATA_PROT_ATAPI; > - qc->flags |= ATA_QCFLAG_POLL; > - qc->tf.ctl |= ATA_NIEN; /* disable interrupts */ > - } else { > - qc->tf.protocol = ATA_PROT_ATAPI_DMA; > - qc->flags |= ATA_QCFLAG_SG; /* data is present; dma-map it */ > - qc->tf.feature |= ATAPI_PKT_DMA; > - } > - > - atapi_start(qc); Note that by deleting this, there is no longer anything to start the ATAPI transfer really. You must the latter half of the above code [the part you are deleting] either to ata_qc_issue_prot() or your new atapi_xlat(). > * ata_scsi_find_dev - lookup ata_device from scsi_cmnd > * @ap: ATA port to which the device is attached > * @cmd: SCSI command to be sent to the device > @@ -1086,13 +1043,13 @@ int ata_scsi_queuecmd(struct scsi_cmnd * > > if (dev->class == ATA_DEV_ATA) { > ata_xlat_func_t xlat_func = ata_get_xlat_func(cmd->cmnd[0]); > - > if (xlat_func) > ata_scsi_translate(ap, dev, cmd, done, xlat_func); > else > ata_scsi_simulate(ap, dev, cmd, done); > - } else > - atapi_scsi_queuecmd(ap, dev, cmd, done); > + } else { > + ata_scsi_translate(ap, dev, cmd, done, atapi_xlat); > + } Good guess, close. At this point, just eliminate the dev->class test, and simplify the above code down to ata_xlat_func_t xlat_func = ata_get_xlat_func(cmd->cmnd[0]); if (xlat_func) ata_scsi_translate(ap, dev, cmd, done, xlat_func); else ata_scsi_simulate(ap, dev, cmd, done); Thus, you must modify ata_get_xlat_func to return atapi_xlat when dev->class == ATAPI, and to execute the existing 'switch' statement for dev->class == ATA. Jeff ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: SATA ATAPI work in progress 2004-05-12 20:40 ` Jeff Garzik @ 2004-05-12 23:14 ` Pat LaVarre 2004-05-13 15:07 ` Pat LaVarre 2004-05-13 21:16 ` Jeff Garzik 0 siblings, 2 replies; 34+ messages in thread From: Pat LaVarre @ 2004-05-12 23:14 UTC (permalink / raw) To: jgarzik; +Cc: linux-ide Jeff G: > decent first step ... > Good guess ... > .. > [1] not sure you should bother ... > [2] don't use ifdef, test ATA_QCFLAG_DMA ... > [3] for DMA, ... zero > [4] for PIO, ... 8K (one SATA FIS) ... > [5] eliminate the dev->class test ... > [6] modify ata_get_xlat_func > to return atapi_xlat when dev->class == ATAPI Changes complete as requested, I hope, in the new patch inline. > > - VPRINTK("ENTER, drv_stat = 0x%x\n", ata_chk_status(ap)); > > ... > > - atapi_start(qc); > > Note that by deleting this, there is no > longer anything to start the ATAPI transfer > really. Aye, the dmesg tell me I'm timing out with command incomplete. > You must the latter half of the above code > [the part you are deleting] either to > ata_qc_issue_prot() or your new atapi_xlat(). The dmesg tell me my guess below of how to do this actually ended up writing the x1F7 Command = xA0 Packet twice, whoops. Please choose for me from: 1) Keep only the atapi_start/ ata_tf_to_host_nolock/ ata_exec_command_pio. 2) Keep only the worker_thread/ atapi_packet_task/ ata_bmdma_start_pio/ ata_exec_command_pio. 3) Try something else entirely. > drivers/ide and Hale Landis's ATADRVR > (http://www.ata-atapi.com/) for comparing IO > write-for-write, to make sure I have the steps > and the state machine correct. And of course > the state machine docs itself, in ATA/ATAPI7. Yes. Myself, I have experience making Dos talk UDMA, a PATA bus analyser on my desk, and intermittent access to a SATA bus analyser. > you need to make sure your ATAPI device > supports ultra DMA, Please tell me if you want more evidence. The paper that accompanied my (Silicon Image 3611CT80 1.5) SATA/ PATA bridge tells me it talks UDMA "66, 100, 133 and 150" Mb/s though not UDMA 33/ MWDMA/ SWMA. I remember the paper that accompanied my PATA device tells me it talks at least UDMA 4, maybe actually UDMA 5. I know I have myself seen this device sustain about 1.5 GB/min = 25 MB/s. I could reboot to quote its op xEC Identify "word"s any time we like. > as you don't want > to bother with PIO or [SM]WDMA right now. I'm happy starting with SATA PIO, if you like that better. I do eventually want to talk SATA UDMA because I hope to end by achieving burst rates more like the 133 MB/s of UDMA, well above the 16.7 MB/s limit of PATA PIO 4. For ATAPI myself I recommend DMA only for well-known block ops, in order to get the exact byte counting of PIO otherwise. > > modprobe -r ata-piix > > modprobe ata-piix modprobe's still mostly work, but depending on my patch, I see that `modprobe ata-piix` may appear to halt my kernel with the HDD activity light lit after the dmesg: ata_bus_reset: ENTER, host 2, port 1 ata_bus_softreset: ata2: bus reset via SRST Pat LaVarre diff -Nurp linux-2.6.6-bk1/drivers/scsi/libata-core.c linux-2.6.6-bk1-pel/drivers/scsi/libata-core.c --- linux-2.6.6-bk1/drivers/scsi/libata-core.c 2004-05-12 09:57:09.000000000 -0600 +++ linux-2.6.6-bk1-pel/drivers/scsi/libata-core.c 2004-05-12 16:32:20.000000000 -0600 @@ -245,6 +245,9 @@ void ata_tf_load_mmio(struct ata_port *a void ata_exec_command_pio(struct ata_port *ap, struct ata_taskfile *tf) { DPRINTK("ata%u: cmd 0x%X\n", ap->id, tf->command); +if (tf->command == 0xA0) { + dump_stack(); +} outb(tf->command, ap->ioaddr.command_addr); ata_pause(ap); @@ -2438,6 +2441,7 @@ static int ata_qc_issue_prot(struct ata_ ata_dev_select(ap, qc->dev->devno, 1, 0); + DPRINTK("qc->tf.protocol = %d\n", qc->tf.protocol); switch (qc->tf.protocol) { case ATA_PROT_NODATA: ata_tf_to_host_nolock(ap, &qc->tf); @@ -2456,6 +2460,12 @@ static int ata_qc_issue_prot(struct ata_ queue_work(ata_wq, &ap->pio_task); break; + case ATA_PROT_ATAPI_DMA: + atapi_start(qc); + break; + + case ATA_PROT_ATAPI: + DPRINTK("Assertion fail: qc->tf.protocol != ATA_PROT_ATAPI\n"); default: WARN_ON(1); return -1; @@ -2812,6 +2822,9 @@ static void atapi_packet_task(void *_dat /* if we are DMA'ing, irq handler takes over from here */ if (qc->tf.protocol == ATA_PROT_ATAPI_DMA) { /* FIXME: start DMA here */ + DPRINTK("before bmdma_start\n"); + ap->ops->bmdma_start(qc); + DPRINTK("after bmdma_start\n"); } else { ap->pio_task_state = PIO_ST; queue_work(ata_wq, &ap->pio_task); diff -Nurp linux-2.6.6-bk1/drivers/scsi/libata-scsi.c linux-2.6.6-bk1-pel/drivers/scsi/libata-scsi.c --- linux-2.6.6-bk1/drivers/scsi/libata-scsi.c 2004-05-12 09:57:09.000000000 -0600 +++ linux-2.6.6-bk1-pel/drivers/scsi/libata-scsi.c 2004-05-12 15:51:10.000000000 -0600 @@ -215,6 +215,45 @@ int ata_scsi_error(struct Scsi_Host *hos DPRINTK("EXIT\n"); return 0; +} + +/** + * atapi_xlat - Pass SCSI r/w command thru to ATAPI + * @qc: Storage for translated ATA taskfile + * @scsicmd: SCSI command to translate + * + * Trust caller already cleared *qc->tf (often via ata_tf_init), + * deciding tf->device & (0x10 ATA_DEV1 | 0x07 SFF 8070i LUN), etc. + * + * Choose ATAPI PIO BCL from: + * + * 0x2000 = 8 Ki = one SATA FIS + * 0xFFFE = largest specified in oldest public docs + * 0xFFFF = first massively distributed (Win 95B) + * + * RETURNS: + * Zero on success, non-zero on error. + */ + +static unsigned int atapi_xlat(struct ata_queued_cmd *qc, u8 *scsicmd) +{ + int bcl = (8 * 0x400); /* PIO "byte count limit" */ + struct ata_taskfile *tf = &qc->tf; + tf->flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE; + tf->protocol = qc->dev->xfer_protocol; + tf->command = ATA_CMD_PACKET; /* often 0xA0 */ + if (qc->flags & ATA_QCFLAG_DMA) { + DPRINTK("DMA ATAPI 0x%lX %d\n", qc->flags, qc->tf.protocol); + qc->tf.protocol = ATA_PROT_ATAPI_DMA; + tf->feature = ATAPI_PKT_DMA; /* often x01 */ + } else { + DPRINTK("PIO ATAPI 0x%lX %d\n", qc->flags, qc->tf.protocol); + qc->tf.protocol = ATA_PROT_ATAPI; + tf->lbam = bcl >> 8; + tf->lbah = bcl; + } + qc->flags |= ATA_QCFLAG_ATAPI; + return 0; } /** @@ -885,78 +924,6 @@ void ata_scsi_badcmd(struct scsi_cmnd *c } /** - * atapi_scsi_queuecmd - Send CDB to ATAPI device - * @ap: Port to which ATAPI device is attached. - * @dev: Target device for CDB. - * @cmd: SCSI command being sent to device. - * @done: SCSI command completion function. - * - * Sends CDB to ATAPI device. If the Linux SCSI layer sends a - * non-data command, then this function handles the command - * directly, via polling. Otherwise, the bmdma engine is started. - * - * LOCKING: - * spin_lock_irqsave(host_set lock) - */ - -static void atapi_scsi_queuecmd(struct ata_port *ap, struct ata_device *dev, - struct scsi_cmnd *cmd, void (*done)(struct scsi_cmnd *)) -{ - struct ata_queued_cmd *qc; - u8 *scsicmd = cmd->cmnd; - - VPRINTK("ENTER, drv_stat = 0x%x\n", ata_chk_status(ap)); - - if (cmd->sc_data_direction == SCSI_DATA_UNKNOWN) { - DPRINTK("unknown data, scsicmd 0x%x\n", scsicmd[0]); - ata_bad_cdb(cmd, done); - return; - } - - switch(scsicmd[0]) { - case READ_6: - case WRITE_6: - case MODE_SELECT: - case MODE_SENSE: - DPRINTK("read6/write6/modesel/modesense trap\n"); - ata_bad_scsiop(cmd, done); - return; - - default: - /* do nothing */ - break; - } - - qc = ata_scsi_qc_new(ap, dev, cmd, done); - if (!qc) { - printk(KERN_ERR "ata%u: command queue empty\n", ap->id); - return; - } - - qc->flags |= ATA_QCFLAG_ATAPI; - - qc->tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE; - if (cmd->sc_data_direction == SCSI_DATA_WRITE) { - qc->tf.flags |= ATA_TFLAG_WRITE; - DPRINTK("direction: write\n"); - } - - qc->tf.command = ATA_CMD_PACKET; - - if (cmd->sc_data_direction == SCSI_DATA_NONE) { - qc->tf.protocol = ATA_PROT_ATAPI; - qc->flags |= ATA_QCFLAG_POLL; - qc->tf.ctl |= ATA_NIEN; /* disable interrupts */ - } else { - qc->tf.protocol = ATA_PROT_ATAPI_DMA; - qc->flags |= ATA_QCFLAG_SG; /* data is present; dma-map it */ - qc->tf.feature |= ATAPI_PKT_DMA; - } - - atapi_start(qc); -} - -/** * ata_scsi_find_dev - lookup ata_device from scsi_cmnd * @ap: ATA port to which the device is attached * @cmd: SCSI command to be sent to the device @@ -1010,8 +977,12 @@ ata_scsi_find_dev(struct ata_port *ap, s * Pointer to translation function if possible, %NULL if not. */ -static inline ata_xlat_func_t ata_get_xlat_func(u8 cmd) +static inline ata_xlat_func_t ata_get_xlat_func(struct ata_device *dev, u8 cmd) { + if (dev->class == ATA_DEV_ATAPI) { /* (thus != ATA_DEV_ATAPI_UNSUP) */ + return atapi_xlat; + } + switch (cmd) { case READ_6: case READ_10: @@ -1072,6 +1043,7 @@ int ata_scsi_queuecmd(struct scsi_cmnd * { struct ata_port *ap; struct ata_device *dev; + ata_xlat_func_t xlat_func; ap = (struct ata_port *) &cmd->device->host->hostdata[0]; @@ -1084,15 +1056,11 @@ int ata_scsi_queuecmd(struct scsi_cmnd * goto out_unlock; } - if (dev->class == ATA_DEV_ATA) { - ata_xlat_func_t xlat_func = ata_get_xlat_func(cmd->cmnd[0]); - - if (xlat_func) - ata_scsi_translate(ap, dev, cmd, done, xlat_func); - else - ata_scsi_simulate(ap, dev, cmd, done); - } else - atapi_scsi_queuecmd(ap, dev, cmd, done); + xlat_func = ata_get_xlat_func(dev, cmd->cmnd[0]); + if (xlat_func) + ata_scsi_translate(ap, dev, cmd, done, xlat_func); + else + ata_scsi_simulate(ap, dev, cmd, done); out_unlock: return 0; ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: SATA ATAPI work in progress 2004-05-12 23:14 ` Pat LaVarre @ 2004-05-13 15:07 ` Pat LaVarre 2004-05-13 20:56 ` Jeff Garzik 2004-05-13 21:16 ` Jeff Garzik 1 sibling, 1 reply; 34+ messages in thread From: Pat LaVarre @ 2004-05-13 15:07 UTC (permalink / raw) To: jgarzik; +Cc: linux-ide > Please choose for me from: > > 1) Keep only the atapi_start/ ata_tf_to_host_nolock/ > ata_exec_command_pio. > > 2) Keep only the worker_thread/ atapi_packet_task/ ata_bmdma_start_pio/ > ata_exec_command_pio. > > 3) Try something else entirely. I'll first try keeping only the ata_exec_command_pio of ata_bmdma_start_pio. I'm thinking we want the bus mastering DMA set up to reliably catch the rising edge of a potentially rapid device INTRQ before we write the last "word" of Command Out. Pat LaVarre ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: SATA ATAPI work in progress 2004-05-13 15:07 ` Pat LaVarre @ 2004-05-13 20:56 ` Jeff Garzik 0 siblings, 0 replies; 34+ messages in thread From: Jeff Garzik @ 2004-05-13 20:56 UTC (permalink / raw) To: Pat LaVarre; +Cc: linux-ide Pat LaVarre wrote: > I'll first try keeping only the ata_exec_command_pio of > ata_bmdma_start_pio. A bit busy today, I'll ponder a bit and respond to your previous mail. > I'm thinking we want the bus mastering DMA set up to reliably catch the > rising edge of a potentially rapid device INTRQ before we write the last > "word" of Command Out. Your basic statement is correct, but remember in the new world of SATA FIS's, you must concentrate on when the HBA transmits the H2D Register FIS (command FIS) to the device, and when the device responds in some manner (either with DMA setup and Data FIS conversations, for which the DMA engine must be armed, or with an error via D2H Register FIS) An interrupt will not be asserted until the command FIS has been sent, and some sort of response is received from the device. Further, and perhaps more importantly, everything is inside spin_lock_irqsave(), so interrupts will be handled after the lock is released. Jeff ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: SATA ATAPI work in progress 2004-05-12 23:14 ` Pat LaVarre 2004-05-13 15:07 ` Pat LaVarre @ 2004-05-13 21:16 ` Jeff Garzik 2004-05-13 21:25 ` Jeff Garzik ` (2 more replies) 1 sibling, 3 replies; 34+ messages in thread From: Jeff Garzik @ 2004-05-13 21:16 UTC (permalink / raw) To: Pat LaVarre; +Cc: linux-ide Pat LaVarre wrote: > The dmesg tell me my guess below of how to do this actually ended up > writing the x1F7 Command = xA0 Packet twice, whoops. > > Please choose for me from: > > 1) Keep only the atapi_start/ ata_tf_to_host_nolock/ > ata_exec_command_pio. > > 2) Keep only the worker_thread/ atapi_packet_task/ ata_bmdma_start_pio/ > ata_exec_command_pio. > > 3) Try something else entirely. You are close: For an ATA device, we * write the taskfile, except for command * set up the DMA engine * write the command * write DMA-start bit And the functions used by the low-level drivers, ata_bmdma_start_mmio and ata_bmdma_start_pio, are hardcoded to use this ordering. By contrast, for ATAPI you should do: * set up DMA engine * write entire taskfile, including command * proceed through state diagram until DRQ==1 * write SCSI CDB * write DMA-start bit >>as you don't want >>to bother with PIO or [SM]WDMA right now. > > > I'm happy starting with SATA PIO, if you like that better. > > I do eventually want to talk SATA UDMA because I hope to end by > achieving burst rates more like the 133 MB/s of UDMA, well above the > 16.7 MB/s limit of PATA PIO 4. > > For ATAPI myself I recommend DMA only for well-known block ops, in order > to get the exact byte counting of PIO otherwise. I prefer starting with Ultra DMA, since that will not only be common case, but it is also how the engine is set up right now. There are a few internal, to-be-removed limitations if you do not use ultra DMA. > diff -Nurp linux-2.6.6-bk1/drivers/scsi/libata-scsi.c linux-2.6.6-bk1-pel/drivers/scsi/libata-scsi.c > --- linux-2.6.6-bk1/drivers/scsi/libata-scsi.c 2004-05-12 09:57:09.000000000 -0600 > +++ linux-2.6.6-bk1-pel/drivers/scsi/libata-scsi.c 2004-05-12 15:51:10.000000000 -0600 > @@ -215,6 +215,45 @@ int ata_scsi_error(struct Scsi_Host *hos > > DPRINTK("EXIT\n"); > return 0; > +} > + > +/** > + * atapi_xlat - Pass SCSI r/w command thru to ATAPI > + * @qc: Storage for translated ATA taskfile > + * @scsicmd: SCSI command to translate > + * > + * Trust caller already cleared *qc->tf (often via ata_tf_init), > + * deciding tf->device & (0x10 ATA_DEV1 | 0x07 SFF 8070i LUN), etc. > + * > + * Choose ATAPI PIO BCL from: > + * > + * 0x2000 = 8 Ki = one SATA FIS > + * 0xFFFE = largest specified in oldest public docs > + * 0xFFFF = first massively distributed (Win 95B) > + * > + * RETURNS: > + * Zero on success, non-zero on error. > + */ > + > +static unsigned int atapi_xlat(struct ata_queued_cmd *qc, u8 *scsicmd) > +{ > + int bcl = (8 * 0x400); /* PIO "byte count limit" */ Make this a more obvious "8 * 1024". For the sake of correctness, I prefer to call this "DRQ block size" rather than the more ambigous "byte count limit". This is important because some AHCI SATA controllers can only transfer a _single_ DRQ block per disk transaction. In anticipation of the demise of PIO, no doubt :) > + struct ata_taskfile *tf = &qc->tf; > + tf->flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE; > + tf->protocol = qc->dev->xfer_protocol; > + tf->command = ATA_CMD_PACKET; /* often 0xA0 */ kill the comment, this should be obvious to anyone who reads the public specs. That's why we have the constant, after all... > + if (qc->flags & ATA_QCFLAG_DMA) { > + DPRINTK("DMA ATAPI 0x%lX %d\n", qc->flags, qc->tf.protocol); > + qc->tf.protocol = ATA_PROT_ATAPI_DMA; > + tf->feature = ATAPI_PKT_DMA; /* often x01 */ kill the comment > + } else { > + DPRINTK("PIO ATAPI 0x%lX %d\n", qc->flags, qc->tf.protocol); > + qc->tf.protocol = ATA_PROT_ATAPI; > + tf->lbam = bcl >> 8; > + tf->lbah = bcl; > + } > + qc->flags |= ATA_QCFLAG_ATAPI; > + return 0; > } key flags that should be used, but are just being deleted in the previous chunk: ATA_QCFLAG_SG ATA_NIEN ATA_QCFLAG_POLL Their proper placement is left as an exercise to the reader, who seems to be doing well so far :) > /** > @@ -885,78 +924,6 @@ void ata_scsi_badcmd(struct scsi_cmnd *c > } > > /** > - * atapi_scsi_queuecmd - Send CDB to ATAPI device > - * @ap: Port to which ATAPI device is attached. > - * @dev: Target device for CDB. > - * @cmd: SCSI command being sent to device. > - * @done: SCSI command completion function. > - * > - * Sends CDB to ATAPI device. If the Linux SCSI layer sends a > - * non-data command, then this function handles the command > - * directly, via polling. Otherwise, the bmdma engine is started. > - * > - * LOCKING: > - * spin_lock_irqsave(host_set lock) > - */ > - > -static void atapi_scsi_queuecmd(struct ata_port *ap, struct ata_device *dev, > - struct scsi_cmnd *cmd, void (*done)(struct scsi_cmnd *)) > -{ > - struct ata_queued_cmd *qc; > - u8 *scsicmd = cmd->cmnd; > - > - VPRINTK("ENTER, drv_stat = 0x%x\n", ata_chk_status(ap)); > - > - if (cmd->sc_data_direction == SCSI_DATA_UNKNOWN) { > - DPRINTK("unknown data, scsicmd 0x%x\n", scsicmd[0]); > - ata_bad_cdb(cmd, done); > - return; > - } > - > - switch(scsicmd[0]) { > - case READ_6: > - case WRITE_6: > - case MODE_SELECT: > - case MODE_SENSE: > - DPRINTK("read6/write6/modesel/modesense trap\n"); > - ata_bad_scsiop(cmd, done); > - return; > - > - default: > - /* do nothing */ > - break; > - } > - > - qc = ata_scsi_qc_new(ap, dev, cmd, done); > - if (!qc) { > - printk(KERN_ERR "ata%u: command queue empty\n", ap->id); > - return; > - } > - > - qc->flags |= ATA_QCFLAG_ATAPI; > - > - qc->tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE; > - if (cmd->sc_data_direction == SCSI_DATA_WRITE) { > - qc->tf.flags |= ATA_TFLAG_WRITE; > - DPRINTK("direction: write\n"); > - } > - > - qc->tf.command = ATA_CMD_PACKET; > - > - if (cmd->sc_data_direction == SCSI_DATA_NONE) { > - qc->tf.protocol = ATA_PROT_ATAPI; > - qc->flags |= ATA_QCFLAG_POLL; > - qc->tf.ctl |= ATA_NIEN; /* disable interrupts */ > - } else { > - qc->tf.protocol = ATA_PROT_ATAPI_DMA; > - qc->flags |= ATA_QCFLAG_SG; /* data is present; dma-map it */ > - qc->tf.feature |= ATAPI_PKT_DMA; > - } > - > - atapi_start(qc); > -} > - > -/** > * ata_scsi_find_dev - lookup ata_device from scsi_cmnd > * @ap: ATA port to which the device is attached > * @cmd: SCSI command to be sent to the device > @@ -1010,8 +977,12 @@ ata_scsi_find_dev(struct ata_port *ap, s > * Pointer to translation function if possible, %NULL if not. > */ > > -static inline ata_xlat_func_t ata_get_xlat_func(u8 cmd) > +static inline ata_xlat_func_t ata_get_xlat_func(struct ata_device *dev, u8 cmd) > { > + if (dev->class == ATA_DEV_ATAPI) { /* (thus != ATA_DEV_ATAPI_UNSUP) */ > + return atapi_xlat; > + } > + > switch (cmd) { > case READ_6: > case READ_10: Remove the extraneous braces and the comment :) otherwise OK. > @@ -1072,6 +1043,7 @@ int ata_scsi_queuecmd(struct scsi_cmnd * > { > struct ata_port *ap; > struct ata_device *dev; > + ata_xlat_func_t xlat_func; > > ap = (struct ata_port *) &cmd->device->host->hostdata[0]; > OK > @@ -1084,15 +1056,11 @@ int ata_scsi_queuecmd(struct scsi_cmnd * > goto out_unlock; > } > > - if (dev->class == ATA_DEV_ATA) { > - ata_xlat_func_t xlat_func = ata_get_xlat_func(cmd->cmnd[0]); > - > - if (xlat_func) > - ata_scsi_translate(ap, dev, cmd, done, xlat_func); > - else > - ata_scsi_simulate(ap, dev, cmd, done); > - } else > - atapi_scsi_queuecmd(ap, dev, cmd, done); > + xlat_func = ata_get_xlat_func(dev, cmd->cmnd[0]); > + if (xlat_func) > + ata_scsi_translate(ap, dev, cmd, done, xlat_func); > + else > + ata_scsi_simulate(ap, dev, cmd, done); OK ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: SATA ATAPI work in progress 2004-05-13 21:16 ` Jeff Garzik @ 2004-05-13 21:25 ` Jeff Garzik 2004-05-13 21:36 ` Pat LaVarre 2004-05-14 18:23 ` Pat LaVarre 2004-05-14 18:28 ` Pat LaVarre 2 siblings, 1 reply; 34+ messages in thread From: Jeff Garzik @ 2004-05-13 21:25 UTC (permalink / raw) To: Pat LaVarre; +Cc: linux-ide Jeff Garzik wrote: > For an ATA device, we > * write the taskfile, except for command > * set up the DMA engine > * write the command > * write DMA-start bit > > And the functions used by the low-level drivers, ata_bmdma_start_mmio > and ata_bmdma_start_pio, are hardcoded to use this ordering. By > contrast, for ATAPI you should do: > * set up DMA engine > * write entire taskfile, including command > * proceed through state diagram until DRQ==1 > * write SCSI CDB > * write DMA-start bit So... Are you willing to work on a separate, pre-requisite patch: We need to split ata_bmdma_start_{pio,mmio} into separate "setup" and "start" pieces. This would involve adding a "bmdma_setup" hook to struct ata_port_operations in include/linux/libata.h, and then doing a simple update of all the SATA drivers, to use the default libata-core implementations. Jeff ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: SATA ATAPI work in progress 2004-05-13 21:25 ` Jeff Garzik @ 2004-05-13 21:36 ` Pat LaVarre 2004-05-13 21:44 ` Jeff Garzik 0 siblings, 1 reply; 34+ messages in thread From: Pat LaVarre @ 2004-05-13 21:36 UTC (permalink / raw) To: Jeff Garzik; +Cc: linux-ide > Are you willing to work Yes I want to learn. I figure I can learn fastest by helping. > on a separate, pre-requisite patch: > > We need to split ata_bmdma_start_{pio,mmio} into separate "setup" and > "start" pieces. > > This would involve adding a "bmdma_setup" hook to struct > ata_port_operations in include/linux/libata.h, and then doing a simple > update of all the SATA drivers, to use the default libata-core > implementations. Sorry as yet I understand very little of the above. Until corrected I will guess I should next: 1) Find a spare PATA UDMA HDD, with UDMA mode above 2 to suit the SATA/ PATA bridge. 2) Connect Linux to SATA/ PATA to PATA UDMA HDD. 3) Turn ATA_DEBUG ATA_VERBOSE_DEBUG on in 2.6.6-bk1. 4) Report back here for next assignment. Pat LaVarre ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: SATA ATAPI work in progress 2004-05-13 21:36 ` Pat LaVarre @ 2004-05-13 21:44 ` Jeff Garzik 2004-05-13 21:49 ` Jeff Garzik 0 siblings, 1 reply; 34+ messages in thread From: Jeff Garzik @ 2004-05-13 21:44 UTC (permalink / raw) To: Pat LaVarre; +Cc: linux-ide Pat LaVarre wrote: >>Are you willing to work > > > Yes I want to learn. > > I figure I can learn fastest by helping. > > >>on a separate, pre-requisite patch: >> >>We need to split ata_bmdma_start_{pio,mmio} into separate "setup" and >>"start" pieces. >> >>This would involve adding a "bmdma_setup" hook to struct >>ata_port_operations in include/linux/libata.h, and then doing a simple >>update of all the SATA drivers, to use the default libata-core >>implementations. > > > Sorry as yet I understand very little of the above. > > Until corrected I will guess I should next: > 4) Report back here for next assignment. hehe The above is something that applies to all of libata, and changes the libata API so that ATAPI is doable. It looks something like: 1) add ->bmdma_setup hook to include/linux/libata.h 2) walk through each of drivers/scsi/*ata_*.c and make an adjustment like the following: .bmdma_start: ata_bmdma_start_mmio, + .bmdma_setup: ata_bmdma_setup_mmio, 3) create new functions ata_bmdma_setup_{mmio,pio}. 4) move EVERYTHING except the final writeb() from ata_bmdma_start_mmio to ata_bmdma_setup_mmio. 5) do the same for ata_bmdma_{start,setup}_pio. at this point, it is highly likely that sata_promise.c and sata_sx4.c are broken, but the other drivers should work 100%. for bonus points, 6) fix sata_promise and sata_sx4 otherwise I can do that. The only hardware required for this task is a SATA hard drive and SATA host controller. Jeff ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: SATA ATAPI work in progress 2004-05-13 21:44 ` Jeff Garzik @ 2004-05-13 21:49 ` Jeff Garzik 0 siblings, 0 replies; 34+ messages in thread From: Jeff Garzik @ 2004-05-13 21:49 UTC (permalink / raw) To: Pat LaVarre; +Cc: linux-ide Jeff Garzik wrote: > 5) do the same for ata_bmdma_{start,setup}_pio. Whoops, I left out a rather important step: 5.5) Everywhere that libata-core and libata-scsi call the ->bmdma_start() hook, add a call to the new ->bmdma_setup hook immediately before it. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: SATA ATAPI work in progress 2004-05-13 21:16 ` Jeff Garzik 2004-05-13 21:25 ` Jeff Garzik @ 2004-05-14 18:23 ` Pat LaVarre 2004-05-14 18:55 ` Jeff Garzik 2004-05-14 18:28 ` Pat LaVarre 2 siblings, 1 reply; 34+ messages in thread From: Pat LaVarre @ 2004-05-14 18:23 UTC (permalink / raw) To: Jeff Garzik; +Cc: linux-ide Jeff G: > This is important because some AHCI SATA controllers can only transfer > a _single_ DRQ block per disk transaction. In anticipation of the > demise of PIO, no doubt :) Eh? What's a "disk transaction"? Do we mean to say that some SATA controllers choke over the (rare) device rudeness of an INTRQ for every two bytes of the standard x24 (36) bytes of op x12 (18) Inquiry, for example? > ... The rest of your instructions I believe I can follow as written. Below I explain why I find some of them surprising, in case that's interesting or useful to anyone here. Pat LaVarre ----- > For an ATA device, we > * write the taskfile, except for command > * set up the DMA engine > * write the command > * write DMA-start bit > > And the functions used by the low-level drivers, ata_bmdma_start_mmio > and ata_bmdma_start_pio, are hardcoded to use this ordering. By > contrast, for ATAPI you should do: > * set up DMA engine > * write entire taskfile, including command > * proceed through state diagram until DRQ==1 > * write SCSI CDB > * write DMA-start bit Why we care whether we write the taskfile apart from the command before or after DMA engine setup, I do not understand. > > + int bcl = (8 * 0x400); /* PIO "byte count limit" */ > > Make this a more obvious "8 * 1024". A point of pain for me moving into the world near linux-ide was discovering that `dd` wanted me to say 512 rather than the more familiar and plainly rounded value x200. > For the sake of correctness, I prefer to call this "DRQ block size" > rather than the more ambigous "byte count limit". "Byte count limit" is the unfortunate phrase found at t13.org. I will assume I should use your term but may once provide a cross-referencing translation to the standard unfortunate phrase as a comment. > > + struct ata_taskfile *tf = &qc->tf; > > + tf->flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE; > > + tf->protocol = qc->dev->xfer_protocol; > > + tf->command = ATA_CMD_PACKET; /* often 0xA0 */ > > kill the comment, this should be obvious to anyone who reads the > public specs. That's why we have the constant, after all... See below. > > + tf->feature = ATAPI_PKT_DMA; /* often x01 */ > > kill the comment I understood your objection to: ATA_CMD_PACKET; /* often 0xA0 */ to mean I should never tell people what hovering the mouse over an identifier should (but often doesn't) tell them. Me, I didn't know ATA_CMD_PACKET meant xA0 until I looked it up. I'm likely to forget again in future. ATA_CMD_PACKET is a Linux only term, whereas "A0" I find in every source I read, be it xA0 or A0h or 0A0h etc., be the source English, bus traces, machine code hex dumps, .... > > + } else { > > + DPRINTK("PIO ATAPI 0x%lX %d\n", qc->flags, > qc->tf.protocol); > > + qc->tf.protocol = ATA_PROT_ATAPI; > > + tf->lbam = bcl >> 8; > > + tf->lbah = bcl; > > + } > > + qc->flags |= ATA_QCFLAG_ATAPI; > > + return 0; > > } > > key flags that should be used, but are just being deleted in the > previous chunk: > ATA_QCFLAG_SG > ATA_NIEN > ATA_QCFLAG_POLL > > Their proper placement is left as an exercise to the reader, who seems > to be doing well so far :) Thank you for your kind words. > > + if (dev->class == ATA_DEV_ATAPI) { /* (thus != > ATA_DEV_ATAPI_UNSUP) */ > > + return atapi_xlat; > > + } > > + > > Remove the extraneous braces ... :) Understood to mean do not enclose in { } unless two or more statements appear as part of the then or else of an if. > > + if (dev->class == ATA_DEV_ATAPI) { /* (thus != > ATA_DEV_ATAPI_UNSUP) */ > > + return atapi_xlat; > > + } > > + > > Remove ... the comment :) Will do, without me yet having fully confirmed that ATA_DEV_ATAPI_UNSUP means known to be ATAPI but treated as ATA_DEV_UNKNOWN. Pat LaVarre ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: SATA ATAPI work in progress 2004-05-14 18:23 ` Pat LaVarre @ 2004-05-14 18:55 ` Jeff Garzik 2004-05-14 19:37 ` when limited to a single DRQ block per disk transaction Pat LaVarre 2004-05-14 23:47 ` SATA ATAPI work in progress Pat LaVarre 0 siblings, 2 replies; 34+ messages in thread From: Jeff Garzik @ 2004-05-14 18:55 UTC (permalink / raw) To: Pat LaVarre; +Cc: linux-ide Pat LaVarre wrote: > Jeff G: > > >>This is important because some AHCI SATA controllers can only transfer >>a _single_ DRQ block per disk transaction. In anticipation of the >>demise of PIO, no doubt :) > > > Eh? What's a "disk transaction"? The point when the Host-to-Device Register FIS ("command FIS") is sent to the device, all the DMA / Data FIS's sent, and then the final Device-to-Host Register FIS. In other words, one ATA taskfile, and the operations that that taskfile generates. Therefore, for controllers that are limited to a single DRQ block per disk transaction, the OS driver _must_ limit each ATA taskfile to no more than a DRQ block's worth of sector. If the ATAPI byte count limit is 8K, then the OS driver must guarantee that a single ATA taskfile does not transfer more than 8K. > Do we mean to say that some SATA controllers choke over the (rare) > device rudeness of an INTRQ for every two bytes of the standard x24 (36) > bytes of op x12 (18) Inquiry, for example? INTRQ is a PATA notion :) SATA controllers receive FIS's with an 'I' bit set, indicating the device wishes to assert an interrupt. [though certainly with bridges often on both sides, INTRQ hasn't gone away] >>For an ATA device, we >>* write the taskfile, except for command >>* set up the DMA engine >>* write the command >>* write DMA-start bit >> >>And the functions used by the low-level drivers, ata_bmdma_start_mmio >>and ata_bmdma_start_pio, are hardcoded to use this ordering. By >>contrast, for ATAPI you should do: >>* set up DMA engine >>* write entire taskfile, including command >>* proceed through state diagram until DRQ==1 >>* write SCSI CDB >>* write DMA-start bit > > > Why we care whether we write the taskfile apart from the command before > or after DMA engine setup, I do not understand. Actually, after research I prefer to set up the DMA engine first, then write the taskfile including command, for both cases. The latest libata atapi work (2.6.6 + the 5 patches I have posted to linux-ide) does not yet do this. >>>+ int bcl = (8 * 0x400); /* PIO "byte count limit" */ >> >>Make this a more obvious "8 * 1024". > > > A point of pain for me moving into the world near linux-ide was > discovering that `dd` wanted me to say 512 rather than the more familiar > and plainly rounded value x200. Using decimal and hexidecimal together is inconsistent, and I feel that 1024 reads to a human eye like "1K" much more than 0x400 does. And we're shooting clarity and readability here. >>>+ struct ata_taskfile *tf = &qc->tf; >>>+ tf->flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE; >>>+ tf->protocol = qc->dev->xfer_protocol; >>>+ tf->command = ATA_CMD_PACKET; /* often 0xA0 */ >> >>kill the comment, this should be obvious to anyone who reads the >>public specs. That's why we have the constant, after all... > > > See below. > > >>>+ tf->feature = ATAPI_PKT_DMA; /* often x01 */ >> >>kill the comment > > > I understood your objection to: ATA_CMD_PACKET; /* often 0xA0 */ > to mean I should never tell people what hovering the mouse over an > identifier should (but often doesn't) tell them. > > Me, I didn't know ATA_CMD_PACKET meant xA0 until I looked it up. I'm > likely to forget again in future. ATA_CMD_PACKET is a Linux only term, > whereas "A0" I find in every source I read, be it xA0 or A0h or 0A0h > etc., be the source English, bus traces, machine code hex dumps, .... You know the value of ATA_CMD_PACKET by looking in the header file. The constant exists to -remove- the hex number completely from the source code. The hex number should only be present in the header file, and in the specifications. It is -crucial- to long term maintenance that C source code be readable, even to someone who is not very familiar with the intimacies of the ATA protocol and associated hardware. (I do not claim libata is perfect in this regard, but it is an important goal) Therefore, regardless of what non-Linux people may do, it is a very conscious decision to avoid putting what we call "magic numbers" in the source code. Experience has shown using "magic numbers" just isn't scalable. It's not human-friendly. >>>+ if (dev->class == ATA_DEV_ATAPI) { /* (thus != >> >>ATA_DEV_ATAPI_UNSUP) */ >> >>>+ return atapi_xlat; >>>+ } >>>+ >> >>Remove the extraneous braces ... :) > > > Understood to mean do not enclose in { } unless two or more statements > appear as part of the then or else of an if. Correct. Jeff ^ permalink raw reply [flat|nested] 34+ messages in thread
* when limited to a single DRQ block per disk transaction 2004-05-14 18:55 ` Jeff Garzik @ 2004-05-14 19:37 ` Pat LaVarre 2004-05-14 19:50 ` Jeff Garzik 2004-05-14 23:47 ` SATA ATAPI work in progress Pat LaVarre 1 sibling, 1 reply; 34+ messages in thread From: Pat LaVarre @ 2004-05-14 19:37 UTC (permalink / raw) To: Jeff Garzik; +Cc: linux-ide Jeff G: > > > Re: SATA ATAPI work in progress > > > ... > > > some AHCI SATA controllers can only transfer a _single_ DRQ block > > > per disk transaction. In anticipation of the demise > > > of PIO, no doubt :) > ... > for controllers that are limited to a single DRQ block per > disk transaction, Please can you say which controllers exhibit this limitation? > the OS driver _must_ limit each ATA taskfile to no > more than a DRQ block's worth of sector. If the ATAPI byte count limit > is 8K, then the OS driver must guarantee that a single ATA taskfile does > not transfer more than 8K. Please can you give an example of a disk transaction that doesn't work on some controllers? I ask because I think somehow even this late in our thread I do not yet understand what the limitation under discussion is. Do we mean to be discussing a limit of one PATA DRQ INTRQ per CDB? Pat LaVarre ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: when limited to a single DRQ block per disk transaction 2004-05-14 19:37 ` when limited to a single DRQ block per disk transaction Pat LaVarre @ 2004-05-14 19:50 ` Jeff Garzik 2004-05-14 20:12 ` Pat LaVarre 0 siblings, 1 reply; 34+ messages in thread From: Jeff Garzik @ 2004-05-14 19:50 UTC (permalink / raw) To: Pat LaVarre; +Cc: linux-ide Pat LaVarre wrote: > Please can you say which controllers exhibit this limitation? AHCI. PIO is being deprecated... future ATA will be DMA-only. > Do we mean to be discussing a limit of one PATA DRQ INTRQ per CDB? Correct. If DRQ block size is 16 sectors, then each CDB is limited to a maximum of 16 sectors. This limit does not apply to DMA. Only to PIO, PIO multi, and ATAPI PIO. Jeff ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: when limited to a single DRQ block per disk transaction 2004-05-14 19:50 ` Jeff Garzik @ 2004-05-14 20:12 ` Pat LaVarre 0 siblings, 0 replies; 34+ messages in thread From: Pat LaVarre @ 2004-05-14 20:12 UTC (permalink / raw) To: Jeff Garzik; +Cc: linux-ide Jeff G: > This limit does not apply to DMA. > Only to PIO, PIO multi, and ATAPI PIO. Understood. > > Do we mean to be discussing a limit of one PATA DRQ INTRQ per CDB? > > Correct. > > If DRQ block size is 16 sectors, then each CDB is limited to a maximum > of 16 sectors. To me, the limit looks worse than that. [If I still don't get this in your next reply, I'll now volunteer to wait a day before I reply again in this thread.] In PATA ATA PIO, distinct from ATAPI PIO, to guarantee at most one PATA DRQ INTRQ per CDB when reading or writing more than one LBA/CDB, we have to resort to ops x C4 C5 C6 = "READ MULTIPLE", "WRITE MULTIPLE", "SET MULTIPLE MODE". linux-2.6.6-bk1/include/linux/* gives these ops the names WIN_SETMULT WIN_MULTREAD WIN_MULTWRITE. In PATA ATAPI PIO, distinct from ATA PIO, we cannot guarantee at most one PATA DRQ INTRQ per CDB, unless we limit the host to reading at most one LBA/CDB, because the ATAPI device may issue more than the minimum number of INTRQ that a chosen byte count limit guarantees. Therefore the AHCI limit of at most one PATA DRQ INTRQ per CDB is in effect an abolition of ATAPI PIO block i/o, except for the mostly theoretical possibility of hosts and devices that together tolerate the overhead of shattering a stream into single blocks. Else I've misunderstood something vital. Pat LaVarre ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: SATA ATAPI work in progress 2004-05-14 18:55 ` Jeff Garzik 2004-05-14 19:37 ` when limited to a single DRQ block per disk transaction Pat LaVarre @ 2004-05-14 23:47 ` Pat LaVarre 2004-05-15 0:02 ` Pat LaVarre 1 sibling, 1 reply; 34+ messages in thread From: Pat LaVarre @ 2004-05-14 23:47 UTC (permalink / raw) To: Jeff Garzik; +Cc: linux-ide Applying patch.[12345] in order to linux-2.6.6-bk1/ here yielded: kernel: ata_scsi_dump_cdb: CDB (1:1,15,0) 12 00 00 00 24 00 00 00 46 kernel: ata_scsi_dump_cdb: CDB (2:0,0,0) 12 00 00 00 24 00 00 00 46 kernel: ata_scsi_translate: ENTER kernel: ata_sg_setup_one: mapped buffer of 36 bytes for read kernel: ata_fill_sg: PRD[0] = (0x469E54, 0x24) kernel: ata_dev_select: ENTER, ata2: device 0, wait 1 kernel: ata_tf_load_pio: feat 0x1 nsect 0x0 lba 0x0 0x0 0x0 kernel: ata_tf_load_pio: device 0xA0 kernel: ata_exec_command_pio: ata2: cmd 0xA0 kernel: ata_scsi_translate: EXIT kernel: atapi_packet_task: busy wait kernel: atapi_packet_task: send cdb kernel: ata_scsi_error: ENTER kernel: ata_eng_timeout: ENTER kernel: ata2: unknown timeout, cmd 0xa0 stat 0xd0 kernel: ata_sg_clean: unmapping 1 sg elements kernel: ata_eng_timeout: EXIT kernel: ata_scsi_error: EXIT kernel: ata_scsi_dump_cdb: CDB (2:0,1,0) 12 00 00 00 24 00 00 00 46 We now get only one ata_exec_command_pio, good. Next I will try o x1F1 Features = x05 Dma In and x01 Dma Out because: > Subject: Re: SATA ATAPI UDMA - first in Linux > ... > Silicon Image has introduced an addition to ATA called the "DMADIR" bit. > Their bridges require this bit in order to use DMA. > > If your OS driver does not know about DMADIR (read: most OS drivers), > then it will fall back _unconditionally_ to PIO. http://www.google.com/search?q=silicon+image+dmadir yields the proposal: Packet DMA Issue ... http://www.t13.org/docs2003/e03132r3.pdf Seemingly impractical on various grounds, such as requiring different device firmware for an old or new host, but including the detail: --- Features ... [x04] DMADIR ... Support for this bit is determined by reading bit 15 of word 62 in the device configuration data. If bit 15 of word 62 is set to 1, the device requires the use of the DMADIR bit for Packet DMA commands. If the device requires the DMADIR bit to be set for Packet DMA operations and the current operations is DMA (i.e. bit 0, the DMA bit, is set), this bit indicates the direction of data transfer (0 = transfer to the device; 1 = transfer to the host). --- Pat LaVarre ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: SATA ATAPI work in progress 2004-05-14 23:47 ` SATA ATAPI work in progress Pat LaVarre @ 2004-05-15 0:02 ` Pat LaVarre 2004-05-15 0:38 ` Jeff Garzik 2004-05-15 0:46 ` [PATCH] " Jeff Garzik 0 siblings, 2 replies; 34+ messages in thread From: Pat LaVarre @ 2004-05-15 0:02 UTC (permalink / raw) To: Jeff Garzik; +Cc: linux-ide [-- Attachment #1: Type: text/plain, Size: 2586 bytes --] > > Applying patch.[12345] in order to linux-2.6.6-bk1/ here yielded: > > ... > > kernel: ata_tf_load_pio: feat 0x1 nsect 0x0 lba 0x0 0x0 0x0 > > kernel: ata_tf_load_pio: device 0xA0 > > kernel: ata_exec_command_pio: ata2: cmd 0xA0 > > kernel: ata_scsi_translate: EXIT > > kernel: atapi_packet_task: busy wait > > kernel: atapi_packet_task: send cdb > > kernel: ata_scsi_error: ENTER > > kernel: ata_eng_timeout: ENTER > > kernel: ata2: unknown timeout, cmd 0xa0 stat 0xd0 > > ... > > Next I will try > > o x1F1 Features = x05 Dma In and x01 Dma Out because ... Results follow. Much improved now, yes? Pat LaVarre kernel: ata_scsi_dump_cdb: CDB (2:0,0,0) 12 00 00 00 24 00 00 00 46 kernel: ata_scsi_translate: ENTER kernel: ata_sg_setup_one: mapped buffer of 36 bytes for read kernel: ata_fill_sg: PRD[0] = (0x49EE54, 0x24) kernel: ata_dev_select: ENTER, ata2: device 0, wait 1 kernel: ata_tf_load_pio: feat 0x5 nsect 0x0 lba 0x0 0x0 0x0 kernel: ata_tf_load_pio: device 0xA0 kernel: ata_exec_command_pio: ata2: cmd 0xA0 kernel: ata_scsi_translate: EXIT kernel: atapi_packet_task: busy wait kernel: atapi_packet_task: send cdb kernel: irq 18: nobody cared! kernel: Call Trace: kernel: [<c0107f83>] __report_bad_irq+0x2a/0x8b kernel: [<c010806d>] note_interrupt+0x6f/0x9f kernel: [<c01083a1>] do_IRQ+0x175/0x1a6 kernel: [<c0103aae>] default_idle+0x0/0x2d kernel: [<c01065c0>] common_interrupt+0x18/0x20 kernel: [<c0103aae>] default_idle+0x0/0x2d kernel: [<c0103ad8>] default_idle+0x2a/0x2d kernel: [<c0103b4c>] cpu_idle+0x37/0x40 kernel: [<c044289c>] start_kernel+0x1b1/0x202 kernel: [<c0442428>] unknown_bootoption+0x0/0x129 kernel: kernel: handlers: kernel: [<df9675a0>] (ata_interrupt+0x0/0x1a5 [libata]) kernel: Disabling IRQ #18 kernel: ata_scsi_error: ENTER kernel: ata_eng_timeout: ENTER kernel: ata2: unknown timeout, cmd 0xa0 stat 0x50 kernel: ata_sg_clean: unmapping 1 sg elements kernel: ata_eng_timeout: EXIT kernel: ata_scsi_error: EXIT kernel: ata_scsi_dump_cdb: CDB (2:0,0,0) 12 00 00 00 c0 00 00 00 46 diff -Nurp o/drivers/scsi linux-2.6.6-bk1/drivers/scsi/libata-scsi.c --- o/drivers/scsi/libata-scsi.c 2004-05-14 18:00:22.472765072 -0600 +++ linux-2.6.6-bk1/drivers/scsi/libata-scsi.c 2004-05-14 17:51:38.000000000 -0600 @@ -920,6 +920,9 @@ static unsigned int atapi_xlat(struct at qc->flags |= ATA_QCFLAG_SG; /* data is present; dma-map it */ qc->tf.protocol = ATA_PROT_ATAPI_DMA; qc->tf.feature |= ATAPI_PKT_DMA; + if (cmd->sc_data_direction != SCSI_DATA_WRITE) { + qc->tf.feature |= 0x04; /* Silicon Image DMADIR */ + } } return 0; [-- Attachment #2: pel.patch.6 --] [-- Type: text/x-patch, Size: 554 bytes --] diff -Nurp o/drivers/scsi linux-2.6.6-bk1/drivers/scsi/libata-scsi.c --- o/drivers/scsi/libata-scsi.c 2004-05-14 18:00:22.472765072 -0600 +++ linux-2.6.6-bk1/drivers/scsi/libata-scsi.c 2004-05-14 17:51:38.000000000 -0600 @@ -920,6 +920,9 @@ static unsigned int atapi_xlat(struct at qc->flags |= ATA_QCFLAG_SG; /* data is present; dma-map it */ qc->tf.protocol = ATA_PROT_ATAPI_DMA; qc->tf.feature |= ATAPI_PKT_DMA; + if (cmd->sc_data_direction != SCSI_DATA_WRITE) { + qc->tf.feature |= 0x04; /* Silicon Image DMADIR */ + } } return 0; ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: SATA ATAPI work in progress 2004-05-15 0:02 ` Pat LaVarre @ 2004-05-15 0:38 ` Jeff Garzik 2004-05-15 13:06 ` Pat LaVarre 2004-05-15 0:46 ` [PATCH] " Jeff Garzik 1 sibling, 1 reply; 34+ messages in thread From: Jeff Garzik @ 2004-05-15 0:38 UTC (permalink / raw) To: Pat LaVarre; +Cc: linux-ide [-- Attachment #1: Type: text/plain, Size: 248 bytes --] Pat LaVarre wrote: > Results follow. Much improved now, yes? Yep, that's definitely a Silicon Image bridge all right :) Here's a better version of the patch you generated. Next thing is to handle ATA_PROT_ATAPI_DMA in ata_host_intr... Jeff [-- Attachment #2: patch --] [-- Type: text/plain, Size: 2219 bytes --] ===== drivers/scsi/libata-core.c 1.57 vs edited ===== --- 1.57/drivers/scsi/libata-core.c Thu May 13 22:45:55 2004 +++ edited/drivers/scsi/libata-core.c Fri May 14 20:35:23 2004 @@ -1168,6 +1168,9 @@ if (ata_id_is_ata(dev)) /* sanity check */ goto err_out_nosup; + if (ata_id_use_dmadir(dev)) + dev->flags |= ATA_DFLAG_DMADIR; + /* see if 16-byte commands supported */ tmp = dev->id[0] & 0x3; if (tmp == 1) ===== drivers/scsi/libata-scsi.c 1.29 vs edited ===== --- 1.29/drivers/scsi/libata-scsi.c Thu May 13 22:39:12 2004 +++ edited/drivers/scsi/libata-scsi.c Fri May 14 20:36:28 2004 @@ -920,6 +920,9 @@ qc->flags |= ATA_QCFLAG_SG; /* data is present; dma-map it */ qc->tf.protocol = ATA_PROT_ATAPI_DMA; qc->tf.feature |= ATAPI_PKT_DMA; + if ((qc->dev->flags & ATA_DFLAG_DMADIR) && + (cmd->sc_data_direction != SCSI_DATA_WRITE)) + qc->tf.feature |= ATAPI_DMADIR; } return 0; ===== include/linux/ata.h 1.5 vs edited ===== --- 1.5/include/linux/ata.h Mon May 10 21:49:59 2004 +++ edited/include/linux/ata.h Fri May 14 20:34:17 2004 @@ -84,6 +84,8 @@ ATA_ERR = (1 << 0), /* have an error */ ATA_SRST = (1 << 2), /* software reset */ ATA_ABORTED = (1 << 2), /* command aborted */ + ATAPI_DMADIR = (1 << 2), /* ATAPI data dir: + 0=to device, 1=to host */ /* ATA command block registers */ ATA_REG_DATA = 0x00, @@ -203,6 +205,7 @@ #define ata_id_has_wcache(dev) ((dev)->id[82] & (1 << 5)) #define ata_id_has_lba(dev) ((dev)->id[49] & (1 << 8)) #define ata_id_has_dma(dev) ((dev)->id[49] & (1 << 9)) +#define ata_id_use_dmadir(dev) ((dev)->id[62] & (1 << 15)) #define ata_id_u32(dev,n) \ (((u32) (dev)->id[(n) + 1] << 16) | ((u32) (dev)->id[(n)])) #define ata_id_u64(dev,n) \ ===== include/linux/libata.h 1.29 vs edited ===== --- 1.29/include/linux/libata.h Thu May 13 22:45:55 2004 +++ edited/include/linux/libata.h Fri May 14 20:28:37 2004 @@ -90,6 +90,7 @@ ATA_DFLAG_MASTER = (1 << 2), /* is device 0? */ ATA_DFLAG_WCACHE = (1 << 3), /* has write cache we can * (hopefully) flush? */ + ATA_DFLAG_DMADIR = (1 << 4), /* use DMADIR bit in ATAPI */ ATA_DEV_UNKNOWN = 0, /* unknown device */ ATA_DEV_ATA = 1, /* ATA device */ ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: SATA ATAPI work in progress 2004-05-15 0:38 ` Jeff Garzik @ 2004-05-15 13:06 ` Pat LaVarre 2004-05-15 13:49 ` Pat LaVarre 0 siblings, 1 reply; 34+ messages in thread From: Pat LaVarre @ 2004-05-15 13:06 UTC (permalink / raw) To: Jeff Garzik; +Cc: linux-ide > > Results follow. Much improved now, yes? > > Yep, that's definitely a Silicon Image bridge all right :) > > Here's a better version of the patch you generated. > > ... > + ATAPI_DMADIR = (1 << 2), /* ATAPI data dir: > ... Thank you. I name this patch.6 as in a virtual thread titled: [PATCH] libata atapi work #6 I doubt I can use this patch as is. I believe, because I assembled my drive from components myself, its op xA1 Identify data has not changed, so I will see a misleading 0 False at: #define ata_id_use_dmadir(dev) ((dev)->id[62] & (1 << 15)) I will work to confirm/ deny that theory. Pat LaVarre ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: SATA ATAPI work in progress 2004-05-15 13:06 ` Pat LaVarre @ 2004-05-15 13:49 ` Pat LaVarre 2004-05-15 15:27 ` Jeff Garzik 0 siblings, 1 reply; 34+ messages in thread From: Pat LaVarre @ 2004-05-15 13:49 UTC (permalink / raw) To: Jeff Garzik; +Cc: linux-ide [-- Attachment #1: Type: text/plain, Size: 1373 bytes --] > I doubt I can use this patch as is. I believe, because I assembled my > drive from components myself, its op xA1 Identify data has not changed, > so I will see a misleading 0 False at: > > #define ata_id_use_dmadir(dev) ((dev)->id[62] & (1 << 15)) Theory confirmed! > ... Specifically I saw: kernel: ata_dump_id: 88==0x203f 93==0x404f kernel: Assertion failed! dev->class == ATA_DEV_ATA,drivers/scsi/libata-core.c,ata_dev_parse_strings,line=867 kernel: ata2: dev 0 ATA ata_id_use_dmadir kernel: ata2: dev 0 ATA 'word' 62 = x0000 kernel: ata2: dev 0 ATAPI, max UDMA/100 kernel: ata_dev_identify: EXIT, drv_stat = 0x50 reported by the following experimental pel.patch.7x, applied after patch.[1234567] from Jeff G. Pat LaVarre --- o/drivers/scsi/libata-core.c 2004-05-15 07:31:49.000000000 -0600 +++ linux-2.6.6-bk1/drivers/scsi/libata-core.c 2004-05-15 07:39:24.737046832 -0600 @@ -1168,8 +1168,16 @@ retry: if (ata_id_is_ata(dev)) /* sanity check */ goto err_out_nosup; + printk(KERN_INFO "ata%u: dev %u ATA ata_id_use_dmadir\n", + ap->id, device); if (ata_id_use_dmadir(dev)) dev->flags |= ATA_DFLAG_DMADIR; + else { + dev->flags |= ATA_DFLAG_DMADIR; + printk(KERN_INFO "ata%u: dev %u ATA" + "'word' 62 = x%04X\n", + ap->id, device, (dev)->id[62]); + } /* see if 16-byte commands supported */ tmp = dev->id[0] & 0x3; [-- Attachment #2: pel.patch.7x --] [-- Type: text/x-troff-man, Size: 633 bytes --] --- o/drivers/scsi/libata-core.c 2004-05-15 07:31:49.000000000 -0600 +++ linux-2.6.6-bk1/drivers/scsi/libata-core.c 2004-05-15 07:39:24.737046832 -0600 @@ -1168,8 +1168,16 @@ retry: if (ata_id_is_ata(dev)) /* sanity check */ goto err_out_nosup; + printk(KERN_INFO "ata%u: dev %u ATA ata_id_use_dmadir\n", + ap->id, device); if (ata_id_use_dmadir(dev)) dev->flags |= ATA_DFLAG_DMADIR; + else { + dev->flags |= ATA_DFLAG_DMADIR; + printk(KERN_INFO "ata%u: dev %u ATA" + "'word' 62 = x%04X\n", + ap->id, device, (dev)->id[62]); + } /* see if 16-byte commands supported */ tmp = dev->id[0] & 0x3; ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: SATA ATAPI work in progress 2004-05-15 13:49 ` Pat LaVarre @ 2004-05-15 15:27 ` Jeff Garzik 2004-05-15 15:49 ` Pat LaVarre 0 siblings, 1 reply; 34+ messages in thread From: Jeff Garzik @ 2004-05-15 15:27 UTC (permalink / raw) To: Pat LaVarre; +Cc: linux-ide Pat LaVarre wrote: > Theory confirmed! [...] > Success! Thank you. Does this mean you actually got ATAPI to work? Or just that we made progress? Jeff ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: SATA ATAPI work in progress 2004-05-15 15:27 ` Jeff Garzik @ 2004-05-15 15:49 ` Pat LaVarre 2004-05-15 15:56 ` Pat LaVarre 2004-05-15 16:30 ` Jeff Garzik 0 siblings, 2 replies; 34+ messages in thread From: Pat LaVarre @ 2004-05-15 15:49 UTC (permalink / raw) To: Jeff Garzik; +Cc: linux-ide [-- Attachment #1: Type: text/plain, Size: 13018 bytes --] > > Success! ... > ... > Does this mean you actually got ATAPI to work? I mean to say, I have seen: $ sudo modprobe ata-piix $ sudo modprobe -r ata-piix $ despite having plugged in an Si 3611CT80 1.4 SATA DVD and having turned on libata.h ATA_ENABLE_ATAPI. In particular, my /var/log/messages now include: kernel: ata_scsi_dump_cdb: CDB (2:0,0,0) 12 00 00 00 24 00 00 00 46 kernel: ata_scsi_translate: ENTER kernel: ata_sg_setup_one: mapped buffer of 36 bytes for read kernel: ata_fill_sg: PRD[0] = (0x46EE54, 0x24) kernel: ata_dev_select: ENTER, ata2: device 0, wait 1 kernel: ata_tf_load_pio: feat 0x5 nsect 0x0 lba 0x0 0x0 0x0 kernel: ata_tf_load_pio: device 0xA0 kernel: ata_exec_command_pio: ata2: cmd 0xA0 kernel: ata_scsi_translate: EXIT kernel: atapi_packet_task: busy wait kernel: atapi_packet_task: send cdb kernel: ata_host_intr: BUS_DMA (host_stat 0x24) kernel: ata_dma_complete: ENTER kernel: ata_dma_complete: host 2, host_stat==0x24, drv_stat==0x50 kernel: ata_sg_clean: unmapping 1 sg elements kernel: ata_scsi_dump_cdb: CDB (2:0,0,0) 12 00 00 00 c0 00 00 00 46 kernel: ata_scsi_translate: ENTER kernel: ata_sg_setup_one: mapped buffer of 192 bytes for read kernel: ata_fill_sg: PRD[0] = (0x46EE54, 0xC0) kernel: ata_dev_select: ENTER, ata2: device 0, wait 1 kernel: ata_tf_load_pio: feat 0x5 nsect 0x0 lba 0x0 0x0 0x0 kernel: ata_tf_load_pio: device 0xA0 kernel: ata_exec_command_pio: ata2: cmd 0xA0 kernel: ata_scsi_translate: EXIT kernel: atapi_packet_task: busy wait kernel: atapi_packet_task: send cdb kernel: ata_host_intr: BUS_DMA (host_stat 0x24) kernel: ata_dma_complete: ENTER kernel: ata_dma_complete: host 2, host_stat==0x24, drv_stat==0x50 kernel: ata_sg_clean: unmapping 1 sg elements kernel: Vendor: Iomega Model: RRD Rev: 74.B kernel: Type: CD-ROM ANSI SCSI revision: 00 kernel: ata_scsi_dump_cdb: CDB (2:0,1,0) 12 00 00 00 24 00 00 00 46 > Or just that we made progress? Yes I have three or more new troubles to report, I will reply again. Meanwhile, attached to this e-mail is the last composite patch I tried. This should match your patch [1234567] except then also it has the dmadir revised as Bartlomiej and I discussed in "Re: [PATCH] libata-core when not ata_id_use_dmadir despite yes Silicon Image". Pat LaVarre P.S. (-: Wow. Always an emotionally powerful moment for me when someone tells me I'm not talking enough. :-) diff -Nurp linux-2.6.6-bk2/include/linux/ata.h linux-2.6.6-bk1-pel/include/linux/ata.h --- linux-2.6.6-bk2/include/linux/ata.h 2004-05-15 08:43:05.677175640 -0600 +++ linux-2.6.6-bk1-pel/include/linux/ata.h 2004-05-15 08:41:04.368617344 -0600 @@ -134,6 +134,8 @@ enum { /* ATAPI stuff */ ATAPI_PKT_DMA = (1 << 0), + ATAPI_DMADIR = (1 << 2), /* ATAPI data dir: + 0=to device, 1=to host */ /* cable types */ ATA_CBL_NONE = 0, diff -Nurp linux-2.6.6-bk2/include/linux/libata.h linux-2.6.6-bk1-pel/include/linux/libata.h --- linux-2.6.6-bk2/include/linux/libata.h 2004-05-15 08:43:05.692173360 -0600 +++ linux-2.6.6-bk1-pel/include/linux/libata.h 2004-05-15 08:40:52.197467640 -0600 @@ -335,6 +335,7 @@ struct ata_port_operations { void (*phy_reset) (struct ata_port *ap); void (*post_set_mode) (struct ata_port *ap); + void (*bmdma_setup) (struct ata_queued_cmd *qc); void (*bmdma_start) (struct ata_queued_cmd *qc); void (*fill_sg) (struct ata_queued_cmd *qc); void (*eng_timeout) (struct ata_port *ap); @@ -397,7 +398,9 @@ extern int ata_port_start (struct ata_po extern void ata_port_stop (struct ata_port *ap); extern irqreturn_t ata_interrupt (int irq, void *dev_instance, struct pt_regs *regs); extern void ata_fill_sg(struct ata_queued_cmd *qc); +extern void ata_bmdma_setup_mmio (struct ata_queued_cmd *qc); extern void ata_bmdma_start_mmio (struct ata_queued_cmd *qc); +extern void ata_bmdma_setup_pio (struct ata_queued_cmd *qc); extern void ata_bmdma_start_pio (struct ata_queued_cmd *qc); extern int pci_test_config_bits(struct pci_dev *pdev, struct pci_bits *bits); extern void ata_qc_complete(struct ata_queued_cmd *qc, u8 drv_stat, unsigned int done_late); @@ -473,6 +481,13 @@ static inline u8 ata_wait_idle(struct at return status; } +static inline void ata_qc_set_polling(struct ata_queued_cmd *qc) +{ + qc->flags |= ATA_QCFLAG_POLL; + qc->flags &= ~ATA_QCFLAG_DMA; + qc->tf.ctl |= ATA_NIEN; +} + static inline struct ata_queued_cmd *ata_qc_from_tag (struct ata_port *ap, unsigned int tag) { diff -Nurp linux-2.6.6-bk2/drivers/scsi/libata-core.c linux-2.6.6-bk1-pel/drivers/scsi/libata-core.c --- linux-2.6.6-bk2/drivers/scsi/libata-core.c 2004-05-15 08:43:03.928441488 -0600 +++ linux-2.6.6-bk1-pel/drivers/scsi/libata-core.c 2004-05-15 08:40:40.017319304 -0600 @@ -2396,16 +2396,18 @@ int ata_qc_issue(struct ata_queued_cmd * struct ata_port *ap = qc->ap; struct scsi_cmnd *cmd = qc->scsicmd; - /* set up SG table */ - if (cmd->use_sg) { - if (ata_sg_setup(qc)) - goto err_out; - } else { - if (ata_sg_setup_one(qc)) - goto err_out; - } + if (qc->flags & ATA_QCFLAG_SG) { + /* set up SG table */ + if (cmd->use_sg) { + if (ata_sg_setup(qc)) + goto err_out; + } else { + if (ata_sg_setup_one(qc)) + goto err_out; + } - ap->ops->fill_sg(qc); + ap->ops->fill_sg(qc); + } qc->ap->active_tag = qc->tag; qc->flags |= ATA_QCFLAG_ACTIVE; @@ -2445,17 +2447,28 @@ static int ata_qc_issue_prot(struct ata_ case ATA_PROT_DMA: ap->ops->tf_load(ap, &qc->tf); /* load tf registers */ + ap->ops->bmdma_setup(qc); /* set up bmdma */ ap->ops->bmdma_start(qc); /* initiate bmdma */ break; case ATA_PROT_PIO: /* load tf registers, initiate polling pio */ - qc->flags |= ATA_QCFLAG_POLL; - qc->tf.ctl |= ATA_NIEN; /* disable interrupts */ + ata_qc_set_polling(qc); ata_tf_to_host_nolock(ap, &qc->tf); ap->pio_task_state = PIO_ST; queue_work(ata_wq, &ap->pio_task); break; + case ATA_PROT_ATAPI: + ata_tf_to_host_nolock(ap, &qc->tf); + queue_work(ata_wq, &ap->packet_task); + break; + + case ATA_PROT_ATAPI_DMA: + ap->ops->tf_load(ap, &qc->tf); /* load tf registers */ + ap->ops->bmdma_setup(qc); /* set up bmdma */ + queue_work(ata_wq, &ap->packet_task); + break; + default: WARN_ON(1); return -1; @@ -2465,14 +2478,14 @@ static int ata_qc_issue_prot(struct ata_ } /** - * ata_bmdma_start_mmio - - * @qc: + * ata_bmdma_setup_mmio - Set up PCI IDE BMDMA transaction (MMIO) + * @qc: Info associated with this ATA transaction. * * LOCKING: * spin_lock_irqsave(host_set lock) */ -void ata_bmdma_start_mmio (struct ata_queued_cmd *qc) +void ata_bmdma_setup_mmio (struct ata_queued_cmd *qc) { struct ata_port *ap = qc->ap; unsigned int rw = (qc->tf.flags & ATA_TFLAG_WRITE); @@ -2496,8 +2509,24 @@ void ata_bmdma_start_mmio (struct ata_qu /* issue r/w command */ ap->ops->exec_command(ap, &qc->tf); +} + +/** + * ata_bmdma_start_mmio - Start a PCI IDE BMDMA transaction (MMIO) + * @qc: Info associated with this ATA transaction. + * + * LOCKING: + * spin_lock_irqsave(host_set lock) + */ + +void ata_bmdma_start_mmio (struct ata_queued_cmd *qc) +{ + struct ata_port *ap = qc->ap; + void *mmio = (void *) ap->ioaddr.bmdma_addr; + u8 dmactl; /* start host DMA transaction */ + dmactl = readb(mmio + ATA_DMA_CMD); writeb(dmactl | ATA_DMA_START, mmio + ATA_DMA_CMD); /* Strictly, one may wish to issue a readb() here, to @@ -2514,14 +2543,14 @@ void ata_bmdma_start_mmio (struct ata_qu } /** - * ata_bmdma_start_pio - - * @qc: + * ata_bmdma_setup_pio - Set up PCI IDE BMDMA transaction (PIO) + * @qc: Info associated with this ATA transaction. * * LOCKING: * spin_lock_irqsave(host_set lock) */ -void ata_bmdma_start_pio (struct ata_queued_cmd *qc) +void ata_bmdma_setup_pio (struct ata_queued_cmd *qc) { struct ata_port *ap = qc->ap; unsigned int rw = (qc->tf.flags & ATA_TFLAG_WRITE); @@ -2544,8 +2573,23 @@ void ata_bmdma_start_pio (struct ata_que /* issue r/w command */ ap->ops->exec_command(ap, &qc->tf); +} + +/** + * ata_bmdma_start_pio - Start a PCI IDE BMDMA transaction (PIO) + * @qc: Info associated with this ATA transaction. + * + * LOCKING: + * spin_lock_irqsave(host_set lock) + */ + +void ata_bmdma_start_pio (struct ata_queued_cmd *qc) +{ + struct ata_port *ap = qc->ap; + u8 dmactl; /* start host DMA transaction */ + dmactl = inb(ap->ioaddr.bmdma_addr + ATA_DMA_CMD); outb(dmactl | ATA_DMA_START, ap->ioaddr.bmdma_addr + ATA_DMA_CMD); } @@ -2620,6 +2664,7 @@ inline unsigned int ata_host_intr (struc switch (qc->tf.protocol) { case ATA_PROT_DMA: + case ATA_PROT_ATAPI_DMA: if (ap->flags & ATA_FLAG_MMIO) { void *mmio = (void *) ap->ioaddr.bmdma_addr; host_stat = readb(mmio + ATA_DMA_STATUS); @@ -2755,20 +2800,6 @@ static unsigned long ata_thread_iter(str return timeout; } -void atapi_start(struct ata_queued_cmd *qc) -{ - struct ata_port *ap = qc->ap; - - qc->flags |= ATA_QCFLAG_ACTIVE; - ap->active_tag = qc->tag; - - ata_dev_select(ap, qc->dev->devno, 1, 0); - ata_tf_to_host_nolock(ap, &qc->tf); - queue_work(ata_wq, &ap->packet_task); - - VPRINTK("EXIT\n"); -} - /** * atapi_packet_task - Write CDB bytes to hardware * @_data: Port to which ATAPI device is attached. @@ -2811,7 +2842,7 @@ static void atapi_packet_task(void *_dat /* if we are DMA'ing, irq handler takes over from here */ if (qc->tf.protocol == ATA_PROT_ATAPI_DMA) { - /* FIXME: start DMA here */ + ap->ops->bmdma_start(qc); /* initiate bmdma */ } else { ap->pio_task_state = PIO_ST; queue_work(ata_wq, &ap->pio_task); @@ -3475,7 +3506,9 @@ EXPORT_SYMBOL_GPL(ata_port_start); EXPORT_SYMBOL_GPL(ata_port_stop); EXPORT_SYMBOL_GPL(ata_interrupt); EXPORT_SYMBOL_GPL(ata_fill_sg); +EXPORT_SYMBOL_GPL(ata_bmdma_setup_pio); EXPORT_SYMBOL_GPL(ata_bmdma_start_pio); +EXPORT_SYMBOL_GPL(ata_bmdma_setup_mmio); EXPORT_SYMBOL_GPL(ata_bmdma_start_mmio); EXPORT_SYMBOL_GPL(ata_port_probe); EXPORT_SYMBOL_GPL(sata_phy_reset); diff -Nurp linux-2.6.6-bk2/drivers/scsi/libata-scsi.c linux-2.6.6-bk1-pel/drivers/scsi/libata-scsi.c --- linux-2.6.6-bk2/drivers/scsi/libata-scsi.c 2004-05-15 08:43:03.930441184 -0600 +++ linux-2.6.6-bk1-pel/drivers/scsi/libata-scsi.c 2004-05-15 08:40:25.263562216 -0600 @@ -885,53 +885,20 @@ void ata_scsi_badcmd(struct scsi_cmnd *c } /** - * atapi_scsi_queuecmd - Send CDB to ATAPI device - * @ap: Port to which ATAPI device is attached. - * @dev: Target device for CDB. - * @cmd: SCSI command being sent to device. - * @done: SCSI command completion function. - * - * Sends CDB to ATAPI device. If the Linux SCSI layer sends a - * non-data command, then this function handles the command - * directly, via polling. Otherwise, the bmdma engine is started. + * atapi_xlat - Initialize PACKET taskfile + * @qc: command structure to be initialized + * @scsicmd: SCSI CDB associated with this PACKET command * * LOCKING: * spin_lock_irqsave(host_set lock) + * + * RETURNS: + * Zero on success, non-zero on failure. */ -static void atapi_scsi_queuecmd(struct ata_port *ap, struct ata_device *dev, - struct scsi_cmnd *cmd, void (*done)(struct scsi_cmnd *)) +static unsigned int atapi_xlat(struct ata_queued_cmd *qc, u8 *scsicmd) { - struct ata_queued_cmd *qc; - u8 *scsicmd = cmd->cmnd; - - VPRINTK("ENTER, drv_stat = 0x%x\n", ata_chk_status(ap)); - - if (cmd->sc_data_direction == SCSI_DATA_UNKNOWN) { - DPRINTK("unknown data, scsicmd 0x%x\n", scsicmd[0]); - ata_bad_cdb(cmd, done); - return; - } - - switch(scsicmd[0]) { - case READ_6: - case WRITE_6: - case MODE_SELECT: - case MODE_SENSE: - DPRINTK("read6/write6/modesel/modesense trap\n"); - ata_bad_scsiop(cmd, done); - return; - - default: - /* do nothing */ - break; - } - - qc = ata_scsi_qc_new(ap, dev, cmd, done); - if (!qc) { - printk(KERN_ERR "ata%u: command queue empty\n", ap->id); - return; - } + struct scsi_cmnd *cmd = qc->scsicmd; qc->flags |= ATA_QCFLAG_ATAPI; @@ -943,17 +910,21 @@ static void atapi_scsi_queuecmd(struct a qc->tf.command = ATA_CMD_PACKET; - if (cmd->sc_data_direction == SCSI_DATA_NONE) { + if ((cmd->sc_data_direction == SCSI_DATA_NONE) || + ((qc->flags & ATA_QCFLAG_DMA) == 0)) { + ata_qc_set_polling(qc); qc->tf.protocol = ATA_PROT_ATAPI; - qc->flags |= ATA_QCFLAG_POLL; - qc->tf.ctl |= ATA_NIEN; /* disable interrupts */ + qc->tf.lbam = (8 * 1024) & 0xff; + qc->tf.lbah = (8 * 1024) >> 8; } else { - qc->tf.protocol = ATA_PROT_ATAPI_DMA; qc->flags |= ATA_QCFLAG_SG; /* data is present; dma-map it */ + qc->tf.protocol = ATA_PROT_ATAPI_DMA; qc->tf.feature |= ATAPI_PKT_DMA; + if (cmd->sc_data_direction != SCSI_DATA_WRITE) + qc->tf.feature |= ATAPI_DMADIR; } - atapi_start(qc); + return 0; } /** @@ -1092,7 +1063,7 @@ int ata_scsi_queuecmd(struct scsi_cmnd * else ata_scsi_simulate(ap, dev, cmd, done); } else - atapi_scsi_queuecmd(ap, dev, cmd, done); + ata_scsi_translate(ap, dev, cmd, done, atapi_xlat); out_unlock: return 0; [-- Attachment #2: pel.patch.1234567 --] [-- Type: text/x-patch, Size: 10592 bytes --] diff -Nurp linux-2.6.6-bk2/include/linux/ata.h linux-2.6.6-bk1-pel/include/linux/ata.h --- linux-2.6.6-bk2/include/linux/ata.h 2004-05-15 08:43:05.677175640 -0600 +++ linux-2.6.6-bk1-pel/include/linux/ata.h 2004-05-15 08:41:04.368617344 -0600 @@ -134,6 +134,8 @@ enum { /* ATAPI stuff */ ATAPI_PKT_DMA = (1 << 0), + ATAPI_DMADIR = (1 << 2), /* ATAPI data dir: + 0=to device, 1=to host */ /* cable types */ ATA_CBL_NONE = 0, diff -Nurp linux-2.6.6-bk2/include/linux/libata.h linux-2.6.6-bk1-pel/include/linux/libata.h --- linux-2.6.6-bk2/include/linux/libata.h 2004-05-15 08:43:05.692173360 -0600 +++ linux-2.6.6-bk1-pel/include/linux/libata.h 2004-05-15 08:40:52.197467640 -0600 @@ -335,6 +335,7 @@ struct ata_port_operations { void (*phy_reset) (struct ata_port *ap); void (*post_set_mode) (struct ata_port *ap); + void (*bmdma_setup) (struct ata_queued_cmd *qc); void (*bmdma_start) (struct ata_queued_cmd *qc); void (*fill_sg) (struct ata_queued_cmd *qc); void (*eng_timeout) (struct ata_port *ap); @@ -397,7 +398,9 @@ extern int ata_port_start (struct ata_po extern void ata_port_stop (struct ata_port *ap); extern irqreturn_t ata_interrupt (int irq, void *dev_instance, struct pt_regs *regs); extern void ata_fill_sg(struct ata_queued_cmd *qc); +extern void ata_bmdma_setup_mmio (struct ata_queued_cmd *qc); extern void ata_bmdma_start_mmio (struct ata_queued_cmd *qc); +extern void ata_bmdma_setup_pio (struct ata_queued_cmd *qc); extern void ata_bmdma_start_pio (struct ata_queued_cmd *qc); extern int pci_test_config_bits(struct pci_dev *pdev, struct pci_bits *bits); extern void ata_qc_complete(struct ata_queued_cmd *qc, u8 drv_stat, unsigned int done_late); @@ -473,6 +481,13 @@ static inline u8 ata_wait_idle(struct at return status; } +static inline void ata_qc_set_polling(struct ata_queued_cmd *qc) +{ + qc->flags |= ATA_QCFLAG_POLL; + qc->flags &= ~ATA_QCFLAG_DMA; + qc->tf.ctl |= ATA_NIEN; +} + static inline struct ata_queued_cmd *ata_qc_from_tag (struct ata_port *ap, unsigned int tag) { diff -Nurp linux-2.6.6-bk2/drivers/scsi/libata-core.c linux-2.6.6-bk1-pel/drivers/scsi/libata-core.c --- linux-2.6.6-bk2/drivers/scsi/libata-core.c 2004-05-15 08:43:03.928441488 -0600 +++ linux-2.6.6-bk1-pel/drivers/scsi/libata-core.c 2004-05-15 08:40:40.017319304 -0600 @@ -2396,16 +2396,18 @@ int ata_qc_issue(struct ata_queued_cmd * struct ata_port *ap = qc->ap; struct scsi_cmnd *cmd = qc->scsicmd; - /* set up SG table */ - if (cmd->use_sg) { - if (ata_sg_setup(qc)) - goto err_out; - } else { - if (ata_sg_setup_one(qc)) - goto err_out; - } + if (qc->flags & ATA_QCFLAG_SG) { + /* set up SG table */ + if (cmd->use_sg) { + if (ata_sg_setup(qc)) + goto err_out; + } else { + if (ata_sg_setup_one(qc)) + goto err_out; + } - ap->ops->fill_sg(qc); + ap->ops->fill_sg(qc); + } qc->ap->active_tag = qc->tag; qc->flags |= ATA_QCFLAG_ACTIVE; @@ -2445,17 +2447,28 @@ static int ata_qc_issue_prot(struct ata_ case ATA_PROT_DMA: ap->ops->tf_load(ap, &qc->tf); /* load tf registers */ + ap->ops->bmdma_setup(qc); /* set up bmdma */ ap->ops->bmdma_start(qc); /* initiate bmdma */ break; case ATA_PROT_PIO: /* load tf registers, initiate polling pio */ - qc->flags |= ATA_QCFLAG_POLL; - qc->tf.ctl |= ATA_NIEN; /* disable interrupts */ + ata_qc_set_polling(qc); ata_tf_to_host_nolock(ap, &qc->tf); ap->pio_task_state = PIO_ST; queue_work(ata_wq, &ap->pio_task); break; + case ATA_PROT_ATAPI: + ata_tf_to_host_nolock(ap, &qc->tf); + queue_work(ata_wq, &ap->packet_task); + break; + + case ATA_PROT_ATAPI_DMA: + ap->ops->tf_load(ap, &qc->tf); /* load tf registers */ + ap->ops->bmdma_setup(qc); /* set up bmdma */ + queue_work(ata_wq, &ap->packet_task); + break; + default: WARN_ON(1); return -1; @@ -2465,14 +2478,14 @@ static int ata_qc_issue_prot(struct ata_ } /** - * ata_bmdma_start_mmio - - * @qc: + * ata_bmdma_setup_mmio - Set up PCI IDE BMDMA transaction (MMIO) + * @qc: Info associated with this ATA transaction. * * LOCKING: * spin_lock_irqsave(host_set lock) */ -void ata_bmdma_start_mmio (struct ata_queued_cmd *qc) +void ata_bmdma_setup_mmio (struct ata_queued_cmd *qc) { struct ata_port *ap = qc->ap; unsigned int rw = (qc->tf.flags & ATA_TFLAG_WRITE); @@ -2496,8 +2509,24 @@ void ata_bmdma_start_mmio (struct ata_qu /* issue r/w command */ ap->ops->exec_command(ap, &qc->tf); +} + +/** + * ata_bmdma_start_mmio - Start a PCI IDE BMDMA transaction (MMIO) + * @qc: Info associated with this ATA transaction. + * + * LOCKING: + * spin_lock_irqsave(host_set lock) + */ + +void ata_bmdma_start_mmio (struct ata_queued_cmd *qc) +{ + struct ata_port *ap = qc->ap; + void *mmio = (void *) ap->ioaddr.bmdma_addr; + u8 dmactl; /* start host DMA transaction */ + dmactl = readb(mmio + ATA_DMA_CMD); writeb(dmactl | ATA_DMA_START, mmio + ATA_DMA_CMD); /* Strictly, one may wish to issue a readb() here, to @@ -2514,14 +2543,14 @@ void ata_bmdma_start_mmio (struct ata_qu } /** - * ata_bmdma_start_pio - - * @qc: + * ata_bmdma_setup_pio - Set up PCI IDE BMDMA transaction (PIO) + * @qc: Info associated with this ATA transaction. * * LOCKING: * spin_lock_irqsave(host_set lock) */ -void ata_bmdma_start_pio (struct ata_queued_cmd *qc) +void ata_bmdma_setup_pio (struct ata_queued_cmd *qc) { struct ata_port *ap = qc->ap; unsigned int rw = (qc->tf.flags & ATA_TFLAG_WRITE); @@ -2544,8 +2573,23 @@ void ata_bmdma_start_pio (struct ata_que /* issue r/w command */ ap->ops->exec_command(ap, &qc->tf); +} + +/** + * ata_bmdma_start_pio - Start a PCI IDE BMDMA transaction (PIO) + * @qc: Info associated with this ATA transaction. + * + * LOCKING: + * spin_lock_irqsave(host_set lock) + */ + +void ata_bmdma_start_pio (struct ata_queued_cmd *qc) +{ + struct ata_port *ap = qc->ap; + u8 dmactl; /* start host DMA transaction */ + dmactl = inb(ap->ioaddr.bmdma_addr + ATA_DMA_CMD); outb(dmactl | ATA_DMA_START, ap->ioaddr.bmdma_addr + ATA_DMA_CMD); } @@ -2620,6 +2664,7 @@ inline unsigned int ata_host_intr (struc switch (qc->tf.protocol) { case ATA_PROT_DMA: + case ATA_PROT_ATAPI_DMA: if (ap->flags & ATA_FLAG_MMIO) { void *mmio = (void *) ap->ioaddr.bmdma_addr; host_stat = readb(mmio + ATA_DMA_STATUS); @@ -2755,20 +2800,6 @@ static unsigned long ata_thread_iter(str return timeout; } -void atapi_start(struct ata_queued_cmd *qc) -{ - struct ata_port *ap = qc->ap; - - qc->flags |= ATA_QCFLAG_ACTIVE; - ap->active_tag = qc->tag; - - ata_dev_select(ap, qc->dev->devno, 1, 0); - ata_tf_to_host_nolock(ap, &qc->tf); - queue_work(ata_wq, &ap->packet_task); - - VPRINTK("EXIT\n"); -} - /** * atapi_packet_task - Write CDB bytes to hardware * @_data: Port to which ATAPI device is attached. @@ -2811,7 +2842,7 @@ static void atapi_packet_task(void *_dat /* if we are DMA'ing, irq handler takes over from here */ if (qc->tf.protocol == ATA_PROT_ATAPI_DMA) { - /* FIXME: start DMA here */ + ap->ops->bmdma_start(qc); /* initiate bmdma */ } else { ap->pio_task_state = PIO_ST; queue_work(ata_wq, &ap->pio_task); @@ -3475,7 +3506,9 @@ EXPORT_SYMBOL_GPL(ata_port_start); EXPORT_SYMBOL_GPL(ata_port_stop); EXPORT_SYMBOL_GPL(ata_interrupt); EXPORT_SYMBOL_GPL(ata_fill_sg); +EXPORT_SYMBOL_GPL(ata_bmdma_setup_pio); EXPORT_SYMBOL_GPL(ata_bmdma_start_pio); +EXPORT_SYMBOL_GPL(ata_bmdma_setup_mmio); EXPORT_SYMBOL_GPL(ata_bmdma_start_mmio); EXPORT_SYMBOL_GPL(ata_port_probe); EXPORT_SYMBOL_GPL(sata_phy_reset); diff -Nurp linux-2.6.6-bk2/drivers/scsi/libata-scsi.c linux-2.6.6-bk1-pel/drivers/scsi/libata-scsi.c --- linux-2.6.6-bk2/drivers/scsi/libata-scsi.c 2004-05-15 08:43:03.930441184 -0600 +++ linux-2.6.6-bk1-pel/drivers/scsi/libata-scsi.c 2004-05-15 08:40:25.263562216 -0600 @@ -885,53 +885,20 @@ void ata_scsi_badcmd(struct scsi_cmnd *c } /** - * atapi_scsi_queuecmd - Send CDB to ATAPI device - * @ap: Port to which ATAPI device is attached. - * @dev: Target device for CDB. - * @cmd: SCSI command being sent to device. - * @done: SCSI command completion function. - * - * Sends CDB to ATAPI device. If the Linux SCSI layer sends a - * non-data command, then this function handles the command - * directly, via polling. Otherwise, the bmdma engine is started. + * atapi_xlat - Initialize PACKET taskfile + * @qc: command structure to be initialized + * @scsicmd: SCSI CDB associated with this PACKET command * * LOCKING: * spin_lock_irqsave(host_set lock) + * + * RETURNS: + * Zero on success, non-zero on failure. */ -static void atapi_scsi_queuecmd(struct ata_port *ap, struct ata_device *dev, - struct scsi_cmnd *cmd, void (*done)(struct scsi_cmnd *)) +static unsigned int atapi_xlat(struct ata_queued_cmd *qc, u8 *scsicmd) { - struct ata_queued_cmd *qc; - u8 *scsicmd = cmd->cmnd; - - VPRINTK("ENTER, drv_stat = 0x%x\n", ata_chk_status(ap)); - - if (cmd->sc_data_direction == SCSI_DATA_UNKNOWN) { - DPRINTK("unknown data, scsicmd 0x%x\n", scsicmd[0]); - ata_bad_cdb(cmd, done); - return; - } - - switch(scsicmd[0]) { - case READ_6: - case WRITE_6: - case MODE_SELECT: - case MODE_SENSE: - DPRINTK("read6/write6/modesel/modesense trap\n"); - ata_bad_scsiop(cmd, done); - return; - - default: - /* do nothing */ - break; - } - - qc = ata_scsi_qc_new(ap, dev, cmd, done); - if (!qc) { - printk(KERN_ERR "ata%u: command queue empty\n", ap->id); - return; - } + struct scsi_cmnd *cmd = qc->scsicmd; qc->flags |= ATA_QCFLAG_ATAPI; @@ -943,17 +910,21 @@ static void atapi_scsi_queuecmd(struct a qc->tf.command = ATA_CMD_PACKET; - if (cmd->sc_data_direction == SCSI_DATA_NONE) { + if ((cmd->sc_data_direction == SCSI_DATA_NONE) || + ((qc->flags & ATA_QCFLAG_DMA) == 0)) { + ata_qc_set_polling(qc); qc->tf.protocol = ATA_PROT_ATAPI; - qc->flags |= ATA_QCFLAG_POLL; - qc->tf.ctl |= ATA_NIEN; /* disable interrupts */ + qc->tf.lbam = (8 * 1024) & 0xff; + qc->tf.lbah = (8 * 1024) >> 8; } else { - qc->tf.protocol = ATA_PROT_ATAPI_DMA; qc->flags |= ATA_QCFLAG_SG; /* data is present; dma-map it */ + qc->tf.protocol = ATA_PROT_ATAPI_DMA; qc->tf.feature |= ATAPI_PKT_DMA; + if (cmd->sc_data_direction != SCSI_DATA_WRITE) + qc->tf.feature |= ATAPI_DMADIR; } - atapi_start(qc); + return 0; } /** @@ -1092,7 +1063,7 @@ int ata_scsi_queuecmd(struct scsi_cmnd * else ata_scsi_simulate(ap, dev, cmd, done); } else - atapi_scsi_queuecmd(ap, dev, cmd, done); + ata_scsi_translate(ap, dev, cmd, done, atapi_xlat); out_unlock: return 0; ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: SATA ATAPI work in progress 2004-05-15 15:49 ` Pat LaVarre @ 2004-05-15 15:56 ` Pat LaVarre 2004-05-15 16:04 ` Pat LaVarre 2004-05-15 16:30 ` Jeff Garzik 1 sibling, 1 reply; 34+ messages in thread From: Pat LaVarre @ 2004-05-15 15:56 UTC (permalink / raw) To: Jeff Garzik; +Cc: linux-ide > > Or just that we made progress? > > Yes I have three or more new troubles to report ... Personally I vote we chase three troubles in three threads, not all three here, but I won't act that way unless someone seconds my motion. In short I am now reporting SATA ATAPI UDMA trouble in: 1) linux-2.6.6-bk2 2) open O_NONBLOCK ioctl SG_IO 3) /proc/ide At length: 1) linux-2.6.6-bk2 I tried substituting linux-2.6.6-bk2 for -bk1. My -bk2 crashed in the second dump_stack past a *NULL of ata_qc_issue_prot. Seemingly the same crash, both times I tried it. My -bk1 and -bk2 diff's vs. kernel.org are byte-for-byte identical when I compare: include/linux/ata.h include/linux/libata.h drivers/scsi/libata-core.c drivers/scsi/libata-scsi.c My .config differ by only adding new # comments. 2) open O_NONBLOCK ioctl SG_IO I tried `plscsi /dev/scd0` i.e. an open O_NONBLOCK/ ioctl SG_IO/ close of a standard op x12 Inquiry for up to x24 bytes. That died in a way that defeated Ctrl+C and `modprobe -r ata-piix`, though `reboot` worked. I'll try ioctl CDROM_SEND_PACKET next, then think harder. Meanwhile, seemingly relevant /var/log/messages include: kernel: ata_scsi_dump_cdb: CDB (2:0,0,0) 00 00 00 00 00 00 00 00 00 kernel: ata_scsi_translate: ENTER kernel: ata2(0): empty request buffer kernel: ata_scsi_badcmd: ENTER kernel: ata_scsi_translate: EXIT - badcmd kernel: ata_scsi_dump_cdb: CDB (2:0,0,0) 00 00 00 00 00 00 00 00 00 3) /proc/ide /proc/ide, perhaps by design, does not show the device (and thus neglects to publish its op xA1 Identify data). Pat LaVarre ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: SATA ATAPI work in progress 2004-05-15 15:56 ` Pat LaVarre @ 2004-05-15 16:04 ` Pat LaVarre 2004-05-15 16:32 ` Jeff Garzik 0 siblings, 1 reply; 34+ messages in thread From: Pat LaVarre @ 2004-05-15 16:04 UTC (permalink / raw) To: Jeff Garzik; +Cc: linux-ide > 2) open O_NONBLOCK ioctl SG_IO > > I tried `plscsi /dev/scd0` i.e. an open O_NONBLOCK/ ioctl SG_IO/ close > of a standard op x12 Inquiry for up to x24 bytes. > > That died in a way that defeated Ctrl+C and `modprobe -r ata-piix`, > though `reboot` worked. I'll try ioctl CDROM_SEND_PACKET next, then > think harder. ioctl CDROM_SEND_PACKET via sf net iomrrdtools gives me same death as ioctl SG_IO, so I blame the open of /dev/scd$n til I hear otherwise. Death occurs as follows. Next I will grep source for "empty request buffer". Pat LaVarre kernel: ata_scsi_dump_cdb: CDB (2:0,0,0) 00 00 00 00 00 00 00 00 00 kernel: ata_scsi_translate: ENTER kernel: ata2(0): empty request buffer kernel: ata_scsi_badcmd: ENTER kernel: ata_scsi_translate: EXIT - badcmd kernel: ata_scsi_dump_cdb: CDB (2:0,0,0) 00 00 00 00 00 00 00 00 00 kernel: ata_scsi_translate: ENTER kernel: ata2(0): empty request buffer kernel: ata_scsi_badcmd: ENTER kernel: ata_scsi_translate: EXIT - badcmd kernel: ata_scsi_dump_cdb: CDB (2:0,0,0) 00 00 00 00 00 00 00 00 00 kernel: ata_scsi_translate: ENTER kernel: ata2(0): empty request buffer kernel: ata_scsi_badcmd: ENTER kernel: ata_scsi_translate: EXIT - badcmd kernel: ata_scsi_dump_cdb: CDB (2:0,0,0) 00 00 00 00 00 00 00 00 00 kernel: ata_scsi_translate: ENTER kernel: ata2(0): empty request buffer kernel: ata_scsi_badcmd: ENTER kernel: ata_scsi_translate: EXIT - badcmd kernel: ata_scsi_dump_cdb: CDB (2:0,0,0) 00 00 00 00 00 00 00 00 00 kernel: ata_scsi_translate: ENTER kernel: ata2(0): empty request buffer kernel: ata_scsi_badcmd: ENTER kernel: ata_scsi_translate: EXIT - badcmd kernel: ata_scsi_dump_cdb: CDB (2:0,0,0) 5a 00 2a 00 00 00 00 00 80 kernel: ata_scsi_translate: ENTER kernel: ata_sg_setup_one: mapped buffer of 128 bytes for read kernel: ata_fill_sg: PRD[0] = (0x46EE00, 0x80) kernel: ata_dev_select: ENTER, ata2: device 0, wait 1 kernel: ata_tf_load_pio: feat 0x5 nsect 0x0 lba 0x0 0x0 0x0 kernel: ata_tf_load_pio: device 0xA0 kernel: ata_exec_command_pio: ata2: cmd 0xA0 kernel: ata_scsi_translate: EXIT kernel: atapi_packet_task: busy wait kernel: atapi_packet_task: send cdb kernel: ata_host_intr: BUS_DMA (host_stat 0x25) kernel: ata_dma_complete: ENTER kernel: ata_dma_complete: host 2, host_stat==0x25, drv_stat==0x51 kernel: ata_sg_clean: unmapping 1 sg elements kernel: ata_scsi_error: ENTER kernel: ata_eng_timeout: ENTER kernel: ata2: BUG: timeout without command kernel: ata_eng_timeout: EXIT kernel: ata_scsi_error: EXIT ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: SATA ATAPI work in progress 2004-05-15 16:04 ` Pat LaVarre @ 2004-05-15 16:32 ` Jeff Garzik 2004-05-15 16:43 ` Pat LaVarre 0 siblings, 1 reply; 34+ messages in thread From: Jeff Garzik @ 2004-05-15 16:32 UTC (permalink / raw) To: Pat LaVarre; +Cc: linux-ide Pat LaVarre wrote: > kernel: ata_scsi_dump_cdb: CDB (2:0,0,0) 00 00 00 00 00 00 00 00 00 > kernel: ata_scsi_translate: ENTER > kernel: ata2(0): empty request buffer > kernel: ata_scsi_badcmd: ENTER > kernel: ata_scsi_translate: EXIT - badcmd TEST UNIT READY. We probably just delete the following check, which appears to be bogus now. I can't remember why I put this in here... if (unlikely(cmd->request_bufflen < 1)) { printk(KERN_WARNING "ata%u(%u): empty request buffer\n", ap->id, dev->devno); goto err_out; } > kernel: ata_scsi_dump_cdb: CDB (2:0,0,0) 5a 00 2a 00 00 00 00 00 80 > kernel: ata_scsi_translate: ENTER > kernel: ata_sg_setup_one: mapped buffer of 128 bytes for read > kernel: ata_fill_sg: PRD[0] = (0x46EE00, 0x80) > kernel: ata_dev_select: ENTER, ata2: device 0, wait 1 > kernel: ata_tf_load_pio: feat 0x5 nsect 0x0 lba 0x0 0x0 0x0 > kernel: ata_tf_load_pio: device 0xA0 > kernel: ata_exec_command_pio: ata2: cmd 0xA0 > kernel: ata_scsi_translate: EXIT > kernel: atapi_packet_task: busy wait > kernel: atapi_packet_task: send cdb > kernel: ata_host_intr: BUS_DMA (host_stat 0x25) > kernel: ata_dma_complete: ENTER > kernel: ata_dma_complete: host 2, host_stat==0x25, drv_stat==0x51 MODE SENSE 10. We see from drv_stat==0x51 that the device is signalling 'CHECK CONDITION', so we need to issue a REQUEST SENSE at this point. Seems like we are pretty close. Once I add the code to issue a REQUEST SENSE, I bet things start working. Jeff ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: SATA ATAPI work in progress 2004-05-15 16:32 ` Jeff Garzik @ 2004-05-15 16:43 ` Pat LaVarre 2004-05-15 16:50 ` Pat LaVarre 0 siblings, 1 reply; 34+ messages in thread From: Pat LaVarre @ 2004-05-15 16:43 UTC (permalink / raw) To: Jeff Garzik; +Cc: linux-ide > > kernel: ata_scsi_dump_cdb: CDB (2:0,0,0) 00 00 00 00 00 00 00 00 00 > > kernel: ata_scsi_translate: ENTER > > kernel: ata2(0): empty request buffer > > kernel: ata_scsi_badcmd: ENTER > > kernel: ata_scsi_translate: EXIT - badcmd > > TEST UNIT READY. We probably just delete the following check, which > appears to be bogus now. I can't remember why I put this in here... > > if (unlikely(cmd->request_bufflen < 1)) { > printk(KERN_WARNING "ata%u(%u): empty request buffer\n", > ap->id, dev->devno); > goto err_out; > } Independently I tried omitting that test. With that test gone, my kernel does not crash, but still the open (and thereafter modprobe -r) does not complete: kernel: ata_scsi_dump_cdb: CDB (2:0,0,0) 00 00 00 00 00 00 00 00 00 kernel: ata_scsi_translate: ENTER kernel: ata_dev_select: ENTER, ata2: device 0, wait 1 kernel: ata_tf_load_pio: feat 0x0 nsect 0x0 lba 0x0 0x0 0x20 kernel: ata_tf_load_pio: device 0xA0 kernel: ata_exec_command_pio: ata2: cmd 0xA0 kernel: ata_scsi_translate: EXIT kernel: atapi_packet_task: busy wait kernel: atapi_packet_task: send cdb kernel: ata_scsi_error: ENTER kernel: ata_eng_timeout: ENTER kernel: ata2: unknown timeout, cmd 0xa0 stat 0x51 kernel: ata_eng_timeout: EXIT kernel: ata_scsi_error: EXIT kernel: ata_scsi_dump_cdb: CDB (2:0,0,0) 00 00 00 00 00 00 00 00 00 kernel: ata_scsi_translate: ENTER kernel: ata_dev_select: ENTER, ata2: device 0, wait 1 kernel: ata_tf_load_pio: feat 0x0 nsect 0x0 lba 0x0 0x0 0x20 kernel: ata_tf_load_pio: device 0xA0 kernel: ata_exec_command_pio: ata2: cmd 0xA0 kernel: ata_scsi_translate: EXIT kernel: atapi_packet_task: busy wait kernel: atapi_packet_task: send cdb In my ignorance, I'm now trying leaving that test commented out but then also trying to teach atapi_xlat to cope if: ((cmd->sc_data_direction != SCSI_DATA_NONE) && (cmd->request_bufflen < 1)) Soon I will give up for this day, sorry to say, real life intrudes. Pat LaVarre ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: SATA ATAPI work in progress 2004-05-15 16:43 ` Pat LaVarre @ 2004-05-15 16:50 ` Pat LaVarre 0 siblings, 0 replies; 34+ messages in thread From: Pat LaVarre @ 2004-05-15 16:50 UTC (permalink / raw) To: Jeff Garzik; +Cc: linux-ide > Independently I tried omitting that test. With that test gone, my > kernel does not crash, but still the open (and thereafter modprobe -r) > does not complete: > ... > kernel: ata_scsi_dump_cdb: CDB (2:0,0,0) 00 00 00 00 00 00 00 00 00 > kernel: ata_scsi_translate: ENTER > kernel: ata_dev_select: ENTER, ata2: device 0, wait 1 > kernel: ata_tf_load_pio: feat 0x0 nsect 0x0 lba 0x0 0x0 0x20 > kernel: ata_tf_load_pio: device 0xA0 > kernel: ata_exec_command_pio: ata2: cmd 0xA0 > kernel: ata_scsi_translate: EXIT > kernel: atapi_packet_task: busy wait > kernel: atapi_packet_task: send cdb > > In my ignorance, I'm now trying ... Sorry to say, as is exactly correct, when dying this way in atapi_xlate I see: cmd->sc_data_direction = 3 SCSI_DATA_NONE cmd->request_bufflen = 0 > Soon I will give up for this day, > sorry to say, real life intrudes. See you later, Pat LaVarre ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: SATA ATAPI work in progress 2004-05-15 15:49 ` Pat LaVarre 2004-05-15 15:56 ` Pat LaVarre @ 2004-05-15 16:30 ` Jeff Garzik 1 sibling, 0 replies; 34+ messages in thread From: Jeff Garzik @ 2004-05-15 16:30 UTC (permalink / raw) To: Pat LaVarre; +Cc: linux-ide Pat LaVarre wrote: > kernel: ata_scsi_dump_cdb: CDB (2:0,0,0) 12 00 00 00 24 00 00 00 46 > kernel: ata_scsi_translate: ENTER > kernel: ata_sg_setup_one: mapped buffer of 36 bytes for read > kernel: ata_fill_sg: PRD[0] = (0x46EE54, 0x24) > kernel: ata_dev_select: ENTER, ata2: device 0, wait 1 > kernel: ata_tf_load_pio: feat 0x5 nsect 0x0 lba 0x0 0x0 0x0 > kernel: ata_tf_load_pio: device 0xA0 > kernel: ata_exec_command_pio: ata2: cmd 0xA0 > kernel: ata_scsi_translate: EXIT > kernel: atapi_packet_task: busy wait > kernel: atapi_packet_task: send cdb > kernel: ata_host_intr: BUS_DMA (host_stat 0x24) > kernel: ata_dma_complete: ENTER > kernel: ata_dma_complete: host 2, host_stat==0x24, drv_stat==0x50 > kernel: ata_sg_clean: unmapping 1 sg elements > kernel: ata_scsi_dump_cdb: CDB (2:0,0,0) 12 00 00 00 c0 00 00 00 46 > kernel: ata_scsi_translate: ENTER > kernel: ata_sg_setup_one: mapped buffer of 192 bytes for read > kernel: ata_fill_sg: PRD[0] = (0x46EE54, 0xC0) > kernel: ata_dev_select: ENTER, ata2: device 0, wait 1 > kernel: ata_tf_load_pio: feat 0x5 nsect 0x0 lba 0x0 0x0 0x0 > kernel: ata_tf_load_pio: device 0xA0 > kernel: ata_exec_command_pio: ata2: cmd 0xA0 > kernel: ata_scsi_translate: EXIT > kernel: atapi_packet_task: busy wait > kernel: atapi_packet_task: send cdb > kernel: ata_host_intr: BUS_DMA (host_stat 0x24) > kernel: ata_dma_complete: ENTER > kernel: ata_dma_complete: host 2, host_stat==0x24, drv_stat==0x50 > kernel: ata_sg_clean: unmapping 1 sg elements > kernel: Vendor: Iomega Model: RRD Rev: 74.B > kernel: Type: CD-ROM ANSI SCSI revision: 00 > kernel: ata_scsi_dump_cdb: CDB (2:0,1,0) 12 00 00 00 24 00 00 00 46 This is some amount of success, as libata appears from this dump to have successfully handled a data-transferring SCSI command (INQUIRY). Jeff ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH] Re: SATA ATAPI work in progress 2004-05-15 0:02 ` Pat LaVarre 2004-05-15 0:38 ` Jeff Garzik @ 2004-05-15 0:46 ` Jeff Garzik 2004-05-15 13:08 ` Pat LaVarre 1 sibling, 1 reply; 34+ messages in thread From: Jeff Garzik @ 2004-05-15 0:46 UTC (permalink / raw) To: Pat LaVarre; +Cc: linux-ide [-- Attachment #1: Type: text/plain, Size: 34 bytes --] Here you go, this will help too. [-- Attachment #2: patch --] [-- Type: text/plain, Size: 410 bytes --] ===== drivers/scsi/libata-core.c 1.57 vs edited ===== --- 1.57/drivers/scsi/libata-core.c Thu May 13 22:45:55 2004 +++ edited/drivers/scsi/libata-core.c Fri May 14 20:45:44 2004 @@ -2664,6 +2667,7 @@ switch (qc->tf.protocol) { case ATA_PROT_DMA: + case ATA_PROT_ATAPI_DMA: if (ap->flags & ATA_FLAG_MMIO) { void *mmio = (void *) ap->ioaddr.bmdma_addr; host_stat = readb(mmio + ATA_DMA_STATUS); ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] Re: SATA ATAPI work in progress 2004-05-15 0:46 ` [PATCH] " Jeff Garzik @ 2004-05-15 13:08 ` Pat LaVarre 2004-05-15 13:10 ` Pat LaVarre 0 siblings, 1 reply; 34+ messages in thread From: Pat LaVarre @ 2004-05-15 13:08 UTC (permalink / raw) To: Jeff Garzik; +Cc: linux-ide > + case ATA_PROT_ATAPI_DMA: In ata_host_intr I see, thank you, I name this: I name this patch.7 as in a virtual thread titled: [PATCH] libata atapi work #7 (I actually prefer one thread, I'm just talking to keep us sync'ed across the kilometers.) Pat LaVarre ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] Re: SATA ATAPI work in progress 2004-05-15 13:08 ` Pat LaVarre @ 2004-05-15 13:10 ` Pat LaVarre 2004-05-15 14:13 ` Pat LaVarre 0 siblings, 1 reply; 34+ messages in thread From: Pat LaVarre @ 2004-05-15 13:10 UTC (permalink / raw) To: Jeff Garzik; +Cc: linux-ide Ouch we're sync'ed enough, but not wholly: $ <~/patch.6 patch -p1 patching file drivers/scsi/libata-core.c patching file drivers/scsi/libata-scsi.c patching file include/linux/ata.h patching file include/linux/libata.h $ $ <~/patch.7 patch -p1 patching file drivers/scsi/libata-core.c Hunk #1 succeeded at 2670 (offset 3 lines). $ Pat LaVarre ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] Re: SATA ATAPI work in progress 2004-05-15 13:10 ` Pat LaVarre @ 2004-05-15 14:13 ` Pat LaVarre 0 siblings, 0 replies; 34+ messages in thread From: Pat LaVarre @ 2004-05-15 14:13 UTC (permalink / raw) To: Jeff Garzik; +Cc: linux-ide > ... > $ <~/patch.6 patch -p1 > $ <~/patch.7 patch -p1 Success! Thank you. I'm writing this reply now to give context in order to the other lessons I'm learning. I do plan to post specific evidence of success together with the composite patch vs. 2.6.6-bk2 that worked for me. Pat LaVarre ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: SATA ATAPI work in progress 2004-05-13 21:16 ` Jeff Garzik 2004-05-13 21:25 ` Jeff Garzik 2004-05-14 18:23 ` Pat LaVarre @ 2004-05-14 18:28 ` Pat LaVarre 2004-05-14 18:38 ` Jeff Garzik 2 siblings, 1 reply; 34+ messages in thread From: Pat LaVarre @ 2004-05-14 18:28 UTC (permalink / raw) To: Jeff Garzik; +Cc: linux-ide > > +static unsigned int atapi_xlat(struct ata_queued_cmd *qc, u8 > *scsicmd) > > +{ > > + int bcl = (8 * 0x400); /* PIO "byte count limit" */ > > Make this a more obvious "8 * 1024". > > For the sake of correctness, I prefer to call this "DRQ block size" > rather than the more ambigous "byte count limit". > > This is important because some AHCI SATA controllers can only transfer > a _single_ DRQ block per disk transaction. In anticipation of the > demise of PIO, no doubt :) Perhaps I should check: Are we agreed that this bcl is the max permissible byte count for a PIO DRQ, distinct from the actual byte count of a PIO DRQ? For example, if our limit is x800, then the t13.org texts let the device reply with x200 + x600 + x800 + x400 ... They allow any distribution of addends, so long as no single INTRQ asks for more than the limit we set. ATAPI often, but not reliably, implies x800 bytes/lba = 2 KiB/lba, by way of implying but not requiring PDT x05 MMC = DVD/CD. Pat LaVarre ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: SATA ATAPI work in progress 2004-05-14 18:28 ` Pat LaVarre @ 2004-05-14 18:38 ` Jeff Garzik 0 siblings, 0 replies; 34+ messages in thread From: Jeff Garzik @ 2004-05-14 18:38 UTC (permalink / raw) To: Pat LaVarre; +Cc: linux-ide Pat LaVarre wrote: > Are we agreed that this bcl is the max permissible byte count for a PIO > DRQ, distinct from the actual byte count of a PIO DRQ? Yes, agreed. Jeff ^ permalink raw reply [flat|nested] 34+ messages in thread
end of thread, other threads:[~2004-05-15 16:51 UTC | newest] Thread overview: 34+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2004-05-12 20:20 SATA ATAPI work in progress Pat LaVarre 2004-05-12 20:40 ` Jeff Garzik 2004-05-12 23:14 ` Pat LaVarre 2004-05-13 15:07 ` Pat LaVarre 2004-05-13 20:56 ` Jeff Garzik 2004-05-13 21:16 ` Jeff Garzik 2004-05-13 21:25 ` Jeff Garzik 2004-05-13 21:36 ` Pat LaVarre 2004-05-13 21:44 ` Jeff Garzik 2004-05-13 21:49 ` Jeff Garzik 2004-05-14 18:23 ` Pat LaVarre 2004-05-14 18:55 ` Jeff Garzik 2004-05-14 19:37 ` when limited to a single DRQ block per disk transaction Pat LaVarre 2004-05-14 19:50 ` Jeff Garzik 2004-05-14 20:12 ` Pat LaVarre 2004-05-14 23:47 ` SATA ATAPI work in progress Pat LaVarre 2004-05-15 0:02 ` Pat LaVarre 2004-05-15 0:38 ` Jeff Garzik 2004-05-15 13:06 ` Pat LaVarre 2004-05-15 13:49 ` Pat LaVarre 2004-05-15 15:27 ` Jeff Garzik 2004-05-15 15:49 ` Pat LaVarre 2004-05-15 15:56 ` Pat LaVarre 2004-05-15 16:04 ` Pat LaVarre 2004-05-15 16:32 ` Jeff Garzik 2004-05-15 16:43 ` Pat LaVarre 2004-05-15 16:50 ` Pat LaVarre 2004-05-15 16:30 ` Jeff Garzik 2004-05-15 0:46 ` [PATCH] " Jeff Garzik 2004-05-15 13:08 ` Pat LaVarre 2004-05-15 13:10 ` Pat LaVarre 2004-05-15 14:13 ` Pat LaVarre 2004-05-14 18:28 ` Pat LaVarre 2004-05-14 18:38 ` Jeff Garzik
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).