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

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH v5 0/4] PCI: xilinx: Fixes, optimisation & MIPS support
@ 2017-06-17 19:57 ` Paul Burton
  0 siblings, 0 replies; 27+ messages in thread
From: Paul Burton @ 2017-06-17 19:57 UTC (permalink / raw)
  To: linux-pci
  Cc: Bharat Kumar Gogada, Ravikiran Gummaluri, Bjorn Helgaas,
	Michal Simek, linux-mips, 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-rc5.

Paul Burton (4):
  PCI: xilinx: Create legacy IRQ domain with size 5
  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 | 55 +++++++++++++++---------------------------
 2 files changed, 20 insertions(+), 37 deletions(-)

-- 
2.13.1

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v5 0/4] PCI: xilinx: Fixes, optimisation & MIPS support
@ 2017-06-17 19:57 ` Paul Burton
  0 siblings, 0 replies; 27+ messages in thread
From: Paul Burton @ 2017-06-17 19:57 UTC (permalink / raw)
  To: linux-pci
  Cc: Bharat Kumar Gogada, Ravikiran Gummaluri, Bjorn Helgaas,
	Michal Simek, linux-mips, 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-rc5.

Paul Burton (4):
  PCI: xilinx: Create legacy IRQ domain with size 5
  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 | 55 +++++++++++++++---------------------------
 2 files changed, 20 insertions(+), 37 deletions(-)

-- 
2.13.1

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v5 1/4] PCI: xilinx: Create legacy IRQ domain with size 5
@ 2017-06-17 19:57   ` Paul Burton
  0 siblings, 0 replies; 27+ messages in thread
From: Paul Burton @ 2017-06-17 19:57 UTC (permalink / raw)
  To: linux-pci
  Cc: Bharat Kumar Gogada, Ravikiran Gummaluri, Bjorn Helgaas,
	Michal Simek, linux-mips, Paul Burton

The driver expects to use hardware IRQ numbers 1 through 4 for INTX
interrupts, but only creates an IRQ domain of size 4 (ie. IRQ numbers 0
through 3). This results in a warning from irq_domain_associate when it
is called with hwirq=4:

     WARNING: CPU: 0 PID: 1 at kernel/irq/irqdomain.c:365
         irq_domain_associate+0x170/0x220
     error: hwirq 0x4 is too large for dummy
     Modules linked in:
     CPU: 0 PID: 1 Comm: swapper/0 Tainted: G        W
         4.12.0-rc5-00126-g19e1b3a10aad-dirty #427
     Stack : 0000000000000000 0000000000000004 0000000000000006 ffffffff8092c78a
             0000000000000061 ffffffff8018bf60 0000000000000000 0000000000000000
             ffffffff8088c287 ffffffff80811d18 a8000000ffc60000 ffffffff80926678
             0000000000000001 0000000000000000 ffffffff80887880 ffffffff80960000
             ffffffff80920000 ffffffff801e6744 ffffffff80887880 a8000000ffc4f8f8
             000000000000089c ffffffff8018d260 0000000000010000 ffffffff80811d18
             0000000000000000 0000000000000001 0000000000000000 0000000000000000
             0000000000000000 a8000000ffc4f840 0000000000000000 ffffffff8042cf34
             0000000000000000 0000000000000000 0000000000000000 0000000000040c00
             0000000000000000 ffffffff8010d1c8 0000000000000000 ffffffff8042cf34
             ...
     Call Trace:
     [<ffffffff8010d1c8>] show_stack+0x80/0xa0
     [<ffffffff8042cf34>] dump_stack+0xd4/0x110
     [<ffffffff8013ea98>] __warn+0xf0/0x108
     [<ffffffff8013eb14>] warn_slowpath_fmt+0x3c/0x48
     [<ffffffff80196528>] irq_domain_associate+0x170/0x220
     [<ffffffff80196bf0>] irq_create_mapping+0x88/0x118
     [<ffffffff801976a8>] irq_create_fwspec_mapping+0xb8/0x320
     [<ffffffff80197970>] irq_create_of_mapping+0x60/0x70
     [<ffffffff805d1318>] of_irq_parse_and_map_pci+0x20/0x38
     [<ffffffff8049c210>] pci_fixup_irqs+0x60/0xe0
     [<ffffffff8049cd64>] xilinx_pcie_probe+0x28c/0x478
     [<ffffffff804e8ca8>] platform_drv_probe+0x50/0xd0
     [<ffffffff804e73a4>] driver_probe_device+0x2c4/0x3a0
     [<ffffffff804e7544>] __driver_attach+0xc4/0xd0
     [<ffffffff804e5254>] bus_for_each_dev+0x64/0xa8
     [<ffffffff804e5e40>] bus_add_driver+0x1f0/0x268
     [<ffffffff804e8000>] driver_register+0x68/0x118
     [<ffffffff801001a4>] do_one_initcall+0x4c/0x178
     [<ffffffff808d3ca8>] kernel_init_freeable+0x204/0x2b0
     [<ffffffff80730b68>] kernel_init+0x10/0xf8
     [<ffffffff80106218>] ret_from_kernel_thread+0x14/0x1c

This patch avoids that warning by creating the legacy IRQ domain with
size 5 rather than 4, allowing it to cover the hwirq=4/INTD case.

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 v5:
- New patch; replacing "PCI: xilinx: Fix INTX irq dispatch".

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

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

diff --git a/drivers/pci/host/pcie-xilinx.c b/drivers/pci/host/pcie-xilinx.c
index 2fe2df51f9f8..94c71fb91648 100644
--- a/drivers/pci/host/pcie-xilinx.c
+++ b/drivers/pci/host/pcie-xilinx.c
@@ -524,7 +524,7 @@ static int xilinx_pcie_init_irq_domain(struct xilinx_pcie_port *port)
 		return -ENODEV;
 	}
 
-	port->leg_domain = irq_domain_add_linear(pcie_intc_node, 4,
+	port->leg_domain = irq_domain_add_linear(pcie_intc_node, 1 + 4,
 						 &intx_domain_ops,
 						 port);
 	if (!port->leg_domain) {
-- 
2.13.1

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v5 1/4] PCI: xilinx: Create legacy IRQ domain with size 5
@ 2017-06-17 19:57   ` Paul Burton
  0 siblings, 0 replies; 27+ messages in thread
From: Paul Burton @ 2017-06-17 19:57 UTC (permalink / raw)
  To: linux-pci
  Cc: Bharat Kumar Gogada, Ravikiran Gummaluri, Bjorn Helgaas,
	Michal Simek, linux-mips, Paul Burton

The driver expects to use hardware IRQ numbers 1 through 4 for INTX
interrupts, but only creates an IRQ domain of size 4 (ie. IRQ numbers 0
through 3). This results in a warning from irq_domain_associate when it
is called with hwirq=4:

     WARNING: CPU: 0 PID: 1 at kernel/irq/irqdomain.c:365
         irq_domain_associate+0x170/0x220
     error: hwirq 0x4 is too large for dummy
     Modules linked in:
     CPU: 0 PID: 1 Comm: swapper/0 Tainted: G        W
         4.12.0-rc5-00126-g19e1b3a10aad-dirty #427
     Stack : 0000000000000000 0000000000000004 0000000000000006 ffffffff8092c78a
             0000000000000061 ffffffff8018bf60 0000000000000000 0000000000000000
             ffffffff8088c287 ffffffff80811d18 a8000000ffc60000 ffffffff80926678
             0000000000000001 0000000000000000 ffffffff80887880 ffffffff80960000
             ffffffff80920000 ffffffff801e6744 ffffffff80887880 a8000000ffc4f8f8
             000000000000089c ffffffff8018d260 0000000000010000 ffffffff80811d18
             0000000000000000 0000000000000001 0000000000000000 0000000000000000
             0000000000000000 a8000000ffc4f840 0000000000000000 ffffffff8042cf34
             0000000000000000 0000000000000000 0000000000000000 0000000000040c00
             0000000000000000 ffffffff8010d1c8 0000000000000000 ffffffff8042cf34
             ...
     Call Trace:
     [<ffffffff8010d1c8>] show_stack+0x80/0xa0
     [<ffffffff8042cf34>] dump_stack+0xd4/0x110
     [<ffffffff8013ea98>] __warn+0xf0/0x108
     [<ffffffff8013eb14>] warn_slowpath_fmt+0x3c/0x48
     [<ffffffff80196528>] irq_domain_associate+0x170/0x220
     [<ffffffff80196bf0>] irq_create_mapping+0x88/0x118
     [<ffffffff801976a8>] irq_create_fwspec_mapping+0xb8/0x320
     [<ffffffff80197970>] irq_create_of_mapping+0x60/0x70
     [<ffffffff805d1318>] of_irq_parse_and_map_pci+0x20/0x38
     [<ffffffff8049c210>] pci_fixup_irqs+0x60/0xe0
     [<ffffffff8049cd64>] xilinx_pcie_probe+0x28c/0x478
     [<ffffffff804e8ca8>] platform_drv_probe+0x50/0xd0
     [<ffffffff804e73a4>] driver_probe_device+0x2c4/0x3a0
     [<ffffffff804e7544>] __driver_attach+0xc4/0xd0
     [<ffffffff804e5254>] bus_for_each_dev+0x64/0xa8
     [<ffffffff804e5e40>] bus_add_driver+0x1f0/0x268
     [<ffffffff804e8000>] driver_register+0x68/0x118
     [<ffffffff801001a4>] do_one_initcall+0x4c/0x178
     [<ffffffff808d3ca8>] kernel_init_freeable+0x204/0x2b0
     [<ffffffff80730b68>] kernel_init+0x10/0xf8
     [<ffffffff80106218>] ret_from_kernel_thread+0x14/0x1c

This patch avoids that warning by creating the legacy IRQ domain with
size 5 rather than 4, allowing it to cover the hwirq=4/INTD case.

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 v5:
- New patch; replacing "PCI: xilinx: Fix INTX irq dispatch".

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

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

diff --git a/drivers/pci/host/pcie-xilinx.c b/drivers/pci/host/pcie-xilinx.c
index 2fe2df51f9f8..94c71fb91648 100644
--- a/drivers/pci/host/pcie-xilinx.c
+++ b/drivers/pci/host/pcie-xilinx.c
@@ -524,7 +524,7 @@ static int xilinx_pcie_init_irq_domain(struct xilinx_pcie_port *port)
 		return -ENODEV;
 	}
 
-	port->leg_domain = irq_domain_add_linear(pcie_intc_node, 4,
+	port->leg_domain = irq_domain_add_linear(pcie_intc_node, 1 + 4,
 						 &intx_domain_ops,
 						 port);
 	if (!port->leg_domain) {
-- 
2.13.1

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v5 2/4] PCI: xilinx: Unify INTx & MSI interrupt decode
@ 2017-06-17 19:57   ` Paul Burton
  0 siblings, 0 replies; 27+ messages in thread
From: Paul Burton @ 2017-06-17 19:57 UTC (permalink / raw)
  To: linux-pci
  Cc: Bharat Kumar Gogada, Ravikiran Gummaluri, Bjorn Helgaas,
	Michal Simek, linux-mips, 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 v5: None
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 94c71fb91648..5436657d142d 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) + 1;
-			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.1

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v5 2/4] PCI: xilinx: Unify INTx & MSI interrupt decode
@ 2017-06-17 19:57   ` Paul Burton
  0 siblings, 0 replies; 27+ messages in thread
From: Paul Burton @ 2017-06-17 19:57 UTC (permalink / raw)
  To: linux-pci
  Cc: Bharat Kumar Gogada, Ravikiran Gummaluri, Bjorn Helgaas,
	Michal Simek, linux-mips, 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 v5: None
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 94c71fb91648..5436657d142d 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) + 1;
-			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.1

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v5 3/4] PCI: xilinx: Don't enable config completion interrupts
@ 2017-06-17 19:57   ` Paul Burton
  0 siblings, 0 replies; 27+ messages in thread
From: Paul Burton @ 2017-06-17 19:57 UTC (permalink / raw)
  To: linux-pci
  Cc: Bharat Kumar Gogada, Ravikiran Gummaluri, Bjorn Helgaas,
	Michal Simek, linux-mips, 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 v5: None
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 5436657d142d..176ad1608d88 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.1

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v5 3/4] PCI: xilinx: Don't enable config completion interrupts
@ 2017-06-17 19:57   ` Paul Burton
  0 siblings, 0 replies; 27+ messages in thread
From: Paul Burton @ 2017-06-17 19:57 UTC (permalink / raw)
  To: linux-pci
  Cc: Bharat Kumar Gogada, Ravikiran Gummaluri, Bjorn Helgaas,
	Michal Simek, linux-mips, 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 v5: None
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 5436657d142d..176ad1608d88 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.1

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v5 4/4] PCI: xilinx: Allow build on MIPS platforms
@ 2017-06-17 19:57   ` Paul Burton
  0 siblings, 0 replies; 27+ messages in thread
From: Paul Burton @ 2017-06-17 19:57 UTC (permalink / raw)
  To: linux-pci
  Cc: Bharat Kumar Gogada, Ravikiran Gummaluri, Bjorn Helgaas,
	Michal Simek, linux-mips, 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 v5: None

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.1

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v5 4/4] PCI: xilinx: Allow build on MIPS platforms
@ 2017-06-17 19:57   ` Paul Burton
  0 siblings, 0 replies; 27+ messages in thread
From: Paul Burton @ 2017-06-17 19:57 UTC (permalink / raw)
  To: linux-pci
  Cc: Bharat Kumar Gogada, Ravikiran Gummaluri, Bjorn Helgaas,
	Michal Simek, linux-mips, 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 v5: None

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.1

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 1/4] PCI: xilinx: Create legacy IRQ domain with size 5
  2017-06-17 19:57   ` Paul Burton
  (?)
@ 2017-06-19 23:47   ` Bjorn Helgaas
  2017-06-20  0:38       ` Ley Foon Tan
  -1 siblings, 1 reply; 27+ messages in thread
From: Bjorn Helgaas @ 2017-06-19 23:47 UTC (permalink / raw)
  To: Paul Burton
  Cc: linux-pci, Bharat Kumar Gogada, Ravikiran Gummaluri,
	Bjorn Helgaas, Michal Simek, linux-mips, Thomas Gleixner,
	Ley Foon Tan

[+cc Thomas, Ley Foon]

On Sat, Jun 17, 2017 at 12:57:38PM -0700, Paul Burton wrote:
> The driver expects to use hardware IRQ numbers 1 through 4 for INTX
> interrupts, but only creates an IRQ domain of size 4 (ie. IRQ numbers 0
> through 3). This results in a warning from irq_domain_associate when it
> is called with hwirq=4:
> 
>      WARNING: CPU: 0 PID: 1 at kernel/irq/irqdomain.c:365
>          irq_domain_associate+0x170/0x220
>      error: hwirq 0x4 is too large for dummy
>      Modules linked in:
>      CPU: 0 PID: 1 Comm: swapper/0 Tainted: G        W
>          4.12.0-rc5-00126-g19e1b3a10aad-dirty #427
>      Stack : 0000000000000000 0000000000000004 0000000000000006 ffffffff8092c78a
>              0000000000000061 ffffffff8018bf60 0000000000000000 0000000000000000
>              ffffffff8088c287 ffffffff80811d18 a8000000ffc60000 ffffffff80926678
>              0000000000000001 0000000000000000 ffffffff80887880 ffffffff80960000
>              ffffffff80920000 ffffffff801e6744 ffffffff80887880 a8000000ffc4f8f8
>              000000000000089c ffffffff8018d260 0000000000010000 ffffffff80811d18
>              0000000000000000 0000000000000001 0000000000000000 0000000000000000
>              0000000000000000 a8000000ffc4f840 0000000000000000 ffffffff8042cf34
>              0000000000000000 0000000000000000 0000000000000000 0000000000040c00
>              0000000000000000 ffffffff8010d1c8 0000000000000000 ffffffff8042cf34
>              ...
>      Call Trace:
>      [<ffffffff8010d1c8>] show_stack+0x80/0xa0
>      [<ffffffff8042cf34>] dump_stack+0xd4/0x110
>      [<ffffffff8013ea98>] __warn+0xf0/0x108
>      [<ffffffff8013eb14>] warn_slowpath_fmt+0x3c/0x48
>      [<ffffffff80196528>] irq_domain_associate+0x170/0x220
>      [<ffffffff80196bf0>] irq_create_mapping+0x88/0x118
>      [<ffffffff801976a8>] irq_create_fwspec_mapping+0xb8/0x320
>      [<ffffffff80197970>] irq_create_of_mapping+0x60/0x70
>      [<ffffffff805d1318>] of_irq_parse_and_map_pci+0x20/0x38
>      [<ffffffff8049c210>] pci_fixup_irqs+0x60/0xe0
>      [<ffffffff8049cd64>] xilinx_pcie_probe+0x28c/0x478
>      [<ffffffff804e8ca8>] platform_drv_probe+0x50/0xd0
>      [<ffffffff804e73a4>] driver_probe_device+0x2c4/0x3a0
>      [<ffffffff804e7544>] __driver_attach+0xc4/0xd0
>      [<ffffffff804e5254>] bus_for_each_dev+0x64/0xa8
>      [<ffffffff804e5e40>] bus_add_driver+0x1f0/0x268
>      [<ffffffff804e8000>] driver_register+0x68/0x118
>      [<ffffffff801001a4>] do_one_initcall+0x4c/0x178
>      [<ffffffff808d3ca8>] kernel_init_freeable+0x204/0x2b0
>      [<ffffffff80730b68>] kernel_init+0x10/0xf8
>      [<ffffffff80106218>] ret_from_kernel_thread+0x14/0x1c
> 
> This patch avoids that warning by creating the legacy IRQ domain with
> size 5 rather than 4, allowing it to cover the hwirq=4/INTD case.
> 
> 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 v5:
> - New patch; replacing "PCI: xilinx: Fix INTX irq dispatch".
> 
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None
> 
>  drivers/pci/host/pcie-xilinx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/host/pcie-xilinx.c b/drivers/pci/host/pcie-xilinx.c
> index 2fe2df51f9f8..94c71fb91648 100644
> --- a/drivers/pci/host/pcie-xilinx.c
> +++ b/drivers/pci/host/pcie-xilinx.c
> @@ -524,7 +524,7 @@ static int xilinx_pcie_init_irq_domain(struct xilinx_pcie_port *port)
>  		return -ENODEV;
>  	}
>  
> -	port->leg_domain = irq_domain_add_linear(pcie_intc_node, 4,
> +	port->leg_domain = irq_domain_add_linear(pcie_intc_node, 1 + 4,

I don't understand this.  Several drivers call irq_domain_add_linear() with
a size of 4:

  dra7xx_pcie_init_irq_domain
  ks_dw_pcie_host_init
  advk_pcie_init_irq_domain
  faraday_pci_setup_cascaded_irq
  rockchip_pcie_init_irq_domain
  nwl_pcie_init_irq_domain

Only one other in drivers/pci uses a size of 5:

  altera_pcie_init_irq_domain

Why can't we use a size of 4 for all of them?  We only have INTA-INTD.  Are
altera and xilinx missing something to apply an offset from the 0-3 space
to the 1-4 space?

>  						 &intx_domain_ops,
>  						 port);
>  	if (!port->leg_domain) {
> -- 
> 2.13.1
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 1/4] PCI: xilinx: Create legacy IRQ domain with size 5
  2017-06-19 23:47   ` Bjorn Helgaas
