[PATCH v2 0/9] add new irq api to pcie-designware

[PATCH v2 0/9] add new irq api to pcie-designware

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

The patch set was made against the Bjorn' next branch.

Joao Pinto (9):
  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
  pci: remove limitation of the number of the available IRQs

 drivers/pci/dwc/pci-exynos.c           |  18 --
 drivers/pci/dwc/pci-imx6.c             |  18 --
 drivers/pci/dwc/pci-keystone-dw.c      |  96 +-------
 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 | 395 +++++++++++++++++++--------------
 drivers/pci/dwc/pcie-designware-plat.c |  15 --
 drivers/pci/dwc/pcie-designware.h      |  30 ++-
 drivers/pci/dwc/pcie-qcom.c            |  15 --
 11 files changed, 255 insertions(+), 359 deletions(-)

-- 
2.9.3

[PATCH v2 1/9] pci: adding new irq api to pci-designware

[Joao Pinto]

This patch adds the new interrupt api to pcie-designware, keeping the old
one. Although the old API is still available, pcie-designware initiates with
the new one.

Signed-off-by: Joao Pinto <jpinto@synopsys.com>
---
Change v1->v2:
- num_vectors is now not configurable by DT. Now it is 32 by default and
can be overiden by any specific SoC driver.

 drivers/pci/dwc/pcie-designware-host.c | 291 +++++++++++++++++++++++++++++----
 drivers/pci/dwc/pcie-designware.h      |  17 ++
 2 files changed, 277 insertions(+), 31 deletions(-)

diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c
index 28ed32b..b203754 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>
@@ -53,6 +54,30 @@ static struct irq_chip dw_msi_irq_chip = {
 	.irq_unmask = pci_msi_unmask_irq,
 };
 
+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_pcie_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 | MSI_FLAG_MULTI_PCI_MSI),
+	.chip	= &dw_pcie_msi_irq_chip,
+};
+
 /* MSI int handler */
 irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
 {
@@ -81,6 +106,191 @@ irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
 	return ret;
 }
 
+/* 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 = lower_32_bits(msi_target);
+	msg->address_hi = upper_32_bits(msi_target);
+
+	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, ctrl;
+	unsigned long flags;
+
+	spin_lock_irqsave(&pp->lock, flags);
+
+	if (pp->ops->msi_clear_irq)
+		pp->ops->msi_clear_irq(pp, data->hwirq);
+	else {
+		ctrl = data->hwirq / 32;
+		res = ctrl * 12;
+		bit = data->hwirq % 32;
+
+		pp->irq_status[ctrl] &= ~(1 << bit);
+		dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4,
+				    pp->irq_status[ctrl]);
+	}
+
+	spin_unlock_irqrestore(&pp->lock, flags);
+}
+
+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, ctrl;
+	unsigned long flags;
+
+	spin_lock_irqsave(&pp->lock, flags);
+
+	if (pp->ops->msi_set_irq)
+		pp->ops->msi_set_irq(pp, data->hwirq);
+	else {
+		ctrl = data->hwirq / 32;
+		res = ctrl * 12;
+		bit = data->hwirq % 32;
+
+		pp->irq_status[ctrl] |= 1 << bit;
+		dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4,
+				    pp->irq_status[ctrl]);
+	}
+
+	spin_unlock_irqrestore(&pp->lock, flags);
+}
+
+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 flags;
+	unsigned long bit;
+	u32 i;
+
+	spin_lock_irqsave(&pp->lock, flags);
+
+	bit = bitmap_find_next_zero_area(pp->msi_irq_in_use, pp->num_vectors, 0,
+					 nr_irqs, 0);
+
+	if (bit >= pp->num_vectors) {
+		spin_unlock_irqrestore(&pp->lock, flags);
+		return -ENOSPC;
+	}
+
+	bitmap_set(pp->msi_irq_in_use, bit, nr_irqs);
+
+	spin_unlock_irqrestore(&pp->lock, flags);
+
+	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);
+
+	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;
+	unsigned long flags;
+
+	spin_lock_irqsave(&pp->lock, flags);
+	bitmap_clear(pp->msi_irq_in_use, data->hwirq, nr_irqs);
+	spin_unlock_irqrestore(&pp->lock, flags);
+}
+
+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_create_linear(fwnode, 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);
+}
+
 void dw_pcie_msi_init(struct pcie_port *pp)
 {
 	u64 msi_target;
@@ -90,20 +300,21 @@ void dw_pcie_msi_init(struct pcie_port *pp)
 
 	/* program the msi_data */
 	dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_LO, 4,
-			    (u32)(msi_target & 0xffffffff));
+			    lower_32_bits(msi_target));
 	dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_HI, 4,
-			    (u32)(msi_target >> 32 & 0xffffffff));
+			    upper_32_bits(msi_target));
 }
 
 static void dw_pcie_msi_clear_irq(struct pcie_port *pp, int irq)
 {
-	unsigned int res, bit, val;
+	unsigned int res, bit, ctrl;
 
-	res = (irq / 32) * 12;
+	ctrl = irq / 32;
+	res = ctrl * 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);
+	pp->irq_status[ctrl] &= ~(1 << bit);
+	dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4,
+			    pp->irq_status[ctrl]);
 }
 
 static void clear_irq_range(struct pcie_port *pp, unsigned int irq_base,
@@ -125,13 +336,14 @@ static void clear_irq_range(struct pcie_port *pp, unsigned int irq_base,
 
 static void dw_pcie_msi_set_irq(struct pcie_port *pp, int irq)
 {
-	unsigned int res, bit, val;
+	unsigned int res, bit, ctrl;
 
-	res = (irq / 32) * 12;
+	ctrl = irq / 32;
+	res = ctrl * 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);
+	pp->irq_status[ctrl] |= 1 << bit;
+	dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4,
+			    pp->irq_status[ctrl]);
 }
 
 static int assign_irq(int no_irqs, struct msi_desc *desc, int *pos)
@@ -279,11 +491,14 @@ 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;
+
 	LIST_HEAD(res);
-	struct resource_entry *win, *tmp;
+
+	spin_lock_init(&pci->pp.lock);
 
 	cfg_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
 	if (cfg_res) {
@@ -377,18 +592,32 @@ int dw_pcie_host_init(struct pcie_port *pp)
 		pci->num_viewport = 2;
 
 	if (IS_ENABLED(CONFIG_PCI_MSI)) {
-		if (!pp->ops->msi_host_init) {
-			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;
+		/*
+		 * If a specific SoC driver needs to change the
+		 * default number of vectors, it needs to implement
+		 * the set_num_vectors callback.
+		 */
+		if (!pp->ops->set_num_vectors) {
+			pp->num_vectors = MSI_DEF_NUM_VECTORS;
+		} else {
+			pp->ops->set_num_vectors(pp);
+
+			if (pp->num_vectors > MAX_MSI_IRQS ||
+			    pp->num_vectors == 0) {
+				dev_err(dev,
+					"Invalid number of vectors\n");
 				goto error;
 			}
+		}
 
-			for (i = 0; i < MAX_MSI_IRQS; i++)
-				irq_create_mapping(pp->irq_domain, i);
+		if (!pp->ops->msi_host_init) {
+			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);
 		} else {
 			ret = pp->ops->msi_host_init(pp, &dw_pcie_msi_chip);
 			if (ret < 0)
@@ -400,14 +629,9 @@ int dw_pcie_host_init(struct pcie_port *pp)
 		pp->ops->host_init(pp);
 
 	pp->root_bus_nr = pp->busn->start;
-	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);
+
+	bus = pci_scan_root_bus(dev, pp->root_bus_nr, &dw_pcie_ops,
+				pp, &res);
 	if (!bus) {
 		ret = -ENOMEM;
 		goto error;
@@ -579,11 +803,16 @@ static u8 dw_pcie_iatu_unroll_enabled(struct dw_pcie *pci)
 
 void dw_pcie_setup_rc(struct pcie_port *pp)
 {
-	u32 val;
+	u32 val, ctrl;
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
 
 	dw_pcie_setup(pci);
 
+	/* Initialize IRQ Status array */
+	for (ctrl = 0; ctrl < MAX_MSI_CTRLS; ctrl++)
+		dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_ENABLE + (ctrl * 12), 4,
+				    &pp->irq_status[ctrl]);
+
 	/* setup RC BARs */
 	dw_pcie_writel_dbi(pci, PCI_BASE_ADDRESS_0, 0x00000004);
 	dw_pcie_writel_dbi(pci, PCI_BASE_ADDRESS_1, 0x00000000);
diff --git a/drivers/pci/dwc/pcie-designware.h b/drivers/pci/dwc/pcie-designware.h
index c6a8405..9838d6d 100644
--- a/drivers/pci/dwc/pcie-designware.h
+++ b/drivers/pci/dwc/pcie-designware.h
@@ -109,6 +109,7 @@
  */
 #define MAX_MSI_IRQS			32
 #define MAX_MSI_CTRLS			(MAX_MSI_IRQS / 32)
+#define MSI_DEF_NUM_VECTORS		32
 
 struct pcie_port;
 struct dw_pcie;
@@ -140,6 +141,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);
+	void (*set_num_vectors)(struct pcie_port *pp);
 	int (*msi_host_init)(struct pcie_port *pp, struct msi_controller *chip);
 };
 
@@ -165,7 +167,11 @@ 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;
+	u32			irq_status[MAX_MSI_CTRLS];
+	spinlock_t		lock;
 	DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
 };
 
