[PATCH 1/2 #upstream] sata_fsl,mv,nv: prepare for NCQ command completionupdate

[Tejun Heo]

Make the following changes to prepare for NCQ command completion
update.  Changes made by this patch don't cause any functional
difference.

* sata_fsl_host_intr(): rename the local variable qc_active to
  done_mask as that's what it is.

* mv_process_crpb_response(): restructure if clause for easier update.

* nv_adma_interrupt(): drop unnecessary error variable.

* nv_swncq_sdbfis(): drop unnecessary nr_done and return 0 on success.
  Typo fix.

* nv_swncq_dmafis(): drop unused return value and return void.

* nv_swncq_host_interrupt(): drop unnecessary return value handling.

Signed-off-by: Tejun Heo <tj@xxxxxxxxxx>
Cc: Ashish Kalra <ashish.kalra@xxxxxxxxxxxxx>
Cc: Saeed Bishara <saeed@xxxxxxxxxxx>
Cc: Mark Lord <liml@xxxxxx>
Cc: Robert Hancock <hancockr@xxxxxxx>
---
So, something like this.  I tested both flavors of sata_nv but don't
have access to sata_mv or sata_fsl, but the conversion is pretty
straight forward and failures should be pretty easy to catch.

Thanks.

 drivers/ata/sata_fsl.c |   20 ++++++++++----------
 drivers/ata/sata_mv.c  |   47 ++++++++++++++++++++++++-----------------------
 drivers/ata/sata_nv.c  |   32 ++++++++++++++------------------
 3 files changed, 48 insertions(+), 51 deletions(-)

Index: ata/drivers/ata/sata_fsl.c
===================================================================
--- ata.orig/drivers/ata/sata_fsl.c
+++ ata/drivers/ata/sata_fsl.c
@@ -1096,7 +1096,7 @@ static void sata_fsl_host_intr(struct at
 {
 	struct sata_fsl_host_priv *host_priv = ap->host->private_data;
 	void __iomem *hcr_base = host_priv->hcr_base;
-	u32 hstatus, qc_active = 0;
+	u32 hstatus, done_mask = 0;
 	struct ata_queued_cmd *qc;
 	u32 SError;

@@ -1116,28 +1116,28 @@ static void sata_fsl_host_intr(struct at
 	}

 	/* Read command completed register */
-	qc_active = ioread32(hcr_base + CC);
+	done_mask = ioread32(hcr_base + CC);

 	VPRINTK("Status of all queues :\n");
-	VPRINTK("qc_active/CC = 0x%x, CA = 0x%x, CE=0x%x,CQ=0x%x,apqa=0x%x\n",
-		qc_active,
+	VPRINTK("done_mask/CC = 0x%x, CA = 0x%x, CE=0x%x,CQ=0x%x,apqa=0x%x\n",
+		done_mask,
 		ioread32(hcr_base + CA),
 		ioread32(hcr_base + CE),
 		ioread32(hcr_base + CQ),
 		ap->qc_active);

-	if (qc_active & ap->qc_active) {
+	if (done_mask & ap->qc_active) {
 		int i;
 		/* clear CC bit, this will also complete the interrupt */
-		iowrite32(qc_active, hcr_base + CC);
+		iowrite32(done_mask, hcr_base + CC);

 		DPRINTK("Status of all queues :\n");
-		DPRINTK("qc_active/CC = 0x%x, CA = 0x%x, CE=0x%x\n",
-			qc_active, ioread32(hcr_base + CA),
+		DPRINTK("done_mask/CC = 0x%x, CA = 0x%x, CE=0x%x\n",
+			done_mask, ioread32(hcr_base + CA),
 			ioread32(hcr_base + CE));

 		for (i = 0; i < SATA_FSL_QUEUE_DEPTH; i++) {
-			if (qc_active & (1 << i)) {
+			if (done_mask & (1 << i)) {
 				qc = ata_qc_from_tag(ap, i);
 				if (qc) {
 					ata_qc_complete(qc);
@@ -1164,7 +1164,7 @@ static void sata_fsl_host_intr(struct at
 		/* Spurious Interrupt!! */
 		DPRINTK("spurious interrupt!!, CC = 0x%x\n",
 			ioread32(hcr_base + CC));
-		iowrite32(qc_active, hcr_base + CC);
+		iowrite32(done_mask, hcr_base + CC);
 		return;
 	}
 }
Index: ata/drivers/ata/sata_mv.c
===================================================================
--- ata.orig/drivers/ata/sata_mv.c
+++ ata/drivers/ata/sata_mv.c
@@ -2716,34 +2716,35 @@ static void mv_err_intr(struct ata_port
 static void mv_process_crpb_response(struct ata_port *ap,
 		struct mv_crpb *response, unsigned int tag, int ncq_enabled)
 {
+	u8 ata_status;
+	u16 edma_status = le16_to_cpu(response->flags);
 	struct ata_queued_cmd *qc = ata_qc_from_tag(ap, tag);

-	if (qc) {
-		u8 ata_status;
-		u16 edma_status = le16_to_cpu(response->flags);
-		/*
-		 * edma_status from a response queue entry:
-		 *   LSB is from EDMA_ERR_IRQ_CAUSE (non-NCQ only).
-		 *   MSB is saved ATA status from command completion.
-		 */
-		if (!ncq_enabled) {
-			u8 err_cause = edma_status & 0xff & ~EDMA_ERR_DEV;
-			if (err_cause) {
-				/*
-				 * Error will be seen/handled by mv_err_intr().
-				 * So do nothing at all here.
-				 */
-				return;
-			}
-		}
-		ata_status = edma_status >> CRPB_FLAG_STATUS_SHIFT;
-		if (!ac_err_mask(ata_status))
-			ata_qc_complete(qc);
-		/* else: leave it for mv_err_intr() */
-	} else {
+	if (unlikely(!qc)) {
 		ata_port_printk(ap, KERN_ERR, "%s: no qc for tag=%d\n",
 				__func__, tag);
+		return;
+	}
+
+	/*
+	 * edma_status from a response queue entry:
+	 *   LSB is from EDMA_ERR_IRQ_CAUSE (non-NCQ only).
+	 *   MSB is saved ATA status from command completion.
+	 */
+	if (!ncq_enabled) {
+		u8 err_cause = edma_status & 0xff & ~EDMA_ERR_DEV;
+		if (err_cause) {
+			/*
+			 * Error will be seen/handled by
+			 * mv_err_intr().  So do nothing at all here.
+			 */
+			return;
+		}
 	}
+	ata_status = edma_status >> CRPB_FLAG_STATUS_SHIFT;
+	if (!ac_err_mask(ata_status))
+		ata_qc_complete(qc);
+	/* else: leave it for mv_err_intr() */
 }

 static void mv_process_crpb_entries(struct ata_port *ap, struct mv_port_priv *pp)
Index: ata/drivers/ata/sata_nv.c
===================================================================
--- ata.orig/drivers/ata/sata_nv.c
+++ ata/drivers/ata/sata_nv.c
@@ -1018,7 +1018,7 @@ static irqreturn_t nv_adma_interrupt(int
 			      NV_ADMA_STAT_CPBERR |
 			      NV_ADMA_STAT_CMD_COMPLETE)) {
 			u32 check_commands = notifier_clears[i];
-			int pos, error = 0;
+			int pos, rc;

 			if (status & NV_ADMA_STAT_CPBERR) {
 				/* check all active commands */
@@ -1030,10 +1030,12 @@ static irqreturn_t nv_adma_interrupt(int
 			}

 			/* check CPBs for completed commands */
-			while ((pos = ffs(check_commands)) && !error) {
+			while ((pos = ffs(check_commands))) {
 				pos--;
-				error = nv_adma_check_cpb(ap, pos,
+				rc = nv_adma_check_cpb(ap, pos,
 						notifier_error & (1 << pos));
+				if (unlikely(rc))
+					check_commands = 0;
 				check_commands &= ~(1 << pos);
 			}
 		}
@@ -2129,7 +2131,6 @@ static int nv_swncq_sdbfis(struct ata_po
 	struct nv_swncq_port_priv *pp = ap->private_data;
 	struct ata_eh_info *ehi = &ap->link.eh_info;
 	u32 sactive;
-	int nr_done = 0;
 	u32 done_mask;
 	int i;
 	u8 host_stat;
@@ -2170,22 +2171,21 @@ static int nv_swncq_sdbfis(struct ata_po
 			pp->dhfis_bits &= ~(1 << i);
 			pp->dmafis_bits &= ~(1 << i);
 			pp->sdbfis_bits |= (1 << i);
-			nr_done++;
 		}
 	}

 	if (!ap->qc_active) {
 		DPRINTK("over\n");
 		nv_swncq_pp_reinit(ap);
-		return nr_done;
+		return 0;
 	}

 	if (pp->qc_active & pp->dhfis_bits)
-		return nr_done;
+		return 0;

 	if ((pp->ncq_flags & ncq_saw_backout) ||
 	    (pp->qc_active ^ pp->dhfis_bits))
-		/* if the controller cann't get a device to host register FIS,
+		/* if the controller can't get a device to host register FIS,
 		 * The driver needs to reissue the new command.
 		 */
 		lack_dhfis = 1;
@@ -2202,7 +2202,7 @@ static int nv_swncq_sdbfis(struct ata_po
 	if (lack_dhfis) {
 		qc = ata_qc_from_tag(ap, pp->last_issue_tag);
 		nv_swncq_issue_atacmd(ap, qc);
-		return nr_done;
+		return 0;
 	}

 	if (pp->defer_queue.defer_bits) {
@@ -2212,7 +2212,7 @@ static int nv_swncq_sdbfis(struct ata_po
 		nv_swncq_issue_atacmd(ap, qc);
 	}

-	return nr_done;
+	return 0;
 }

 static inline u32 nv_swncq_tag(struct ata_port *ap)
@@ -2224,7 +2224,7 @@ static inline u32 nv_swncq_tag(struct at
 	return (tag & 0x1f);
 }

-static int nv_swncq_dmafis(struct ata_port *ap)
+static void nv_swncq_dmafis(struct ata_port *ap)
 {
 	struct ata_queued_cmd *qc;
 	unsigned int rw;
@@ -2239,7 +2239,7 @@ static int nv_swncq_dmafis(struct ata_po
 	qc = ata_qc_from_tag(ap, tag);

 	if (unlikely(!qc))
-		return 0;
+		return;

 	rw = qc->tf.flags & ATA_TFLAG_WRITE;

@@ -2254,8 +2254,6 @@ static int nv_swncq_dmafis(struct ata_po
 		dmactl |= ATA_DMA_WR;

 	iowrite8(dmactl | ATA_DMA_START, ap->ioaddr.bmdma_addr + ATA_DMA_CMD);
-
-	return 1;
 }

 static void nv_swncq_host_interrupt(struct ata_port *ap, u16 fis)
@@ -2265,7 +2263,6 @@ static void nv_swncq_host_interrupt(stru
 	struct ata_eh_info *ehi = &ap->link.eh_info;
 	u32 serror;
 	u8 ata_stat;
-	int rc = 0;

 	ata_stat = ap->ops->sff_check_status(ap);
 	nv_swncq_irq_clear(ap, fis);
@@ -2310,8 +2307,7 @@ static void nv_swncq_host_interrupt(stru
 			"dhfis 0x%X dmafis 0x%X sactive 0x%X\n",
 			ap->print_id, pp->qc_active, pp->dhfis_bits,
 			pp->dmafis_bits, readl(pp->sactive_block));
-		rc = nv_swncq_sdbfis(ap);
-		if (rc < 0)
+		if (nv_swncq_sdbfis(ap) < 0)
 			goto irq_error;
 	}

@@ -2348,7 +2344,7 @@ static void nv_swncq_host_interrupt(stru
 		 */
 		pp->dmafis_bits |= (0x1 << nv_swncq_tag(ap));
 		pp->ncq_flags |= ncq_saw_dmas;
-		rc = nv_swncq_dmafis(ap);
+		nv_swncq_dmafis(ap);
 	}

 irq_exit:
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/