[PATCH v3 0/6] Xilinx AXI PCIe Host Bridge driver fixes

[PATCH v3 0/6] Xilinx AXI PCIe Host Bridge driver fixes

[Paul Burton]

This series fixes a number of issues found using the Xilinx AXI PCIe
Host Bridge IP on the Imagination Technologies MIPS Boston development
board. It has been split out of the larger Boston board support series
at Michal's request.

Applies atop v4.5-rc2.

Paul Burton (6):
  PCI: xilinx: Keep references to both IRQ domains
  PCI: xilinx: Unify INTx & MSI interrupt FIFO decode
  PCI: xilinx: Always clear interrupt decode register
  PCI: xilinx: Clear interrupt FIFO during probe
  PCI: xilinx: Fix INTX irq dispatch
  PCI: xilinx: Allow build on MIPS platforms

 drivers/pci/host/Kconfig       |   2 +-
 drivers/pci/host/pcie-xilinx.c | 125 +++++++++++++++++++----------------------
 2 files changed, 60 insertions(+), 67 deletions(-)

-- 
2.7.0

[PATCH v3 1/6] PCI: xilinx: Keep references to both IRQ domains

[Paul Burton]

pcie-xilinx creates 2 IRQ domains when built with MSI support: one for
MSI interrupts & one for legacy INTx interrupts. However, it only kept a
reference to the MSI IRQ domain. This means that any INTx interrupts
that may occur would be mapped using the wrong domain, and that only the
MSI IRQ domain would be removed along with the driver. Track both IRQ
domains & clean up both as appropriate.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Fixes: 8961def56845 ("PCI: xilinx: Add Xilinx AXI PCIe Host Bridge IP driver")

---

Changes in v3:
- Split out from Boston patchset.

Changes in v2:
- Add Fixes tag.

 drivers/pci/host/pcie-xilinx.c | 58 ++++++++++++++++++++----------------------
 1 file changed, 28 insertions(+), 30 deletions(-)

diff --git a/drivers/pci/host/pcie-xilinx.c b/drivers/pci/host/pcie-xilinx.c
index 4cfa463..1490bd1 100644
--- a/drivers/pci/host/pcie-xilinx.c
+++ b/drivers/pci/host/pcie-xilinx.c
@@ -105,6 +105,7 @@
  * @root_busno: Root Bus number
  * @dev: Device pointer
  * @irq_domain: IRQ domain pointer
+ * @msi_irq_domain: MSI IRQ domain pointer
  * @bus_range: Bus range
  * @resources: Bus Resources
  */
@@ -115,6 +116,7 @@ struct xilinx_pcie_port {
 	u8 root_busno;
 	struct device *dev;
 	struct irq_domain *irq_domain;
+	struct irq_domain *msi_irq_domain;
 	struct resource bus_range;
 	struct list_head resources;
 };
