Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v5 0/6] PCI: uniphier: Add features for UniPhier PCIe host controller
@ 2020-06-18  8:38 Kunihiko Hayashi
  2020-06-18  8:38 ` [PATCH v5 1/6] PCI: dwc: Add msi_host_isr() callback Kunihiko Hayashi
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Kunihiko Hayashi @ 2020-06-18  8:38 UTC (permalink / raw)
  To: Bjorn Helgaas, Lorenzo Pieralisi, Jingoo Han, Gustavo Pimentel,
	Rob Herring, Masahiro Yamada, Marc Zyngier
  Cc: devicetree, Kunihiko Hayashi, Masami Hiramatsu, linux-pci,
	linux-kernel, Jassi Brar, linux-arm-kernel

This series adds some features for UniPhier PCIe host controller.

- Add support for PME and AER invoked by MSI interrupt
- Add iATU register view support for PCIe version >= 4.80
- Add an error message when failing to get phy driver

This adds a new function called by MSI handler in DesignWare PCIe framework,
that invokes PME and AER funcions to detect the factor from SoC-dependent
registers.

Changes since v4:
- Add Acked-by: line to dwc patch

Changes since v3:
- Move msi_host_isr() call into dw_handle_msi_irq()
- Move uniphier_pcie_misc_isr() call into the guard of chained_irq
- Use a bool argument is_msi instead of pci_msi_enabled()
- Consolidate handler calls for the same interrupt
- Fix typos in commit messages

Changes since v2:
- Avoid printing phy error message in case of EPROBE_DEFER
- Fix iATU register mapping method
- dt-bindings: Add Acked-by: line
- Fix typos in commit messages
- Use devm_platform_ioremap_resource_byname()

Changes since v1:
- Add check if struct resource is NULL
- Fix warning in the type of dev_err() argument

Kunihiko Hayashi (6):
  PCI: dwc: Add msi_host_isr() callback
  PCI: uniphier: Add misc interrupt handler to invoke PME and AER
  dt-bindings: PCI: uniphier: Add iATU register description
  PCI: uniphier: Add iATU register support
  PCI: uniphier: Add error message when failed to get phy
  PCI: uniphier: Use devm_platform_ioremap_resource_byname()

 .../devicetree/bindings/pci/uniphier-pcie.txt      |  1 +
 drivers/pci/controller/dwc/pcie-designware-host.c  |  3 +
 drivers/pci/controller/dwc/pcie-designware.h       |  1 +
 drivers/pci/controller/dwc/pcie-uniphier.c         | 73 +++++++++++++++++-----
 4 files changed, 63 insertions(+), 15 deletions(-)

-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v5 1/6] PCI: dwc: Add msi_host_isr() callback
  2020-06-18  8:38 [PATCH v5 0/6] PCI: uniphier: Add features for UniPhier PCIe host controller Kunihiko Hayashi
@ 2020-06-18  8:38 ` Kunihiko Hayashi
  2020-06-18  8:38 ` [PATCH v5 2/6] PCI: uniphier: Add misc interrupt handler to invoke PME and AER Kunihiko Hayashi
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Kunihiko Hayashi @ 2020-06-18  8:38 UTC (permalink / raw)
  To: Bjorn Helgaas, Lorenzo Pieralisi, Jingoo Han, Gustavo Pimentel,
	Rob Herring, Masahiro Yamada, Marc Zyngier
  Cc: devicetree, Kunihiko Hayashi, Masami Hiramatsu, linux-pci,
	linux-kernel, Jassi Brar, linux-arm-kernel

This adds msi_host_isr() callback function support to describe
SoC-dependent service triggered by MSI.

For example, when AER interrupt is triggered by MSI, the callback function
reads SoC-dependent registers and detects that the interrupt is from AER,
and invoke AER interrupts related to MSI.

Cc: Marc Zyngier <maz@kernel.org>
Cc: Jingoo Han <jingoohan1@gmail.com>
Cc: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
Acked-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 3 +++
 drivers/pci/controller/dwc/pcie-designware.h      | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 0a4a5aa..026edb1 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -83,6 +83,9 @@ irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
 	u32 status, num_ctrls;
 	irqreturn_t ret = IRQ_NONE;
 
+	if (pp->ops->msi_host_isr)
+		pp->ops->msi_host_isr(pp);
+
 	num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
 
 	for (i = 0; i < num_ctrls; i++) {
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 656e00f..e741967 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -170,6 +170,7 @@ struct dw_pcie_host_ops {
 	void (*scan_bus)(struct pcie_port *pp);
 	void (*set_num_vectors)(struct pcie_port *pp);
 	int (*msi_host_init)(struct pcie_port *pp);
+	void (*msi_host_isr)(struct pcie_port *pp);
 };
 
 struct pcie_port {
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v5 2/6] PCI: uniphier: Add misc interrupt handler to invoke PME and AER
  2020-06-18  8:38 [PATCH v5 0/6] PCI: uniphier: Add features for UniPhier PCIe host controller Kunihiko Hayashi
  2020-06-18  8:38 ` [PATCH v5 1/6] PCI: dwc: Add msi_host_isr() callback Kunihiko Hayashi