@ 2017-06-20  0:38       ` Ley Foon Tan
  0 siblings, 0 replies; 27+ messages in thread
From: Ley Foon Tan @ 2017-06-20  0:38 UTC (permalink / raw)
  To: Bjorn Helgaas, Paul Burton
  Cc: linux-pci, Bharat Kumar Gogada, Ravikiran Gummaluri,
	Bjorn Helgaas, Michal Simek, linux-mips, Thomas Gleixner,
	Ley Foon Tan

On Mon, 2017-06-19 at 18:47 -0500, Bjorn Helgaas wrote:
> [+cc Thomas, Ley Foon]
>=20
> On Sat, Jun 17, 2017 at 12:57:38PM -0700, Paul Burton wrote:
> >=20
> > The driver expects to use hardware IRQ numbers 1 through 4 for INTX
> > interrupts, but only creates an IRQ domain of size 4 (ie. IRQ
> > numbers 0
> > through 3). This results in a warning from irq_domain_associate
> > when it
> > is called with hwirq=3D4:
> >=20
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0WARNING: CPU: 0 PID: 1 at kernel/irq/irqd=
omain.c:365
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0irq_domain_associ=
ate+0x170/0x220
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0error: hwirq 0x4 is too large for dummy
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0Modules linked in:
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0CPU: 0 PID: 1 Comm: swapper/0 Tainted: G=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0W
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A04.12.0-rc5-00126-=
g19e1b3a10aad-dirty #427
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0Stack : 0000000000000000 0000000000000004=
 0000000000000006
> > ffffffff8092c78a
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A00000000000000061 ffffffff8018bf60 0000000000000000
> > 0000000000000000
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0ffffffff8088c287 ffffffff80811d18 a8000000ffc60000
> > ffffffff80926678
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A00000000000000001 0000000000000000 ffffffff80887880
> > ffffffff80960000
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0ffffffff80920000 ffffffff801e6744 ffffffff80887880
> > a8000000ffc4f8f8
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0000000000000089c ffffffff8018d260 0000000000010000
> > ffffffff80811d18
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A00000000000000000 0000000000000001 0000000000000000
> > 0000000000000000
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A00000000000000000 a8000000ffc4f840 0000000000000000
> > ffffffff8042cf34
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A00000000000000000 0000000000000000 0000000000000000
> > 0000000000040c00
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A00000000000000000 ffffffff8010d1c8 0000000000000000
> > ffffffff8042cf34
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0...
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0Call Trace:
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff8010d1c8>] show_stack+0x80/0xa0
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff8042cf34>] dump_stack+0xd4/0x11=
0
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff8013ea98>] __warn+0xf0/0x108
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff8013eb14>] warn_slowpath_fmt+0x=
3c/0x48
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff80196528>] irq_domain_associate=
+0x170/0x220
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff80196bf0>] irq_create_mapping+0=
x88/0x118
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff801976a8>] irq_create_fwspec_ma=
pping+0xb8/0x320
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff80197970>] irq_create_of_mappin=
g+0x60/0x70
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff805d1318>] of_irq_parse_and_map=
_pci+0x20/0x38
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff8049c210>] pci_fixup_irqs+0x60/=
0xe0
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff8049cd64>] xilinx_pcie_probe+0x=
28c/0x478
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff804e8ca8>] platform_drv_probe+0=
x50/0xd0
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff804e73a4>] driver_probe_device+=
0x2c4/0x3a0
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff804e7544>] __driver_attach+0xc4=
/0xd0
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff804e5254>] bus_for_each_dev+0x6=
4/0xa8
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff804e5e40>] bus_add_driver+0x1f0=
/0x268
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff804e8000>] driver_register+0x68=
/0x118
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff801001a4>] do_one_initcall+0x4c=
/0x178
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff808d3ca8>] kernel_init_freeable=
+0x204/0x2b0
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff80730b68>] kernel_init+0x10/0xf=
8
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff80106218>] ret_from_kernel_thre=
ad+0x14/0x1c
> >=20
> > This patch avoids that warning by creating the legacy IRQ domain
> > with
> > size 5 rather than 4, allowing it to cover the hwirq=3D4/INTD case.
> >=20
> > 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
> >=20
> > ---
> >=20
> > Changes in v5:
> > - New patch; replacing "PCI: xilinx: Fix INTX irq dispatch".
> >=20
> > Changes in v4: None
> > Changes in v3: None
> > Changes in v2: None
> >=20
> > =C2=A0drivers/pci/host/pcie-xilinx.c | 2 +-
> > =C2=A01 file changed, 1 insertion(+), 1 deletion(-)
> >=20
> > diff --git a/drivers/pci/host/pcie-xilinx.c
> > b/drivers/pci/host/pcie-xilinx.c
> > index 2fe2df51f9f8..94c71fb91648 100644
> > --- a/drivers/pci/host/pcie-xilinx.c
> > +++ b/drivers/pci/host/pcie-xilinx.c
> > @@ -524,7 +524,7 @@ static int xilinx_pcie_init_irq_domain(struct
> > xilinx_pcie_port *port)
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0return -ENODEV;
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0}
> >=20
> > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0port->leg_domain =3D irq_domain_add_line=
ar(pcie_intc_node, 4,
> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0port->leg_domain =3D irq_domain_add_line=
ar(pcie_intc_node, 1 +
> > 4,
> I don't understand this.=C2=A0=C2=A0Several drivers call
> irq_domain_add_linear() with
> a size of 4:
>=20
> =C2=A0 dra7xx_pcie_init_irq_domain
> =C2=A0 ks_dw_pcie_host_init
> =C2=A0 advk_pcie_init_irq_domain
> =C2=A0 faraday_pci_setup_cascaded_irq
> =C2=A0 rockchip_pcie_init_irq_domain
> =C2=A0 nwl_pcie_init_irq_domain
>=20
> Only one other in drivers/pci uses a size of 5:
>=20
> =C2=A0 altera_pcie_init_irq_domain
>=20
> Why can't we use a size of 4 for all of them?=C2=A0=C2=A0We only have INT=
A-
> INTD.=C2=A0=C2=A0Are
> altera and xilinx missing something to apply an offset from the 0-3
> space
> to the 1-4 space?
We have the same discussion before in 2016:=C2=A0https://lkml.org/lkml/2016=
/
8/30/198

This is because legacy interrupt is start with index 1 instead of 0.

>=20
> >=20
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0&intx_domain=
_ops,
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0port);
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0if (!port->leg_domain) {
> > --
> > 2.13.1
> >=20
> ________________________________
>=20
> Confidentiality Notice.
> This message may contain information that is confidential or
> otherwise protected from disclosure. If you are not the intended
> recipient, you are hereby notified that any use, disclosure,
> dissemination, distribution, or copying of this message, or any
> attachments, is strictly prohibited. If you have received this
> message in error, please advise the sender by reply e-mail, and
> delete the message and any attachments. Thank you.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 1/4] PCI: xilinx: Create legacy IRQ domain with size 5
@ 2017-06-20  0:38       ` Ley Foon Tan
  0 siblings, 0 replies; 27+ messages in thread
From: Ley Foon Tan @ 2017-06-20  0:38 UTC (permalink / raw)
  To: Bjorn Helgaas, Paul Burton
  Cc: linux-pci, Bharat Kumar Gogada, Ravikiran Gummaluri,
	Bjorn Helgaas, Michal Simek, linux-mips, Thomas Gleixner,
	Ley Foon Tan