@@ -282,8 +288,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)
 {
@@ -294,6 +302,10 @@ static inline void dw_pcie_msi_init(struct pcie_port *pp)
 {
 }
 
+static inline void dw_pcie_free_msi(struct pcie_port *pp)
+{
+}
+
 static inline void dw_pcie_setup_rc(struct pcie_port *pp)
 {
 }
@@ -302,6 +314,11 @@ static inline int dw_pcie_host_init(struct pcie_port *pp)
 {
 	return 0;
 }
+
+static inline int dw_pcie_allocate_domains(struct dw_pcie *pci)
+{
+	return 0;
+}
 #endif
 
 #ifdef CONFIG_PCIE_DW_EP
-- 
2.9.3

Re: [PATCH v2 1/9] pci: adding new irq api to pci-designware

[Lucas Stach]

Am Montag, den 05.06.2017, 17:19 +0100 schrieb Joao Pinto:
> This patch adds the new interrupt api to pcie-designware, keeping the old
> one. Although the old API is still available, pcie-designware initiates with
> the new one.
> 
> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
> ---
> Change v1->v2:
> - num_vectors is now not configurable by DT. Now it is 32 by default and
> can be overiden by any specific SoC driver.
> 
>  drivers/pci/dwc/pcie-designware-host.c | 291 +++++++++++++++++++++++++++++----
>  drivers/pci/dwc/pcie-designware.h      |  17 ++
>  2 files changed, 277 insertions(+), 31 deletions(-)
> 
[...]
>  static int assign_irq(int no_irqs, struct msi_desc *desc, int *pos)
> @@ -279,11 +491,14 @@ 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;
> +
>  	LIST_HEAD(res);
> -	struct resource_entry *win, *tmp;
> +
> +	spin_lock_init(&pci->pp.lock);
>  
>  	cfg_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
>  	if (cfg_res) {
> @@ -377,18 +592,32 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  		pci->num_viewport = 2;
>  
>  	if (IS_ENABLED(CONFIG_PCI_MSI)) {
> -		if (!pp->ops->msi_host_init) {
> -			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;
> +		/*
> +		 * If a specific SoC driver needs to change the
> +		 * default number of vectors, it needs to implement
> +		 * the set_num_vectors callback.
> +		 */
No need for a function to implement this. The implementation glue driver
can just set up pcie_port.num_vectors to the correct number before
calling dw_pcie_host_init.

> +		if (!pp->ops->set_num_vectors) {
> +			pp->num_vectors = MSI_DEF_NUM_VECTORS;
> +		} else {
> +			pp->ops->set_num_vectors(pp);
> +
> +			if (pp->num_vectors > MAX_MSI_IRQS ||
> +			    pp->num_vectors == 0) {
> +				dev_err(dev,
> +					"Invalid number of vectors\n");
>  				goto error;
>  			}
> +		}
>  
> -			for (i = 0; i < MAX_MSI_IRQS; i++)
> -				irq_create_mapping(pp->irq_domain, i);
> +		if (!pp->ops->msi_host_init) {
> +			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);

So this breaks legacy PCI irqs on DWC platforms even more, as the IRQ
line can not be shared, even if the endpoint device doesn't support
MSIs.

Do we have any solution for that? In the worst case we would need a DT
property to enable MSI support and not register the MSI IRQ domain in
that case.

Regards,
Lucas

>  		} else {
>  			ret = pp->ops->msi_host_init(pp, &dw_pcie_msi_chip);
>  			if (ret < 0)
> @@ -400,14 +629,9 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  		pp->ops->host_init(pp);
>  
>  	pp->root_bus_nr = pp->busn->start;
> -	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);
> +
> +	bus = pci_scan_root_bus(dev, pp->root_bus_nr, &dw_pcie_ops,
> +				pp, &res);
>  	if (!bus) {
>  		ret = -ENOMEM;
>  		goto error;
> @@ -579,11 +803,16 @@ static u8 dw_pcie_iatu_unroll_enabled(struct dw_pcie *pci)

[PATCH v2 2/9] pci: exynos SoC driver adapted to new irq API

[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>
---
Change v1->v2:
- Nothing changed, just to follow the patch set version.

 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

[PATCH v2 3/9] pci: imx6 SoC driver adapted to new irq API

[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>
---
Change v1->v2:
- Nothing changed, just to follow the patch set version.

 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

[PATCH v2 4/9] pci: artpec6 SoC driver adapted to new irq API

[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>
---
Change v1->v2:
- Nothing changed, just to follow the patch set version.

 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

[PATCH v2 5/9] pci: generic PCIe DW driver adapted to new irq API

[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>
---
Change v1->v2:
- Nothing changed, just to follow the patch set version.

 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

[PATCH v2 6/9] pci: qcom SoC driver adapted to new irq API

[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>
---
Change v1->v2:
- Nothing changed, just to follow the patch set version.

 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 289cda1..c9d24a4 100644
--- a/drivers/pci/dwc/pcie-qcom.c
+++ b/drivers/pci/dwc/pcie-qcom.c
@@ -145,13 +145,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 int qcom_pcie_establish_link(struct qcom_pcie *pcie)
 {
 	struct dw_pcie *pci = pcie->pci;
@@ -1029,14 +1022,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

[PATCH v2 7/9] pci: keystone SoC driver adapted to new irq API

[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>
---
Change v1->v2:
- Removed hardcoded num_vectors configuration (now it is done in the core
driver by default)

 drivers/pci/dwc/pci-keystone-dw.c      | 96 ++--------------------------------
 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, 21 insertions(+), 98 deletions(-)

diff --git a/drivers/pci/dwc/pci-keystone-dw.c b/drivers/pci/dwc/pci-keystone-dw.c
index 8bc626e..1b7314d 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,9 @@ 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);
-}
-
-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 b203754..feeee6a 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)
 	spin_unlock_irqrestore(&pp->lock, flags);
 }
 
+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,
@@ -619,7 +629,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
 							 dw_chained_msi_isr,
 							 pci);
 		} 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 9838d6d..7385b4e 100644
--- a/drivers/pci/dwc/pcie-designware.h
+++ b/drivers/pci/dwc/pcie-designware.h
@@ -142,7 +142,8 @@ struct dw_pcie_host_ops {
 	u32 (*get_msi_data)(struct pcie_port *pp, int pos);
 	void (*scan_bus)(struct pcie_port *pp);
 	void (*set_num_vectors)(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

[PATCH v2 8/9] pci: removing old irq api from pcie-designware

[Joao Pinto]

This patch removes the old interrupt API.

Signed-off-by: Joao Pinto <jpinto@synopsys.com>
---
Change v1->v2:
- Nothing changed, just to follow the patch set version.

 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 | 192 +--------------------------------
 drivers/pci/dwc/pcie-designware.h      |   2 +-
 5 files changed, 6 insertions(+), 195 deletions(-)

diff --git a/drivers/pci/dwc/pci-keystone-dw.c b/drivers/pci/dwc/pci-keystone-dw.c
index 1b7314d..776f8a2 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)
 {
 	return dw_pcie_allocate_domains(pci);
 }
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 feeee6a..6c771cc 100644
--- a/drivers/pci/dwc/pcie-designware-host.c
+++ b/drivers/pci/dwc/pcie-designware-host.c
@@ -46,14 +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);
 }
 
-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,
-};
-
 static void dw_msi_mask_irq(struct irq_data *d)
 {
 	pci_msi_mask_irq(d);
@@ -315,186 +307,6 @@ void dw_pcie_msi_init(struct pcie_port *pp)
 			    upper_32_bits(msi_target));
 }
 
-static void dw_pcie_msi_clear_irq(struct pcie_port *pp, int irq)
-{
-	unsigned int res, bit, ctrl;
-
-	ctrl = irq / 32;
-	res = ctrl * 12;
-	bit = irq % 32;
-	pp->irq_status[ctrl] &= ~(1 << bit);
-	dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4,
-			    pp->irq_status[ctrl]);
-}
-
-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, ctrl;
-
-	ctrl = irq / 32;
-	res = ctrl * 12;
-	bit = irq % 32;
-	pp->irq_status[ctrl] |= 1 << bit;
-	dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4,
-			    pp->irq_status[ctrl]);
-}
-
-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,7 +316,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
 	struct resource_entry *win, *tmp;
 	struct pci_bus *bus, *child;
 	struct resource *cfg_res;
-	int i, ret;
+	int ret;
 
 	LIST_HEAD(res);
 
@@ -629,7 +441,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
 							 dw_chained_msi_isr,
 							 pci);
 		} else {
-			ret = pp->ops->msi_host_init(pci, &dw_pcie_msi_chip);
+			ret = pp->ops->msi_host_init(pci);
 			if (ret < 0)
 				goto error;
 		}
