ata: ahci_xgene: Add support for 2nd HW version of APM X-Gene SoC AHCI SATA Host controller driver.

PATCH Superseded Suman Tripathi 5 participants 13 messages May 3, 2015 – May 5, 2015 1 message not in the archive

Subsystems (2)

SCSI libata

lore

Share & tools

lore

Suman Tripathi

author

2015-05-03 15:33 UTC #0 lore raw

v3 cover ata: ahci_xgene: Add support for 2nd HW version of APM X-Gene SoC AHCI SATA Host controller driver.

V2 change:
* Drop the MIDR based implementation.

V3 change:
* Implement the support for HOST_IRQ_STAT as edge trigger latch.

Signed-off-by Suman Tripathi <[email protected]>

---

Suman Tripathi (2):
libahci: Add support to handle HOST_IRQ_STAT as edge trigger latch.
ata: ahci_xgene: Add AHCI Support for 2nd HW version of APM X-Gene SoC
AHCI SATA Host controller.

 drivers/ata/ahci.h       |  2 ++
 drivers/ata/ahci_xgene.c | 90 ++++++++++++++++++++++++++++++++++++++----------
 drivers/ata/libahci.c    | 19 ++++++++++
 3 files changed, 93 insertions(+), 18 deletions(-)

--
1.8.2.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 1430667220-23477-1-git-send-email-stripathi-qTEPVZfXA3Y@public.gmane.org

Suman Tripathi

author

2015-05-03 15:33 UTC #2 lore raw

PATCH v3 1/2 libahci: Add support to handle HOST_IRQ_STAT as edge trigger latch.

This patch adds the support to handle HOST_IRQ_STAT as edge trigger latch.

Signed-off-by Suman Tripathi <[email protected]>

---
 drivers/ata/ahci.h    |  2 ++
 drivers/ata/libahci.c | 19 +++++++++++++++++++
 2 files changed, 21 insertions(+)

drivers/ata/ahci.h +2 -0

