[PATCH v4 0/4] PCI: xilinx: Fixes, optimisation & MIPS support

[PATCH v4 0/4] PCI: xilinx: Fixes, optimisation & MIPS support

[Paul Burton]

This series fixes an issue found using INTx interrupts with the Xilinx
AXI PCIe Host Bridge IP on the Imagination Technologies MIPS Boston
development board, performs a couple of optimisations to interrupt
handling & allows the driver to be used on MIPS systems.

Applies atop v4.12-rc3.

Paul Burton (4):
  PCI: xilinx: Fix INTX irq dispatch
  PCI: xilinx: Unify INTx & MSI interrupt decode
  PCI: xilinx: Don't enable config completion interrupts
  PCI: xilinx: Allow build on MIPS platforms

 drivers/pci/host/Kconfig       |  2 +-
 drivers/pci/host/pcie-xilinx.c | 57 +++++++++++++++---------------------------
 2 files changed, 21 insertions(+), 38 deletions(-)

-- 
2.13.0

[PATCH v4 1/4] 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")
Cc: Bharat Kumar Gogada <bharatku@xilinx.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Michal Simek <michal.simek@xilinx.com>
Cc: Ravikiran Gummaluri <rgummal@xilinx.com>
Cc: linux-pci@vger.kernel.org

---

Changes in v4: None

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 2fe2df51f9f8..6be2e5ee44f1 100644
--- a/drivers/pci/host/pcie-xilinx.c
+++ b/drivers/pci/host/pcie-xilinx.c
@@ -440,8 +440,8 @@ static irqreturn_t xilinx_pcie_intr_handler(int irq, void *data)
 				   XILINX_PCIE_REG_RPIFR1);
 
 			/* Handle INTx Interrupt */
-			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;
 			generic_handle_irq(irq_find_mapping(port->leg_domain,
 							    val));
 		}
-- 
2.13.0

[PATCH v4 2/4] PCI: xilinx: Unify INTx & MSI interrupt decode

[Paul Burton]

The INTx & MSI interrupt decode paths duplicated a fair bit of common
functionality. They also strictly handled interrupts in order of INTx
then MSI, so if both types of interrupt were to be asserted
simultaneously and the MSI interrupt were first in the FIFO then the
INTx code would read it & ignore it before the MSI code then had to read
it again, wasting the original FIFO read.

Unify the INTx & MSI decode in order to reduce that duplication & allow
a single FIFO read to be performed for each interrupt regardless of its
type.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Cc: Bharat Kumar Gogada <bharatku@xilinx.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Michal Simek <michal.simek@xilinx.com>
Cc: Ravikiran Gummaluri <rgummal@xilinx.com>
Cc: linux-pci@vger.kernel.org
---

Changes in v4: None
Changes in v3: None
Changes in v2: None

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

diff --git a/drivers/pci/host/pcie-xilinx.c b/drivers/pci/host/pcie-xilinx.c
index 6be2e5ee44f1..0c0ecc463f6b 100644
--- a/drivers/pci/host/pcie-xilinx.c
+++ b/drivers/pci/host/pcie-xilinx.c
@@ -384,7 +384,7 @@ static irqreturn_t xilinx_pcie_intr_handler(int irq, void *data)
 {
 	struct xilinx_pcie_port *port = (struct xilinx_pcie_port *)data;
 	struct device *dev = port->dev;
-	u32 val, mask, status, msi_data;
+	u32 val, mask, status;
 
 	/* Read interrupt decode and mask registers */
 	val = pcie_read(port, XILINX_PCIE_REG_IDR);
@@ -424,8 +424,7 @@ 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)) {
 		val = pcie_read(port, XILINX_PCIE_REG_RPIFR1);
 
 		/* Check whether interrupt valid */
@@ -434,41 +433,24 @@ static irqreturn_t xilinx_pcie_intr_handler(int irq, void *data)
 			goto error;
 		}
 
-		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 */
+		/* Decode the IRQ number */
+		if (val & XILINX_PCIE_RPIFR1_MSI_INTR) {
+			val = 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;
-			generic_handle_irq(irq_find_mapping(port->leg_domain,
-							    val));
+			val = irq_find_mapping(port->leg_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(dev, "RP Intr FIFO1 read error\n");
-			goto error;
-		}
-
-		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);
-			}
-		}
+		/* Handle the interrupt */
+		if (IS_ENABLED(CONFIG_PCI_MSI) ||
+		    !(val & XILINX_PCIE_RPIFR1_MSI_INTR))
+			generic_handle_irq(val);
 	}
 
 	if (status & XILINX_PCIE_INTR_SLV_UNSUPP)