diff --git a/drivers/pci/dwc/pcie-designware.h b/drivers/pci/dwc/pcie-designware.h
index 7385b4e..f95c086 100644
--- a/drivers/pci/dwc/pcie-designware.h
+++ b/drivers/pci/dwc/pcie-designware.h
@@ -142,7 +142,7 @@ struct dw_pcie_host_ops {
 	u32 (*get_msi_data)(struct pcie_port *pp, int pos);
 	void (*scan_bus)(struct pcie_port *pp);
 	void (*set_num_vectors)(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

[PATCH v2 9/9] pci: remove limitation of the number of the available IRQs

[Joao Pinto]

The Synopsys PCIe Controller supports up to 256 IRQs distributed
by 8 controller registers.

Having this in mind, the number of the maximum number of
IRQs was changed to 256 and now the number of controllers is
calculated based on the number of vectors used by the specific
SoC driver.

Signed-off-by: Joao Pinto <jpinto@synopsys.com>
---
Change v1->v2:
- New patch

 drivers/pci/dwc/pcie-designware-host.c | 12 ++++++++----
 drivers/pci/dwc/pcie-designware.h      | 10 +++-------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c
index 6c771cc..31c7ccd 100644
--- a/drivers/pci/dwc/pcie-designware-host.c
+++ b/drivers/pci/dwc/pcie-designware-host.c
@@ -73,11 +73,13 @@ static struct msi_domain_info dw_pcie_msi_domain_info = {
 /* MSI int handler */
 irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
 {
-	u32 val;
 	int i, pos, irq;
+	u32 val, num_ctrls;
 	irqreturn_t ret = IRQ_NONE;
 
-	for (i = 0; i < MAX_MSI_CTRLS; i++) {
+	num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
+
+	for (i = 0; i < num_ctrls; i++) {
 		dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_STATUS + i * 12, 4,
 				    &val);
 		if (!val)
@@ -625,13 +627,15 @@ static u8 dw_pcie_iatu_unroll_enabled(struct dw_pcie *pci)
 
 void dw_pcie_setup_rc(struct pcie_port *pp)
 {
-	u32 val, ctrl;
+	u32 val, ctrl, num_ctrls;
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
 
 	dw_pcie_setup(pci);
 
+	num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
+
 	/* Initialize IRQ Status array */
-	for (ctrl = 0; ctrl < MAX_MSI_CTRLS; ctrl++)
+	for (ctrl = 0; ctrl < num_ctrls; ctrl++)
 		dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_ENABLE + (ctrl * 12), 4,
 				    &pp->irq_status[ctrl]);
 
diff --git a/drivers/pci/dwc/pcie-designware.h b/drivers/pci/dwc/pcie-designware.h
index f95c086..4a72f16 100644
--- a/drivers/pci/dwc/pcie-designware.h
+++ b/drivers/pci/dwc/pcie-designware.h
@@ -102,13 +102,9 @@
 #define MSI_MESSAGE_ADDR_L32		0x54
 #define MSI_MESSAGE_ADDR_U32		0x58
 
-/*
- * Maximum number of MSI IRQs can be 256 per controller. But keep
- * it 32 as of now. Probably we will never need more than 32. If needed,
- * then increment it in multiple of 32.
- */
-#define MAX_MSI_IRQS			32
-#define MAX_MSI_CTRLS			(MAX_MSI_IRQS / 32)
+#define MAX_MSI_IRQS			256
+#define MAX_MSI_IRQS_PER_CTRL		32
+#define MAX_MSI_CTRLS			(MAX_MSI_IRQS / (MAX_MSI_IRQS_PER_CTRL))
 #define MSI_DEF_NUM_VECTORS		32
 
 struct pcie_port;
-- 
2.9.3

Re: [PATCH v2 9/9] pci: remove limitation of the number of the available IRQs

[Niklas Cassel]

On 06/05/2017 06:19 PM, Joao Pinto wrote:
> The Synopsys PCIe Controller supports up to 256 IRQs distributed
> by 8 controller registers.
> 
> Having this in mind, the number of the maximum number of
> IRQs was changed to 256 and now the number of controllers is
> calculated based on the number of vectors used by the specific
> SoC driver.
> 
> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
> ---
> Change v1->v2:
> - New patch
> 
>  drivers/pci/dwc/pcie-designware-host.c | 12 ++++++++----
>  drivers/pci/dwc/pcie-designware.h      | 10 +++-------
>  2 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c
> index 6c771cc..31c7ccd 100644
> --- a/drivers/pci/dwc/pcie-designware-host.c
> +++ b/drivers/pci/dwc/pcie-designware-host.c
> @@ -73,11 +73,13 @@ static struct msi_domain_info dw_pcie_msi_domain_info = {
>  /* MSI int handler */
>  irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
>  {
> -	u32 val;
>  	int i, pos, irq;
> +	u32 val, num_ctrls;
>  	irqreturn_t ret = IRQ_NONE;
>  
> -	for (i = 0; i < MAX_MSI_CTRLS; i++) {
> +	num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
> +
> +	for (i = 0; i < num_ctrls; i++) {
>  		dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_STATUS + i * 12, 4,
>  				    &val);

Hello Joao


Nice to see that we now have support for all 8 controllers,
good job!

Older versions of the DesignWare IP only had support for a
combined msi irq (msi_ctrl_int), but since recent versions
of the IP also supports a dedicated irq line per controller
(msi_ctrl_int_vec[7:0]), do you	think that it makes sense
to take this into consideration for this patchset?

By doing so, dw_handle_msi_irq would not have to iterate
over all 8 controllers, as the irq itself would tell us
what controller to read the status from (to finally see
which irq(s) that actually triggered).


Regards,
Niklas

Re: [PATCH v2 9/9] pci: remove limitation of the number of the available IRQs

[Joao Pinto]

Hi Niklas,

Às 3:22 PM de 7/6/2017, Niklas Cassel escreveu:
> On 06/05/2017 06:19 PM, Joao Pinto wrote:
>> The Synopsys PCIe Controller supports up to 256 IRQs distributed
>> by 8 controller registers.
>>
>> Having this in mind, the number of the maximum number of
>> IRQs was changed to 256 and now the number of controllers is
>> calculated based on the number of vectors used by the specific
>> SoC driver.
>>
>> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
>> ---
>> Change v1->v2:
>> - New patch
>>
>>  drivers/pci/dwc/pcie-designware-host.c | 12 ++++++++----
>>  drivers/pci/dwc/pcie-designware.h      | 10 +++-------
>>  2 files changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c
>> index 6c771cc..31c7ccd 100644
>> --- a/drivers/pci/dwc/pcie-designware-host.c
>> +++ b/drivers/pci/dwc/pcie-designware-host.c
>> @@ -73,11 +73,13 @@ static struct msi_domain_info dw_pcie_msi_domain_info = {
>>  /* MSI int handler */
>>  irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
>>  {
>> -	u32 val;
>>  	int i, pos, irq;
>> +	u32 val, num_ctrls;
>>  	irqreturn_t ret = IRQ_NONE;
>>  
>> -	for (i = 0; i < MAX_MSI_CTRLS; i++) {
>> +	num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
>> +
>> +	for (i = 0; i < num_ctrls; i++) {
>>  		dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_STATUS + i * 12, 4,
>>  				    &val);
> 
> Hello Joao
> 
> 
> Nice to see that we now have support for all 8 controllers,
> good job!
> 
> Older versions of the DesignWare IP only had support for a
> combined msi irq (msi_ctrl_int), but since recent versions
> of the IP also supports a dedicated irq line per controller
> (msi_ctrl_int_vec[7:0]), do you	think that it makes sense
> to take this into consideration for this patchset?
> 
> By doing so, dw_handle_msi_irq would not have to iterate
> over all 8 controllers, as the irq itself would tell us
> what controller to read the status from (to finally see
> which irq(s) that actually triggered).

This would be interesting, thanks for the tips!

Joao

> 
> 
> Regards,
> Niklas
> 

Re: [PATCH v2 0/9] add new irq api to pcie-designware

[Murali Karicheri]

Joao,

On 06/05/2017 12:19 PM, Joao Pinto wrote:
> 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.
> 
> The patch set was made against the Bjorn' next branch.
> 
> Joao Pinto (9):
>   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
>   pci: remove limitation of the number of the available IRQs
> 
>  drivers/pci/dwc/pci-exynos.c           |  18 --
>  drivers/pci/dwc/pci-imx6.c             |  18 --
>  drivers/pci/dwc/pci-keystone-dw.c      |  96 +-------
>  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 | 395 +++++++++++++++++++--------------
>  drivers/pci/dwc/pcie-designware-plat.c |  15 --
>  drivers/pci/dwc/pcie-designware.h      |  30 ++-
>  drivers/pci/dwc/pcie-qcom.c            |  15 --
>  11 files changed, 255 insertions(+), 359 deletions(-)
> 

I gave this a try today and it failed. Logs at
http://pastebin.ubuntu.com/24843963/

The first part of the log is with your patch series.
The second part is before applying the patch. You will see
that there is qc timeout log that shows the issue

[   12.791852] ata2.00: qc timeout (cmd 0xec)
[   12.795947] ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4)
[   12.802061] ata2: limiting SATA link speed to 3.0 Gbps
[   13.301853] ata2: SATA link up 3.0 Gbps (SStatus 123 SControl 320)
[   23.511852] ata2.00: qc timeout (cmd 0xec)
[   23.515947] ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4)
[   24.021850] ata2: SATA link up 3.0 Gbps (SStatus 123 SControl 320)

Is there any DT change I need to make for this? I didn't have a
chance to review your patch w.r.t Keystone. But I see you have change
the MSI IRq handling.

-- 
Murali Karicheri
Linux Kernel, Keystone

Re: [PATCH v2 0/9] add new irq api to pcie-designware

[Joao Pinto]

Hi Murali,

Às 10:18 PM de 6/12/2017, Murali Karicheri escreveu:
> Joao,
> 
> On 06/05/2017 12:19 PM, Joao Pinto wrote:
>> 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.
>>
>> The patch set was made against the Bjorn' next branch.
>>
>> Joao Pinto (9):
>>   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
>>   pci: remove limitation of the number of the available IRQs
>>
>>  drivers/pci/dwc/pci-exynos.c           |  18 --
>>  drivers/pci/dwc/pci-imx6.c             |  18 --
>>  drivers/pci/dwc/pci-keystone-dw.c      |  96 +-------
>>  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 | 395 +++++++++++++++++++--------------
>>  drivers/pci/dwc/pcie-designware-plat.c |  15 --
>>  drivers/pci/dwc/pcie-designware.h      |  30 ++-
>>  drivers/pci/dwc/pcie-qcom.c            |  15 --
>>  11 files changed, 255 insertions(+), 359 deletions(-)
>>
> 
> I gave this a try today and it failed. Logs at
> https://urldefense.proofpoint.com/v2/url?u=http-3A__pastebin.ubuntu.com_24843963_&d=DwIC-g&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=FR_WRctJN74GGjjHco1VUqNfINiGA7lGruObX4bplAY&s=8QYJU-SCW5JEmMwLUqbOEBJA5hio9vIyeNyotVRos4Q&e= 
> 
> The first part of the log is with your patch series.
> The second part is before applying the patch. You will see
> that there is qc timeout log that shows the issue
> 
> [   12.791852] ata2.00: qc timeout (cmd 0xec)
> [   12.795947] ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4)
> [   12.802061] ata2: limiting SATA link speed to 3.0 Gbps
> [   13.301853] ata2: SATA link up 3.0 Gbps (SStatus 123 SControl 320)
> [   23.511852] ata2.00: qc timeout (cmd 0xec)
> [   23.515947] ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4)
> [   24.021850] ata2: SATA link up 3.0 Gbps (SStatus 123 SControl 320)
> 
> Is there any DT change I need to make for this? I didn't have a
> chance to review your patch w.r.t Keystone. But I see you have change
> the MSI IRq handling.
> 

There is no need to change your DT. I think I will need to debug this issue
further. Could you please advice a board containing this SoC?

Thanks.

Re: [PATCH v2 0/9] add new irq api to pcie-designware

[Murali Karicheri]

On 06/19/2017 12:31 PM, Joao Pinto wrote:
> 
> Hi Murali,
> 
> Às 10:18 PM de 6/12/2017, Murali Karicheri escreveu:
>> Joao,
>>
>> On 06/05/2017 12:19 PM, Joao Pinto wrote:
>>> 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.
>>>
>>> The patch set was made against the Bjorn' next branch.
>>>
>>> Joao Pinto (9):
>>>   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
>>>   pci: remove limitation of the number of the available IRQs
>>>
>>>  drivers/pci/dwc/pci-exynos.c           |  18 --
>>>  drivers/pci/dwc/pci-imx6.c             |  18 --
>>>  drivers/pci/dwc/pci-keystone-dw.c      |  96 +-------
>>>  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 | 395 +++++++++++++++++++--------------
>>>  drivers/pci/dwc/pcie-designware-plat.c |  15 --
>>>  drivers/pci/dwc/pcie-designware.h      |  30 ++-
>>>  drivers/pci/dwc/pcie-qcom.c            |  15 --
>>>  11 files changed, 255 insertions(+), 359 deletions(-)
>>>
>>
>> I gave this a try today and it failed. Logs at
>> https://urldefense.proofpoint.com/v2/url?u=http-3A__pastebin.ubuntu.com_24843963_&d=DwIC-g&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=FR_WRctJN74GGjjHco1VUqNfINiGA7lGruObX4bplAY&s=8QYJU-SCW5JEmMwLUqbOEBJA5hio9vIyeNyotVRos4Q&e= 
>>
>> The first part of the log is with your patch series.
>> The second part is before applying the patch. You will see
>> that there is qc timeout log that shows the issue
>>
>> [   12.791852] ata2.00: qc timeout (cmd 0xec)
>> [   12.795947] ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4)
>> [   12.802061] ata2: limiting SATA link speed to 3.0 Gbps
>> [   13.301853] ata2: SATA link up 3.0 Gbps (SStatus 123 SControl 320)
>> [   23.511852] ata2.00: qc timeout (cmd 0xec)
>> [   23.515947] ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4)
>> [   24.021850] ata2: SATA link up 3.0 Gbps (SStatus 123 SControl 320)
>>
>> Is there any DT change I need to make for this? I didn't have a
>> chance to review your patch w.r.t Keystone. But I see you have change
>> the MSI IRq handling.
>>
> 
> There is no need to change your DT. I think I will need to debug this issue
> further. Could you please advice a board containing this SoC?
> 
I think I may be able to work with you on this. Keystone has different 
register set for MSI IRQ. It is in application space. Also there are
8 interrupts to ARM GIC that multiplexes the 32 MSI interrupts. So
please review the original code vs the changed one w.r.t to this. I haven't
had a chance to review the code as I am in the middle of a release.
But will be able to spend some time after two weeks. Meanwhile please
spend some time reviewing the code and exchange email to identify
the issue. I can add specific debug prints for you as well and provide
you the log for analysis.

Let me know.

Murali
> Thanks.
> 
> 
> 
> 


-- 
Murali Karicheri
Linux Kernel, Keystone

Re: [PATCH v2 0/9] add new irq api to pcie-designware

[Joao Pinto]

Às 4:50 PM de 6/20/2017, Murali Karicheri escreveu:
> On 06/19/2017 12:31 PM, Joao Pinto wrote:
>>
>> Hi Murali,
>>
>> Às 10:18 PM de 6/12/2017, Murali Karicheri escreveu:
>>> Joao,
>>>
>>> On 06/05/2017 12:19 PM, Joao Pinto wrote:
>>>> 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.
>>>>
>>>> The patch set was made against the Bjorn' next branch.
>>>>
>>>> Joao Pinto (9):
>>>>   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
>>>>   pci: remove limitation of the number of the available IRQs
>>>>
>>>>  drivers/pci/dwc/pci-exynos.c           |  18 --
>>>>  drivers/pci/dwc/pci-imx6.c             |  18 --
>>>>  drivers/pci/dwc/pci-keystone-dw.c      |  96 +-------
>>>>  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 | 395 +++++++++++++++++++--------------
>>>>  drivers/pci/dwc/pcie-designware-plat.c |  15 --
>>>>  drivers/pci/dwc/pcie-designware.h      |  30 ++-
>>>>  drivers/pci/dwc/pcie-qcom.c            |  15 --
>>>>  11 files changed, 255 insertions(+), 359 deletions(-)
>>>>
>>>
>>> I gave this a try today and it failed. Logs at
>>> https://urldefense.proofpoint.com/v2/url?u=http-3A__pastebin.ubuntu.com_24843963_&d=DwIC-g&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=FR_WRctJN74GGjjHco1VUqNfINiGA7lGruObX4bplAY&s=8QYJU-SCW5JEmMwLUqbOEBJA5hio9vIyeNyotVRos4Q&e= 
>>>
>>> The first part of the log is with your patch series.
>>> The second part is before applying the patch. You will see
>>> that there is qc timeout log that shows the issue
>>>
>>> [   12.791852] ata2.00: qc timeout (cmd 0xec)
>>> [   12.795947] ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4)
>>> [   12.802061] ata2: limiting SATA link speed to 3.0 Gbps
>>> [   13.301853] ata2: SATA link up 3.0 Gbps (SStatus 123 SControl 320)
>>> [   23.511852] ata2.00: qc timeout (cmd 0xec)
>>> [   23.515947] ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4)
>>> [   24.021850] ata2: SATA link up 3.0 Gbps (SStatus 123 SControl 320)
>>>
>>> Is there any DT change I need to make for this? I didn't have a
>>> chance to review your patch w.r.t Keystone. But I see you have change
>>> the MSI IRq handling.
>>>
>>
>> There is no need to change your DT. I think I will need to debug this issue
>> further. Could you please advice a board containing this SoC?
>>
> I think I may be able to work with you on this. Keystone has different 
> register set for MSI IRQ. It is in application space. Also there are
> 8 interrupts to ARM GIC that multiplexes the 32 MSI interrupts. So
> please review the original code vs the changed one w.r.t to this. I haven't
> had a chance to review the code as I am in the middle of a release.
> But will be able to spend some time after two weeks. Meanwhile please
> spend some time reviewing the code and exchange email to identify
> the issue. I can add specific debug prints for you as well and provide
> you the log for analysis.

Ok, I agree! Thanks!

> 
> Let me know.
> 
> Murali
>> Thanks.
>>
>>
>>
>>
> 
> 

Re: [PATCH v2 0/9] add new irq api to pcie-designware

[Murali Karicheri]

On 06/20/2017 12:28 PM, Joao Pinto wrote:
> Às 4:50 PM de 6/20/2017, Murali Karicheri escreveu:
>> On 06/19/2017 12:31 PM, Joao Pinto wrote:
>>>
>>> Hi Murali,
>>>
>>> Às 10:18 PM de 6/12/2017, Murali Karicheri escreveu:
>>>> Joao,
>>>>
>>>> On 06/05/2017 12:19 PM, Joao Pinto wrote:
>>>>> 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.
>>>>>
>>>>> The patch set was made against the Bjorn' next branch.
>>>>>
>>>>> Joao Pinto (9):
>>>>>   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
>>>>>   pci: remove limitation of the number of the available IRQs
>>>>>
>>>>>  drivers/pci/dwc/pci-exynos.c           |  18 --
>>>>>  drivers/pci/dwc/pci-imx6.c             |  18 --
>>>>>  drivers/pci/dwc/pci-keystone-dw.c      |  96 +-------
>>>>>  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 | 395 +++++++++++++++++++--------------
>>>>>  drivers/pci/dwc/pcie-designware-plat.c |  15 --
>>>>>  drivers/pci/dwc/pcie-designware.h      |  30 ++-
>>>>>  drivers/pci/dwc/pcie-qcom.c            |  15 --
>>>>>  11 files changed, 255 insertions(+), 359 deletions(-)
>>>>>
>>>>
>>>> I gave this a try today and it failed. Logs at
>>>> https://urldefense.proofpoint.com/v2/url?u=http-3A__pastebin.ubuntu.com_24843963_&d=DwIC-g&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=FR_WRctJN74GGjjHco1VUqNfINiGA7lGruObX4bplAY&s=8QYJU-SCW5JEmMwLUqbOEBJA5hio9vIyeNyotVRos4Q&e= 
>>>>
>>>> The first part of the log is with your patch series.
>>>> The second part is before applying the patch. You will see
>>>> that there is qc timeout log that shows the issue
>>>>
>>>> [   12.791852] ata2.00: qc timeout (cmd 0xec)
>>>> [   12.795947] ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4)
>>>> [   12.802061] ata2: limiting SATA link speed to 3.0 Gbps
>>>> [   13.301853] ata2: SATA link up 3.0 Gbps (SStatus 123 SControl 320)
>>>> [   23.511852] ata2.00: qc timeout (cmd 0xec)
>>>> [   23.515947] ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4)
>>>> [   24.021850] ata2: SATA link up 3.0 Gbps (SStatus 123 SControl 320)
>>>>
>>>> Is there any DT change I need to make for this? I didn't have a
>>>> chance to review your patch w.r.t Keystone. But I see you have change
>>>> the MSI IRq handling.
>>>>
>>>
>>> There is no need to change your DT. I think I will need to debug this issue
>>> further. Could you please advice a board containing this SoC?
>>>
>> I think I may be able to work with you on this. Keystone has different 
>> register set for MSI IRQ. It is in application space. Also there are
>> 8 interrupts to ARM GIC that multiplexes the 32 MSI interrupts. So
>> please review the original code vs the changed one w.r.t to this. I haven't
>> had a chance to review the code as I am in the middle of a release.
>> But will be able to spend some time after two weeks. Meanwhile please
>> spend some time reviewing the code and exchange email to identify
>> the issue. I can add specific debug prints for you as well and provide
>> you the log for analysis.
> 
> Ok, I agree! Thanks!
> 
>>
Look at this logic in the patch where a potential issue exists for Keystone.

+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 = lower_32_bits(msi_target);
+       msg->address_hi = upper_32_bits(msi_target);
+
+       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);
+}

