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

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

[Suman Tripathi]

Suman Tripathi (2):
  libahci: Implement the restart_engine to restart the port dma engine.
  ata: Fix the dma state machine lockup for the PIO mode commands.

 drivers/ata/ahci_xgene.c  | 17 +++--------------
 drivers/ata/libahci.c     | 13 +++++++++++++
 drivers/ata/libata-core.c | 10 ++++++++++
 include/linux/libata.h    |  3 +++
 4 files changed, 29 insertions(+), 14 deletions(-)

--
1.8.2.1

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

[Suman Tripathi]

This patch implements the function restart_engine function to add the flexibility to  restart the port dma engine from the libata framework.

Signed-off-by: Loc Ho <lho@apm.com>
Signed-off-by: Suman Tripathi <stripathi@apm.com>
---
 drivers/ata/libahci.c  | 11 +++++++++++
 include/linux/libata.h |  1 +
 2 files changed, 12 insertions(+)

diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index b986145..5f9a13e 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -70,6 +70,7 @@ 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_restart_engine(struct ata_port *ap);
 static int ahci_port_start(struct ata_port *ap);
 static void ahci_port_stop(struct ata_port *ap);
 static void ahci_qc_prep(struct ata_queued_cmd *qc);
@@ -178,6 +179,7 @@ struct ata_port_operations ahci_ops = {
 #endif
 	.port_start		= ahci_port_start,
 	.port_stop		= ahci_port_stop,
+	.restart_engine		= ahci_restart_engine,
 };
 EXPORT_SYMBOL_GPL(ahci_ops);

@@ -2362,6 +2364,15 @@ static int ahci_port_start(struct ata_port *ap)
 	return ahci_port_resume(ap);
 }