On Mon, 2017-06-19 at 18:47 -0500, Bjorn Helgaas wrote:
> [+cc Thomas, Ley Foon]
> 
> On Sat, Jun 17, 2017 at 12:57:38PM -0700, Paul Burton wrote:
> > 
> > The driver expects to use hardware IRQ numbers 1 through 4 for INTX
> > interrupts, but only creates an IRQ domain of size 4 (ie. IRQ
> > numbers 0
> > through 3). This results in a warning from irq_domain_associate
> > when it
> > is called with hwirq=4:
> > 
> >      WARNING: CPU: 0 PID: 1 at kernel/irq/irqdomain.c:365
> >          irq_domain_associate+0x170/0x220
> >      error: hwirq 0x4 is too large for dummy
> >      Modules linked in:
> >      CPU: 0 PID: 1 Comm: swapper/0 Tainted: G        W
> >          4.12.0-rc5-00126-g19e1b3a10aad-dirty #427
> >      Stack : 0000000000000000 0000000000000004 0000000000000006
> > ffffffff8092c78a
> >              0000000000000061 ffffffff8018bf60 0000000000000000
> > 0000000000000000
> >              ffffffff8088c287 ffffffff80811d18 a8000000ffc60000
> > ffffffff80926678
> >              0000000000000001 0000000000000000 ffffffff80887880
> > ffffffff80960000
> >              ffffffff80920000 ffffffff801e6744 ffffffff80887880
> > a8000000ffc4f8f8
> >              000000000000089c ffffffff8018d260 0000000000010000
> > ffffffff80811d18
> >              0000000000000000 0000000000000001 0000000000000000
> > 0000000000000000
> >              0000000000000000 a8000000ffc4f840 0000000000000000
> > ffffffff8042cf34
> >              0000000000000000 0000000000000000 0000000000000000
> > 0000000000040c00
> >              0000000000000000 ffffffff8010d1c8 0000000000000000
> > ffffffff8042cf34
> >              ...
> >      Call Trace:
> >      [<ffffffff8010d1c8>] show_stack+0x80/0xa0
> >      [<ffffffff8042cf34>] dump_stack+0xd4/0x110
> >      [<ffffffff8013ea98>] __warn+0xf0/0x108
> >      [<ffffffff8013eb14>] warn_slowpath_fmt+0x3c/0x48
> >      [<ffffffff80196528>] irq_domain_associate+0x170/0x220
> >      [<ffffffff80196bf0>] irq_create_mapping+0x88/0x118
> >      [<ffffffff801976a8>] irq_create_fwspec_mapping+0xb8/0x320
> >      [<ffffffff80197970>] irq_create_of_mapping+0x60/0x70
> >      [<ffffffff805d1318>] of_irq_parse_and_map_pci+0x20/0x38
> >      [<ffffffff8049c210>] pci_fixup_irqs+0x60/0xe0
> >      [<ffffffff8049cd64>] xilinx_pcie_probe+0x28c/0x478
> >      [<ffffffff804e8ca8>] platform_drv_probe+0x50/0xd0
> >      [<ffffffff804e73a4>] driver_probe_device+0x2c4/0x3a0
> >      [<ffffffff804e7544>] __driver_attach+0xc4/0xd0
> >      [<ffffffff804e5254>] bus_for_each_dev+0x64/0xa8
> >      [<ffffffff804e5e40>] bus_add_driver+0x1f0/0x268
> >      [<ffffffff804e8000>] driver_register+0x68/0x118
> >      [<ffffffff801001a4>] do_one_initcall+0x4c/0x178
> >      [<ffffffff808d3ca8>] kernel_init_freeable+0x204/0x2b0
> >      [<ffffffff80730b68>] kernel_init+0x10/0xf8
> >      [<ffffffff80106218>] ret_from_kernel_thread+0x14/0x1c
> > 
> > This patch avoids that warning by creating the legacy IRQ domain
> > with
> > size 5 rather than 4, allowing it to cover the hwirq=4/INTD case.
> > 
> > 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 v5:
> > - New patch; replacing "PCI: xilinx: Fix INTX irq dispatch".
> > 
> > Changes in v4: None
> > Changes in v3: None
> > Changes in v2: None
> > 
> >  drivers/pci/host/pcie-xilinx.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pci/host/pcie-xilinx.c
> > b/drivers/pci/host/pcie-xilinx.c
> > index 2fe2df51f9f8..94c71fb91648 100644
> > --- a/drivers/pci/host/pcie-xilinx.c
> > +++ b/drivers/pci/host/pcie-xilinx.c
> > @@ -524,7 +524,7 @@ static int xilinx_pcie_init_irq_domain(struct
> > xilinx_pcie_port *port)
> >               return -ENODEV;
> >       }
> > 
> > -     port->leg_domain = irq_domain_add_linear(pcie_intc_node, 4,
> > +     port->leg_domain = irq_domain_add_linear(pcie_intc_node, 1 +
> > 4,
> I don't understand this.  Several drivers call
> irq_domain_add_linear() with
> a size of 4:
> 
>   dra7xx_pcie_init_irq_domain
>   ks_dw_pcie_host_init
>   advk_pcie_init_irq_domain
>   faraday_pci_setup_cascaded_irq
>   rockchip_pcie_init_irq_domain
>   nwl_pcie_init_irq_domain
> 
> Only one other in drivers/pci uses a size of 5:
> 
>   altera_pcie_init_irq_domain
> 
> Why can't we use a size of 4 for all of them?  We only have INTA-
> INTD.  Are
> altera and xilinx missing something to apply an offset from the 0-3
> space
> to the 1-4 space?
We have the same discussion before in 2016: https://lkml.org/lkml/2016/
8/30/198

This is because legacy interrupt is start with index 1 instead of 0.

> 
> > 
> >                                                &intx_domain_ops,
> >                                                port);
> >       if (!port->leg_domain) {
> > --
> > 2.13.1
> > 
> ________________________________
> 
> Confidentiality Notice.
> This message may contain information that is confidential or
> otherwise protected from disclosure. If you are not the intended
> recipient, you are hereby notified that any use, disclosure,
> dissemination, distribution, or copying of this message, or any
> attachments, is strictly prohibited. If you have received this
> message in error, please advise the sender by reply e-mail, and
> delete the message and any attachments. Thank you.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 1/4] PCI: xilinx: Create legacy IRQ domain with size 5
  2017-06-20  0:38       ` Ley Foon Tan
  (?)
@ 2017-06-20  1:49       ` Bjorn Helgaas
  2017-06-20  1:55           ` Ley Foon Tan
  2017-06-20  2:07           ` Paul Burton
  -1 siblings, 2 replies; 27+ messages in thread
From: Bjorn Helgaas @ 2017-06-20  1:49 UTC (permalink / raw)
  To: Ley Foon Tan
  Cc: Paul Burton, linux-pci, Bharat Kumar Gogada, Ravikiran Gummaluri,
	Bjorn Helgaas, Michal Simek, linux-mips, Thomas Gleixner,
	Ley Foon Tan, Marc Zyngier

[+cc Marc]

On Tue, Jun 20, 2017 at 08:38:14AM +0800, Ley Foon Tan wrote:
> On Mon, 2017-06-19 at 18:47 -0500, Bjorn Helgaas wrote:
> > [+cc Thomas, Ley Foon]
> > 
> > On Sat, Jun 17, 2017 at 12:57:38PM -0700, Paul Burton wrote:
> > > 
> > > The driver expects to use hardware IRQ numbers 1 through 4 for INTX
> > > interrupts, but only creates an IRQ domain of size 4 (ie. IRQ
> > > numbers 0
> > > through 3). This results in a warning from irq_domain_associate
> > > when it
> > > is called with hwirq=4:
> > > 
> > >      WARNING: CPU: 0 PID: 1 at kernel/irq/irqdomain.c:365
> > >          irq_domain_associate+0x170/0x220
> > >      error: hwirq 0x4 is too large for dummy
> > >      Modules linked in:
> > >      CPU: 0 PID: 1 Comm: swapper/0 Tainted: G        W
> > >          4.12.0-rc5-00126-g19e1b3a10aad-dirty #427
> > >      Stack : 0000000000000000 0000000000000004 0000000000000006
> > > ffffffff8092c78a
> > >              0000000000000061 ffffffff8018bf60 0000000000000000
> > > 0000000000000000
> > >              ffffffff8088c287 ffffffff80811d18 a8000000ffc60000
> > > ffffffff80926678
> > >              0000000000000001 0000000000000000 ffffffff80887880
> > > ffffffff80960000
> > >              ffffffff80920000 ffffffff801e6744 ffffffff80887880
> > > a8000000ffc4f8f8
> > >              000000000000089c ffffffff8018d260 0000000000010000
> > > ffffffff80811d18
> > >              0000000000000000 0000000000000001 0000000000000000
> > > 0000000000000000
> > >              0000000000000000 a8000000ffc4f840 0000000000000000
> > > ffffffff8042cf34
> > >              0000000000000000 0000000000000000 0000000000000000
> > > 0000000000040c00
> > >              0000000000000000 ffffffff8010d1c8 0000000000000000
> > > ffffffff8042cf34
> > >              ...
> > >      Call Trace:
> > >      [<ffffffff8010d1c8>] show_stack+0x80/0xa0
> > >      [<ffffffff8042cf34>] dump_stack+0xd4/0x110
> > >      [<ffffffff8013ea98>] __warn+0xf0/0x108
> > >      [<ffffffff8013eb14>] warn_slowpath_fmt+0x3c/0x48
> > >      [<ffffffff80196528>] irq_domain_associate+0x170/0x220
> > >      [<ffffffff80196bf0>] irq_create_mapping+0x88/0x118
> > >      [<ffffffff801976a8>] irq_create_fwspec_mapping+0xb8/0x320
> > >      [<ffffffff80197970>] irq_create_of_mapping+0x60/0x70
> > >      [<ffffffff805d1318>] of_irq_parse_and_map_pci+0x20/0x38
> > >      [<ffffffff8049c210>] pci_fixup_irqs+0x60/0xe0
> > >      [<ffffffff8049cd64>] xilinx_pcie_probe+0x28c/0x478
> > >      [<ffffffff804e8ca8>] platform_drv_probe+0x50/0xd0
> > >      [<ffffffff804e73a4>] driver_probe_device+0x2c4/0x3a0
> > >      [<ffffffff804e7544>] __driver_attach+0xc4/0xd0
> > >      [<ffffffff804e5254>] bus_for_each_dev+0x64/0xa8
> > >      [<ffffffff804e5e40>] bus_add_driver+0x1f0/0x268
> > >      [<ffffffff804e8000>] driver_register+0x68/0x118
> > >      [<ffffffff801001a4>] do_one_initcall+0x4c/0x178
> > >      [<ffffffff808d3ca8>] kernel_init_freeable+0x204/0x2b0
> > >      [<ffffffff80730b68>] kernel_init+0x10/0xf8
> > >      [<ffffffff80106218>] ret_from_kernel_thread+0x14/0x1c
> > > 
> > > This patch avoids that warning by creating the legacy IRQ domain
> > > with
> > > size 5 rather than 4, allowing it to cover the hwirq=4/INTD case.
> > > 
> > > 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 v5:
> > > - New patch; replacing "PCI: xilinx: Fix INTX irq dispatch".
> > > 
> > > Changes in v4: None
> > > Changes in v3: None
> > > Changes in v2: None
> > > 
> > >  drivers/pci/host/pcie-xilinx.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/pci/host/pcie-xilinx.c
> > > b/drivers/pci/host/pcie-xilinx.c
> > > index 2fe2df51f9f8..94c71fb91648 100644
> > > --- a/drivers/pci/host/pcie-xilinx.c
> > > +++ b/drivers/pci/host/pcie-xilinx.c
> > > @@ -524,7 +524,7 @@ static int xilinx_pcie_init_irq_domain(struct
> > > xilinx_pcie_port *port)
> > >               return -ENODEV;
> > >       }
> > > 
> > > -     port->leg_domain = irq_domain_add_linear(pcie_intc_node, 4,
> > > +     port->leg_domain = irq_domain_add_linear(pcie_intc_node, 1 +
> > > 4,
> > I don't understand this.  Several drivers call
> > irq_domain_add_linear() with
> > a size of 4:
> > 
> >   dra7xx_pcie_init_irq_domain
> >   ks_dw_pcie_host_init
> >   advk_pcie_init_irq_domain
> >   faraday_pci_setup_cascaded_irq
> >   rockchip_pcie_init_irq_domain
> >   nwl_pcie_init_irq_domain
> > 
> > Only one other in drivers/pci uses a size of 5:
> > 
> >   altera_pcie_init_irq_domain
> > 
> > Why can't we use a size of 4 for all of them?  We only have INTA-
> > INTD.  Are
> > altera and xilinx missing something to apply an offset from the 0-3
> > space
> > to the 1-4 space?
> We have the same discussion before in 2016: https://lkml.org/lkml/2016/
> 8/30/198

Thanks for digging that out.  I knew we'd discussed this before, but I
couldn't find it in the archives.  I don't think anybody was really
satisfied with the outcome, but we accepted it to make forward
progress.

> This is because legacy interrupt is start with index 1 instead of 0.

I'm not buying this.  Your argument was that "the hwirq for legacy
interrupts will start at 0x1 to 0x4 (INTA to INTD) and these values
are as per PCIe specification for legacy interrupts.  So these cannot
be numbered from 0."

But all the other drivers I mentioned get along with the 0-3 range
somehow.  If there's something different about altera and xilinx that
means they can't use the same solution the others do, I'd like to know
what it is.

> > >                                                &intx_domain_ops,
> > >                                                port);
> > >       if (!port->leg_domain) {
> > > --
> > > 2.13.1
> > > 
> > ________________________________
> > 
> > Confidentiality Notice.
> > This message may contain information that is confidential or
> > otherwise protected from disclosure. If you are not the intended
> > recipient, you are hereby notified that any use, disclosure,
> > dissemination, distribution, or copying of this message, or any
> > attachments, is strictly prohibited. If you have received this
> > message in error, please advise the sender by reply e-mail, and
> > delete the message and any attachments. Thank you.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 1/4] PCI: xilinx: Create legacy IRQ domain with size 5
  2017-06-20  1:49       ` Bjorn Helgaas
@ 2017-06-20  1:55           ` Ley Foon Tan
  2017-06-20  2:07           ` Paul Burton
  1 sibling, 0 replies; 27+ messages in thread
From: Ley Foon Tan @ 2017-06-20  1:55 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Paul Burton, linux-pci, Bharat Kumar Gogada, Ravikiran Gummaluri,
	Bjorn Helgaas, Michal Simek, linux-mips, Thomas Gleixner,
	Ley Foon Tan, Marc Zyngier

On Mon, 2017-06-19 at 20:49 -0500, Bjorn Helgaas wrote:
> [+cc Marc]
>=20
> On Tue, Jun 20, 2017 at 08:38:14AM +0800, Ley Foon Tan wrote:
> >=20
> > On Mon, 2017-06-19 at 18:47 -0500, Bjorn Helgaas wrote:
> > >=20
> > > [+cc Thomas, Ley Foon]
> > >=20
> > > On Sat, Jun 17, 2017 at 12:57:38PM -0700, Paul Burton wrote:
> > > >=20
> > > >=20
> > > > The driver expects to use hardware IRQ numbers 1 through 4 for
> > > > INTX
> > > > interrupts, but only creates an IRQ domain of size 4 (ie. IRQ
> > > > numbers 0
> > > > through 3). This results in a warning from irq_domain_associate
> > > > when it
> > > > is called with hwirq=3D4:
> > > >=20
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0WARNING: CPU: 0 PID: 1 at kernel/irq/=
irqdomain.c:365
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0irq_domain_as=
sociate+0x170/0x220
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0error: hwirq 0x4 is too large for dum=
my
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0Modules linked in:
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0CPU: 0 PID: 1 Comm: swapper/0 Tainted=
: G=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0W
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A04.12.0-rc5-00=
126-g19e1b3a10aad-dirty #427
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0Stack : 0000000000000000 000000000000=
0004 0000000000000006
> > > > ffffffff8092c78a
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A00000000000000061 ffffffff8018bf60 0000000000000000
> > > > 0000000000000000
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0ffffffff8088c287 ffffffff80811d18 a8000000ffc60000
> > > > ffffffff80926678
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A00000000000000001 0000000000000000 ffffffff80887880
> > > > ffffffff80960000
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0ffffffff80920000 ffffffff801e6744 ffffffff80887880
> > > > a8000000ffc4f8f8
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0000000000000089c ffffffff8018d260 0000000000010000
> > > > ffffffff80811d18
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A00000000000000000 0000000000000001 0000000000000000
> > > > 0000000000000000
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A00000000000000000 a8000000ffc4f840 0000000000000000
> > > > ffffffff8042cf34
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A00000000000000000 0000000000000000 0000000000000000
> > > > 0000000000040c00
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A00000000000000000 ffffffff8010d1c8 0000000000000000
> > > > ffffffff8042cf34
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0...
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0Call Trace:
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff8010d1c8>] show_stack+0x80/=
0xa0
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff8042cf34>] dump_stack+0xd4/=
0x110
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff8013ea98>] __warn+0xf0/0x10=
8
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff8013eb14>] warn_slowpath_fm=
t+0x3c/0x48
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff80196528>] irq_domain_assoc=
iate+0x170/0x220
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff80196bf0>] irq_create_mappi=
ng+0x88/0x118
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff801976a8>] irq_create_fwspe=
c_mapping+0xb8/0x320
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff80197970>] irq_create_of_ma=
pping+0x60/0x70
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff805d1318>] of_irq_parse_and=
_map_pci+0x20/0x38
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff8049c210>] pci_fixup_irqs+0=
x60/0xe0
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff8049cd64>] xilinx_pcie_prob=
e+0x28c/0x478
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff804e8ca8>] platform_drv_pro=
be+0x50/0xd0
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff804e73a4>] driver_probe_dev=
ice+0x2c4/0x3a0
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff804e7544>] __driver_attach+=
0xc4/0xd0
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff804e5254>] bus_for_each_dev=
+0x64/0xa8
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff804e5e40>] bus_add_driver+0=
x1f0/0x268
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff804e8000>] driver_register+=
0x68/0x118
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff801001a4>] do_one_initcall+=
0x4c/0x178
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff808d3ca8>] kernel_init_free=
able+0x204/0x2b0
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff80730b68>] kernel_init+0x10=
/0xf8
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff80106218>] ret_from_kernel_=
thread+0x14/0x1c
> > > >=20
> > > > This patch avoids that warning by creating the legacy IRQ
> > > > domain
> > > > with
> > > > size 5 rather than 4, allowing it to cover the hwirq=3D4/INTD
> > > > case.
> > > >=20
> > > > 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
> > > >=20
> > > > ---
> > > >=20
> > > > Changes in v5:
> > > > - New patch; replacing "PCI: xilinx: Fix INTX irq dispatch".
> > > >=20
> > > > Changes in v4: None
> > > > Changes in v3: None
> > > > Changes in v2: None
> > > >=20
> > > > =C2=A0drivers/pci/host/pcie-xilinx.c | 2 +-
> > > > =C2=A01 file changed, 1 insertion(+), 1 deletion(-)
> > > >=20
> > > > diff --git a/drivers/pci/host/pcie-xilinx.c
> > > > b/drivers/pci/host/pcie-xilinx.c
> > > > index 2fe2df51f9f8..94c71fb91648 100644
> > > > --- a/drivers/pci/host/pcie-xilinx.c
> > > > +++ b/drivers/pci/host/pcie-xilinx.c
> > > > @@ -524,7 +524,7 @@ static int
> > > > xilinx_pcie_init_irq_domain(struct
> > > > xilinx_pcie_port *port)
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0return -ENODEV;
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0}
> > > >=20
> > > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0port->leg_domain =3D irq_domain_add_=
linear(pcie_intc_node,
> > > > 4,
> > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0port->leg_domain =3D irq_domain_add_=
linear(pcie_intc_node,
> > > > 1 +
> > > > 4,
> > > I don't understand this.=C2=A0=C2=A0Several drivers call
> > > irq_domain_add_linear() with
> > > a size of 4:
> > >=20
> > > =C2=A0 dra7xx_pcie_init_irq_domain
> > > =C2=A0 ks_dw_pcie_host_init
> > > =C2=A0 advk_pcie_init_irq_domain
> > > =C2=A0 faraday_pci_setup_cascaded_irq
> > > =C2=A0 rockchip_pcie_init_irq_domain
> > > =C2=A0 nwl_pcie_init_irq_domain
> > >=20
> > > Only one other in drivers/pci uses a size of 5:
> > >=20
> > > =C2=A0 altera_pcie_init_irq_domain
> > >=20
> > > Why can't we use a size of 4 for all of them?=C2=A0=C2=A0We only have=
 INTA-
> > > INTD.=C2=A0=C2=A0Are
> > > altera and xilinx missing something to apply an offset from the
> > > 0-3
> > > space
> > > to the 1-4 space?
> > We have the same discussion before in 2016:=C2=A0https://lkml.org/lkml/=
2
> > 016/
> > 8/30/198
> Thanks for digging that out.=C2=A0=C2=A0I knew we'd discussed this before=
, but
> I
> couldn't find it in the archives.=C2=A0=C2=A0I don't think anybody was re=
ally
> satisfied with the outcome, but we accepted it to make forward
> progress.
>=20
> >=20
> > This is because legacy interrupt is start with index 1 instead of
> > 0.
> I'm not buying this.=C2=A0=C2=A0Your argument was that "the hwirq for leg=
acy
> interrupts will start at 0x1 to 0x4 (INTA to INTD) and these values
> are as per PCIe specification for legacy interrupts.=C2=A0=C2=A0So these =
cannot
> be numbered from 0."
>=20
> But all the other drivers I mentioned get along with the 0-3 range
> somehow.=C2=A0=C2=A0If there's something different about altera and xilin=
x that
> means they can't use the same solution the others do, I'd like to
> know
> what it is.
I'm not sure those drivers with index 0-3 range tested with 4 legacy
interrupts or not. It will not has error until someone requesting 4
legacy interrupts. We see this error when we enabling multi-function
endpoint (4 functions). I believe this is not altera or xilinx
specific.


Regards
Ley Foon

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 1/4] PCI: xilinx: Create legacy IRQ domain with size 5
@ 2017-06-20  1:55           ` Ley Foon Tan
  0 siblings, 0 replies; 27+ messages in thread
From: Ley Foon Tan @ 2017-06-20  1:55 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Paul Burton, linux-pci, Bharat Kumar Gogada, Ravikiran Gummaluri,
	Bjorn Helgaas, Michal Simek, linux-mips, Thomas Gleixner,
	Ley Foon Tan, Marc Zyngier

On Mon, 2017-06-19 at 20:49 -0500, Bjorn Helgaas wrote:
> [+cc Marc]
> 
> On Tue, Jun 20, 2017 at 08:38:14AM +0800, Ley Foon Tan wrote:
> > 
> > On Mon, 2017-06-19 at 18:47 -0500, Bjorn Helgaas wrote:
> > > 
> > > [+cc Thomas, Ley Foon]
> > > 
> > > On Sat, Jun 17, 2017 at 12:57:38PM -0700, Paul Burton wrote:
> > > > 
> > > > 
> > > > The driver expects to use hardware IRQ numbers 1 through 4 for
> > > > INTX
> > > > interrupts, but only creates an IRQ domain of size 4 (ie. IRQ
> > > > numbers 0
> > > > through 3). This results in a warning from irq_domain_associate
> > > > when it
> > > > is called with hwirq=4:
> > > > 
> > > >      WARNING: CPU: 0 PID: 1 at kernel/irq/irqdomain.c:365
> > > >          irq_domain_associate+0x170/0x220
> > > >      error: hwirq 0x4 is too large for dummy
> > > >      Modules linked in:
> > > >      CPU: 0 PID: 1 Comm: swapper/0 Tainted: G        W
> > > >          4.12.0-rc5-00126-g19e1b3a10aad-dirty #427
> > > >      Stack : 0000000000000000 0000000000000004 0000000000000006
> > > > ffffffff8092c78a
> > > >              0000000000000061 ffffffff8018bf60 0000000000000000
> > > > 0000000000000000
> > > >              ffffffff8088c287 ffffffff80811d18 a8000000ffc60000
> > > > ffffffff80926678
> > > >              0000000000000001 0000000000000000 ffffffff80887880
> > > > ffffffff80960000
> > > >              ffffffff80920000 ffffffff801e6744 ffffffff80887880
> > > > a8000000ffc4f8f8
> > > >              000000000000089c ffffffff8018d260 0000000000010000
> > > > ffffffff80811d18
> > > >              0000000000000000 0000000000000001 0000000000000000
> > > > 0000000000000000
> > > >              0000000000000000 a8000000ffc4f840 0000000000000000
> > > > ffffffff8042cf34
> > > >              0000000000000000 0000000000000000 0000000000000000
> > > > 0000000000040c00
> > > >              0000000000000000 ffffffff8010d1c8 0000000000000000
> > > > ffffffff8042cf34
> > > >              ...
> > > >      Call Trace:
> > > >      [<ffffffff8010d1c8>] show_stack+0x80/0xa0
> > > >      [<ffffffff8042cf34>] dump_stack+0xd4/0x110
> > > >      [<ffffffff8013ea98>] __warn+0xf0/0x108
> > > >      [<ffffffff8013eb14>] warn_slowpath_fmt+0x3c/0x48
> > > >      [<ffffffff80196528>] irq_domain_associate+0x170/0x220
> > > >      [<ffffffff80196bf0>] irq_create_mapping+0x88/0x118
> > > >      [<ffffffff801976a8>] irq_create_fwspec_mapping+0xb8/0x320
> > > >      [<ffffffff80197970>] irq_create_of_mapping+0x60/0x70
> > > >      [<ffffffff805d1318>] of_irq_parse_and_map_pci+0x20/0x38
> > > >      [<ffffffff8049c210>] pci_fixup_irqs+0x60/0xe0
> > > >      [<ffffffff8049cd64>] xilinx_pcie_probe+0x28c/0x478
> > > >      [<ffffffff804e8ca8>] platform_drv_probe+0x50/0xd0
> > > >      [<ffffffff804e73a4>] driver_probe_device+0x2c4/0x3a0
> > > >      [<ffffffff804e7544>] __driver_attach+0xc4/0xd0
> > > >      [<ffffffff804e5254>] bus_for_each_dev+0x64/0xa8
> > > >      [<ffffffff804e5e40>] bus_add_driver+0x1f0/0x268
> > > >      [<ffffffff804e8000>] driver_register+0x68/0x118
> > > >      [<ffffffff801001a4>] do_one_initcall+0x4c/0x178
> > > >      [<ffffffff808d3ca8>] kernel_init_freeable+0x204/0x2b0
> > > >      [<ffffffff80730b68>] kernel_init+0x10/0xf8
> > > >      [<ffffffff80106218>] ret_from_kernel_thread+0x14/0x1c
> > > > 
> > > > This patch avoids that warning by creating the legacy IRQ
> > > > domain
> > > > with
> > > > size 5 rather than 4, allowing it to cover the hwirq=4/INTD
> > > > case.
> > > > 
> > > > 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 v5:
> > > > - New patch; replacing "PCI: xilinx: Fix INTX irq dispatch".
> > > > 
> > > > Changes in v4: None
> > > > Changes in v3: None
> > > > Changes in v2: None
> > > > 
> > > >  drivers/pci/host/pcie-xilinx.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/pci/host/pcie-xilinx.c
> > > > b/drivers/pci/host/pcie-xilinx.c
> > > > index 2fe2df51f9f8..94c71fb91648 100644
> > > > --- a/drivers/pci/host/pcie-xilinx.c
> > > > +++ b/drivers/pci/host/pcie-xilinx.c
> > > > @@ -524,7 +524,7 @@ static int
> > > > xilinx_pcie_init_irq_domain(struct
> > > > xilinx_pcie_port *port)
> > > >               return -ENODEV;
> > > >       }
> > > > 
> > > > -     port->leg_domain = irq_domain_add_linear(pcie_intc_node,
> > > > 4,
> > > > +     port->leg_domain = irq_domain_add_linear(pcie_intc_node,
> > > > 1 +
> > > > 4,
> > > I don't understand this.  Several drivers call
> > > irq_domain_add_linear() with
> > > a size of 4:
> > > 
> > >   dra7xx_pcie_init_irq_domain
> > >   ks_dw_pcie_host_init
> > >   advk_pcie_init_irq_domain
> > >   faraday_pci_setup_cascaded_irq
> > >   rockchip_pcie_init_irq_domain
> > >   nwl_pcie_init_irq_domain
> > > 
> > > Only one other in drivers/pci uses a size of 5:
> > > 
> > >   altera_pcie_init_irq_domain
> > > 
> > > Why can't we use a size of 4 for all of them?  We only have INTA-
> > > INTD.  Are
> > > altera and xilinx missing something to apply an offset from the
> > > 0-3
> > > space
> > > to the 1-4 space?
> > We have the same discussion before in 2016: https://lkml.org/lkml/2
> > 016/
> > 8/30/198
> Thanks for digging that out.  I knew we'd discussed this before, but
> I
> couldn't find it in the archives.  I don't think anybody was really
> satisfied with the outcome, but we accepted it to make forward
> progress.
> 
> > 
> > This is because legacy interrupt is start with index 1 instead of
> > 0.
> I'm not buying this.  Your argument was that "the hwirq for legacy
> interrupts will start at 0x1 to 0x4 (INTA to INTD) and these values
> are as per PCIe specification for legacy interrupts.  So these cannot
> be numbered from 0."
> 
> But all the other drivers I mentioned get along with the 0-3 range
> somehow.  If there's something different about altera and xilinx that
> means they can't use the same solution the others do, I'd like to
> know
> what it is.
I'm not sure those drivers with index 0-3 range tested with 4 legacy
interrupts or not. It will not has error until someone requesting 4
legacy interrupts. We see this error when we enabling multi-function
endpoint (4 functions). I believe this is not altera or xilinx
specific.


Regards
Ley Foon

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 1/4] PCI: xilinx: Create legacy IRQ domain with size 5
  2017-06-20  1:55           ` Ley Foon Tan
@ 2017-06-20  2:02             ` Ley Foon Tan
  -1 siblings, 0 replies; 27+ messages in thread
From: Ley Foon Tan @ 2017-06-20  2:02 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Paul Burton, linux-pci, Bharat Kumar Gogada, Ravikiran Gummaluri,
	Bjorn Helgaas, Michal Simek, linux-mips, Thomas Gleixner,
	Ley Foon Tan, Marc Zyngier

On Tue, 2017-06-20 at 09:55 +0800, Ley Foon Tan wrote:
> On Mon, 2017-06-19 at 20:49 -0500, Bjorn Helgaas wrote:
> >=20
> > [+cc Marc]
> >=20
> > On Tue, Jun 20, 2017 at 08:38:14AM +0800, Ley Foon Tan wrote:
> > >=20
> > >=20
> > > On Mon, 2017-06-19 at 18:47 -0500, Bjorn Helgaas wrote:
> > > >=20
> > > >=20
> > > > [+cc Thomas, Ley Foon]
> > > >=20
> > > > On Sat, Jun 17, 2017 at 12:57:38PM -0700, Paul Burton wrote:
> > > > >=20
> > > > >=20
> > > > >=20
> > > > > The driver expects to use hardware IRQ numbers 1 through 4
> > > > > for
> > > > > INTX
> > > > > interrupts, but only creates an IRQ domain of size 4 (ie. IRQ
> > > > > numbers 0
> > > > > through 3). This results in a warning from
> > > > > irq_domain_associate
> > > > > when it
> > > > > is called with hwirq=3D4:
> > > > >=20
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0WARNING: CPU: 0 PID: 1 at kernel/ir=
q/irqdomain.c:365
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0irq_domain_=
associate+0x170/0x220
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0error: hwirq 0x4 is too large for d=
ummy
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0Modules linked in:
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0CPU: 0 PID: 1 Comm: swapper/0 Taint=
ed: G=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0W
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A04.12.0-rc5-=
00126-g19e1b3a10aad-dirty #427
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0Stack : 0000000000000000 0000000000=
000004
> > > > > 0000000000000006
> > > > > ffffffff8092c78a
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A00000000000000061 ffffffff8018bf60
> > > > > 0000000000000000
> > > > > 0000000000000000
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0ffffffff8088c287 ffffffff80811d18
> > > > > a8000000ffc60000
> > > > > ffffffff80926678
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A00000000000000001 0000000000000000
> > > > > ffffffff80887880
> > > > > ffffffff80960000
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0ffffffff80920000 ffffffff801e6744
> > > > > ffffffff80887880
> > > > > a8000000ffc4f8f8
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0000000000000089c ffffffff8018d260
> > > > > 0000000000010000
> > > > > ffffffff80811d18
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A00000000000000000 0000000000000001
> > > > > 0000000000000000
> > > > > 0000000000000000
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A00000000000000000 a8000000ffc4f840
> > > > > 0000000000000000
> > > > > ffffffff8042cf34
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A00000000000000000 0000000000000000
> > > > > 0000000000000000
> > > > > 0000000000040c00
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A00000000000000000 ffffffff8010d1c8
> > > > > 0000000000000000
> > > > > ffffffff8042cf34
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0...
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0Call Trace:
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff8010d1c8>] show_stack+0x8=
0/0xa0
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff8042cf34>] dump_stack+0xd=
4/0x110
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff8013ea98>] __warn+0xf0/0x=
108
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff8013eb14>] warn_slowpath_=
fmt+0x3c/0x48
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff80196528>] irq_domain_ass=
ociate+0x170/0x220
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff80196bf0>] irq_create_map=
ping+0x88/0x118
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff801976a8>]
> > > > > irq_create_fwspec_mapping+0xb8/0x320
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff80197970>] irq_create_of_=
mapping+0x60/0x70
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff805d1318>] of_irq_parse_a=
nd_map_pci+0x20/0x38
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff8049c210>] pci_fixup_irqs=
+0x60/0xe0
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff8049cd64>] xilinx_pcie_pr=
obe+0x28c/0x478
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff804e8ca8>] platform_drv_p=
robe+0x50/0xd0
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff804e73a4>] driver_probe_d=
evice+0x2c4/0x3a0
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff804e7544>] __driver_attac=
h+0xc4/0xd0
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff804e5254>] bus_for_each_d=
ev+0x64/0xa8
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff804e5e40>] bus_add_driver=
+0x1f0/0x268
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff804e8000>] driver_registe=
r+0x68/0x118
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff801001a4>] do_one_initcal=
l+0x4c/0x178
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff808d3ca8>] kernel_init_fr=
eeable+0x204/0x2b0
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff80730b68>] kernel_init+0x=
10/0xf8
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0[<ffffffff80106218>] ret_from_kerne=
l_thread+0x14/0x1c
> > > > >=20
> > > > > This patch avoids that warning by creating the legacy IRQ
> > > > > domain
> > > > > with
> > > > > size 5 rather than 4, allowing it to cover the hwirq=3D4/INTD
> > > > > case.
> > > > >=20
> > > > > 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
> > > > >=20
> > > > > ---
> > > > >=20
> > > > > Changes in v5:
> > > > > - New patch; replacing "PCI: xilinx: Fix INTX irq dispatch".
> > > > >=20
> > > > > Changes in v4: None
> > > > > Changes in v3: None
> > > > > Changes in v2: None
> > > > >=20
> > > > > =C2=A0drivers/pci/host/pcie-xilinx.c | 2 +-
> > > > > =C2=A01 file changed, 1 insertion(+), 1 deletion(-)
> > > > >=20
> > > > > diff --git a/drivers/pci/host/pcie-xilinx.c
> > > > > b/drivers/pci/host/pcie-xilinx.c
> > > > > index 2fe2df51f9f8..94c71fb91648 100644
> > > > > --- a/drivers/pci/host/pcie-xilinx.c
> > > > > +++ b/drivers/pci/host/pcie-xilinx.c
> > > > > @@ -524,7 +524,7 @@ static int
> > > > > xilinx_pcie_init_irq_domain(struct
> > > > > xilinx_pcie_port *port)
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0return -ENODEV;
> > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0}
> > > > >=20
> > > > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0port->leg_domain =3D
> > > > > irq_domain_add_linear(pcie_intc_node,
> > > > > 4,
> > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0port->leg_domain =3D
> > > > > irq_domain_add_linear(pcie_intc_node,
> > > > > 1 +
> > > > > 4,
> > > > I don't understand this.=C2=A0=C2=A0Several drivers call
> > > > irq_domain_add_linear() with
> > > > a size of 4:
> > > >=20
> > > > =C2=A0 dra7xx_pcie_init_irq_domain
> > > > =C2=A0 ks_dw_pcie_host_init
> > > > =C2=A0 advk_pcie_init_irq_domain
> > > > =C2=A0 faraday_pci_setup_cascaded_irq
> > > > =C2=A0 rockchip_pcie_init_irq_domain
> > > > =C2=A0 nwl_pcie_init_irq_domain
> > > >=20
> > > > Only one other in drivers/pci uses a size of 5:
> > > >=20
> > > > =C2=A0 altera_pcie_init_irq_domain
> > > >=20
> > > > Why can't we use a size of 4 for all of them?=C2=A0=C2=A0We only ha=
ve
> > > > INTA-
> > > > INTD.=C2=A0=C2=A0Are
> > > > altera and xilinx missing something to apply an offset from the
> > > > 0-3
> > > > space
> > > > to the 1-4 space?
> > > We have the same discussion before in 2016: https://lkml.org/lkml
> > > /2
> > > 016/
> > > 8/30/198
> > Thanks for digging that out.=C2=A0=C2=A0I knew we'd discussed this befo=
re,
> > but
> > I
> > couldn't find it in the archives.=C2=A0=C2=A0I don't think anybody was =
really
> > satisfied with the outcome, but we accepted it to make forward
> > progress.
> >=20
> > >=20
> > >=20
> > > This is because legacy interrupt is start with index 1 instead of
> > > 0.
> > I'm not buying this.=C2=A0=C2=A0Your argument was that "the hwirq for l=
egacy
> > interrupts will start at 0x1 to 0x4 (INTA to INTD) and these values
> > are as per PCIe specification for legacy interrupts.=C2=A0=C2=A0So thes=
e
> > cannot
> > be numbered from 0."
> >=20
> > But all the other drivers I mentioned get along with the 0-3 range
> > somehow.=C2=A0=C2=A0If there's something different about altera and xil=
inx
> > that
> > means they can't use the same solution the others do, I'd like to
> > know
> > what it is.
> I'm not sure those drivers with index 0-3 range tested with 4 legacy
> interrupts or not. It will not has error until someone requesting 4
> legacy interrupts. We see this error when we enabling multi-function
> endpoint (4 functions). I believe this is not altera or xilinx
> specific.
>=20

It is broken in=C2=A0dra7xx too.
https://lkml.org/lkml/2016/9/14/241

Regards
Ley Foon

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 1/4] PCI: xilinx: Create legacy IRQ domain with size 5
@ 2017-06-20  2:02             ` Ley Foon Tan
  0 siblings, 0 replies; 27+ messages in thread
From: Ley Foon Tan @ 2017-06-20  2:02 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Paul Burton, linux-pci, Bharat Kumar Gogada, Ravikiran Gummaluri,
	Bjorn Helgaas, Michal Simek, linux-mips, Thomas Gleixner,
	Ley Foon Tan, Marc Zyngier

On Tue, 2017-06-20 at 09:55 +0800, Ley Foon Tan wrote:
> On Mon, 2017-06-19 at 20:49 -0500, Bjorn Helgaas wrote:
> > 
> > [+cc Marc]
> > 
> > On Tue, Jun 20, 2017 at 08:38:14AM +0800, Ley Foon Tan wrote:
> > > 
> > > 
> > > On Mon, 2017-06-19 at 18:47 -0500, Bjorn Helgaas wrote:
> > > > 
> > > > 
> > > > [+cc Thomas, Ley Foon]
> > > > 
> > > > On Sat, Jun 17, 2017 at 12:57:38PM -0700, Paul Burton wrote:
> > > > > 
> > > > > 
> > > > > 
> > > > > The driver expects to use hardware IRQ numbers 1 through 4
> > > > > for
> > > > > INTX
> > > > > interrupts, but only creates an IRQ domain of size 4 (ie. IRQ
> > > > > numbers 0
> > > > > through 3). This results in a warning from
> > > > > irq_domain_associate
> > > > > when it
> > > > > is called with hwirq=4:
> > > > > 
> > > > >      WARNING: CPU: 0 PID: 1 at kernel/irq/irqdomain.c:365
> > > > >          irq_domain_associate+0x170/0x220
> > > > >      error: hwirq 0x4 is too large for dummy
> > > > >      Modules linked in:
> > > > >      CPU: 0 PID: 1 Comm: swapper/0 Tainted: G        W
> > > > >          4.12.0-rc5-00126-g19e1b3a10aad-dirty #427
> > > > >      Stack : 0000000000000000 0000000000000004
> > > > > 0000000000000006
> > > > > ffffffff8092c78a
> > > > >              0000000000000061 ffffffff8018bf60
> > > > > 0000000000000000
> > > > > 0000000000000000
> > > > >              ffffffff8088c287 ffffffff80811d18
> > > > > a8000000ffc60000
> > > > > ffffffff80926678
> > > > >              0000000000000001 0000000000000000
> > > > > ffffffff80887880
> > > > > ffffffff80960000
> > > > >              ffffffff80920000 ffffffff801e6744
> > > > > ffffffff80887880
> > > > > a8000000ffc4f8f8
> > > > >              000000000000089c ffffffff8018d260
> > > > > 0000000000010000
> > > > > ffffffff80811d18
> > > > >              0000000000000000 0000000000000001
> > > > > 0000000000000000
> > > > > 0000000000000000
> > > > >              0000000000000000 a8000000ffc4f840
> > > > > 0000000000000000
> > > > > ffffffff8042cf34
> > > > >              0000000000000000 0000000000000000
> > > > > 0000000000000000
> > > > > 0000000000040c00
> > > > >              0000000000000000 ffffffff8010d1c8
> > > > > 0000000000000000
> > > > > ffffffff8042cf34
> > > > >              ...
> > > > >      Call Trace:
> > > > >      [<ffffffff8010d1c8>] show_stack+0x80/0xa0
> > > > >      [<ffffffff8042cf34>] dump_stack+0xd4/0x110
> > > > >      [<ffffffff8013ea98>] __warn+0xf0/0x108
> > > > >      [<ffffffff8013eb14>] warn_slowpath_fmt+0x3c/0x48
> > > > >      [<ffffffff80196528>] irq_domain_associate+0x170/0x220
> > > > >      [<ffffffff80196bf0>] irq_create_mapping+0x88/0x118
> > > > >      [<ffffffff801976a8>]
> > > > > irq_create_fwspec_mapping+0xb8/0x320
> > > > >      [<ffffffff80197970>] irq_create_of_mapping+0x60/0x70
> > > > >      [<ffffffff805d1318>] of_irq_parse_and_map_pci+0x20/0x38
> > > > >      [<ffffffff8049c210>] pci_fixup_irqs+0x60/0xe0
> > > > >      [<ffffffff8049cd64>] xilinx_pcie_probe+0x28c/0x478
> > > > >      [<ffffffff804e8ca8>] platform_drv_probe+0x50/0xd0
> > > > >      [<ffffffff804e73a4>] driver_probe_device+0x2c4/0x3a0
> > > > >      [<ffffffff804e7544>] __driver_attach+0xc4/0xd0
> > > > >      [<ffffffff804e5254>] bus_for_each_dev+0x64/0xa8
> > > > >      [<ffffffff804e5e40>] bus_add_driver+0x1f0/0x268
> > > > >      [<ffffffff804e8000>] driver_register+0x68/0x118
> > > > >      [<ffffffff801001a4>] do_one_initcall+0x4c/0x178
> > > > >      [<ffffffff808d3ca8>] kernel_init_freeable+0x204/0x2b0
> > > > >      [<ffffffff80730b68>] kernel_init+0x10/0xf8
> > > > >      [<ffffffff80106218>] ret_from_kernel_thread+0x14/0x1c
> > > > > 
> > > > > This patch avoids that warning by creating the legacy IRQ
> > > > > domain
> > > > > with
> > > > > size 5 rather than 4, allowing it to cover the hwirq=4/INTD
> > > > > case.
> > > > > 
> > > > > 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 v5:
> > > > > - New patch; replacing "PCI: xilinx: Fix INTX irq dispatch".
> > > > > 
> > > > > Changes in v4: None
> > > > > Changes in v3: None
> > > > > Changes in v2: None
> > > > > 
> > > > >  drivers/pci/host/pcie-xilinx.c | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/pci/host/pcie-xilinx.c
> > > > > b/drivers/pci/host/pcie-xilinx.c
> > > > > index 2fe2df51f9f8..94c71fb91648 100644
> > > > > --- a/drivers/pci/host/pcie-xilinx.c
> > > > > +++ b/drivers/pci/host/pcie-xilinx.c
> > > > > @@ -524,7 +524,7 @@ static int
> > > > > xilinx_pcie_init_irq_domain(struct
> > > > > xilinx_pcie_port *port)
> > > > >               return -ENODEV;
> > > > >       }
> > > > > 
> > > > > -     port->leg_domain =
> > > > > irq_domain_add_linear(pcie_intc_node,
> > > > > 4,
> > > > > +     port->leg_domain =
> > > > > irq_domain_add_linear(pcie_intc_node,
> > > > > 1 +
> > > > > 4,
> > > > I don't understand this.  Several drivers call
> > > > irq_domain_add_linear() with
> > > > a size of 4:
> > > > 
> > > >   dra7xx_pcie_init_irq_domain
> > > >   ks_dw_pcie_host_init
> > > >   advk_pcie_init_irq_domain
> > > >   faraday_pci_setup_cascaded_irq
> > > >   rockchip_pcie_init_irq_domain
> > > >   nwl_pcie_init_irq_domain
> > > > 
> > > > Only one other in drivers/pci uses a size of 5:
> > > > 
> > > >   altera_pcie_init_irq_domain
> > > > 
> > > > Why can't we use a size of 4 for all of them?  We only have
> > > > INTA-
> > > > INTD.  Are
> > > > altera and xilinx missing something to apply an offset from the
> > > > 0-3
> > > > space
> > > > to the 1-4 space?
> > > We have the same discussion before in 2016: https://lkml.org/lkml
> > > /2
> > > 016/
> > > 8/30/198
> > Thanks for digging that out.  I knew we'd discussed this before,
> > but
> > I
> > couldn't find it in the archives.  I don't think anybody was really
> > satisfied with the outcome, but we accepted it to make forward
> > progress.
> > 
> > > 
> > > 
> > > This is because legacy interrupt is start with index 1 instead of
> > > 0.
> > I'm not buying this.  Your argument was that "the hwirq for legacy
> > interrupts will start at 0x1 to 0x4 (INTA to INTD) and these values
> > are as per PCIe specification for legacy interrupts.  So these
> > cannot
> > be numbered from 0."
> > 
> > But all the other drivers I mentioned get along with the 0-3 range
> > somehow.  If there's something different about altera and xilinx
> > that
> > means they can't use the same solution the others do, I'd like to
> > know
> > what it is.
> I'm not sure those drivers with index 0-3 range tested with 4 legacy
> interrupts or not. It will not has error until someone requesting 4
> legacy interrupts. We see this error when we enabling multi-function
> endpoint (4 functions). I believe this is not altera or xilinx
> specific.
> 

It is broken in dra7xx too.
https://lkml.org/lkml/2016/9/14/241

Regards
Ley Foon

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 1/4] PCI: xilinx: Create legacy IRQ domain with size 5
@ 2017-06-20  2:07           ` Paul Burton
  0 siblings, 0 replies; 27+ messages in thread
From: Paul Burton @ 2017-06-20  2:07 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Ley Foon Tan, linux-pci, Bharat Kumar Gogada,
	Ravikiran Gummaluri, Bjorn Helgaas, Michal Simek, linux-mips,
	Thomas Gleixner, Ley Foon Tan, Marc Zyngier

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

Hi Bjorn,

On Monday, 19 June 2017 18:49:03 PDT Bjorn Helgaas wrote:
> [+cc Marc]
> 
> On Tue, Jun 20, 2017 at 08:38:14AM +0800, Ley Foon Tan wrote:
> > On Mon, 2017-06-19 at 18:47 -0500, Bjorn Helgaas wrote:
> > > [+cc Thomas, Ley Foon]
> > > 
> > > On Sat, Jun 17, 2017 at 12:57:38PM -0700, Paul Burton wrote:
> > > > The driver expects to use hardware IRQ numbers 1 through 4 for INTX
> > > > interrupts, but only creates an IRQ domain of size 4 (ie. IRQ
> > > > numbers 0
> > > > through 3). This results in a warning from irq_domain_associate
> > > > when it
> > > > is called with hwirq=4:
> > > > 
> > > >      WARNING: CPU: 0 PID: 1 at kernel/irq/irqdomain.c:365
> > > >          irq_domain_associate+0x170/0x220
> > > >      error: hwirq 0x4 is too large for dummy
> > > >      Modules linked in:
> > > >      CPU: 0 PID: 1 Comm: swapper/0 Tainted: G        W
> > > >          4.12.0-rc5-00126-g19e1b3a10aad-dirty #427
> > > >      Stack : 0000000000000000 0000000000000004 0000000000000006
> > > > ffffffff8092c78a
> > > >              0000000000000061 ffffffff8018bf60 0000000000000000
> > > > 0000000000000000
> > > >              ffffffff8088c287 ffffffff80811d18 a8000000ffc60000
> > > > ffffffff80926678
> > > >              0000000000000001 0000000000000000 ffffffff80887880
> > > > ffffffff80960000
> > > >              ffffffff80920000 ffffffff801e6744 ffffffff80887880
> > > > a8000000ffc4f8f8
> > > >              000000000000089c ffffffff8018d260 0000000000010000
> > > > ffffffff80811d18
> > > >              0000000000000000 0000000000000001 0000000000000000
> > > > 0000000000000000
> > > >              0000000000000000 a8000000ffc4f840 0000000000000000
> > > > ffffffff8042cf34
> > > >              0000000000000000 0000000000000000 0000000000000000
> > > > 0000000000040c00
> > > >              0000000000000000 ffffffff8010d1c8 0000000000000000
> > > > ffffffff8042cf34
> > > >              ...
> > > >      Call Trace:
> > > >      [<ffffffff8010d1c8>] show_stack+0x80/0xa0
> > > >      [<ffffffff8042cf34>] dump_stack+0xd4/0x110
> > > >      [<ffffffff8013ea98>] __warn+0xf0/0x108
> > > >      [<ffffffff8013eb14>] warn_slowpath_fmt+0x3c/0x48
> > > >      [<ffffffff80196528>] irq_domain_associate+0x170/0x220
> > > >      [<ffffffff80196bf0>] irq_create_mapping+0x88/0x118
> > > >      [<ffffffff801976a8>] irq_create_fwspec_mapping+0xb8/0x320
> > > >      [<ffffffff80197970>] irq_create_of_mapping+0x60/0x70
> > > >      [<ffffffff805d1318>] of_irq_parse_and_map_pci+0x20/0x38
> > > >      [<ffffffff8049c210>] pci_fixup_irqs+0x60/0xe0
> > > >      [<ffffffff8049cd64>] xilinx_pcie_probe+0x28c/0x478
> > > >      [<ffffffff804e8ca8>] platform_drv_probe+0x50/0xd0
> > > >      [<ffffffff804e73a4>] driver_probe_device+0x2c4/0x3a0
> > > >      [<ffffffff804e7544>] __driver_attach+0xc4/0xd0
> > > >      [<ffffffff804e5254>] bus_for_each_dev+0x64/0xa8
> > > >      [<ffffffff804e5e40>] bus_add_driver+0x1f0/0x268
> > > >      [<ffffffff804e8000>] driver_register+0x68/0x118
> > > >      [<ffffffff801001a4>] do_one_initcall+0x4c/0x178
> > > >      [<ffffffff808d3ca8>] kernel_init_freeable+0x204/0x2b0
> > > >      [<ffffffff80730b68>] kernel_init+0x10/0xf8
> > > >      [<ffffffff80106218>] ret_from_kernel_thread+0x14/0x1c
> > > > 
> > > > This patch avoids that warning by creating the legacy IRQ domain
> > > > with
> > > > size 5 rather than 4, allowing it to cover the hwirq=4/INTD case.
> > > > 
> > > > 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 v5:
> > > > - New patch; replacing "PCI: xilinx: Fix INTX irq dispatch".
> > > > 
> > > > Changes in v4: None
> > > > Changes in v3: None
> > > > Changes in v2: None
> > > > 
> > > >  drivers/pci/host/pcie-xilinx.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/pci/host/pcie-xilinx.c
> > > > b/drivers/pci/host/pcie-xilinx.c
> > > > index 2fe2df51f9f8..94c71fb91648 100644
> > > > --- a/drivers/pci/host/pcie-xilinx.c
> > > > +++ b/drivers/pci/host/pcie-xilinx.c
> > > > @@ -524,7 +524,7 @@ static int xilinx_pcie_init_irq_domain(struct
> > > > xilinx_pcie_port *port)
> > > >               return -ENODEV;
> > > >       }
> > > > 
> > > > -     port->leg_domain = irq_domain_add_linear(pcie_intc_node, 4,
> > > > +     port->leg_domain = irq_domain_add_linear(pcie_intc_node, 1 +
> > > > 4,
> > > 
> > > I don't understand this.  Several drivers call
> > > irq_domain_add_linear() with
> > > a size of 4:
> > > 
> > >   dra7xx_pcie_init_irq_domain
> > >   ks_dw_pcie_host_init
> > >   advk_pcie_init_irq_domain
> > >   faraday_pci_setup_cascaded_irq
> > >   rockchip_pcie_init_irq_domain
> > >   nwl_pcie_init_irq_domain
> > > 
> > > Only one other in drivers/pci uses a size of 5:
> > > 
> > >   altera_pcie_init_irq_domain
> > > 
> > > Why can't we use a size of 4 for all of them?  We only have INTA-
> > > INTD.  Are
> > > altera and xilinx missing something to apply an offset from the 0-3
> > > space
> > > to the 1-4 space?
> > 
> > We have the same discussion before in 2016: https://lkml.org/lkml/2016/
> > 8/30/198
> 
> Thanks for digging that out.  I knew we'd discussed this before, but I
> couldn't find it in the archives.  I don't think anybody was really
> satisfied with the outcome, but we accepted it to make forward
> progress.
> 
> > This is because legacy interrupt is start with index 1 instead of 0.
> 
> I'm not buying this.  Your argument was that "the hwirq for legacy
> interrupts will start at 0x1 to 0x4 (INTA to INTD) and these values
> are as per PCIe specification for legacy interrupts.  So these cannot
> be numbered from 0."
> 
> But all the other drivers I mentioned get along with the 0-3 range
> somehow.  If there's something different about altera and xilinx that
> means they can't use the same solution the others do, I'd like to know
> what it is.

Note that with v4 of this patchset[1] I was using hwirq numbers 0-3 with pcie-
xilinx just fine, however:

1) Bharat complained.

2) It does require that the DT interrupt-map property be set accordingly, 
which I guess may mean we're stuck with hwirq 1-4 for drivers that already use 
them.