Keystone has 8 hw irqs, 4 multiplexed over each since the register
set is different than other DWC cores. So will not work for Keystone,
right?

Murali

>> Let me know.
>>
>> Murali
>>> Thanks.
>>>
>>>
>>>
>>>
>>
>>
> 


-- 
Murali Karicheri
Linux Kernel, Keystone

Re: [PATCH v2 0/9] add new irq api to pcie-designware

[Joao Pinto]

Às 6:05 PM de 6/20/2017, Murali Karicheri escreveu:
> On 06/20/2017 12:28 PM, Joao Pinto wrote:
>> Às 4:50 PM de 6/20/2017, Murali Karicheri escreveu:
>>> On 06/19/2017 12:31 PM, Joao Pinto wrote:
>>>>
>>>> Hi Murali,
>>>>
>>>> Às 10:18 PM de 6/12/2017, Murali Karicheri escreveu:
>>>>> Joao,
>>>>>
>>>>> On 06/05/2017 12:19 PM, Joao Pinto wrote:
>>>>>> 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.
>>>>>>
>>>>>> The patch set was made against the Bjorn' next branch.
>>>>>>
>>>>>> Joao Pinto (9):
>>>>>>   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
>>>>>>   pci: remove limitation of the number of the available IRQs
>>>>>>
>>>>>>  drivers/pci/dwc/pci-exynos.c           |  18 --
>>>>>>  drivers/pci/dwc/pci-imx6.c             |  18 --
>>>>>>  drivers/pci/dwc/pci-keystone-dw.c      |  96 +-------
>>>>>>  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 | 395 +++++++++++++++++++--------------
>>>>>>  drivers/pci/dwc/pcie-designware-plat.c |  15 --
>>>>>>  drivers/pci/dwc/pcie-designware.h      |  30 ++-
>>>>>>  drivers/pci/dwc/pcie-qcom.c            |  15 --
>>>>>>  11 files changed, 255 insertions(+), 359 deletions(-)
>>>>>>
>>>>>
>>>>> I gave this a try today and it failed. Logs at
>>>>> https://urldefense.proofpoint.com/v2/url?u=http-3A__pastebin.ubuntu.com_24843963_&d=DwIC-g&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=FR_WRctJN74GGjjHco1VUqNfINiGA7lGruObX4bplAY&s=8QYJU-SCW5JEmMwLUqbOEBJA5hio9vIyeNyotVRos4Q&e= 
>>>>>
>>>>> The first part of the log is with your patch series.
>>>>> The second part is before applying the patch. You will see
>>>>> that there is qc timeout log that shows the issue
>>>>>
>>>>> [   12.791852] ata2.00: qc timeout (cmd 0xec)
>>>>> [   12.795947] ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4)
>>>>> [   12.802061] ata2: limiting SATA link speed to 3.0 Gbps
>>>>> [   13.301853] ata2: SATA link up 3.0 Gbps (SStatus 123 SControl 320)
>>>>> [   23.511852] ata2.00: qc timeout (cmd 0xec)
>>>>> [   23.515947] ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4)
>>>>> [   24.021850] ata2: SATA link up 3.0 Gbps (SStatus 123 SControl 320)
>>>>>
>>>>> Is there any DT change I need to make for this? I didn't have a
>>>>> chance to review your patch w.r.t Keystone. But I see you have change
>>>>> the MSI IRq handling.
>>>>>
>>>>
>>>> There is no need to change your DT. I think I will need to debug this issue
>>>> further. Could you please advice a board containing this SoC?
>>>>
>>> I think I may be able to work with you on this. Keystone has different 
>>> register set for MSI IRQ. It is in application space. Also there are
>>> 8 interrupts to ARM GIC that multiplexes the 32 MSI interrupts. So
>>> please review the original code vs the changed one w.r.t to this. I haven't
>>> had a chance to review the code as I am in the middle of a release.
>>> But will be able to spend some time after two weeks. Meanwhile please
>>> spend some time reviewing the code and exchange email to identify
>>> the issue. I can add specific debug prints for you as well and provide
>>> you the log for analysis.
>>
>> Ok, I agree! Thanks!
>>
>>>
> Look at this logic in the patch where a potential issue exists for Keystone.
> 
> +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 = lower_32_bits(msi_target);
> +       msg->address_hi = upper_32_bits(msi_target);
> +
> +       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);
> +}
> 
> Keystone has 8 hw irqs, 4 multiplexed over each since the register
> set is different than other DWC cores. So will not work for Keystone,
> right?