@ 2020-06-18  8:38 ` Kunihiko Hayashi
  2020-06-27  9:48   ` Marc Zyngier
  2020-06-18  8:38 ` [PATCH v5 3/6] dt-bindings: PCI: uniphier: Add iATU register description Kunihiko Hayashi
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Kunihiko Hayashi @ 2020-06-18  8:38 UTC (permalink / raw)
  To: Bjorn Helgaas, Lorenzo Pieralisi, Jingoo Han, Gustavo Pimentel,
	Rob Herring, Masahiro Yamada, Marc Zyngier
  Cc: devicetree, Kunihiko Hayashi, Masami Hiramatsu, linux-pci,
	linux-kernel, Jassi Brar, linux-arm-kernel

The misc interrupts consisting of PME, AER, and Link event, is handled
by INTx handler, however, these interrupts should be also handled by
MSI handler.

This adds the function uniphier_pcie_misc_isr() that handles misc
interrupts, which is called from both INTx and MSI handlers.
This function detects PME and AER interrupts with the status register,
and invoke PME and AER drivers related to MSI.

And this sets the mask for misc interrupts from INTx if MSI is enabled
and sets the mask for misc interrupts from MSI if MSI is disabled.

Cc: Marc Zyngier <maz@kernel.org>
Cc: Jingoo Han <jingoohan1@gmail.com>
Cc: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
 drivers/pci/controller/dwc/pcie-uniphier.c | 57 ++++++++++++++++++++++++------
 1 file changed, 46 insertions(+), 11 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-uniphier.c b/drivers/pci/controller/dwc/pcie-uniphier.c
index a5401a0..5ce2479 100644
--- a/drivers/pci/controller/dwc/pcie-uniphier.c
+++ b/drivers/pci/controller/dwc/pcie-uniphier.c
@@ -44,7 +44,9 @@
 #define PCL_SYS_AUX_PWR_DET		BIT(8)
 
 #define PCL_RCV_INT			0x8108
+#define PCL_RCV_INT_ALL_INT_MASK	GENMASK(28, 25)
 #define PCL_RCV_INT_ALL_ENABLE		GENMASK(20, 17)
+#define PCL_RCV_INT_ALL_MSI_MASK	GENMASK(12, 9)
 #define PCL_CFG_BW_MGT_STATUS		BIT(4)
 #define PCL_CFG_LINK_AUTO_BW_STATUS	BIT(3)
 #define PCL_CFG_AER_RC_ERR_MSI_STATUS	BIT(2)
@@ -167,7 +169,15 @@ static void uniphier_pcie_stop_link(struct dw_pcie *pci)
 
 static void uniphier_pcie_irq_enable(struct uniphier_pcie_priv *priv)
 {
-	writel(PCL_RCV_INT_ALL_ENABLE, priv->base + PCL_RCV_INT);
+	u32 val;
+
+	val = PCL_RCV_INT_ALL_ENABLE;
+	if (pci_msi_enabled())
+		val |= PCL_RCV_INT_ALL_INT_MASK;
+	else
+		val |= PCL_RCV_INT_ALL_MSI_MASK;
+
+	writel(val, priv->base + PCL_RCV_INT);
 	writel(PCL_RCV_INTX_ALL_ENABLE, priv->base + PCL_RCV_INTX);
 }
 
@@ -231,32 +241,56 @@ static const struct irq_domain_ops uniphier_intx_domain_ops = {
 	.map = uniphier_pcie_intx_map,
 };
 
-static void uniphier_pcie_irq_handler(struct irq_desc *desc)
+static void uniphier_pcie_misc_isr(struct pcie_port *pp, bool is_msi)
 {
-	struct pcie_port *pp = irq_desc_get_handler_data(desc);
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
 	struct uniphier_pcie_priv *priv = to_uniphier_pcie(pci);
-	struct irq_chip *chip = irq_desc_get_chip(desc);
-	unsigned long reg;
-	u32 val, bit, virq;
+	u32 val, virq;
 
-	/* INT for debug */
 	val = readl(priv->base + PCL_RCV_INT);
 
 	if (val & PCL_CFG_BW_MGT_STATUS)
 		dev_dbg(pci->dev, "Link Bandwidth Management Event\n");
+
 	if (val & PCL_CFG_LINK_AUTO_BW_STATUS)
 		dev_dbg(pci->dev, "Link Autonomous Bandwidth Event\n");
-	if (val & PCL_CFG_AER_RC_ERR_MSI_STATUS)
-		dev_dbg(pci->dev, "Root Error\n");
-	if (val & PCL_CFG_PME_MSI_STATUS)
-		dev_dbg(pci->dev, "PME Interrupt\n");
+
+	if (is_msi) {
+		if (val & PCL_CFG_AER_RC_ERR_MSI_STATUS)
+			dev_dbg(pci->dev, "Root Error Status\n");
+
+		if (val & PCL_CFG_PME_MSI_STATUS)
+			dev_dbg(pci->dev, "PME Interrupt\n");
+
+		if (val & (PCL_CFG_AER_RC_ERR_MSI_STATUS |
+			   PCL_CFG_PME_MSI_STATUS)) {
+			virq = irq_linear_revmap(pp->irq_domain, 0);
+			generic_handle_irq(virq);
+		}
+	}
 
 	writel(val, priv->base + PCL_RCV_INT);
+}
+
+static void uniphier_pcie_msi_host_isr(struct pcie_port *pp)
+{
+	uniphier_pcie_misc_isr(pp, true);
+}
+
+static void uniphier_pcie_irq_handler(struct irq_desc *desc)
+{
+	struct pcie_port *pp = irq_desc_get_handler_data(desc);
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct uniphier_pcie_priv *priv = to_uniphier_pcie(pci);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	unsigned long reg;
+	u32 val, bit, virq;
 
 	/* INTx */
 	chained_irq_enter(chip, desc);
 
+	uniphier_pcie_misc_isr(pp, false);
+
 	val = readl(priv->base + PCL_RCV_INTX);
 	reg = FIELD_GET(PCL_RCV_INTX_ALL_STATUS, val);
 
@@ -330,6 +364,7 @@ static int uniphier_pcie_host_init(struct pcie_port *pp)
 
 static const struct dw_pcie_host_ops uniphier_pcie_host_ops = {
 	.host_init = uniphier_pcie_host_init,
+	.msi_host_isr = uniphier_pcie_msi_host_isr,
 };
 
 static int uniphier_add_pcie_port(struct uniphier_pcie_priv *priv,
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v5 3/6] dt-bindings: PCI: uniphier: Add iATU register description
  2020-06-18  8:38 [PATCH v5 0/6] PCI: uniphier: Add features for UniPhier PCIe host controller Kunihiko Hayashi
  2020-06-18  8:38 ` [PATCH v5 1/6] PCI: dwc: Add msi_host_isr() callback Kunihiko Hayashi
  2020-06-18  8:38 ` [PATCH v5 2/6] PCI: uniphier: Add misc interrupt handler to invoke PME and AER Kunihiko Hayashi
@ 2020-06-18  8:38 ` Kunihiko Hayashi
  2020-06-18  8:38 ` [PATCH v5 4/6] PCI: uniphier: Add iATU register support Kunihiko Hayashi
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Kunihiko Hayashi @ 2020-06-18  8:38 UTC (permalink / raw)
  To: Bjorn Helgaas, Lorenzo Pieralisi, Jingoo Han, Gustavo Pimentel,
	Rob Herring, Masahiro Yamada, Marc Zyngier
  Cc: devicetree, Kunihiko Hayashi, Masami Hiramatsu, linux-pci,
	linux-kernel, Jassi Brar, linux-arm-kernel

In the dt-bindings, "atu" reg-names is required to get the register space
for iATU in Synopsys DWC version 4.80 or later.

Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/pci/uniphier-pcie.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/pci/uniphier-pcie.txt b/Documentation/devicetree/bindings/pci/uniphier-pcie.txt
index 1fa2c59..c4b7381 100644
--- a/Documentation/devicetree/bindings/pci/uniphier-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/uniphier-pcie.txt
@@ -16,6 +16,7 @@ Required properties:
     "dbi"    - controller configuration registers
     "link"   - SoC-specific glue layer registers
     "config" - PCIe configuration space
+    "atu"    - iATU registers for DWC version 4.80 or later
 - clocks: A phandle to the clock gate for PCIe glue layer including
 	the host controller.
 - resets: A phandle to the reset line for PCIe glue layer including
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v5 4/6] PCI: uniphier: Add iATU register support
  2020-06-18  8:38 [PATCH v5 0/6] PCI: uniphier: Add features for UniPhier PCIe host controller Kunihiko Hayashi
                   ` (2 preceding siblings ...)
  2020-06-18  8:38 ` [PATCH v5 3/6] dt-bindings: PCI: uniphier: Add iATU register description Kunihiko Hayashi
@ 2020-06-18  8:38 ` Kunihiko Hayashi
  2020-06-18  8:38 ` [PATCH v5 5/6] PCI: uniphier: Add error message when failed to get phy Kunihiko Hayashi
  2020-06-18  8:38 ` [PATCH v5 6/6] PCI: uniphier: Use devm_platform_ioremap_resource_byname() Kunihiko Hayashi
  5 siblings, 0 replies; 11+ messages in thread