Thanks,
    Paul

[1] https://patchwork.kernel.org/patch/9763191/

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 1/4] PCI: xilinx: Create legacy IRQ domain with size 5
@ 2017-06-20  2:07           ` Paul Burton
  0 siblings, 0 replies; 27+ messages in thread
From: Paul Burton @ 2017-06-20  2:07 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Ley Foon Tan, linux-pci, Bharat Kumar Gogada,
	Ravikiran Gummaluri, Bjorn Helgaas, Michal Simek, linux-mips,
	Thomas Gleixner, Ley Foon Tan, Marc Zyngier

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

Hi Bjorn,

On Monday, 19 June 2017 18:49:03 PDT Bjorn Helgaas wrote:
> [+cc Marc]
> 
> On Tue, Jun 20, 2017 at 08:38:14AM +0800, Ley Foon Tan wrote:
> > On Mon, 2017-06-19 at 18:47 -0500, Bjorn Helgaas wrote:
> > > [+cc Thomas, Ley Foon]
> > > 
> > > On Sat, Jun 17, 2017 at 12:57:38PM -0700, Paul Burton wrote:
> > > > The driver expects to use hardware IRQ numbers 1 through 4 for INTX
> > > > interrupts, but only creates an IRQ domain of size 4 (ie. IRQ
> > > > numbers 0
> > > > through 3). This results in a warning from irq_domain_associate
> > > > when it
> > > > is called with hwirq=4:
> > > > 
> > > >      WARNING: CPU: 0 PID: 1 at kernel/irq/irqdomain.c:365
> > > >          irq_domain_associate+0x170/0x220
> > > >      error: hwirq 0x4 is too large for dummy
> > > >      Modules linked in:
> > > >      CPU: 0 PID: 1 Comm: swapper/0 Tainted: G        W
> > > >          4.12.0-rc5-00126-g19e1b3a10aad-dirty #427
> > > >      Stack : 0000000000000000 0000000000000004 0000000000000006
> > > > ffffffff8092c78a
> > > >              0000000000000061 ffffffff8018bf60 0000000000000000
> > > > 0000000000000000
> > > >              ffffffff8088c287 ffffffff80811d18 a8000000ffc60000
> > > > ffffffff80926678
> > > >              0000000000000001 0000000000000000 ffffffff80887880
> > > > ffffffff80960000
> > > >              ffffffff80920000 ffffffff801e6744 ffffffff80887880
> > > > a8000000ffc4f8f8
> > > >              000000000000089c ffffffff8018d260 0000000000010000
> > > > ffffffff80811d18
> > > >              0000000000000000 0000000000000001 0000000000000000
> > > > 0000000000000000
> > > >              0000000000000000 a8000000ffc4f840 0000000000000000
> > > > ffffffff8042cf34
> > > >              0000000000000000 0000000000000000 0000000000000000
> > > > 0000000000040c00
> > > >              0000000000000000 ffffffff8010d1c8 0000000000000000
> > > > ffffffff8042cf34
> > > >              ...
> > > >      Call Trace:
> > > >      [<ffffffff8010d1c8>] show_stack+0x80/0xa0
> > > >      [<ffffffff8042cf34>] dump_stack+0xd4/0x110
> > > >      [<ffffffff8013ea98>] __warn+0xf0/0x108
> > > >      [<ffffffff8013eb14>] warn_slowpath_fmt+0x3c/0x48
> > > >      [<ffffffff80196528>] irq_domain_associate+0x170/0x220
> > > >      [<ffffffff80196bf0>] irq_create_mapping+0x88/0x118
> > > >      [<ffffffff801976a8>] irq_create_fwspec_mapping+0xb8/0x320
> > > >      [<ffffffff80197970>] irq_create_of_mapping+0x60/0x70
> > > >      [<ffffffff805d1318>] of_irq_parse_and_map_pci+0x20/0x38
> > > >      [<ffffffff8049c210>] pci_fixup_irqs+0x60/0xe0
> > > >      [<ffffffff8049cd64>] xilinx_pcie_probe+0x28c/0x478
> > > >      [<ffffffff804e8ca8>] platform_drv_probe+0x50/0xd0
> > > >      [<ffffffff804e73a4>] driver_probe_device+0x2c4/0x3a0
> > > >      [<ffffffff804e7544>] __driver_attach+0xc4/0xd0
> > > >      [<ffffffff804e5254>] bus_for_each_dev+0x64/0xa8
> > > >      [<ffffffff804e5e40>] bus_add_driver+0x1f0/0x268
> > > >      [<ffffffff804e8000>] driver_register+0x68/0x118
> > > >      [<ffffffff801001a4>] do_one_initcall+0x4c/0x178
> > > >      [<ffffffff808d3ca8>] kernel_init_freeable+0x204/0x2b0
> > > >      [<ffffffff80730b68>] kernel_init+0x10/0xf8
> > > >      [<ffffffff80106218>] ret_from_kernel_thread+0x14/0x1c
> > > > 
> > > > This patch avoids that warning by creating the legacy IRQ domain
> > > > with
> > > > size 5 rather than 4, allowing it to cover the hwirq=4/INTD case.
> > > > 
> > > > 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 v5:
> > > > - New patch; replacing "PCI: xilinx: Fix INTX irq dispatch".
> > > > 
> > > > Changes in v4: None
> > > > Changes in v3: None
> > > > Changes in v2: None
> > > > 
> > > >  drivers/pci/host/pcie-xilinx.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/pci/host/pcie-xilinx.c
> > > > b/drivers/pci/host/pcie-xilinx.c
> > > > index 2fe2df51f9f8..94c71fb91648 100644
> > > > --- a/drivers/pci/host/pcie-xilinx.c
> > > > +++ b/drivers/pci/host/pcie-xilinx.c
> > > > @@ -524,7 +524,7 @@ static int xilinx_pcie_init_irq_domain(struct
> > > > xilinx_pcie_port *port)
> > > >               return -ENODEV;
> > > >       }
> > > > 
> > > > -     port->leg_domain = irq_domain_add_linear(pcie_intc_node, 4,
> > > > +     port->leg_domain = irq_domain_add_linear(pcie_intc_node, 1 +
> > > > 4,
> > > 
> > > I don't understand this.  Several drivers call
> > > irq_domain_add_linear() with
> > > a size of 4:
> > > 
> > >   dra7xx_pcie_init_irq_domain
> > >   ks_dw_pcie_host_init
> > >   advk_pcie_init_irq_domain
> > >   faraday_pci_setup_cascaded_irq
> > >   rockchip_pcie_init_irq_domain
> > >   nwl_pcie_init_irq_domain
> > > 
> > > Only one other in drivers/pci uses a size of 5:
> > > 
> > >   altera_pcie_init_irq_domain
> > > 
> > > Why can't we use a size of 4 for all of them?  We only have INTA-
> > > INTD.  Are
> > > altera and xilinx missing something to apply an offset from the 0-3
> > > space
> > > to the 1-4 space?
> > 
> > We have the same discussion before in 2016: https://lkml.org/lkml/2016/
> > 8/30/198
> 
> Thanks for digging that out.  I knew we'd discussed this before, but I
> couldn't find it in the archives.  I don't think anybody was really
> satisfied with the outcome, but we accepted it to make forward
> progress.
> 
> > This is because legacy interrupt is start with index 1 instead of 0.
> 
> I'm not buying this.  Your argument was that "the hwirq for legacy
> interrupts will start at 0x1 to 0x4 (INTA to INTD) and these values
> are as per PCIe specification for legacy interrupts.  So these cannot
> be numbered from 0."
> 
> But all the other drivers I mentioned get along with the 0-3 range
> somehow.  If there's something different about altera and xilinx that
> means they can't use the same solution the others do, I'd like to know
> what it is.

Note that with v4 of this patchset[1] I was using hwirq numbers 0-3 with pcie-
xilinx just fine, however:

1) Bharat complained.

2) It does require that the DT interrupt-map property be set accordingly, 
which I guess may mean we're stuck with hwirq 1-4 for drivers that already use 
them.

Thanks,
    Paul

[1] https://patchwork.kernel.org/patch/9763191/

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* RE: [PATCH v5 1/4] PCI: xilinx: Create legacy IRQ domain with size 5
  2017-06-20  1:55           ` Ley Foon Tan