+static int ahci_restart_engine(struct ata_port *ap)
+{
+	ahci_stop_engine(ap);
+	ahci_start_fis_rx(ap);
+	ahci_start_engine(ap);
+
+	return 0;
+}
+
 static void ahci_port_stop(struct ata_port *ap)
 {
 	const char *emsg = NULL;
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 5ab4e3a..42ab826 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -918,6 +918,7 @@ struct ata_port_operations {
 	void (*pmp_detach)(struct ata_port *ap);
 	int  (*set_lpm)(struct ata_link *link, enum ata_lpm_policy policy,
 			unsigned hints);
+	int (*restart_engine) (struct ata_port *ap);

 	/*
 	 * Start, stop, suspend and resume
--
1.8.2.1

[PATCH v0 2/2] ata: Fix the dma state machine lockup for the PIO mode commands.

[Suman Tripathi]

This patches fixes the dma state machine lockup due to the PIO mode
commands. The contoller is unable to clear the BSY bit after receiving the
PIO Setup FIS and results the dma state machine to go into the
CMFatalErrorUpdate state resulting in dma state machine lockup.

Signed-off-by: Loc Ho <lho@apm.com>
Signed-off-by: Suman Tripathi <stripathi@apm.com>
---
 drivers/ata/ahci_xgene.c  | 17 +++--------------
 drivers/ata/libata-core.c | 10 ++++++++++
 include/linux/libata.h    |  2 ++
 3 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/ata/ahci_xgene.c b/drivers/ata/ahci_xgene.c
index 77c89bf..ed6c60c 100644
--- a/drivers/ata/ahci_xgene.c
+++ b/drivers/ata/ahci_xgene.c
@@ -104,14 +104,12 @@ static int xgene_ahci_init_memram(struct xgene_ahci_context *ctx)
  * @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 +131,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;
 }

@@ -304,7 +292,8 @@ static struct ata_port_operations xgene_ahci_ops = {

 static const struct ata_port_info xgene_ahci_port_info = {
 	AHCI_HFLAGS(AHCI_HFLAG_NO_PMP | AHCI_HFLAG_YES_NCQ),
-	.flags = AHCI_FLAG_COMMON | ATA_FLAG_NCQ,
+	.flags = AHCI_FLAG_COMMON | ATA_FLAG_NCQ |
+		 ATA_HORKAGE_BROKEN_PIO_CMD,
 	.pio_mask = ATA_PIO4,
 	.udma_mask = ATA_UDMA6,
 	.port_ops = &xgene_ahci_ops,
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 18d97d5..a893ed8 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -5072,6 +5072,16 @@ int ata_qc_complete_multiple(struct ata_port *ap, u32 qc_active)
 		if (qc) {
 			ata_qc_complete(qc);
 			nr_done++;
+			/*
+			 * Some controller unable to clear the BSY bit after
+			 * receiving the PIO Setup FIS from device resulting
+			 * the DMA state to go into CMFatalErrorUpdate state.
+			 * So need to restart the dma engine to get the
+			 * controller out of this state.
+			 */
+			if ((ap->flags & ATA_HORKAGE_BROKEN_PIO_CMD) &&
+			    (qc->tf.protocol == ATA_PROT_PIO))
+				ap->ops->restart_engine(ap);
 		}
 		done_mask &= ~(1 << tag);
 	}
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 42ab826..d16bbfa 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -421,6 +421,8 @@ enum {
 	ATA_HORKAGE_NO_NCQ_TRIM	= (1 << 19),	/* don't use queued TRIM */
 	ATA_HORKAGE_NOLPM	= (1 << 20),	/* don't use LPM */
 	ATA_HORKAGE_WD_BROKEN_LPM = (1 << 21),	/* some WDs have broken LPM */
+	ATA_HORKAGE_BROKEN_PIO_CMD = (1 << 22), /* PIO cmds resulting in
+						   HBA dma state lockup */

 	 /* DMA mask for user DMA control: User visible values; DO NOT
 	    renumber */
--
1.8.2.1

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

[Tejun Heo]

On Sat, Jun 07, 2014 at 02:58:53AM +0530, Suman Tripathi wrote:
> This patch implements the function restart_engine function to add the flexibility to  restart the port dma engine from the libata framework.
> 
> Signed-off-by: Loc Ho <lho@apm.com>
> Signed-off-by: Suman Tripathi <stripathi@apm.com>
> ---
>  drivers/ata/libahci.c  | 11 +++++++++++
>  include/linux/libata.h |  1 +
>  2 files changed, 12 insertions(+)
> 
> diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
> index b986145..5f9a13e 100644
> --- a/drivers/ata/libahci.c
> +++ b/drivers/ata/libahci.c
> @@ -70,6 +70,7 @@ 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_restart_engine(struct ata_port *ap);
>  static int ahci_port_start(struct ata_port *ap);
>  static void ahci_port_stop(struct ata_port *ap);
>  static void ahci_qc_prep(struct ata_queued_cmd *qc);
> @@ -178,6 +179,7 @@ struct ata_port_operations ahci_ops = {
>  #endif
>  	.port_start		= ahci_port_start,
>  	.port_stop		= ahci_port_stop,
> +	.restart_engine		= ahci_restart_engine,

Umm... you're adding something very ahci-specific as general libata
callback.  I haven't looked at the second patch yet but don't think
this is gonna work out.

Thanks.

-- 
tejun

Re: [PATCH v0 2/2] ata: Fix the dma state machine lockup for the PIO mode commands.

[Tejun Heo]

On Sat, Jun 07, 2014 at 02:58:54AM +0530, Suman Tripathi wrote:
> @@ -5072,6 +5072,16 @@ int ata_qc_complete_multiple(struct ata_port *ap, u32 qc_active)
>  		if (qc) {
>  			ata_qc_complete(qc);
>  			nr_done++;
> +			/*
> +			 * Some controller unable to clear the BSY bit after
> +			 * receiving the PIO Setup FIS from device resulting
> +			 * the DMA state to go into CMFatalErrorUpdate state.
> +			 * So need to restart the dma engine to get the
> +			 * controller out of this state.
> +			 */
> +			if ((ap->flags & ATA_HORKAGE_BROKEN_PIO_CMD) &&
> +			    (qc->tf.protocol == ATA_PROT_PIO))
> +				ap->ops->restart_engine(ap);

Why aren't you doing this by wrapping irq handler?

Thanks.

-- 
tejun