From: Kunihiko Hayashi @ 2020-06-18  8:38 UTC (permalink / raw)
  To: Bjorn Helgaas, Lorenzo Pieralisi, Jingoo Han, Gustavo Pimentel,
	Rob Herring, Masahiro Yamada, Marc Zyngier
  Cc: devicetree, Kunihiko Hayashi, Masami Hiramatsu, linux-pci,
	linux-kernel, Jassi Brar, linux-arm-kernel

This gets iATU register area from reg property. In Synopsys DWC version
4.80 or later, since iATU register area is separated from core register
area, this area is necessary to get from DT independently.

Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
 drivers/pci/controller/dwc/pcie-uniphier.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-uniphier.c b/drivers/pci/controller/dwc/pcie-uniphier.c
index 5ce2479..c37a968 100644
--- a/drivers/pci/controller/dwc/pcie-uniphier.c
+++ b/drivers/pci/controller/dwc/pcie-uniphier.c
@@ -451,6 +451,11 @@ static int uniphier_pcie_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->pci.dbi_base))
 		return PTR_ERR(priv->pci.dbi_base);
 
+	priv->pci.atu_base =
+		devm_platform_ioremap_resource_byname(pdev, "atu");
+	if (IS_ERR(priv->pci.atu_base))
+		priv->pci.atu_base = NULL;
+
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "link");
 	priv->base = devm_ioremap_resource(dev, res);
 	if (IS_ERR(priv->base))
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v5 5/6] PCI: uniphier: Add error message when failed to get phy
  2020-06-18  8:38 [PATCH v5 0/6] PCI: uniphier: Add features for UniPhier PCIe host controller Kunihiko Hayashi
                   ` (3 preceding siblings ...)
  2020-06-18  8:38 ` [PATCH v5 4/6] PCI: uniphier: Add iATU register support Kunihiko Hayashi
@ 2020-06-18  8:38 ` Kunihiko Hayashi
  2020-06-18  8:38 ` [PATCH v5 6/6] PCI: uniphier: Use devm_platform_ioremap_resource_byname() Kunihiko Hayashi
  5 siblings, 0 replies; 11+ messages in thread
From: Kunihiko Hayashi @ 2020-06-18  8:38 UTC (permalink / raw)
  To: Bjorn Helgaas, Lorenzo Pieralisi, Jingoo Han, Gustavo Pimentel,
	Rob Herring, Masahiro Yamada, Marc Zyngier
  Cc: devicetree, Kunihiko Hayashi, Masami Hiramatsu, linux-pci,
	linux-kernel, Jassi Brar, linux-arm-kernel

Even if phy driver doesn't probe, the error message can't be distinguished
from other errors. This displays error message caused by the phy driver
explicitly.

Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
 drivers/pci/controller/dwc/pcie-uniphier.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-uniphier.c b/drivers/pci/controller/dwc/pcie-uniphier.c
index c37a968..8356dd3 100644
--- a/drivers/pci/controller/dwc/pcie-uniphier.c
+++ b/drivers/pci/controller/dwc/pcie-uniphier.c
@@ -470,8 +470,12 @@ static int uniphier_pcie_probe(struct platform_device *pdev)
 		return PTR_ERR(priv->rst);
 
 	priv->phy = devm_phy_optional_get(dev, "pcie-phy");
-	if (IS_ERR(priv->phy))
-		return PTR_ERR(priv->phy);
+	if (IS_ERR(priv->phy)) {
+		ret = PTR_ERR(priv->phy);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "Failed to get phy (%d)\n", ret);
+		return ret;
+	}
 
 	platform_set_drvdata(pdev, priv);
 
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v5 6/6] PCI: uniphier: Use devm_platform_ioremap_resource_byname()
  2020-06-18  8:38 [PATCH v5 0/6] PCI: uniphier: Add features for UniPhier PCIe host controller Kunihiko Hayashi
                   ` (4 preceding siblings ...)
  2020-06-18  8:38 ` [PATCH v5 5/6] PCI: uniphier: Add error message when failed to get phy Kunihiko Hayashi