Well, that can be challenging, since it is a very particular operation.
Maybe we should implement a _pci_setup_msi_msg in the keystone driver and
overide this one if necessary. What do you think?

Thanks.

> 
> Murali
> 
>>> Let me know.
>>>
>>> Murali
>>>> Thanks.
>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>
> 
> 

Re: [PATCH v2 0/9] add new irq api to pcie-designware

[Marc Zyngier]

On 05/07/17 11:57, Joao Pinto wrote:
> Às 6:05 PM de 6/20/2017, Murali Karicheri escreveu:
>> On 06/20/2017 12:28 PM, Joao Pinto wrote:
>>> Às 4:50 PM de 6/20/2017, Murali Karicheri escreveu:
>>>> On 06/19/2017 12:31 PM, Joao Pinto wrote:
>>>>>
>>>>> Hi Murali,
>>>>>
>>>>> Às 10:18 PM de 6/12/2017, Murali Karicheri escreveu:
>>>>>> Joao,
>>>>>>
>>>>>> On 06/05/2017 12:19 PM, Joao Pinto wrote:
>>>>>>> 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.
>>>>>>>
>>>>>>> The patch set was made against the Bjorn' next branch.
>>>>>>>
>>>>>>> Joao Pinto (9):
>>>>>>>   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
>>>>>>>   pci: remove limitation of the number of the available IRQs
>>>>>>>
>>>>>>>  drivers/pci/dwc/pci-exynos.c           |  18 --
>>>>>>>  drivers/pci/dwc/pci-imx6.c             |  18 --
>>>>>>>  drivers/pci/dwc/pci-keystone-dw.c      |  96 +-------
>>>>>>>  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 | 395 +++++++++++++++++++--------------
>>>>>>>  drivers/pci/dwc/pcie-designware-plat.c |  15 --
>>>>>>>  drivers/pci/dwc/pcie-designware.h      |  30 ++-
>>>>>>>  drivers/pci/dwc/pcie-qcom.c            |  15 --
>>>>>>>  11 files changed, 255 insertions(+), 359 deletions(-)
>>>>>>>
>>>>>>
>>>>>> I gave this a try today and it failed. Logs at
>>>>>> https://urldefense.proofpoint.com/v2/url?u=http-3A__pastebin.ubuntu.com_24843963_&d=DwIC-g&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=FR_WRctJN74GGjjHco1VUqNfINiGA7lGruObX4bplAY&s=8QYJU-SCW5JEmMwLUqbOEBJA5hio9vIyeNyotVRos4Q&e= 
>>>>>>
>>>>>> The first part of the log is with your patch series.
>>>>>> The second part is before applying the patch. You will see
>>>>>> that there is qc timeout log that shows the issue
>>>>>>
>>>>>> [   12.791852] ata2.00: qc timeout (cmd 0xec)
>>>>>> [   12.795947] ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4)
>>>>>> [   12.802061] ata2: limiting SATA link speed to 3.0 Gbps
>>>>>> [   13.301853] ata2: SATA link up 3.0 Gbps (SStatus 123 SControl 320)
>>>>>> [   23.511852] ata2.00: qc timeout (cmd 0xec)
>>>>>> [   23.515947] ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4)
>>>>>> [   24.021850] ata2: SATA link up 3.0 Gbps (SStatus 123 SControl 320)
>>>>>>
>>>>>> Is there any DT change I need to make for this? I didn't have a
>>>>>> chance to review your patch w.r.t Keystone. But I see you have change
>>>>>> the MSI IRq handling.
>>>>>>
>>>>>
>>>>> There is no need to change your DT. I think I will need to debug this issue
>>>>> further. Could you please advice a board containing this SoC?
>>>>>
>>>> I think I may be able to work with you on this. Keystone has different 
>>>> register set for MSI IRQ. It is in application space. Also there are
>>>> 8 interrupts to ARM GIC that multiplexes the 32 MSI interrupts. So
>>>> please review the original code vs the changed one w.r.t to this. I haven't
>>>> had a chance to review the code as I am in the middle of a release.
>>>> But will be able to spend some time after two weeks. Meanwhile please
>>>> spend some time reviewing the code and exchange email to identify
>>>> the issue. I can add specific debug prints for you as well and provide
>>>> you the log for analysis.
>>>
>>> Ok, I agree! Thanks!
>>>
>>>>
>> Look at this logic in the patch where a potential issue exists for Keystone.
>>
>> +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 = lower_32_bits(msi_target);
>> +       msg->address_hi = upper_32_bits(msi_target);
>> +
>> +       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);
>> +}
>>
>> Keystone has 8 hw irqs, 4 multiplexed over each since the register
>> set is different than other DWC cores. So will not work for Keystone,
>> right?
> 
> Well, that can be challenging, since it is a very particular operation.
> Maybe we should implement a _pci_setup_msi_msg in the keystone driver and
> overide this one if necessary. What do you think?