@ 2017-06-20  2:30             ` Bharat Kumar Gogada
  -1 siblings, 0 replies; 27+ messages in thread
From: Bharat Kumar Gogada @ 2017-06-20  2:30 UTC (permalink / raw)
  To: Ley Foon Tan, Bjorn Helgaas
  Cc: Paul Burton, linux-pci, Ravikiran Gummaluri, Bjorn Helgaas,
	Michal Simek, linux-mips, Thomas Gleixner, Ley Foon Tan,
	Marc Zyngier

PiBTdWJqZWN0OiBSZTogW1BBVENIIHY1IDEvNF0gUENJOiB4aWxpbng6IENyZWF0ZSBsZWdhY3kg
SVJRIGRvbWFpbiB3aXRoIHNpemUgNQ0KPiANCj4gT24gTW9uLCAyMDE3LTA2LTE5IGF0IDIwOjQ5
IC0wNTAwLCBCam9ybiBIZWxnYWFzIHdyb3RlOg0KPiA+IFsrY2MgTWFyY10NCj4gPg0KPiA+IE9u
IFR1ZSwgSnVuIDIwLCAyMDE3IGF0IDA4OjM4OjE0QU0gKzA4MDAsIExleSBGb29uIFRhbiB3cm90
ZToNCj4gPiA+DQo+ID4gPiBPbiBNb24sIDIwMTctMDYtMTkgYXQgMTg6NDcgLTA1MDAsIEJqb3Ju
IEhlbGdhYXMgd3JvdGU6DQo+ID4gPiA+DQo+ID4gPiA+IFsrY2MgVGhvbWFzLCBMZXkgRm9vbl0N
Cj4gPiA+ID4NCj4gPiA+ID4gT24gU2F0LCBKdW4gMTcsIDIwMTcgYXQgMTI6NTc6MzhQTSAtMDcw
MCwgUGF1bCBCdXJ0b24gd3JvdGU6DQo+ID4gPiA+ID4NCj4gPiA+ID4gPg0KPiA+ID4gPiA+IFRo
ZSBkcml2ZXIgZXhwZWN0cyB0byB1c2UgaGFyZHdhcmUgSVJRIG51bWJlcnMgMSB0aHJvdWdoIDQg
Zm9yDQo+ID4gPiA+ID4gSU5UWCBpbnRlcnJ1cHRzLCBidXQgb25seSBjcmVhdGVzIGFuIElSUSBk
b21haW4gb2Ygc2l6ZSA0IChpZS4NCj4gPiA+ID4gPiBJUlEgbnVtYmVycyAwIHRocm91Z2ggMyku
IFRoaXMgcmVzdWx0cyBpbiBhIHdhcm5pbmcgZnJvbQ0KPiA+ID4gPiA+IGlycV9kb21haW5fYXNz
b2NpYXRlIHdoZW4gaXQgaXMgY2FsbGVkIHdpdGggaHdpcnE9NDoNCj4gPiA+ID4gPg0KPiA+ID4g
PiA+IMKgwqDCoMKgwqBXQVJOSU5HOiBDUFU6IDAgUElEOiAxIGF0IGtlcm5lbC9pcnEvaXJxZG9t
YWluLmM6MzY1DQo+ID4gPiA+ID4gwqDCoMKgwqDCoMKgwqDCoMKgaXJxX2RvbWFpbl9hc3NvY2lh
dGUrMHgxNzAvMHgyMjANCj4gPiA+ID4gPiDCoMKgwqDCoMKgZXJyb3I6IGh3aXJxIDB4NCBpcyB0
b28gbGFyZ2UgZm9yIGR1bW15DQo+ID4gPiA+ID4gwqDCoMKgwqDCoE1vZHVsZXMgbGlua2VkIGlu
Og0KPiA+ID4gPiA+IMKgwqDCoMKgwqBDUFU6IDAgUElEOiAxIENvbW06IHN3YXBwZXIvMCBUYWlu
dGVkOiBHwqDCoMKgwqDCoMKgwqDCoFcNCj4gPiA+ID4gPiDCoMKgwqDCoMKgwqDCoMKgwqA0LjEy
LjAtcmM1LTAwMTI2LWcxOWUxYjNhMTBhYWQtZGlydHkgIzQyNw0KPiA+ID4gPiA+IMKgwqDCoMKg
wqBTdGFjayA6IDAwMDAwMDAwMDAwMDAwMDAgMDAwMDAwMDAwMDAwMDAwNCAwMDAwMDAwMDAwMDAw
MDA2DQo+ID4gPiA+ID4gZmZmZmZmZmY4MDkyYzc4YQ0KPiA+ID4gPiA+IMKgwqDCoMKgwqDCoMKg
wqDCoMKgwqDCoMKgMDAwMDAwMDAwMDAwMDA2MSBmZmZmZmZmZjgwMThiZjYwIDAwMDAwMDAwMDAw
MDAwMDANCj4gPiA+ID4gPiAwMDAwMDAwMDAwMDAwMDAwDQo+ID4gPiA+ID4gwqDCoMKgwqDCoMKg
wqDCoMKgwqDCoMKgwqBmZmZmZmZmZjgwODhjMjg3IGZmZmZmZmZmODA4MTFkMTggYTgwMDAwMDBm
ZmM2MDAwMA0KPiA+ID4gPiA+IGZmZmZmZmZmODA5MjY2NzgNCj4gPiA+ID4gPiDCoMKgwqDCoMKg
wqDCoMKgwqDCoMKgwqDCoDAwMDAwMDAwMDAwMDAwMDEgMDAwMDAwMDAwMDAwMDAwMCBmZmZmZmZm
ZjgwODg3ODgwDQo+ID4gPiA+ID4gZmZmZmZmZmY4MDk2MDAwMA0KPiA+ID4gPiA+IMKgwqDCoMKg
wqDCoMKgwqDCoMKgwqDCoMKgZmZmZmZmZmY4MDkyMDAwMCBmZmZmZmZmZjgwMWU2NzQ0IGZmZmZm
ZmZmODA4ODc4ODANCj4gPiA+ID4gPiBhODAwMDAwMGZmYzRmOGY4DQo+ID4gPiA+ID4gwqDCoMKg
wqDCoMKgwqDCoMKgwqDCoMKgwqAwMDAwMDAwMDAwMDAwODljIGZmZmZmZmZmODAxOGQyNjAgMDAw
MDAwMDAwMDAxMDAwMA0KPiA+ID4gPiA+IGZmZmZmZmZmODA4MTFkMTgNCj4gPiA+ID4gPiDCoMKg
wqDCoMKgwqDCoMKgwqDCoMKgwqDCoDAwMDAwMDAwMDAwMDAwMDAgMDAwMDAwMDAwMDAwMDAwMSAw
MDAwMDAwMDAwMDAwMDAwDQo+ID4gPiA+ID4gMDAwMDAwMDAwMDAwMDAwMA0KPiA+ID4gPiA+IMKg
wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgMDAwMDAwMDAwMDAwMDAwMCBhODAwMDAwMGZmYzRmODQw
IDAwMDAwMDAwMDAwMDAwMDANCj4gPiA+ID4gPiBmZmZmZmZmZjgwNDJjZjM0DQo+ID4gPiA+ID4g
wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqAwMDAwMDAwMDAwMDAwMDAwIDAwMDAwMDAwMDAwMDAw
MDAgMDAwMDAwMDAwMDAwMDAwMA0KPiA+ID4gPiA+IDAwMDAwMDAwMDAwNDBjMDANCj4gPiA+ID4g
PiDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoDAwMDAwMDAwMDAwMDAwMDAgZmZmZmZmZmY4MDEw
ZDFjOCAwMDAwMDAwMDAwMDAwMDAwDQo+ID4gPiA+ID4gZmZmZmZmZmY4MDQyY2YzNA0KPiA+ID4g
PiA+IMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgLi4uDQo+ID4gPiA+ID4gwqDCoMKgwqDCoENh
bGwgVHJhY2U6DQo+ID4gPiA+ID4gwqDCoMKgwqDCoFs8ZmZmZmZmZmY4MDEwZDFjOD5dIHNob3df
c3RhY2srMHg4MC8weGEwDQo+ID4gPiA+ID4gwqDCoMKgwqDCoFs8ZmZmZmZmZmY4MDQyY2YzND5d
IGR1bXBfc3RhY2srMHhkNC8weDExMA0KPiA+ID4gPiA+IMKgwqDCoMKgwqBbPGZmZmZmZmZmODAx
M2VhOTg+XSBfX3dhcm4rMHhmMC8weDEwOA0KPiA+ID4gPiA+IMKgwqDCoMKgwqBbPGZmZmZmZmZm
ODAxM2ViMTQ+XSB3YXJuX3Nsb3dwYXRoX2ZtdCsweDNjLzB4NDgNCj4gPiA+ID4gPiDCoMKgwqDC
oMKgWzxmZmZmZmZmZjgwMTk2NTI4Pl0gaXJxX2RvbWFpbl9hc3NvY2lhdGUrMHgxNzAvMHgyMjAN
Cj4gPiA+ID4gPiDCoMKgwqDCoMKgWzxmZmZmZmZmZjgwMTk2YmYwPl0gaXJxX2NyZWF0ZV9tYXBw
aW5nKzB4ODgvMHgxMTgNCj4gPiA+ID4gPiDCoMKgwqDCoMKgWzxmZmZmZmZmZjgwMTk3NmE4Pl0g
aXJxX2NyZWF0ZV9md3NwZWNfbWFwcGluZysweGI4LzB4MzIwDQo+ID4gPiA+ID4gwqDCoMKgwqDC
oFs8ZmZmZmZmZmY4MDE5Nzk3MD5dIGlycV9jcmVhdGVfb2ZfbWFwcGluZysweDYwLzB4NzANCj4g
PiA+ID4gPiDCoMKgwqDCoMKgWzxmZmZmZmZmZjgwNWQxMzE4Pl0gb2ZfaXJxX3BhcnNlX2FuZF9t
YXBfcGNpKzB4MjAvMHgzOA0KPiA+ID4gPiA+IMKgwqDCoMKgwqBbPGZmZmZmZmZmODA0OWMyMTA+
XSBwY2lfZml4dXBfaXJxcysweDYwLzB4ZTANCj4gPiA+ID4gPiDCoMKgwqDCoMKgWzxmZmZmZmZm
ZjgwNDljZDY0Pl0geGlsaW54X3BjaWVfcHJvYmUrMHgyOGMvMHg0NzgNCj4gPiA+ID4gPiDCoMKg
wqDCoMKgWzxmZmZmZmZmZjgwNGU4Y2E4Pl0gcGxhdGZvcm1fZHJ2X3Byb2JlKzB4NTAvMHhkMA0K
PiA+ID4gPiA+IMKgwqDCoMKgwqBbPGZmZmZmZmZmODA0ZTczYTQ+XSBkcml2ZXJfcHJvYmVfZGV2
aWNlKzB4MmM0LzB4M2EwDQo+ID4gPiA+ID4gwqDCoMKgwqDCoFs8ZmZmZmZmZmY4MDRlNzU0ND5d
IF9fZHJpdmVyX2F0dGFjaCsweGM0LzB4ZDANCj4gPiA+ID4gPiDCoMKgwqDCoMKgWzxmZmZmZmZm
ZjgwNGU1MjU0Pl0gYnVzX2Zvcl9lYWNoX2RldisweDY0LzB4YTgNCj4gPiA+ID4gPiDCoMKgwqDC
oMKgWzxmZmZmZmZmZjgwNGU1ZTQwPl0gYnVzX2FkZF9kcml2ZXIrMHgxZjAvMHgyNjgNCj4gPiA+
ID4gPiDCoMKgwqDCoMKgWzxmZmZmZmZmZjgwNGU4MDAwPl0gZHJpdmVyX3JlZ2lzdGVyKzB4Njgv
MHgxMTgNCj4gPiA+ID4gPiDCoMKgwqDCoMKgWzxmZmZmZmZmZjgwMTAwMWE0Pl0gZG9fb25lX2lu
aXRjYWxsKzB4NGMvMHgxNzgNCj4gPiA+ID4gPiDCoMKgwqDCoMKgWzxmZmZmZmZmZjgwOGQzY2E4
Pl0ga2VybmVsX2luaXRfZnJlZWFibGUrMHgyMDQvMHgyYjANCj4gPiA+ID4gPiDCoMKgwqDCoMKg
WzxmZmZmZmZmZjgwNzMwYjY4Pl0ga2VybmVsX2luaXQrMHgxMC8weGY4DQo+ID4gPiA+ID4gwqDC
oMKgwqDCoFs8ZmZmZmZmZmY4MDEwNjIxOD5dIHJldF9mcm9tX2tlcm5lbF90aHJlYWQrMHgxNC8w
eDFjDQo+ID4gPiA+ID4NCj4gPiA+ID4gPiBUaGlzIHBhdGNoIGF2b2lkcyB0aGF0IHdhcm5pbmcg
YnkgY3JlYXRpbmcgdGhlIGxlZ2FjeSBJUlEgZG9tYWluDQo+ID4gPiA+ID4gd2l0aCBzaXplIDUg
cmF0aGVyIHRoYW4gNCwgYWxsb3dpbmcgaXQgdG8gY292ZXIgdGhlIGh3aXJxPTQvSU5URA0KPiA+
ID4gPiA+IGNhc2UuDQo+ID4gPiA+ID4NCj4gPiA+ID4gPiBTaWduZWQtb2ZmLWJ5OiBQYXVsIEJ1
cnRvbiA8cGF1bC5idXJ0b25AaW1ndGVjLmNvbT4NCj4gPiA+ID4gPiBDYzogQmhhcmF0IEt1bWFy
IEdvZ2FkYSA8YmhhcmF0a3VAeGlsaW54LmNvbT4NCj4gPiA+ID4gPiBDYzogQmpvcm4gSGVsZ2Fh
cyA8YmhlbGdhYXNAZ29vZ2xlLmNvbT4NCj4gPiA+ID4gPiBDYzogTWljaGFsIFNpbWVrIDxtaWNo
YWwuc2ltZWtAeGlsaW54LmNvbT4NCj4gPiA+ID4gPiBDYzogUmF2aWtpcmFuIEd1bW1hbHVyaSA8
cmd1bW1hbEB4aWxpbnguY29tPg0KPiA+ID4gPiA+IENjOiBsaW51eC1wY2lAdmdlci5rZXJuZWwu
b3JnDQo+ID4gPiA+ID4NCj4gPiA+ID4gPiAtLS0NCj4gPiA+ID4gPg0KPiA+ID4gPiA+IENoYW5n
ZXMgaW4gdjU6DQo+ID4gPiA+ID4gLSBOZXcgcGF0Y2g7IHJlcGxhY2luZyAiUENJOiB4aWxpbng6
IEZpeCBJTlRYIGlycSBkaXNwYXRjaCIuDQo+ID4gPiA+ID4NCj4gPiA+ID4gPiBDaGFuZ2VzIGlu
IHY0OiBOb25lDQo+ID4gPiA+ID4gQ2hhbmdlcyBpbiB2MzogTm9uZQ0KPiA+ID4gPiA+IENoYW5n
ZXMgaW4gdjI6IE5vbmUNCj4gPiA+ID4gPg0KPiA+ID4gPiA+IMKgZHJpdmVycy9wY2kvaG9zdC9w
Y2llLXhpbGlueC5jIHwgMiArLQ0KPiA+ID4gPiA+IMKgMSBmaWxlIGNoYW5nZWQsIDEgaW5zZXJ0
aW9uKCspLCAxIGRlbGV0aW9uKC0pDQo+ID4gPiA+ID4NCj4gPiA+ID4gPiBkaWZmIC0tZ2l0IGEv
ZHJpdmVycy9wY2kvaG9zdC9wY2llLXhpbGlueC5jDQo+ID4gPiA+ID4gYi9kcml2ZXJzL3BjaS9o
b3N0L3BjaWUteGlsaW54LmMgaW5kZXgNCj4gPiA+ID4gPiAyZmUyZGY1MWY5ZjguLjk0YzcxZmI5
MTY0OCAxMDA2NDQNCj4gPiA+ID4gPiAtLS0gYS9kcml2ZXJzL3BjaS9ob3N0L3BjaWUteGlsaW54
LmMNCj4gPiA+ID4gPiArKysgYi9kcml2ZXJzL3BjaS9ob3N0L3BjaWUteGlsaW54LmMNCj4gPiA+
ID4gPiBAQCAtNTI0LDcgKzUyNCw3IEBAIHN0YXRpYyBpbnQNCj4gPiA+ID4gPiB4aWxpbnhfcGNp
ZV9pbml0X2lycV9kb21haW4oc3RydWN0DQo+ID4gPiA+ID4geGlsaW54X3BjaWVfcG9ydCAqcG9y
dCkNCj4gPiA+ID4gPiDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgcmV0dXJuIC1FTk9ERVY7
DQo+ID4gPiA+ID4gwqDCoMKgwqDCoMKgfQ0KPiA+ID4gPiA+DQo+ID4gPiA+ID4gLcKgwqDCoMKg
wqBwb3J0LT5sZWdfZG9tYWluID0gaXJxX2RvbWFpbl9hZGRfbGluZWFyKHBjaWVfaW50Y19ub2Rl
LA0KPiA+ID4gPiA+IDQsDQo+ID4gPiA+ID4gK8KgwqDCoMKgwqBwb3J0LT5sZWdfZG9tYWluID0g
aXJxX2RvbWFpbl9hZGRfbGluZWFyKHBjaWVfaW50Y19ub2RlLA0KPiA+ID4gPiA+IDEgKw0KPiA+
ID4gPiA+IDQsDQo+ID4gPiA+IEkgZG9uJ3QgdW5kZXJzdGFuZCB0aGlzLsKgwqBTZXZlcmFsIGRy
aXZlcnMgY2FsbA0KPiA+ID4gPiBpcnFfZG9tYWluX2FkZF9saW5lYXIoKSB3aXRoDQo+ID4gPiA+
IGEgc2l6ZSBvZiA0Og0KPiA+ID4gPg0KPiA+ID4gPiDCoCBkcmE3eHhfcGNpZV9pbml0X2lycV9k
b21haW4NCj4gPiA+ID4gwqAga3NfZHdfcGNpZV9ob3N0X2luaXQNCj4gPiA+ID4gwqAgYWR2a19w
Y2llX2luaXRfaXJxX2RvbWFpbg0KPiA+ID4gPiDCoCBmYXJhZGF5X3BjaV9zZXR1cF9jYXNjYWRl
ZF9pcnENCj4gPiA+ID4gwqAgcm9ja2NoaXBfcGNpZV9pbml0X2lycV9kb21haW4NCj4gPiA+ID4g
wqAgbndsX3BjaWVfaW5pdF9pcnFfZG9tYWluDQo+ID4gPiA+DQo+ID4gPiA+IE9ubHkgb25lIG90
aGVyIGluIGRyaXZlcnMvcGNpIHVzZXMgYSBzaXplIG9mIDU6DQo+ID4gPiA+DQo+ID4gPiA+IMKg
IGFsdGVyYV9wY2llX2luaXRfaXJxX2RvbWFpbg0KPiA+ID4gPg0KPiA+ID4gPiBXaHkgY2FuJ3Qg
d2UgdXNlIGEgc2l6ZSBvZiA0IGZvciBhbGwgb2YgdGhlbT/CoMKgV2Ugb25seSBoYXZlIElOVEEt
DQo+ID4gPiA+IElOVEQuwqDCoEFyZSBhbHRlcmEgYW5kIHhpbGlueCBtaXNzaW5nIHNvbWV0aGlu
ZyB0byBhcHBseSBhbiBvZmZzZXQNCj4gPiA+ID4gZnJvbSB0aGUNCj4gPiA+ID4gMC0zDQo+ID4g
PiA+IHNwYWNlDQo+ID4gPiA+IHRvIHRoZSAxLTQgc3BhY2U/DQo+ID4gPiBXZSBoYXZlIHRoZSBz
YW1lIGRpc2N1c3Npb24gYmVmb3JlIGluIDIwMTY6wqBodHRwczovL2xrbWwub3JnL2xrbWwvMg0K
PiA+ID4gMDE2Lw0KPiA+ID4gOC8zMC8xOTgNCj4gPiBUaGFua3MgZm9yIGRpZ2dpbmcgdGhhdCBv
dXQuwqDCoEkga25ldyB3ZSdkIGRpc2N1c3NlZCB0aGlzIGJlZm9yZSwgYnV0IEkNCj4gPiBjb3Vs
ZG4ndCBmaW5kIGl0IGluIHRoZSBhcmNoaXZlcy7CoMKgSSBkb24ndCB0aGluayBhbnlib2R5IHdh
cyByZWFsbHkNCj4gPiBzYXRpc2ZpZWQgd2l0aCB0aGUgb3V0Y29tZSwgYnV0IHdlIGFjY2VwdGVk
IGl0IHRvIG1ha2UgZm9yd2FyZA0KPiA+IHByb2dyZXNzLg0KPiA+DQo+ID4gPg0KPiA+ID4gVGhp
cyBpcyBiZWNhdXNlIGxlZ2FjeSBpbnRlcnJ1cHQgaXMgc3RhcnQgd2l0aCBpbmRleCAxIGluc3Rl
YWQgb2YgMC4NCj4gPiBJJ20gbm90IGJ1eWluZyB0aGlzLsKgwqBZb3VyIGFyZ3VtZW50IHdhcyB0
aGF0ICJ0aGUgaHdpcnEgZm9yIGxlZ2FjeQ0KPiA+IGludGVycnVwdHMgd2lsbCBzdGFydCBhdCAw
eDEgdG8gMHg0IChJTlRBIHRvIElOVEQpIGFuZCB0aGVzZSB2YWx1ZXMNCj4gPiBhcmUgYXMgcGVy
IFBDSWUgc3BlY2lmaWNhdGlvbiBmb3IgbGVnYWN5IGludGVycnVwdHMuwqDCoFNvIHRoZXNlIGNh
bm5vdA0KPiA+IGJlIG51bWJlcmVkIGZyb20gMC4iDQo+ID4NCj4gPiBCdXQgYWxsIHRoZSBvdGhl
ciBkcml2ZXJzIEkgbWVudGlvbmVkIGdldCBhbG9uZyB3aXRoIHRoZSAwLTMgcmFuZ2UNCj4gPiBz
b21laG93LsKgwqBJZiB0aGVyZSdzIHNvbWV0aGluZyBkaWZmZXJlbnQgYWJvdXQgYWx0ZXJhIGFu
ZCB4aWxpbnggdGhhdA0KPiA+IG1lYW5zIHRoZXkgY2FuJ3QgdXNlIHRoZSBzYW1lIHNvbHV0aW9u
IHRoZSBvdGhlcnMgZG8sIEknZCBsaWtlIHRvIGtub3cNCj4gPiB3aGF0IGl0IGlzLg0KPiBJJ20g
bm90IHN1cmUgdGhvc2UgZHJpdmVycyB3aXRoIGluZGV4IDAtMyByYW5nZSB0ZXN0ZWQgd2l0aCA0
IGxlZ2FjeSBpbnRlcnJ1cHRzIG9yDQo+IG5vdC4gSXQgd2lsbCBub3QgaGFzIGVycm9yIHVudGls
IHNvbWVvbmUgcmVxdWVzdGluZyA0IGxlZ2FjeSBpbnRlcnJ1cHRzLiBXZSBzZWUNCj4gdGhpcyBl
cnJvciB3aGVuIHdlIGVuYWJsaW5nIG11bHRpLWZ1bmN0aW9uIGVuZHBvaW50ICg0IGZ1bmN0aW9u
cykuIEkgYmVsaWV2ZSB0aGlzDQo+IGlzIG5vdCBhbHRlcmEgb3IgeGlsaW54IHNwZWNpZmljLg0K
DQpIaSBCam9ybiwNCg0KWWVzIGFzIG1lbnRpb25lZCBieSBMZXkgRm9vbiBpdCdzIG5vdCBYaWxp
bnggb3IgQWx0ZXJhIHNwZWNpZmljLCBhbmQgdGhlIGlzc3VlIHNob3dzIA0KdXAgb25seSwgd2hl
biB3ZSBoYXZlIG11bHRpZnVuY3Rpb24gZGV2aWNlIHdpdGggNCBmdW5jdGlvbnMuIA0KQXMgSSBh
bHJlYWR5IG1lbnRpb25lZCBpbiB0aGUgYWJvdmUgcG9pbnRlZCBkaXNjdXNzaW9uLCB0aGUgaXNz
dWUgaXMgc3Vic3lzdGVtIA0KY3JlYXRlcyAgaHdpcnEgYmFzZWQgb24gUENJX0lOVEVSUlVQVF9Q
SU4gd2hpY2ggc3RhcnRzIGZyb20gMHgxLCBidXQgaW4gDQpJUlEgZG9tYWlucyBod2lycSBzdGFy
dCBmcm9tIDAsIGR1ZSB0byB0aGlzIGRpZmZlcmVuY2UsIGlzc3VlIGFyaXNlcyANCndoZW4gd2Ug
dXNlIG11bHRpZnVuY3Rpb24gZGV2aWNlLg0KDQpCaGFyYXQNCg==

^ permalink raw reply	[flat|nested] 27+ messages in thread

* RE: [PATCH v5 1/4] PCI: xilinx: Create legacy IRQ domain with size 5
@ 2017-06-20  2:30             ` Bharat Kumar Gogada
  0 siblings, 0 replies; 27+ messages in thread