-- 
2.13.0

[PATCH v4 3/4] PCI: xilinx: Don't enable config completion interrupts

[Paul Burton]

The Xilinx AXI bridge for PCI Express device provides interrupts
indicating the completion of config space accesses. We have previously
enabled/unmasked them but do nothing with them besides acknowledge them.

Leave the interrupts masked in order to avoid servicing a large number
of pointless interrupts during boot.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Cc: Bharat Kumar Gogada <bharatku@xilinx.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Michal Simek <michal.simek@xilinx.com>
Cc: Ravikiran Gummaluri <rgummal@xilinx.com>
Cc: linux-pci@vger.kernel.org
---

Changes in v4: None
Changes in v3: None
Changes in v2: None

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

diff --git a/drivers/pci/host/pcie-xilinx.c b/drivers/pci/host/pcie-xilinx.c
index 0c0ecc463f6b..f3018c1c8a59 100644
--- a/drivers/pci/host/pcie-xilinx.c
+++ b/drivers/pci/host/pcie-xilinx.c
@@ -60,6 +60,7 @@
 #define XILINX_PCIE_INTR_MST_SLVERR	BIT(27)
 #define XILINX_PCIE_INTR_MST_ERRP	BIT(28)
 #define XILINX_PCIE_IMR_ALL_MASK	0x1FF30FED
+#define XILINX_PCIE_IMR_ENABLE_MASK	0x1FF30F0D
 #define XILINX_PCIE_IDR_ALL_MASK	0xFFFFFFFF
 
 /* Root Port Error FIFO Read Register definitions */
@@ -553,8 +554,8 @@ static void xilinx_pcie_init_port(struct xilinx_pcie_port *port)
 			 XILINX_PCIE_IMR_ALL_MASK,
 		   XILINX_PCIE_REG_IDR);
 
-	/* Enable all interrupts */
-	pcie_write(port, XILINX_PCIE_IMR_ALL_MASK, XILINX_PCIE_REG_IMR);
+	/* Enable all interrupts we handle */
+	pcie_write(port, XILINX_PCIE_IMR_ENABLE_MASK, XILINX_PCIE_REG_IMR);
 
 	/* Enable the Bridge enable bit */
 	pcie_write(port, pcie_read(port, XILINX_PCIE_REG_RPSC) |
-- 
2.13.0

[PATCH v4 4/4] PCI: xilinx: Allow build on MIPS platforms

[Paul Burton]

Allow the xilinx-pcie driver to be built on MIPS platforms which make
use of generic PCI drivers rather than legacy MIPS-specific interfaces.
This is used on the MIPS Boston development board.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Cc: Bharat Kumar Gogada <bharatku@xilinx.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Michal Simek <michal.simek@xilinx.com>
Cc: Ravikiran Gummaluri <rgummal@xilinx.com>
Cc: linux-pci@vger.kernel.org

---

Changes in v4:
- Depend on PCI_DRIVERS_GENERIC, which the driver won't work on MIPS without.

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 7f47cd5e10a5..22d4405914ec 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -71,7 +71,7 @@ config PCI_HOST_GENERIC
 
 config PCIE_XILINX
 	bool "Xilinx AXI PCIe host bridge support"
-	depends on ARCH_ZYNQ || MICROBLAZE
+	depends on ARCH_ZYNQ || MICROBLAZE || (MIPS && PCI_DRIVERS_GENERIC)
 	help
 	  Say 'Y' here if you want kernel to support the Xilinx AXI PCIe
 	  Host Bridge driver.
-- 
2.13.0

RE: [PATCH v4 1/4] PCI: xilinx: Fix INTX irq dispatch

[Bharat Kumar Gogada]

> The IRQ domain for INTX interrupts has 4 entries, numbered 0 to 3. This m=
atches
> what the hardware reports from the interrupt FIFO exactly, but
> xilinx_pcie_intr_handler was adding 1 to that value to convert to the ran=
ge 1 to
> 4. Stop adding 1, such that all of INTA through to INTD fall within the r=
ange of
> the IRQ domain.
>=20
Paul, this change is not required.
hwirq for legacy domain is being assigned by kernel PCIe subsystem based on=
 PCI_INTERRUPT_PIN, which as per
protocol starts from 1 and not zero, due to this 1 is being added.

Regards,
Bharat

> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
> Fixes: 8961def56845 ("PCI: xilinx: Add Xilinx AXI PCIe Host Bridge IP dri=
ver")
> Cc: Bharat Kumar Gogada <bharatku@xilinx.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Michal Simek <michal.simek@xilinx.com>
> Cc: Ravikiran Gummaluri <rgummal@xilinx.com>
> Cc: linux-pci@vger.kernel.org
>=20
> ---
>=20
> Changes in v4: None
>=20
> Changes in v3:
> - Split out from Boston patchset.
>=20
> Changes in v2:
> - Add Fixes tag.
>=20
>  drivers/pci/host/pcie-xilinx.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>=20
> diff --git a/drivers/pci/host/pcie-xilinx.c b/drivers/pci/host/pcie-xilin=
x.c index
> 2fe2df51f9f8..6be2e5ee44f1 100644
> --- a/drivers/pci/host/pcie-xilinx.c
> +++ b/drivers/pci/host/pcie-xilinx.c
> @@ -440,8 +440,8 @@ static irqreturn_t xilinx_pcie_intr_handler(int irq, =
void
> *data)
>  				   XILINX_PCIE_REG_RPIFR1);
>=20
>  			/* Handle INTx Interrupt */
> -			val =3D ((val & XILINX_PCIE_RPIFR1_INTR_MASK) >>
> -				XILINX_PCIE_RPIFR1_INTR_SHIFT) + 1;
> +			val =3D (val & XILINX_PCIE_RPIFR1_INTR_MASK) >>
> +				XILINX_PCIE_RPIFR1_INTR_SHIFT;
>  			generic_handle_irq(irq_find_mapping(port-
> >leg_domain,
>  							    val));
>  		}
> --
> 2.13.0

