[PATCH v4 1/6] PCI: layerscape: remove ls_pcie_establish_link()

[PATCH v4 1/6] PCI: layerscape: remove ls_pcie_establish_link()

[Minghuan Lian]

ls_pcie_establish_link() does not do any real operation, except
to wait for the linkup establishment. In fact, this is not
necessary. Moreover, each PCIe controller not inserted device
will increase the Linux startup time about 200ms.

Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
---
Change log
v4: split from [PATCH v3] PCI: layerscape: Add PCIe support for LS1043a and LS2080a

 drivers/pci/host/pci-layerscape.c | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/drivers/pci/host/pci-layerscape.c b/drivers/pci/host/pci-layerscape.c
index b2328ea1..6dd44a0 100644
--- a/drivers/pci/host/pci-layerscape.c
+++ b/drivers/pci/host/pci-layerscape.c
@@ -11,7 +11,6 @@
  */
 
 #include <linux/kernel.h>
-#include <linux/delay.h>
 #include <linux/interrupt.h>
 #include <linux/module.h>
 #include <linux/of_pci.h>
@@ -62,27 +61,12 @@ static int ls_pcie_link_up(struct pcie_port *pp)
 	return 1;
 }
 
-static int ls_pcie_establish_link(struct pcie_port *pp)
-{
-	unsigned int retries;
-
-	for (retries = 0; retries < 200; retries++) {
-		if (dw_pcie_link_up(pp))
-			return 0;
-		usleep_range(100, 1000);
-	}
-
-	dev_err(pp->dev, "phy link never came up\n");
-	return -EINVAL;
-}
-
 static void ls_pcie_host_init(struct pcie_port *pp)
 {
 	struct ls_pcie *pcie = to_ls_pcie(pp);
 	u32 val;
 
 	dw_pcie_setup_rc(pp);
-	ls_pcie_establish_link(pp);
 
 	/*
 	 * LS1021A Workaround for internal TKT228622
-- 
1.9.1

[PATCH v4 2/6] PCI: layerscape: check PCIe controller work mode

[Minghuan Lian]

Layerscape PCIe controller supports root complex (RC) and
endpoint (EP) mode. The mode can be set by RCW. The patch
will check it. If not in RC mode, the driver directly
returns -ENODEV.

Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
---
Change log
v4: split from [PATCH v3] PCI: layerscape: Add PCIe support for LS1043a and LS2080a

 drivers/pci/host/pci-layerscape.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/pci/host/pci-layerscape.c b/drivers/pci/host/pci-layerscape.c
index 6dd44a0..5eabe92 100644
--- a/drivers/pci/host/pci-layerscape.c
+++ b/drivers/pci/host/pci-layerscape.c
@@ -47,6 +47,16 @@ struct ls_pcie {
 
 #define to_ls_pcie(x)	container_of(x, struct ls_pcie, pp)
 
+static bool ls_pcie_is_bridge(struct ls_pcie *pcie)
+{
+	u32 header_type;
+
+	header_type = ioread8(pcie->dbi + PCI_HEADER_TYPE);
+	header_type &= 0x7f;
+
+	return header_type == PCI_HEADER_TYPE_BRIDGE;
+}
+
 static int ls_pcie_link_up(struct pcie_port *pp)
 {
 	u32 state;
@@ -135,6 +145,9 @@ static int __init ls_pcie_probe(struct platform_device *pdev)
 		return ret;
 	pcie->index = index[1];
 
+	if (!ls_pcie_is_bridge(pcie))
+		return -ENODEV;
+
 	ret = ls_add_pcie_port(pcie);
 	if (ret < 0)
 		return ret;
-- 
1.9.1

[PATCH v4 3/6] PCI: layerscape: factor out SCFG related function

[Minghuan Lian]

For LS1021a PCIe controller, some status registers are located
in SCFG. This is different to other kind of Layerscape. The
patch moves SCFG related code to ls1021_pcie_host_init() and
rename ls_pcie_link_up to ls1021_pcie_link_up because LTSSM
status is also in SCFG.

Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
---
Change log
v4: split from [PATCH v3] PCI: layerscape: Add PCIe support for LS1043a and LS2080a

 drivers/pci/host/pci-layerscape.c | 72 +++++++++++++++++++++++++--------------
 1 file changed, 46 insertions(+), 26 deletions(-)

diff --git a/drivers/pci/host/pci-layerscape.c b/drivers/pci/host/pci-layerscape.c
index 5eabe92..89b7eb8 100644
--- a/drivers/pci/host/pci-layerscape.c
+++ b/drivers/pci/host/pci-layerscape.c
@@ -34,6 +34,10 @@
 /* Symbol Timer Register and Filter Mask Register 1 */
 #define PCIE_STRFMR1 0x71c
 
+struct ls_pcie_drvdata {
+	struct pcie_host_ops *ops;
+};
+
 struct ls_pcie {
 	struct list_head node;
 	struct device *dev;
@@ -41,6 +45,7 @@ struct ls_pcie {
 	void __iomem *dbi;
 	struct regmap *scfg;
 	struct pcie_port pp;
+	const struct ls_pcie_drvdata *drvdata;
 	int index;
 	int msi_irq;
 };
@@ -57,11 +62,14 @@ static bool ls_pcie_is_bridge(struct ls_pcie *pcie)
 	return header_type == PCI_HEADER_TYPE_BRIDGE;
 }
 
-static int ls_pcie_link_up(struct pcie_port *pp)
+static int ls1021_pcie_link_up(struct pcie_port *pp)
 {
 	u32 state;
 	struct ls_pcie *pcie = to_ls_pcie(pp);
 
+	if (!pcie->scfg)
+		return 0;
+
 	regmap_read(pcie->scfg, SCFG_PEXMSCPORTSR(pcie->index), &state);
 	state = (state >> LTSSM_STATE_SHIFT) & LTSSM_STATE_MASK;
 
@@ -71,10 +79,25 @@ static int ls_pcie_link_up(struct pcie_port *pp)
 	return 1;
 }
 
-static void ls_pcie_host_init(struct pcie_port *pp)
+static void ls1021_pcie_host_init(struct pcie_port *pp)
 {
 	struct ls_pcie *pcie = to_ls_pcie(pp);
-	u32 val;
+	u32 val, index[2];
+
+	pcie->scfg = syscon_regmap_lookup_by_phandle(pp->dev->of_node,
+						     "fsl,pcie-scfg");
+	if (IS_ERR(pcie->scfg)) {
+		dev_err(pp->dev, "No syscfg phandle specified\n");
+		pcie->scfg = NULL;
+		return;
+	}
+
+	if (of_property_read_u32_array(pp->dev->of_node,
+				       "fsl,pcie-scfg", index, 2)) {
+		pcie->scfg = NULL;
+		return;
+	}
+	pcie->index = index[1];
 
 	dw_pcie_setup_rc(pp);
 
@@ -87,11 +110,21 @@ static void ls_pcie_host_init(struct pcie_port *pp)
 	iowrite32(val, pcie->dbi + PCIE_STRFMR1);
 }
 
-static struct pcie_host_ops ls_pcie_host_ops = {
-	.link_up = ls_pcie_link_up,
-	.host_init = ls_pcie_host_init,
+static struct pcie_host_ops ls1021_pcie_host_ops = {
+	.link_up = ls1021_pcie_link_up,
+	.host_init = ls1021_pcie_host_init,
+};
+
+static struct ls_pcie_drvdata ls1021_drvdata = {
+	.ops = &ls1021_pcie_host_ops,
 };
 
+static const struct of_device_id ls_pcie_of_match[] = {
+	{ .compatible = "fsl,ls1021a-pcie", .data = &ls1021_drvdata },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, ls_pcie_of_match);
+
 static int ls_add_pcie_port(struct ls_pcie *pcie)
 {
 	struct pcie_port *pp;
@@ -101,7 +134,7 @@ static int ls_add_pcie_port(struct ls_pcie *pcie)
 	pp->dev = pcie->dev;
 	pp->dbi_base = pcie->dbi;
 	pp->root_bus_nr = -1;
-	pp->ops = &ls_pcie_host_ops;
+	pp->ops = pcie->drvdata->ops;
 
 	ret = dw_pcie_host_init(pp);
 	if (ret) {
@@ -114,11 +147,15 @@ static int ls_add_pcie_port(struct ls_pcie *pcie)
 
 static int __init ls_pcie_probe(struct platform_device *pdev)
 {
+	const struct of_device_id *match;
 	struct ls_pcie *pcie;
 	struct resource *dbi_base;
-	u32 index[2];
 	int ret;
 
+	match = of_match_device(ls_pcie_of_match, &pdev->dev);
+	if (!match)
+		return -ENODEV;
+
 	pcie = devm_kzalloc(&pdev->dev, sizeof(*pcie), GFP_KERNEL);
 	if (!pcie)
 		return -ENOMEM;
@@ -132,18 +169,7 @@ static int __init ls_pcie_probe(struct platform_device *pdev)
 		return PTR_ERR(pcie->dbi);
 	}
 
-	pcie->scfg = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
-						     "fsl,pcie-scfg");
-	if (IS_ERR(pcie->scfg)) {
-		dev_err(&pdev->dev, "No syscfg phandle specified\n");
-		return PTR_ERR(pcie->scfg);
-	}
-
-	ret = of_property_read_u32_array(pdev->dev.of_node,
-					 "fsl,pcie-scfg", index, 2);
-	if (ret)
-		return ret;
-	pcie->index = index[1];
+	pcie->drvdata = match->data;
 
 	if (!ls_pcie_is_bridge(pcie))
 		return -ENODEV;
@@ -157,12 +183,6 @@ static int __init ls_pcie_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static const struct of_device_id ls_pcie_of_match[] = {
-	{ .compatible = "fsl,ls1021a-pcie" },
-	{ },
-};
-MODULE_DEVICE_TABLE(of, ls_pcie_of_match);
-
 static struct platform_driver ls_pcie_driver = {
 	.driver = {
 		.name = "layerscape-pcie",
-- 
1.9.1

[PATCH v4 4/6] PCI: layerscape: update ls_add_pcie_port()

[Minghuan Lian]

To keep consistent with the other DesignWare-based host drivers,
The patch updates ls_add_pcie_port().

Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
---
Change log
v4: split from [PATCH v3] PCI: layerscape: Add PCIe support for LS1043a and LS2080a

 drivers/pci/host/pci-layerscape.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/host/pci-layerscape.c b/drivers/pci/host/pci-layerscape.c
index 89b7eb8..891e504 100644
--- a/drivers/pci/host/pci-layerscape.c
+++ b/drivers/pci/host/pci-layerscape.c
@@ -125,15 +125,14 @@ static const struct of_device_id ls_pcie_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, ls_pcie_of_match);
 
-static int ls_add_pcie_port(struct ls_pcie *pcie)
+static int __init
+ls_add_pcie_port(struct pcie_port *pp, struct platform_device *pdev)
 {
-	struct pcie_port *pp;
 	int ret;
+	struct ls_pcie *pcie = to_ls_pcie(pp);
 
-	pp = &pcie->pp;
-	pp->dev = pcie->dev;
+	pp->dev = &pdev->dev;
 	pp->dbi_base = pcie->dbi;
-	pp->root_bus_nr = -1;
 	pp->ops = pcie->drvdata->ops;
 
 	ret = dw_pcie_host_init(pp);
@@ -160,8 +159,6 @@ static int __init ls_pcie_probe(struct platform_device *pdev)
 	if (!pcie)
 		return -ENOMEM;
 
-	pcie->dev = &pdev->dev;
-
 	dbi_base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
 	pcie->dbi = devm_ioremap_resource(&pdev->dev, dbi_base);
 	if (IS_ERR(pcie->dbi)) {
@@ -174,7 +171,7 @@ static int __init ls_pcie_probe(struct platform_device *pdev)
 	if (!ls_pcie_is_bridge(pcie))
 		return -ENODEV;
 
-	ret = ls_add_pcie_port(pcie);
+	ret = ls_add_pcie_port(&pcie->pp, pdev);
 	if (ret < 0)
 		return ret;
 
-- 
1.9.1

[PATCH v4 5/6] PCI: layerscape: add PCIe support for LS1043a and LS2080a

[Minghuan Lian]

Both LS1043a and LS2080a are based on ARMv8 64-bit architecture
and have similar PCIe implementation. LUT is added to controller.
The patch removes the necessary fields from struct ls_pcie.

Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
---
This patch is based on v4.3-rc4 and [PATCH v10 3/6]
PCI: designware: Add ARM64 support.

change log
v4: 
1. split to 6 patches.
2. use ARCH_LAYERSCAPE instead of ARM64

v3:
1. Use 8 or 16 bit access function to simplify code
2. Add ls_add_pcie_port in accordance with other DesignWare-based drivers

v2:
1. Rename ls2085a to ls2080a
2. Add ls_pcie_msi_host_init()

 drivers/pci/host/Kconfig          |  2 +-
 drivers/pci/host/pci-layerscape.c | 72 +++++++++++++++++++++++++++++++++++----
 2 files changed, 67 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index ae873be..8eb09ea 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -105,7 +105,7 @@ config PCI_XGENE_MSI
 
 config PCI_LAYERSCAPE
 	bool "Freescale Layerscape PCIe controller"
-	depends on OF && ARM
+	depends on OF && (ARM || ARCH_LAYERSCAPE)
 	select PCIE_DW
 	select MFD_SYSCON
 	help
diff --git a/drivers/pci/host/pci-layerscape.c b/drivers/pci/host/pci-layerscape.c
index 891e504..c53692a 100644
--- a/drivers/pci/host/pci-layerscape.c
+++ b/drivers/pci/host/pci-layerscape.c
@@ -31,23 +31,26 @@
 #define LTSSM_STATE_MASK	0x3f
 #define LTSSM_PCIE_L0		0x11 /* L0 state */
 
-/* Symbol Timer Register and Filter Mask Register 1 */
-#define PCIE_STRFMR1 0x71c
+/* PEX Internal Configuration Registers */
+#define PCIE_STRFMR1		0x71c /* Symbol Timer & Filter Mask Register1 */
+#define PCIE_DBI_RO_WR_EN	0x8bc /* DBI Read-Only Write Enable Register */
+
+/* PEX LUT registers */
+#define PCIE_LUT_DBG		0x7FC /* PEX LUT Debug Register */
 
 struct ls_pcie_drvdata {
+	u32 lut_offset;
+	u32 ltssm_shift;
 	struct pcie_host_ops *ops;
 };
 
 struct ls_pcie {
-	struct list_head node;
-	struct device *dev;
-	struct pci_bus *bus;
 	void __iomem *dbi;
+	void __iomem *lut;
 	struct regmap *scfg;
 	struct pcie_port pp;
 	const struct ls_pcie_drvdata *drvdata;
 	int index;
-	int msi_irq;
 };
 
 #define to_ls_pcie(x)	container_of(x, struct ls_pcie, pp)
@@ -62,6 +65,18 @@ static bool ls_pcie_is_bridge(struct ls_pcie *pcie)
 	return header_type == PCI_HEADER_TYPE_BRIDGE;
 }
 
+/* Clear multi-function bit */
+static void ls_pcie_clear_multifunction(struct ls_pcie *pcie)
+{
+	iowrite8(PCI_HEADER_TYPE_BRIDGE, pcie->dbi + PCI_HEADER_TYPE);
+}
+
+/* Fix class value */
+static void ls_pcie_fix_class(struct ls_pcie *pcie)
+{
+	iowrite16(PCI_CLASS_BRIDGE_PCI, pcie->dbi + PCI_CLASS_DEVICE);
+}
+
 static int ls1021_pcie_link_up(struct pcie_port *pp)
 {
 	u32 state;
@@ -110,17 +125,61 @@ static void ls1021_pcie_host_init(struct pcie_port *pp)
 	iowrite32(val, pcie->dbi + PCIE_STRFMR1);
 }
 
+static int ls_pcie_link_up(struct pcie_port *pp)
+{
+	struct ls_pcie *pcie = to_ls_pcie(pp);
+	u32 state;
+
+	state = (ioread32(pcie->lut + PCIE_LUT_DBG) >>
+		 pcie->drvdata->ltssm_shift) &
+		 LTSSM_STATE_MASK;
+
+	if (state < LTSSM_PCIE_L0)
+		return 0;
+
+	return 1;
+}
+
+static void ls_pcie_host_init(struct pcie_port *pp)
+{
+	struct ls_pcie *pcie = to_ls_pcie(pp);
+
+	iowrite32(1, pcie->dbi + PCIE_DBI_RO_WR_EN);
+	ls_pcie_fix_class(pcie);
+	ls_pcie_clear_multifunction(pcie);
+	iowrite32(0, pcie->dbi + PCIE_DBI_RO_WR_EN);
+}
+
 static struct pcie_host_ops ls1021_pcie_host_ops = {
 	.link_up = ls1021_pcie_link_up,
 	.host_init = ls1021_pcie_host_init,
 };
 
+static struct pcie_host_ops ls_pcie_host_ops = {
+	.link_up = ls_pcie_link_up,
+	.host_init = ls_pcie_host_init,
+};
+
 static struct ls_pcie_drvdata ls1021_drvdata = {
 	.ops = &ls1021_pcie_host_ops,
 };
 
+static struct ls_pcie_drvdata ls1043_drvdata = {
+	.lut_offset = 0x10000,
+	.ltssm_shift = 24,
+	.ops = &ls_pcie_host_ops,
+};
+
+static struct ls_pcie_drvdata ls2080_drvdata = {
+	.lut_offset = 0x80000,
+	.ltssm_shift = 0,
+	.ops = &ls_pcie_host_ops,
+};
+
 static const struct of_device_id ls_pcie_of_match[] = {
 	{ .compatible = "fsl,ls1021a-pcie", .data = &ls1021_drvdata },
+	{ .compatible = "fsl,ls1043a-pcie", .data = &ls1043_drvdata },
+	{ .compatible = "fsl,ls2080a-pcie", .data = &ls2080_drvdata },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, ls_pcie_of_match);
@@ -167,6 +226,7 @@ static int __init ls_pcie_probe(struct platform_device *pdev)
 	}
 
 	pcie->drvdata = match->data;
+	pcie->lut = pcie->dbi + pcie->drvdata->lut_offset;
 
 	if (!ls_pcie_is_bridge(pcie))
 		return -ENODEV;
-- 
1.9.1

[PATCH v4 5/6] PCI: layerscape: add PCIe support for LS1043a and LS2080a

[Bjorn Helgaas]

On Fri, Oct 16, 2015 at 03:19:19PM +0800, Minghuan Lian wrote:
> Both LS1043a and LS2080a are based on ARMv8 64-bit architecture
> and have similar PCIe implementation. LUT is added to controller.
> The patch removes the necessary fields from struct ls_pcie.
> 
> Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
> ---
> This patch is based on v4.3-rc4 and [PATCH v10 3/6]
> PCI: designware: Add ARM64 support.
> 
> change log
> v4: 
> 1. split to 6 patches.
> 2. use ARCH_LAYERSCAPE instead of ARM64
> 
> v3:
> 1. Use 8 or 16 bit access function to simplify code
> 2. Add ls_add_pcie_port in accordance with other DesignWare-based drivers
> 
> v2:
> 1. Rename ls2085a to ls2080a
> 2. Add ls_pcie_msi_host_init()
> 
>  drivers/pci/host/Kconfig          |  2 +-
>  drivers/pci/host/pci-layerscape.c | 72 +++++++++++++++++++++++++++++++++++----
>  2 files changed, 67 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index ae873be..8eb09ea 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -105,7 +105,7 @@ config PCI_XGENE_MSI
>  
>  config PCI_LAYERSCAPE
>  	bool "Freescale Layerscape PCIe controller"
> -	depends on OF && ARM
> +	depends on OF && (ARM || ARCH_LAYERSCAPE)
>  	select PCIE_DW
>  	select MFD_SYSCON
>  	help
> diff --git a/drivers/pci/host/pci-layerscape.c b/drivers/pci/host/pci-layerscape.c
> index 891e504..c53692a 100644
> --- a/drivers/pci/host/pci-layerscape.c
> +++ b/drivers/pci/host/pci-layerscape.c
> @@ -31,23 +31,26 @@
>  #define LTSSM_STATE_MASK	0x3f
>  #define LTSSM_PCIE_L0		0x11 /* L0 state */
>  
> -/* Symbol Timer Register and Filter Mask Register 1 */
> -#define PCIE_STRFMR1 0x71c
> +/* PEX Internal Configuration Registers */
> +#define PCIE_STRFMR1		0x71c /* Symbol Timer & Filter Mask Register1 */
> +#define PCIE_DBI_RO_WR_EN	0x8bc /* DBI Read-Only Write Enable Register */
> +
> +/* PEX LUT registers */
> +#define PCIE_LUT_DBG		0x7FC /* PEX LUT Debug Register */
>  
>  struct ls_pcie_drvdata {
> +	u32 lut_offset;
> +	u32 ltssm_shift;
>  	struct pcie_host_ops *ops;
>  };
>  
>  struct ls_pcie {
> -	struct list_head node;
> -	struct device *dev;
> -	struct pci_bus *bus;

These fields look unused and unrelated to LS1043a and LS2080a.  I split
their removal into a separate patch.

>  	void __iomem *dbi;
> +	void __iomem *lut;
>  	struct regmap *scfg;
>  	struct pcie_port pp;
>  	const struct ls_pcie_drvdata *drvdata;
>  	int index;
> -	int msi_irq;
>  };
>  
>  #define to_ls_pcie(x)	container_of(x, struct ls_pcie, pp)
> @@ -62,6 +65,18 @@ static bool ls_pcie_is_bridge(struct ls_pcie *pcie)
>  	return header_type == PCI_HEADER_TYPE_BRIDGE;
>  }
>  
> +/* Clear multi-function bit */
> +static void ls_pcie_clear_multifunction(struct ls_pcie *pcie)
> +{
> +	iowrite8(PCI_HEADER_TYPE_BRIDGE, pcie->dbi + PCI_HEADER_TYPE);
> +}
> +
> +/* Fix class value */
> +static void ls_pcie_fix_class(struct ls_pcie *pcie)
> +{
> +	iowrite16(PCI_CLASS_BRIDGE_PCI, pcie->dbi + PCI_CLASS_DEVICE);
> +}
> +
>  static int ls1021_pcie_link_up(struct pcie_port *pp)
>  {
>  	u32 state;
> @@ -110,17 +125,61 @@ static void ls1021_pcie_host_init(struct pcie_port *pp)
>  	iowrite32(val, pcie->dbi + PCIE_STRFMR1);
>  }
>  
> +static int ls_pcie_link_up(struct pcie_port *pp)
> +{
> +	struct ls_pcie *pcie = to_ls_pcie(pp);
> +	u32 state;
> +
> +	state = (ioread32(pcie->lut + PCIE_LUT_DBG) >>
> +		 pcie->drvdata->ltssm_shift) &
> +		 LTSSM_STATE_MASK;
> +
> +	if (state < LTSSM_PCIE_L0)
> +		return 0;
> +
> +	return 1;
> +}
> +
> +static void ls_pcie_host_init(struct pcie_port *pp)
> +{
> +	struct ls_pcie *pcie = to_ls_pcie(pp);
> +
> +	iowrite32(1, pcie->dbi + PCIE_DBI_RO_WR_EN);
> +	ls_pcie_fix_class(pcie);
> +	ls_pcie_clear_multifunction(pcie);
> +	iowrite32(0, pcie->dbi + PCIE_DBI_RO_WR_EN);
> +}
> +
>  static struct pcie_host_ops ls1021_pcie_host_ops = {
>  	.link_up = ls1021_pcie_link_up,
>  	.host_init = ls1021_pcie_host_init,
>  };
>  
> +static struct pcie_host_ops ls_pcie_host_ops = {
> +	.link_up = ls_pcie_link_up,
> +	.host_init = ls_pcie_host_init,
> +};
> +
>  static struct ls_pcie_drvdata ls1021_drvdata = {
>  	.ops = &ls1021_pcie_host_ops,
>  };
>  
> +static struct ls_pcie_drvdata ls1043_drvdata = {
> +	.lut_offset = 0x10000,
> +	.ltssm_shift = 24,
> +	.ops = &ls_pcie_host_ops,
> +};
> +
> +static struct ls_pcie_drvdata ls2080_drvdata = {
> +	.lut_offset = 0x80000,
> +	.ltssm_shift = 0,
> +	.ops = &ls_pcie_host_ops,
> +};
> +
>  static const struct of_device_id ls_pcie_of_match[] = {
>  	{ .compatible = "fsl,ls1021a-pcie", .data = &ls1021_drvdata },
> +	{ .compatible = "fsl,ls1043a-pcie", .data = &ls1043_drvdata },
> +	{ .compatible = "fsl,ls2080a-pcie", .data = &ls2080_drvdata },
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(of, ls_pcie_of_match);
> @@ -167,6 +226,7 @@ static int __init ls_pcie_probe(struct platform_device *pdev)
>  	}
>  
>  	pcie->drvdata = match->data;
> +	pcie->lut = pcie->dbi + pcie->drvdata->lut_offset;
>  
>  	if (!ls_pcie_is_bridge(pcie))
>  		return -ENODEV;
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 5/6] PCI: layerscape: add PCIe support for LS1043a and LS2080a

[Li Yang]

On Wed, Oct 21, 2015 at 4:36 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> On Fri, Oct 16, 2015 at 03:19:19PM +0800, Minghuan Lian wrote:
>> Both LS1043a and LS2080a are based on ARMv8 64-bit architecture
>> and have similar PCIe implementation. LUT is added to controller.
>> The patch removes the necessary fields from struct ls_pcie.
>>
>> Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
>> ---
>> This patch is based on v4.3-rc4 and [PATCH v10 3/6]
>> PCI: designware: Add ARM64 support.

Hi Bjorn,

Are you also applying this patch
https://patchwork.ozlabs.org/patch/531184/ in this merge windows?
Because the new hardware can only work with the generic ARM64 support
added in the designware pcie driver.

Regards,
Leo

[PATCH v4 5/6] PCI: layerscape: add PCIe support for LS1043a and LS2080a

[Bjorn Helgaas]

Hi Leo,

On Thu, Oct 22, 2015 at 12:38:48PM -0500, Li Yang wrote:
> On Wed, Oct 21, 2015 at 4:36 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> > On Fri, Oct 16, 2015 at 03:19:19PM +0800, Minghuan Lian wrote:
> >> Both LS1043a and LS2080a are based on ARMv8 64-bit architecture
> >> and have similar PCIe implementation. LUT is added to controller.
> >> The patch removes the necessary fields from struct ls_pcie.
> >>
> >> Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
> >> ---
> >> This patch is based on v4.3-rc4 and [PATCH v10 3/6]
> >> PCI: designware: Add ARM64 support.
> 
> Hi Bjorn,
> 
> Are you also applying this patch
> https://patchwork.ozlabs.org/patch/531184/ in this merge windows?
> Because the new hardware can only work with the generic ARM64 support
> added in the designware pcie driver.

I'm reviewing that patch right now, and I do hope to apply it for
v4.4.

There was no cover letter for this series (Minghuan's Layerscape v4
series), and I didn't see any mention of dependencies, so I applied it
on top of v4.3-rc1.  Is that the wrong thing to do?  If it depends on
the Zhou's v11 series, I can hold the Layerscape branch until after
that.

Bjorn

[PATCH v4 5/6] PCI: layerscape: add PCIe support for LS1043a and LS2080a

[Li Yang]

On Thu, Oct 22, 2015 at 1:08 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> Hi Leo,
>
> On Thu, Oct 22, 2015 at 12:38:48PM -0500, Li Yang wrote:
>> On Wed, Oct 21, 2015 at 4:36 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
>> > On Fri, Oct 16, 2015 at 03:19:19PM +0800, Minghuan Lian wrote:
>> >> Both LS1043a and LS2080a are based on ARMv8 64-bit architecture
>> >> and have similar PCIe implementation. LUT is added to controller.
>> >> The patch removes the necessary fields from struct ls_pcie.
>> >>
>> >> Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
>> >> ---
>> >> This patch is based on v4.3-rc4 and [PATCH v10 3/6]
>> >> PCI: designware: Add ARM64 support.
>>
>> Hi Bjorn,
>>
>> Are you also applying this patch
>> https://patchwork.ozlabs.org/patch/531184/ in this merge windows?
>> Because the new hardware can only work with the generic ARM64 support
>> added in the designware pcie driver.
>
> I'm reviewing that patch right now, and I do hope to apply it for
> v4.4.
>
> There was no cover letter for this series (Minghuan's Layerscape v4
> series), and I didn't see any mention of dependencies, so I applied it
> on top of v4.3-rc1.  Is that the wrong thing to do?  If it depends on
> the Zhou's v11 series, I can hold the Layerscape branch until after
> that.

I don't know if this patch will break anything without Zhou's patch.
But the new hardware mentioned in the description will not be working
without the dependent patch.  We should hold this patch until after
the generic change is merged.  Minghuan, are there any bug fixing
patch in the series that need to be merged earlier?

Regards,
Leo

[PATCH v4 5/6] PCI: layerscape: add PCIe support for LS1043a and LS2080a

[Lian M.H.]

Hi Bjorn and Leo,

> -----Original Message-----
> From: pku.leo at gmail.com [mailto:pku.leo at gmail.com] On Behalf Of Li Yang
> Sent: Friday, October 23, 2015 3:18 AM
> To: Bjorn Helgaas <helgaas@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>; linux-pci at vger.kernel.org; Jingoo Han
> <jg1.han@samsung.com>; Hu Mingkai-B21284 <Mingkai.Hu@freescale.com>;
> Zang Roy-R61911 <tie-fei.zang@freescale.com>; Yoder Stuart-B08248
> <stuart.yoder@freescale.com>; Lian Minghuan-B31939
> <Minghuan.Lian@freescale.com>; Zhou Wang <wangzhou1@hisilicon.com>;
> Bjorn Helgaas <bhelgaas@google.com>; linux-arm-kernel at lists.infradead.org
> Subject: Re: [PATCH v4 5/6] PCI: layerscape: add PCIe support for LS1043a and
> LS2080a
> 
> On Thu, Oct 22, 2015 at 1:08 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> > Hi Leo,
> >
> > On Thu, Oct 22, 2015 at 12:38:48PM -0500, Li Yang wrote:
> >> On Wed, Oct 21, 2015 at 4:36 PM, Bjorn Helgaas <helgaas@kernel.org>
> wrote:
> >> > On Fri, Oct 16, 2015 at 03:19:19PM +0800, Minghuan Lian wrote:
> >> >> Both LS1043a and LS2080a are based on ARMv8 64-bit architecture
> >> >> and have similar PCIe implementation. LUT is added to controller.
> >> >> The patch removes the necessary fields from struct ls_pcie.
> >> >>
> >> >> Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
> >> >> ---
> >> >> This patch is based on v4.3-rc4 and [PATCH v10 3/6]
> >> >> PCI: designware: Add ARM64 support.
> >>
> >> Hi Bjorn,
> >>
> >> Are you also applying this patch
> >> https://patchwork.ozlabs.org/patch/531184/ in this merge windows?
> >> Because the new hardware can only work with the generic ARM64 support
> >> added in the designware pcie driver.
> >
> > I'm reviewing that patch right now, and I do hope to apply it for
> > v4.4.
> >
> > There was no cover letter for this series (Minghuan's Layerscape v4
> > series), and I didn't see any mention of dependencies, so I applied it
> > on top of v4.3-rc1.  Is that the wrong thing to do?  If it depends on
> > the Zhou's v11 series, I can hold the Layerscape branch until after
> > that.
> 
> I don't know if this patch will break anything without Zhou's patch.
> But the new hardware mentioned in the description will not be working without
> the dependent patch.  We should hold this patch until after the generic
> change is merged.  Minghuan, are there any bug fixing patch in the series that
> need to be merged earlier?
> 
[Lian Minghuan-B31939] [PATCH v4 5/6] PCI: layerscape: add PCIe support for LS1043a and LS2080a] depended on Zhou's patch. I have described this in this patch. Actually, without zhou's patch, pci_layerscape.c can also be compiled and works well for LS1021A arm32 architecture, just do not support arm64 architecture. 

Thank Bjorn very much, all the PCIe patches have been applied except MSI related patches.

Thanks,
Minghuan
 


> Regards,
> Leo

[PATCH v4 5/6] PCI: layerscape: add PCIe support for LS1043a and LS2080a

[Li Yang]

On Thu, Oct 22, 2015 at 1:08 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> Hi Leo,
>
> On Thu, Oct 22, 2015 at 12:38:48PM -0500, Li Yang wrote:
>> On Wed, Oct 21, 2015 at 4:36 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
>> > On Fri, Oct 16, 2015 at 03:19:19PM +0800, Minghuan Lian wrote:
>> >> Both LS1043a and LS2080a are based on ARMv8 64-bit architecture
>> >> and have similar PCIe implementation. LUT is added to controller.
>> >> The patch removes the necessary fields from struct ls_pcie.
>> >>
>> >> Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
>> >> ---
>> >> This patch is based on v4.3-rc4 and [PATCH v10 3/6]
>> >> PCI: designware: Add ARM64 support.
>>
>> Hi Bjorn,
>>
>> Are you also applying this patch
>> https://patchwork.ozlabs.org/patch/531184/ in this merge windows?
>> Because the new hardware can only work with the generic ARM64 support
>> added in the designware pcie driver.
>
> I'm reviewing that patch right now, and I do hope to apply it for
> v4.4.
>
> There was no cover letter for this series (Minghuan's Layerscape v4
> series), and I didn't see any mention of dependencies, so I applied it
> on top of v4.3-rc1.  Is that the wrong thing to do?  If it depends on
> the Zhou's v11 series, I can hold the Layerscape branch until after
> that.

I noticed that you have merged Zhou's latest patch.  Probably you can
also merge the Layerscape branch now.

Regards,
Leo

[PATCH v4 5/6] PCI: layerscape: add PCIe support for LS1043a and LS2080a

[Bjorn Helgaas]

On Mon, Nov 02, 2015 at 03:08:04PM -0600, Li Yang wrote:
> On Thu, Oct 22, 2015 at 1:08 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> > Hi Leo,
> >
> > On Thu, Oct 22, 2015 at 12:38:48PM -0500, Li Yang wrote:
> >> On Wed, Oct 21, 2015 at 4:36 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> >> > On Fri, Oct 16, 2015 at 03:19:19PM +0800, Minghuan Lian wrote:
> >> >> Both LS1043a and LS2080a are based on ARMv8 64-bit architecture
> >> >> and have similar PCIe implementation. LUT is added to controller.
> >> >> The patch removes the necessary fields from struct ls_pcie.
> >> >>
> >> >> Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
> >> >> ---
> >> >> This patch is based on v4.3-rc4 and [PATCH v10 3/6]
> >> >> PCI: designware: Add ARM64 support.
> >>
> >> Hi Bjorn,
> >>
> >> Are you also applying this patch
> >> https://patchwork.ozlabs.org/patch/531184/ in this merge windows?
> >> Because the new hardware can only work with the generic ARM64 support
> >> added in the designware pcie driver.
> >
> > I'm reviewing that patch right now, and I do hope to apply it for
> > v4.4.
> >
> > There was no cover letter for this series (Minghuan's Layerscape v4
> > series), and I didn't see any mention of dependencies, so I applied it
> > on top of v4.3-rc1.  Is that the wrong thing to do?  If it depends on
> > the Zhou's v11 series, I can hold the Layerscape branch until after
> > that.
> 
> I noticed that you have merged Zhou's latest patch.  Probably you can
> also merge the Layerscape branch now.

That's my intention.

Bjorn

[PATCH v4 5/6] PCI: layerscape: add PCIe support for LS1043a and LS2080a

[Bjorn Helgaas]

[+cc Fabio, Lucas]

Hi Minghuan,

On Fri, Oct 16, 2015 at 03:19:19PM +0800, Minghuan Lian wrote:
> Both LS1043a and LS2080a are based on ARMv8 64-bit architecture
> and have similar PCIe implementation. LUT is added to controller.
> The patch removes the necessary fields from struct ls_pcie.
> 
> Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
> ---
> This patch is based on v4.3-rc4 and [PATCH v10 3/6]
> PCI: designware: Add ARM64 support.
> 
> change log
> v4: 
> 1. split to 6 patches.
> 2. use ARCH_LAYERSCAPE instead of ARM64
> 
> v3:
> 1. Use 8 or 16 bit access function to simplify code
> 2. Add ls_add_pcie_port in accordance with other DesignWare-based drivers
> 
> v2:
> 1. Rename ls2085a to ls2080a
> 2. Add ls_pcie_msi_host_init()
> 
>  drivers/pci/host/Kconfig          |  2 +-
>  drivers/pci/host/pci-layerscape.c | 72 +++++++++++++++++++++++++++++++++++----
>  2 files changed, 67 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index ae873be..8eb09ea 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -105,7 +105,7 @@ config PCI_XGENE_MSI
>  
>  config PCI_LAYERSCAPE
>  	bool "Freescale Layerscape PCIe controller"
> -	depends on OF && ARM
> +	depends on OF && (ARM || ARCH_LAYERSCAPE)
>  	select PCIE_DW
>  	select MFD_SYSCON
>  	help
> diff --git a/drivers/pci/host/pci-layerscape.c b/drivers/pci/host/pci-layerscape.c
> index 891e504..c53692a 100644
> --- a/drivers/pci/host/pci-layerscape.c
> +++ b/drivers/pci/host/pci-layerscape.c
> @@ -31,23 +31,26 @@
>  #define LTSSM_STATE_MASK	0x3f

BTW, not related to *this* patch, but does LTSSM_STATE_MASK really need to
be 0x3f (6 bits), or could it be 0x1f (5 bits)?

I'd like to include Layerscape in the LTSSM_STATE_MASK cleanup done by
Fabio:

https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=pci/host-designware&id=4788fe6ebf4594c9a95b620cbff05147c8504823

I don't have specs for any of these devices, so I don't know if this is
really something that can vary between the different DesignWare-based
devices, or if they all should use a mask of 0x1f.

Bjorn

[PATCH v4 5/6] PCI: layerscape: add PCIe support for LS1043a and LS2080a

[Lian M.H.]

Hi Bjorn,

Our LTSSM mask is 0x3f because it includes the following states:
0x20 S_RCVRY_EQ0
0x21 S_RCVRY_EQ1
0x22 S_RCVRY_EQ2
0x23 S_RCVRY_EQ3

And I checked DesignWare Cores PCI Express Controller Databook v4.21a and found the following descriptor:
[5:0]: smlh_ltssm_state: LTSSM current state. Encoding is defined in workspace/src/include/cxpl_defs.vh

So could we use the mask 0x3f for all SoCs?

Anyway, LTSSM_PCIE_L0 in pci-layerscape.c can be replaced by LTSSM_STATE_L0 defined in pcie-designware.h.

Thanks,
Minghuan

> -----Original Message-----
> From: Bjorn Helgaas [mailto:helgaas at kernel.org]
> Sent: Thursday, October 22, 2015 11:47 PM
> To: Lian Minghuan-B31939 <Minghuan.Lian@freescale.com>
> Cc: linux-pci at vger.kernel.org; linux-arm-kernel at lists.infradead.org; Zang
> Roy-R61911 <tie-fei.zang@freescale.com>; Hu Mingkai-B21284
> <Mingkai.Hu@freescale.com>; Yoder Stuart-B08248
> <stuart.yoder@freescale.com>; Li Yang-Leo-R58472 <LeoLi@freescale.com>;
> Arnd Bergmann <arnd@arndb.de>; Bjorn Helgaas <bhelgaas@google.com>;
> Jingoo Han <jg1.han@samsung.com>; Zhou Wang
> <wangzhou1@hisilicon.com>; Estevam Fabio-R49496
> <Fabio.Estevam@freescale.com>; Lucas Stach <l.stach@pengutronix.de>
> Subject: Re: [PATCH v4 5/6] PCI: layerscape: add PCIe support for LS1043a and
> LS2080a
> 
> [+cc Fabio, Lucas]
> 
> Hi Minghuan,
> 
> On Fri, Oct 16, 2015 at 03:19:19PM +0800, Minghuan Lian wrote:
> > Both LS1043a and LS2080a are based on ARMv8 64-bit architecture and
> > have similar PCIe implementation. LUT is added to controller.
> > The patch removes the necessary fields from struct ls_pcie.
> >
> > Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
> > ---
> > This patch is based on v4.3-rc4 and [PATCH v10 3/6]
> > PCI: designware: Add ARM64 support.
> >
> > change log
> > v4:
> > 1. split to 6 patches.
> > 2. use ARCH_LAYERSCAPE instead of ARM64
> >
> > v3:
> > 1. Use 8 or 16 bit access function to simplify code 2. Add
> > ls_add_pcie_port in accordance with other DesignWare-based drivers
> >
> > v2:
> > 1. Rename ls2085a to ls2080a
> > 2. Add ls_pcie_msi_host_init()
> >
> >  drivers/pci/host/Kconfig          |  2 +-
> >  drivers/pci/host/pci-layerscape.c | 72
> > +++++++++++++++++++++++++++++++++++----
> >  2 files changed, 67 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig index
> > ae873be..8eb09ea 100644
> > --- a/drivers/pci/host/Kconfig
> > +++ b/drivers/pci/host/Kconfig
> > @@ -105,7 +105,7 @@ config PCI_XGENE_MSI
> >
> >  config PCI_LAYERSCAPE
> >  	bool "Freescale Layerscape PCIe controller"
> > -	depends on OF && ARM
> > +	depends on OF && (ARM || ARCH_LAYERSCAPE)
> >  	select PCIE_DW
> >  	select MFD_SYSCON
> >  	help
> > diff --git a/drivers/pci/host/pci-layerscape.c
> > b/drivers/pci/host/pci-layerscape.c
> > index 891e504..c53692a 100644
> > --- a/drivers/pci/host/pci-layerscape.c
> > +++ b/drivers/pci/host/pci-layerscape.c
> > @@ -31,23 +31,26 @@
> >  #define LTSSM_STATE_MASK	0x3f
> 
> BTW, not related to *this* patch, but does LTSSM_STATE_MASK really need to
> be 0x3f (6 bits), or could it be 0x1f (5 bits)?
> 
> I'd like to include Layerscape in the LTSSM_STATE_MASK cleanup done by
> Fabio:
> 
> https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=pci/host
> -designware&id=4788fe6ebf4594c9a95b620cbff05147c8504823
> 
> I don't have specs for any of these devices, so I don't know if this is really
> something that can vary between the different DesignWare-based devices, or if
> they all should use a mask of 0x1f.
> 
> Bjorn

[PATCH v4 6/6] PCI: layerscape: add ls_pcie_msi_host_init

[Minghuan Lian]

Layerscape PCIe has its own MSI implementation. The patch registers
ls_pcie_msi_host_init() to avoid using Designware's MSI.

Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
---
Change log
v4: split from [PATCH v3] PCI: layerscape: Add PCIe support for LS1043a and LS2080a

 drivers/pci/host/pci-layerscape.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/pci/host/pci-layerscape.c b/drivers/pci/host/pci-layerscape.c
index c53692a..8fac6c8 100644
--- a/drivers/pci/host/pci-layerscape.c
+++ b/drivers/pci/host/pci-layerscape.c
@@ -150,14 +150,31 @@ static void ls_pcie_host_init(struct pcie_port *pp)
 	iowrite32(0, pcie->dbi + PCIE_DBI_RO_WR_EN);
 }
 