From: Bharat Kumar Gogada @ 2017-06-20  2:30 UTC (permalink / raw)
  To: Ley Foon Tan, Bjorn Helgaas
  Cc: Paul Burton, linux-pci, Ravikiran Gummaluri, Bjorn Helgaas,
	Michal Simek, linux-mips, Thomas Gleixner, Ley Foon Tan,
	Marc Zyngier

> Subject: Re: [PATCH v5 1/4] PCI: xilinx: Create legacy IRQ domain with size 5
> 
> On Mon, 2017-06-19 at 20:49 -0500, Bjorn Helgaas wrote:
> > [+cc Marc]
> >
> > On Tue, Jun 20, 2017 at 08:38:14AM +0800, Ley Foon Tan wrote:
> > >
> > > On Mon, 2017-06-19 at 18:47 -0500, Bjorn Helgaas wrote:
> > > >
> > > > [+cc Thomas, Ley Foon]
> > > >
> > > > On Sat, Jun 17, 2017 at 12:57:38PM -0700, Paul Burton wrote:
> > > > >
> > > > >
> > > > > The driver expects to use hardware IRQ numbers 1 through 4 for
> > > > > INTX interrupts, but only creates an IRQ domain of size 4 (ie.
> > > > > IRQ numbers 0 through 3). This results in a warning from
> > > > > irq_domain_associate when it is called with hwirq=4:
> > > > >
> > > > >      WARNING: CPU: 0 PID: 1 at kernel/irq/irqdomain.c:365
> > > > >          irq_domain_associate+0x170/0x220
> > > > >      error: hwirq 0x4 is too large for dummy
> > > > >      Modules linked in:
> > > > >      CPU: 0 PID: 1 Comm: swapper/0 Tainted: G        W
> > > > >          4.12.0-rc5-00126-g19e1b3a10aad-dirty #427
> > > > >      Stack : 0000000000000000 0000000000000004 0000000000000006
> > > > > ffffffff8092c78a
> > > > >              0000000000000061 ffffffff8018bf60 0000000000000000
> > > > > 0000000000000000
> > > > >              ffffffff8088c287 ffffffff80811d18 a8000000ffc60000
> > > > > ffffffff80926678
> > > > >              0000000000000001 0000000000000000 ffffffff80887880
> > > > > ffffffff80960000
> > > > >              ffffffff80920000 ffffffff801e6744 ffffffff80887880
> > > > > a8000000ffc4f8f8
> > > > >              000000000000089c ffffffff8018d260 0000000000010000
> > > > > ffffffff80811d18
> > > > >              0000000000000000 0000000000000001 0000000000000000
> > > > > 0000000000000000
> > > > >              0000000000000000 a8000000ffc4f840 0000000000000000
> > > > > ffffffff8042cf34
> > > > >              0000000000000000 0000000000000000 0000000000000000
> > > > > 0000000000040c00
> > > > >              0000000000000000 ffffffff8010d1c8 0000000000000000
> > > > > ffffffff8042cf34
> > > > >              ...
> > > > >      Call Trace:
> > > > >      [<ffffffff8010d1c8>] show_stack+0x80/0xa0
> > > > >      [<ffffffff8042cf34>] dump_stack+0xd4/0x110
> > > > >      [<ffffffff8013ea98>] __warn+0xf0/0x108
> > > > >      [<ffffffff8013eb14>] warn_slowpath_fmt+0x3c/0x48
> > > > >      [<ffffffff80196528>] irq_domain_associate+0x170/0x220
> > > > >      [<ffffffff80196bf0>] irq_create_mapping+0x88/0x118
> > > > >      [<ffffffff801976a8>] irq_create_fwspec_mapping+0xb8/0x320
> > > > >      [<ffffffff80197970>] irq_create_of_mapping+0x60/0x70
> > > > >      [<ffffffff805d1318>] of_irq_parse_and_map_pci+0x20/0x38
> > > > >      [<ffffffff8049c210>] pci_fixup_irqs+0x60/0xe0
> > > > >      [<ffffffff8049cd64>] xilinx_pcie_probe+0x28c/0x478
> > > > >      [<ffffffff804e8ca8>] platform_drv_probe+0x50/0xd0
> > > > >      [<ffffffff804e73a4>] driver_probe_device+0x2c4/0x3a0
> > > > >      [<ffffffff804e7544>] __driver_attach+0xc4/0xd0
> > > > >      [<ffffffff804e5254>] bus_for_each_dev+0x64/0xa8
> > > > >      [<ffffffff804e5e40>] bus_add_driver+0x1f0/0x268
> > > > >      [<ffffffff804e8000>] driver_register+0x68/0x118
> > > > >      [<ffffffff801001a4>] do_one_initcall+0x4c/0x178
> > > > >      [<ffffffff808d3ca8>] kernel_init_freeable+0x204/0x2b0
> > > > >      [<ffffffff80730b68>] kernel_init+0x10/0xf8
> > > > >      [<ffffffff80106218>] ret_from_kernel_thread+0x14/0x1c
> > > > >
> > > > > This patch avoids that warning by creating the legacy IRQ domain
> > > > > with size 5 rather than 4, allowing it to cover the hwirq=4/INTD
> > > > > case.
> > > > >
> > > > > 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 v5:
> > > > > - New patch; replacing "PCI: xilinx: Fix INTX irq dispatch".
> > > > >
> > > > > Changes in v4: None
> > > > > Changes in v3: None
> > > > > Changes in v2: None
> > > > >
> > > > >  drivers/pci/host/pcie-xilinx.c | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/pci/host/pcie-xilinx.c
> > > > > b/drivers/pci/host/pcie-xilinx.c index
> > > > > 2fe2df51f9f8..94c71fb91648 100644
> > > > > --- a/drivers/pci/host/pcie-xilinx.c
> > > > > +++ b/drivers/pci/host/pcie-xilinx.c
> > > > > @@ -524,7 +524,7 @@ static int
> > > > > xilinx_pcie_init_irq_domain(struct
> > > > > xilinx_pcie_port *port)
> > > > >               return -ENODEV;
> > > > >       }
> > > > >
> > > > > -     port->leg_domain = irq_domain_add_linear(pcie_intc_node,
> > > > > 4,
> > > > > +     port->leg_domain = irq_domain_add_linear(pcie_intc_node,
> > > > > 1 +
> > > > > 4,
> > > > I don't understand this.  Several drivers call
> > > > irq_domain_add_linear() with
> > > > a size of 4:
> > > >
> > > >   dra7xx_pcie_init_irq_domain
> > > >   ks_dw_pcie_host_init
> > > >   advk_pcie_init_irq_domain
> > > >   faraday_pci_setup_cascaded_irq
> > > >   rockchip_pcie_init_irq_domain
> > > >   nwl_pcie_init_irq_domain
> > > >
> > > > Only one other in drivers/pci uses a size of 5:
> > > >
> > > >   altera_pcie_init_irq_domain
> > > >
> > > > Why can't we use a size of 4 for all of them?  We only have INTA-
> > > > INTD.  Are altera and xilinx missing something to apply an offset
> > > > from the
> > > > 0-3
> > > > space
> > > > to the 1-4 space?
> > > We have the same discussion before in 2016: https://lkml.org/lkml/2
> > > 016/
> > > 8/30/198
> > Thanks for digging that out.  I knew we'd discussed this before, but I
> > couldn't find it in the archives.  I don't think anybody was really
> > satisfied with the outcome, but we accepted it to make forward
> > progress.
> >
> > >
> > > This is because legacy interrupt is start with index 1 instead of 0.
> > I'm not buying this.  Your argument was that "the hwirq for legacy
> > interrupts will start at 0x1 to 0x4 (INTA to INTD) and these values
> > are as per PCIe specification for legacy interrupts.  So these cannot
> > be numbered from 0."
> >
> > But all the other drivers I mentioned get along with the 0-3 range
> > somehow.  If there's something different about altera and xilinx that
> > means they can't use the same solution the others do, I'd like to know
> > what it is.
> I'm not sure those drivers with index 0-3 range tested with 4 legacy interrupts or
> not. It will not has error until someone requesting 4 legacy interrupts. We see
> this error when we enabling multi-function endpoint (4 functions). I believe this
> is not altera or xilinx specific.

Hi Bjorn,

Yes as mentioned by Ley Foon it's not Xilinx or Altera specific, and the issue shows 
up only, when we have multifunction device with 4 functions. 
As I already mentioned in the above pointed discussion, the issue is subsystem 
creates  hwirq based on PCI_INTERRUPT_PIN which starts from 0x1, but in 
IRQ domains hwirq start from 0, due to this difference, issue arises 
when we use multifunction device.

Bharat

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 1/4] PCI: xilinx: Create legacy IRQ domain with size 5
@ 2017-07-09 22:59             ` Paul Burton
  0 siblings, 0 replies; 27+ messages in thread
From: Paul Burton @ 2017-07-09 22:59 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Ley Foon Tan, linux-pci, Bharat Kumar Gogada,
	Ravikiran Gummaluri, Bjorn Helgaas, Michal Simek, linux-mips,
	Thomas Gleixner, Ley Foon Tan, Marc Zyngier

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

Hi Bjorn,

On Monday, 19 June 2017 19:07:05 PDT Paul Burton wrote:
> Hi Bjorn,
> 
> On Monday, 19 June 2017 18:49:03 PDT Bjorn Helgaas wrote:
> > [+cc Marc]
> > 
> > On Tue, Jun 20, 2017 at 08:38:14AM +0800, Ley Foon Tan wrote:
> > > On Mon, 2017-06-19 at 18:47 -0500, Bjorn Helgaas wrote:
> > > > [+cc Thomas, Ley Foon]
> > > > 
> > > > On Sat, Jun 17, 2017 at 12:57:38PM -0700, Paul Burton wrote:
> > > > > The driver expects to use hardware IRQ numbers 1 through 4 for INTX
> > > > > interrupts, but only creates an IRQ domain of size 4 (ie. IRQ
> > > > > numbers 0
> > > > > through 3). This results in a warning from irq_domain_associate
> > > > > when it
> > > > > 
> > > > > is called with hwirq=4:
> > > > >      WARNING: CPU: 0 PID: 1 at kernel/irq/irqdomain.c:365
> > > > >      
> > > > >          irq_domain_associate+0x170/0x220
> > > > >      
> > > > >      error: hwirq 0x4 is too large for dummy
> > > > >      Modules linked in:
> > > > >      CPU: 0 PID: 1 Comm: swapper/0 Tainted: G        W
> > > > >      
> > > > >          4.12.0-rc5-00126-g19e1b3a10aad-dirty #427
> > > > >      
> > > > >      Stack : 0000000000000000 0000000000000004 0000000000000006
> > > > > 
> > > > > ffffffff8092c78a
> > > > > 
> > > > >              0000000000000061 ffffffff8018bf60 0000000000000000
> > > > > 
> > > > > 0000000000000000
> > > > > 
> > > > >              ffffffff8088c287 ffffffff80811d18 a8000000ffc60000
> > > > > 
> > > > > ffffffff80926678
> > > > > 
> > > > >              0000000000000001 0000000000000000 ffffffff80887880
> > > > > 
> > > > > ffffffff80960000
> > > > > 
> > > > >              ffffffff80920000 ffffffff801e6744 ffffffff80887880
> > > > > 
> > > > > a8000000ffc4f8f8
> > > > > 
> > > > >              000000000000089c ffffffff8018d260 0000000000010000
> > > > > 
> > > > > ffffffff80811d18
> > > > > 
> > > > >              0000000000000000 0000000000000001 0000000000000000
> > > > > 
> > > > > 0000000000000000
> > > > > 
> > > > >              0000000000000000 a8000000ffc4f840 0000000000000000
> > > > > 
> > > > > ffffffff8042cf34
> > > > > 
> > > > >              0000000000000000 0000000000000000 0000000000000000
> > > > > 
> > > > > 0000000000040c00
> > > > > 
> > > > >              0000000000000000 ffffffff8010d1c8 0000000000000000
> > > > > 
> > > > > ffffffff8042cf34
> > > > > 
> > > > >              ...
> > > > >      
> > > > >      Call Trace:
> > > > >      [<ffffffff8010d1c8>] show_stack+0x80/0xa0
> > > > >      [<ffffffff8042cf34>] dump_stack+0xd4/0x110
> > > > >      [<ffffffff8013ea98>] __warn+0xf0/0x108
> > > > >      [<ffffffff8013eb14>] warn_slowpath_fmt+0x3c/0x48
> > > > >      [<ffffffff80196528>] irq_domain_associate+0x170/0x220
> > > > >      [<ffffffff80196bf0>] irq_create_mapping+0x88/0x118
> > > > >      [<ffffffff801976a8>] irq_create_fwspec_mapping+0xb8/0x320
> > > > >      [<ffffffff80197970>] irq_create_of_mapping+0x60/0x70
> > > > >      [<ffffffff805d1318>] of_irq_parse_and_map_pci+0x20/0x38
> > > > >      [<ffffffff8049c210>] pci_fixup_irqs+0x60/0xe0
> > > > >      [<ffffffff8049cd64>] xilinx_pcie_probe+0x28c/0x478
> > > > >      [<ffffffff804e8ca8>] platform_drv_probe+0x50/0xd0
> > > > >      [<ffffffff804e73a4>] driver_probe_device+0x2c4/0x3a0
> > > > >      [<ffffffff804e7544>] __driver_attach+0xc4/0xd0
> > > > >      [<ffffffff804e5254>] bus_for_each_dev+0x64/0xa8
> > > > >      [<ffffffff804e5e40>] bus_add_driver+0x1f0/0x268
> > > > >      [<ffffffff804e8000>] driver_register+0x68/0x118
> > > > >      [<ffffffff801001a4>] do_one_initcall+0x4c/0x178
> > > > >      [<ffffffff808d3ca8>] kernel_init_freeable+0x204/0x2b0
> > > > >      [<ffffffff80730b68>] kernel_init+0x10/0xf8
> > > > >      [<ffffffff80106218>] ret_from_kernel_thread+0x14/0x1c
> > > > > 
> > > > > This patch avoids that warning by creating the legacy IRQ domain
> > > > > with
> > > > > size 5 rather than 4, allowing it to cover the hwirq=4/INTD case.
> > > > > 
> > > > > 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 v5:
> > > > > - New patch; replacing "PCI: xilinx: Fix INTX irq dispatch".
> > > > > 
> > > > > Changes in v4: None
> > > > > Changes in v3: None
> > > > > Changes in v2: None
> > > > > 
> > > > >  drivers/pci/host/pcie-xilinx.c | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/pci/host/pcie-xilinx.c
> > > > > b/drivers/pci/host/pcie-xilinx.c
> > > > > index 2fe2df51f9f8..94c71fb91648 100644
> > > > > --- a/drivers/pci/host/pcie-xilinx.c
> > > > > +++ b/drivers/pci/host/pcie-xilinx.c
> > > > > @@ -524,7 +524,7 @@ static int xilinx_pcie_init_irq_domain(struct
> > > > > xilinx_pcie_port *port)
> > > > > 
> > > > >               return -ENODEV;
> > > > >       
> > > > >       }
> > > > > 
> > > > > -     port->leg_domain = irq_domain_add_linear(pcie_intc_node, 4,
> > > > > +     port->leg_domain = irq_domain_add_linear(pcie_intc_node, 1 +
> > > > > 4,
> > > > 
> > > > I don't understand this.  Several drivers call
> > > > irq_domain_add_linear() with
> > > > 
> > > > a size of 4:
> > > >   dra7xx_pcie_init_irq_domain
> > > >   ks_dw_pcie_host_init
> > > >   advk_pcie_init_irq_domain
> > > >   faraday_pci_setup_cascaded_irq
> > > >   rockchip_pcie_init_irq_domain
> > > >   nwl_pcie_init_irq_domain
> > > > 
> > > > Only one other in drivers/pci uses a size of 5:
> > > >   altera_pcie_init_irq_domain
> > > > 
> > > > Why can't we use a size of 4 for all of them?  We only have INTA-
> > > > INTD.  Are
> > > > altera and xilinx missing something to apply an offset from the 0-3
> > > > space
> > > > to the 1-4 space?
> > > 
> > > We have the same discussion before in 2016: https://lkml.org/lkml/2016/
> > > 8/30/198
> > 
> > Thanks for digging that out.  I knew we'd discussed this before, but I
> > couldn't find it in the archives.  I don't think anybody was really
> > satisfied with the outcome, but we accepted it to make forward
> > progress.
> > 
> > > This is because legacy interrupt is start with index 1 instead of 0.
> > 
> > I'm not buying this.  Your argument was that "the hwirq for legacy
> > interrupts will start at 0x1 to 0x4 (INTA to INTD) and these values
> > are as per PCIe specification for legacy interrupts.  So these cannot
> > be numbered from 0."
> > 
> > But all the other drivers I mentioned get along with the 0-3 range
> > somehow.  If there's something different about altera and xilinx that
> > means they can't use the same solution the others do, I'd like to know
> > what it is.
> 
> Note that with v4 of this patchset[1] I was using hwirq numbers 0-3 with
> pcie- xilinx just fine, however:
> 
> 1) Bharat complained.
> 
> 2) It does require that the DT interrupt-map property be set accordingly,
> which I guess may mean we're stuck with hwirq 1-4 for drivers that already
> use them.
> 
> Thanks,
>     Paul
> 
> [1] https://patchwork.kernel.org/patch/9763191/

I see this series wasn't included in your pull request for v4.13 - is there 
anything you're waiting on?

I've produced revisions of the series that work both ways now (0<=hwirq<=3 in 
v4, 1<=hwirq<=4 in v5) so I'm not sure what more I can do.

Thanks,
    Paul

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 1/4] PCI: xilinx: Create legacy IRQ domain with size 5
@ 2017-07-09 22:59             ` Paul Burton
  0 siblings, 0 replies; 27+ messages in thread
From: Paul Burton @ 2017-07-09 22:59 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Ley Foon Tan, linux-pci, Bharat Kumar Gogada,
	Ravikiran Gummaluri, Bjorn Helgaas, Michal Simek, linux-mips,
	Thomas Gleixner, Ley Foon Tan, Marc Zyngier

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

Hi Bjorn,

On Monday, 19 June 2017 19:07:05 PDT Paul Burton wrote:
> Hi Bjorn,
> 
> On Monday, 19 June 2017 18:49:03 PDT Bjorn Helgaas wrote:
> > [+cc Marc]
> > 
> > On Tue, Jun 20, 2017 at 08:38:14AM +0800, Ley Foon Tan wrote:
> > > On Mon, 2017-06-19 at 18:47 -0500, Bjorn Helgaas wrote:
> > > > [+cc Thomas, Ley Foon]
> > > > 
> > > > On Sat, Jun 17, 2017 at 12:57:38PM -0700, Paul Burton wrote:
> > > > > The driver expects to use hardware IRQ numbers 1 through 4 for INTX
> > > > > interrupts, but only creates an IRQ domain of size 4 (ie. IRQ
> > > > > numbers 0
> > > > > through 3). This results in a warning from irq_domain_associate
> > > > > when it
> > > > > 
> > > > > is called with hwirq=4:
> > > > >      WARNING: CPU: 0 PID: 1 at kernel/irq/irqdomain.c:365
> > > > >      
> > > > >          irq_domain_associate+0x170/0x220
> > > > >      
> > > > >      error: hwirq 0x4 is too large for dummy
> > > > >      Modules linked in:
> > > > >      CPU: 0 PID: 1 Comm: swapper/0 Tainted: G        W
> > > > >      
> > > > >          4.12.0-rc5-00126-g19e1b3a10aad-dirty #427
> > > > >      
> > > > >      Stack : 0000000000000000 0000000000000004 0000000000000006
> > > > > 
> > > > > ffffffff8092c78a
> > > > > 
> > > > >              0000000000000061 ffffffff8018bf60 0000000000000000
> > > > > 
> > > > > 0000000000000000
> > > > > 
> > > > >              ffffffff8088c287 ffffffff80811d18 a8000000ffc60000
> > > > > 
> > > > > ffffffff80926678
> > > > > 
> > > > >              0000000000000001 0000000000000000 ffffffff80887880
> > > > > 
> > > > > ffffffff80960000
> > > > > 
> > > > >              ffffffff80920000 ffffffff801e6744 ffffffff80887880
> > > > > 
> > > > > a8000000ffc4f8f8
> > > > > 
> > > > >              000000000000089c ffffffff8018d260 0000000000010000
> > > > > 
> > > > > ffffffff80811d18
> > > > > 
> > > > >              0000000000000000 0000000000000001 0000000000000000
> > > > > 
> > > > > 0000000000000000
> > > > > 
> > > > >              0000000000000000 a8000000ffc4f840 0000000000000000
> > > > > 
> > > > > ffffffff8042cf34
> > > > > 
> > > > >              0000000000000000 0000000000000000 0000000000000000
> > > > > 
> > > > > 0000000000040c00
> > > > > 
> > > > >              0000000000000000 ffffffff8010d1c8 0000000000000000
> > > > > 
> > > > > ffffffff8042cf34
> > > > > 
> > > > >              ...
> > > > >      
> > > > >      Call Trace:
> > > > >      [<ffffffff8010d1c8>] show_stack+0x80/0xa0
> > > > >      [<ffffffff8042cf34>] dump_stack+0xd4/0x110
> > > > >      [<ffffffff8013ea98>] __warn+0xf0/0x108
> > > > >      [<ffffffff8013eb14>] warn_slowpath_fmt+0x3c/0x48
> > > > >      [<ffffffff80196528>] irq_domain_associate+0x170/0x220
> > > > >      [<ffffffff80196bf0>] irq_create_mapping+0x88/0x118
> > > > >      [<ffffffff801976a8>] irq_create_fwspec_mapping+0xb8/0x320
> > > > >      [<ffffffff80197970>] irq_create_of_mapping+0x60/0x70
> > > > >      [<ffffffff805d1318>] of_irq_parse_and_map_pci+0x20/0x38
> > > > >      [<ffffffff8049c210>] pci_fixup_irqs+0x60/0xe0
> > > > >      [<ffffffff8049cd64>] xilinx_pcie_probe+0x28c/0x478
> > > > >      [<ffffffff804e8ca8>] platform_drv_probe+0x50/0xd0
> > > > >      [<ffffffff804e73a4>] driver_probe_device+0x2c4/0x3a0
> > > > >      [<ffffffff804e7544>] __driver_attach+0xc4/0xd0
> > > > >      [<ffffffff804e5254>] bus_for_each_dev+0x64/0xa8
> > > > >      [<ffffffff804e5e40>] bus_add_driver+0x1f0/0x268
> > > > >      [<ffffffff804e8000>] driver_register+0x68/0x118
> > > > >      [<ffffffff801001a4>] do_one_initcall+0x4c/0x178
> > > > >      [<ffffffff808d3ca8>] kernel_init_freeable+0x204/0x2b0
> > > > >      [<ffffffff80730b68>] kernel_init+0x10/0xf8
> > > > >      [<ffffffff80106218>] ret_from_kernel_thread+0x14/0x1c
> > > > > 
> > > > > This patch avoids that warning by creating the legacy IRQ domain
> > > > > with
> > > > > size 5 rather than 4, allowing it to cover the hwirq=4/INTD case.
> > > > > 
> > > > > 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 v5:
> > > > > - New patch; replacing "PCI: xilinx: Fix INTX irq dispatch".
> > > > > 
> > > > > Changes in v4: None
> > > > > Changes in v3: None
> > > > > Changes in v2: None
> > > > > 
> > > > >  drivers/pci/host/pcie-xilinx.c | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/pci/host/pcie-xilinx.c
> > > > > b/drivers/pci/host/pcie-xilinx.c
> > > > > index 2fe2df51f9f8..94c71fb91648 100644
> > > > > --- a/drivers/pci/host/pcie-xilinx.c
> > > > > +++ b/drivers/pci/host/pcie-xilinx.c
> > > > > @@ -524,7 +524,7 @@ static int xilinx_pcie_init_irq_domain(struct
> > > > > xilinx_pcie_port *port)
> > > > > 
> > > > >               return -ENODEV;
> > > > >       
> > > > >       }
> > > > > 
> > > > > -     port->leg_domain = irq_domain_add_linear(pcie_intc_node, 4,
> > > > > +     port->leg_domain = irq_domain_add_linear(pcie_intc_node, 1 +
> > > > > 4,
> > > > 
> > > > I don't understand this.  Several drivers call
> > > > irq_domain_add_linear() with
> > > > 
> > > > a size of 4:
> > > >   dra7xx_pcie_init_irq_domain
> > > >   ks_dw_pcie_host_init
> > > >   advk_pcie_init_irq_domain
> > > >   faraday_pci_setup_cascaded_irq
> > > >   rockchip_pcie_init_irq_domain
> > > >   nwl_pcie_init_irq_domain
> > > > 
> > > > Only one other in drivers/pci uses a size of 5:
> > > >   altera_pcie_init_irq_domain
> > > > 
> > > > Why can't we use a size of 4 for all of them?  We only have INTA-
> > > > INTD.  Are
> > > > altera and xilinx missing something to apply an offset from the 0-3
> > > > space
> > > > to the 1-4 space?
> > > 
> > > We have the same discussion before in 2016: https://lkml.org/lkml/2016/
> > > 8/30/198
> > 
> > Thanks for digging that out.  I knew we'd discussed this before, but I
> > couldn't find it in the archives.  I don't think anybody was really
> > satisfied with the outcome, but we accepted it to make forward
> > progress.
> > 
> > > This is because legacy interrupt is start with index 1 instead of 0.
> > 
> > I'm not buying this.  Your argument was that "the hwirq for legacy
> > interrupts will start at 0x1 to 0x4 (INTA to INTD) and these values
> > are as per PCIe specification for legacy interrupts.  So these cannot
> > be numbered from 0."
> > 
> > But all the other drivers I mentioned get along with the 0-3 range
> > somehow.  If there's something different about altera and xilinx that
> > means they can't use the same solution the others do, I'd like to know
> > what it is.
> 
> Note that with v4 of this patchset[1] I was using hwirq numbers 0-3 with
> pcie- xilinx just fine, however:
> 
> 1) Bharat complained.
> 
> 2) It does require that the DT interrupt-map property be set accordingly,
> which I guess may mean we're stuck with hwirq 1-4 for drivers that already
> use them.
> 
> Thanks,
>     Paul
> 
> [1] https://patchwork.kernel.org/patch/9763191/