At that rate, and given that keystone seems to override almost all
operations in this driver, wouldn't it make sense for this to be
implemented as a *separate* driver?

Thanks,

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

Re: [PATCH v2 0/9] add new irq api to pcie-designware

[Murali Karicheri]

On 07/05/2017 11:26 AM, Marc Zyngier wrote:
> On 05/07/17 11:57, Joao Pinto wrote:
>> Às 6:05 PM de 6/20/2017, Murali Karicheri escreveu:
>>> On 06/20/2017 12:28 PM, Joao Pinto wrote:
>>>> Às 4:50 PM de 6/20/2017, Murali Karicheri escreveu:
>>>>> On 06/19/2017 12:31 PM, Joao Pinto wrote:
>>>>>>
>>>>>> Hi Murali,
>>>>>>
>>>>>> Às 10:18 PM de 6/12/2017, Murali Karicheri escreveu:
>>>>>>> Joao,
>>>>>>>
>>>>>>> On 06/05/2017 12:19 PM, Joao Pinto wrote:
>>>>>>>> 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.
>>>>>>>>
>>>>>>>> The patch set was made against the Bjorn' next branch.
>>>>>>>>
>>>>>>>> Joao Pinto (9):
>>>>>>>>   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
>>>>>>>>   pci: remove limitation of the number of the available IRQs
>>>>>>>>
>>>>>>>>  drivers/pci/dwc/pci-exynos.c           |  18 --
>>>>>>>>  drivers/pci/dwc/pci-imx6.c             |  18 --
>>>>>>>>  drivers/pci/dwc/pci-keystone-dw.c      |  96 +-------
>>>>>>>>  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 | 395 +++++++++++++++++++--------------
>>>>>>>>  drivers/pci/dwc/pcie-designware-plat.c |  15 --
>>>>>>>>  drivers/pci/dwc/pcie-designware.h      |  30 ++-
>>>>>>>>  drivers/pci/dwc/pcie-qcom.c            |  15 --
>>>>>>>>  11 files changed, 255 insertions(+), 359 deletions(-)
>>>>>>>>
>>>>>>>
>>>>>>> I gave this a try today and it failed. Logs at
>>>>>>> https://urldefense.proofpoint.com/v2/url?u=http-3A__pastebin.ubuntu.com_24843963_&d=DwIC-g&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=FR_WRctJN74GGjjHco1VUqNfINiGA7lGruObX4bplAY&s=8QYJU-SCW5JEmMwLUqbOEBJA5hio9vIyeNyotVRos4Q&e= 
>>>>>>>
>>>>>>> The first part of the log is with your patch series.
>>>>>>> The second part is before applying the patch. You will see
>>>>>>> that there is qc timeout log that shows the issue
>>>>>>>
>>>>>>> [   12.791852] ata2.00: qc timeout (cmd 0xec)
>>>>>>> [   12.795947] ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4)
>>>>>>> [   12.802061] ata2: limiting SATA link speed to 3.0 Gbps
>>>>>>> [   13.301853] ata2: SATA link up 3.0 Gbps (SStatus 123 SControl 320)
>>>>>>> [   23.511852] ata2.00: qc timeout (cmd 0xec)
>>>>>>> [   23.515947] ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4)
>>>>>>> [   24.021850] ata2: SATA link up 3.0 Gbps (SStatus 123 SControl 320)
>>>>>>>
>>>>>>> Is there any DT change I need to make for this? I didn't have a
>>>>>>> chance to review your patch w.r.t Keystone. But I see you have change
>>>>>>> the MSI IRq handling.
>>>>>>>
>>>>>>
>>>>>> There is no need to change your DT. I think I will need to debug this issue
>>>>>> further. Could you please advice a board containing this SoC?
>>>>>>
>>>>> I think I may be able to work with you on this. Keystone has different 
>>>>> register set for MSI IRQ. It is in application space. Also there are
>>>>> 8 interrupts to ARM GIC that multiplexes the 32 MSI interrupts. So
>>>>> please review the original code vs the changed one w.r.t to this. I haven't
>>>>> had a chance to review the code as I am in the middle of a release.
>>>>> But will be able to spend some time after two weeks. Meanwhile please
>>>>> spend some time reviewing the code and exchange email to identify
>>>>> the issue. I can add specific debug prints for you as well and provide
>>>>> you the log for analysis.
>>>>
>>>> Ok, I agree! Thanks!
>>>>
>>>>>
>>> Look at this logic in the patch where a potential issue exists for Keystone.
>>>
>>> +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 = lower_32_bits(msi_target);
>>> +       msg->address_hi = upper_32_bits(msi_target);
>>> +
>>> +       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);
>>> +}
>>>
>>> Keystone has 8 hw irqs, 4 multiplexed over each since the register
>>> set is different than other DWC cores. So will not work for Keystone,
>>> right?
>>
>> Well, that can be challenging, since it is a very particular operation.
>> Maybe we should implement a _pci_setup_msi_msg in the keystone driver and
>> overide this one if necessary. What do you think?
> 
> At that rate, and given that keystone seems to override almost all
> operations in this driver, wouldn't it make sense for this to be
> implemented as a *separate* driver?
Marc,

What you mean by separate driver? Just the MSI handling part? 

Murali
> 
> Thanks,
> 
> 	M.
> 


-- 
Murali Karicheri
Linux Kernel, Keystone

Re: [PATCH v2 0/9] add new irq api to pcie-designware

[Marc Zyngier]

