SATA ATAPI work in progress

SATA ATAPI work in progress

[Pat LaVarre]

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;

Re: SATA ATAPI work in progress

[Jeff Garzik]

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

Re: SATA ATAPI work in progress

[Pat LaVarre]

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;

Re: SATA ATAPI work in progress

[Pat LaVarre]

> 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

Re: SATA ATAPI work in progress

[Jeff Garzik]

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

Re: SATA ATAPI work in progress

[Jeff Garzik]

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

Re: SATA ATAPI work in progress

[Jeff Garzik]

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

Re: SATA ATAPI work in progress

[Pat LaVarre]

> 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

Re: SATA ATAPI work in progress

[Jeff Garzik]

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

Re: SATA ATAPI work in progress

[Jeff Garzik]

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.

Re: SATA ATAPI work in progress

[Pat LaVarre]

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

Re: SATA ATAPI work in progress

[Pat LaVarre]

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

Re: SATA ATAPI work in progress

[Jeff Garzik]

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

Re: SATA ATAPI work in progress

[Jeff Garzik]

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

when limited to a single DRQ block per disk transaction

[Pat LaVarre]

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

Re: when limited to a single DRQ block per disk transaction

[Jeff Garzik]

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

Re: when limited to a single DRQ block per disk transaction

[Pat LaVarre]

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

Re: SATA ATAPI work in progress

[Pat LaVarre]

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

Re: SATA ATAPI work in progress

[Pat LaVarre]

[-- 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;

Re: SATA ATAPI work in progress

[Jeff Garzik]

[-- 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 */

[PATCH] Re: SATA ATAPI work in progress

[Jeff Garzik]

[-- 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);

Re: SATA ATAPI work in progress

[Pat LaVarre]

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

Re: [PATCH] Re: SATA ATAPI work in progress

[Pat LaVarre]

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

Re: [PATCH] Re: SATA ATAPI work in progress

[Pat LaVarre]

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

Re: SATA ATAPI work in progress

[Pat LaVarre]

[-- 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;

Re: [PATCH] Re: SATA ATAPI work in progress

[Pat LaVarre]

> ...
> $ <~/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

Re: SATA ATAPI work in progress

[Jeff Garzik]

Pat LaVarre wrote:
> Theory confirmed!
[...]
> Success!  Thank you.


Does this mean you actually got ATAPI to work?  Or just that we made 
progress?

	Jeff

Re: SATA ATAPI work in progress

[Pat LaVarre]

[-- 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;

Re: SATA ATAPI work in progress

[Pat LaVarre]

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

Re: SATA ATAPI work in progress

[Pat LaVarre]

> 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

Re: SATA ATAPI work in progress

[Jeff Garzik]

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

Re: SATA ATAPI work in progress

[Jeff Garzik]

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

Re: SATA ATAPI work in progress

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

Re: SATA ATAPI work in progress

[Pat LaVarre]

> 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