I see this series wasn't included in your pull request for v4.13 - is there 
anything you're waiting on?

I've produced revisions of the series that work both ways now (0<=hwirq<=3 in 
v4, 1<=hwirq<=4 in v5) so I'm not sure what more I can do.

Thanks,
    Paul

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* RE: [PATCH v5 1/4] PCI: xilinx: Create legacy IRQ domain with size 5
  2017-07-09 22:59             ` Paul Burton
@ 2017-07-10  5:43               ` Bharat Kumar Gogada
  -1 siblings, 0 replies; 27+ messages in thread
From: Bharat Kumar Gogada @ 2017-07-10  5:43 UTC (permalink / raw)
  To: Paul Burton, Bjorn Helgaas
  Cc: Ley Foon Tan, linux-pci, Ravikiran Gummaluri, Bjorn Helgaas,
	Michal Simek, linux-mips, Thomas Gleixner, Ley Foon Tan,
	Marc Zyngier

> -----Original Message-----
> From: linux-pci-owner@vger.kernel.org [mailto:linux-pci-
> owner@vger.kernel.org] On Behalf Of Paul Burton
> Sent: Monday, July 10, 2017 4:30 AM
> To: Bjorn Helgaas <helgaas@kernel.org>
> Cc: Ley Foon Tan <ley.foon.tan@intel.com>; linux-pci@vger.kernel.org; Bha=
rat
> Kumar Gogada <bharatku@xilinx.com>; Ravikiran Gummaluri
> <rgummal@xilinx.com>; Bjorn Helgaas <bhelgaas@google.com>; Michal Simek
> <michal.simek@xilinx.com>; linux-mips@linux-mips.org; Thomas Gleixner
> <tglx@linutronix.de>; Ley Foon Tan <lftan@altera.com>; Marc Zyngier
> <marc.zyngier@arm.com>
> Subject: Re: [PATCH v5 1/4] PCI: xilinx: Create legacy IRQ domain with si=
ze 5
>=20
> Hi Bjorn,
>=20
> On Monday, 19 June 2017 19:07:05 PDT Paul Burton wrote:
> > Hi Bjorn,
> >
> > On Monday, 19 June 2017 18:49:03 PDT Bjorn Helgaas wrote:
> > > [+cc Marc]
> > >
> > > On Tue, Jun 20, 2017 at 08:38:14AM +0800, Ley Foon Tan wrote:
> > > > On Mon, 2017-06-19 at 18:47 -0500, Bjorn Helgaas wrote:
> > > > > [+cc Thomas, Ley Foon]
> > > > >
> > > > > On Sat, Jun 17, 2017 at 12:57:38PM -0700, Paul Burton wrote:
> > > > > > The driver expects to use hardware IRQ numbers 1 through 4 for
> > > > > > INTX interrupts, but only creates an IRQ domain of size 4 (ie.
> > > > > > IRQ numbers 0 through 3). This results in a warning from
> > > > > > irq_domain_associate when it
> > > > > >
> > > > > > is called with hwirq=3D4:
> > > > > >      WARNING: CPU: 0 PID: 1 at kernel/irq/irqdomain.c:365
> > > > > >
> > > > > >          irq_domain_associate+0x170/0x220
> > > > > >
> > > > > >      error: hwirq 0x4 is too large for dummy
> > > > > >      Modules linked in:
> > > > > >      CPU: 0 PID: 1 Comm: swapper/0 Tainted: G        W
> > > > > >
> > > > > >          4.12.0-rc5-00126-g19e1b3a10aad-dirty #427
> > > > > >
> > > > > >      Stack : 0000000000000000 0000000000000004
> > > > > > 0000000000000006
> > > > > >
> > > > > > ffffffff8092c78a
> > > > > >
> > > > > >              0000000000000061 ffffffff8018bf60
> > > > > > 0000000000000000
> > > > > >
> > > > > > 0000000000000000
> > > > > >
> > > > > >              ffffffff8088c287 ffffffff80811d18
> > > > > > a8000000ffc60000
> > > > > >
> > > > > > ffffffff80926678
> > > > > >
> > > > > >              0000000000000001 0000000000000000
> > > > > > ffffffff80887880
> > > > > >
> > > > > > ffffffff80960000
> > > > > >
> > > > > >              ffffffff80920000 ffffffff801e6744
> > > > > > ffffffff80887880
> > > > > >
> > > > > > a8000000ffc4f8f8
> > > > > >
> > > > > >              000000000000089c ffffffff8018d260
> > > > > > 0000000000010000
> > > > > >
> > > > > > ffffffff80811d18
> > > > > >
> > > > > >              0000000000000000 0000000000000001
> > > > > > 0000000000000000
> > > > > >
> > > > > > 0000000000000000
> > > > > >
> > > > > >              0000000000000000 a8000000ffc4f840
> > > > > > 0000000000000000
> > > > > >
> > > > > > ffffffff8042cf34
> > > > > >
> > > > > >              0000000000000000 0000000000000000
> > > > > > 0000000000000000
> > > > > >
> > > > > > 0000000000040c00
> > > > > >
> > > > > >              0000000000000000 ffffffff8010d1c8
> > > > > > 0000000000000000
> > > > > >
> > > > > > ffffffff8042cf34
> > > > > >
> > > > > >              ...
> > > > > >
> > > > > >      Call Trace:
> > > > > >      [<ffffffff8010d1c8>] show_stack+0x80/0xa0
> > > > > >      [<ffffffff8042cf34>] dump_stack+0xd4/0x110
> > > > > >      [<ffffffff8013ea98>] __warn+0xf0/0x108
> > > > > >      [<ffffffff8013eb14>] warn_slowpath_fmt+0x3c/0x48
> > > > > >      [<ffffffff80196528>] irq_domain_associate+0x170/0x220
> > > > > >      [<ffffffff80196bf0>] irq_create_mapping+0x88/0x118
> > > > > >      [<ffffffff801976a8>] irq_create_fwspec_mapping+0xb8/0x320
> > > > > >      [<ffffffff80197970>] irq_create_of_mapping+0x60/0x70
> > > > > >      [<ffffffff805d1318>] of_irq_parse_and_map_pci+0x20/0x38
> > > > > >      [<ffffffff8049c210>] pci_fixup_irqs+0x60/0xe0
> > > > > >      [<ffffffff8049cd64>] xilinx_pcie_probe+0x28c/0x478
> > > > > >      [<ffffffff804e8ca8>] platform_drv_probe+0x50/0xd0
> > > > > >      [<ffffffff804e73a4>] driver_probe_device+0x2c4/0x3a0
> > > > > >      [<ffffffff804e7544>] __driver_attach+0xc4/0xd0
> > > > > >      [<ffffffff804e5254>] bus_for_each_dev+0x64/0xa8
> > > > > >      [<ffffffff804e5e40>] bus_add_driver+0x1f0/0x268
> > > > > >      [<ffffffff804e8000>] driver_register+0x68/0x118
> > > > > >      [<ffffffff801001a4>] do_one_initcall+0x4c/0x178
> > > > > >      [<ffffffff808d3ca8>] kernel_init_freeable+0x204/0x2b0
> > > > > >      [<ffffffff80730b68>] kernel_init+0x10/0xf8
> > > > > >      [<ffffffff80106218>] ret_from_kernel_thread+0x14/0x1c
> > > > > >
> > > > > > This patch avoids that warning by creating the legacy IRQ
> > > > > > domain with size 5 rather than 4, allowing it to cover the
> > > > > > hwirq=3D4/INTD case.
> > > > > >
> > > > > > 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 v5:
> > > > > > - New patch; replacing "PCI: xilinx: Fix INTX irq dispatch".
> > > > > >
> > > > > > Changes in v4: None
> > > > > > Changes in v3: None
> > > > > > Changes in v2: None
> > > > > >
> > > > > >  drivers/pci/host/pcie-xilinx.c | 2 +-
> > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/drivers/pci/host/pcie-xilinx.c
> > > > > > b/drivers/pci/host/pcie-xilinx.c index
> > > > > > 2fe2df51f9f8..94c71fb91648 100644
> > > > > > --- a/drivers/pci/host/pcie-xilinx.c
> > > > > > +++ b/drivers/pci/host/pcie-xilinx.c
> > > > > > @@ -524,7 +524,7 @@ static int
> > > > > > xilinx_pcie_init_irq_domain(struct
> > > > > > xilinx_pcie_port *port)
> > > > > >
> > > > > >               return -ENODEV;
> > > > > >
> > > > > >       }
> > > > > >
> > > > > > -     port->leg_domain =3D irq_domain_add_linear(pcie_intc_node=
, 4,
> > > > > > +     port->leg_domain =3D irq_domain_add_linear(pcie_intc_node=
,
> > > > > > + 1 +
> > > > > > 4,
> > > > >
> > > > > I don't understand this.  Several drivers call
> > > > > irq_domain_add_linear() with
> > > > >
> > > > > a size of 4:
> > > > >   dra7xx_pcie_init_irq_domain
> > > > >   ks_dw_pcie_host_init
> > > > >   advk_pcie_init_irq_domain
> > > > >   faraday_pci_setup_cascaded_irq
> > > > >   rockchip_pcie_init_irq_domain
> > > > >   nwl_pcie_init_irq_domain
> > > > >
> > > > > Only one other in drivers/pci uses a size of 5:
> > > > >   altera_pcie_init_irq_domain
> > > > >
> > > > > Why can't we use a size of 4 for all of them?  We only have
> > > > > INTA- INTD.  Are altera and xilinx missing something to apply an
> > > > > offset from the 0-3 space to the 1-4 space?
> > > >
> > > > We have the same discussion before in 2016:
> > > > https://lkml.org/lkml/2016/
> > > > 8/30/198
> > >
> > > Thanks for digging that out.  I knew we'd discussed this before, but
> > > I couldn't find it in the archives.  I don't think anybody was
> > > really satisfied with the outcome, but we accepted it to make
> > > forward progress.
> > >
> > > > This is because legacy interrupt is start with index 1 instead of 0=
.
> > >
> > > I'm not buying this.  Your argument was that "the hwirq for legacy
> > > interrupts will start at 0x1 to 0x4 (INTA to INTD) and these values
> > > are as per PCIe specification for legacy interrupts.  So these
> > > cannot be numbered from 0."
> > >
> > > But all the other drivers I mentioned get along with the 0-3 range
> > > somehow.  If there's something different about altera and xilinx
> > > that means they can't use the same solution the others do, I'd like
> > > to know what it is.
> >
> > Note that with v4 of this patchset[1] I was using hwirq numbers 0-3
> > with
> > pcie- xilinx just fine, however:
> >
> > 1) Bharat complained.
> >
> > 2) It does require that the DT interrupt-map property be set
> > accordingly, which I guess may mean we're stuck with hwirq 1-4 for
> > drivers that already use them.
> >
> > Thanks,
> >     Paul
> >
> > [1] https://patchwork.kernel.org/patch/9763191/
>=20
> I see this series wasn't included in your pull request for v4.13 - is the=
re anything
> you're waiting on?
>=20
> I've produced revisions of the series that work both ways now (0<=3Dhwirq=
<=3D3 in
> v4, 1<=3Dhwirq<=3D4 in v5) so I'm not sure what more I can do.
>=20

Hi Bjorn,

I will test and give ack on paul's final series of patches.
I'm waiting for you to respond on this particular patch.

Regards,
Bharat

^ permalink raw reply	[flat|nested] 27+ messages in thread