@ 2020-06-18  8:38 ` Kunihiko Hayashi
  5 siblings, 0 replies; 11+ messages in thread
From: Kunihiko Hayashi @ 2020-06-18  8:38 UTC (permalink / raw)
  To: Bjorn Helgaas, Lorenzo Pieralisi, Jingoo Han, Gustavo Pimentel,
	Rob Herring, Masahiro Yamada, Marc Zyngier
  Cc: devicetree, Kunihiko Hayashi, Masami Hiramatsu, linux-pci,
	linux-kernel, Jassi Brar, linux-arm-kernel

Use devm_platform_ioremap_resource_byname() to simplify the code a bit.

Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
 drivers/pci/controller/dwc/pcie-uniphier.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-uniphier.c b/drivers/pci/controller/dwc/pcie-uniphier.c
index 8356dd3..233d624 100644
--- a/drivers/pci/controller/dwc/pcie-uniphier.c
+++ b/drivers/pci/controller/dwc/pcie-uniphier.c
@@ -456,8 +456,7 @@ static int uniphier_pcie_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->pci.atu_base))
 		priv->pci.atu_base = NULL;
 
-	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "link");
-	priv->base = devm_ioremap_resource(dev, res);
+	priv->base = devm_platform_ioremap_resource_byname(pdev, "link");
 	if (IS_ERR(priv->base))
 		return PTR_ERR(priv->base);
 
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v5 2/6] PCI: uniphier: Add misc interrupt handler to invoke PME and AER
  2020-06-18  8:38 ` [PATCH v5 2/6] PCI: uniphier: Add misc interrupt handler to invoke PME and AER Kunihiko Hayashi
@ 2020-06-27  9:48   ` Marc Zyngier
  2020-06-29  9:49     ` Kunihiko Hayashi
  0 siblings, 1 reply; 11+ messages in thread
From: Marc Zyngier @ 2020-06-27  9:48 UTC (permalink / raw)
  To: Kunihiko Hayashi
  Cc: devicetree, Lorenzo Pieralisi, Masami Hiramatsu, Jassi Brar,
	Jingoo Han, linux-pci, linux-kernel, Masahiro Yamada,
	Rob Herring, Gustavo Pimentel, Bjorn Helgaas, linux-arm-kernel