On 05/07/17 22:03, Murali Karicheri wrote:
> On 07/05/2017 11:26 AM, Marc Zyngier wrote:
>> On 05/07/17 11:57, Joao Pinto wrote:
>>> Às 6:05 PM de 6/20/2017, Murali Karicheri escreveu:
>>>> On 06/20/2017 12:28 PM, Joao Pinto wrote:
>>>>> Às 4:50 PM de 6/20/2017, Murali Karicheri escreveu:
>>>>>> On 06/19/2017 12:31 PM, Joao Pinto wrote:
>>>>>>>
>>>>>>> Hi Murali,
>>>>>>>
>>>>>>> Às 10:18 PM de 6/12/2017, Murali Karicheri escreveu:
>>>>>>>> Joao,
>>>>>>>>
>>>>>>>> On 06/05/2017 12:19 PM, Joao Pinto wrote:
>>>>>>>>> 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.
>>>>>>>>>
>>>>>>>>> The patch set was made against the Bjorn' next branch.
>>>>>>>>>
>>>>>>>>> Joao Pinto (9):
>>>>>>>>>   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
>>>>>>>>>   pci: remove limitation of the number of the available IRQs
>>>>>>>>>
>>>>>>>>>  drivers/pci/dwc/pci-exynos.c           |  18 --
>>>>>>>>>  drivers/pci/dwc/pci-imx6.c             |  18 --
>>>>>>>>>  drivers/pci/dwc/pci-keystone-dw.c      |  96 +-------
>>>>>>>>>  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 | 395 +++++++++++++++++++--------------
>>>>>>>>>  drivers/pci/dwc/pcie-designware-plat.c |  15 --
>>>>>>>>>  drivers/pci/dwc/pcie-designware.h      |  30 ++-
>>>>>>>>>  drivers/pci/dwc/pcie-qcom.c            |  15 --
>>>>>>>>>  11 files changed, 255 insertions(+), 359 deletions(-)
>>>>>>>>>
>>>>>>>>
>>>>>>>> I gave this a try today and it failed. Logs at
>>>>>>>> https://urldefense.proofpoint.com/v2/url?u=http-3A__pastebin.ubuntu.com_24843963_&d=DwIC-g&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=FR_WRctJN74GGjjHco1VUqNfINiGA7lGruObX4bplAY&s=8QYJU-SCW5JEmMwLUqbOEBJA5hio9vIyeNyotVRos4Q&e= 
>>>>>>>>
>>>>>>>> The first part of the log is with your patch series.
>>>>>>>> The second part is before applying the patch. You will see
>>>>>>>> that there is qc timeout log that shows the issue
>>>>>>>>
>>>>>>>> [   12.791852] ata2.00: qc timeout (cmd 0xec)
>>>>>>>> [   12.795947] ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4)
>>>>>>>> [   12.802061] ata2: limiting SATA link speed to 3.0 Gbps
>>>>>>>> [   13.301853] ata2: SATA link up 3.0 Gbps (SStatus 123 SControl 320)
>>>>>>>> [   23.511852] ata2.00: qc timeout (cmd 0xec)
>>>>>>>> [   23.515947] ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4)
>>>>>>>> [   24.021850] ata2: SATA link up 3.0 Gbps (SStatus 123 SControl 320)
>>>>>>>>
>>>>>>>> Is there any DT change I need to make for this? I didn't have a
>>>>>>>> chance to review your patch w.r.t Keystone. But I see you have change
>>>>>>>> the MSI IRq handling.
>>>>>>>>
>>>>>>>
>>>>>>> There is no need to change your DT. I think I will need to debug this issue
>>>>>>> further. Could you please advice a board containing this SoC?
>>>>>>>
>>>>>> I think I may be able to work with you on this. Keystone has different 
>>>>>> register set for MSI IRQ. It is in application space. Also there are
>>>>>> 8 interrupts to ARM GIC that multiplexes the 32 MSI interrupts. So
>>>>>> please review the original code vs the changed one w.r.t to this. I haven't
>>>>>> had a chance to review the code as I am in the middle of a release.
>>>>>> But will be able to spend some time after two weeks. Meanwhile please
>>>>>> spend some time reviewing the code and exchange email to identify
>>>>>> the issue. I can add specific debug prints for you as well and provide
>>>>>> you the log for analysis.
>>>>>
>>>>> Ok, I agree! Thanks!
>>>>>
>>>>>>
>>>> Look at this logic in the patch where a potential issue exists for Keystone.
>>>>
>>>> +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 = lower_32_bits(msi_target);
>>>> +       msg->address_hi = upper_32_bits(msi_target);
>>>> +
>>>> +       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);
>>>> +}
>>>>
>>>> Keystone has 8 hw irqs, 4 multiplexed over each since the register
>>>> set is different than other DWC cores. So will not work for Keystone,
>>>> right?
>>>
>>> Well, that can be challenging, since it is a very particular operation.
>>> Maybe we should implement a _pci_setup_msi_msg in the keystone driver and
>>> overide this one if necessary. What do you think?
>>
>> At that rate, and given that keystone seems to override almost all
>> operations in this driver, wouldn't it make sense for this to be
>> implemented as a *separate* driver?
> Marc,
> 
> What you mean by separate driver? Just the MSI handling part? 

The MSI part indeed. Looking at this series, KS looks like a sore spot
on the face of rest of the MSI code, and it'd be better of opting out of
this MSI management code and provide its own.

It seems to me that this would simplify the DW MSI code (no more
indirections all over the place), and make it obvious that KW MSIs are
managed in a very different way.

Thoughts?

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

Re: [PATCH v2 0/9] add new irq api to pcie-designware

[Joao Pinto]

Hi to all,

Às 9:02 AM de 7/6/2017, Marc Zyngier escreveu:
> On 05/07/17 22:03, Murali Karicheri wrote:
>> On 07/05/2017 11:26 AM, Marc Zyngier wrote:
>>> On 05/07/17 11:57, Joao Pinto wrote:
>>>> Às 6:05 PM de 6/20/2017, Murali Karicheri escreveu:
>>>>> On 06/20/2017 12:28 PM, Joao Pinto wrote:
>>>>>> Às 4:50 PM de 6/20/2017, Murali Karicheri escreveu:
>>>>>>> On 06/19/2017 12:31 PM, Joao Pinto wrote:
>>>>>>>>
>>>>>>>> Hi Murali,
>>>>>>>>
>>>>>>>> Às 10:18 PM de 6/12/2017, Murali Karicheri escreveu:
>>>>>>>>> Joao,
>>>>>>>>>
>>>>>>>>> On 06/05/2017 12:19 PM, Joao Pinto wrote:
>>>>>>>>>> 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.
>>>>>>>>>>
>>>>>>>>>> The patch set was made against the Bjorn' next branch.
>>>>>>>>>>
>>>>>>>>>> Joao Pinto (9):
>>>>>>>>>>   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
>>>>>>>>>>   pci: remove limitation of the number of the available IRQs
>>>>>>>>>>
>>>>>>>>>>  drivers/pci/dwc/pci-exynos.c           |  18 --
>>>>>>>>>>  drivers/pci/dwc/pci-imx6.c             |  18 --
>>>>>>>>>>  drivers/pci/dwc/pci-keystone-dw.c      |  96 +-------
>>>>>>>>>>  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 | 395 +++++++++++++++++++--------------
>>>>>>>>>>  drivers/pci/dwc/pcie-designware-plat.c |  15 --
>>>>>>>>>>  drivers/pci/dwc/pcie-designware.h      |  30 ++-
>>>>>>>>>>  drivers/pci/dwc/pcie-qcom.c            |  15 --
>>>>>>>>>>  11 files changed, 255 insertions(+), 359 deletions(-)
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I gave this a try today and it failed. Logs at
>>>>>>>>> https://urldefense.proofpoint.com/v2/url?u=http-3A__pastebin.ubuntu.com_24843963_&d=DwIC-g&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=FR_WRctJN74GGjjHco1VUqNfINiGA7lGruObX4bplAY&s=8QYJU-SCW5JEmMwLUqbOEBJA5hio9vIyeNyotVRos4Q&e= 
>>>>>>>>>
>>>>>>>>> The first part of the log is with your patch series.
>>>>>>>>> The second part is before applying the patch. You will see
>>>>>>>>> that there is qc timeout log that shows the issue
>>>>>>>>>
>>>>>>>>> [   12.791852] ata2.00: qc timeout (cmd 0xec)
>>>>>>>>> [   12.795947] ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4)
>>>>>>>>> [   12.802061] ata2: limiting SATA link speed to 3.0 Gbps
>>>>>>>>> [   13.301853] ata2: SATA link up 3.0 Gbps (SStatus 123 SControl 320)
>>>>>>>>> [   23.511852] ata2.00: qc timeout (cmd 0xec)
>>>>>>>>> [   23.515947] ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4)
>>>>>>>>> [   24.021850] ata2: SATA link up 3.0 Gbps (SStatus 123 SControl 320)
>>>>>>>>>
>>>>>>>>> Is there any DT change I need to make for this? I didn't have a
>>>>>>>>> chance to review your patch w.r.t Keystone. But I see you have change
>>>>>>>>> the MSI IRq handling.
>>>>>>>>>
>>>>>>>>
>>>>>>>> There is no need to change your DT. I think I will need to debug this issue
>>>>>>>> further. Could you please advice a board containing this SoC?
>>>>>>>>
>>>>>>> I think I may be able to work with you on this. Keystone has different 
>>>>>>> register set for MSI IRQ. It is in application space. Also there are
>>>>>>> 8 interrupts to ARM GIC that multiplexes the 32 MSI interrupts. So
>>>>>>> please review the original code vs the changed one w.r.t to this. I haven't
>>>>>>> had a chance to review the code as I am in the middle of a release.
>>>>>>> But will be able to spend some time after two weeks. Meanwhile please
>>>>>>> spend some time reviewing the code and exchange email to identify
>>>>>>> the issue. I can add specific debug prints for you as well and provide
>>>>>>> you the log for analysis.
>>>>>>
>>>>>> Ok, I agree! Thanks!
>>>>>>
>>>>>>>
>>>>> Look at this logic in the patch where a potential issue exists for Keystone.
>>>>>
>>>>> +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 = lower_32_bits(msi_target);
>>>>> +       msg->address_hi = upper_32_bits(msi_target);
>>>>> +
>>>>> +       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);
>>>>> +}
>>>>>
>>>>> Keystone has 8 hw irqs, 4 multiplexed over each since the register
>>>>> set is different than other DWC cores. So will not work for Keystone,
>>>>> right?
>>>>
>>>> Well, that can be challenging, since it is a very particular operation.
>>>> Maybe we should implement a _pci_setup_msi_msg in the keystone driver and
>>>> overide this one if necessary. What do you think?
>>>
>>> At that rate, and given that keystone seems to override almost all
>>> operations in this driver, wouldn't it make sense for this to be
>>> implemented as a *separate* driver?
>> Marc,
>>
>> What you mean by separate driver? Just the MSI handling part? 
> 
> The MSI part indeed. Looking at this series, KS looks like a sore spot
> on the face of rest of the MSI code, and it'd be better of opting out of
> this MSI management code and provide its own.
> 
> It seems to me that this would simplify the DW MSI code (no more
> indirections all over the place), and make it obvious that KW MSIs are
> managed in a very different way.
> 
> Thoughts?