index 71262e0..2df2237 100644
@@ -238,6 +238,8 @@ enum {
 	AHCI_HFLAG_MULTI_MSI		= (1 << 16), /* multiple PCI MSIs */
 	AHCI_HFLAG_NO_DEVSLP		= (1 << 17), /* no device sleep */
 	AHCI_HFLAG_NO_FBS		= (1 << 18), /* no FBS */
+	AHCI_HFLAG_EDGE_TRIG_IRQ	= (1 << 19), /* HOST_IRQ_STAT behaves as
+							Edge Triggered */
 
 	/* ap->flags bits */

drivers/ata/libahci.c +19 -0

index 61a9c07..0e7e8b4 100644
@@ -1879,6 +1879,25 @@ static irqreturn_t ahci_single_irq_intr(int irq, void *dev_instance)
 	 */
 	writel(irq_stat, mmio + HOST_IRQ_STAT);
 
+	/*
+	 * HOST_IRQ_STAT behaves as edge trigger latch. When HOST_IRQ_STAT
+	 * detects a egde from PORT_IRQ_STAT, it happens to loose interrupts
+	 * when interrupts are triggered from both ports. So handling of
+	 * the residual interrupt is required.
+	 */
+	if (hpriv->flags & AHCI_HFLAG_EDGE_TRIG_IRQ) {
+		for (i = 0; i < host->n_ports; i++) {
+			struct ata_port *ap;
+
+			ap = host->ports[i];
+			if (ap) {
+				ahci_port_intr(ap);
+				VPRINTK("Residual irq from port %u\n", i);
+			}
+			handled = 1;
+		}
+	}
+
 	spin_unlock(&host->lock);
 
 	VPRINTK("EXIT\n");

--
1.8.2.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Tejun Heo

2015-05-04 15:47 UTC #4 lore raw

PATCH v3 1/2 libahci: Add support to handle HOST_IRQ_STAT as edge trigger latch.

In reply to Suman Tripathi

Quoted 2 lines · Suman Tripathi

This patch adds the support to handle HOST_IRQ_STAT as edge trigger latch.

...

Quoted 8 lines

drivers/ata/libahci.c +8 -0

+	/*
+	 * HOST_IRQ_STAT behaves as edge trigger latch. When HOST_IRQ_STAT
+	 * detects a egde from PORT_IRQ_STAT, it happens to loose interrupts
+	 * when interrupts are triggered from both ports. So handling of
+	 * the residual interrupt is required.
+	 */
+	if (hpriv->flags & AHCI_HFLAG_EDGE_TRIG_IRQ) {
+		for (i = 0; i < host->n_ports; i++) {

drivers/ata/libahci.c static irqreturn_t ahci_single_irq_intr(int irq, void *dev_instance) · line 1899

Quoted 10 lines

drivers/ata/libahci.c +10 -0

+			struct ata_port *ap;
+
+			ap = host->ports[i];
+			if (ap) {
+				ahci_port_intr(ap);
+				VPRINTK("Residual irq from port %u\n", i);
+			}
+			handled = 1;
+		}
+	}

This is kinda gross. The right thing do is clearing irq stat registers before handling the events, right? That shouldn't be too difficult to implement. Create a separate set of irq functions which clear irqs before processing rather than after.

Thanks.

Signature · 1 line

tejun

Suman Tripathi

2015-05-04 16:43 UTC #5 lore raw

PATCH v3 1/2 libahci: Add support to handle HOST_IRQ_STAT as edge trigger latch.

Quoted 23 lines · Tejun Heo

Quoted 2 lines · Suman Tripathi

This patch adds the support to handle HOST_IRQ_STAT as edge trigger latch.

...

Quoted 8 lines

drivers/ata/libahci.c +8 -0

+     /*
+      * HOST_IRQ_STAT behaves as edge trigger latch. When HOST_IRQ_STAT
+      * detects a egde from PORT_IRQ_STAT, it happens to loose interrupts
+      * when interrupts are triggered from both ports. So handling of
+      * the residual interrupt is required.
+      */
+     if (hpriv->flags & AHCI_HFLAG_EDGE_TRIG_IRQ) {
+             for (i = 0; i < host->n_ports; i++) {

drivers/ata/libahci.c static irqreturn_t ahci_single_irq_intr(int irq, void *dev_instance) · line 1899

Quoted 10 lines

drivers/ata/libahci.c +10 -0

+                     struct ata_port *ap;
+
+                     ap = host->ports[i];
+                     if (ap) {
+                             ahci_port_intr(ap);
+                             VPRINTK("Residual irq from port %u\n", i);
+                     }
+                     handled = 1;
+             }
+     }

Quoted 4 lines

This is kinda gross. The right thing do is clearing irq stat registers before handling the events, right? That shouldn't be too difficult to implement. Create a separate set of irq functions which clear irqs before processing rather than after.

AFAIK clearing host_irq_stat means we have handled port interrupts . Now for our case we still have interrupts left because it didn't get detected on
first ahci_port_intr. So you mean to handle that residual irq in the next cycle (i mean next call intr handler ) ??

Quoted 5 lines

Thanks.
--
tejun

Signature · 3 lines

Thanks,
with regards,
Suman Tripathi

Tejun Heo

2015-05-04 18:26 UTC #6 lore raw

PATCH v3 1/2 libahci: Add support to handle HOST_IRQ_STAT as edge trigger latch.

Hello,

Quoted 5 lines · Suman Tripathi

AFAIK clearing host_irq_stat means we have handled port interrupts . Now for our case we still have interrupts left because it didn't get detected on
first ahci_port_intr. So you mean to handle that residual irq in the next cycle (i mean next call intr handler ) ??

Heh, I think we're talking past each other. For level triggered IRQs, the latched IRQ bits should be cleared after handling the events; otherwise, the latched bits are gonna get set immediately as the events are still pending. Also, this doesn't lose any events as they're level triggered latches - if any event is pending, the IRQ is gonna be raised again.

Edge triggered latches are the other way around. You should clear the latches before actually handling and clearing the events. The pending events won't trigger the latches again as it's edge-triggered and the events which happens after this irq handling starts won't get lost as they'll be latched for the next round.

Thanks.

Signature · 1 line

tejun

Suman Tripathi

2015-05-05 05:55 UTC #7 lore raw

PATCH v3 1/2 libahci: Add support to handle HOST_IRQ_STAT as edge trigger latch.

Hi Tejun,

Quoted 16 lines · Tejun Heo

Hello,
Quoted 5 lines · Suman Tripathi
AFAIK clearing host_irq_stat means we have handled port interrupts . Now for our case we still have interrupts left because it didn't get detected on
first ahci_port_intr. So you mean to handle that residual irq in the next cycle (i mean next call intr handler ) ??
Heh, I think we're talking past each other. For level triggered IRQs, the latched IRQ bits should be cleared after handling the events; otherwise, the latched bits are gonna get set immediately as the events are still pending. Also, this doesn't lose any events as they're level triggered latches - if any event is pending, the IRQ is gonna be raised again.

Quoted 5 lines

Edge triggered latches are the other way around. You should clear the latches before actually handling and clearing the events. The pending events won't trigger the latches again as it's edge-triggered and the events which happens after this irq handling starts won't get lost as they'll be latched for the next round.

Yeah it's very simple. The clearing of irq_stat needs to move up. That's it. tried and it worked. Thanks a lot !!
Just a quick question : Why edge trigger handling is not yet present in libahci yet? I mean no one has used it yet ??

Quoted 5 lines

Thanks.
--
tejun

Signature · 3 lines

Thanks,
with regards,
Suman Tripathi

Tejun Heo

2015-05-05 13:18 UTC #8 lore raw

PATCH v3 1/2 libahci: Add support to handle HOST_IRQ_STAT as edge trigger latch.

Quoted 4 lines · Suman Tripathi

Yeah it's very simple. The clearing of irq_stat needs to move up. That's it. tried and it worked. Thanks a lot !!
Just a quick question : Why edge trigger handling is not yet present in libahci yet? I mean no one has used it yet ??

Yeah, no other ahci controller uses edge triggered latches. I think ahci spec says irq is supposed to be level triggered.

Thanks.

Signature · 1 line

tejun

Suman Tripathi

author

2015-05-03 15:33 UTC #9 lore raw

PATCH v3 2/2 ata: ahci_xgene: Add AHCI Support for 2nd HW version of APM X-Gene SoC AHCI SATA Host controller.

In reply to a message not in the archive

This patch enables full AHCI feature support for APM X-Gene SoC SATA host controller. The following errata's are removed:

1. 2a0bdff6b95 ("ahci-xgene: fix the dma state machine lockup for the IDENTIFY DEVICE PIO mode command")
2. 09c32aaa368 ("ahci_xgene: Fix the dma state machine lockup for the ATA_CMD_SMART PIO mode command")
3. 1540035da71 ("ahci_xgene: Implement the xgene_ahci_poll_reg_val to support PMP")
4. a3a84bc7c88 ("ahci_xgene: Implement the workaround to support PMP enumeration and discovery")
5. 1102407bb71 ("ahci_xgene: Fix the DMA state machine lockup for the ATA_CMD_PACKET PIO mode command")
6. 72f79f9e35b ("ahci_xgene: Removing NCQ support from the APM X-Gene SoC AHCI SATA Host Controller driver")

In addition, enable PMP support for APM X-Gene SoC and enable FBS support for second generation APM X-Gene SoC.

Signed-off-by Suman Tripathi <[email protected]>

---
 drivers/ata/ahci_xgene.c | 90 ++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 72 insertions(+), 18 deletions(-)

drivers/ata/ahci_xgene.c +72 ~~-18~~

index 2b78510..2ced663 100644
@@ -27,6 +27,7 @@
 #include <linux/platform_device.h>
 #include <linux/ahci_platform.h>
 #include <linux/of_address.h>
+#include <linux/of_device.h>
 #include <linux/of_irq.h>
 #include <linux/phy/phy.h>
 #include "ahci.h"

@@ -84,6 +85,11 @@
 /* Max retry for link down */
 #define MAX_LINK_DOWN_RETRY 3
 
+enum xgene_ahci_version {
+	XGENE_AHCI_V1 = 1,
+	XGENE_AHCI_V2,
+};
+
 struct xgene_ahci_context {
 	struct ahci_host_priv *hpriv;
 	struct device *dev;

@@ -547,9 +553,6 @@ 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,
-	.softreset = xgene_ahci_softreset,
-	.pmp_softreset = xgene_ahci_pmp_softreset
 };
 
 static const struct ata_port_info xgene_ahci_port_info = {

@@ -629,12 +632,30 @@ static struct scsi_host_template ahci_platform_sht = {
 	AHCI_SHT(DRV_NAME),
 };
 
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id xgene_ahci_acpi_match[] = {
+	{ "APMC0D0D", XGENE_AHCI_V1},
+	{ "APMC0D32", XGENE_AHCI_V2},
+	{},
+};
+MODULE_DEVICE_TABLE(acpi, xgene_ahci_acpi_match);
+#endif
+
+static const struct of_device_id xgene_ahci_of_match[] = {
+	{.compatible = "apm,xgene-ahci", .data = (void *) XGENE_AHCI_V1},
+	{.compatible = "apm,xgene-ahci-v2", .data = (void *) XGENE_AHCI_V2},
+	{},
+};
+MODULE_DEVICE_TABLE(of, xgene_ahci_of_match);
+
 static int xgene_ahci_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct ahci_host_priv *hpriv;
 	struct xgene_ahci_context *ctx;
 	struct resource *res;
+	const struct of_device_id *of_devid;
+	enum xgene_ahci_version version = XGENE_AHCI_V1;
 	int rc;
 
 	hpriv = ahci_platform_get_resources(pdev);

@@ -677,6 +698,37 @@ static int xgene_ahci_probe(struct platform_device *pdev)
 		ctx->csr_mux = csr;
 	}
 
+	of_devid = of_match_device(xgene_ahci_of_match, dev);
+	if (of_devid) {
+		if (of_devid->data)
+			version = (enum xgene_ahci_version) of_devid->data;
+	}
+#ifdef CONFIG_ACPI
+	else {
+		const struct acpi_device_id *acpi_id;
+		struct acpi_device_info *info;
+		acpi_status status;
+
+		acpi_id = acpi_match_device(xgene_ahci_acpi_match, &pdev->dev);
+		if (!acpi_id) {
+			dev_warn(&pdev->dev, "No node entry in ACPI table. Assume version1\n");
+			version = XGENE_AHCI_V1;
+		}
+
+		if (acpi_id->driver_data) {
+			version = (enum xgene_ahci_version) acpi_id->driver_data;
+			status = acpi_get_object_info(ACPI_HANDLE(&pdev->dev), &info);
+			if (ACPI_FAILURE(status)) {
+				dev_warn(&pdev->dev, "%s: Error reading device info. Assume version1\n",
+					__func__);
+				version = XGENE_AHCI_V1;
+			}
+			if (info->valid & ACPI_VALID_CID)
+				version = XGENE_AHCI_V2;
+		}
+	}
+#endif
+
 	dev_dbg(dev, "VAddr 0x%p Mmio VAddr 0x%p\n", ctx->csr_core,
 		hpriv->mmio);

@@ -704,7 +756,23 @@ static int xgene_ahci_probe(struct platform_device *pdev)
 	/* Configure the host controller */
 	xgene_ahci_hw_init(hpriv);
 skip_clk_phy:
-	hpriv->flags = AHCI_HFLAG_NO_PMP | AHCI_HFLAG_NO_NCQ;
+
+	switch (version) {
+	case XGENE_AHCI_V1:
+		hpriv->flags = AHCI_HFLAG_NO_NCQ;
+		/*
+		 * Override the callbacks for storm ERRATA
+		 */
+		xgene_ahci_ops.qc_issue = xgene_ahci_qc_issue;
+		xgene_ahci_ops.softreset = xgene_ahci_softreset;
+		xgene_ahci_ops.pmp_softreset = xgene_ahci_pmp_softreset;
+		break;
+	case XGENE_AHCI_V2:
+		hpriv->flags |= AHCI_HFLAG_YES_FBS | AHCI_HFLAG_EDGE_TRIG_IRQ;
+		break;
+	default:
+		break;
+	}
 
 	rc = ahci_platform_init_host(pdev, hpriv, &xgene_ahci_port_info,
 				     &ahci_platform_sht);

@@ -719,20 +787,6 @@ disable_resources:
 	return rc;
 }
 
