linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC 0/8] add new irq api to pcie-designware
@ 2017-05-12 16:56 Joao Pinto
  2017-05-12 16:56 ` [RFC 1/8] pci: adding new irq api to pci-designware Joao Pinto
                   ` (7 more replies)
  0 siblings, 8 replies; 12+ messages in thread
From: Joao Pinto @ 2017-05-12 16:56 UTC (permalink / raw)
  To: bhelgaas, marc.zyngier
  Cc: jingoohan1, m-karicheri2, thomas.petazzoni, minghuan.Lian,
	mingkai.hu, tie-fei.zang, hongxing.zhu, l.stach, niklas.cassel,
	jesper.nilsson, wangzhou1, gabriele.paoloni, svarbanov, linux-pci,
	Joao Pinto

This patch series adds the new interrupt api to pcie-designware
make it possible to use features like MSIX.

The work consisted of adapting the pcie-designware-host and each SoC
specific driver.

Joao Pinto (8):
  pci: adding new irq api to pci-designware
  pci: exynos SoC driver adapted to new irq API
  pci: imx6 SoC driver adapted to new irq API
  pci: artpec6 SoC driver adapted to new irq API
  pci: generic PCIe DW driver adapted to new irq API
  pci: qcom SoC driver adapted to new irq API
  pci: keystone SoC driver adapted to new irq API
  pci: removing old irq api from pcie-designware

 drivers/pci/dwc/pci-exynos.c           |  18 --
 drivers/pci/dwc/pci-imx6.c             |  18 --
 drivers/pci/dwc/pci-keystone-dw.c      |  97 +--------
 drivers/pci/dwc/pci-keystone.c         |   1 +
 drivers/pci/dwc/pci-keystone.h         |   4 +-
 drivers/pci/dwc/pci-layerscape.c       |   4 +-
 drivers/pci/dwc/pcie-artpec6.c         |  18 --
 drivers/pci/dwc/pcie-designware-host.c | 349 ++++++++++++++++++---------------
 drivers/pci/dwc/pcie-designware-plat.c |  15 --
 drivers/pci/dwc/pcie-designware.h      |   8 +-
 drivers/pci/dwc/pcie-qcom.c            |  15 --
 11 files changed, 205 insertions(+), 342 deletions(-)

-- 
2.9.3

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

* [RFC 1/8] pci: adding new irq api to pci-designware
  2017-05-12 16:56 [RFC 0/8] add new irq api to pcie-designware Joao Pinto
@ 2017-05-12 16:56 ` Joao Pinto
  2017-05-12 17:02   ` Lucas Stach
  2017-05-12 17:36   ` Marc Zyngier
  2017-05-12 16:56 ` [RFC 2/8] pci: exynos SoC driver adapted to new irq API Joao Pinto
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 12+ messages in thread
From: Joao Pinto @ 2017-05-12 16:56 UTC (permalink / raw)
  To: bhelgaas, marc.zyngier
  Cc: jingoohan1, m-karicheri2, thomas.petazzoni, minghuan.Lian,
	mingkai.hu, tie-fei.zang, hongxing.zhu, l.stach, niklas.cassel,
	jesper.nilsson, wangzhou1, gabriele.paoloni, svarbanov, linux-pci,
	Joao Pinto

This patch adds the new interrupt api to pecie-designware, keeping the old
one. A define was added to toggle between the two apis (for testing purposes).

Signed-off-by: Joao Pinto <jpinto@synopsys.com>
---
 drivers/pci/dwc/pcie-designware-host.c | 241 ++++++++++++++++++++++++++++++++-
 drivers/pci/dwc/pcie-designware.h      |   5 +
 2 files changed, 244 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c
index 28ed32b..6810ea10b 100644
--- a/drivers/pci/dwc/pcie-designware-host.c
+++ b/drivers/pci/dwc/pcie-designware-host.c
@@ -11,6 +11,7 @@
  * published by the Free Software Foundation.
  */
 
+#include <linux/irqchip/chained_irq.h>
 #include <linux/irqdomain.h>
 #include <linux/of_address.h>
 #include <linux/of_pci.h>
@@ -19,6 +20,9 @@
 
 #include "pcie-designware.h"
 
+/* Temporary: Activate to use new IRQ API */
+/*#define CONFIG_PCIEDW_NEW_IRQ_API*/
+
 static struct pci_ops dw_pcie_ops;
 
 static int dw_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
@@ -45,6 +49,7 @@ static int dw_pcie_wr_own_conf(struct pcie_port *pp, int where, int size,
 	return dw_pcie_write(pci->dbi_base + where, size, val);
 }
 
+#ifndef CONFIG_PCIEDW_NEW_IRQ_API
 static struct irq_chip dw_msi_irq_chip = {
 	.name = "PCI-MSI",
 	.irq_enable = pci_msi_unmask_irq,
@@ -52,6 +57,29 @@ static struct irq_chip dw_msi_irq_chip = {
 	.irq_mask = pci_msi_mask_irq,
 	.irq_unmask = pci_msi_unmask_irq,
 };
+#else
+static void dw_msi_mask_irq(struct irq_data *d) {
+	pci_msi_mask_irq(d);
+	irq_chip_mask_parent(d);
+}
+
+static void dw_msi_unmask_irq(struct irq_data *d) {
+	pci_msi_unmask_irq(d);
+	irq_chip_unmask_parent(d);
+}
+
+static struct irq_chip dw_msi_irq_chip = {
+	.name = "PCI-MSI",
+	.irq_mask = dw_msi_mask_irq,
+	.irq_unmask = dw_msi_unmask_irq,
+};
+
+static struct msi_domain_info dw_pcie_msi_domain_info = {
+	.flags	= (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
+		     MSI_FLAG_PCI_MSIX),
+	.chip	= &dw_msi_irq_chip,
+};
+#endif
 
 /* MSI int handler */
 irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
@@ -81,6 +109,190 @@ irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
 	return ret;
 }
 
