All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <htejun@gmail.com>
To: jeff@garzik.org, alan@lxorguk.ukuu.org.uk, linux-ide@vger.kernel.org
Cc: Tejun Heo <htejun@gmail.com>
Subject: [PATCH 07/10] sata_inic162x: use IDMA for ATAPI commands
Date: Wed, 30 Apr 2008 16:35:14 +0900	[thread overview]
Message-ID: <12095409193866-git-send-email-htejun@gmail.com> (raw)
In-Reply-To: <12095409172824-git-send-email-htejun@gmail.com>

Use IDMA for ATAPI commands.  Write and some misc commands time out
when executed using ATAPI_PROT_DMA but ATAPI_PROT_PIO works fine.  As
PIO is driven by DMA too, it doesn't make any noticeable difference
for native SATA devices.  inic_check_atapi_dma() is implemented to
force PIO for those ATAPI commands.

After this change, sata_inic162x issues all commands using IDMA.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 drivers/ata/sata_inic162x.c |   81 +++++++++++++++++++++++++------------------
 1 files changed, 47 insertions(+), 34 deletions(-)

diff --git a/drivers/ata/sata_inic162x.c b/drivers/ata/sata_inic162x.c
index c132942..a31dc24 100644
--- a/drivers/ata/sata_inic162x.c
+++ b/drivers/ata/sata_inic162x.c
@@ -192,7 +192,8 @@ struct inic_prd {
 
 struct inic_pkt {
 	struct inic_cpb	cpb;
-	struct inic_prd	prd[LIBATA_MAX_PRD];
+	struct inic_prd	prd[LIBATA_MAX_PRD + 1];	/* + 1 for cdb */
+	u8		cdb[ATAPI_CDB_LEN];
 } __packed;
 
 struct inic_host_priv {
@@ -361,23 +362,18 @@ static void inic_host_intr(struct ata_port *ap)
 		goto spurious;
 	}
 
-	if (!ata_is_atapi(qc->tf.protocol)) {
-		if (likely(idma_stat & IDMA_STAT_DONE)) {
-			inic_stop_idma(ap);
+	if (likely(idma_stat & IDMA_STAT_DONE)) {
+		inic_stop_idma(ap);
 
-			/* Depending on circumstances, device error
-			 * isn't reported by IDMA, check it explicitly.
-			 */
-			if (unlikely(readb(port_base + PORT_TF_COMMAND) &
-				     (ATA_DF | ATA_ERR)))
-				qc->err_mask |= AC_ERR_DEV;
+		/* Depending on circumstances, device error
+		 * isn't reported by IDMA, check it explicitly.
+		 */
+		if (unlikely(readb(port_base + PORT_TF_COMMAND) &
+			     (ATA_DF | ATA_ERR)))
+			qc->err_mask |= AC_ERR_DEV;
 
-			ata_qc_complete(qc);
-			return;
-		}
-	} else {
-		if (likely(ata_sff_host_intr(ap, qc)))
-			return;
+		ata_qc_complete(qc);
+		return;
 	}
 
  spurious:
@@ -421,6 +417,19 @@ static irqreturn_t inic_interrupt(int irq, void *dev_instance)
 	return IRQ_RETVAL(handled);
 }
 
+static int inic_check_atapi_dma(struct ata_queued_cmd *qc)
+{
+	/* For some reason ATAPI_PROT_DMA doesn't work for some
+	 * commands including writes and other misc ops.  Use PIO
+	 * protocol instead, which BTW is driven by the DMA engine
+	 * anyway, so it shouldn't make much difference for native
+	 * SATA devices.
+	 */
+	if (atapi_cmd_type(qc->cdb[0]) == READ)
+		return 0;
+	return 1;
+}
+
 static void inic_fill_sg(struct inic_prd *prd, struct ata_queued_cmd *qc)
 {
 	struct scatterlist *sg;
@@ -452,20 +461,21 @@ static void inic_qc_prep(struct ata_queued_cmd *qc)
 	struct inic_prd *prd = pkt->prd;
 	bool is_atapi = ata_is_atapi(qc->tf.protocol);
 	bool is_data = ata_is_data(qc->tf.protocol);
+	unsigned int cdb_len = 0;
 
 	VPRINTK("ENTER\n");
 
 	if (is_atapi)
-		return;
+		cdb_len = qc->dev->cdb_len;
 
 	/* prepare packet, based on initio driver */
 	memset(pkt, 0, sizeof(struct inic_pkt));
 
 	cpb->ctl_flags = CPB_CTL_VALID | CPB_CTL_IEN;
-	if (is_data)
+	if (is_atapi || is_data)
 		cpb->ctl_flags |= CPB_CTL_DATA;
 
-	cpb->len = cpu_to_le32(qc->nbytes);
+	cpb->len = cpu_to_le32(qc->nbytes + cdb_len);
 	cpb->prd = cpu_to_le32(pp->pkt_dma + offsetof(struct inic_pkt, prd));
 
 	cpb->device = qc->tf.device;
@@ -486,6 +496,18 @@ static void inic_qc_prep(struct ata_queued_cmd *qc)
 	cpb->command = qc->tf.command;
 	/* don't load ctl - dunno why.  it's like that in the initio driver */
 
+	/* setup PRD for CDB */
+	if (is_atapi) {
+		memcpy(pkt->cdb, qc->cdb, ATAPI_CDB_LEN);
+		prd->mad = cpu_to_le32(pp->pkt_dma +
+				       offsetof(struct inic_pkt, cdb));
+		prd->len = cpu_to_le16(cdb_len);
+		prd->flags = PRD_CDB | PRD_WRITE;
+		if (!is_data)
+			prd->flags |= PRD_END;
+		prd++;
+	}
+
 	/* setup sg table */
 	if (is_data)
 		inic_fill_sg(prd, qc);
@@ -498,16 +520,12 @@ static unsigned int inic_qc_issue(struct ata_queued_cmd *qc)
 	struct ata_port *ap = qc->ap;
 	void __iomem *port_base = inic_port_base(ap);
 
-	if (!ata_is_atapi(qc->tf.protocol)) {
-		/* fire up the ADMA engine */
-		writew(HCTL_FTHD0, port_base + HOST_CTL);
-		writew(IDMA_CTL_GO, port_base + PORT_IDMA_CTL);
-		writeb(0, port_base + PORT_CPB_PTQFIFO);
+	/* fire up the ADMA engine */
+	writew(HCTL_FTHD0, port_base + HOST_CTL);
+	writew(IDMA_CTL_GO, port_base + PORT_IDMA_CTL);
+	writeb(0, port_base + PORT_CPB_PTQFIFO);
 
-		return 0;
-	}
-
-	return ata_sff_qc_issue(qc);
+	return 0;
 }
 
 static void inic_tf_read(struct ata_port *ap, struct ata_taskfile *tf)
@@ -704,6 +722,7 @@ static int inic_port_start(struct ata_port *ap)
 static struct ata_port_operations inic_port_ops = {
 	.inherits		= &ata_sff_port_ops,
 
+	.check_atapi_dma	= inic_check_atapi_dma,
 	.qc_prep		= inic_qc_prep,
 	.qc_issue		= inic_qc_issue,
 	.qc_fill_rtf		= inic_qc_fill_rtf,
@@ -723,12 +742,6 @@ static struct ata_port_operations inic_port_ops = {
 };
 
 static struct ata_port_info inic_port_info = {
-	/* For some reason, ATAPI_PROT_PIO is broken on this
-	 * controller, and no, PIO_POLLING does't fix it.  It somehow
-	 * manages to report the wrong ireason and ignoring ireason
-	 * results in machine lock up.  Tell libata to always prefer
-	 * DMA.
-	 */
 	.flags			= ATA_FLAG_SATA | ATA_FLAG_PIO_DMA,
 	.pio_mask		= 0x1f,	/* pio0-4 */
 	.mwdma_mask		= 0x07, /* mwdma0-2 */
-- 
1.5.2.4


  parent reply	other threads:[~2008-04-30  7:35 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-04-30  7:35 [PATCHSET] sata_inic162x: make it work Tejun Heo
2008-04-30  7:35 ` [PATCH 01/10] sata_inic162x: misc clean ups Tejun Heo
2008-05-06 14:08   ` Jeff Garzik
2008-04-30  7:35 ` [PATCH 02/10] sata_inic162x: add / update constants Tejun Heo
2008-04-30  7:35 ` [PATCH 03/10] sata_inic162x: update TF read handling Tejun Heo
2008-05-01 14:55   ` [PATCH 03/10 UPDATED] " Tejun Heo
2008-04-30  7:35 ` [PATCH 04/10] sata_inic162x: use IDMA for ATA_PROT_DMA Tejun Heo
2008-04-30  7:35 ` [PATCH 05/10] sata_inic162x: kill now unused bmdma related stuff Tejun Heo
2008-04-30  7:35 ` [PATCH 06/10] sata_inic162x: use IDMA for non DMA ATA commands Tejun Heo
2008-04-30  7:35 ` Tejun Heo [this message]
2008-04-30  7:35 ` [PATCH 08/10] sata_inic162x: kill now unused SFF related stuff Tejun Heo
2008-04-30  7:35 ` [PATCH 09/10] sata_inic162x: add cardbus support Tejun Heo
2008-04-30  7:35 ` [PATCH 10/10] sata_inic162x: update intro comment, up the version and drop EXPERIMENTAL Tejun Heo
2008-04-30  7:39 ` [PATCHSET] sata_inic162x: make it work Tejun Heo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=12095409193866-git-send-email-htejun@gmail.com \
    --to=htejun@gmail.com \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=jeff@garzik.org \
    --cc=linux-ide@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.