* RE: [PATCH v5 1/4] PCI: xilinx: Create legacy IRQ domain with size 5
@ 2017-07-10  5:43               ` Bharat Kumar Gogada
  0 siblings, 0 replies; 27+ messages in thread
From: Bharat Kumar Gogada @ 2017-07-10  5:43 UTC (permalink / raw)
  To: Paul Burton, Bjorn Helgaas
  Cc: Ley Foon Tan, linux-pci, Ravikiran Gummaluri, Bjorn Helgaas,
	Michal Simek, linux-mips, Thomas Gleixner, Ley Foon Tan,
	Marc Zyngier

> -----Original Message-----
> From: linux-pci-owner@vger.kernel.org [mailto:linux-pci-
> owner@vger.kernel.org] On Behalf Of Paul Burton
> Sent: Monday, July 10, 2017 4:30 AM
> To: Bjorn Helgaas <helgaas@kernel.org>
> Cc: Ley Foon Tan <ley.foon.tan@intel.com>; linux-pci@vger.kernel.org; Bharat
> Kumar Gogada <bharatku@xilinx.com>; Ravikiran Gummaluri
> <rgummal@xilinx.com>; Bjorn Helgaas <bhelgaas@google.com>; Michal Simek
> <michal.simek@xilinx.com>; linux-mips@linux-mips.org; Thomas Gleixner
> <tglx@linutronix.de>; Ley Foon Tan <lftan@altera.com>; Marc Zyngier
> <marc.zyngier@arm.com>
> Subject: Re: [PATCH v5 1/4] PCI: xilinx: Create legacy IRQ domain with size 5
> 
> Hi Bjorn,
> 
> On Monday, 19 June 2017 19:07:05 PDT Paul Burton wrote:
> > Hi Bjorn,
> >
> > On Monday, 19 June 2017 18:49:03 PDT Bjorn Helgaas wrote:
> > > [+cc Marc]
> > >
> > > On Tue, Jun 20, 2017 at 08:38:14AM +0800, Ley Foon Tan wrote:
> > > > On Mon, 2017-06-19 at 18:47 -0500, Bjorn Helgaas wrote:
> > > > > [+cc Thomas, Ley Foon]
> > > > >
> > > > > On Sat, Jun 17, 2017 at 12:57:38PM -0700, Paul Burton wrote:
> > > > > > The driver expects to use hardware IRQ numbers 1 through 4 for
> > > > > > INTX interrupts, but only creates an IRQ domain of size 4 (ie.
> > > > > > IRQ numbers 0 through 3). This results in a warning from
> > > > > > irq_domain_associate when it
> > > > > >
> > > > > > is called with hwirq=4:
> > > > > >      WARNING: CPU: 0 PID: 1 at kernel/irq/irqdomain.c:365
> > > > > >
> > > > > >          irq_domain_associate+0x170/0x220
> > > > > >
> > > > > >      error: hwirq 0x4 is too large for dummy
> > > > > >      Modules linked in:
> > > > > >      CPU: 0 PID: 1 Comm: swapper/0 Tainted: G        W
> > > > > >
> > > > > >          4.12.0-rc5-00126-g19e1b3a10aad-dirty #427
> > > > > >
> > > > > >      Stack : 0000000000000000 0000000000000004
> > > > > > 0000000000000006
> > > > > >
> > > > > > ffffffff8092c78a
> > > > > >
> > > > > >              0000000000000061 ffffffff8018bf60
> > > > > > 0000000000000000
> > > > > >
> > > > > > 0000000000000000
> > > > > >
> > > > > >              ffffffff8088c287 ffffffff80811d18
> > > > > > a8000000ffc60000
> > > > > >
> > > > > > ffffffff80926678
> > > > > >
> > > > > >              0000000000000001 0000000000000000
> > > > > > ffffffff80887880
> > > > > >
> > > > > > ffffffff80960000
> > > > > >
> > > > > >              ffffffff80920000 ffffffff801e6744
> > > > > > ffffffff80887880
> > > > > >
> > > > > > a8000000ffc4f8f8
> > > > > >
> > > > > >              000000000000089c ffffffff8018d260
> > > > > > 0000000000010000
> > > > > >
> > > > > > ffffffff80811d18
> > > > > >
> > > > > >              0000000000000000 0000000000000001
> > > > > > 0000000000000000
> > > > > >
> > > > > > 0000000000000000
> > > > > >
> > > > > >              0000000000000000 a8000000ffc4f840
> > > > > > 0000000000000000
> > > > > >
> > > > > > ffffffff8042cf34
> > > > > >
> > > > > >              0000000000000000 0000000000000000
> > > > > > 0000000000000000
> > > > > >
> > > > > > 0000000000040c00
> > > > > >
> > > > > >              0000000000000000 ffffffff8010d1c8
> > > > > > 0000000000000000
> > > > > >
> > > > > > ffffffff8042cf34
> > > > > >
> > > > > >              ...
> > > > > >
> > > > > >      Call Trace:
> > > > > >      [<ffffffff8010d1c8>] show_stack+0x80/0xa0
> > > > > >      [<ffffffff8042cf34>] dump_stack+0xd4/0x110
> > > > > >      [<ffffffff8013ea98>] __warn+0xf0/0x108
> > > > > >      [<ffffffff8013eb14>] warn_slowpath_fmt+0x3c/0x48
> > > > > >      [<ffffffff80196528>] irq_domain_associate+0x170/0x220
> > > > > >      [<ffffffff80196bf0>] irq_create_mapping+0x88/0x118
> > > > > >      [<ffffffff801976a8>] irq_create_fwspec_mapping+0xb8/0x320
> > > > > >      [<ffffffff80197970>] irq_create_of_mapping+0x60/0x70
> > > > > >      [<ffffffff805d1318>] of_irq_parse_and_map_pci+0x20/0x38
> > > > > >      [<ffffffff8049c210>] pci_fixup_irqs+0x60/0xe0
> > > > > >      [<ffffffff8049cd64>] xilinx_pcie_probe+0x28c/0x478
> > > > > >      [<ffffffff804e8ca8>] platform_drv_probe+0x50/0xd0
> > > > > >      [<ffffffff804e73a4>] driver_probe_device+0x2c4/0x3a0
> > > > > >      [<ffffffff804e7544>] __driver_attach+0xc4/0xd0
> > > > > >      [<ffffffff804e5254>] bus_for_each_dev+0x64/0xa8
> > > > > >      [<ffffffff804e5e40>] bus_add_driver+0x1f0/0x268
> > > > > >      [<ffffffff804e8000>] driver_register+0x68/0x118
> > > > > >      [<ffffffff801001a4>] do_one_initcall+0x4c/0x178
> > > > > >      [<ffffffff808d3ca8>] kernel_init_freeable+0x204/0x2b0
> > > > > >      [<ffffffff80730b68>] kernel_init+0x10/0xf8
> > > > > >      [<ffffffff80106218>] ret_from_kernel_thread+0x14/0x1c
> > > > > >
> > > > > > This patch avoids that warning by creating the legacy IRQ
> > > > > > domain with size 5 rather than 4, allowing it to cover the
> > > > > > hwirq=4/INTD case.
> > > > > >
> > > > > > 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 v5:
> > > > > > - New patch; replacing "PCI: xilinx: Fix INTX irq dispatch".
> > > > > >
> > > > > > Changes in v4: None
> > > > > > Changes in v3: None
> > > > > > Changes in v2: None
> > > > > >
> > > > > >  drivers/pci/host/pcie-xilinx.c | 2 +-
> > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/drivers/pci/host/pcie-xilinx.c
> > > > > > b/drivers/pci/host/pcie-xilinx.c index
> > > > > > 2fe2df51f9f8..94c71fb91648 100644
> > > > > > --- a/drivers/pci/host/pcie-xilinx.c
> > > > > > +++ b/drivers/pci/host/pcie-xilinx.c
> > > > > > @@ -524,7 +524,7 @@ static int
> > > > > > xilinx_pcie_init_irq_domain(struct
> > > > > > xilinx_pcie_port *port)
> > > > > >
> > > > > >               return -ENODEV;
> > > > > >
> > > > > >       }
> > > > > >
> > > > > > -     port->leg_domain = irq_domain_add_linear(pcie_intc_node, 4,
> > > > > > +     port->leg_domain = irq_domain_add_linear(pcie_intc_node,
> > > > > > + 1 +
> > > > > > 4,
> > > > >
> > > > > I don't understand this.  Several drivers call
> > > > > irq_domain_add_linear() with
> > > > >
> > > > > a size of 4:
> > > > >   dra7xx_pcie_init_irq_domain
> > > > >   ks_dw_pcie_host_init
> > > > >   advk_pcie_init_irq_domain
> > > > >   faraday_pci_setup_cascaded_irq
> > > > >   rockchip_pcie_init_irq_domain
> > > > >   nwl_pcie_init_irq_domain
> > > > >
> > > > > Only one other in drivers/pci uses a size of 5:
> > > > >   altera_pcie_init_irq_domain
> > > > >
> > > > > Why can't we use a size of 4 for all of them?  We only have
> > > > > INTA- INTD.  Are altera and xilinx missing something to apply an
> > > > > offset from the 0-3 space to the 1-4 space?
> > > >
> > > > We have the same discussion before in 2016:
> > > > https://lkml.org/lkml/2016/
> > > > 8/30/198
> > >
> > > Thanks for digging that out.  I knew we'd discussed this before, but
> > > I couldn't find it in the archives.  I don't think anybody was
> > > really satisfied with the outcome, but we accepted it to make
> > > forward progress.
> > >
> > > > This is because legacy interrupt is start with index 1 instead of 0.
> > >
> > > I'm not buying this.  Your argument was that "the hwirq for legacy
> > > interrupts will start at 0x1 to 0x4 (INTA to INTD) and these values
> > > are as per PCIe specification for legacy interrupts.  So these
> > > cannot be numbered from 0."
> > >
> > > But all the other drivers I mentioned get along with the 0-3 range
> > > somehow.  If there's something different about altera and xilinx
> > > that means they can't use the same solution the others do, I'd like
> > > to know what it is.
> >
> > Note that with v4 of this patchset[1] I was using hwirq numbers 0-3
> > with
> > pcie- xilinx just fine, however:
> >
> > 1) Bharat complained.
> >
> > 2) It does require that the DT interrupt-map property be set
> > accordingly, which I guess may mean we're stuck with hwirq 1-4 for
> > drivers that already use them.
> >
> > Thanks,
> >     Paul
> >
> > [1] https://patchwork.kernel.org/patch/9763191/
> 
> I see this series wasn't included in your pull request for v4.13 - is there anything
> you're waiting on?
> 
> I've produced revisions of the series that work both ways now (0<=hwirq<=3 in
> v4, 1<=hwirq<=4 in v5) so I'm not sure what more I can do.
> 

Hi Bjorn,

I will test and give ack on paul's final series of patches.
I'm waiting for you to respond on this particular patch.

Regards,
Bharat

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 1/4] PCI: xilinx: Create legacy IRQ domain with size 5
  2017-06-20  2:30             ` Bharat Kumar Gogada
  (?)
@ 2017-07-12 22:14             ` Bjorn Helgaas
  -1 siblings, 0 replies; 27+ messages in thread
From: Bjorn Helgaas @ 2017-07-12 22:14 UTC (permalink / raw)
  To: Bharat Kumar Gogada
  Cc: Ley Foon Tan, Paul Burton, linux-pci, Ravikiran Gummaluri,
	Bjorn Helgaas, Michal Simek, linux-mips, Thomas Gleixner,
	Ley Foon Tan, Marc Zyngier

On Tue, Jun 20, 2017 at 02:30:39AM +0000, Bharat Kumar Gogada wrote:
> > Subject: Re: [PATCH v5 1/4] PCI: xilinx: Create legacy IRQ domain with size 5
> > 
> > On Mon, 2017-06-19 at 20:49 -0500, Bjorn Helgaas wrote:
> > > [+cc Marc]
> > >
> > > On Tue, Jun 20, 2017 at 08:38:14AM +0800, Ley Foon Tan wrote:
> > > >
> > > > On Mon, 2017-06-19 at 18:47 -0500, Bjorn Helgaas wrote:
> > > > >
> > > > > [+cc Thomas, Ley Foon]
> > > > >
> > > > > On Sat, Jun 17, 2017 at 12:57:38PM -0700, Paul Burton wrote:
> > > > > >
> > > > > >
> > > > > > The driver expects to use hardware IRQ numbers 1 through 4 for
> > > > > > INTX interrupts, but only creates an IRQ domain of size 4 (ie.
> > > > > > IRQ numbers 0 through 3). This results in a warning from
> > > > > > irq_domain_associate when it is called with hwirq=4:
> > > > > >
> > > > > >      WARNING: CPU: 0 PID: 1 at kernel/irq/irqdomain.c:365
> > > > > >          irq_domain_associate+0x170/0x220
> > > > > >      error: hwirq 0x4 is too large for dummy
> > > > > >      Modules linked in:
> > > > > >      CPU: 0 PID: 1 Comm: swapper/0 Tainted: G        W
> > > > > >          4.12.0-rc5-00126-g19e1b3a10aad-dirty #427
> > > > > >      Stack : 0000000000000000 0000000000000004 0000000000000006
> > > > > > ffffffff8092c78a
> > > > > >              0000000000000061 ffffffff8018bf60 0000000000000000
> > > > > > 0000000000000000
> > > > > >              ffffffff8088c287 ffffffff80811d18 a8000000ffc60000
> > > > > > ffffffff80926678
> > > > > >              0000000000000001 0000000000000000 ffffffff80887880
> > > > > > ffffffff80960000
> > > > > >              ffffffff80920000 ffffffff801e6744 ffffffff80887880
> > > > > > a8000000ffc4f8f8
> > > > > >              000000000000089c ffffffff8018d260 0000000000010000
> > > > > > ffffffff80811d18
> > > > > >              0000000000000000 0000000000000001 0000000000000000
> > > > > > 0000000000000000
> > > > > >              0000000000000000 a8000000ffc4f840 0000000000000000
> > > > > > ffffffff8042cf34
> > > > > >              0000000000000000 0000000000000000 0000000000000000
> > > > > > 0000000000040c00
> > > > > >              0000000000000000 ffffffff8010d1c8 0000000000000000
> > > > > > ffffffff8042cf34
> > > > > >              ...
> > > > > >      Call Trace:
> > > > > >      [<ffffffff8010d1c8>] show_stack+0x80/0xa0
> > > > > >      [<ffffffff8042cf34>] dump_stack+0xd4/0x110
> > > > > >      [<ffffffff8013ea98>] __warn+0xf0/0x108
> > > > > >      [<ffffffff8013eb14>] warn_slowpath_fmt+0x3c/0x48
> > > > > >      [<ffffffff80196528>] irq_domain_associate+0x170/0x220
> > > > > >      [<ffffffff80196bf0>] irq_create_mapping+0x88/0x118
> > > > > >      [<ffffffff801976a8>] irq_create_fwspec_mapping+0xb8/0x320
> > > > > >      [<ffffffff80197970>] irq_create_of_mapping+0x60/0x70
> > > > > >      [<ffffffff805d1318>] of_irq_parse_and_map_pci+0x20/0x38
> > > > > >      [<ffffffff8049c210>] pci_fixup_irqs+0x60/0xe0
> > > > > >      [<ffffffff8049cd64>] xilinx_pcie_probe+0x28c/0x478
> > > > > >      [<ffffffff804e8ca8>] platform_drv_probe+0x50/0xd0
> > > > > >      [<ffffffff804e73a4>] driver_probe_device+0x2c4/0x3a0
> > > > > >      [<ffffffff804e7544>] __driver_attach+0xc4/0xd0
> > > > > >      [<ffffffff804e5254>] bus_for_each_dev+0x64/0xa8
> > > > > >      [<ffffffff804e5e40>] bus_add_driver+0x1f0/0x268
> > > > > >      [<ffffffff804e8000>] driver_register+0x68/0x118
> > > > > >      [<ffffffff801001a4>] do_one_initcall+0x4c/0x178
> > > > > >      [<ffffffff808d3ca8>] kernel_init_freeable+0x204/0x2b0
> > > > > >      [<ffffffff80730b68>] kernel_init+0x10/0xf8
> > > > > >      [<ffffffff80106218>] ret_from_kernel_thread+0x14/0x1c
> > > > > >
> > > > > > This patch avoids that warning by creating the legacy IRQ domain
> > > > > > with size 5 rather than 4, allowing it to cover the hwirq=4/INTD
> > > > > > case.
> > > > > >
> > > > > > 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 v5:
> > > > > > - New patch; replacing "PCI: xilinx: Fix INTX irq dispatch".
> > > > > >
> > > > > > Changes in v4: None
> > > > > > Changes in v3: None
> > > > > > Changes in v2: None
> > > > > >
> > > > > >  drivers/pci/host/pcie-xilinx.c | 2 +-
> > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/drivers/pci/host/pcie-xilinx.c
> > > > > > b/drivers/pci/host/pcie-xilinx.c index
> > > > > > 2fe2df51f9f8..94c71fb91648 100644
> > > > > > --- a/drivers/pci/host/pcie-xilinx.c
> > > > > > +++ b/drivers/pci/host/pcie-xilinx.c
> > > > > > @@ -524,7 +524,7 @@ static int
> > > > > > xilinx_pcie_init_irq_domain(struct
> > > > > > xilinx_pcie_port *port)
> > > > > >               return -ENODEV;
> > > > > >       }
> > > > > >
> > > > > > -     port->leg_domain = irq_domain_add_linear(pcie_intc_node,
> > > > > > 4,
> > > > > > +     port->leg_domain = irq_domain_add_linear(pcie_intc_node,
> > > > > > 1 +
> > > > > > 4,
> > > > > I don't understand this.  Several drivers call
> > > > > irq_domain_add_linear() with
> > > > > a size of 4:
> > > > >
> > > > >   dra7xx_pcie_init_irq_domain
> > > > >   ks_dw_pcie_host_init
> > > > >   advk_pcie_init_irq_domain
> > > > >   faraday_pci_setup_cascaded_irq
> > > > >   rockchip_pcie_init_irq_domain
> > > > >   nwl_pcie_init_irq_domain
> > > > >
> > > > > Only one other in drivers/pci uses a size of 5:
> > > > >
> > > > >   altera_pcie_init_irq_domain
> > > > >
> > > > > Why can't we use a size of 4 for all of them?  We only have INTA-
> > > > > INTD.  Are altera and xilinx missing something to apply an offset
> > > > > from the
> > > > > 0-3
> > > > > space
> > > > > to the 1-4 space?
> > > > We have the same discussion before in 2016: https://lkml.org/lkml/2
> > > > 016/
> > > > 8/30/198
> > > Thanks for digging that out.  I knew we'd discussed this before, but I
> > > couldn't find it in the archives.  I don't think anybody was really
> > > satisfied with the outcome, but we accepted it to make forward
> > > progress.
> > >
> > > >
> > > > This is because legacy interrupt is start with index 1 instead of 0.
> > > I'm not buying this.  Your argument was that "the hwirq for legacy
> > > interrupts will start at 0x1 to 0x4 (INTA to INTD) and these values
> > > are as per PCIe specification for legacy interrupts.  So these cannot
> > > be numbered from 0."
> > >
> > > But all the other drivers I mentioned get along with the 0-3 range
> > > somehow.  If there's something different about altera and xilinx that
> > > means they can't use the same solution the others do, I'd like to know
> > > what it is.
> > I'm not sure those drivers with index 0-3 range tested with 4 legacy interrupts or
> > not. It will not has error until someone requesting 4 legacy interrupts. We see
> > this error when we enabling multi-function endpoint (4 functions). I believe this
> > is not altera or xilinx specific.
> 
> Hi Bjorn,
> 
> Yes as mentioned by Ley Foon it's not Xilinx or Altera specific, and the issue shows 
> up only, when we have multifunction device with 4 functions. 
> As I already mentioned in the above pointed discussion, the issue is subsystem 
> creates  hwirq based on PCI_INTERRUPT_PIN which starts from 0x1, but in 
> IRQ domains hwirq start from 0, due to this difference, issue arises 
> when we use multifunction device.

There are 4 PCI INTx interrupts.  That says to me that ideally the
irq_domain would be of size 4.

I think I see the core code you're referring to:

  of_irq_parse_and_map_pci
    of_irq_parse_pci(&irq_data)
      pci_read_config_byte(pdev, PCI_INTERRUPT_PIN, &pin)
      irq_data->args[0] = pin            # 1 == INTA
    irq_create_of_mapping(&irq_data)
      of_phandle_args_to_fwspec(irq_data, &fwspec)
        fwspec->param[0] = irq_data->args[0]
      irq_create_fwspec_mapping(&fwspec)
        irq_domain_translate(domain, fwspec, &hwirq, ...)
          if (d->ops->xlate)
            return d->ops->xlate(..., fwspec->param, hwirq, ...)
          *hwirq = fwspec->param[0]      # default

The default in irq_domain_translate() is to use fwspec->param[0],
i.e., the value from PCI_INTERRUPT_PIN, as the hwirq value.

The fact that of_irq_parse_pci() sets irq_data->args[0] to the 1-4
range instead of a 0-3 range seems bogus to me.  At that point, we
know there are only 4 valid values, and it seems pointless to waste
the 0 value.  Changing this would affect a fair amount of code (about
25 callers of of_irq_parse_and_map_pci()), but it doesn't seem out of
the realm of possibility to fix them all.

Alternatively, there *is* provision for a translation function in
irq_domain_translate().  Maybe that could be used to translate the 1-4
range from PCI_INTERRUPT_PIN to a 0-3 range?  That's also a change to
every affected driver, so it would be ugly and might be almost as
intrusive as fixing of_irq_parse_pci().

Bjorn

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2017-07-12 22:14 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-17 19:57 [PATCH v5 0/4] PCI: xilinx: Fixes, optimisation & MIPS support Paul Burton
2017-06-17 19:57 ` Paul Burton
2017-06-17 19:57 ` [PATCH v5 1/4] PCI: xilinx: Create legacy IRQ domain with size 5 Paul Burton
2017-06-17 19:57   ` Paul Burton
2017-06-19 23:47   ` Bjorn Helgaas
2017-06-20  0:38     ` Ley Foon Tan
2017-06-20  0:38       ` Ley Foon Tan
2017-06-20  1:49       ` Bjorn Helgaas
2017-06-20  1:55         ` Ley Foon Tan
2017-06-20  1:55           ` Ley Foon Tan
2017-06-20  2:02           ` Ley Foon Tan
2017-06-20  2:02             ` Ley Foon Tan
2017-06-20  2:30           ` Bharat Kumar Gogada
2017-06-20  2:30             ` Bharat Kumar Gogada
2017-07-12 22:14             ` Bjorn Helgaas
2017-06-20  2:07         ` Paul Burton
2017-06-20  2:07           ` Paul Burton
2017-07-09 22:59           ` Paul Burton
2017-07-09 22:59             ` Paul Burton
2017-07-10  5:43             ` Bharat Kumar Gogada
2017-07-10  5:43               ` Bharat Kumar Gogada
2017-06-17 19:57 ` [PATCH v5 2/4] PCI: xilinx: Unify INTx & MSI interrupt decode Paul Burton
2017-06-17 19:57   ` Paul Burton
2017-06-17 19:57 ` [PATCH v5 3/4] PCI: xilinx: Don't enable config completion interrupts Paul Burton
2017-06-17 19:57   ` Paul Burton
2017-06-17 19:57 ` [PATCH v5 4/4] PCI: xilinx: Allow build on MIPS platforms Paul Burton
2017-06-17 19:57   ` Paul Burton

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.