Re: [PATCH v4 0/4] PCI: xilinx: Fixes, optimisation & MIPS support

[Bjorn Helgaas]

On Fri, Jun 02, 2017 at 11:54:15AM -0700, Paul Burton wrote:
> This series fixes an issue found using INTx interrupts with the Xilinx
> AXI PCIe Host Bridge IP on the Imagination Technologies MIPS Boston
> development board, performs a couple of optimisations to interrupt
> handling & allows the driver to be used on MIPS systems.
> 
> Applies atop v4.12-rc3.
> 
> Paul Burton (4):
>   PCI: xilinx: Fix INTX irq dispatch
>   PCI: xilinx: Unify INTx & MSI interrupt decode
>   PCI: xilinx: Don't enable config completion interrupts
>   PCI: xilinx: Allow build on MIPS platforms
> 
>  drivers/pci/host/Kconfig       |  2 +-
>  drivers/pci/host/pcie-xilinx.c | 57 +++++++++++++++---------------------------
>  2 files changed, 21 insertions(+), 38 deletions(-)

Hi Paul, et al.,

Waiting for resolution on patch 1 and ack from maintainers.

Re: [PATCH v4 0/4] PCI: xilinx: Fixes, optimisation & MIPS support

[Paul Burton]

[-- Attachment #1: Type: text/plain, Size: 998 bytes --]

On Thursday, 15 June 2017 14:35:05 PDT Bjorn Helgaas wrote:
> On Fri, Jun 02, 2017 at 11:54:15AM -0700, Paul Burton wrote:
> > This series fixes an issue found using INTx interrupts with the Xilinx
> > AXI PCIe Host Bridge IP on the Imagination Technologies MIPS Boston
> > development board, performs a couple of optimisations to interrupt
> > handling & allows the driver to be used on MIPS systems.
> > 
> > Applies atop v4.12-rc3.
> > 
> > Paul Burton (4):
> >   PCI: xilinx: Fix INTX irq dispatch
> >   PCI: xilinx: Unify INTx & MSI interrupt decode
> >   PCI: xilinx: Don't enable config completion interrupts
> >   PCI: xilinx: Allow build on MIPS platforms
> >  
> >  drivers/pci/host/Kconfig       |  2 +-
> >  drivers/pci/host/pcie-xilinx.c | 57
> >  +++++++++++++++--------------------------- 2 files changed, 21
> >  insertions(+), 38 deletions(-)
> 
> Hi Paul, et al.,
> 
> Waiting for resolution on patch 1 and ack from maintainers.

Thanks Bjorn, hopefully v5 will pass muster.

Paul

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]