On Thu, 18 Jun 2020 09:38:09 +0100,
Kunihiko Hayashi <hayashi.kunihiko@socionext.com> wrote:
> 
> The misc interrupts consisting of PME, AER, and Link event, is handled
> by INTx handler, however, these interrupts should be also handled by
> MSI handler.
> 
> This adds the function uniphier_pcie_misc_isr() that handles misc
> interrupts, which is called from both INTx and MSI handlers.
> This function detects PME and AER interrupts with the status register,
> and invoke PME and AER drivers related to MSI.
> 
> And this sets the mask for misc interrupts from INTx if MSI is enabled
> and sets the mask for misc interrupts from MSI if MSI is disabled.
> 
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Jingoo Han <jingoohan1@gmail.com>
> Cc: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> ---
>  drivers/pci/controller/dwc/pcie-uniphier.c | 57 ++++++++++++++++++++++++------
>  1 file changed, 46 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-uniphier.c b/drivers/pci/controller/dwc/pcie-uniphier.c
> index a5401a0..5ce2479 100644
> --- a/drivers/pci/controller/dwc/pcie-uniphier.c
> +++ b/drivers/pci/controller/dwc/pcie-uniphier.c
> @@ -44,7 +44,9 @@
>  #define PCL_SYS_AUX_PWR_DET		BIT(8)
>  
>  #define PCL_RCV_INT			0x8108
> +#define PCL_RCV_INT_ALL_INT_MASK	GENMASK(28, 25)
>  #define PCL_RCV_INT_ALL_ENABLE		GENMASK(20, 17)
> +#define PCL_RCV_INT_ALL_MSI_MASK	GENMASK(12, 9)
>  #define PCL_CFG_BW_MGT_STATUS		BIT(4)
>  #define PCL_CFG_LINK_AUTO_BW_STATUS	BIT(3)
>  #define PCL_CFG_AER_RC_ERR_MSI_STATUS	BIT(2)
> @@ -167,7 +169,15 @@ static void uniphier_pcie_stop_link(struct dw_pcie *pci)
>  
>  static void uniphier_pcie_irq_enable(struct uniphier_pcie_priv *priv)
>  {
> -	writel(PCL_RCV_INT_ALL_ENABLE, priv->base + PCL_RCV_INT);
> +	u32 val;
> +
> +	val = PCL_RCV_INT_ALL_ENABLE;
> +	if (pci_msi_enabled())
> +		val |= PCL_RCV_INT_ALL_INT_MASK;
> +	else
> +		val |= PCL_RCV_INT_ALL_MSI_MASK;

Does this affect endpoints? Or just the RC itself?

> +
> +	writel(val, priv->base + PCL_RCV_INT);
>  	writel(PCL_RCV_INTX_ALL_ENABLE, priv->base + PCL_RCV_INTX);
>  }
>  
> @@ -231,32 +241,56 @@ static const struct irq_domain_ops uniphier_intx_domain_ops = {
>  	.map = uniphier_pcie_intx_map,
>  };
>  
> -static void uniphier_pcie_irq_handler(struct irq_desc *desc)
> +static void uniphier_pcie_misc_isr(struct pcie_port *pp, bool is_msi)
>  {
> -	struct pcie_port *pp = irq_desc_get_handler_data(desc);
>  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>  	struct uniphier_pcie_priv *priv = to_uniphier_pcie(pci);
> -	struct irq_chip *chip = irq_desc_get_chip(desc);
> -	unsigned long reg;
> -	u32 val, bit, virq;
> +	u32 val, virq;
>  
> -	/* INT for debug */
>  	val = readl(priv->base + PCL_RCV_INT);
>  
>  	if (val & PCL_CFG_BW_MGT_STATUS)
>  		dev_dbg(pci->dev, "Link Bandwidth Management Event\n");
> +
>  	if (val & PCL_CFG_LINK_AUTO_BW_STATUS)
>  		dev_dbg(pci->dev, "Link Autonomous Bandwidth Event\n");
> -	if (val & PCL_CFG_AER_RC_ERR_MSI_STATUS)
> -		dev_dbg(pci->dev, "Root Error\n");
> -	if (val & PCL_CFG_PME_MSI_STATUS)
> -		dev_dbg(pci->dev, "PME Interrupt\n");
> +
> +	if (is_msi) {
> +		if (val & PCL_CFG_AER_RC_ERR_MSI_STATUS)
> +			dev_dbg(pci->dev, "Root Error Status\n");
> +
> +		if (val & PCL_CFG_PME_MSI_STATUS)
> +			dev_dbg(pci->dev, "PME Interrupt\n");
> +
> +		if (val & (PCL_CFG_AER_RC_ERR_MSI_STATUS |
> +			   PCL_CFG_PME_MSI_STATUS)) {
> +			virq = irq_linear_revmap(pp->irq_domain, 0);
> +			generic_handle_irq(virq);
> +		}
> +	}

Please have two handlers: one for interrupts that are from the RC,
another for interrupts coming from the endpoints.

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v5 2/6] PCI: uniphier: Add misc interrupt handler to invoke PME and AER
  2020-06-27  9:48   ` Marc Zyngier
@ 2020-06-29  9:49     ` Kunihiko Hayashi
  2020-06-30 13:23       ` Marc Zyngier
  0 siblings, 1 reply; 11+ messages in thread
From: Kunihiko Hayashi @ 2020-06-29  9:49 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: devicetree, Lorenzo Pieralisi, Masami Hiramatsu, Jassi Brar,
	Jingoo Han, linux-pci, linux-kernel, Masahiro Yamada,
	Rob Herring, Gustavo Pimentel, Bjorn Helgaas, linux-arm-kernel

Hi Marc,

On 2020/06/27 18:48, Marc Zyngier wrote:
> On Thu, 18 Jun 2020 09:38:09 +0100,
> Kunihiko Hayashi <hayashi.kunihiko@socionext.com> wrote:
>>
>> The misc interrupts consisting of PME, AER, and Link event, is handled
>> by INTx handler, however, these interrupts should be also handled by
>> MSI handler.
>>
>> This adds the function uniphier_pcie_misc_isr() that handles misc
>> interrupts, which is called from both INTx and MSI handlers.
>> This function detects PME and AER interrupts with the status register,
>> and invoke PME and AER drivers related to MSI.
>>
>> And this sets the mask for misc interrupts from INTx if MSI is enabled
>> and sets the mask for misc interrupts from MSI if MSI is disabled.
>>
>> Cc: Marc Zyngier <maz@kernel.org>
>> Cc: Jingoo Han <jingoohan1@gmail.com>
>> Cc: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
>> ---
>>   drivers/pci/controller/dwc/pcie-uniphier.c | 57 ++++++++++++++++++++++++------
>>   1 file changed, 46 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-uniphier.c b/drivers/pci/controller/dwc/pcie-uniphier.c
>> index a5401a0..5ce2479 100644
>> --- a/drivers/pci/controller/dwc/pcie-uniphier.c
>> +++ b/drivers/pci/controller/dwc/pcie-uniphier.c
>> @@ -44,7 +44,9 @@
>>   #define PCL_SYS_AUX_PWR_DET		BIT(8)
>>   
>>   #define PCL_RCV_INT			0x8108
>> +#define PCL_RCV_INT_ALL_INT_MASK	GENMASK(28, 25)
>>   #define PCL_RCV_INT_ALL_ENABLE		GENMASK(20, 17)
>> +#define PCL_RCV_INT_ALL_MSI_MASK	GENMASK(12, 9)
>>   #define PCL_CFG_BW_MGT_STATUS		BIT(4)
>>   #define PCL_CFG_LINK_AUTO_BW_STATUS	BIT(3)
>>   #define PCL_CFG_AER_RC_ERR_MSI_STATUS	BIT(2)
>> @@ -167,7 +169,15 @@ static void uniphier_pcie_stop_link(struct dw_pcie *pci)
>>   
>>   static void uniphier_pcie_irq_enable(struct uniphier_pcie_priv *priv)
>>   {
>> -	writel(PCL_RCV_INT_ALL_ENABLE, priv->base + PCL_RCV_INT);
>> +	u32 val;
>> +
>> +	val = PCL_RCV_INT_ALL_ENABLE;
>> +	if (pci_msi_enabled())
>> +		val |= PCL_RCV_INT_ALL_INT_MASK;
>> +	else
>> +		val |= PCL_RCV_INT_ALL_MSI_MASK;
> 
> Does this affect endpoints? Or just the RC itself?

These interrupts are asserted by RC itself, so this part affects only RC.

>> +
>> +	writel(val, priv->base + PCL_RCV_INT);
>>   	writel(PCL_RCV_INTX_ALL_ENABLE, priv->base + PCL_RCV_INTX);
>>   }
>>   
>> @@ -231,32 +241,56 @@ static const struct irq_domain_ops uniphier_intx_domain_ops = {
>>   	.map = uniphier_pcie_intx_map,
>>   };
>>   
>> -static void uniphier_pcie_irq_handler(struct irq_desc *desc)
>> +static void uniphier_pcie_misc_isr(struct pcie_port *pp, bool is_msi)
>>   {
>> -	struct pcie_port *pp = irq_desc_get_handler_data(desc);
>>   	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>>   	struct uniphier_pcie_priv *priv = to_uniphier_pcie(pci);
>> -	struct irq_chip *chip = irq_desc_get_chip(desc);
>> -	unsigned long reg;
>> -	u32 val, bit, virq;
>> +	u32 val, virq;
>>   
>> -	/* INT for debug */
>>   	val = readl(priv->base + PCL_RCV_INT);
>>   
>>   	if (val & PCL_CFG_BW_MGT_STATUS)
>>   		dev_dbg(pci->dev, "Link Bandwidth Management Event\n");
>> +
>>   	if (val & PCL_CFG_LINK_AUTO_BW_STATUS)
>>   		dev_dbg(pci->dev, "Link Autonomous Bandwidth Event\n");
>> -	if (val & PCL_CFG_AER_RC_ERR_MSI_STATUS)
>> -		dev_dbg(pci->dev, "Root Error\n");
>> -	if (val & PCL_CFG_PME_MSI_STATUS)
>> -		dev_dbg(pci->dev, "PME Interrupt\n");
>> +
>> +	if (is_msi) {
>> +		if (val & PCL_CFG_AER_RC_ERR_MSI_STATUS)
>> +			dev_dbg(pci->dev, "Root Error Status\n");
>> +
>> +		if (val & PCL_CFG_PME_MSI_STATUS)
>> +			dev_dbg(pci->dev, "PME Interrupt\n");
>> +
>> +		if (val & (PCL_CFG_AER_RC_ERR_MSI_STATUS |
>> +			   PCL_CFG_PME_MSI_STATUS)) {
>> +			virq = irq_linear_revmap(pp->irq_domain, 0);
>> +			generic_handle_irq(virq);
>> +		}
>> +	}
> 
> Please have two handlers: one for interrupts that are from the RC,
> another for interrupts coming from the endpoints.
I assume that this handler treats interrupts from the RC only and
this is set on the member ".msi_host_isr" added in the patch 1/6.
I think that the handler for interrupts coming from endpoints should be
treated as a normal case (after calling .msi_host_isr in
dw_handle_msi_irq()).

Thank you,

---
Best Regards
Kunihiko Hayashi

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v5 2/6] PCI: uniphier: Add misc interrupt handler to invoke PME and AER
  2020-06-29  9:49     ` Kunihiko Hayashi