+static int ls_pcie_msi_host_init(struct pcie_port *pp,
+				 struct msi_controller *chip)
+{
+	struct device_node *msi_node;
+	struct device_node *np = pp->dev->of_node;
+
+	msi_node = of_parse_phandle(np, "msi-parent", 0);
+	if (!msi_node) {
+		dev_err(pp->dev, "failed to find msi-parent\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static struct pcie_host_ops ls1021_pcie_host_ops = {
 	.link_up = ls1021_pcie_link_up,
 	.host_init = ls1021_pcie_host_init,
+	.msi_host_init = ls_pcie_msi_host_init,
 };
 
 static struct pcie_host_ops ls_pcie_host_ops = {
 	.link_up = ls_pcie_link_up,
 	.host_init = ls_pcie_host_init,
+	.msi_host_init = ls_pcie_msi_host_init,
 };
 
 static struct ls_pcie_drvdata ls1021_drvdata = {
-- 
1.9.1

[PATCH v4 6/6] PCI: layerscape: add ls_pcie_msi_host_init

[Bjorn Helgaas]

Hi Minghuan,

On Fri, Oct 16, 2015 at 03:19:20PM +0800, Minghuan Lian wrote:
> Layerscape PCIe has its own MSI implementation. The patch registers
> ls_pcie_msi_host_init() to avoid using Designware's MSI.
> 
> Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
> ---
> Change log
> v4: split from [PATCH v3] PCI: layerscape: Add PCIe support for LS1043a and LS2080a
> 
>  drivers/pci/host/pci-layerscape.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/pci/host/pci-layerscape.c b/drivers/pci/host/pci-layerscape.c
> index c53692a..8fac6c8 100644
> --- a/drivers/pci/host/pci-layerscape.c
> +++ b/drivers/pci/host/pci-layerscape.c
> @@ -150,14 +150,31 @@ static void ls_pcie_host_init(struct pcie_port *pp)
>  	iowrite32(0, pcie->dbi + PCIE_DBI_RO_WR_EN);
>  }
>  
> +static int ls_pcie_msi_host_init(struct pcie_port *pp,
> +				 struct msi_controller *chip)
> +{
> +	struct device_node *msi_node;
> +	struct device_node *np = pp->dev->of_node;
> +
> +	msi_node = of_parse_phandle(np, "msi-parent", 0);
> +	if (!msi_node) {
> +		dev_err(pp->dev, "failed to find msi-parent\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;

I don't see how this can be right.  I think it's OK if you want to enforce
the presence of "msi-parent", but the other implementations of
.msi_host_init (ks_dw_pcie_msi_host_init() and the default implementation
in dw_pcie_host_init()) both set pp->irq_domain and call
irq_create_mapping().

You don't do either of those, so I don't see how MSIs can work, because I
assume the generic DesignWare code will depend on pp->irq_domain.  If
you're planning to add more Layerscape-specific MSI support later, I think
you should wait and include this patch with that work.

> +}
> +
>  static struct pcie_host_ops ls1021_pcie_host_ops = {
>  	.link_up = ls1021_pcie_link_up,
>  	.host_init = ls1021_pcie_host_init,
> +	.msi_host_init = ls_pcie_msi_host_init,
>  };
>  
>  static struct pcie_host_ops ls_pcie_host_ops = {
>  	.link_up = ls_pcie_link_up,
>  	.host_init = ls_pcie_host_init,
> +	.msi_host_init = ls_pcie_msi_host_init,
>  };
>  
>  static struct ls_pcie_drvdata ls1021_drvdata = {
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 6/6] PCI: layerscape: add ls_pcie_msi_host_init

[Lian M.H.]

Hi Bjorn,

I thank you very much to help review and help revise my patches.

Regarding MSI, both LS1021a and LS1043a use SCFG to implement it. I have submitted patch:
https://patchwork.kernel.org/patch/7411131/
https://patchwork.kernel.org/patch/7411141/

While ls2085a use ITS for it, we just re-use the ITS driver.

I notice some platform MSI driver files were placed in pci/host folder not irqchip.
If it is ok, I would like to change driver folder and re-submitted the MSI patch.

Thanks,
Minghaun

> -----Original Message-----
> From: Bjorn Helgaas [mailto:helgaas at kernel.org]
> Sent: Thursday, October 22, 2015 5:34 AM
> To: Lian Minghuan-B31939 <Minghuan.Lian@freescale.com>
> Cc: linux-pci at vger.kernel.org; linux-arm-kernel at lists.infradead.org; Zang
> Roy-R61911 <tie-fei.zang@freescale.com>; Hu Mingkai-B21284
> <Mingkai.Hu@freescale.com>; Yoder Stuart-B08248
> <stuart.yoder@freescale.com>; Li Yang-Leo-R58472 <LeoLi@freescale.com>;
> Arnd Bergmann <arnd@arndb.de>; Bjorn Helgaas <bhelgaas@google.com>;
> Jingoo Han <jg1.han@samsung.com>; Zhou Wang <wangzhou1@hisilicon.com>
> Subject: Re: [PATCH v4 6/6] PCI: layerscape: add ls_pcie_msi_host_init
> 
> Hi Minghuan,
> 
> On Fri, Oct 16, 2015 at 03:19:20PM +0800, Minghuan Lian wrote:
> > Layerscape PCIe has its own MSI implementation. The patch registers
> > ls_pcie_msi_host_init() to avoid using Designware's MSI.
> >
> > Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
> > ---
> > Change log
> > v4: split from [PATCH v3] PCI: layerscape: Add PCIe support for
> > LS1043a and LS2080a
> >
> >  drivers/pci/host/pci-layerscape.c | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> >
> > diff --git a/drivers/pci/host/pci-layerscape.c
> > b/drivers/pci/host/pci-layerscape.c
> > index c53692a..8fac6c8 100644
> > --- a/drivers/pci/host/pci-layerscape.c
> > +++ b/drivers/pci/host/pci-layerscape.c
> > @@ -150,14 +150,31 @@ static void ls_pcie_host_init(struct pcie_port *pp)
> >  	iowrite32(0, pcie->dbi + PCIE_DBI_RO_WR_EN);  }
> >
> > +static int ls_pcie_msi_host_init(struct pcie_port *pp,
> > +				 struct msi_controller *chip)
> > +{
> > +	struct device_node *msi_node;
> > +	struct device_node *np = pp->dev->of_node;
> > +
> > +	msi_node = of_parse_phandle(np, "msi-parent", 0);
> > +	if (!msi_node) {
> > +		dev_err(pp->dev, "failed to find msi-parent\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	return 0;
> 
> I don't see how this can be right.  I think it's OK if you want to enforce the
> presence of "msi-parent", but the other implementations of .msi_host_init
> (ks_dw_pcie_msi_host_init() and the default implementation in
> dw_pcie_host_init()) both set pp->irq_domain and call irq_create_mapping().
> 
> You don't do either of those, so I don't see how MSIs can work, because I
> assume the generic DesignWare code will depend on pp->irq_domain.  If
> you're planning to add more Layerscape-specific MSI support later, I think you
> should wait and include this patch with that work.
> 
> > +}
> > +
> >  static struct pcie_host_ops ls1021_pcie_host_ops = {
> >  	.link_up = ls1021_pcie_link_up,
> >  	.host_init = ls1021_pcie_host_init,
> > +	.msi_host_init = ls_pcie_msi_host_init,
> >  };
> >
> >  static struct pcie_host_ops ls_pcie_host_ops = {
> >  	.link_up = ls_pcie_link_up,
> >  	.host_init = ls_pcie_host_init,
> > +	.msi_host_init = ls_pcie_msi_host_init,
> >  };
> >
> >  static struct ls_pcie_drvdata ls1021_drvdata = {
> > --
> > 1.9.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-pci"
> > in the body of a message to majordomo at vger.kernel.org More majordomo
> > info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 6/6] PCI: layerscape: add ls_pcie_msi_host_init

[Bjorn Helgaas]

[+cc Thomas for MSI driver file placement question + PCI MSI driver
structure]

Hi Minghuan,

Your reply was base64-encoded and thus rejected by the vger mailing lists.
I mentioned this before
(http://lkml.kernel.org/r/20151011191027.GA29221 at localhost).   You might
want to fix your mail strategy, because it's really hard to carry on a
conversation if nobody can hear your side :)

But I'll include your response here again by hand.

Minghuan wrote:
> On Wed, Oct 21, 2015 at 04:34:16PM -0500, Bjorn Helgaas wrote:
> > Hi Minghuan,
> > 
> > On Fri, Oct 16, 2015 at 03:19:20PM +0800, Minghuan Lian wrote:
> > > Layerscape PCIe has its own MSI implementation. The patch registers
> > > ls_pcie_msi_host_init() to avoid using Designware's MSI.
> > > 
> > > Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
> > > ---
> > > Change log
> > > v4: split from [PATCH v3] PCI: layerscape: Add PCIe support for LS1043a and LS2080a
> > > 
> > >  drivers/pci/host/pci-layerscape.c | 17 +++++++++++++++++
> > >  1 file changed, 17 insertions(+)
> > > 
> > > diff --git a/drivers/pci/host/pci-layerscape.c b/drivers/pci/host/pci-layerscape.c
> > > index c53692a..8fac6c8 100644
> > > --- a/drivers/pci/host/pci-layerscape.c
> > > +++ b/drivers/pci/host/pci-layerscape.c
> > > @@ -150,14 +150,31 @@ static void ls_pcie_host_init(struct pcie_port *pp)
> > >  	iowrite32(0, pcie->dbi + PCIE_DBI_RO_WR_EN);
> > >  }
> > >  
> > > +static int ls_pcie_msi_host_init(struct pcie_port *pp,
> > > +				 struct msi_controller *chip)
> > > +{
> > > +	struct device_node *msi_node;
> > > +	struct device_node *np = pp->dev->of_node;
> > > +
> > > +	msi_node = of_parse_phandle(np, "msi-parent", 0);
> > > +	if (!msi_node) {
> > > +		dev_err(pp->dev, "failed to find msi-parent\n");
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	return 0;
> > 
> > I don't see how this can be right.  I think it's OK if you want to enforce
> > the presence of "msi-parent", but the other implementations of
> > .msi_host_init (ks_dw_pcie_msi_host_init() and the default implementation
> > in dw_pcie_host_init()) both set pp->irq_domain and call
> > irq_create_mapping().
> > 
> > You don't do either of those, so I don't see how MSIs can work, because I
> > assume the generic DesignWare code will depend on pp->irq_domain.  If
> > you're planning to add more Layerscape-specific MSI support later, I think
> > you should wait and include this patch with that work.

> Regarding MSI, both LS1021a and LS1043a use SCFG to implement it. I have submitted patch:
> https://patchwork.kernel.org/patch/7411131/
> https://patchwork.kernel.org/patch/7411141/

> While ls2085a use ITS for it, we just re-use the ITS driver.

> I notice some platform MSI driver files were placed in pci/host
> folder not irqchip.  If it is ok, I would like to change driver
> folder and re-submitted the MSI patch.

I suppose you're referring to drivers/pci/host/pci-xgene-msi.c.  
That file doesn't really have much PCI stuff in it.  It does call
pci_msi_create_irq_domain(), but that's really the only PCI interface or
data structure it uses.  So I don't know if drivers/pci/host or
drivers/irqchip is the best place for it and for your
irq-ls-scfg-msi.c.

The connection between pci-xgene-msi.c and pci-xgene.c is not very
clear to me, and that's sort of what I'm complaining about here.
You're overriding a default MSI initialization method.  Usually that
means you do the same thing as the default method, but in a different
way.  You aren't doing the same thing at all, which makes the code
hard to review.

Maybe a comment about how the MSI controller gets connected to devices
below this host bridge would be enough.

Bjorn

[PATCH v4 6/6] PCI: layerscape: add ls_pcie_msi_host_init

[Lian M.H.]

Hi Bjorn,

I am sorry, I just change to outlook as email client instead of thunderbird.
And I changed email type to plain text but forgot to check code type. Now, I have updated the options and hope it is ok.


> -----Original Message-----
> From: Bjorn Helgaas [mailto:helgaas at kernel.org]
> Sent: Friday, October 23, 2015 12:22 AM
> To: Lian Minghuan-B31939 <Minghuan.Lian@freescale.com>
> Cc: linux-pci at vger.kernel.org; linux-arm-kernel at lists.infradead.org; Zang
> Roy-R61911 <tie-fei.zang@freescale.com>; Hu Mingkai-B21284
> <Mingkai.Hu@freescale.com>; Yoder Stuart-B08248
> <stuart.yoder@freescale.com>; Li Yang-Leo-R58472 <LeoLi@freescale.com>;
> Arnd Bergmann <arnd@arndb.de>; Bjorn Helgaas <bhelgaas@google.com>;
> Jingoo Han <jg1.han@samsung.com>; Zhou Wang
> <wangzhou1@hisilicon.com>; Thomas Gleixner <tglx@linutronix.de>
> Subject: Re: [PATCH v4 6/6] PCI: layerscape: add ls_pcie_msi_host_init
> 
> [+cc Thomas for MSI driver file placement question + PCI MSI driver structure]
> 
> Hi Minghuan,
> 
> Your reply was base64-encoded and thus rejected by the vger mailing lists.
> I mentioned this before
> (http://lkml.kernel.org/r/20151011191027.GA29221 at localhost).   You might
> want to fix your mail strategy, because it's really hard to carry on a
> conversation if nobody can hear your side :)
> 
> But I'll include your response here again by hand.
> 
> Minghuan wrote:
> > On Wed, Oct 21, 2015 at 04:34:16PM -0500, Bjorn Helgaas wrote:
> > > Hi Minghuan,
> > >
> > > On Fri, Oct 16, 2015 at 03:19:20PM +0800, Minghuan Lian wrote:
> > > > Layerscape PCIe has its own MSI implementation. The patch
> > > > registers
> > > > ls_pcie_msi_host_init() to avoid using Designware's MSI.
> > > >
> > > > Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
> > > > ---
> > > > Change log
> > > > v4: split from [PATCH v3] PCI: layerscape: Add PCIe support for
> > > > LS1043a and LS2080a
> > > >
> > > >  drivers/pci/host/pci-layerscape.c | 17 +++++++++++++++++
> > > >  1 file changed, 17 insertions(+)
> > > >
> > > > diff --git a/drivers/pci/host/pci-layerscape.c
> > > > b/drivers/pci/host/pci-layerscape.c
> > > > index c53692a..8fac6c8 100644
> > > > --- a/drivers/pci/host/pci-layerscape.c
> > > > +++ b/drivers/pci/host/pci-layerscape.c
> > > > @@ -150,14 +150,31 @@ static void ls_pcie_host_init(struct pcie_port
> *pp)
> > > >  	iowrite32(0, pcie->dbi + PCIE_DBI_RO_WR_EN);  }
> > > >
> > > > +static int ls_pcie_msi_host_init(struct pcie_port *pp,
> > > > +				 struct msi_controller *chip) {
> > > > +	struct device_node *msi_node;
> > > > +	struct device_node *np = pp->dev->of_node;
> > > > +
> > > > +	msi_node = of_parse_phandle(np, "msi-parent", 0);
> > > > +	if (!msi_node) {
> > > > +		dev_err(pp->dev, "failed to find msi-parent\n");
> > > > +		return -EINVAL;
> > > > +	}
> > > > +
> > > > +	return 0;
> > >
> > > I don't see how this can be right.  I think it's OK if you want to
> > > enforce the presence of "msi-parent", but the other implementations
> > > of .msi_host_init (ks_dw_pcie_msi_host_init() and the default
> > > implementation in dw_pcie_host_init()) both set pp->irq_domain and
> > > call irq_create_mapping().
> > >
> > > You don't do either of those, so I don't see how MSIs can work,
> > > because I assume the generic DesignWare code will depend on
> > > pp->irq_domain.  If you're planning to add more Layerscape-specific
> > > MSI support later, I think you should wait and include this patch with that
> work.
> 
> > Regarding MSI, both LS1021a and LS1043a use SCFG to implement it. I have
> submitted patch:
> > https://patchwork.kernel.org/patch/7411131/
> > https://patchwork.kernel.org/patch/7411141/
> 
> > While ls2085a use ITS for it, we just re-use the ITS driver.
> 
> > I notice some platform MSI driver files were placed in pci/host folder
> > not irqchip.  If it is ok, I would like to change driver folder and
> > re-submitted the MSI patch.
> 
> I suppose you're referring to drivers/pci/host/pci-xgene-msi.c.
> That file doesn't really have much PCI stuff in it.  It does call
> pci_msi_create_irq_domain(), but that's really the only PCI interface or data
> structure it uses.  So I don't know if drivers/pci/host or drivers/irqchip is the
> best place for it and for your irq-ls-scfg-msi.c.
> 
> The connection between pci-xgene-msi.c and pci-xgene.c is not very clear to me,
> and that's sort of what I'm complaining about here.
> You're overriding a default MSI initialization method.  Usually that means you
> do the same thing as the default method, but in a different way.  You aren't
> doing the same thing at all, which makes the code hard to review.
> 
> Maybe a comment about how the MSI controller gets connected to devices
> below this host bridge would be enough.
> 
[Lian Minghuan-B31939] 
1. We need to define callback function msi_host_init() to avoid the running desginware MSI related code
Because our PCIe controller do not enable this feature.
2. we do not need to do anything such as bus->msi= ls_scfg_msi
Because with the latest code, when PCIe controller device is created of_msi_configure() will be called and MSI domain pointed by 'msi-parent' will be bound  to the device. pci_msi_get_domain(struct pci_dev *dev) -> dev_get_msi_domain(&dev->dev) will return this MSI domain.
3. I will add a comment in the next version.

Thanks,
Minghuan


> Bjorn

[PATCH v4 6/6] PCI: layerscape: add ls_pcie_msi_host_init

[Bjorn Helgaas]

Your reply was *still* base64-encoded and thus rejected by the vger
mailing lists.

Minghuan wrote:
> On Thu, Oct 22, 2015 at 11:21:30AM -0500, Bjorn Helgaas wrote:
> > Minghuan wrote:
> > > On Wed, Oct 21, 2015 at 04:34:16PM -0500, Bjorn Helgaas wrote:
> > > > Hi Minghuan,
> > > > 
> > > > On Fri, Oct 16, 2015 at 03:19:20PM +0800, Minghuan Lian wrote:
> > > > > Layerscape PCIe has its own MSI implementation. The patch registers
> > > > > ls_pcie_msi_host_init() to avoid using Designware's MSI.
> > > > > 
> > > > > Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
> > > > > ---
> > > > > Change log
> > > > > v4: split from [PATCH v3] PCI: layerscape: Add PCIe support for LS1043a and LS2080a
> > > > > 
> > > > >  drivers/pci/host/pci-layerscape.c | 17 +++++++++++++++++
> > > > >  1 file changed, 17 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/pci/host/pci-layerscape.c b/drivers/pci/host/pci-layerscape.c
> > > > > index c53692a..8fac6c8 100644
> > > > > --- a/drivers/pci/host/pci-layerscape.c
> > > > > +++ b/drivers/pci/host/pci-layerscape.c
> > > > > @@ -150,14 +150,31 @@ static void ls_pcie_host_init(struct pcie_port *pp)
> > > > >  	iowrite32(0, pcie->dbi + PCIE_DBI_RO_WR_EN);
> > > > >  }
> > > > >  
> > > > > +static int ls_pcie_msi_host_init(struct pcie_port *pp,
> > > > > +				 struct msi_controller *chip)
> > > > > +{
> > > > > +	struct device_node *msi_node;
> > > > > +	struct device_node *np = pp->dev->of_node;
> > > > > +
> > > > > +	msi_node = of_parse_phandle(np, "msi-parent", 0);
> > > > > +	if (!msi_node) {
> > > > > +		dev_err(pp->dev, "failed to find msi-parent\n");
> > > > > +		return -EINVAL;
> > > > > +	}
> > > > > +
> > > > > +	return 0;
> > > > 
> > > > I don't see how this can be right.  I think it's OK if you want to enforce
> > > > the presence of "msi-parent", but the other implementations of
> > > > .msi_host_init (ks_dw_pcie_msi_host_init() and the default implementation
> > > > in dw_pcie_host_init()) both set pp->irq_domain and call
> > > > irq_create_mapping().
> > > > 
> > > > You don't do either of those, so I don't see how MSIs can work, because I
> > > > assume the generic DesignWare code will depend on pp->irq_domain.  If
> > > > you're planning to add more Layerscape-specific MSI support later, I think
> > > > you should wait and include this patch with that work.
> > 
> > > Regarding MSI, both LS1021a and LS1043a use SCFG to implement it. I have submitted patch:
> > > https://patchwork.kernel.org/patch/7411131/
> > > https://patchwork.kernel.org/patch/7411141/
> > 
> > > While ls2085a use ITS for it, we just re-use the ITS driver.
> > 
> > > I notice some platform MSI driver files were placed in pci/host
> > > folder not irqchip.  If it is ok, I would like to change driver
> > > folder and re-submitted the MSI patch.
> > 
> > I suppose you're referring to drivers/pci/host/pci-xgene-msi.c.  
> > That file doesn't really have much PCI stuff in it.  It does call
> > pci_msi_create_irq_domain(), but that's really the only PCI interface or
> > data structure it uses.  So I don't know if drivers/pci/host or
> > drivers/irqchip is the best place for it and for your
> > irq-ls-scfg-msi.c.
> > 
> > The connection between pci-xgene-msi.c and pci-xgene.c is not very
> > clear to me, and that's sort of what I'm complaining about here.
> > You're overriding a default MSI initialization method.  Usually that
> > means you do the same thing as the default method, but in a different
> > way.  You aren't doing the same thing at all, which makes the code
> > hard to review.
> > 
> > Maybe a comment about how the MSI controller gets connected to devices
> > below this host bridge would be enough.

> 1. We need to define callback function msi_host_init() to avoid the
> running desginware MSI related code Because our PCIe controller do not
> enable this feature.
> 
> 2. we do not need to do anything such as bus->msi= ls_scfg_msi Because
> with the latest code, when PCIe controller device is created
> of_msi_configure() will be called and MSI domain pointed by
> 'msi-parent' will be bound  to the device. pci_msi_get_domain(struct
> pci_dev *dev) -> dev_get_msi_domain(&dev->dev) will return this MSI
> domain.
> 
> 3. I will add a comment in the next version.

Don't bother, I added a comment and applied the patch like this:


commit 90cbdf8412d206f65ece3dcc53230e673da569c6
Author: Minghuan Lian <Minghuan.Lian@freescale.com>
Date:   Fri Oct 16 15:19:20 2015 +0800

    PCI: layerscape: Add ls_pcie_msi_host_init()
    
    Layerscape PCIe has its own MSI implementation.
    
    Register ls_pcie_msi_host_init() to avoid using DesignWare's MSI.
    
    [bhelgaas: add comment]
    Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

diff --git a/drivers/pci/host/pci-layerscape.c b/drivers/pci/host/pci-layerscape.c
index 6a9fa8a..1677890 100644
--- a/drivers/pci/host/pci-layerscape.c
+++ b/drivers/pci/host/pci-layerscape.c
@@ -150,14 +150,37 @@ static void ls_pcie_host_init(struct pcie_port *pp)
 	iowrite32(0, pcie->dbi + PCIE_DBI_RO_WR_EN);
 }
 
+static int ls_pcie_msi_host_init(struct pcie_port *pp,
+				 struct msi_controller *chip)
+{
+	struct device_node *msi_node;
+	struct device_node *np = pp->dev->of_node;
+
+	/*
+	 * The MSI domain is set by the generic of_msi_configure().  This
+	 * .msi_host_init() function keeps us from doing the default MSI
+	 * domain setup in dw_pcie_host_init() and also enforces the
+	 * requirement that "msi-parent" exists.
+	 */
+	msi_node = of_parse_phandle(np, "msi-parent", 0);
+	if (!msi_node) {
+		dev_err(pp->dev, "failed to find msi-parent\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static struct pcie_host_ops ls1021_pcie_host_ops = {
 	.link_up = ls1021_pcie_link_up,
 	.host_init = ls1021_pcie_host_init,
+	.msi_host_init = ls_pcie_msi_host_init,
 };
 
 static struct pcie_host_ops ls_pcie_host_ops = {
 	.link_up = ls_pcie_link_up,
 	.host_init = ls_pcie_host_init,
+	.msi_host_init = ls_pcie_msi_host_init,
 };
 
 static struct ls_pcie_drvdata ls1021_drvdata = {

[PATCH v4 6/6] PCI: layerscape: add ls_pcie_msi_host_init

[Lian M.H.]

Hi Bjorn,

I am so sorry about text code setting. Now, I carefully checked the mail configuration again, hope it is ok.
I greatly appreciate your review and comments.

Thanks,
Minghuan

> -----Original Message-----
> From: Bjorn Helgaas [mailto:helgaas at kernel.org]
> Sent: Tuesday, November 03, 2015 5:07 AM
> To: Lian Minghuan-B31939 <Minghuan.Lian@freescale.com>
> Cc: linux-pci at vger.kernel.org; linux-arm-kernel at lists.infradead.org; Zang
> Roy-R61911 <tie-fei.zang@freescale.com>; Hu Mingkai-B21284
> <Mingkai.Hu@freescale.com>; Yoder Stuart-B08248
> <stuart.yoder@freescale.com>; Li Yang-Leo-R58472 <LeoLi@freescale.com>;
> Arnd Bergmann <arnd@arndb.de>; Bjorn Helgaas <bhelgaas@google.com>;
> Jingoo Han <jg1.han@samsung.com>; Zhou Wang
> <wangzhou1@hisilicon.com>; Thomas Gleixner <tglx@linutronix.de>
> Subject: Re: [PATCH v4 6/6] PCI: layerscape: add ls_pcie_msi_host_init
> 
> Your reply was *still* base64-encoded and thus rejected by the vger mailing
> lists.
> 
> Minghuan wrote:
> > On Thu, Oct 22, 2015 at 11:21:30AM -0500, Bjorn Helgaas wrote:
> > > Minghuan wrote:
> > > > On Wed, Oct 21, 2015 at 04:34:16PM -0500, Bjorn Helgaas wrote:
> > > > > Hi Minghuan,
> > > > >
> > > > > On Fri, Oct 16, 2015 at 03:19:20PM +0800, Minghuan Lian wrote:
> > > > > > Layerscape PCIe has its own MSI implementation. The patch
> > > > > > registers
> > > > > > ls_pcie_msi_host_init() to avoid using Designware's MSI.
> > > > > >
> > > > > > Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
> > > > > > ---
> > > > > > Change log
> > > > > > v4: split from [PATCH v3] PCI: layerscape: Add PCIe support
> > > > > > for LS1043a and LS2080a
> > > > > >
> > > > > >  drivers/pci/host/pci-layerscape.c | 17 +++++++++++++++++
> > > > > >  1 file changed, 17 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/pci/host/pci-layerscape.c
> > > > > > b/drivers/pci/host/pci-layerscape.c
> > > > > > index c53692a..8fac6c8 100644
> > > > > > --- a/drivers/pci/host/pci-layerscape.c
> > > > > > +++ b/drivers/pci/host/pci-layerscape.c
> > > > > > @@ -150,14 +150,31 @@ static void ls_pcie_host_init(struct
> pcie_port *pp)
> > > > > >  	iowrite32(0, pcie->dbi + PCIE_DBI_RO_WR_EN);  }
> > > > > >
> > > > > > +static int ls_pcie_msi_host_init(struct pcie_port *pp,
> > > > > > +				 struct msi_controller *chip) {
> > > > > > +	struct device_node *msi_node;
> > > > > > +	struct device_node *np = pp->dev->of_node;
> > > > > > +
> > > > > > +	msi_node = of_parse_phandle(np, "msi-parent", 0);
> > > > > > +	if (!msi_node) {
> > > > > > +		dev_err(pp->dev, "failed to find msi-parent\n");
> > > > > > +		return -EINVAL;
> > > > > > +	}
> > > > > > +
> > > > > > +	return 0;
> > > > >
> > > > > I don't see how this can be right.  I think it's OK if you want
> > > > > to enforce the presence of "msi-parent", but the other
> > > > > implementations of .msi_host_init (ks_dw_pcie_msi_host_init()
> > > > > and the default implementation in dw_pcie_host_init()) both set
> > > > > pp->irq_domain and call irq_create_mapping().
> > > > >
> > > > > You don't do either of those, so I don't see how MSIs can work,
> > > > > because I assume the generic DesignWare code will depend on
> > > > > pp->irq_domain.  If you're planning to add more
> > > > > Layerscape-specific MSI support later, I think you should wait and
> include this patch with that work.
> > >
> > > > Regarding MSI, both LS1021a and LS1043a use SCFG to implement it. I
> have submitted patch:
> > > > https://patchwork.kernel.org/patch/7411131/
> > > > https://patchwork.kernel.org/patch/7411141/
> > >
> > > > While ls2085a use ITS for it, we just re-use the ITS driver.
> > >
> > > > I notice some platform MSI driver files were placed in pci/host
> > > > folder not irqchip.  If it is ok, I would like to change driver
> > > > folder and re-submitted the MSI patch.
> > >
> > > I suppose you're referring to drivers/pci/host/pci-xgene-msi.c.
> > > That file doesn't really have much PCI stuff in it.  It does call
> > > pci_msi_create_irq_domain(), but that's really the only PCI
> > > interface or data structure it uses.  So I don't know if
> > > drivers/pci/host or drivers/irqchip is the best place for it and for
> > > your irq-ls-scfg-msi.c.
> > >
> > > The connection between pci-xgene-msi.c and pci-xgene.c is not very
> > > clear to me, and that's sort of what I'm complaining about here.
> > > You're overriding a default MSI initialization method.  Usually that
> > > means you do the same thing as the default method, but in a
> > > different way.  You aren't doing the same thing at all, which makes
> > > the code hard to review.
> > >
> > > Maybe a comment about how the MSI controller gets connected to
> > > devices below this host bridge would be enough.
> 
> > 1. We need to define callback function msi_host_init() to avoid the
> > running desginware MSI related code Because our PCIe controller do not
> > enable this feature.
> >
> > 2. we do not need to do anything such as bus->msi= ls_scfg_msi Because
> > with the latest code, when PCIe controller device is created
> > of_msi_configure() will be called and MSI domain pointed by
> > 'msi-parent' will be bound  to the device. pci_msi_get_domain(struct
> > pci_dev *dev) -> dev_get_msi_domain(&dev->dev) will return this MSI
> > domain.
> >
> > 3. I will add a comment in the next version.
> 
> Don't bother, I added a comment and applied the patch like this:
> 
> 
> commit 90cbdf8412d206f65ece3dcc53230e673da569c6
> Author: Minghuan Lian <Minghuan.Lian@freescale.com>
> Date:   Fri Oct 16 15:19:20 2015 +0800
> 
>     PCI: layerscape: Add ls_pcie_msi_host_init()
> 
>     Layerscape PCIe has its own MSI implementation.
> 
>     Register ls_pcie_msi_host_init() to avoid using DesignWare's MSI.
> 
>     [bhelgaas: add comment]
>     Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
>     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> 
> diff --git a/drivers/pci/host/pci-layerscape.c b/drivers/pci/host/pci-layerscape.c
> index 6a9fa8a..1677890 100644
> --- a/drivers/pci/host/pci-layerscape.c
> +++ b/drivers/pci/host/pci-layerscape.c
> @@ -150,14 +150,37 @@ static void ls_pcie_host_init(struct pcie_port *pp)
>  	iowrite32(0, pcie->dbi + PCIE_DBI_RO_WR_EN);  }
> 
> +static int ls_pcie_msi_host_init(struct pcie_port *pp,
> +				 struct msi_controller *chip)
> +{
> +	struct device_node *msi_node;
> +	struct device_node *np = pp->dev->of_node;
> +
> +	/*
> +	 * The MSI domain is set by the generic of_msi_configure().  This
> +	 * .msi_host_init() function keeps us from doing the default MSI
> +	 * domain setup in dw_pcie_host_init() and also enforces the
> +	 * requirement that "msi-parent" exists.
> +	 */
> +	msi_node = of_parse_phandle(np, "msi-parent", 0);
> +	if (!msi_node) {
> +		dev_err(pp->dev, "failed to find msi-parent\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  static struct pcie_host_ops ls1021_pcie_host_ops = {
>  	.link_up = ls1021_pcie_link_up,
>  	.host_init = ls1021_pcie_host_init,
> +	.msi_host_init = ls_pcie_msi_host_init,
>  };
> 
>  static struct pcie_host_ops ls_pcie_host_ops = {
>  	.link_up = ls_pcie_link_up,
>  	.host_init = ls_pcie_host_init,
> +	.msi_host_init = ls_pcie_msi_host_init,
>  };
> 
>  static struct ls_pcie_drvdata ls1021_drvdata = {

[PATCH v4 1/6] PCI: layerscape: remove ls_pcie_establish_link()

[Bjorn Helgaas]

On Fri, Oct 16, 2015 at 03:19:15PM +0800, Minghuan Lian wrote:
> ls_pcie_establish_link() does not do any real operation, except
> to wait for the linkup establishment. In fact, this is not
> necessary. Moreover, each PCIe controller not inserted device
> will increase the Linux startup time about 200ms.
> 
> Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>

It'd be handy if there were a [0/6] cover letter where I could reply
with this update about the whole series.

I applied patches 1-5 to pci/host-layerscape for v4.4, thanks!

I split patch 5 (adding LS1043a and LS2080a support) into one patch
that removes the unused struct fields and a second patch that actually
adds the LS1043a and LS2080a support.

I haven't applied patch 6 because it doesn't make sense to me (more
details in the response to it).  I'll be happy to apply it if you
explain what I'm missing.

Bjorn

> ---
> Change log
> v4: split from [PATCH v3] PCI: layerscape: Add PCIe support for LS1043a and LS2080a
> 
>  drivers/pci/host/pci-layerscape.c | 16 ----------------
>  1 file changed, 16 deletions(-)
> 
> diff --git a/drivers/pci/host/pci-layerscape.c b/drivers/pci/host/pci-layerscape.c
> index b2328ea1..6dd44a0 100644
> --- a/drivers/pci/host/pci-layerscape.c
> +++ b/drivers/pci/host/pci-layerscape.c
> @@ -11,7 +11,6 @@
>   */
>  
>  #include <linux/kernel.h>
> -#include <linux/delay.h>
>  #include <linux/interrupt.h>
>  #include <linux/module.h>
>  #include <linux/of_pci.h>
> @@ -62,27 +61,12 @@ static int ls_pcie_link_up(struct pcie_port *pp)
>  	return 1;
>  }
>  
> -static int ls_pcie_establish_link(struct pcie_port *pp)
> -{
> -	unsigned int retries;
> -
> -	for (retries = 0; retries < 200; retries++) {
> -		if (dw_pcie_link_up(pp))
> -			return 0;
> -		usleep_range(100, 1000);
> -	}
> -
> -	dev_err(pp->dev, "phy link never came up\n");
> -	return -EINVAL;
> -}
> -
>  static void ls_pcie_host_init(struct pcie_port *pp)
>  {
>  	struct ls_pcie *pcie = to_ls_pcie(pp);
>  	u32 val;
>  
>  	dw_pcie_setup_rc(pp);
> -	ls_pcie_establish_link(pp);
>  
>  	/*
>  	 * LS1021A Workaround for internal TKT228622
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html