+#ifdef CONFIG_PCIEDW_NEW_IRQ_API
+/* Chained MSI interrupt service routine */
+static void dw_chained_msi_isr(struct irq_desc *desc)
+{
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	struct pcie_port *pp;
+	struct dw_pcie *pci;
+
+	chained_irq_enter(chip, desc);
+	pci = irq_desc_get_handler_data(desc);
+	pp = &pci->pp;
+
+	dw_handle_msi_irq(pp);
+
+	chained_irq_exit(chip, desc);
+}
+
+static void dw_pci_setup_msi_msg(struct irq_data *data, struct msi_msg *msg)
+{
+	struct dw_pcie *pci = irq_data_get_irq_chip_data(data);
+	struct pcie_port *pp = &pci->pp;
+	u64 msi_target;
+
+	if (pp->ops->get_msi_addr)
+		msi_target = pp->ops->get_msi_addr(pp);
+	else
+		msi_target = virt_to_phys((void *)pp->msi_data);
+
+	msg->address_lo = (u32)(msi_target & 0xffffffff);
+	msg->address_hi = (u32)(msi_target >> 32 & 0xffffffff);
+
+	if (pp->ops->get_msi_data)
+		msg->data = pp->ops->get_msi_data(pp, data->hwirq);
+	else
+		msg->data = data->hwirq;
+
+	dev_dbg(pci->dev, "msi#%d address_hi %#x address_lo %#x\n",
+		(int)data->hwirq, msg->address_hi, msg->address_lo);
+}
+
+static int dw_pci_msi_set_affinity(struct irq_data *irq_data,
+				   const struct cpumask *mask, bool force)
+{
+	return -EINVAL;
+}
+
+static void dw_pci_bottom_mask(struct irq_data *data)
+{
+	struct dw_pcie *pci = irq_data_get_irq_chip_data(data);
+	struct pcie_port *pp = &pci->pp;
+	unsigned int res, bit, val;
+
+	mutex_lock(&pp->lock);
+
+	if (pp->ops->msi_clear_irq)
+		pp->ops->msi_clear_irq(pp, data->hwirq);
+	else {
+		res = (data->hwirq / 32) * 12;
+		bit = data->hwirq % 32;
+
+		dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, &val);
+		val &= ~(1 << bit);
+		dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, val);
+	}
+
+	mutex_unlock(&pp->lock);
+}
+
+static void dw_pci_bottom_unmask(struct irq_data *data)
+{
+	struct dw_pcie *pci = irq_data_get_irq_chip_data(data);
+	struct pcie_port *pp = &pci->pp;
+	unsigned int res, bit, val;
+
+	mutex_lock(&pp->lock);
+
+	if (pp->ops->msi_set_irq)
+		pp->ops->msi_set_irq(pp, data->hwirq);
+	else {
+		res = (data->hwirq / 32) * 12;
+		bit = data->hwirq % 32;
+
+		dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, &val);
+		val |= 1 << bit;
+		dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, val);
+	}
+
+	mutex_unlock(&pp->lock);
+}
+
+static struct irq_chip dw_pci_msi_bottom_irq_chip = {
+	.name			= "DWPCI-MSI",
+	.irq_compose_msi_msg	= dw_pci_setup_msi_msg,
+	.irq_set_affinity	= dw_pci_msi_set_affinity,
+	.irq_mask		= dw_pci_bottom_mask,
+	.irq_unmask		= dw_pci_bottom_unmask,
+};
+
+static int dw_pcie_irq_domain_alloc(struct irq_domain *domain,
+				    unsigned int virq, unsigned int nr_irqs,
+				    void *args)
+{
+	struct dw_pcie *pci = domain->host_data;
+	struct pcie_port *pp = &pci->pp;
+	unsigned long bit;
+
+	mutex_lock(&pp->lock);
+
+	WARN_ON(nr_irqs != 1);
+
+	bit = find_first_zero_bit(pp->msi_irq_in_use, pp->num_vectors);
+	if (bit >= pp->num_vectors) {
+		mutex_unlock(&pp->lock);
+		return -ENOSPC;
+	}
+
+	set_bit(bit, pp->msi_irq_in_use);
+
+	mutex_unlock(&pp->lock);
+
+	irq_domain_set_info(domain, virq, bit, &dw_pci_msi_bottom_irq_chip,
+			    domain->host_data, handle_simple_irq,
+			    NULL, NULL);
+
+	return 0;
+}
+
+static void dw_pcie_irq_domain_free(struct irq_domain *domain,
+				    unsigned int virq, unsigned int nr_irqs)
+{
+	struct irq_data *data = irq_domain_get_irq_data(domain, virq);
+	struct dw_pcie *pci = irq_data_get_irq_chip_data(data);
+	struct pcie_port *pp = &pci->pp;
+
+	mutex_lock(&pp->lock);
+
+	if (!test_bit(data->hwirq, pp->msi_irq_in_use))
+		dev_err(pci->dev, "trying to free unused MSI#%lu\n",
+			data->hwirq);
+	else
+		__clear_bit(data->hwirq, pp->msi_irq_in_use);
+
+	mutex_unlock(&pp->lock);
+}
+
+static const struct irq_domain_ops dw_pcie_msi_domain_ops = {
+	.alloc	= dw_pcie_irq_domain_alloc,
+	.free	= dw_pcie_irq_domain_free,
+};
+
+int dw_pcie_allocate_domains(struct dw_pcie *pci)
+{
+	struct pcie_port *pp = &pci->pp;
+	struct fwnode_handle *fwnode = of_node_to_fwnode(pci->dev->of_node);
+
+	pp->irq_domain = irq_domain_add_linear(NULL, pp->num_vectors,
+					       &dw_pcie_msi_domain_ops, pci);
+	if (!pp->irq_domain) {
+		dev_err(pci->dev, "failed to create IRQ domain\n");
+		return -ENOMEM;
+	}
+
+	pp->msi_domain = pci_msi_create_irq_domain(fwnode,
+						   &dw_pcie_msi_domain_info,
+						   pp->irq_domain);
+	if (!pp->msi_domain) {
+		dev_err(pci->dev, "failed to create MSI domain\n");
+		irq_domain_remove(pp->irq_domain);
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+void dw_pcie_free_msi(struct pcie_port *pp)
+{
+	irq_set_chained_handler(pp->msi_irq, NULL);
+	irq_set_handler_data(pp->msi_irq, NULL);
+
+	irq_domain_remove(pp->msi_domain);
+	irq_domain_remove(pp->irq_domain);
+}
+#endif
+
 void dw_pcie_msi_init(struct pcie_port *pp)
 {
 	u64 msi_target;
@@ -279,11 +491,16 @@ int dw_pcie_host_init(struct pcie_port *pp)
 	struct device *dev = pci->dev;
 	struct device_node *np = dev->of_node;
 	struct platform_device *pdev = to_platform_device(dev);
+	struct resource_entry *win, *tmp;
 	struct pci_bus *bus, *child;
 	struct resource *cfg_res;
-	int i, ret;
+#ifndef CONFIG_PCIEDW_NEW_IRQ_API
+	int i;
+#endif
+	int ret;
 	LIST_HEAD(res);
-	struct resource_entry *win, *tmp;
+
+	mutex_init(&pci->pp.lock);
 
 	cfg_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
 	if (cfg_res) {
@@ -378,6 +595,12 @@ int dw_pcie_host_init(struct pcie_port *pp)
 
 	if (IS_ENABLED(CONFIG_PCI_MSI)) {
 		if (!pp->ops->msi_host_init) {
+			ret = of_property_read_u32(np, "num-vectors",
+						   &pp->num_vectors);
+			if (ret)
+				pp->num_vectors = 1;
+
+#ifndef CONFIG_PCIEDW_NEW_IRQ_API
 			pp->irq_domain = irq_domain_add_linear(dev->of_node,
 						MAX_MSI_IRQS, &msi_domain_ops,
 						&dw_pcie_msi_chip);
@@ -389,6 +612,15 @@ int dw_pcie_host_init(struct pcie_port *pp)
 
 			for (i = 0; i < MAX_MSI_IRQS; i++)
 				irq_create_mapping(pp->irq_domain, i);
+#else
+			ret = dw_pcie_allocate_domains(pci);
+			if (ret)
+				goto error;
+
+			irq_set_chained_handler_and_data(pci->pp.msi_irq,
+							 dw_chained_msi_isr,
+							 pci);
+#endif
 		} else {
 			ret = pp->ops->msi_host_init(pp, &dw_pcie_msi_chip);
 			if (ret < 0)
@@ -400,6 +632,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
 		pp->ops->host_init(pp);
 
 	pp->root_bus_nr = pp->busn->start;
+#ifndef CONFIG_PCIEDW_NEW_IRQ_API
 	if (IS_ENABLED(CONFIG_PCI_MSI)) {
 		bus = pci_scan_root_bus_msi(dev, pp->root_bus_nr,
 					    &dw_pcie_ops, pp, &res,
@@ -408,6 +641,10 @@ int dw_pcie_host_init(struct pcie_port *pp)
 	} else
 		bus = pci_scan_root_bus(dev, pp->root_bus_nr, &dw_pcie_ops,
 					pp, &res);
+#else
+	bus = pci_scan_root_bus(dev, pp->root_bus_nr, &dw_pcie_ops,
+				pp, &res);
+#endif
 	if (!bus) {
 		ret = -ENOMEM;
 		goto error;
diff --git a/drivers/pci/dwc/pcie-designware.h b/drivers/pci/dwc/pcie-designware.h
index c6a8405..15481a5 100644
--- a/drivers/pci/dwc/pcie-designware.h
+++ b/drivers/pci/dwc/pcie-designware.h
@@ -165,7 +165,10 @@ struct pcie_port {
 	struct dw_pcie_host_ops	*ops;
 	int			msi_irq;
 	struct irq_domain	*irq_domain;
+	struct irq_domain	*msi_domain;
 	unsigned long		msi_data;
+	u32			num_vectors;
+	struct mutex		lock;
 	DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
 };
 
@@ -282,8 +285,10 @@ static inline u32 dw_pcie_readl_dbi2(struct dw_pcie *pci, u32 reg)
 #ifdef CONFIG_PCIE_DW_HOST
 irqreturn_t dw_handle_msi_irq(struct pcie_port *pp);
 void dw_pcie_msi_init(struct pcie_port *pp);
+void dw_pcie_free_msi(struct pcie_port *pp);
 void dw_pcie_setup_rc(struct pcie_port *pp);
 int dw_pcie_host_init(struct pcie_port *pp);
+int dw_pcie_allocate_domains(struct dw_pcie *pci);
 #else
 static inline irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
 {
-- 
2.9.3

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

* [RFC 2/8] pci: exynos SoC driver adapted to new irq API
  2017-05-12 16:56 [RFC 0/8] add new irq api to pcie-designware Joao Pinto
  2017-05-12 16:56 ` [RFC 1/8] pci: adding new irq api to pci-designware Joao Pinto
@ 2017-05-12 16:56 ` Joao Pinto
  2017-05-12 16:56 ` [RFC 3/8] pci: imx6 " Joao Pinto
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Joao Pinto @ 2017-05-12 16:56 UTC (permalink / raw)
  To: bhelgaas, marc.zyngier
  Cc: jingoohan1, m-karicheri2, thomas.petazzoni, minghuan.Lian,
	mingkai.hu, tie-fei.zang, hongxing.zhu, l.stach, niklas.cassel,
	jesper.nilsson, wangzhou1, gabriele.paoloni, svarbanov, linux-pci,
	Joao Pinto

This patch adapts Exynos SoC specific driver to use the new interrupt api
available in pcie-designware.

Signed-off-by: Joao Pinto <jpinto@synopsys.com>
---
 drivers/pci/dwc/pci-exynos.c | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/drivers/pci/dwc/pci-exynos.c b/drivers/pci/dwc/pci-exynos.c
index 546082a..e49c39a 100644
--- a/drivers/pci/dwc/pci-exynos.c
+++ b/drivers/pci/dwc/pci-exynos.c
@@ -490,15 +490,6 @@ static irqreturn_t exynos_pcie_irq_handler(int irq, void *arg)
 	return IRQ_HANDLED;
 }
 
-static irqreturn_t exynos_pcie_msi_irq_handler(int irq, void *arg)
-{
-	struct exynos_pcie *ep = arg;
-	struct dw_pcie *pci = ep->pci;
-	struct pcie_port *pp = &pci->pp;
-
-	return dw_handle_msi_irq(pp);
-}
-
 static void exynos_pcie_msi_init(struct exynos_pcie *ep)
 {
 	struct dw_pcie *pci = ep->pci;
@@ -622,15 +613,6 @@ static int __init exynos_add_pcie_port(struct exynos_pcie *ep,
 			dev_err(dev, "failed to get msi irq\n");
 			return -ENODEV;
 		}
-
-		ret = devm_request_irq(dev, pp->msi_irq,
-					exynos_pcie_msi_irq_handler,
-					IRQF_SHARED | IRQF_NO_THREAD,
-					"exynos-pcie", ep);
-		if (ret) {
-			dev_err(dev, "failed to request msi irq\n");
-			return ret;
-		}
 	}
 
 	pp->root_bus_nr = -1;
-- 
2.9.3

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

* [RFC 3/8] pci: imx6 SoC driver adapted to new irq API
  2017-05-12 16:56 [RFC 0/8] add new irq api to pcie-designware Joao Pinto
  2017-05-12 16:56 ` [RFC 1/8] pci: adding new irq api to pci-designware Joao Pinto
  2017-05-12 16:56 ` [RFC 2/8] pci: exynos SoC driver adapted to new irq API Joao Pinto
@ 2017-05-12 16:56 ` Joao Pinto
  2017-05-12 16:56 ` [RFC 4/8] pci: artpec6 " Joao Pinto
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Joao Pinto @ 2017-05-12 16:56 UTC (permalink / raw)
  To: bhelgaas, marc.zyngier
  Cc: jingoohan1, m-karicheri2, thomas.petazzoni, minghuan.Lian,
	mingkai.hu, tie-fei.zang, hongxing.zhu, l.stach, niklas.cassel,
	jesper.nilsson, wangzhou1, gabriele.paoloni, svarbanov, linux-pci,
	Joao Pinto

This patch adapts imx6 SoC specific driver to use the new interrupt api
available in pcie-designware.


Signed-off-by: Joao Pinto <jpinto@synopsys.com>
---
 drivers/pci/dwc/pci-imx6.c | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/drivers/pci/dwc/pci-imx6.c b/drivers/pci/dwc/pci-imx6.c
index a98cba5..f22ce1f 100644
--- a/drivers/pci/dwc/pci-imx6.c
+++ b/drivers/pci/dwc/pci-imx6.c
@@ -490,15 +490,6 @@ static int imx6_pcie_wait_for_speed_change(struct imx6_pcie *imx6_pcie)
 	return -EINVAL;
 }
 
-static irqreturn_t imx6_pcie_msi_handler(int irq, void *arg)
-{
-	struct imx6_pcie *imx6_pcie = arg;
-	struct dw_pcie *pci = imx6_pcie->pci;
-	struct pcie_port *pp = &pci->pp;
-
-	return dw_handle_msi_irq(pp);
-}
-
 static int imx6_pcie_establish_link(struct imx6_pcie *imx6_pcie)
 {
 	struct dw_pcie *pci = imx6_pcie->pci;
@@ -620,15 +611,6 @@ static int imx6_add_pcie_port(struct imx6_pcie *imx6_pcie,
 			dev_err(dev, "failed to get MSI irq\n");
 			return -ENODEV;
 		}
-
-		ret = devm_request_irq(dev, pp->msi_irq,
-				       imx6_pcie_msi_handler,
-				       IRQF_SHARED | IRQF_NO_THREAD,
-				       "mx6-pcie-msi", imx6_pcie);
-		if (ret) {
-			dev_err(dev, "failed to request MSI irq\n");
-			return ret;
-		}
 	}
 
 	pp->root_bus_nr = -1;
-- 
2.9.3

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

* [RFC 4/8] pci: artpec6 SoC driver adapted to new irq API
  2017-05-12 16:56 [RFC 0/8] add new irq api to pcie-designware Joao Pinto
                   ` (2 preceding siblings ...)
  2017-05-12 16:56 ` [RFC 3/8] pci: imx6 " Joao Pinto
@ 2017-05-12 16:56 ` Joao Pinto
  2017-05-12 16:56 ` [RFC 5/8] pci: generic PCIe DW " Joao Pinto
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Joao Pinto @ 2017-05-12 16:56 UTC (permalink / raw)
  To: bhelgaas, marc.zyngier
  Cc: jingoohan1, m-karicheri2, thomas.petazzoni, minghuan.Lian,
	mingkai.hu, tie-fei.zang, hongxing.zhu, l.stach, niklas.cassel,
	jesper.nilsson, wangzhou1, gabriele.paoloni, svarbanov, linux-pci,
	Joao Pinto

This patch adapts artpec6 SoC specific driver to use the new interrupt api
available in pcie-designware.

Signed-off-by: Joao Pinto <jpinto@synopsys.com>
---
 drivers/pci/dwc/pcie-artpec6.c | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/drivers/pci/dwc/pcie-artpec6.c b/drivers/pci/dwc/pcie-artpec6.c
index 82a04ac..d81789b 100644
--- a/drivers/pci/dwc/pcie-artpec6.c
+++ b/drivers/pci/dwc/pcie-artpec6.c
@@ -188,15 +188,6 @@ static struct dw_pcie_host_ops artpec6_pcie_host_ops = {
 	.host_init = artpec6_pcie_host_init,
 };
 
-static irqreturn_t artpec6_pcie_msi_handler(int irq, void *arg)
-{
-	struct artpec6_pcie *artpec6_pcie = arg;
-	struct dw_pcie *pci = artpec6_pcie->pci;
-	struct pcie_port *pp = &pci->pp;
-
-	return dw_handle_msi_irq(pp);
-}
-
 static int artpec6_add_pcie_port(struct artpec6_pcie *artpec6_pcie,
 				 struct platform_device *pdev)
 {
@@ -211,15 +202,6 @@ static int artpec6_add_pcie_port(struct artpec6_pcie *artpec6_pcie,
 			dev_err(dev, "failed to get MSI irq\n");
 			return -ENODEV;
 		}
-
-		ret = devm_request_irq(dev, pp->msi_irq,
-				       artpec6_pcie_msi_handler,
-				       IRQF_SHARED | IRQF_NO_THREAD,
-				       "artpec6-pcie-msi", artpec6_pcie);
-		if (ret) {
-			dev_err(dev, "failed to request MSI irq\n");
-			return ret;
-		}
 	}
 
 	pp->root_bus_nr = -1;
-- 
2.9.3

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

* [RFC 5/8] pci: generic PCIe DW driver adapted to new irq API
  2017-05-12 16:56 [RFC 0/8] add new irq api to pcie-designware Joao Pinto
                   ` (3 preceding siblings ...)
  2017-05-12 16:56 ` [RFC 4/8] pci: artpec6 " Joao Pinto
@ 2017-05-12 16:56 ` Joao Pinto
  2017-05-12 16:56 ` [RFC 6/8] pci: qcom SoC " Joao Pinto
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Joao Pinto @ 2017-05-12 16:56 UTC (permalink / raw)
  To: bhelgaas, marc.zyngier
  Cc: jingoohan1, m-karicheri2, thomas.petazzoni, minghuan.Lian,
	mingkai.hu, tie-fei.zang, hongxing.zhu, l.stach, niklas.cassel,
	jesper.nilsson, wangzhou1, gabriele.paoloni, svarbanov, linux-pci,
	Joao Pinto

This patch adapts the generic PCIe DW driver to use the new interrupt api
available in pcie-designware.

Signed-off-by: Joao Pinto <jpinto@synopsys.com>
---
 drivers/pci/dwc/pcie-designware-plat.c | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/drivers/pci/dwc/pcie-designware-plat.c b/drivers/pci/dwc/pcie-designware-plat.c
index 32091b3..3b9717b 100644
--- a/drivers/pci/dwc/pcie-designware-plat.c
+++ b/drivers/pci/dwc/pcie-designware-plat.c
@@ -28,13 +28,6 @@ struct dw_plat_pcie {
 	struct dw_pcie		*pci;
 };
 
-static irqreturn_t dw_plat_pcie_msi_irq_handler(int irq, void *arg)
-{
-	struct pcie_port *pp = arg;
-
-	return dw_handle_msi_irq(pp);
-}
-
 static void dw_plat_pcie_host_init(struct pcie_port *pp)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
@@ -64,14 +57,6 @@ static int dw_plat_add_pcie_port(struct pcie_port *pp,
 		pp->msi_irq = platform_get_irq(pdev, 0);
 		if (pp->msi_irq < 0)
 			return pp->msi_irq;
-
-		ret = devm_request_irq(dev, pp->msi_irq,
-					dw_plat_pcie_msi_irq_handler,
-					IRQF_SHARED, "dw-plat-pcie-msi", pp);
-		if (ret) {
-			dev_err(dev, "failed to request MSI IRQ\n");
-			return ret;
-		}
 	}
 
 	pp->root_bus_nr = -1;
-- 
2.9.3

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

* [RFC 6/8] pci: qcom SoC driver adapted to new irq API
  2017-05-12 16:56 [RFC 0/8] add new irq api to pcie-designware Joao Pinto
                   ` (4 preceding siblings ...)
  2017-05-12 16:56 ` [RFC 5/8] pci: generic PCIe DW " Joao Pinto
@ 2017-05-12 16:56 ` Joao Pinto
  2017-05-12 16:56 ` [RFC 7/8] pci: keystone " Joao Pinto
  2017-05-12 16:56 ` [RFC 8/8] pci: removing old irq api from pcie-designware Joao Pinto
  7 siblings, 0 replies; 12+ messages in thread
From: Joao Pinto @ 2017-05-12 16:56 UTC (permalink / raw)
  To: bhelgaas, marc.zyngier
  Cc: jingoohan1, m-karicheri2, thomas.petazzoni, minghuan.Lian,
	mingkai.hu, tie-fei.zang, hongxing.zhu, l.stach, niklas.cassel,
	jesper.nilsson, wangzhou1, gabriele.paoloni, svarbanov, linux-pci,
	Joao Pinto

This patch adapts Qcom SoC specific driver to use the new interrupt api
available in pcie-designware.

Signed-off-by: Joao Pinto <jpinto@synopsys.com>
---
 drivers/pci/dwc/pcie-qcom.c | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/drivers/pci/dwc/pcie-qcom.c b/drivers/pci/dwc/pcie-qcom.c
index 5bf23d4..4e1cc02 100644
--- a/drivers/pci/dwc/pcie-qcom.c
+++ b/drivers/pci/dwc/pcie-qcom.c
@@ -126,13 +126,6 @@ static void qcom_ep_reset_deassert(struct qcom_pcie *pcie)
 	usleep_range(PERST_DELAY_US, PERST_DELAY_US + 500);
 }
 
-static irqreturn_t qcom_pcie_msi_irq_handler(int irq, void *arg)
-{
-	struct pcie_port *pp = arg;
-
-	return dw_handle_msi_irq(pp);
-}
-
 static void qcom_pcie_v0_v1_ltssm_enable(struct qcom_pcie *pcie)
 {
 	u32 val;
@@ -724,14 +717,6 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 		pp->msi_irq = platform_get_irq_byname(pdev, "msi");
 		if (pp->msi_irq < 0)
 			return pp->msi_irq;
-
-		ret = devm_request_irq(dev, pp->msi_irq,
-				       qcom_pcie_msi_irq_handler,
-				       IRQF_SHARED, "qcom-pcie-msi", pp);
-		if (ret) {
-			dev_err(dev, "cannot request msi irq\n");
-			return ret;
-		}
 	}
 
 	ret = phy_init(pcie->phy);
-- 
2.9.3

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

* [RFC 7/8] pci: keystone SoC driver adapted to new irq API
  2017-05-12 16:56 [RFC 0/8] add new irq api to pcie-designware Joao Pinto
                   ` (5 preceding siblings ...)
  2017-05-12 16:56 ` [RFC 6/8] pci: qcom SoC " Joao Pinto
@ 2017-05-12 16:56 ` Joao Pinto
  2017-05-12 16:56 ` [RFC 8/8] pci: removing old irq api from pcie-designware Joao Pinto
  7 siblings, 0 replies; 12+ messages in thread
From: Joao Pinto @ 2017-05-12 16:56 UTC (permalink / raw)
  To: bhelgaas, marc.zyngier
  Cc: jingoohan1, m-karicheri2, thomas.petazzoni, minghuan.Lian,
	mingkai.hu, tie-fei.zang, hongxing.zhu, l.stach, niklas.cassel,
	jesper.nilsson, wangzhou1, gabriele.paoloni, svarbanov, linux-pci,
	Joao Pinto

This patch adapts Keystone SoC specific driver to use the new interrupt api
available in pcie-designware. A new callback was added to dw_pcie_host_ops
to handle a specific Keystone function and msi_host_init callback is changed
to simplify the access to pci data structure for keystone all SoC drivers
that use the structure.

Signed-off-by: Joao Pinto <jpinto@synopsys.com>
---
 drivers/pci/dwc/pci-keystone-dw.c      | 97 +++-------------------------------
 drivers/pci/dwc/pci-keystone.c         |  1 +
 drivers/pci/dwc/pci-keystone.h         |  4 +-
 drivers/pci/dwc/pci-layerscape.c       |  3 +-
 drivers/pci/dwc/pcie-designware-host.c | 12 ++++-
 drivers/pci/dwc/pcie-designware.h      |  3 +-
 6 files changed, 23 insertions(+), 97 deletions(-)

diff --git a/drivers/pci/dwc/pci-keystone-dw.c b/drivers/pci/dwc/pci-keystone-dw.c
index 8bc626e..da1472e 100644
--- a/drivers/pci/dwc/pci-keystone-dw.c
+++ b/drivers/pci/dwc/pci-keystone-dw.c
@@ -124,19 +124,15 @@ void ks_dw_pcie_handle_msi_irq(struct keystone_pcie *ks_pcie, int offset)
 	}
 }
 
-static void ks_dw_pcie_msi_irq_ack(struct irq_data *d)
+void ks_dw_pcie_msi_irq_ack(int irq, struct pcie_port *pp)
 {
 	u32 offset, reg_offset, bit_pos;
 	struct keystone_pcie *ks_pcie;
-	struct msi_desc *msi;
-	struct pcie_port *pp;
 	struct dw_pcie *pci;
 
-	msi = irq_data_get_msi_desc(d);
-	pp = (struct pcie_port *) msi_desc_to_pci_sysdata(msi);
 	pci = to_dw_pcie_from_pp(pp);
 	ks_pcie = to_keystone_pcie(pci);
-	offset = d->irq - irq_linear_revmap(pp->irq_domain, 0);
+	offset = irq - irq_linear_revmap(pp->irq_domain, 0);
 	update_reg_offset_bit_pos(offset, &reg_offset, &bit_pos);
 
 	ks_dw_app_writel(ks_pcie, MSI0_IRQ_STATUS + (reg_offset << 4),
@@ -166,93 +162,12 @@ void ks_dw_pcie_msi_clear_irq(struct pcie_port *pp, int irq)
 			 BIT(bit_pos));
 }
 
-static void ks_dw_pcie_msi_irq_mask(struct irq_data *d)
+int ks_dw_pcie_msi_host_init(struct dw_pcie *pci, struct msi_controller *chip)
 {
-	struct keystone_pcie *ks_pcie;
-	struct msi_desc *msi;
-	struct pcie_port *pp;
-	struct dw_pcie *pci;
-	u32 offset;
-
-	msi = irq_data_get_msi_desc(d);
-	pp = (struct pcie_port *) msi_desc_to_pci_sysdata(msi);
-	pci = to_dw_pcie_from_pp(pp);
-	ks_pcie = to_keystone_pcie(pci);
-	offset = d->irq - irq_linear_revmap(pp->irq_domain, 0);
-
-	/* Mask the end point if PVM implemented */
-	if (IS_ENABLED(CONFIG_PCI_MSI)) {
-		if (msi->msi_attrib.maskbit)
-			pci_msi_mask_irq(d);
-	}
-
-	ks_dw_pcie_msi_clear_irq(pp, offset);
-}
-
-static void ks_dw_pcie_msi_irq_unmask(struct irq_data *d)
-{
-	struct keystone_pcie *ks_pcie;
-	struct msi_desc *msi;
-	struct pcie_port *pp;
-	struct dw_pcie *pci;
-	u32 offset;
-
-	msi = irq_data_get_msi_desc(d);
-	pp = (struct pcie_port *) msi_desc_to_pci_sysdata(msi);
-	pci = to_dw_pcie_from_pp(pp);
-	ks_pcie = to_keystone_pcie(pci);
-	offset = d->irq - irq_linear_revmap(pp->irq_domain, 0);
-
-	/* Mask the end point if PVM implemented */
-	if (IS_ENABLED(CONFIG_PCI_MSI)) {
-		if (msi->msi_attrib.maskbit)
-			pci_msi_unmask_irq(d);
-	}
-
-	ks_dw_pcie_msi_set_irq(pp, offset);
-}
+	/* overide number of vectors and sent them to Keystone Max MSI IRQs */
+	pci->pp.num_vectors = MAX_MSI_IRQS;
 
-static struct irq_chip ks_dw_pcie_msi_irq_chip = {
-	.name = "Keystone-PCIe-MSI-IRQ",
-	.irq_ack = ks_dw_pcie_msi_irq_ack,
-	.irq_mask = ks_dw_pcie_msi_irq_mask,
-	.irq_unmask = ks_dw_pcie_msi_irq_unmask,
-};
-
-static int ks_dw_pcie_msi_map(struct irq_domain *domain, unsigned int irq,
-			      irq_hw_number_t hwirq)
-{
-	irq_set_chip_and_handler(irq, &ks_dw_pcie_msi_irq_chip,
-				 handle_level_irq);
-	irq_set_chip_data(irq, domain->host_data);
-
-	return 0;
-}
-
-static const struct irq_domain_ops ks_dw_pcie_msi_domain_ops = {
-	.map = ks_dw_pcie_msi_map,
-};
-
-int ks_dw_pcie_msi_host_init(struct pcie_port *pp, struct msi_controller *chip)
-{
-	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
-	struct keystone_pcie *ks_pcie = to_keystone_pcie(pci);
-	struct device *dev = pci->dev;
-	int i;
-
-	pp->irq_domain = irq_domain_add_linear(ks_pcie->msi_intc_np,
-					MAX_MSI_IRQS,
-					&ks_dw_pcie_msi_domain_ops,
-					chip);
-	if (!pp->irq_domain) {
-		dev_err(dev, "irq domain init failed\n");
-		return -ENXIO;
-	}
-
-	for (i = 0; i < MAX_MSI_IRQS; i++)
-		irq_create_mapping(pp->irq_domain, i);
-
-	return 0;
+	return dw_pcie_allocate_domains(pci);
 }
 
 void ks_dw_pcie_enable_legacy_irqs(struct keystone_pcie *ks_pcie)
diff --git a/drivers/pci/dwc/pci-keystone.c b/drivers/pci/dwc/pci-keystone.c
index fcc9723..a59bd9b 100644
--- a/drivers/pci/dwc/pci-keystone.c
+++ b/drivers/pci/dwc/pci-keystone.c
@@ -299,6 +299,7 @@ static struct dw_pcie_host_ops keystone_pcie_host_ops = {
 	.msi_clear_irq = ks_dw_pcie_msi_clear_irq,
 	.get_msi_addr = ks_dw_pcie_get_msi_addr,
 	.msi_host_init = ks_dw_pcie_msi_host_init,
+	.msi_irq_ack = ks_dw_pcie_msi_irq_ack,
 	.scan_bus = ks_dw_pcie_v3_65_scan_bus,
 };
 
diff --git a/drivers/pci/dwc/pci-keystone.h b/drivers/pci/dwc/pci-keystone.h
index 74c5825..83179382 100644
--- a/drivers/pci/dwc/pci-keystone.h
+++ b/drivers/pci/dwc/pci-keystone.h
@@ -55,9 +55,9 @@ int ks_dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
 		unsigned int devfn, int where, int size, u32 *val);
 void ks_dw_pcie_setup_rc_app_regs(struct keystone_pcie *ks_pcie);
 void ks_dw_pcie_initiate_link_train(struct keystone_pcie *ks_pcie);
+void ks_dw_pcie_msi_irq_ack(int i, struct pcie_port *pp);
 void ks_dw_pcie_msi_set_irq(struct pcie_port *pp, int irq);
 void ks_dw_pcie_msi_clear_irq(struct pcie_port *pp, int irq);
 void ks_dw_pcie_v3_65_scan_bus(struct pcie_port *pp);
-int ks_dw_pcie_msi_host_init(struct pcie_port *pp,
-		struct msi_controller *chip);
+int ks_dw_pcie_msi_host_init(struct dw_pcie *pci, struct msi_controller *chip);
 int ks_dw_pcie_link_up(struct dw_pcie *pci);
diff --git a/drivers/pci/dwc/pci-layerscape.c b/drivers/pci/dwc/pci-layerscape.c
index 27d638c..4fb491b 100644
--- a/drivers/pci/dwc/pci-layerscape.c
+++ b/drivers/pci/dwc/pci-layerscape.c
@@ -162,10 +162,9 @@ static void ls_pcie_host_init(struct pcie_port *pp)
 	iowrite32(0, pci->dbi_base + PCIE_DBI_RO_WR_EN);
 }
 
-static int ls_pcie_msi_host_init(struct pcie_port *pp,
+static int ls_pcie_msi_host_init(struct dw_pcie *pci,
 				 struct msi_controller *chip)
 {
-	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
 	struct device *dev = pci->dev;
 	struct device_node *np = dev->of_node;
 	struct device_node *msi_node;
diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c
index 6810ea10b..fbea485 100644
--- a/drivers/pci/dwc/pcie-designware-host.c
+++ b/drivers/pci/dwc/pcie-designware-host.c
@@ -199,8 +199,18 @@ static void dw_pci_bottom_unmask(struct irq_data *data)
 	mutex_unlock(&pp->lock);
 }
 
+static void dw_pci_bottom_ack(struct irq_data *d)
+{
+	struct msi_desc *msi = irq_data_get_msi_desc(d);
+	struct pcie_port *pp = (struct pcie_port *) msi_desc_to_pci_sysdata(msi);
+
+	if (pp->ops->msi_irq_ack)
+		pp->ops->msi_irq_ack(d->irq, pp);
+}
+
 static struct irq_chip dw_pci_msi_bottom_irq_chip = {
 	.name			= "DWPCI-MSI",
+	.irq_ack		= dw_pci_bottom_ack,
 	.irq_compose_msi_msg	= dw_pci_setup_msi_msg,
 	.irq_set_affinity	= dw_pci_msi_set_affinity,
 	.irq_mask		= dw_pci_bottom_mask,
@@ -622,7 +632,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
 							 pci);
 #endif
 		} else {
-			ret = pp->ops->msi_host_init(pp, &dw_pcie_msi_chip);
+			ret = pp->ops->msi_host_init(pci, &dw_pcie_msi_chip);
 			if (ret < 0)
 				goto error;
 		}
diff --git a/drivers/pci/dwc/pcie-designware.h b/drivers/pci/dwc/pcie-designware.h
index 15481a5..9b621f9 100644
--- a/drivers/pci/dwc/pcie-designware.h
+++ b/drivers/pci/dwc/pcie-designware.h
@@ -140,7 +140,8 @@ struct dw_pcie_host_ops {
 	phys_addr_t (*get_msi_addr)(struct pcie_port *pp);
 	u32 (*get_msi_data)(struct pcie_port *pp, int pos);
 	void (*scan_bus)(struct pcie_port *pp);
-	int (*msi_host_init)(struct pcie_port *pp, struct msi_controller *chip);
+	int (*msi_host_init)(struct dw_pcie *pci, struct msi_controller *chip);
+	void (*msi_irq_ack)(int irq, struct pcie_port *pp);
 };
 
 struct pcie_port {
-- 
2.9.3

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

* [RFC 8/8] pci: removing old irq api from pcie-designware
  2017-05-12 16:56 [RFC 0/8] add new irq api to pcie-designware Joao Pinto
                   ` (6 preceding siblings ...)
  2017-05-12 16:56 ` [RFC 7/8] pci: keystone " Joao Pinto
@ 2017-05-12 16:56 ` Joao Pinto
  7 siblings, 0 replies; 12+ messages in thread
From: Joao Pinto @ 2017-05-12 16:56 UTC (permalink / raw)
  To: bhelgaas, marc.zyngier
  Cc: jingoohan1, m-karicheri2, thomas.petazzoni, minghuan.Lian,
	mingkai.hu, tie-fei.zang, hongxing.zhu, l.stach, niklas.cassel,
	jesper.nilsson, wangzhou1, gabriele.paoloni, svarbanov, linux-pci,
	Joao Pinto

This patch removes the old interrupt API.

Signed-off-by: Joao Pinto <jpinto@synopsys.com>
---
 drivers/pci/dwc/pci-keystone-dw.c      |   2 +-
 drivers/pci/dwc/pci-keystone.h         |   2 +-
 drivers/pci/dwc/pci-layerscape.c       |   3 +-
 drivers/pci/dwc/pcie-designware-host.c | 224 +--------------------------------
 drivers/pci/dwc/pcie-designware.h      |   2 +-
 5 files changed, 6 insertions(+), 227 deletions(-)

diff --git a/drivers/pci/dwc/pci-keystone-dw.c b/drivers/pci/dwc/pci-keystone-dw.c
index da1472e..30a5352 100644
--- a/drivers/pci/dwc/pci-keystone-dw.c
+++ b/drivers/pci/dwc/pci-keystone-dw.c
@@ -162,7 +162,7 @@ void ks_dw_pcie_msi_clear_irq(struct pcie_port *pp, int irq)
 			 BIT(bit_pos));
 }
 
-int ks_dw_pcie_msi_host_init(struct dw_pcie *pci, struct msi_controller *chip)
+int ks_dw_pcie_msi_host_init(struct dw_pcie *pci)
 {
 	/* overide number of vectors and sent them to Keystone Max MSI IRQs */
 	pci->pp.num_vectors = MAX_MSI_IRQS;
diff --git a/drivers/pci/dwc/pci-keystone.h b/drivers/pci/dwc/pci-keystone.h
index 83179382..89119271 100644
--- a/drivers/pci/dwc/pci-keystone.h
+++ b/drivers/pci/dwc/pci-keystone.h
@@ -59,5 +59,5 @@ void ks_dw_pcie_msi_irq_ack(int i, struct pcie_port *pp);
 void ks_dw_pcie_msi_set_irq(struct pcie_port *pp, int irq);
 void ks_dw_pcie_msi_clear_irq(struct pcie_port *pp, int irq);
 void ks_dw_pcie_v3_65_scan_bus(struct pcie_port *pp);
-int ks_dw_pcie_msi_host_init(struct dw_pcie *pci, struct msi_controller *chip);
+int ks_dw_pcie_msi_host_init(struct dw_pcie *pci);
 int ks_dw_pcie_link_up(struct dw_pcie *pci);
diff --git a/drivers/pci/dwc/pci-layerscape.c b/drivers/pci/dwc/pci-layerscape.c
index 4fb491b..231ea75 100644
--- a/drivers/pci/dwc/pci-layerscape.c
+++ b/drivers/pci/dwc/pci-layerscape.c
@@ -162,8 +162,7 @@ static void ls_pcie_host_init(struct pcie_port *pp)
 	iowrite32(0, pci->dbi_base + PCIE_DBI_RO_WR_EN);
 }
 
-static int ls_pcie_msi_host_init(struct dw_pcie *pci,
-				 struct msi_controller *chip)
+static int ls_pcie_msi_host_init(struct dw_pcie *pci)
 {
 	struct device *dev = pci->dev;
 	struct device_node *np = dev->of_node;
diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c
index fbea485..9ef2c26 100644
--- a/drivers/pci/dwc/pcie-designware-host.c
+++ b/drivers/pci/dwc/pcie-designware-host.c
@@ -20,9 +20,6 @@
 
 #include "pcie-designware.h"
 
-/* Temporary: Activate to use new IRQ API */
-/*#define CONFIG_PCIEDW_NEW_IRQ_API*/
-
 static struct pci_ops dw_pcie_ops;
 
 static int dw_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
@@ -49,15 +46,6 @@ static int dw_pcie_wr_own_conf(struct pcie_port *pp, int where, int size,
 	return dw_pcie_write(pci->dbi_base + where, size, val);
 }
 
-#ifndef CONFIG_PCIEDW_NEW_IRQ_API
-static struct irq_chip dw_msi_irq_chip = {
-	.name = "PCI-MSI",
-	.irq_enable = pci_msi_unmask_irq,
-	.irq_disable = pci_msi_mask_irq,
-	.irq_mask = pci_msi_mask_irq,
-	.irq_unmask = pci_msi_unmask_irq,
-};
-#else
 static void dw_msi_mask_irq(struct irq_data *d) {
 	pci_msi_mask_irq(d);
 	irq_chip_mask_parent(d);
@@ -79,7 +67,6 @@ static struct msi_domain_info dw_pcie_msi_domain_info = {
 		     MSI_FLAG_PCI_MSIX),
 	.chip	= &dw_msi_irq_chip,
 };
-#endif
 
 /* MSI int handler */
 irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
@@ -109,7 +96,6 @@ irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
 	return ret;
 }
 
-#ifdef CONFIG_PCIEDW_NEW_IRQ_API
 /* Chained MSI interrupt service routine */
 static void dw_chained_msi_isr(struct irq_desc *desc)
 {
@@ -301,7 +287,6 @@ void dw_pcie_free_msi(struct pcie_port *pp)
 	irq_domain_remove(pp->msi_domain);
 	irq_domain_remove(pp->irq_domain);
 }
-#endif
 
 void dw_pcie_msi_init(struct pcie_port *pp)
 {
@@ -317,184 +302,6 @@ void dw_pcie_msi_init(struct pcie_port *pp)
 			    (u32)(msi_target >> 32 & 0xffffffff));
 }
 
-static void dw_pcie_msi_clear_irq(struct pcie_port *pp, int irq)
-{
-	unsigned int res, bit, val;
-
-	res = (irq / 32) * 12;
-	bit = irq % 32;
-	dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, &val);
-	val &= ~(1 << bit);
-	dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, val);
-}
-
-static void clear_irq_range(struct pcie_port *pp, unsigned int irq_base,
-			    unsigned int nvec, unsigned int pos)
-{
-	unsigned int i;
-
-	for (i = 0; i < nvec; i++) {
-		irq_set_msi_desc_off(irq_base, i, NULL);
-		/* Disable corresponding interrupt on MSI controller */
-		if (pp->ops->msi_clear_irq)
-			pp->ops->msi_clear_irq(pp, pos + i);
-		else
-			dw_pcie_msi_clear_irq(pp, pos + i);
-	}
-
-	bitmap_release_region(pp->msi_irq_in_use, pos, order_base_2(nvec));
-}
-
-static void dw_pcie_msi_set_irq(struct pcie_port *pp, int irq)
-{
-	unsigned int res, bit, val;
-
-	res = (irq / 32) * 12;
-	bit = irq % 32;
-	dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, &val);
-	val |= 1 << bit;
-	dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, val);
-}
-
-static int assign_irq(int no_irqs, struct msi_desc *desc, int *pos)
-{
-	int irq, pos0, i;
-	struct pcie_port *pp;
-
-	pp = (struct pcie_port *)msi_desc_to_pci_sysdata(desc);
-	pos0 = bitmap_find_free_region(pp->msi_irq_in_use, MAX_MSI_IRQS,
-				       order_base_2(no_irqs));
-	if (pos0 < 0)
-		goto no_valid_irq;
-
-	irq = irq_find_mapping(pp->irq_domain, pos0);
-	if (!irq)
-		goto no_valid_irq;
-
-	/*
-	 * irq_create_mapping (called from dw_pcie_host_init) pre-allocates
-	 * descs so there is no need to allocate descs here. We can therefore
-	 * assume that if irq_find_mapping above returns non-zero, then the
-	 * descs are also successfully allocated.
-	 */
-
-	for (i = 0; i < no_irqs; i++) {
-		if (irq_set_msi_desc_off(irq, i, desc) != 0) {
-			clear_irq_range(pp, irq, i, pos0);
-			goto no_valid_irq;
-		}
-		/*Enable corresponding interrupt in MSI interrupt controller */
-		if (pp->ops->msi_set_irq)
-			pp->ops->msi_set_irq(pp, pos0 + i);
-		else
-			dw_pcie_msi_set_irq(pp, pos0 + i);
-	}
-
-	*pos = pos0;
-	desc->nvec_used = no_irqs;
-	desc->msi_attrib.multiple = order_base_2(no_irqs);
-
-	return irq;
-
-no_valid_irq:
-	*pos = pos0;
-	return -ENOSPC;
-}
-
-static void dw_msi_setup_msg(struct pcie_port *pp, unsigned int irq, u32 pos)
-{
-	struct msi_msg msg;
-	u64 msi_target;
-
-	if (pp->ops->get_msi_addr)
-		msi_target = pp->ops->get_msi_addr(pp);
-	else
-		msi_target = virt_to_phys((void *)pp->msi_data);
-
-	msg.address_lo = (u32)(msi_target & 0xffffffff);
-	msg.address_hi = (u32)(msi_target >> 32 & 0xffffffff);
-
-	if (pp->ops->get_msi_data)
-		msg.data = pp->ops->get_msi_data(pp, pos);
-	else
-		msg.data = pos;
-
-	pci_write_msi_msg(irq, &msg);
-}
-
-static int dw_msi_setup_irq(struct msi_controller *chip, struct pci_dev *pdev,
-			    struct msi_desc *desc)
-{
-	int irq, pos;
-	struct pcie_port *pp = pdev->bus->sysdata;
-
-	if (desc->msi_attrib.is_msix)
-		return -EINVAL;
-
-	irq = assign_irq(1, desc, &pos);
-	if (irq < 0)
-		return irq;
-
-	dw_msi_setup_msg(pp, irq, pos);
-
-	return 0;
-}
-
-static int dw_msi_setup_irqs(struct msi_controller *chip, struct pci_dev *pdev,
-			     int nvec, int type)
-{
-#ifdef CONFIG_PCI_MSI
-	int irq, pos;
-	struct msi_desc *desc;
-	struct pcie_port *pp = pdev->bus->sysdata;
-
-	/* MSI-X interrupts are not supported */
-	if (type == PCI_CAP_ID_MSIX)
-		return -EINVAL;
-
-	WARN_ON(!list_is_singular(&pdev->dev.msi_list));
-	desc = list_entry(pdev->dev.msi_list.next, struct msi_desc, list);
-
-	irq = assign_irq(nvec, desc, &pos);
-	if (irq < 0)
-		return irq;
-
-	dw_msi_setup_msg(pp, irq, pos);
-
-	return 0;
-#else
-	return -EINVAL;
-#endif
-}
-
-static void dw_msi_teardown_irq(struct msi_controller *chip, unsigned int irq)
-{
-	struct irq_data *data = irq_get_irq_data(irq);
-	struct msi_desc *msi = irq_data_get_msi_desc(data);
-	struct pcie_port *pp = (struct pcie_port *)msi_desc_to_pci_sysdata(msi);
-
-	clear_irq_range(pp, irq, 1, data->hwirq);
-}
-
-static struct msi_controller dw_pcie_msi_chip = {
-	.setup_irq = dw_msi_setup_irq,
-	.setup_irqs = dw_msi_setup_irqs,
-	.teardown_irq = dw_msi_teardown_irq,
-};
-
-static int dw_pcie_msi_map(struct irq_domain *domain, unsigned int irq,
-			   irq_hw_number_t hwirq)
-{
-	irq_set_chip_and_handler(irq, &dw_msi_irq_chip, handle_simple_irq);
-	irq_set_chip_data(irq, domain->host_data);
-
-	return 0;
-}
-
-static const struct irq_domain_ops msi_domain_ops = {
-	.map = dw_pcie_msi_map,
-};
-
 int dw_pcie_host_init(struct pcie_port *pp)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
@@ -504,9 +311,6 @@ int dw_pcie_host_init(struct pcie_port *pp)
 	struct resource_entry *win, *tmp;
 	struct pci_bus *bus, *child;
 	struct resource *cfg_res;
-#ifndef CONFIG_PCIEDW_NEW_IRQ_API
-	int i;
-#endif
 	int ret;
 	LIST_HEAD(res);
 
@@ -610,19 +414,6 @@ int dw_pcie_host_init(struct pcie_port *pp)
 			if (ret)
 				pp->num_vectors = 1;
 
-#ifndef CONFIG_PCIEDW_NEW_IRQ_API
-			pp->irq_domain = irq_domain_add_linear(dev->of_node,
-						MAX_MSI_IRQS, &msi_domain_ops,
-						&dw_pcie_msi_chip);
-			if (!pp->irq_domain) {
-				dev_err(dev, "irq domain init failed\n");
-				ret = -ENXIO;
-				goto error;
-			}
-
-			for (i = 0; i < MAX_MSI_IRQS; i++)
-				irq_create_mapping(pp->irq_domain, i);
-#else
 			ret = dw_pcie_allocate_domains(pci);
 			if (ret)
 				goto error;
@@ -630,9 +421,8 @@ int dw_pcie_host_init(struct pcie_port *pp)
 			irq_set_chained_handler_and_data(pci->pp.msi_irq,
 							 dw_chained_msi_isr,
 							 pci);
-#endif
 		} else {
-			ret = pp->ops->msi_host_init(pci, &dw_pcie_msi_chip);
+			ret = pp->ops->msi_host_init(pci);
 			if (ret < 0)
 				goto error;
 		}
@@ -642,19 +432,9 @@ int dw_pcie_host_init(struct pcie_port *pp)
 		pp->ops->host_init(pp);
 
 	pp->root_bus_nr = pp->busn->start;
-#ifndef CONFIG_PCIEDW_NEW_IRQ_API
-	if (IS_ENABLED(CONFIG_PCI_MSI)) {
-		bus = pci_scan_root_bus_msi(dev, pp->root_bus_nr,
-					    &dw_pcie_ops, pp, &res,
-					    &dw_pcie_msi_chip);
-		dw_pcie_msi_chip.dev = dev;
-	} else
-		bus = pci_scan_root_bus(dev, pp->root_bus_nr, &dw_pcie_ops,
-					pp, &res);
-#else
+
 	bus = pci_scan_root_bus(dev, pp->root_bus_nr, &dw_pcie_ops,
 				pp, &res);
-#endif
 	if (!bus) {
 		ret = -ENOMEM;
 		goto error;
diff --git a/drivers/pci/dwc/pcie-designware.h b/drivers/pci/dwc/pcie-designware.h
index 9b621f9..e79c85d 100644
--- a/drivers/pci/dwc/pcie-designware.h
+++ b/drivers/pci/dwc/pcie-designware.h
@@ -140,7 +140,7 @@ struct dw_pcie_host_ops {
 	phys_addr_t (*get_msi_addr)(struct pcie_port *pp);
 	u32 (*get_msi_data)(struct pcie_port *pp, int pos);
 	void (*scan_bus)(struct pcie_port *pp);
-	int (*msi_host_init)(struct dw_pcie *pci, struct msi_controller *chip);
+	int (*msi_host_init)(struct dw_pcie *pci);
 	void (*msi_irq_ack)(int irq, struct pcie_port *pp);
 };
 
-- 
2.9.3

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

* Re: [RFC 1/8] pci: adding new irq api to pci-designware
  2017-05-12 16:56 ` [RFC 1/8] pci: adding new irq api to pci-designware Joao Pinto
@ 2017-05-12 17:02   ` Lucas Stach
  2017-05-12 17:36   ` Marc Zyngier
  1 sibling, 0 replies; 12+ messages in thread
From: Lucas Stach @ 2017-05-12 17:02 UTC (permalink / raw)
  To: Joao Pinto
  Cc: bhelgaas, marc.zyngier, jingoohan1, m-karicheri2,
	thomas.petazzoni, minghuan.Lian, mingkai.hu, tie-fei.zang,
	hongxing.zhu, niklas.cassel, jesper.nilsson, wangzhou1,
	gabriele.paoloni, svarbanov, linux-pci

Am Freitag, den 12.05.2017, 17:56 +0100 schrieb Joao Pinto:
> This patch adds the new interrupt api to pecie-designware, keeping the old
> one. A define was added to toggle between the two apis (for testing purposes).
> 
> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
> ---
>  drivers/pci/dwc/pcie-designware-host.c | 241 ++++++++++++++++++++++++++++++++-
>  drivers/pci/dwc/pcie-designware.h      |   5 +
>  2 files changed, 244 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c
> index 28ed32b..6810ea10b 100644
> --- a/drivers/pci/dwc/pcie-designware-host.c
> +++ b/drivers/pci/dwc/pcie-designware-host.c

[...]

> +static int dw_pcie_irq_domain_alloc(struct irq_domain *domain,
> +				    unsigned int virq, unsigned int nr_irqs,
> +				    void *args)
> +{
> +	struct dw_pcie *pci = domain->host_data;
> +	struct pcie_port *pp = &pci->pp;
> +	unsigned long bit;
> +
> +	mutex_lock(&pp->lock);
> +
> +	WARN_ON(nr_irqs != 1);

So the new API doesn't support multi MSI, where we need more than a
single contiguous MSI in a range? If this is the case then this change
is breaking real use-cases that work today.

Regards,
Luvas

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

* Re: [RFC 1/8] pci: adding new irq api to pci-designware
  2017-05-12 16:56 ` [RFC 1/8] pci: adding new irq api to pci-designware Joao Pinto
  2017-05-12 17:02   ` Lucas Stach
@ 2017-05-12 17:36   ` Marc Zyngier
  2017-05-15 10:32     ` Joao Pinto
  1 sibling, 1 reply; 12+ messages in thread
From: Marc Zyngier @ 2017-05-12 17:36 UTC (permalink / raw)
  To: Joao Pinto, bhelgaas
  Cc: jingoohan1, m-karicheri2, thomas.petazzoni, minghuan.Lian,
	mingkai.hu, tie-fei.zang, hongxing.zhu, l.stach, niklas.cassel,
	jesper.nilsson, wangzhou1, gabriele.paoloni, svarbanov, linux-pci

Joao,

You seem to have dropped a number of things I had mentioned on my
previous review of this patch. See below:

On 12/05/17 17:56, Joao Pinto wrote:
> This patch adds the new interrupt api to pecie-designware, keeping the old
> one. A define was added to toggle between the two apis (for testing purposes).
> 
> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
> ---
>  drivers/pci/dwc/pcie-designware-host.c | 241 ++++++++++++++++++++++++++++++++-
>  drivers/pci/dwc/pcie-designware.h      |   5 +
>  2 files changed, 244 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c
> index 28ed32b..6810ea10b 100644
> --- a/drivers/pci/dwc/pcie-designware-host.c
> +++ b/drivers/pci/dwc/pcie-designware-host.c
> @@ -11,6 +11,7 @@
>   * published by the Free Software Foundation.
>   */
>  
> +#include <linux/irqchip/chained_irq.h>
>  #include <linux/irqdomain.h>
>  #include <linux/of_address.h>
>  #include <linux/of_pci.h>
> @@ -19,6 +20,9 @@
>  
>  #include "pcie-designware.h"
>  
> +/* Temporary: Activate to use new IRQ API */
> +/*#define CONFIG_PCIEDW_NEW_IRQ_API*/

You should be able to support both APIs at the same time until the last
patch, without relying on a kernel configuration (we still want to be
able to boot a single kernel that works both with the old and new methods).

> +
>  static struct pci_ops dw_pcie_ops;
>  
>  static int dw_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
> @@ -45,6 +49,7 @@ static int dw_pcie_wr_own_conf(struct pcie_port *pp, int where, int size,
>  	return dw_pcie_write(pci->dbi_base + where, size, val);
>  }
>  
> +#ifndef CONFIG_PCIEDW_NEW_IRQ_API
>  static struct irq_chip dw_msi_irq_chip = {
>  	.name = "PCI-MSI",
>  	.irq_enable = pci_msi_unmask_irq,
> @@ -52,6 +57,29 @@ static struct irq_chip dw_msi_irq_chip = {
>  	.irq_mask = pci_msi_mask_irq,
>  	.irq_unmask = pci_msi_unmask_irq,
>  };
> +#else
> +static void dw_msi_mask_irq(struct irq_data *d) {
> +	pci_msi_mask_irq(d);
> +	irq_chip_mask_parent(d);
> +}
> +
> +static void dw_msi_unmask_irq(struct irq_data *d) {
> +	pci_msi_unmask_irq(d);
> +	irq_chip_unmask_parent(d);
> +}
> +
> +static struct irq_chip dw_msi_irq_chip = {
> +	.name = "PCI-MSI",
> +	.irq_mask = dw_msi_mask_irq,
> +	.irq_unmask = dw_msi_unmask_irq,
> +};
> +
> +static struct msi_domain_info dw_pcie_msi_domain_info = {
> +	.flags	= (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
> +		     MSI_FLAG_PCI_MSIX),
> +	.chip	= &dw_msi_irq_chip,
> +};
> +#endif
>  
>  /* MSI int handler */
>  irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
> @@ -81,6 +109,190 @@ irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
>  	return ret;
>  }
>  
> +#ifdef CONFIG_PCIEDW_NEW_IRQ_API
> +/* Chained MSI interrupt service routine */
> +static void dw_chained_msi_isr(struct irq_desc *desc)
> +{
> +	struct irq_chip *chip = irq_desc_get_chip(desc);
> +	struct pcie_port *pp;
> +	struct dw_pcie *pci;
> +
> +	chained_irq_enter(chip, desc);
> +	pci = irq_desc_get_handler_data(desc);
> +	pp = &pci->pp;
> +
> +	dw_handle_msi_irq(pp);
> +
> +	chained_irq_exit(chip, desc);
> +}
> +
> +static void dw_pci_setup_msi_msg(struct irq_data *data, struct msi_msg *msg)
> +{
> +	struct dw_pcie *pci = irq_data_get_irq_chip_data(data);
> +	struct pcie_port *pp = &pci->pp;
> +	u64 msi_target;
> +
> +	if (pp->ops->get_msi_addr)
> +		msi_target = pp->ops->get_msi_addr(pp);
> +	else
> +		msi_target = virt_to_phys((void *)pp->msi_data);
> +
> +	msg->address_lo = (u32)(msi_target & 0xffffffff);
> +	msg->address_hi = (u32)(msi_target >> 32 & 0xffffffff);

Use lower_32_bit/upper_32_bit macros.

> +
> +	if (pp->ops->get_msi_data)
> +		msg->data = pp->ops->get_msi_data(pp, data->hwirq);
> +	else
> +		msg->data = data->hwirq;
> +
> +	dev_dbg(pci->dev, "msi#%d address_hi %#x address_lo %#x\n",
> +		(int)data->hwirq, msg->address_hi, msg->address_lo);
> +}
> +
> +static int dw_pci_msi_set_affinity(struct irq_data *irq_data,
> +				   const struct cpumask *mask, bool force)
> +{
> +	return -EINVAL;
> +}
> +
> +static void dw_pci_bottom_mask(struct irq_data *data)
> +{
> +	struct dw_pcie *pci = irq_data_get_irq_chip_data(data);
> +	struct pcie_port *pp = &pci->pp;
> +	unsigned int res, bit, val;
> +
> +	mutex_lock(&pp->lock);

This is wrong. You can also end-up here from interrupt context, so you
need an spin_lock_irqsave().

> +
> +	if (pp->ops->msi_clear_irq)
> +		pp->ops->msi_clear_irq(pp, data->hwirq);

Why do you need this? Can you have alternative implementations of
mask/unmask?

> +	else {
> +		res = (data->hwirq / 32) * 12;
> +		bit = data->hwirq % 32;
> +
> +		dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, &val);

No use reading back from the HW. Just maintain a cached version in your
private structure and just use that.

> +		val &= ~(1 << bit);
> +		dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, val);
> +	}
> +
> +	mutex_unlock(&pp->lock);
> +}
> +
> +static void dw_pci_bottom_unmask(struct irq_data *data)
> +{
> +	struct dw_pcie *pci = irq_data_get_irq_chip_data(data);
> +	struct pcie_port *pp = &pci->pp;
> +	unsigned int res, bit, val;
> +
> +	mutex_lock(&pp->lock);
> +
> +	if (pp->ops->msi_set_irq)
> +		pp->ops->msi_set_irq(pp, data->hwirq);
> +	else {
> +		res = (data->hwirq / 32) * 12;
> +		bit = data->hwirq % 32;
> +
> +		dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, &val);
> +		val |= 1 << bit;
> +		dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, val);
> +	}
> +
> +	mutex_unlock(&pp->lock);

Same comments.

> +}
> +
> +static struct irq_chip dw_pci_msi_bottom_irq_chip = {
> +	.name			= "DWPCI-MSI",
> +	.irq_compose_msi_msg	= dw_pci_setup_msi_msg,
> +	.irq_set_affinity	= dw_pci_msi_set_affinity,
> +	.irq_mask		= dw_pci_bottom_mask,
> +	.irq_unmask		= dw_pci_bottom_unmask,
> +};
> +
> +static int dw_pcie_irq_domain_alloc(struct irq_domain *domain,
> +				    unsigned int virq, unsigned int nr_irqs,
> +				    void *args)
> +{
> +	struct dw_pcie *pci = domain->host_data;
> +	struct pcie_port *pp = &pci->pp;
> +	unsigned long bit;
> +
> +	mutex_lock(&pp->lock);
> +
> +	WARN_ON(nr_irqs != 1);
> +
> +	bit = find_first_zero_bit(pp->msi_irq_in_use, pp->num_vectors);
> +	if (bit >= pp->num_vectors) {
> +		mutex_unlock(&pp->lock);
> +		return -ENOSPC;
> +	}
> +
> +	set_bit(bit, pp->msi_irq_in_use);
> +
> +	mutex_unlock(&pp->lock);

As mentioned by Lucas, you probably want to support Multi-MSI.

> +
> +	irq_domain_set_info(domain, virq, bit, &dw_pci_msi_bottom_irq_chip,
> +			    domain->host_data, handle_simple_irq,
> +			    NULL, NULL);
> +
> +	return 0;
> +}
> +
> +static void dw_pcie_irq_domain_free(struct irq_domain *domain,
> +				    unsigned int virq, unsigned int nr_irqs)
> +{
> +	struct irq_data *data = irq_domain_get_irq_data(domain, virq);
> +	struct dw_pcie *pci = irq_data_get_irq_chip_data(data);
> +	struct pcie_port *pp = &pci->pp;
> +
> +	mutex_lock(&pp->lock);
> +
> +	if (!test_bit(data->hwirq, pp->msi_irq_in_use))
> +		dev_err(pci->dev, "trying to free unused MSI#%lu\n",
> +			data->hwirq);
> +	else
> +		__clear_bit(data->hwirq, pp->msi_irq_in_use);
> +
> +	mutex_unlock(&pp->lock);

Same here.

> +}
> +
> +static const struct irq_domain_ops dw_pcie_msi_domain_ops = {
> +	.alloc	= dw_pcie_irq_domain_alloc,
> +	.free	= dw_pcie_irq_domain_free,
> +};
> +
> +int dw_pcie_allocate_domains(struct dw_pcie *pci)
> +{
> +	struct pcie_port *pp = &pci->pp;
> +	struct fwnode_handle *fwnode = of_node_to_fwnode(pci->dev->of_node);
> +
> +	pp->irq_domain = irq_domain_add_linear(NULL, pp->num_vectors,
> +					       &dw_pcie_msi_domain_ops, pci);

Please use irq_domain_create_linear and pass the fwnode there as well.

> +	if (!pp->irq_domain) {
> +		dev_err(pci->dev, "failed to create IRQ domain\n");
> +		return -ENOMEM;
> +	}
> +
> +	pp->msi_domain = pci_msi_create_irq_domain(fwnode,
> +						   &dw_pcie_msi_domain_info,
> +						   pp->irq_domain);
> +	if (!pp->msi_domain) {
> +		dev_err(pci->dev, "failed to create MSI domain\n");
> +		irq_domain_remove(pp->irq_domain);
> +		return -ENOMEM;
> +	}
> +
> +	return 0;
> +}
> +
> +void dw_pcie_free_msi(struct pcie_port *pp)
> +{
> +	irq_set_chained_handler(pp->msi_irq, NULL);
> +	irq_set_handler_data(pp->msi_irq, NULL);
> +
> +	irq_domain_remove(pp->msi_domain);
> +	irq_domain_remove(pp->irq_domain);
> +}
> +#endif
> +
>  void dw_pcie_msi_init(struct pcie_port *pp)
>  {
>  	u64 msi_target;
> @@ -279,11 +491,16 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  	struct device *dev = pci->dev;
>  	struct device_node *np = dev->of_node;
>  	struct platform_device *pdev = to_platform_device(dev);
> +	struct resource_entry *win, *tmp;
>  	struct pci_bus *bus, *child;
>  	struct resource *cfg_res;
> -	int i, ret;
> +#ifndef CONFIG_PCIEDW_NEW_IRQ_API
> +	int i;
> +#endif
> +	int ret;
>  	LIST_HEAD(res);
> -	struct resource_entry *win, *tmp;
> +
> +	mutex_init(&pci->pp.lock);
>  
>  	cfg_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
>  	if (cfg_res) {
> @@ -378,6 +595,12 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  
>  	if (IS_ENABLED(CONFIG_PCI_MSI)) {
>  		if (!pp->ops->msi_host_init) {
> +			ret = of_property_read_u32(np, "num-vectors",
> +						   &pp->num_vectors);
> +			if (ret)
> +				pp->num_vectors = 1;
> +
> +#ifndef CONFIG_PCIEDW_NEW_IRQ_API
>  			pp->irq_domain = irq_domain_add_linear(dev->of_node,
>  						MAX_MSI_IRQS, &msi_domain_ops,
>  						&dw_pcie_msi_chip);
> @@ -389,6 +612,15 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  
>  			for (i = 0; i < MAX_MSI_IRQS; i++)
>  				irq_create_mapping(pp->irq_domain, i);
> +#else
> +			ret = dw_pcie_allocate_domains(pci);
> +			if (ret)
> +				goto error;
> +
> +			irq_set_chained_handler_and_data(pci->pp.msi_irq,
> +							 dw_chained_msi_isr,
> +							 pci);
> +#endif
>  		} else {
>  			ret = pp->ops->msi_host_init(pp, &dw_pcie_msi_chip);
>  			if (ret < 0)
> @@ -400,6 +632,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  		pp->ops->host_init(pp);
>  
>  	pp->root_bus_nr = pp->busn->start;
> +#ifndef CONFIG_PCIEDW_NEW_IRQ_API
>  	if (IS_ENABLED(CONFIG_PCI_MSI)) {
>  		bus = pci_scan_root_bus_msi(dev, pp->root_bus_nr,
>  					    &dw_pcie_ops, pp, &res,
> @@ -408,6 +641,10 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  	} else
>  		bus = pci_scan_root_bus(dev, pp->root_bus_nr, &dw_pcie_ops,
>  					pp, &res);
> +#else
> +	bus = pci_scan_root_bus(dev, pp->root_bus_nr, &dw_pcie_ops,
> +				pp, &res);
> +#endif
>  	if (!bus) {
>  		ret = -ENOMEM;
>  		goto error;
> diff --git a/drivers/pci/dwc/pcie-designware.h b/drivers/pci/dwc/pcie-designware.h
> index c6a8405..15481a5 100644
> --- a/drivers/pci/dwc/pcie-designware.h
> +++ b/drivers/pci/dwc/pcie-designware.h
> @@ -165,7 +165,10 @@ struct pcie_port {
>  	struct dw_pcie_host_ops	*ops;
>  	int			msi_irq;
>  	struct irq_domain	*irq_domain;
> +	struct irq_domain	*msi_domain;
>  	unsigned long		msi_data;
> +	u32			num_vectors;
> +	struct mutex		lock;
>  	DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
>  };
>  
> @@ -282,8 +285,10 @@ static inline u32 dw_pcie_readl_dbi2(struct dw_pcie *pci, u32 reg)
>  #ifdef CONFIG_PCIE_DW_HOST
>  irqreturn_t dw_handle_msi_irq(struct pcie_port *pp);
>  void dw_pcie_msi_init(struct pcie_port *pp);
> +void dw_pcie_free_msi(struct pcie_port *pp);
>  void dw_pcie_setup_rc(struct pcie_port *pp);
>  int dw_pcie_host_init(struct pcie_port *pp);
> +int dw_pcie_allocate_domains(struct dw_pcie *pci);
>  #else
>  static inline irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
>  {
> 

Thanks,

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

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

* Re: [RFC 1/8] pci: adding new irq api to pci-designware
  2017-05-12 17:36   ` Marc Zyngier
@ 2017-05-15 10:32     ` Joao Pinto
  0 siblings, 0 replies; 12+ messages in thread
From: Joao Pinto @ 2017-05-15 10:32 UTC (permalink / raw)
  To: Marc Zyngier, Joao Pinto, bhelgaas, l.stach
  Cc: jingoohan1, m-karicheri2, thomas.petazzoni, minghuan.Lian,
	mingkai.hu, tie-fei.zang, hongxing.zhu, niklas.cassel,
	jesper.nilsson, wangzhou1, gabriele.paoloni, svarbanov, linux-pci


Hi Mark and Lucas,

Às 6:36 PM de 5/12/2017, Marc Zyngier escreveu:
> Joao,
> 
> You seem to have dropped a number of things I had mentioned on my
> previous review of this patch. See below:
> 
> On 12/05/17 17:56, Joao Pinto wrote:
>> This patch adds the new interrupt api to pecie-designware, keeping the old
>> one. A define was added to toggle between the two apis (for testing purposes).
>>
>> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
>> ---
>>  drivers/pci/dwc/pcie-designware-host.c | 241 ++++++++++++++++++++++++++++++++-
>>  drivers/pci/dwc/pcie-designware.h      |   5 +
>>  2 files changed, 244 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c
>> index 28ed32b..6810ea10b 100644
>> --- a/drivers/pci/dwc/pcie-designware-host.c
>> +++ b/drivers/pci/dwc/pcie-designware-host.c
>> @@ -11,6 +11,7 @@
>>   * published by the Free Software Foundation.
>>   */
>>  
>> +#include <linux/irqchip/chained_irq.h>
>>  #include <linux/irqdomain.h>
>>  #include <linux/of_address.h>
>>  #include <linux/of_pci.h>
>> @@ -19,6 +20,9 @@
>>  
>>  #include "pcie-designware.h"
>>  
>> +/* Temporary: Activate to use new IRQ API */
>> +/*#define CONFIG_PCIEDW_NEW_IRQ_API*/
> 
> You should be able to support both APIs at the same time until the last
> patch, without relying on a kernel configuration (we still want to be
> able to boot a single kernel that works both with the old and new methods).

I think that the APIs are mutual-exclusive. pcie-designware has to initialize of
one of them. Your idea is to have the new and the old API functions, but only
initialize pcie-designware with the old one until the last patch?

[...]

>> +
>> +	msg->address_lo = (u32)(msi_target & 0xffffffff);
>> +	msg->address_hi = (u32)(msi_target >> 32 & 0xffffffff);
> 
> Use lower_32_bit/upper_32_bit macros.

Right, I am going to update this.

[...]

>> +
>> +static void dw_pci_bottom_mask(struct irq_data *data)
>> +{
>> +	struct dw_pcie *pci = irq_data_get_irq_chip_data(data);
>> +	struct pcie_port *pp = &pci->pp;
>> +	unsigned int res, bit, val;
>> +
>> +	mutex_lock(&pp->lock);
> 
> This is wrong. You can also end-up here from interrupt context, so you
> need an spin_lock_irqsave().

Right, I am going to update this.

[...]

> 
>> +
>> +	if (pp->ops->msi_clear_irq)
>> +		pp->ops->msi_clear_irq(pp, data->hwirq);
> 
> Why do you need this? Can you have alternative implementations of
> mask/unmask?

msi_clear_irq and msi_set_irq are specific SoC operations to set and clear bits
in the interrupt register. In my opinion we should keep the current callbacks.

[...]

> 
>> +	else {
>> +		res = (data->hwirq / 32) * 12;
>> +		bit = data->hwirq % 32;
>> +
>> +		dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, &val);
> 
> No use reading back from the HW. Just maintain a cached version in your
> private structure and just use that.

Ok, I can do that.

[...]

>> +
>> +	bit = find_first_zero_bit(pp->msi_irq_in_use, pp->num_vectors);
>> +	if (bit >= pp->num_vectors) {
>> +		mutex_unlock(&pp->lock);
>> +		return -ENOSPC;
>> +	}
>> +
>> +	set_bit(bit, pp->msi_irq_in_use);
>> +
>> +	mutex_unlock(&pp->lock);
> 
> As mentioned by Lucas, you probably want to support Multi-MSI.
> 

This could be achieved by declaring data for each virq, correct?

for (i = 0; i < nr_irqs; i++) {
irq_domain_set_info(domain, virq + i, bit + i,
&dw_pci_msi_bottom_irq_chip,domain->host_data, handle_simple_irq,
NULL, NULL);
}

[...]

>> +
>> +	pp->irq_domain = irq_domain_add_linear(NULL, pp->num_vectors,
>> +					       &dw_pcie_msi_domain_ops, pci);
> 
> Please use irq_domain_create_linear and pass the fwnode there as well.
> 

Ok, I will do that.

[...]

>>  static inline irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
>>  {
>>
> 
> Thanks,
> 
> 	M.
> 

Thank you for you feeback!

Joao

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

end of thread, other threads:[~2017-05-15 10:32 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-05-12 16:56 [RFC 0/8] add new irq api to pcie-designware Joao Pinto
2017-05-12 16:56 ` [RFC 1/8] pci: adding new irq api to pci-designware Joao Pinto
2017-05-12 17:02   ` Lucas Stach
2017-05-12 17:36   ` Marc Zyngier
2017-05-15 10:32     ` Joao Pinto
2017-05-12 16:56 ` [RFC 2/8] pci: exynos SoC driver adapted to new irq API Joao Pinto
2017-05-12 16:56 ` [RFC 3/8] pci: imx6 " Joao Pinto
2017-05-12 16:56 ` [RFC 4/8] pci: artpec6 " Joao Pinto
2017-05-12 16:56 ` [RFC 5/8] pci: generic PCIe DW " Joao Pinto
2017-05-12 16:56 ` [RFC 6/8] pci: qcom SoC " Joao Pinto
2017-05-12 16:56 ` [RFC 7/8] pci: keystone " Joao Pinto
2017-05-12 16:56 ` [RFC 8/8] pci: removing old irq api from pcie-designware Joao Pinto

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).