I agree with Mark. The core driver is full of callbacks for custom SoC
operations, and most of them are not massively used.
Keystone with no doubt has its own way of dealing with interrupts, so I fully
support the a keystone_msi driver to be created as altera does.

At this time my bandwidth is almost full, so I won't be able to do it. Muralli
what are your thoughts? Would you help on this task?

Thanks,
Joao


> 
> 	M.
> 

Re: [PATCH v2 0/9] add new irq api to pcie-designware

[Murali Karicheri]

On 07/06/2017 05:05 AM, Joao Pinto wrote:
> 
> Hi to all,
> 
> Às 9:02 AM de 7/6/2017, Marc Zyngier escreveu:
>> On 05/07/17 22:03, Murali Karicheri wrote:
>>> On 07/05/2017 11:26 AM, Marc Zyngier wrote:
>>>> On 05/07/17 11:57, Joao Pinto wrote:
>>>>> Às 6:05 PM de 6/20/2017, Murali Karicheri escreveu:
>>>>>> On 06/20/2017 12:28 PM, Joao Pinto wrote:
>>>>>>> Às 4:50 PM de 6/20/2017, Murali Karicheri escreveu:
>>>>>>>> On 06/19/2017 12:31 PM, Joao Pinto wrote:
>>>>>>>>>
>>>>>>>>> Hi Murali,
>>>>>>>>>
>>>>>>>>> Às 10:18 PM de 6/12/2017, Murali Karicheri escreveu:
>>>>>>>>>> Joao,
>>>>>>>>>>
>>>>>>>>>> On 06/05/2017 12:19 PM, Joao Pinto wrote:
>>>>>>>>>>> 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.
>>>>>>>>>>>
>>>>>>>>>>> The patch set was made against the Bjorn' next branch.
>>>>>>>>>>>
>>>>>>>>>>> Joao Pinto (9):
>>>>>>>>>>>   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
>>>>>>>>>>>   pci: remove limitation of the number of the available IRQs
>>>>>>>>>>>
>>>>>>>>>>>  drivers/pci/dwc/pci-exynos.c           |  18 --
>>>>>>>>>>>  drivers/pci/dwc/pci-imx6.c             |  18 --
>>>>>>>>>>>  drivers/pci/dwc/pci-keystone-dw.c      |  96 +-------
>>>>>>>>>>>  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 | 395 +++++++++++++++++++--------------
>>>>>>>>>>>  drivers/pci/dwc/pcie-designware-plat.c |  15 --
>>>>>>>>>>>  drivers/pci/dwc/pcie-designware.h      |  30 ++-
>>>>>>>>>>>  drivers/pci/dwc/pcie-qcom.c            |  15 --
>>>>>>>>>>>  11 files changed, 255 insertions(+), 359 deletions(-)
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> I gave this a try today and it failed. Logs at
>>>>>>>>>> https://urldefense.proofpoint.com/v2/url?u=http-3A__pastebin.ubuntu.com_24843963_&d=DwIC-g&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=FR_WRctJN74GGjjHco1VUqNfINiGA7lGruObX4bplAY&s=8QYJU-SCW5JEmMwLUqbOEBJA5hio9vIyeNyotVRos4Q&e= 
>>>>>>>>>>
>>>>>>>>>> The first part of the log is with your patch series.
>>>>>>>>>> The second part is before applying the patch. You will see
>>>>>>>>>> that there is qc timeout log that shows the issue
>>>>>>>>>>
>>>>>>>>>> [   12.791852] ata2.00: qc timeout (cmd 0xec)
>>>>>>>>>> [   12.795947] ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4)
>>>>>>>>>> [   12.802061] ata2: limiting SATA link speed to 3.0 Gbps
>>>>>>>>>> [   13.301853] ata2: SATA link up 3.0 Gbps (SStatus 123 SControl 320)
>>>>>>>>>> [   23.511852] ata2.00: qc timeout (cmd 0xec)
>>>>>>>>>> [   23.515947] ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4)
>>>>>>>>>> [   24.021850] ata2: SATA link up 3.0 Gbps (SStatus 123 SControl 320)
>>>>>>>>>>
>>>>>>>>>> Is there any DT change I need to make for this? I didn't have a
>>>>>>>>>> chance to review your patch w.r.t Keystone. But I see you have change
>>>>>>>>>> the MSI IRq handling.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> There is no need to change your DT. I think I will need to debug this issue
>>>>>>>>> further. Could you please advice a board containing this SoC?
>>>>>>>>>
>>>>>>>> I think I may be able to work with you on this. Keystone has different 
>>>>>>>> register set for MSI IRQ. It is in application space. Also there are
>>>>>>>> 8 interrupts to ARM GIC that multiplexes the 32 MSI interrupts. So
>>>>>>>> please review the original code vs the changed one w.r.t to this. I haven't
>>>>>>>> had a chance to review the code as I am in the middle of a release.
>>>>>>>> But will be able to spend some time after two weeks. Meanwhile please
>>>>>>>> spend some time reviewing the code and exchange email to identify
>>>>>>>> the issue. I can add specific debug prints for you as well and provide
>>>>>>>> you the log for analysis.
>>>>>>>
>>>>>>> Ok, I agree! Thanks!
>>>>>>>
>>>>>>>>
>>>>>> Look at this logic in the patch where a potential issue exists for Keystone.
>>>>>>
>>>>>> +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 = lower_32_bits(msi_target);
>>>>>> +       msg->address_hi = upper_32_bits(msi_target);
>>>>>> +
>>>>>> +       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);
>>>>>> +}
>>>>>>
>>>>>> Keystone has 8 hw irqs, 4 multiplexed over each since the register
>>>>>> set is different than other DWC cores. So will not work for Keystone,
>>>>>> right?
>>>>>
>>>>> Well, that can be challenging, since it is a very particular operation.
>>>>> Maybe we should implement a _pci_setup_msi_msg in the keystone driver and
>>>>> overide this one if necessary. What do you think?
>>>>
>>>> At that rate, and given that keystone seems to override almost all
>>>> operations in this driver, wouldn't it make sense for this to be
>>>> implemented as a *separate* driver?
>>> Marc,
>>>
>>> What you mean by separate driver? Just the MSI handling part? 
>>
>> The MSI part indeed. Looking at this series, KS looks like a sore spot
>> on the face of rest of the MSI code, and it'd be better of opting out of
>> this MSI management code and provide its own.
>>
>> It seems to me that this would simplify the DW MSI code (no more
>> indirections all over the place), and make it obvious that KW MSIs are
>> managed in a very different way.
>>
>> Thoughts?
> 
> I agree with Mark. The core driver is full of callbacks for custom SoC
> operations, and most of them are not massively used.
> Keystone with no doubt has its own way of dealing with interrupts, so I fully
> support the a keystone_msi driver to be created as altera does.
> 
> At this time my bandwidth is almost full, so I won't be able to do it. Muralli
> what are your thoughts? Would you help on this task?
I agree as well. If I recall, that was how the initial version of the driver
looked like. But then based on review, it was changed to use core API and
I had to add the callback functions into keystone dwc to handle msi. 

Unfortunately I am running into the same situation as Joao and afraid I can
get to this anytime soon. Is there a possibility of excluding Keystone from this
patch set without breaking the driver? If so we need an interim solution to
have keystone not affected by this change and I can add the new driver later
when I get some bandwidth.

Murali

> 
> Thanks,
> Joao
> 
> 
>>
>> 	M.
>>
> 


-- 
Murali Karicheri
Linux Kernel, Keystone