@ 2020-06-30 13:23       ` Marc Zyngier
  2020-07-01  2:18         ` Kunihiko Hayashi
  0 siblings, 1 reply; 11+ messages in thread
From: Marc Zyngier @ 2020-06-30 13:23 UTC (permalink / raw)
  To: Kunihiko Hayashi
  Cc: devicetree, Lorenzo Pieralisi, Masami Hiramatsu, Jassi Brar,
	Jingoo Han, linux-pci, linux-kernel, Masahiro Yamada,
	Rob Herring, Gustavo Pimentel, Bjorn Helgaas, linux-arm-kernel

On 2020-06-29 10:49, Kunihiko Hayashi wrote:
> Hi Marc,
> 
> On 2020/06/27 18:48, Marc Zyngier wrote:
>> On Thu, 18 Jun 2020 09:38:09 +0100,
>> Kunihiko Hayashi <hayashi.kunihiko@socionext.com> wrote:
>>> 
>>> The misc interrupts consisting of PME, AER, and Link event, is 
>>> handled
>>> by INTx handler, however, these interrupts should be also handled by
>>> MSI handler.
>>> 
>>> This adds the function uniphier_pcie_misc_isr() that handles misc
>>> interrupts, which is called from both INTx and MSI handlers.
>>> This function detects PME and AER interrupts with the status 
>>> register,
>>> and invoke PME and AER drivers related to MSI.
>>> 
>>> And this sets the mask for misc interrupts from INTx if MSI is 
>>> enabled
>>> and sets the mask for misc interrupts from MSI if MSI is disabled.
>>> 
>>> Cc: Marc Zyngier <maz@kernel.org>
>>> Cc: Jingoo Han <jingoohan1@gmail.com>
>>> Cc: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>>> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
>>> ---
>>>   drivers/pci/controller/dwc/pcie-uniphier.c | 57 
>>> ++++++++++++++++++++++++------
>>>   1 file changed, 46 insertions(+), 11 deletions(-)
>>> 
>>> diff --git a/drivers/pci/controller/dwc/pcie-uniphier.c 
>>> b/drivers/pci/controller/dwc/pcie-uniphier.c
>>> index a5401a0..5ce2479 100644
>>> --- a/drivers/pci/controller/dwc/pcie-uniphier.c
>>> +++ b/drivers/pci/controller/dwc/pcie-uniphier.c
>>> @@ -44,7 +44,9 @@
>>>   #define PCL_SYS_AUX_PWR_DET		BIT(8)
>>>     #define PCL_RCV_INT			0x8108
>>> +#define PCL_RCV_INT_ALL_INT_MASK	GENMASK(28, 25)
>>>   #define PCL_RCV_INT_ALL_ENABLE		GENMASK(20, 17)
>>> +#define PCL_RCV_INT_ALL_MSI_MASK	GENMASK(12, 9)
>>>   #define PCL_CFG_BW_MGT_STATUS		BIT(4)
>>>   #define PCL_CFG_LINK_AUTO_BW_STATUS	BIT(3)
>>>   #define PCL_CFG_AER_RC_ERR_MSI_STATUS	BIT(2)
>>> @@ -167,7 +169,15 @@ static void uniphier_pcie_stop_link(struct 
>>> dw_pcie *pci)
>>>     static void uniphier_pcie_irq_enable(struct uniphier_pcie_priv 
>>> *priv)
>>>   {
>>> -	writel(PCL_RCV_INT_ALL_ENABLE, priv->base + PCL_RCV_INT);
>>> +	u32 val;
>>> +
>>> +	val = PCL_RCV_INT_ALL_ENABLE;
>>> +	if (pci_msi_enabled())
>>> +		val |= PCL_RCV_INT_ALL_INT_MASK;
>>> +	else
>>> +		val |= PCL_RCV_INT_ALL_MSI_MASK;
>> 
>> Does this affect endpoints? Or just the RC itself?
> 
> These interrupts are asserted by RC itself, so this part affects only 
> RC.
> 
>>> +
>>> +	writel(val, priv->base + PCL_RCV_INT);
>>>   	writel(PCL_RCV_INTX_ALL_ENABLE, priv->base + PCL_RCV_INTX);
>>>   }
>>>   @@ -231,32 +241,56 @@ static const struct irq_domain_ops 
>>> uniphier_intx_domain_ops = {
>>>   	.map = uniphier_pcie_intx_map,
>>>   };
>>>   -static void uniphier_pcie_irq_handler(struct irq_desc *desc)
>>> +static void uniphier_pcie_misc_isr(struct pcie_port *pp, bool 
>>> is_msi)
>>>   {
>>> -	struct pcie_port *pp = irq_desc_get_handler_data(desc);
>>>   	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>>>   	struct uniphier_pcie_priv *priv = to_uniphier_pcie(pci);
>>> -	struct irq_chip *chip = irq_desc_get_chip(desc);
>>> -	unsigned long reg;
>>> -	u32 val, bit, virq;
>>> +	u32 val, virq;
>>>   -	/* INT for debug */
>>>   	val = readl(priv->base + PCL_RCV_INT);
>>>     	if (val & PCL_CFG_BW_MGT_STATUS)
>>>   		dev_dbg(pci->dev, "Link Bandwidth Management Event\n");
>>> +
>>>   	if (val & PCL_CFG_LINK_AUTO_BW_STATUS)
>>>   		dev_dbg(pci->dev, "Link Autonomous Bandwidth Event\n");
>>> -	if (val & PCL_CFG_AER_RC_ERR_MSI_STATUS)
>>> -		dev_dbg(pci->dev, "Root Error\n");
>>> -	if (val & PCL_CFG_PME_MSI_STATUS)
>>> -		dev_dbg(pci->dev, "PME Interrupt\n");
>>> +
>>> +	if (is_msi) {
>>> +		if (val & PCL_CFG_AER_RC_ERR_MSI_STATUS)
>>> +			dev_dbg(pci->dev, "Root Error Status\n");
>>> +
>>> +		if (val & PCL_CFG_PME_MSI_STATUS)
>>> +			dev_dbg(pci->dev, "PME Interrupt\n");
>>> +
>>> +		if (val & (PCL_CFG_AER_RC_ERR_MSI_STATUS |
>>> +			   PCL_CFG_PME_MSI_STATUS)) {
>>> +			virq = irq_linear_revmap(pp->irq_domain, 0);
>>> +			generic_handle_irq(virq);
>>> +		}
>>> +	}
>> 
>> Please have two handlers: one for interrupts that are from the RC,
>> another for interrupts coming from the endpoints.
> I assume that this handler treats interrupts from the RC only and
> this is set on the member ".msi_host_isr" added in the patch 1/6.
> I think that the handler for interrupts coming from endpoints should be
> treated as a normal case (after calling .msi_host_isr in
> dw_handle_msi_irq()).

It looks pretty odd that you end-up dealing with both from the
same "parent" interrupt. I guess this is in keeping with the
rest of the DW PCIe hacks... :-/

It is for Lorenzo to make up his mind about this anyway.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v5 2/6] PCI: uniphier: Add misc interrupt handler to invoke PME and AER
  2020-06-30 13:23       ` Marc Zyngier
@ 2020-07-01  2:18         ` Kunihiko Hayashi
  0 siblings, 0 replies; 11+ messages in thread