-#ifdef CONFIG_ACPI
-static const struct acpi_device_id xgene_ahci_acpi_match[] = {
-	{ "APMC0D0D", },
-	{ }
-};
-MODULE_DEVICE_TABLE(acpi, xgene_ahci_acpi_match);
-#endif
-
-static const struct of_device_id xgene_ahci_of_match[] = {
-	{.compatible = "apm,xgene-ahci"},
-	{},
-};
-MODULE_DEVICE_TABLE(of, xgene_ahci_of_match);
-
 static struct platform_driver xgene_ahci_driver = {
 	.probe = xgene_ahci_probe,
 	.remove = ata_platform_remove_one,

--
1.8.2.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Julian Calaby

2015-05-04 00:26 UTC #10 lore raw

PATCH v3 2/2 ata: ahci_xgene: Add AHCI Support for 2nd HW version of APM X-Gene SoC AHCI SATA Host controller.

Hi Suman,

Quoted 15 lines · Suman Tripathi

This patch enables full AHCI feature support for APM X-Gene SoC SATA host controller. The following errata's are removed:
1. 2a0bdff6b95 ("ahci-xgene: fix the dma state machine lockup for the IDENTIFY DEVICE PIO mode command")
2. 09c32aaa368 ("ahci_xgene: Fix the dma state machine lockup for the ATA_CMD_SMART PIO mode command")
3. 1540035da71 ("ahci_xgene: Implement the xgene_ahci_poll_reg_val to support PMP")
4. a3a84bc7c88 ("ahci_xgene: Implement the workaround to support PMP enumeration and discovery")
5. 1102407bb71 ("ahci_xgene: Fix the DMA state machine lockup for the ATA_CMD_PACKET PIO mode command")
6. 72f79f9e35b ("ahci_xgene: Removing NCQ support from the APM X-Gene SoC AHCI SATA Host Controller driver")

Won't this mean that the 1st HW version won't work properly?

Thanks,

Signature · 4 lines

Julian Calaby

Email: julian.calaby at gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

Suman Tripathi

2015-05-04 02:51 UTC #11 lore raw

PATCH v3 2/2 ata: ahci_xgene: Add AHCI Support for 2nd HW version of APM X-Gene SoC AHCI SATA Host controller.

Hi Julian.

Quoted 19 lines · Julian Calaby

Hi Suman,
Quoted 15 lines · Suman Tripathi
This patch enables full AHCI feature support for APM X-Gene SoC SATA host controller. The following errata's are removed:
1. 2a0bdff6b95 ("ahci-xgene: fix the dma state machine lockup for the IDENTIFY DEVICE PIO mode command")
2. 09c32aaa368 ("ahci_xgene: Fix the dma state machine lockup for the ATA_CMD_SMART PIO mode command")
3. 1540035da71 ("ahci_xgene: Implement the xgene_ahci_poll_reg_val to support PMP")
4. a3a84bc7c88 ("ahci_xgene: Implement the workaround to support PMP enumeration and discovery")
5. 1102407bb71 ("ahci_xgene: Fix the DMA state machine lockup for the ATA_CMD_PACKET PIO mode command")
6. 72f79f9e35b ("ahci_xgene: Removing NCQ support from the APM X-Gene SoC AHCI SATA Host Controller driver")

Quoted 1 line

Won't this mean that the 1st HW version won't work properly?

No it doesn't mean that, It means whatever workaround we had in 1st HW version has been
fixed in 2nd HW version.

Quoted 8 lines

Thanks,
--
Julian Calaby
Email: julian.calaby at gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

Signature · 3 lines

Thanks,
with regards,
Suman Tripathi

Julian Calaby

2015-05-04 03:22 UTC #12 lore raw

PATCH v3 2/2 ata: ahci_xgene: Add AHCI Support for 2nd HW version of APM X-Gene SoC AHCI SATA Host controller.

Hi Suman,

Quoted 22 lines · Suman Tripathi

Hi Julian.
Quoted 19 lines · Julian Calaby
Hi Suman,
Quoted 15 lines · Suman Tripathi
This patch enables full AHCI feature support for APM X-Gene SoC SATA host controller. The following errata's are removed:
1. 2a0bdff6b95 ("ahci-xgene: fix the dma state machine lockup for the IDENTIFY DEVICE PIO mode command")
2. 09c32aaa368 ("ahci_xgene: Fix the dma state machine lockup for the ATA_CMD_SMART PIO mode command")
3. 1540035da71 ("ahci_xgene: Implement the xgene_ahci_poll_reg_val to support PMP")
4. a3a84bc7c88 ("ahci_xgene: Implement the workaround to support PMP enumeration and discovery")
5. 1102407bb71 ("ahci_xgene: Fix the DMA state machine lockup for the ATA_CMD_PACKET PIO mode command")
6. 72f79f9e35b ("ahci_xgene: Removing NCQ support from the APM X-Gene SoC AHCI SATA Host Controller driver")

Quoted 5 lines

Quoted 1 line
Won't this mean that the 1st HW version won't work properly?
No it doesn't mean that, It means whatever workaround we had in 1st HW version has been
fixed in 2nd HW version.

I'm sorry, I read the description and assumed that these fixes were being removed from the driver, hence breaking support for the 1st hardware version.

Sorry for the confusion.

Thanks,

Signature · 4 lines

Julian Calaby

Email: julian.calaby at gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

Suman Tripathi

2015-05-04 04:33 UTC #13 lore raw

PATCH v3 2/2 ata: ahci_xgene: Add AHCI Support for 2nd HW version of APM X-Gene SoC AHCI SATA Host controller.

Hi Julian,

Quoted 31 lines · Julian Calaby

Hi Suman,
Quoted 22 lines · Suman Tripathi
Hi Julian.
Quoted 19 lines · Julian Calaby
Hi Suman,
Quoted 15 lines · Suman Tripathi
This patch enables full AHCI feature support for APM X-Gene SoC SATA host controller. The following errata's are removed:
1. 2a0bdff6b95 ("ahci-xgene: fix the dma state machine lockup for the IDENTIFY DEVICE PIO mode command")
2. 09c32aaa368 ("ahci_xgene: Fix the dma state machine lockup for the ATA_CMD_SMART PIO mode command")
3. 1540035da71 ("ahci_xgene: Implement the xgene_ahci_poll_reg_val to support PMP")
4. a3a84bc7c88 ("ahci_xgene: Implement the workaround to support PMP enumeration and discovery")
5. 1102407bb71 ("ahci_xgene: Fix the DMA state machine lockup for the ATA_CMD_PACKET PIO mode command")
6. 72f79f9e35b ("ahci_xgene: Removing NCQ support from the APM X-Gene SoC AHCI SATA Host Controller driver")
Quoted 5 lines
Quoted 1 line
Won't this mean that the 1st HW version won't work properly?
No it doesn't mean that, It means whatever workaround we had in 1st HW version has been
fixed in 2nd HW version.

Quoted 5 lines

I'm sorry, I read the description and assumed that these fixes were being removed from the driver, hence breaking support for the 1st hardware version.
Sorry for the confusion.

No probs ..

Quoted 8 lines

Thanks,
--
Julian Calaby
Email: julian.calaby at gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

Signature · 3 lines

Thanks,
with regards,
Suman Tripathi