[PATCH v5 0/2] ata: Fix the dma state machine lockup for APM X-Gene SoC

[PATCH v5 0/2] ata: Fix the dma state machine lockup for APM X-Gene SoC

[Suman Tripathi]

This patch addresses the dma state machine lockup for APM X-Gene SoC.

Signed-off-by: Loc Ho <lho@apm.com>
Signed-off-by: Suman Tripathi <stripathi@apm.com>
---

Suman Tripathi (2):
  libahci: Implement the function ahci_restart_engine to restart the
    port dma engine.
  ata: Fix the dma state machine lockup for the IDENTIFY DEVICE PIO mode
    command.

 drivers/ata/ahci.h       |  3 ++-
 drivers/ata/ahci_xgene.c | 43 ++++++++++++++++++++++++++++++-------------
 drivers/ata/libahci.c    | 16 ++++++++++++++--
 3 files changed, 46 insertions(+), 16 deletions(-)

--
1.8.2.1

[PATCH v5 1/2] libahci: Implement the function ahci_restart_engine to restart the port dma engine.

[Suman Tripathi]

This patch adds an function to restart the port dma engine.
This patch also exports the function ahci_qc_issue. The subsequent
patches will make use of them.

Signed-off-by: Loc Ho <lho@apm.com>
Signed-off-by: Suman Tripathi <stripathi@apm.com>
---
 drivers/ata/ahci.h    |  3 ++-
 drivers/ata/libahci.c | 16 ++++++++++++++--
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index af63c75..98e8839 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -369,9 +369,10 @@ int ahci_reset_controller(struct ata_host *host);
 int ahci_do_softreset(struct ata_link *link, unsigned int *class,
 		      int pmp, unsigned long deadline,
 		      int (*check_ready)(struct ata_link *link));
-
+unsigned int ahci_qc_issue(struct ata_queued_cmd *qc);
 int ahci_stop_engine(struct ata_port *ap);
 void ahci_start_engine(struct ata_port *ap);
+int ahci_restart_engine(struct ata_port *ap);
 int ahci_check_ready(struct ata_link *link);
 int ahci_kick_engine(struct ata_port *ap);
 int ahci_port_resume(struct ata_port *ap);
diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index b986145..1b86cf4 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -68,7 +68,6 @@ static ssize_t ahci_transmit_led_message(struct ata_port *ap, u32 state,

 static int ahci_scr_read(struct ata_link *link, unsigned int sc_reg, u32 *val);
 static int ahci_scr_write(struct ata_link *link, unsigned int sc_reg, u32 val);
-static unsigned int ahci_qc_issue(struct ata_queued_cmd *qc);
 static bool ahci_qc_fill_rtf(struct ata_queued_cmd *qc);
 static int ahci_port_start(struct ata_port *ap);
 static void ahci_port_stop(struct ata_port *ap);
@@ -742,6 +741,18 @@ static int ahci_set_lpm(struct ata_link *link, enum ata_lpm_policy policy,
 	return 0;
 }

+int ahci_restart_engine(struct ata_port *ap)
+{
+	struct ahci_host_priv *hpriv = ap->host->private_data;
+
+	ahci_stop_engine(ap);
+	ahci_start_fis_rx(ap);
+	hpriv->start_engine(ap);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(ahci_restart_engine);
+
 #ifdef CONFIG_PM
 static void ahci_power_down(struct ata_port *ap)
 {
@@ -1940,7 +1951,7 @@ irqreturn_t ahci_interrupt(int irq, void *dev_instance)
 }
 EXPORT_SYMBOL_GPL(ahci_interrupt);

-static unsigned int ahci_qc_issue(struct ata_queued_cmd *qc)
+unsigned int ahci_qc_issue(struct ata_queued_cmd *qc)
 {
 	struct ata_port *ap = qc->ap;
 	void __iomem *port_mmio = ahci_port_base(ap);
@@ -1969,6 +1980,7 @@ static unsigned int ahci_qc_issue(struct ata_queued_cmd *qc)

 	return 0;
 }
+EXPORT_SYMBOL_GPL(ahci_qc_issue);

 static bool ahci_qc_fill_rtf(struct ata_queued_cmd *qc)
 {
--
1.8.2.1

Re: [PATCH v5 1/2] libahci: Implement the function ahci_restart_engine to restart the port dma engine.

[Tejun Heo]

Hello,

On Wed, Jul 02, 2014 at 03:45:07PM +0530, Suman Tripathi wrote:
> +int ahci_restart_engine(struct ata_port *ap)
> +{
> +	struct ahci_host_priv *hpriv = ap->host->private_data;
> +
> +	ahci_stop_engine(ap);
> +	ahci_start_fis_rx(ap);
> +	hpriv->start_engine(ap);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(ahci_restart_engine);

I don't get this function.  No other ahci needs it and it's not even
contained libahci proper.  It calls back to the calling low level
driver.  Why not just expose ahci_start_fis_rx() and implement the
above in xgene?

Thanks.

-- 
tejun

[PATCH v5 2/2] ata: Fix the dma state machine lockup for the IDENTIFY DEVICE PIO mode command.

[Suman Tripathi]

This patch fixes the dma state machine lockup due to the processing
of IDENTIFY DEVICE PIO mode command. The X-Gene AHCI controller
has an errata in which it cannot clear the BSY bit after the PIO setup
FIS. The dma state machine enters CMFatalErrorUpdate state and locks
up. This patch also removes the dma restart workaround from the read_id
function as the read_id function is only called by libata layer for
ATA_INTERNAL commands. But for some cases eg: PORT MULTIPLIER
and udev, the framework will enumerate using SCSI commands and it will
not call read_id function.

Signed-off-by: Loc Ho <lho@apm.com>
Signed-off-by: Suman Tripathi <stripathi@apm.com>
---
 drivers/ata/ahci_xgene.c | 43 ++++++++++++++++++++++++++++++-------------
 1 file changed, 30 insertions(+), 13 deletions(-)

diff --git a/drivers/ata/ahci_xgene.c b/drivers/ata/ahci_xgene.c
index 77c89bf..5b9bddf 100644
--- a/drivers/ata/ahci_xgene.c
+++ b/drivers/ata/ahci_xgene.c
@@ -78,6 +78,7 @@
 struct xgene_ahci_context {
 	struct ahci_host_priv *hpriv;
 	struct device *dev;
+	u8 last_cmd[MAX_AHCI_CHN_PERCTR]; /* tracking the last command issued*/
 	void __iomem *csr_core;		/* Core CSR address of IP */
 	void __iomem *csr_diag;		/* Diag CSR address of IP */
 	void __iomem *csr_axi;		/* AXI CSR address of IP */
@@ -98,20 +99,45 @@ static int xgene_ahci_init_memram(struct xgene_ahci_context *ctx)
 }

 /**
+ * xgene_ahci_qc_issue - Issue commands to the device
+ * @qc: Command to issue
+ *
+ * Due to Hardware errata for IDENTIFY DEVICE command, the controller cannot
+ * clear the BSY bit after receiving the PIO setup FIS. This results in the dma
+ * state machine goes into the CMFatalErrorUpdate state and locks up. By
+ * restarting the dma engine, it removes the controller out of lock up state.
+ */
+static unsigned int xgene_ahci_qc_issue(struct ata_queued_cmd *qc)
+{
+	struct ata_port *ap = qc->ap;
+	struct ahci_host_priv *hpriv = ap->host->private_data;
+	struct xgene_ahci_context *ctx = hpriv->plat_data;
+	int rc = 0;
+
+	if (unlikely(ctx->last_cmd[ap->port_no] == ATA_CMD_ID_ATA))
+		ahci_restart_engine(ap);
+
+	rc = ahci_qc_issue(qc);
+
+	/* Save the last command issued */
+	ctx->last_cmd[ap->port_no] = qc->tf.command;
+
+	return rc;
+}
+
+/**
  * xgene_ahci_read_id - Read ID data from the specified device
  * @dev: device
  * @tf: proposed taskfile
  * @id: data buffer
  *
  * This custom read ID function is required due to the fact that the HW
- * does not support DEVSLP and the controller state machine may get stuck
- * after processing the ID query command.
+ * does not support DEVSLP.
  */
 static unsigned int xgene_ahci_read_id(struct ata_device *dev,
 				       struct ata_taskfile *tf, u16 *id)
 {
 	u32 err_mask;
-	void __iomem *port_mmio = ahci_port_base(dev->link->ap);

 	err_mask = ata_do_dev_read_id(dev, tf, id);
 	if (err_mask)
@@ -133,16 +159,6 @@ static unsigned int xgene_ahci_read_id(struct ata_device *dev,
 	 */
 	id[ATA_ID_FEATURE_SUPP] &= ~(1 << 8);

-	/*
-	 * Due to HW errata, restart the port if no other command active.
-	 * Otherwise the controller may get stuck.
-	 */
-	if (!readl(port_mmio + PORT_CMD_ISSUE)) {
-		writel(PORT_CMD_FIS_RX, port_mmio + PORT_CMD);
-		readl(port_mmio + PORT_CMD);	/* Force a barrier */
-		writel(PORT_CMD_FIS_RX | PORT_CMD_START, port_mmio + PORT_CMD);
-		readl(port_mmio + PORT_CMD);	/* Force a barrier */
-	}
 	return 0;
 }

@@ -300,6 +316,7 @@ static struct ata_port_operations xgene_ahci_ops = {
 	.host_stop = xgene_ahci_host_stop,
 	.hardreset = xgene_ahci_hardreset,
 	.read_id = xgene_ahci_read_id,
+	.qc_issue = xgene_ahci_qc_issue,
 };

 static const struct ata_port_info xgene_ahci_port_info = {
--
1.8.2.1