From: Kunihiko Hayashi @ 2020-07-01  2:18 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: devicetree, Lorenzo Pieralisi, Masami Hiramatsu, Jassi Brar,
	Jingoo Han, linux-pci, linux-kernel, Masahiro Yamada,
	Rob Herring, Gustavo Pimentel, Bjorn Helgaas, linux-arm-kernel

Hi Marc,

On 2020/06/30 22:23, Marc Zyngier wrote:
> On 2020-06-29 10:49, Kunihiko Hayashi wrote:
>> Hi Marc,
>>
>> On 2020/06/27 18:48, Marc Zyngier wrote:
>>> On Thu, 18 Jun 2020 09:38:09 +0100,
>>> Kunihiko Hayashi <hayashi.kunihiko@socionext.com> wrote:
>>>>
>>>> The misc interrupts consisting of PME, AER, and Link event, is handled
>>>> by INTx handler, however, these interrupts should be also handled by
>>>> MSI handler.
>>>>
>>>> This adds the function uniphier_pcie_misc_isr() that handles misc
>>>> interrupts, which is called from both INTx and MSI handlers.
>>>> This function detects PME and AER interrupts with the status register,
>>>> and invoke PME and AER drivers related to MSI.
>>>>
>>>> And this sets the mask for misc interrupts from INTx if MSI is enabled
>>>> and sets the mask for misc interrupts from MSI if MSI is disabled.
>>>>
>>>> Cc: Marc Zyngier <maz@kernel.org>
>>>> Cc: Jingoo Han <jingoohan1@gmail.com>
>>>> Cc: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>>>> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
>>>> ---
>>>>   drivers/pci/controller/dwc/pcie-uniphier.c | 57 ++++++++++++++++++++++++------
>>>>   1 file changed, 46 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/drivers/pci/controller/dwc/pcie-uniphier.c b/drivers/pci/controller/dwc/pcie-uniphier.c
>>>> index a5401a0..5ce2479 100644
>>>> --- a/drivers/pci/controller/dwc/pcie-uniphier.c
>>>> +++ b/drivers/pci/controller/dwc/pcie-uniphier.c
>>>> @@ -44,7 +44,9 @@
>>>>   #define PCL_SYS_AUX_PWR_DET        BIT(8)
>>>>     #define PCL_RCV_INT            0x8108
>>>> +#define PCL_RCV_INT_ALL_INT_MASK    GENMASK(28, 25)
>>>>   #define PCL_RCV_INT_ALL_ENABLE        GENMASK(20, 17)
>>>> +#define PCL_RCV_INT_ALL_MSI_MASK    GENMASK(12, 9)
>>>>   #define PCL_CFG_BW_MGT_STATUS        BIT(4)
>>>>   #define PCL_CFG_LINK_AUTO_BW_STATUS    BIT(3)
>>>>   #define PCL_CFG_AER_RC_ERR_MSI_STATUS    BIT(2)
>>>> @@ -167,7 +169,15 @@ static void uniphier_pcie_stop_link(struct dw_pcie *pci)
>>>>     static void uniphier_pcie_irq_enable(struct uniphier_pcie_priv *priv)
>>>>   {
>>>> -    writel(PCL_RCV_INT_ALL_ENABLE, priv->base + PCL_RCV_INT);
>>>> +    u32 val;
>>>> +
>>>> +    val = PCL_RCV_INT_ALL_ENABLE;
>>>> +    if (pci_msi_enabled())
>>>> +        val |= PCL_RCV_INT_ALL_INT_MASK;
>>>> +    else
>>>> +        val |= PCL_RCV_INT_ALL_MSI_MASK;
>>>
>>> Does this affect endpoints? Or just the RC itself?
>>
>> These interrupts are asserted by RC itself, so this part affects only RC.
>>
>>>> +
>>>> +    writel(val, priv->base + PCL_RCV_INT);
>>>>       writel(PCL_RCV_INTX_ALL_ENABLE, priv->base + PCL_RCV_INTX);
>>>>   }
>>>>   @@ -231,32 +241,56 @@ static const struct irq_domain_ops uniphier_intx_domain_ops = {
>>>>       .map = uniphier_pcie_intx_map,
>>>>   };
>>>>   -static void uniphier_pcie_irq_handler(struct irq_desc *desc)
>>>> +static void uniphier_pcie_misc_isr(struct pcie_port *pp, bool is_msi)
>>>>   {
>>>> -    struct pcie_port *pp = irq_desc_get_handler_data(desc);
>>>>       struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>>>>       struct uniphier_pcie_priv *priv = to_uniphier_pcie(pci);
>>>> -    struct irq_chip *chip = irq_desc_get_chip(desc);
>>>> -    unsigned long reg;
>>>> -    u32 val, bit, virq;
>>>> +    u32 val, virq;
>>>>   -    /* INT for debug */
>>>>       val = readl(priv->base + PCL_RCV_INT);
>>>>         if (val & PCL_CFG_BW_MGT_STATUS)
>>>>           dev_dbg(pci->dev, "Link Bandwidth Management Event\n");
>>>> +
>>>>       if (val & PCL_CFG_LINK_AUTO_BW_STATUS)
>>>>           dev_dbg(pci->dev, "Link Autonomous Bandwidth Event\n");
>>>> -    if (val & PCL_CFG_AER_RC_ERR_MSI_STATUS)
>>>> -        dev_dbg(pci->dev, "Root Error\n");
>>>> -    if (val & PCL_CFG_PME_MSI_STATUS)
>>>> -        dev_dbg(pci->dev, "PME Interrupt\n");
>>>> +
>>>> +    if (is_msi) {
>>>> +        if (val & PCL_CFG_AER_RC_ERR_MSI_STATUS)
>>>> +            dev_dbg(pci->dev, "Root Error Status\n");
>>>> +
>>>> +        if (val & PCL_CFG_PME_MSI_STATUS)
>>>> +            dev_dbg(pci->dev, "PME Interrupt\n");
>>>> +
>>>> +        if (val & (PCL_CFG_AER_RC_ERR_MSI_STATUS |
>>>> +               PCL_CFG_PME_MSI_STATUS)) {
>>>> +            virq = irq_linear_revmap(pp->irq_domain, 0);
>>>> +            generic_handle_irq(virq);
>>>> +        }
>>>> +    }
>>>
>>> Please have two handlers: one for interrupts that are from the RC,
>>> another for interrupts coming from the endpoints.
>> I assume that this handler treats interrupts from the RC only and
>> this is set on the member ".msi_host_isr" added in the patch 1/6.
>> I think that the handler for interrupts coming from endpoints should be
>> treated as a normal case (after calling .msi_host_isr in
>> dw_handle_msi_irq()).
> 
> It looks pretty odd that you end-up dealing with both from the
> same "parent" interrupt. I guess this is in keeping with the
> rest of the DW PCIe hacks... :-/

It might be odd, however, in case of UniPhier SoC,
both MSI interrupts from endpoints and PME/AER interrupts from RC are
asserted by same "parent" interrupt. In other words, PME/AER interrupts
are notified using the parent interrupt for MSI.

MSI interrupts are treated as child interrupts with reference to
the status register in DW core. This is handled in a for-loop in
dw_handle_msi_irq().

PME/AER interrupts are treated with reference to the status register
in UniPhier glue layer, however, this couldn't be handled in the same way
directly.

So I'm trying to add .msi_host_isr function to handle this
with reference to the SoC-specific registers.

This exported function asserts MSI-0 as a shared child interrupt.
As a result, PME/AER are registered like the followings in dmesg:

    pcieport 0000:00:00.0: PME: Signaling with IRQ 25
    pcieport 0000:00:00.0: AER: enabled with IRQ 25

And these interrupts are shared as MSI-0:

    # cat /proc/interrupts | grep 25:
     25:          0          0          0          0   PCI-MSI   0 Edge      PCIe PME, aerdrv

This might be a special case, though, I think that this is needed to handle
interrupts from RC sharing MSI parent.
  
> It is for Lorenzo to make up his mind about this anyway.

I'd like to Lorenzo's opinion, too.

Thank you,

---
Best Regards
Kunihiko Hayashi

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

end of thread, back to index

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-18  8:38 [PATCH v5 0/6] PCI: uniphier: Add features for UniPhier PCIe host controller Kunihiko Hayashi
2020-06-18  8:38 ` [PATCH v5 1/6] PCI: dwc: Add msi_host_isr() callback Kunihiko Hayashi
2020-06-18  8:38 ` [PATCH v5 2/6] PCI: uniphier: Add misc interrupt handler to invoke PME and AER Kunihiko Hayashi
2020-06-27  9:48   ` Marc Zyngier
2020-06-29  9:49     ` Kunihiko Hayashi
2020-06-30 13:23       ` Marc Zyngier
2020-07-01  2:18         ` Kunihiko Hayashi
2020-06-18  8:38 ` [PATCH v5 3/6] dt-bindings: PCI: uniphier: Add iATU register description Kunihiko Hayashi
2020-06-18  8:38 ` [PATCH v5 4/6] PCI: uniphier: Add iATU register support Kunihiko Hayashi
2020-06-18  8:38 ` [PATCH v5 5/6] PCI: uniphier: Add error message when failed to get phy Kunihiko Hayashi
2020-06-18  8:38 ` [PATCH v5 6/6] PCI: uniphier: Use devm_platform_ioremap_resource_byname() Kunihiko Hayashi

Linux-ARM-Kernel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/0 linux-arm-kernel/git/0.git
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/1 linux-arm-kernel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-kernel linux-arm-kernel/ https://lore.kernel.org/linux-arm-kernel \
		linux-arm-kernel@lists.infradead.org
	public-inbox-index linux-arm-kernel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-arm-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git