@@ -291,7 +293,7 @@ static int xilinx_pcie_msi_setup_irq(struct msi_controller *chip,
 	if (hwirq < 0)
 		return hwirq;
 
-	irq = irq_create_mapping(port->irq_domain, hwirq);
+	irq = irq_create_mapping(port->msi_irq_domain, hwirq);
 	if (!irq)
 		return -EINVAL;
 
@@ -517,31 +519,21 @@ static irqreturn_t xilinx_pcie_intr_handler(int irq, void *data)
 
 /**
  * xilinx_pcie_free_irq_domain - Free IRQ domain
- * @port: PCIe port information
+ * @domain: the IRQ domain to free
+ * @nr: the number of IRQs in the domain
  */
-static void xilinx_pcie_free_irq_domain(struct xilinx_pcie_port *port)
+static void xilinx_pcie_free_irq_domain(struct irq_domain *domain, int nr)
 {
 	int i;
-	u32 irq, num_irqs;
-
-	/* Free IRQ Domain */
-	if (IS_ENABLED(CONFIG_PCI_MSI)) {
-
-		free_pages(port->msi_pages, 0);
-
-		num_irqs = XILINX_NUM_MSI_IRQS;
-	} else {
-		/* INTx */
-		num_irqs = 4;
-	}
+	u32 irq;
 
-	for (i = 0; i < num_irqs; i++) {
-		irq = irq_find_mapping(port->irq_domain, i);
+	for (i = 0; i < nr; i++) {
+		irq = irq_find_mapping(domain, i);
 		if (irq > 0)
 			irq_dispose_mapping(irq);
 	}
 
-	irq_domain_remove(port->irq_domain);
+	irq_domain_remove(domain);
 }
 
 /**
@@ -571,20 +563,20 @@ static int xilinx_pcie_init_irq_domain(struct xilinx_pcie_port *port)
 		return PTR_ERR(port->irq_domain);
 	}
 
-	/* Setup MSI */
-	if (IS_ENABLED(CONFIG_PCI_MSI)) {
-		port->irq_domain = irq_domain_add_linear(node,
-							 XILINX_NUM_MSI_IRQS,
-							 &msi_domain_ops,
-							 &xilinx_pcie_msi_chip);
-		if (!port->irq_domain) {
-			dev_err(dev, "Failed to get a MSI IRQ domain\n");
-			return PTR_ERR(port->irq_domain);
-		}
+	if (!IS_ENABLED(CONFIG_PCI_MSI))
+		return 0;
 
-		xilinx_pcie_enable_msi(port);
+	/* Setup MSI */
+	port->msi_irq_domain = irq_domain_add_linear(node,
+						     XILINX_NUM_MSI_IRQS,
+						     &msi_domain_ops,
+						     &xilinx_pcie_msi_chip);
+	if (!port->msi_irq_domain) {
+		dev_err(dev, "Failed to get a MSI IRQ domain\n");
+		return PTR_ERR(port->msi_irq_domain);
 	}
 
+	xilinx_pcie_enable_msi(port);
 	return 0;
 }
 
@@ -869,7 +861,13 @@ static int xilinx_pcie_remove(struct platform_device *pdev)
 {
 	struct xilinx_pcie_port *port = platform_get_drvdata(pdev);
 
-	xilinx_pcie_free_irq_domain(port);
+	xilinx_pcie_free_irq_domain(port->irq_domain, 4);
+
+	if (config_enabled(CONFIG_MSI)) {
+		free_pages(port->msi_pages, 0);
+		xilinx_pcie_free_irq_domain(port->msi_irq_domain,
+					    XILINX_NUM_MSI_IRQS);
+	}
 
 	return 0;
 }
-- 
2.7.0

[PATCH v3 2/6] PCI: xilinx: Unify INTx & MSI interrupt FIFO decode

[Paul Burton]

When decoding either an INTx or MSI interrupt, the driver has no way to
know which it will pull out of the interrupt FIFO. If both were pending
then this would lead to either the interrupt being handled incorrectly
(MSI interrupt treated as INTx) or not at all (INTx interrupt dropped by
MSI path). Unify the reading of the interrupt FIFO & act according to
the type of interrupt actually read.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Fixes: 8961def56845 ("PCI: xilinx: Add Xilinx AXI PCIe Host Bridge IP driver")

---

Changes in v3:
- Split out from Boston patchset.

Changes in v2:
- Add Fixes tag.

 drivers/pci/host/pcie-xilinx.c | 47 +++++++++++++-----------------------------
 1 file changed, 14 insertions(+), 33 deletions(-)

diff --git a/drivers/pci/host/pcie-xilinx.c b/drivers/pci/host/pcie-xilinx.c
index 1490bd1..afdfb09 100644
--- a/drivers/pci/host/pcie-xilinx.c
+++ b/drivers/pci/host/pcie-xilinx.c
@@ -397,7 +397,7 @@ static const struct irq_domain_ops intx_domain_ops = {
 static irqreturn_t xilinx_pcie_intr_handler(int irq, void *data)
 {
 	struct xilinx_pcie_port *port = (struct xilinx_pcie_port *)data;
-	u32 val, mask, status, msi_data;
+	u32 val, mask, status;
 
 	/* Read interrupt decode and mask registers */
 	val = pcie_read(port, XILINX_PCIE_REG_IDR);
@@ -437,8 +437,8 @@ static irqreturn_t xilinx_pcie_intr_handler(int irq, void *data)
 		xilinx_pcie_clear_err_interrupts(port);
 	}
 
-	if (status & XILINX_PCIE_INTR_INTX) {
-		/* INTx interrupt received */
+	if (status & (XILINX_PCIE_INTR_INTX | XILINX_PCIE_INTR_MSI)) {
+		/* Interrupt received */
 		val = pcie_read(port, XILINX_PCIE_REG_RPIFR1);
 
 		/* Check whether interrupt valid */
@@ -447,41 +447,22 @@ static irqreturn_t xilinx_pcie_intr_handler(int irq, void *data)
 			return IRQ_HANDLED;
 		}
 
-		if (!(val & XILINX_PCIE_RPIFR1_MSI_INTR)) {
-			/* Clear interrupt FIFO register 1 */
-			pcie_write(port, XILINX_PCIE_RPIFR1_ALL_MASK,
-				   XILINX_PCIE_REG_RPIFR1);
-
-			/* Handle INTx Interrupt */
+		if (val & XILINX_PCIE_RPIFR1_MSI_INTR) {
+			irq = pcie_read(port, XILINX_PCIE_REG_RPIFR2) &
+				XILINX_PCIE_RPIFR2_MSG_DATA;
+		} else {
 			val = ((val & XILINX_PCIE_RPIFR1_INTR_MASK) >>
 				XILINX_PCIE_RPIFR1_INTR_SHIFT) + 1;
-			generic_handle_irq(irq_find_mapping(port->irq_domain,
-							    val));
+			irq = irq_find_mapping(port->irq_domain, val);
 		}
-	}
 
-	if (status & XILINX_PCIE_INTR_MSI) {
-		/* MSI Interrupt */
-		val = pcie_read(port, XILINX_PCIE_REG_RPIFR1);
+		/* Clear interrupt FIFO register 1 */
+		pcie_write(port, XILINX_PCIE_RPIFR1_ALL_MASK,
+			   XILINX_PCIE_REG_RPIFR1);
 
-		if (!(val & XILINX_PCIE_RPIFR1_INTR_VALID)) {
-			dev_warn(port->dev, "RP Intr FIFO1 read error\n");
-			return IRQ_HANDLED;
-		}
-
-		if (val & XILINX_PCIE_RPIFR1_MSI_INTR) {
-			msi_data = pcie_read(port, XILINX_PCIE_REG_RPIFR2) &
-				   XILINX_PCIE_RPIFR2_MSG_DATA;
-
-			/* Clear interrupt FIFO register 1 */
-			pcie_write(port, XILINX_PCIE_RPIFR1_ALL_MASK,
-				   XILINX_PCIE_REG_RPIFR1);
-
-			if (IS_ENABLED(CONFIG_PCI_MSI)) {
-				/* Handle MSI Interrupt */
-				generic_handle_irq(msi_data);
-			}
-		}
+		if (IS_ENABLED(CONFIG_PCI_MSI) ||
+			!(val & XILINX_PCIE_RPIFR1_MSI_INTR))
+			generic_handle_irq(irq);
 	}
 
 	if (status & XILINX_PCIE_INTR_SLV_UNSUPP)
-- 
2.7.0

RE: [PATCH v3 2/6] PCI: xilinx: Unify INTx & MSI interrupt FIFO decode

[Bharat Kumar Gogada]

> Subject: [PATCH v3 2/6] PCI: xilinx: Unify INTx & MSI interrupt FIFO decode
> 
> When decoding either an INTx or MSI interrupt, the driver has no way to
> know which it will pull out of the interrupt FIFO. If both were pending then
> this would lead to either the interrupt being handled incorrectly (MSI
> interrupt treated as INTx) or not at all (INTx interrupt dropped by MSI path).
> Unify the reading of the interrupt FIFO & act according to the type of
> interrupt actually read.
> 
> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
> Fixes: 8961def56845 ("PCI: xilinx: Add Xilinx AXI PCIe Host Bridge IP driver")
> 
> ---
> 
> Changes in v3:
> - Split out from Boston patchset.
> 
> Changes in v2:
> - Add Fixes tag.
> 
>  drivers/pci/host/pcie-xilinx.c | 47 +++++++++++++-----------------------------
>  1 file changed, 14 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/pci/host/pcie-xilinx.c b/drivers/pci/host/pcie-xilinx.c index
> 1490bd1..afdfb09 100644
> --- a/drivers/pci/host/pcie-xilinx.c
> +++ b/drivers/pci/host/pcie-xilinx.c
> @@ -397,7 +397,7 @@ static const struct irq_domain_ops intx_domain_ops
> = {  static irqreturn_t xilinx_pcie_intr_handler(int irq, void *data)  {
>  	struct xilinx_pcie_port *port = (struct xilinx_pcie_port *)data;
> -	u32 val, mask, status, msi_data;
> +	u32 val, mask, status;
> 
>  	/* Read interrupt decode and mask registers */
>  	val = pcie_read(port, XILINX_PCIE_REG_IDR); @@ -437,8 +437,8 @@
> static irqreturn_t xilinx_pcie_intr_handler(int irq, void *data)
>  		xilinx_pcie_clear_err_interrupts(port);
>  	}
> 
> -	if (status & XILINX_PCIE_INTR_INTX) {
> -		/* INTx interrupt received */
> +	if (status & (XILINX_PCIE_INTR_INTX | XILINX_PCIE_INTR_MSI)) {
> +		/* Interrupt received */
>  		val = pcie_read(port, XILINX_PCIE_REG_RPIFR1);
> 
>  		/* Check whether interrupt valid */
> @@ -447,41 +447,22 @@ static irqreturn_t xilinx_pcie_intr_handler(int irq,
> void *data)
>  			return IRQ_HANDLED;
>  		}
> 
> -		if (!(val & XILINX_PCIE_RPIFR1_MSI_INTR)) {
> -			/* Clear interrupt FIFO register 1 */
> -			pcie_write(port, XILINX_PCIE_RPIFR1_ALL_MASK,
> -				   XILINX_PCIE_REG_RPIFR1);
> -
> -			/* Handle INTx Interrupt */
> +		if (val & XILINX_PCIE_RPIFR1_MSI_INTR) {
> +			irq = pcie_read(port, XILINX_PCIE_REG_RPIFR2) &
> +				XILINX_PCIE_RPIFR2_MSG_DATA;
> +		} else {
>  			val = ((val & XILINX_PCIE_RPIFR1_INTR_MASK) >>
>  				XILINX_PCIE_RPIFR1_INTR_SHIFT) + 1;
> -			generic_handle_irq(irq_find_mapping(port-
> >irq_domain,
> -							    val));
> +			irq = irq_find_mapping(port->irq_domain, val);
>  		}
> -	}
> 
> -	if (status & XILINX_PCIE_INTR_MSI) {
> -		/* MSI Interrupt */
> -		val = pcie_read(port, XILINX_PCIE_REG_RPIFR1);
> +		/* Clear interrupt FIFO register 1 */
> +		pcie_write(port, XILINX_PCIE_RPIFR1_ALL_MASK,
> +			   XILINX_PCIE_REG_RPIFR1);
> 
> -		if (!(val & XILINX_PCIE_RPIFR1_INTR_VALID)) {
> -			dev_warn(port->dev, "RP Intr FIFO1 read error\n");
> -			return IRQ_HANDLED;
> -		}
> -
> -		if (val & XILINX_PCIE_RPIFR1_MSI_INTR) {
> -			msi_data = pcie_read(port,
> XILINX_PCIE_REG_RPIFR2) &
> -				   XILINX_PCIE_RPIFR2_MSG_DATA;
> -
> -			/* Clear interrupt FIFO register 1 */
> -			pcie_write(port, XILINX_PCIE_RPIFR1_ALL_MASK,
> -				   XILINX_PCIE_REG_RPIFR1);
> -
> -			if (IS_ENABLED(CONFIG_PCI_MSI)) {
> -				/* Handle MSI Interrupt */
> -				generic_handle_irq(msi_data);
> -			}
> -		}
> +		if (IS_ENABLED(CONFIG_PCI_MSI) ||
> +			!(val & XILINX_PCIE_RPIFR1_MSI_INTR))
> +			generic_handle_irq(irq);
>  	}
> 
>  	if (status & XILINX_PCIE_INTR_SLV_UNSUPP)
> --

Hi Paul,

Even with above condition you are still missing either MSI or legacy interrupt handling, when both MSI and legacy interrupts occurred.

Bharat

RE: [PATCH v3 2/6] PCI: xilinx: Unify INTx & MSI interrupt FIFO decode

[Bharat Kumar Gogada]

> > Subject: [PATCH v3 2/6] PCI: xilinx: Unify INTx & MSI interrupt FIFO
> > decode
> >
> > When decoding either an INTx or MSI interrupt, the driver has no way
> > to know which it will pull out of the interrupt FIFO. If both were
> > pending then this would lead to either the interrupt being handled
> > incorrectly (MSI interrupt treated as INTx) or not at all (INTx interrupt
> dropped by MSI path).
> > Unify the reading of the interrupt FIFO & act according to the type of
> > interrupt actually read.
> >
> > Signed-off-by: Paul Burton <paul.burton@imgtec.com>
> > Fixes: 8961def56845 ("PCI: xilinx: Add Xilinx AXI PCIe Host Bridge IP
> > driver")
> >
> > ---
> >
> > Changes in v3:
> > - Split out from Boston patchset.
> >
> > Changes in v2:
> > - Add Fixes tag.
> >
> >  drivers/pci/host/pcie-xilinx.c | 47
> > +++++++++++++-----------------------------
> >  1 file changed, 14 insertions(+), 33 deletions(-)
> >
> > diff --git a/drivers/pci/host/pcie-xilinx.c
> > b/drivers/pci/host/pcie-xilinx.c index
> > 1490bd1..afdfb09 100644
> > --- a/drivers/pci/host/pcie-xilinx.c
> > +++ b/drivers/pci/host/pcie-xilinx.c
> > @@ -397,7 +397,7 @@ static const struct irq_domain_ops
> intx_domain_ops
> > = {  static irqreturn_t xilinx_pcie_intr_handler(int irq, void *data)  {
> >  	struct xilinx_pcie_port *port = (struct xilinx_pcie_port *)data;
> > -	u32 val, mask, status, msi_data;
> > +	u32 val, mask, status;
> >
> >  	/* Read interrupt decode and mask registers */
> >  	val = pcie_read(port, XILINX_PCIE_REG_IDR); @@ -437,8 +437,8 @@
> > static irqreturn_t xilinx_pcie_intr_handler(int irq, void *data)
> >  		xilinx_pcie_clear_err_interrupts(port);
> >  	}
> >
> > -	if (status & XILINX_PCIE_INTR_INTX) {
> > -		/* INTx interrupt received */
> > +	if (status & (XILINX_PCIE_INTR_INTX | XILINX_PCIE_INTR_MSI)) {
> > +		/* Interrupt received */
> >  		val = pcie_read(port, XILINX_PCIE_REG_RPIFR1);
> >
> >  		/* Check whether interrupt valid */ @@ -447,41 +447,22 @@
> static
> > irqreturn_t xilinx_pcie_intr_handler(int irq, void *data)
> >  			return IRQ_HANDLED;
> >  		}
> >
> > -		if (!(val & XILINX_PCIE_RPIFR1_MSI_INTR)) {
> > -			/* Clear interrupt FIFO register 1 */
> > -			pcie_write(port, XILINX_PCIE_RPIFR1_ALL_MASK,
> > -				   XILINX_PCIE_REG_RPIFR1);
> > -
> > -			/* Handle INTx Interrupt */
> > +		if (val & XILINX_PCIE_RPIFR1_MSI_INTR) {
> > +			irq = pcie_read(port, XILINX_PCIE_REG_RPIFR2) &
> > +				XILINX_PCIE_RPIFR2_MSG_DATA;
> > +		} else {
> >  			val = ((val & XILINX_PCIE_RPIFR1_INTR_MASK) >>
> >  				XILINX_PCIE_RPIFR1_INTR_SHIFT) + 1;
> > -			generic_handle_irq(irq_find_mapping(port-
> > >irq_domain,
> > -							    val));
> > +			irq = irq_find_mapping(port->irq_domain, val);
> >  		}
> > -	}
> >
> > -	if (status & XILINX_PCIE_INTR_MSI) {
> > -		/* MSI Interrupt */
> > -		val = pcie_read(port, XILINX_PCIE_REG_RPIFR1);
> > +		/* Clear interrupt FIFO register 1 */
> > +		pcie_write(port, XILINX_PCIE_RPIFR1_ALL_MASK,
> > +			   XILINX_PCIE_REG_RPIFR1);
> >
> > -		if (!(val & XILINX_PCIE_RPIFR1_INTR_VALID)) {
> > -			dev_warn(port->dev, "RP Intr FIFO1 read error\n");
> > -			return IRQ_HANDLED;
> > -		}
> > -
> > -		if (val & XILINX_PCIE_RPIFR1_MSI_INTR) {
> > -			msi_data = pcie_read(port,
> > XILINX_PCIE_REG_RPIFR2) &
> > -				   XILINX_PCIE_RPIFR2_MSG_DATA;
> > -
> > -			/* Clear interrupt FIFO register 1 */
> > -			pcie_write(port, XILINX_PCIE_RPIFR1_ALL_MASK,
> > -				   XILINX_PCIE_REG_RPIFR1);
> > -
> > -			if (IS_ENABLED(CONFIG_PCI_MSI)) {
> > -				/* Handle MSI Interrupt */
> > -				generic_handle_irq(msi_data);
> > -			}
> > -		}
> > +		if (IS_ENABLED(CONFIG_PCI_MSI) ||
> > +			!(val & XILINX_PCIE_RPIFR1_MSI_INTR))
> > +			generic_handle_irq(irq);
> >  	}
> >
> >  	if (status & XILINX_PCIE_INTR_SLV_UNSUPP)
> > --
> 
> Hi Paul,
> 
> Even with above condition you are still missing either MSI or legacy interrupt
> handling, when both MSI and legacy interrupts occurred.

It would be better if the condition when both legacy and MSI interrupts occurred can be handled separately, leaving the current individual interrupt cases as they are.

Bharat

[PATCH v3 3/6] PCI: xilinx: Always clear interrupt decode register

[Paul Burton]

If an MSI or INTx interrupt is incorrectly triggered with an empty FIFO
then xilinx_pcie_intr_handler will print a warning & skip further
processing. However it did not clear the interrupt in the decode
register, so the same INTX or MSI interrupt would trigger again
immediately even though the FIFO is still empty. Clear the interrupt in
the decode register to avoid that situation.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Fixes: 8961def56845 ("PCI: xilinx: Add Xilinx AXI PCIe Host Bridge IP driver")

---

Changes in v3:
- Split out from Boston patchset.

Changes in v2:
- Add Fixes tag.

 drivers/pci/host/pcie-xilinx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/host/pcie-xilinx.c b/drivers/pci/host/pcie-xilinx.c
index afdfb09..1eb74a2 100644
--- a/drivers/pci/host/pcie-xilinx.c
+++ b/drivers/pci/host/pcie-xilinx.c
@@ -444,7 +444,7 @@ static irqreturn_t xilinx_pcie_intr_handler(int irq, void *data)
 		/* Check whether interrupt valid */
 		if (!(val & XILINX_PCIE_RPIFR1_INTR_VALID)) {
 			dev_warn(port->dev, "RP Intr FIFO1 read error\n");
-			return IRQ_HANDLED;
+			goto out;
 		}
 
 		if (val & XILINX_PCIE_RPIFR1_MSI_INTR) {
@@ -492,6 +492,7 @@ static irqreturn_t xilinx_pcie_intr_handler(int irq, void *data)
 	if (status & XILINX_PCIE_INTR_MST_ERRP)
 		dev_warn(port->dev, "Master error poison\n");
 
+out:
 	/* Clear the Interrupt Decode register */
 	pcie_write(port, status, XILINX_PCIE_REG_IDR);
 
-- 
2.7.0

[PATCH v3 4/6] PCI: xilinx: Clear interrupt FIFO during probe

[Paul Burton]

xilinx_pcie_init_port clears the pending interrupts in the interrupt
decode register, but does not clear the interrupt FIFO. This would lead
to spurious interrupts if any were present in the FIFO at probe time.
Clear the interrupt FIFO prior to the interrupt decode register in order
to start with a clean slate as expected.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Fixes: 8961def56845 ("PCI: xilinx: Add Xilinx AXI PCIe Host Bridge IP driver")

---

Changes in v3:
- Split out from Boston patchset.

Changes in v2:
- Add Fixes tag.

 drivers/pci/host/pcie-xilinx.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/pci/host/pcie-xilinx.c b/drivers/pci/host/pcie-xilinx.c
index 1eb74a2..6c5a503 100644
--- a/drivers/pci/host/pcie-xilinx.c
+++ b/drivers/pci/host/pcie-xilinx.c
@@ -568,6 +568,8 @@ static int xilinx_pcie_init_irq_domain(struct xilinx_pcie_port *port)
  */
 static void xilinx_pcie_init_port(struct xilinx_pcie_port *port)
 {
+	u32 val;
+
 	if (xilinx_pcie_link_is_up(port))
 		dev_info(port->dev, "PCIe Link is UP\n");
 	else
@@ -577,6 +579,17 @@ static void xilinx_pcie_init_port(struct xilinx_pcie_port *port)
 	pcie_write(port, ~XILINX_PCIE_IDR_ALL_MASK,
 		   XILINX_PCIE_REG_IMR);
 
+	/* Clear interrupt FIFO */
+	while (1) {
+		val = pcie_read(port, XILINX_PCIE_REG_RPIFR1);
+
+		if (!(val & XILINX_PCIE_RPIFR1_INTR_VALID))
+			break;
+
+		pcie_write(port, XILINX_PCIE_RPIFR1_ALL_MASK,
+			   XILINX_PCIE_REG_RPIFR1);
+	}
+
 	/* Clear pending interrupts */
 	pcie_write(port, pcie_read(port, XILINX_PCIE_REG_IDR) &
 			 XILINX_PCIE_IMR_ALL_MASK,
-- 
2.7.0

RE: [PATCH v3 4/6] PCI: xilinx: Clear interrupt FIFO during probe

[Bharat Kumar Gogada]

 > xilinx_pcie_init_port clears the pending interrupts in the interrupt decode
> register, but does not clear the interrupt FIFO. This would lead to spurious
> interrupts if any were present in the FIFO at probe time.
> Clear the interrupt FIFO prior to the interrupt decode register in order to
> start with a clean slate as expected.
> 
> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
> Fixes: 8961def56845 ("PCI: xilinx: Add Xilinx AXI PCIe Host Bridge IP driver")
> 
> ---
> 
> Changes in v3:
> - Split out from Boston patchset.
> 
> Changes in v2:
> - Add Fixes tag.
> 
>  drivers/pci/host/pcie-xilinx.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/pci/host/pcie-xilinx.c b/drivers/pci/host/pcie-xilinx.c index
> 1eb74a2..6c5a503 100644
> --- a/drivers/pci/host/pcie-xilinx.c
> +++ b/drivers/pci/host/pcie-xilinx.c
> @@ -568,6 +568,8 @@ static int xilinx_pcie_init_irq_domain(struct
> xilinx_pcie_port *port)
>   */
>  static void xilinx_pcie_init_port(struct xilinx_pcie_port *port)  {
> +	u32 val;
> +
>  	if (xilinx_pcie_link_is_up(port))
>  		dev_info(port->dev, "PCIe Link is UP\n");
>  	else
> @@ -577,6 +579,17 @@ static void xilinx_pcie_init_port(struct
> xilinx_pcie_port *port)
>  	pcie_write(port, ~XILINX_PCIE_IDR_ALL_MASK,
>  		   XILINX_PCIE_REG_IMR);
> 
> +	/* Clear interrupt FIFO */
> +	while (1) {
> +		val = pcie_read(port, XILINX_PCIE_REG_RPIFR1);
> +
> +		if (!(val & XILINX_PCIE_RPIFR1_INTR_VALID))
> +			break;
> +
> +		pcie_write(port, XILINX_PCIE_RPIFR1_ALL_MASK,
> +			   XILINX_PCIE_REG_RPIFR1);
> +	}
> +
Hi Paul,

This case will create problem with error case, suppose if we have continuous correctable errors on link this will always be while loop.

Bharat

[PATCH v3 5/6] PCI: xilinx: Fix INTX irq dispatch

[Paul Burton]

The IRQ domain for INTX interrupts has 4 entries, numbered 0 to 3. This
matches what the hardware reports from the interrupt FIFO exactly, but
xilinx_pcie_intr_handler was adding 1 to that value to convert to the
range 1 to 4. Stop adding 1, such that all of INTA through to INTD fall
within the range of the IRQ domain.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Fixes: 8961def56845 ("PCI: xilinx: Add Xilinx AXI PCIe Host Bridge IP driver")

---

Changes in v3:
- Split out from Boston patchset.

Changes in v2:
- Add Fixes tag.

 drivers/pci/host/pcie-xilinx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/host/pcie-xilinx.c b/drivers/pci/host/pcie-xilinx.c
index 6c5a503..8013e83 100644
--- a/drivers/pci/host/pcie-xilinx.c
+++ b/drivers/pci/host/pcie-xilinx.c
@@ -451,8 +451,8 @@ static irqreturn_t xilinx_pcie_intr_handler(int irq, void *data)
 			irq = pcie_read(port, XILINX_PCIE_REG_RPIFR2) &
 				XILINX_PCIE_RPIFR2_MSG_DATA;
 		} else {
-			val = ((val & XILINX_PCIE_RPIFR1_INTR_MASK) >>
-				XILINX_PCIE_RPIFR1_INTR_SHIFT) + 1;
+			val = (val & XILINX_PCIE_RPIFR1_INTR_MASK) >>
+				XILINX_PCIE_RPIFR1_INTR_SHIFT;
 			irq = irq_find_mapping(port->irq_domain, val);
 		}
 
-- 
2.7.0

[PATCH v3 6/6] PCI: xilinx: Allow build on MIPS platforms

[Paul Burton]

Allow the xilinx-pcie driver to be built on MIPS platforms. This will be
used on the MIPS Boston board.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>

---

Changes in v3:
- Split out from Boston patchset.

Changes in v2: None

 drivers/pci/host/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index 75a6054..0aee193 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -81,7 +81,7 @@ config PCI_KEYSTONE
 
 config PCIE_XILINX
 	bool "Xilinx AXI PCIe host bridge support"
-	depends on ARCH_ZYNQ
+	depends on ARCH_ZYNQ || MIPS
 	help
 	  Say 'Y' here if you want kernel to support the Xilinx AXI PCIe
 	  Host Bridge driver.
-- 
2.7.0

Re: [PATCH v3 6/6] PCI: xilinx: Allow build on MIPS platforms

[Rob Herring]

On Thu, Feb 4, 2016 at 10:10 AM, Paul Burton <paul.burton@imgtec.com> wrote:
> Allow the xilinx-pcie driver to be built on MIPS platforms. This will be
> used on the MIPS Boston board.
>
> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
>
> ---
>
> Changes in v3:
> - Split out from Boston patchset.
>
> Changes in v2: None
>
>  drivers/pci/host/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index 75a6054..0aee193 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -81,7 +81,7 @@ config PCI_KEYSTONE
>
>  config PCIE_XILINX
>         bool "Xilinx AXI PCIe host bridge support"
> -       depends on ARCH_ZYNQ
> +       depends on ARCH_ZYNQ || MIPS

Why don't you just remove the dependency? Then it gets better build coverage.

Rob

Re: [PATCH v3 6/6] PCI: xilinx: Allow build on MIPS platforms

[Paul Burton]

On Thu, Feb 04, 2016 at 11:46:28AM -0600, Rob Herring wrote:
> On Thu, Feb 4, 2016 at 10:10 AM, Paul Burton <paul.burton@imgtec.com> wrote:
> > Allow the xilinx-pcie driver to be built on MIPS platforms. This will be
> > used on the MIPS Boston board.
> >
> > Signed-off-by: Paul Burton <paul.burton@imgtec.com>
> >
> > ---
> >
> > Changes in v3:
> > - Split out from Boston patchset.
> >
> > Changes in v2: None
> >
> >  drivers/pci/host/Kconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> > index 75a6054..0aee193 100644
> > --- a/drivers/pci/host/Kconfig
> > +++ b/drivers/pci/host/Kconfig
> > @@ -81,7 +81,7 @@ config PCI_KEYSTONE
> >
> >  config PCIE_XILINX
> >         bool "Xilinx AXI PCIe host bridge support"
> > -       depends on ARCH_ZYNQ
> > +       depends on ARCH_ZYNQ || MIPS
> 
> Why don't you just remove the dependency? Then it gets better build coverage.
> 
> Rob

That seems like a call best made by whomever has to maintain this - if
that's the preferred way to go I'm fine with it.

Thanks,
    Paul

Re: [PATCH v3 6/6] PCI: xilinx: Allow build on MIPS platforms

[Bjorn Helgaas]

On Thu, Feb 04, 2016 at 05:53:25PM +0000, Paul Burton wrote:
> On Thu, Feb 04, 2016 at 11:46:28AM -0600, Rob Herring wrote:
> > On Thu, Feb 4, 2016 at 10:10 AM, Paul Burton <paul.burton@imgtec.com> wrote:
> > > Allow the xilinx-pcie driver to be built on MIPS platforms. This will be
> > > used on the MIPS Boston board.
> > >
> > > Signed-off-by: Paul Burton <paul.burton@imgtec.com>
> > >
> > > ---
> > >
> > > Changes in v3:
> > > - Split out from Boston patchset.
> > >
> > > Changes in v2: None
> > >
> > >  drivers/pci/host/Kconfig | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> > > index 75a6054..0aee193 100644
> > > --- a/drivers/pci/host/Kconfig
> > > +++ b/drivers/pci/host/Kconfig
> > > @@ -81,7 +81,7 @@ config PCI_KEYSTONE
> > >
> > >  config PCIE_XILINX
> > >         bool "Xilinx AXI PCIe host bridge support"
> > > -       depends on ARCH_ZYNQ
> > > +       depends on ARCH_ZYNQ || MIPS
> > 
> > Why don't you just remove the dependency? Then it gets better build coverage.
> > 
> > Rob
> 
> That seems like a call best made by whomever has to maintain this - if
> that's the preferred way to go I'm fine with it.

I'm in favor of removing the dependency if possible.  I guess Michal
would be the person to ack that.

Right now (in my current "next" branch),
drivers/pci/host/pcie-xilinx.c uses struct hw_pci, which is only
defined by arm.

Bjorn

Re: [PATCH v3 6/6] PCI: xilinx: Allow build on MIPS platforms

[Michal Simek]

On 25.2.2016 16:43, Bjorn Helgaas wrote:
> On Thu, Feb 04, 2016 at 05:53:25PM +0000, Paul Burton wrote:
>> On Thu, Feb 04, 2016 at 11:46:28AM -0600, Rob Herring wrote:
>>> On Thu, Feb 4, 2016 at 10:10 AM, Paul Burton <paul.burton@imgtec.com> wrote:
>>>> Allow the xilinx-pcie driver to be built on MIPS platforms. This will be
>>>> used on the MIPS Boston board.
>>>>
>>>> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
>>>>
>>>> ---
>>>>
>>>> Changes in v3:
>>>> - Split out from Boston patchset.
>>>>
>>>> Changes in v2: None
>>>>
>>>>  drivers/pci/host/Kconfig | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
>>>> index 75a6054..0aee193 100644
>>>> --- a/drivers/pci/host/Kconfig
>>>> +++ b/drivers/pci/host/Kconfig
>>>> @@ -81,7 +81,7 @@ config PCI_KEYSTONE
>>>>
>>>>  config PCIE_XILINX
>>>>         bool "Xilinx AXI PCIe host bridge support"
>>>> -       depends on ARCH_ZYNQ
>>>> +       depends on ARCH_ZYNQ || MIPS
>>>
>>> Why don't you just remove the dependency? Then it gets better build coverage.
>>>
>>> Rob
>>
>> That seems like a call best made by whomever has to maintain this - if
>> that's the preferred way to go I'm fine with it.
> 
> I'm in favor of removing the dependency if possible.  I guess Michal
> would be the person to ack that.

Not a problem to remove dependency on archs. There is pending support
for Microblaze anyway.

Thanks,
Michal

Re: [PATCH v3 0/6] Xilinx AXI PCIe Host Bridge driver fixes

[Bjorn Helgaas]

Hi Paul,

On Thu, Feb 04, 2016 at 04:10:07PM +0000, Paul Burton wrote:
> This series fixes a number of issues found using the Xilinx AXI PCIe
> Host Bridge IP on the Imagination Technologies MIPS Boston development
> board. It has been split out of the larger Boston board support series
> at Michal's request.
> 
> Applies atop v4.5-rc2.
> 
> Paul Burton (6):
>   PCI: xilinx: Keep references to both IRQ domains
>   PCI: xilinx: Unify INTx & MSI interrupt FIFO decode
>   PCI: xilinx: Always clear interrupt decode register
>   PCI: xilinx: Clear interrupt FIFO during probe
>   PCI: xilinx: Fix INTX irq dispatch
>   PCI: xilinx: Allow build on MIPS platforms
> 
>  drivers/pci/host/Kconfig       |   2 +-
>  drivers/pci/host/pcie-xilinx.c | 125 +++++++++++++++++++----------------------

Looks like Bharat has some IRQ concerns, so I'm guessing you'll be
posting a v4?

Bjorn