[PATCH v3 0/2] ata: ahci_xgene: Add support for 2nd HW version of APM X-Gene SoC AHCI SATA Host controller driver.

[PATCH v3 0/2] ata: ahci_xgene: Add support for 2nd HW version of APM X-Gene SoC AHCI SATA Host controller driver.

[Suman Tripathi]

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 <stripathi-qTEPVZfXA3Y@public.gmane.org>
---

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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Suman Tripathi]

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

Signed-off-by: Suman Tripathi <stripathi-qTEPVZfXA3Y@public.gmane.org>
---
 drivers/ata/ahci.h    |  2 ++
 drivers/ata/libahci.c | 19 +++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index 71262e0..2df2237 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -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 */

diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index 61a9c07..0e7e8b4 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[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")

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 <stripathi-qTEPVZfXA3Y@public.gmane.org>
---
 drivers/ata/ahci_xgene.c | 90 ++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 72 insertions(+), 18 deletions(-)

diff --git a/drivers/ata/ahci_xgene.c b/drivers/ata/ahci_xgene.c
index 2b78510..2ced663 100644
--- a/drivers/ata/ahci_xgene.c
+++ b/drivers/ata/ahci_xgene.c
@@ -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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Julian Calaby]

Hi Suman,

On Mon, May 4, 2015 at 1:33 AM, Suman Tripathi <stripathi@apm.com> wrote:
> 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,

-- 
Julian Calaby

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

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

[Suman Tripathi]

Hi Julian.

On Mon, May 4, 2015 at 5:56 AM, Julian Calaby <julian.calaby@gmail.com> wrote:
> Hi Suman,
>
> On Mon, May 4, 2015 at 1:33 AM, Suman Tripathi <stripathi@apm.com> wrote:
>> 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?

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

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



-- 
Thanks,
with regards,
Suman Tripathi

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

[Julian Calaby]

Hi Suman,

On Mon, May 4, 2015 at 12:51 PM, Suman Tripathi <stripathi@apm.com> wrote:
> Hi Julian.
>
> On Mon, May 4, 2015 at 5:56 AM, Julian Calaby <julian.calaby@gmail.com> wrote:
>> Hi Suman,
>>
>> On Mon, May 4, 2015 at 1:33 AM, Suman Tripathi <stripathi@apm.com> wrote:
>>> 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?
>
> 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,

-- 
Julian Calaby

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

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

[Suman Tripathi]

Hi Julian,

On Mon, May 4, 2015 at 8:52 AM, Julian Calaby <julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Hi Suman,
>
> On Mon, May 4, 2015 at 12:51 PM, Suman Tripathi <stripathi-qTEPVZfXA3Y@public.gmane.org> wrote:
>> Hi Julian.
>>
>> On Mon, May 4, 2015 at 5:56 AM, Julian Calaby <julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> Hi Suman,
>>>
>>> On Mon, May 4, 2015 at 1:33 AM, Suman Tripathi <stripathi-qTEPVZfXA3Y@public.gmane.org> wrote:
>>>> 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?
>>
>> 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.

No probs ..

>
> Thanks,
>
> --
> Julian Calaby
>
> Email: julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
> Profile: http://www.google.com/profiles/julian.calaby/



-- 
Thanks,
with regards,
Suman Tripathi
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Sergei Shtylyov]

Hello.

On 5/3/2015 6:33 PM, Suman Tripathi wrote:

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

> Signed-off-by: Suman Tripathi <stripathi@apm.com>
> ---
>   drivers/ata/ahci.h    |  2 ++
>   drivers/ata/libahci.c | 19 +++++++++++++++++++
>   2 files changed, 21 insertions(+)

> diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
> index 71262e0..2df2237 100644
> --- a/drivers/ata/ahci.h
> +++ b/drivers/ata/ahci.h
[...]
> +++ b/drivers/ata/libahci.c
> @@ -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

    s/loose/lose/.

MBR, Sergei

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

[Tejun Heo]

On Sun, May 03, 2015 at 09:03:39PM +0530, Suman Tripathi wrote:
> This patch adds the support to handle HOST_IRQ_STAT as edge trigger
> latch.
...
> +	/*
> +	 * 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;
> +		}
> +	}

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.

-- 
tejun

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

[Suman Tripathi]

On Mon, May 4, 2015 at 9:17 PM, Tejun Heo <tj@kernel.org> wrote:
> On Sun, May 03, 2015 at 09:03:39PM +0530, Suman Tripathi wrote:
>> This patch adds the support to handle HOST_IRQ_STAT as edge trigger
>> latch.
> ...
>> +     /*
>> +      * 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;
>> +             }
>> +     }
>
> 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 ) ??

>
> Thanks.
>
> --
> tejun



-- 
Thanks,
with regards,
Suman Tripathi

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

[Tejun Heo]

Hello,

On Mon, May 04, 2015 at 10:13:11PM +0530, Suman Tripathi wrote:
> 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.

-- 
tejun

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

[Suman Tripathi]

Hi Tejun,

On Mon, May 4, 2015 at 11:56 PM, Tejun Heo <tj@kernel.org> wrote:
> Hello,
>
> On Mon, May 04, 2015 at 10:13:11PM +0530, Suman Tripathi wrote:
>> 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.

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 ??

>
> Thanks.
>
> --
> tejun



-- 
Thanks,
with regards,
Suman Tripathi

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

[Tejun Heo]

On Tue, May 05, 2015 at 11:25:14AM +0530, Suman Tripathi wrote:
> 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.

-- 
tejun