[RFC 1/2] pci: dwc: pci-exynos: modify the Kconfig dependency

[RFC 1/2] pci: dwc: pci-exynos: modify the Kconfig dependency

[Jaehoon Chung]

PCI_EXYNOS has the dependency with SOC_EXYNOS5440.
It's modified to ARCH_EXYNOS from SOC_EXYNOS5440, because other
SoCs needs to use this driver.

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
 drivers/pci/dwc/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/dwc/Kconfig b/drivers/pci/dwc/Kconfig
index 113e09440f85..0ff9795df8e8 100644
--- a/drivers/pci/dwc/Kconfig
+++ b/drivers/pci/dwc/Kconfig
@@ -65,7 +65,7 @@ config PCIE_DW_PLAT
 config PCI_EXYNOS
 	bool "Samsung Exynos PCIe controller"
 	depends on PCI
-	depends on SOC_EXYNOS5440
+	depends on ARCH_EXYNOS
 	depends on PCI_MSI_IRQ_DOMAIN
 	select PCIEPORTBUS
 	select PCIE_DW_HOST
-- 
2.15.1

[RFC 2/2] pci: dwc: pci-exynos: add the codes to support the exynos5433

[Jaehoon Chung]

Exynos5433 has the PCIe for WiFi.
Added the codes relevant to PCIe for supporting the exynos5433.
Also changed the binding documentation name to
'samsung,exynos-pcie.txt'.
(It's not only exynos5440 anymore.)

Signed-off-by: Jaehoon Chung <jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 ...exynos5440-pcie.txt => samsung,exynos-pcie.txt} |   2 +-
 drivers/pci/dwc/pci-exynos.c                       | 183 ++++++++++++++++-----
 2 files changed, 144 insertions(+), 41 deletions(-)
 rename Documentation/devicetree/bindings/pci/{samsung,exynos5440-pcie.txt => samsung,exynos-pcie.txt} (97%)

diff --git a/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.txt
similarity index 97%
rename from Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt
rename to Documentation/devicetree/bindings/pci/samsung,exynos-pcie.txt
index 34a11bfbfb60..958dcc150505 100644
--- a/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.txt
@@ -4,7 +4,7 @@ This PCIe host controller is based on the Synopsys DesignWare PCIe IP
 and thus inherits all the common properties defined in designware-pcie.txt.
 
 Required properties:
-- compatible: "samsung,exynos5440-pcie"
+- compatible: "samsung,exynos5440-pcie" or "samsung,exynos5433-pcie"
 - reg: base addresses and lengths of the PCIe controller,
 	the PHY controller, additional register for the PHY controller.
 	(Registers for the PHY controller are DEPRECATED.
diff --git a/drivers/pci/dwc/pci-exynos.c b/drivers/pci/dwc/pci-exynos.c
index 5596fdedbb94..8dee2e90347e 100644
--- a/drivers/pci/dwc/pci-exynos.c
+++ b/drivers/pci/dwc/pci-exynos.c
@@ -40,6 +40,8 @@
 #define PCIE_IRQ_SPECIAL		0x008
 #define PCIE_IRQ_EN_PULSE		0x00c
 #define PCIE_IRQ_EN_LEVEL		0x010
+#define PCIE_SW_WAKE			0x018
+#define PCIE_BUS_EN			BIT(1)
 #define IRQ_MSI_ENABLE			BIT(2)
 #define PCIE_IRQ_EN_SPECIAL		0x014
 #define PCIE_PWR_RESET			0x018
@@ -49,7 +51,8 @@
 #define PCIE_NONSTICKY_RESET		0x024
 #define PCIE_APP_INIT_RESET		0x028
 #define PCIE_APP_LTSSM_ENABLE		0x02c
-#define PCIE_ELBI_RDLH_LINKUP		0x064
+#define PCIE_ELBI_RDLH_LINKUP		0x074
+#define PCIE_ELBI_XMLH_LINKUP		BIT(4)
 #define PCIE_ELBI_LTSSM_ENABLE		0x1
 #define PCIE_ELBI_SLV_AWMISC		0x11c
 #define PCIE_ELBI_SLV_ARMISC		0x120
@@ -94,6 +97,10 @@
 #define PCIE_PHY_TRSV3_PD_TSV		BIT(7)
 #define PCIE_PHY_TRSV3_LVCC		0x31c
 
+/* DBI register */
+#define PCIE_MISC_CONTROL_1_OFF		0x8BC
+#define DBI_RO_WR_EN			BIT(0)
+
 struct exynos_pcie_mem_res {
 	void __iomem *elbi_base;   /* DT 0th resource: PCIe CTRL */
 	void __iomem *phy_base;    /* DT 1st resource: PHY CTRL */
@@ -221,6 +228,96 @@ static const struct exynos_pcie_ops exynos5440_pcie_ops = {
 	.deinit_clk_resources	= exynos5440_pcie_deinit_clk_resources,
 };
 
+static int exynos5433_pcie_get_mem_resources(struct platform_device *pdev,
+					     struct exynos_pcie *ep)
+{
+	struct dw_pcie *pci = ep->pci;
+	struct device *dev = pci->dev;
+	struct resource *res;
+
+	ep->mem_res = devm_kzalloc(dev, sizeof(*ep->mem_res), GFP_KERNEL);
+	if (!ep->mem_res)
+		return -ENOMEM;
+
+	/* External Local Bus interface(ELBI) Register */
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "elbi");
+	ep->mem_res->elbi_base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(ep->mem_res->elbi_base))
+		return PTR_ERR(ep->mem_res->elbi_base);
+
+	/* Data Bus Interface(DBI) Register */
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
+	pci->dbi_base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(pci->dbi_base))
+		return PTR_ERR(pci->dbi_base);
+
+	return 0;
+}
+
+static int exynos5433_pcie_get_clk_resources(struct exynos_pcie *ep)
+{
+	struct dw_pcie *pci = ep->pci;
+	struct device *dev = pci->dev;
+
+	ep->clk_res = devm_kzalloc(dev, sizeof(*ep->clk_res), GFP_KERNEL);
+	if (!ep->clk_res)
+		return -ENOMEM;
+
+	ep->clk_res->clk = devm_clk_get(dev, "pcie");
+	if (IS_ERR(ep->clk_res->clk)) {
+		dev_err(dev, "Failed to get pcie rc clock\n");
+		return PTR_ERR(ep->clk_res->clk);
+	}
+
+	ep->clk_res->bus_clk = devm_clk_get(dev, "pcie_bus");
+	if (IS_ERR(ep->clk_res->bus_clk)) {
+		dev_err(dev, "Failed to get pcie bus clock\n");
+		return PTR_ERR(ep->clk_res->bus_clk);
+	}
+
+	return 0;
+}
+
+static void exynos5433_pcie_deinit_clk_resources(struct exynos_pcie *ep)
+{
+	clk_disable_unprepare(ep->clk_res->bus_clk);
+	clk_disable_unprepare(ep->clk_res->clk);
+}
+
+
+static int exynos5433_pcie_init_clk_resources(struct exynos_pcie *ep)
+{
+	struct dw_pcie *pci = ep->pci;
+	struct device *dev = pci->dev;
+	int ret;
+
+	ret = clk_prepare_enable(ep->clk_res->clk);
+	if (ret) {
+		dev_err(dev, "cannot enable pcie rc clock");
+		return ret;
+	}
+
+	ret = clk_prepare_enable(ep->clk_res->bus_clk);
+	if (ret) {
+		dev_err(dev, "cannot enable pcie bus clock");
+		goto err_bus_clk;
+	}
+
+	return 0;
+
+err_bus_clk:
+	clk_disable_unprepare(ep->clk_res->clk);
+
+	return ret;
+}
+
+static const struct exynos_pcie_ops exynos5433_pcie_ops = {
+	.get_mem_resources	= exynos5433_pcie_get_mem_resources,
+	.get_clk_resources	= exynos5433_pcie_get_clk_resources,
+	.init_clk_resources	= exynos5433_pcie_init_clk_resources,
+	.deinit_clk_resources	= exynos5433_pcie_deinit_clk_resources,
+};
+
 static void exynos_pcie_writel(void __iomem *base, u32 val, u32 reg)
 {
 	writel(val, base + reg);
@@ -279,7 +376,9 @@ static void exynos_pcie_deassert_core_reset(struct exynos_pcie *ep)
 	exynos_pcie_writel(ep->mem_res->elbi_base, 1, PCIE_NONSTICKY_RESET);
 	exynos_pcie_writel(ep->mem_res->elbi_base, 1, PCIE_APP_INIT_RESET);
 	exynos_pcie_writel(ep->mem_res->elbi_base, 0, PCIE_APP_INIT_RESET);
-	exynos_pcie_writel(ep->mem_res->block_base, 1, PCIE_PHY_MAC_RESET);
+	if (ep->mem_res->block_base)
+		exynos_pcie_writel(ep->mem_res->block_base, 1,
+				PCIE_PHY_MAC_RESET);
 }
 
 static void exynos_pcie_assert_phy_reset(struct exynos_pcie *ep)
@@ -413,9 +512,6 @@ static int exynos_pcie_establish_link(struct exynos_pcie *ep)
 	if (ep->using_phy) {
 		phy_reset(ep->phy);
 
-		exynos_pcie_writel(ep->mem_res->elbi_base, 1,
-				PCIE_PWR_RESET);
-
 		phy_power_on(ep->phy);
 		phy_init(ep->phy);
 	} else {
@@ -430,14 +526,16 @@ static int exynos_pcie_establish_link(struct exynos_pcie *ep)
 		udelay(500);
 		exynos_pcie_writel(ep->mem_res->block_base, 0,
 					PCIE_PHY_COMMON_RESET);
+		exynos_pcie_deassert_core_reset(ep);
 	}
 
-	/* pulse for common reset */
-	exynos_pcie_writel(ep->mem_res->block_base, 1, PCIE_PHY_COMMON_RESET);
-	udelay(500);
-	exynos_pcie_writel(ep->mem_res->block_base, 0, PCIE_PHY_COMMON_RESET);
+	/*
+	 * Enable DBI_RO_WR_EN bit.
+	 * - When set to 1, some RO and HWinit bits are wriatble from
+	 *   the local application through the DBI.
+	 */
+	dw_pcie_writel_dbi(pci, PCIE_MISC_CONTROL_1_OFF, DBI_RO_WR_EN);
 
-	exynos_pcie_deassert_core_reset(ep);
 	dw_pcie_setup_rc(pp);
 	exynos_pcie_assert_reset(ep);
 
@@ -472,16 +570,6 @@ static void exynos_pcie_clear_irq_pulse(struct exynos_pcie *ep)
 	exynos_pcie_writel(ep->mem_res->elbi_base, val, PCIE_IRQ_PULSE);
 }
 
-static void exynos_pcie_enable_irq_pulse(struct exynos_pcie *ep)
-{
-	u32 val;
-
-	/* enable INTX interrupt */
-	val = IRQ_INTA_ASSERT | IRQ_INTB_ASSERT |
-		IRQ_INTC_ASSERT | IRQ_INTD_ASSERT;
-	exynos_pcie_writel(ep->mem_res->elbi_base, val, PCIE_IRQ_EN_PULSE);
-}
-
 static irqreturn_t exynos_pcie_irq_handler(int irq, void *arg)
 {
 	struct exynos_pcie *ep = arg;
@@ -513,9 +601,16 @@ static void exynos_pcie_msi_init(struct exynos_pcie *ep)
 	exynos_pcie_writel(ep->mem_res->elbi_base, val, PCIE_IRQ_EN_LEVEL);
 }
 
-static void exynos_pcie_enable_interrupts(struct exynos_pcie *ep)
+static void exynos_pcie_enable_irq_pulse(struct exynos_pcie *ep)
 {
-	exynos_pcie_enable_irq_pulse(ep);
+	u32 val;
+
+	val = IRQ_INTA_ASSERT | IRQ_INTB_ASSERT |
+		IRQ_INTC_ASSERT | IRQ_INTD_ASSERT;
+	exynos_pcie_writel(ep->mem_res->elbi_base, val, PCIE_IRQ_EN_PULSE);
+
+	exynos_pcie_writel(ep->mem_res->elbi_base, 0, PCIE_IRQ_EN_LEVEL);
+	exynos_pcie_writel(ep->mem_res->elbi_base, 0, PCIE_IRQ_EN_SPECIAL);
 
 	if (IS_ENABLED(CONFIG_PCI_MSI))
 		exynos_pcie_msi_init(ep);
@@ -575,10 +670,8 @@ static int exynos_pcie_link_up(struct dw_pcie *pci)
 	u32 val;
 
 	val = exynos_pcie_readl(ep->mem_res->elbi_base, PCIE_ELBI_RDLH_LINKUP);
-	if (val == PCIE_ELBI_LTSSM_ENABLE)
-		return 1;
 
-	return 0;
+	return (val & PCIE_ELBI_XMLH_LINKUP);
 }
 
 static int exynos_pcie_host_init(struct pcie_port *pp)
@@ -587,7 +680,7 @@ static int exynos_pcie_host_init(struct pcie_port *pp)
 	struct exynos_pcie *ep = to_exynos_pcie(pci);
 
 	exynos_pcie_establish_link(ep);
-	exynos_pcie_enable_interrupts(ep);
+	exynos_pcie_enable_irq_pulse(ep);
 
 	return 0;
 }
@@ -608,8 +701,11 @@ static int __init exynos_add_pcie_port(struct exynos_pcie *ep,
 
 	pp->irq = platform_get_irq(pdev, 1);
 	if (pp->irq < 0) {
-		dev_err(dev, "failed to get irq\n");
-		return pp->irq;
+		pp->irq = platform_get_irq_byname(pdev, "intr");
+		if (pp->irq < 0) {
+			dev_err(dev, "failed to get irq\n");
+			return pp->irq;
+		}
 	}
 	ret = devm_request_irq(dev, pp->irq, exynos_pcie_irq_handler,
 				IRQF_SHARED, "exynos-pcie", ep);
@@ -678,13 +774,23 @@ static int __init exynos_pcie_probe(struct platform_device *pdev)
 
 	ep->reset_gpio = of_get_named_gpio(np, "reset-gpio", 0);
 
-	/* Assume that controller doesn't use the PHY framework */
-	ep->using_phy = false;
+	/*
+	 * In case of Exynos5440,
+	 * Assume that controller doesn't use the PHY frameork.
+	 * Other SoCs might be used the PHY framework.
+	 */
+
+	if (of_device_is_compatible(np, "samsung,exynos5440-pcie"))
+		ep->using_phy = false;
 
-	ep->phy = devm_of_phy_get(dev, np, NULL);
+	ep->phy = devm_of_phy_get(dev, np, "pcie-phy");
 	if (IS_ERR(ep->phy)) {
 		if (PTR_ERR(ep->phy) == -EPROBE_DEFER)
 			return PTR_ERR(ep->phy);
+		if (!of_device_is_compatible(np, "samsung,exynos5440-pcie")) {
+			dev_err(dev, "Can't find the pcie-phy\n");
+			return PTR_ERR(ep->phy);
+		}
 		dev_warn(dev, "Use the 'phy' property. Current DT of pci-exynos was deprecated!!\n");
 	} else
 		ep->using_phy = true;
@@ -734,23 +840,20 @@ static int __exit exynos_pcie_remove(struct platform_device *pdev)
 static const struct of_device_id exynos_pcie_of_match[] = {
 	{
 		.compatible = "samsung,exynos5440-pcie",
-		.data = &exynos5440_pcie_ops
+		.data = &exynos5440_pcie_ops,
+	}, {
+		.compatible = "samsung,exynos5433-pcie",
+		.data = &exynos5433_pcie_ops,
 	},
 	{},
 };
 
 static struct platform_driver exynos_pcie_driver = {
+	.probe		= exynos_pcie_probe,
 	.remove		= __exit_p(exynos_pcie_remove),
 	.driver = {
 		.name	= "exynos-pcie",
 		.of_match_table = exynos_pcie_of_match,
 	},
 };
-
-/* Exynos PCIe driver does not allow module unload */
-
-static int __init exynos_pcie_init(void)
-{
-	return platform_driver_probe(&exynos_pcie_driver, exynos_pcie_probe);
-}
-subsys_initcall(exynos_pcie_init);
+builtin_platform_driver(exynos_pcie_driver);
-- 
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 2/2] pci: dwc: pci-exynos: add the codes to support the exynos5433

[Jingoo Han]

On Thursday, December 21, 2017 7:14 AM, Jaehoon Chung wrote:
> 
> Exynos5433 has the PCIe for WiFi.
> Added the codes relevant to PCIe for supporting the exynos5433.
> Also changed the binding documentation name to
> 'samsung,exynos-pcie.txt'.
> (It's not only exynos5440 anymore.)
> 

I have no objection.
However, I added some comments about Exynos5440.

> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
>  ...exynos5440-pcie.txt => samsung,exynos-pcie.txt} |   2 +-
>  drivers/pci/dwc/pci-exynos.c                       | 183
++++++++++++++++-----
>  2 files changed, 144 insertions(+), 41 deletions(-)
>  rename Documentation/devicetree/bindings/pci/{samsung,exynos5440-pcie.txt
> => samsung,exynos-pcie.txt} (97%)
> 
> diff --git a/Documentation/devicetree/bindings/pci/samsung,exynos5440-
> pcie.txt b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.txt
> similarity index 97%
> rename from Documentation/devicetree/bindings/pci/samsung,exynos5440-
> pcie.txt
> rename to Documentation/devicetree/bindings/pci/samsung,exynos-pcie.txt
> index 34a11bfbfb60..958dcc150505 100644
> --- a/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.txt
> @@ -4,7 +4,7 @@ This PCIe host controller is based on the Synopsys
> DesignWare PCIe IP
>  and thus inherits all the common properties defined in designware-
> pcie.txt.
> 
>  Required properties:
> -- compatible: "samsung,exynos5440-pcie"
> +- compatible: "samsung,exynos5440-pcie" or "samsung,exynos5433-pcie"
>  - reg: base addresses and lengths of the PCIe controller,
>  	the PHY controller, additional register for the PHY controller.
>  	(Registers for the PHY controller are DEPRECATED.
> diff --git a/drivers/pci/dwc/pci-exynos.c b/drivers/pci/dwc/pci-exynos.c
> index 5596fdedbb94..8dee2e90347e 100644
> --- a/drivers/pci/dwc/pci-exynos.c
> +++ b/drivers/pci/dwc/pci-exynos.c
> @@ -40,6 +40,8 @@
>  #define PCIE_IRQ_SPECIAL		0x008
>  #define PCIE_IRQ_EN_PULSE		0x00c
>  #define PCIE_IRQ_EN_LEVEL		0x010
> +#define PCIE_SW_WAKE			0x018
> +#define PCIE_BUS_EN			BIT(1)
>  #define IRQ_MSI_ENABLE			BIT(2)
>  #define PCIE_IRQ_EN_SPECIAL		0x014
>  #define PCIE_PWR_RESET			0x018
> @@ -49,7 +51,8 @@
>  #define PCIE_NONSTICKY_RESET		0x024
>  #define PCIE_APP_INIT_RESET		0x028
>  #define PCIE_APP_LTSSM_ENABLE		0x02c
> -#define PCIE_ELBI_RDLH_LINKUP		0x064
> +#define PCIE_ELBI_RDLH_LINKUP		0x074

The address of this register should be 0x064 for exynos5440.
Howe about the following?

+#define EXYNOS5440_PCIE_ELBI_RDLH_LINKUP	0x064
+#define PCIE_ELBI_RDLH_LINKUP		0x074

Or you can add the following.

/* Exynos5440 PCIe ELBI registers */
#define EXYNOS5440_PCIE_ELBI_RDLH_LINKUP	0x064

> +#define PCIE_ELBI_XMLH_LINKUP		BIT(4)
>  #define PCIE_ELBI_LTSSM_ENABLE		0x1
>  #define PCIE_ELBI_SLV_AWMISC		0x11c
>  #define PCIE_ELBI_SLV_ARMISC		0x120
> @@ -94,6 +97,10 @@
>  #define PCIE_PHY_TRSV3_PD_TSV		BIT(7)
>  #define PCIE_PHY_TRSV3_LVCC		0x31c
> 
> +/* DBI register */
> +#define PCIE_MISC_CONTROL_1_OFF		0x8BC
> +#define DBI_RO_WR_EN			BIT(0)
> +
>  struct exynos_pcie_mem_res {
>  	void __iomem *elbi_base;   /* DT 0th resource: PCIe CTRL */
>  	void __iomem *phy_base;    /* DT 1st resource: PHY CTRL */
> @@ -221,6 +228,96 @@ static const struct exynos_pcie_ops
> exynos5440_pcie_ops = {
>  	.deinit_clk_resources	= exynos5440_pcie_deinit_clk_resources,
>  };
> 
> +static int exynos5433_pcie_get_mem_resources(struct platform_device
*pdev,
> +					     struct exynos_pcie *ep)
> +{
> +	struct dw_pcie *pci = ep->pci;
> +	struct device *dev = pci->dev;
> +	struct resource *res;
> +
> +	ep->mem_res = devm_kzalloc(dev, sizeof(*ep->mem_res), GFP_KERNEL);
> +	if (!ep->mem_res)
> +		return -ENOMEM;
> +
> +	/* External Local Bus interface(ELBI) Register */
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "elbi");
> +	ep->mem_res->elbi_base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(ep->mem_res->elbi_base))
> +		return PTR_ERR(ep->mem_res->elbi_base);
> +
> +	/* Data Bus Interface(DBI) Register */
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
> +	pci->dbi_base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(pci->dbi_base))
> +		return PTR_ERR(pci->dbi_base);
> +
> +	return 0;
> +}
> +
> +static int exynos5433_pcie_get_clk_resources(struct exynos_pcie *ep)
> +{
> +	struct dw_pcie *pci = ep->pci;
> +	struct device *dev = pci->dev;
> +
> +	ep->clk_res = devm_kzalloc(dev, sizeof(*ep->clk_res), GFP_KERNEL);
> +	if (!ep->clk_res)
> +		return -ENOMEM;
> +
> +	ep->clk_res->clk = devm_clk_get(dev, "pcie");
> +	if (IS_ERR(ep->clk_res->clk)) {
> +		dev_err(dev, "Failed to get pcie rc clock\n");
> +		return PTR_ERR(ep->clk_res->clk);
> +	}
> +
> +	ep->clk_res->bus_clk = devm_clk_get(dev, "pcie_bus");
> +	if (IS_ERR(ep->clk_res->bus_clk)) {
> +		dev_err(dev, "Failed to get pcie bus clock\n");
> +		return PTR_ERR(ep->clk_res->bus_clk);
> +	}
> +
> +	return 0;
> +}
> +
> +static void exynos5433_pcie_deinit_clk_resources(struct exynos_pcie *ep)
> +{
> +	clk_disable_unprepare(ep->clk_res->bus_clk);
> +	clk_disable_unprepare(ep->clk_res->clk);
> +}
> +
> +
> +static int exynos5433_pcie_init_clk_resources(struct exynos_pcie *ep)
> +{
> +	struct dw_pcie *pci = ep->pci;
> +	struct device *dev = pci->dev;
> +	int ret;
> +
> +	ret = clk_prepare_enable(ep->clk_res->clk);
> +	if (ret) {
> +		dev_err(dev, "cannot enable pcie rc clock");
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(ep->clk_res->bus_clk);
> +	if (ret) {
> +		dev_err(dev, "cannot enable pcie bus clock");
> +		goto err_bus_clk;
> +	}
> +
> +	return 0;
> +
> +err_bus_clk:
> +	clk_disable_unprepare(ep->clk_res->clk);
> +
> +	return ret;
> +}
> +
> +static const struct exynos_pcie_ops exynos5433_pcie_ops = {
> +	.get_mem_resources	= exynos5433_pcie_get_mem_resources,
> +	.get_clk_resources	= exynos5433_pcie_get_clk_resources,
> +	.init_clk_resources	= exynos5433_pcie_init_clk_resources,
> +	.deinit_clk_resources	= exynos5433_pcie_deinit_clk_resources,
> +};
> +
>  static void exynos_pcie_writel(void __iomem *base, u32 val, u32 reg)
>  {
>  	writel(val, base + reg);
> @@ -279,7 +376,9 @@ static void exynos_pcie_deassert_core_reset(struct
> exynos_pcie *ep)
>  	exynos_pcie_writel(ep->mem_res->elbi_base, 1, PCIE_NONSTICKY_RESET);
>  	exynos_pcie_writel(ep->mem_res->elbi_base, 1, PCIE_APP_INIT_RESET);
>  	exynos_pcie_writel(ep->mem_res->elbi_base, 0, PCIE_APP_INIT_RESET);
> -	exynos_pcie_writel(ep->mem_res->block_base, 1, PCIE_PHY_MAC_RESET);
> +	if (ep->mem_res->block_base)
> +		exynos_pcie_writel(ep->mem_res->block_base, 1,
> +				PCIE_PHY_MAC_RESET);

Good.

>  }
> 
>  static void exynos_pcie_assert_phy_reset(struct exynos_pcie *ep)
> @@ -413,9 +512,6 @@ static int exynos_pcie_establish_link(struct
> exynos_pcie *ep)
>  	if (ep->using_phy) {
>  		phy_reset(ep->phy);
> 
> -		exynos_pcie_writel(ep->mem_res->elbi_base, 1,
> -				PCIE_PWR_RESET);
> -
>  		phy_power_on(ep->phy);
>  		phy_init(ep->phy);
>  	} else {
> @@ -430,14 +526,16 @@ static int exynos_pcie_establish_link(struct
> exynos_pcie *ep)
>  		udelay(500);
>  		exynos_pcie_writel(ep->mem_res->block_base, 0,
>  					PCIE_PHY_COMMON_RESET);
> +		exynos_pcie_deassert_core_reset(ep);
>  	}
> 
> -	/* pulse for common reset */
> -	exynos_pcie_writel(ep->mem_res->block_base, 1,
> PCIE_PHY_COMMON_RESET);
> -	udelay(500);
> -	exynos_pcie_writel(ep->mem_res->block_base, 0,
> PCIE_PHY_COMMON_RESET);

These codes are also necessary for Exyno5440.
How about moving these codes instead of removing them?

@@ -430,14 +526,16 @@ static int exynos_pcie_establish_link(struct
exynos_pcie *ep)
 		udelay(500);
 		exynos_pcie_writel(ep->mem_res->block_base, 0,
 					PCIE_PHY_COMMON_RESET);
+		/* pulse for common reset */
+		exynos_pcie_writel(ep->mem_res->block_base, 1,
+					PCIE_PHY_COMMON_RESET);
+		udelay(500);
+		exynos_pcie_writel(ep->mem_res->block_base, 0,
+					PCIE_PHY_COMMON_RESET);
+		exynos_pcie_deassert_core_reset(ep);
 	}


> +	/*
> +	 * Enable DBI_RO_WR_EN bit.
> +	 * - When set to 1, some RO and HWinit bits are wriatble from
> +	 *   the local application through the DBI.
> +	 */
> +	dw_pcie_writel_dbi(pci, PCIE_MISC_CONTROL_1_OFF, DBI_RO_WR_EN);
> 
> -	exynos_pcie_deassert_core_reset(ep);
>  	dw_pcie_setup_rc(pp);
>  	exynos_pcie_assert_reset(ep);
> 
> @@ -472,16 +570,6 @@ static void exynos_pcie_clear_irq_pulse(struct
> exynos_pcie *ep)
>  	exynos_pcie_writel(ep->mem_res->elbi_base, val, PCIE_IRQ_PULSE);
>  }
> 
> -static void exynos_pcie_enable_irq_pulse(struct exynos_pcie *ep)
> -{
> -	u32 val;
> -
> -	/* enable INTX interrupt */
> -	val = IRQ_INTA_ASSERT | IRQ_INTB_ASSERT |
> -		IRQ_INTC_ASSERT | IRQ_INTD_ASSERT;
> -	exynos_pcie_writel(ep->mem_res->elbi_base, val, PCIE_IRQ_EN_PULSE);
> -}
> -
>  static irqreturn_t exynos_pcie_irq_handler(int irq, void *arg)
>  {
>  	struct exynos_pcie *ep = arg;
> @@ -513,9 +601,16 @@ static void exynos_pcie_msi_init(struct exynos_pcie
> *ep)
>  	exynos_pcie_writel(ep->mem_res->elbi_base, val, PCIE_IRQ_EN_LEVEL);
>  }
> 
> -static void exynos_pcie_enable_interrupts(struct exynos_pcie *ep)
> +static void exynos_pcie_enable_irq_pulse(struct exynos_pcie *ep)
>  {
> -	exynos_pcie_enable_irq_pulse(ep);
> +	u32 val;
> +
> +	val = IRQ_INTA_ASSERT | IRQ_INTB_ASSERT |
> +		IRQ_INTC_ASSERT | IRQ_INTD_ASSERT;
> +	exynos_pcie_writel(ep->mem_res->elbi_base, val, PCIE_IRQ_EN_PULSE);
> +
> +	exynos_pcie_writel(ep->mem_res->elbi_base, 0, PCIE_IRQ_EN_LEVEL);
> +	exynos_pcie_writel(ep->mem_res->elbi_base, 0, PCIE_IRQ_EN_SPECIAL);

Good.

> 
>  	if (IS_ENABLED(CONFIG_PCI_MSI))
>  		exynos_pcie_msi_init(ep);
> @@ -575,10 +670,8 @@ static int exynos_pcie_link_up(struct dw_pcie *pci)
>  	u32 val;
> 
>  	val = exynos_pcie_readl(ep->mem_res->elbi_base,
> PCIE_ELBI_RDLH_LINKUP);
> -	if (val == PCIE_ELBI_LTSSM_ENABLE)
> -		return 1;

Exynos5440 uses 'PCIE_ELBI_LTSSM_ENABLE'.
Can you keep this code for Exyno5440?

This register can be added as below.

/* Exynos5440 PCIe ELBI registers */
#define EXYNOS5440_PCIE_ELBI_RDLH_LINKUP	0x064
#define EXYNOS5440_PCIE_ELBI_LTSSM_ENABLE	BIT(0)

Best regards,
Jingoo Han

> 
> -	return 0;
> +	return (val & PCIE_ELBI_XMLH_LINKUP);
>  }
> 
>  static int exynos_pcie_host_init(struct pcie_port *pp)
> @@ -587,7 +680,7 @@ static int exynos_pcie_host_init(struct pcie_port *pp)
>  	struct exynos_pcie *ep = to_exynos_pcie(pci);
> 
>  	exynos_pcie_establish_link(ep);
> -	exynos_pcie_enable_interrupts(ep);
> +	exynos_pcie_enable_irq_pulse(ep);
> 
>  	return 0;
>  }
> @@ -608,8 +701,11 @@ static int __init exynos_add_pcie_port(struct
> exynos_pcie *ep,
> 
>  	pp->irq = platform_get_irq(pdev, 1);
>  	if (pp->irq < 0) {
> -		dev_err(dev, "failed to get irq\n");
> -		return pp->irq;
> +		pp->irq = platform_get_irq_byname(pdev, "intr");
> +		if (pp->irq < 0) {
> +			dev_err(dev, "failed to get irq\n");
> +			return pp->irq;
> +		}
>  	}
>  	ret = devm_request_irq(dev, pp->irq, exynos_pcie_irq_handler,
>  				IRQF_SHARED, "exynos-pcie", ep);
> @@ -678,13 +774,23 @@ static int __init exynos_pcie_probe(struct
> platform_device *pdev)
> 
>  	ep->reset_gpio = of_get_named_gpio(np, "reset-gpio", 0);
> 
> -	/* Assume that controller doesn't use the PHY framework */
> -	ep->using_phy = false;
> +	/*
> +	 * In case of Exynos5440,
> +	 * Assume that controller doesn't use the PHY frameork.
> +	 * Other SoCs might be used the PHY framework.
> +	 */
> +
> +	if (of_device_is_compatible(np, "samsung,exynos5440-pcie"))
> +		ep->using_phy = false;
> 
> -	ep->phy = devm_of_phy_get(dev, np, NULL);
> +	ep->phy = devm_of_phy_get(dev, np, "pcie-phy");
>  	if (IS_ERR(ep->phy)) {
>  		if (PTR_ERR(ep->phy) == -EPROBE_DEFER)
>  			return PTR_ERR(ep->phy);
> +		if (!of_device_is_compatible(np, "samsung,exynos5440-pcie"))
> {
> +			dev_err(dev, "Can't find the pcie-phy\n");
> +			return PTR_ERR(ep->phy);
> +		}
>  		dev_warn(dev, "Use the 'phy' property. Current DT of pci-
> exynos was deprecated!!\n");
>  	} else
>  		ep->using_phy = true;
> @@ -734,23 +840,20 @@ static int __exit exynos_pcie_remove(struct
> platform_device *pdev)
>  static const struct of_device_id exynos_pcie_of_match[] = {
>  	{
>  		.compatible = "samsung,exynos5440-pcie",
> -		.data = &exynos5440_pcie_ops
> +		.data = &exynos5440_pcie_ops,
> +	}, {
> +		.compatible = "samsung,exynos5433-pcie",
> +		.data = &exynos5433_pcie_ops,
>  	},
>  	{},
>  };
> 
>  static struct platform_driver exynos_pcie_driver = {
> +	.probe		= exynos_pcie_probe,
>  	.remove		= __exit_p(exynos_pcie_remove),
>  	.driver = {
>  		.name	= "exynos-pcie",
>  		.of_match_table = exynos_pcie_of_match,
>  	},
>  };
> -
> -/* Exynos PCIe driver does not allow module unload */
> -
> -static int __init exynos_pcie_init(void)
> -{
> -	return platform_driver_probe(&exynos_pcie_driver,
> exynos_pcie_probe);
> -}
> -subsys_initcall(exynos_pcie_init);
> +builtin_platform_driver(exynos_pcie_driver);
> --
> 2.15.1

Re: [RFC 2/2] pci: dwc: pci-exynos: add the codes to support the exynos5433

[Jingoo Han]

On Thursday, December 21, 2017 11:13 AM, Jingoo Han wrote:
> On Thursday, December 21, 2017 7:14 AM, Jaehoon Chung wrote:
> >
> > Exynos5433 has the PCIe for WiFi.
> > Added the codes relevant to PCIe for supporting the exynos5433.
> > Also changed the binding documentation name to
> > 'samsung,exynos-pcie.txt'.
> > (It's not only exynos5440 anymore.)
> >
> 
> I have no objection.
> However, I added some comments about Exynos5440.
> 
> > Signed-off-by: Jaehoon Chung <jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> > ---
> >  ...exynos5440-pcie.txt => samsung,exynos-pcie.txt} |   2 +-
> >  drivers/pci/dwc/pci-exynos.c                       | 183
> ++++++++++++++++-----
> >  2 files changed, 144 insertions(+), 41 deletions(-)
> >  rename Documentation/devicetree/bindings/pci/{samsung,exynos5440-
> pcie.txt
> > => samsung,exynos-pcie.txt} (97%)
> >
> > diff --git a/Documentation/devicetree/bindings/pci/samsung,exynos5440-
> > pcie.txt b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.txt
> > similarity index 97%
> > rename from Documentation/devicetree/bindings/pci/samsung,exynos5440-
> > pcie.txt
> > rename to Documentation/devicetree/bindings/pci/samsung,exynos-pcie.txt
> > index 34a11bfbfb60..958dcc150505 100644
> > --- a/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt
> > +++ b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.txt
> > @@ -4,7 +4,7 @@ This PCIe host controller is based on the Synopsys
> > DesignWare PCIe IP
> >  and thus inherits all the common properties defined in designware-
> > pcie.txt.
> >
> >  Required properties:
> > -- compatible: "samsung,exynos5440-pcie"
> > +- compatible: "samsung,exynos5440-pcie" or "samsung,exynos5433-pcie"
> >  - reg: base addresses and lengths of the PCIe controller,
> >  	the PHY controller, additional register for the PHY controller.
> >  	(Registers for the PHY controller are DEPRECATED.
> > diff --git a/drivers/pci/dwc/pci-exynos.c b/drivers/pci/dwc/pci-exynos.c
> > index 5596fdedbb94..8dee2e90347e 100644
> > --- a/drivers/pci/dwc/pci-exynos.c
> > +++ b/drivers/pci/dwc/pci-exynos.c
> > @@ -40,6 +40,8 @@
> >  #define PCIE_IRQ_SPECIAL		0x008
> >  #define PCIE_IRQ_EN_PULSE		0x00c
> >  #define PCIE_IRQ_EN_LEVEL		0x010
> > +#define PCIE_SW_WAKE			0x018
> > +#define PCIE_BUS_EN			BIT(1)
> >  #define IRQ_MSI_ENABLE			BIT(2)
> >  #define PCIE_IRQ_EN_SPECIAL		0x014
> >  #define PCIE_PWR_RESET			0x018
> > @@ -49,7 +51,8 @@
> >  #define PCIE_NONSTICKY_RESET		0x024
> >  #define PCIE_APP_INIT_RESET		0x028
> >  #define PCIE_APP_LTSSM_ENABLE		0x02c
> > -#define PCIE_ELBI_RDLH_LINKUP		0x064
> > +#define PCIE_ELBI_RDLH_LINKUP		0x074
> 
> The address of this register should be 0x064 for exynos5440.
> Howe about the following?
> 
> +#define EXYNOS5440_PCIE_ELBI_RDLH_LINKUP	0x064
> +#define PCIE_ELBI_RDLH_LINKUP		0x074
> 
> Or you can add the following.
> 
> /* Exynos5440 PCIe ELBI registers */
> #define EXYNOS5440_PCIE_ELBI_RDLH_LINKUP	0x064
> 
> > +#define PCIE_ELBI_XMLH_LINKUP		BIT(4)
> >  #define PCIE_ELBI_LTSSM_ENABLE		0x1
> >  #define PCIE_ELBI_SLV_AWMISC		0x11c
> >  #define PCIE_ELBI_SLV_ARMISC		0x120
> > @@ -94,6 +97,10 @@
> >  #define PCIE_PHY_TRSV3_PD_TSV		BIT(7)
> >  #define PCIE_PHY_TRSV3_LVCC		0x31c
> >
> > +/* DBI register */
> > +#define PCIE_MISC_CONTROL_1_OFF		0x8BC
> > +#define DBI_RO_WR_EN			BIT(0)
> > +
> >  struct exynos_pcie_mem_res {
> >  	void __iomem *elbi_base;   /* DT 0th resource: PCIe CTRL */
> >  	void __iomem *phy_base;    /* DT 1st resource: PHY CTRL */
> > @@ -221,6 +228,96 @@ static const struct exynos_pcie_ops
> > exynos5440_pcie_ops = {
> >  	.deinit_clk_resources	= exynos5440_pcie_deinit_clk_resources,
> >  };
> >
> > +static int exynos5433_pcie_get_mem_resources(struct platform_device
> *pdev,
> > +					     struct exynos_pcie *ep)
> > +{
> > +	struct dw_pcie *pci = ep->pci;
> > +	struct device *dev = pci->dev;
> > +	struct resource *res;
> > +
> > +	ep->mem_res = devm_kzalloc(dev, sizeof(*ep->mem_res), GFP_KERNEL);
> > +	if (!ep->mem_res)
> > +		return -ENOMEM;
> > +
> > +	/* External Local Bus interface(ELBI) Register */
> > +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "elbi");
> > +	ep->mem_res->elbi_base = devm_ioremap_resource(&pdev->dev, res);
> > +	if (IS_ERR(ep->mem_res->elbi_base))
> > +		return PTR_ERR(ep->mem_res->elbi_base);
> > +
> > +	/* Data Bus Interface(DBI) Register */
> > +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
> > +	pci->dbi_base = devm_ioremap_resource(&pdev->dev, res);
> > +	if (IS_ERR(pci->dbi_base))
> > +		return PTR_ERR(pci->dbi_base);
> > +
> > +	return 0;
> > +}
> > +
> > +static int exynos5433_pcie_get_clk_resources(struct exynos_pcie *ep)
> > +{
> > +	struct dw_pcie *pci = ep->pci;
> > +	struct device *dev = pci->dev;
> > +
> > +	ep->clk_res = devm_kzalloc(dev, sizeof(*ep->clk_res), GFP_KERNEL);
> > +	if (!ep->clk_res)
> > +		return -ENOMEM;
> > +
> > +	ep->clk_res->clk = devm_clk_get(dev, "pcie");
> > +	if (IS_ERR(ep->clk_res->clk)) {
> > +		dev_err(dev, "Failed to get pcie rc clock\n");
> > +		return PTR_ERR(ep->clk_res->clk);
> > +	}
> > +
> > +	ep->clk_res->bus_clk = devm_clk_get(dev, "pcie_bus");
> > +	if (IS_ERR(ep->clk_res->bus_clk)) {
> > +		dev_err(dev, "Failed to get pcie bus clock\n");
> > +		return PTR_ERR(ep->clk_res->bus_clk);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static void exynos5433_pcie_deinit_clk_resources(struct exynos_pcie
*ep)
> > +{
> > +	clk_disable_unprepare(ep->clk_res->bus_clk);
> > +	clk_disable_unprepare(ep->clk_res->clk);
> > +}
> > +
> > +
> > +static int exynos5433_pcie_init_clk_resources(struct exynos_pcie *ep)
> > +{
> > +	struct dw_pcie *pci = ep->pci;
> > +	struct device *dev = pci->dev;
> > +	int ret;
> > +
> > +	ret = clk_prepare_enable(ep->clk_res->clk);
> > +	if (ret) {
> > +		dev_err(dev, "cannot enable pcie rc clock");
> > +		return ret;
> > +	}
> > +
> > +	ret = clk_prepare_enable(ep->clk_res->bus_clk);
> > +	if (ret) {
> > +		dev_err(dev, "cannot enable pcie bus clock");
> > +		goto err_bus_clk;
> > +	}
> > +
> > +	return 0;
> > +
> > +err_bus_clk:
> > +	clk_disable_unprepare(ep->clk_res->clk);
> > +
> > +	return ret;
> > +}
> > +
> > +static const struct exynos_pcie_ops exynos5433_pcie_ops = {
> > +	.get_mem_resources	= exynos5433_pcie_get_mem_resources,
> > +	.get_clk_resources	= exynos5433_pcie_get_clk_resources,
> > +	.init_clk_resources	= exynos5433_pcie_init_clk_resources,
> > +	.deinit_clk_resources	= exynos5433_pcie_deinit_clk_resources,
> > +};
> > +
> >  static void exynos_pcie_writel(void __iomem *base, u32 val, u32 reg)
> >  {
> >  	writel(val, base + reg);
> > @@ -279,7 +376,9 @@ static void exynos_pcie_deassert_core_reset(struct
> > exynos_pcie *ep)
> >  	exynos_pcie_writel(ep->mem_res->elbi_base, 1, PCIE_NONSTICKY_RESET);
> >  	exynos_pcie_writel(ep->mem_res->elbi_base, 1, PCIE_APP_INIT_RESET);
> >  	exynos_pcie_writel(ep->mem_res->elbi_base, 0, PCIE_APP_INIT_RESET);
> > -	exynos_pcie_writel(ep->mem_res->block_base, 1, PCIE_PHY_MAC_RESET);
> > +	if (ep->mem_res->block_base)
> > +		exynos_pcie_writel(ep->mem_res->block_base, 1,
> > +				PCIE_PHY_MAC_RESET);
> 
> Good.
> 
> >  }
> >
> >  static void exynos_pcie_assert_phy_reset(struct exynos_pcie *ep)
> > @@ -413,9 +512,6 @@ static int exynos_pcie_establish_link(struct
> > exynos_pcie *ep)
> >  	if (ep->using_phy) {
> >  		phy_reset(ep->phy);
> >
> > -		exynos_pcie_writel(ep->mem_res->elbi_base, 1,
> > -				PCIE_PWR_RESET);
> > -
> >  		phy_power_on(ep->phy);
> >  		phy_init(ep->phy);
> >  	} else {
> > @@ -430,14 +526,16 @@ static int exynos_pcie_establish_link(struct
> > exynos_pcie *ep)
> >  		udelay(500);
> >  		exynos_pcie_writel(ep->mem_res->block_base, 0,
> >  					PCIE_PHY_COMMON_RESET);
> > +		exynos_pcie_deassert_core_reset(ep);
> >  	}
> >
> > -	/* pulse for common reset */
> > -	exynos_pcie_writel(ep->mem_res->block_base, 1,
> > PCIE_PHY_COMMON_RESET);
> > -	udelay(500);
> > -	exynos_pcie_writel(ep->mem_res->block_base, 0,
> > PCIE_PHY_COMMON_RESET);
> 
> These codes are also necessary for Exyno5440.
> How about moving these codes instead of removing them?
> 
> @@ -430,14 +526,16 @@ static int exynos_pcie_establish_link(struct
> exynos_pcie *ep)
>  		udelay(500);
>  		exynos_pcie_writel(ep->mem_res->block_base, 0,
>  					PCIE_PHY_COMMON_RESET);
> +		/* pulse for common reset */
> +		exynos_pcie_writel(ep->mem_res->block_base, 1,
> +					PCIE_PHY_COMMON_RESET);
> +		udelay(500);
> +		exynos_pcie_writel(ep->mem_res->block_base, 0,
> +					PCIE_PHY_COMMON_RESET);
> +		exynos_pcie_deassert_core_reset(ep);
>  	}

I checked that this code is duplicated.
So, you can remove this one.
Sorry for making you confused.

Best regards,
Jingoo Han

> 
> 
> > +	/*
> > +	 * Enable DBI_RO_WR_EN bit.
> > +	 * - When set to 1, some RO and HWinit bits are wriatble from
> > +	 *   the local application through the DBI.
> > +	 */
> > +	dw_pcie_writel_dbi(pci, PCIE_MISC_CONTROL_1_OFF, DBI_RO_WR_EN);
> >
> > -	exynos_pcie_deassert_core_reset(ep);
> >  	dw_pcie_setup_rc(pp);
> >  	exynos_pcie_assert_reset(ep);
> >
> > @@ -472,16 +570,6 @@ static void exynos_pcie_clear_irq_pulse(struct
> > exynos_pcie *ep)
> >  	exynos_pcie_writel(ep->mem_res->elbi_base, val, PCIE_IRQ_PULSE);
> >  }
> >
> > -static void exynos_pcie_enable_irq_pulse(struct exynos_pcie *ep)
> > -{
> > -	u32 val;
> > -
> > -	/* enable INTX interrupt */
> > -	val = IRQ_INTA_ASSERT | IRQ_INTB_ASSERT |
> > -		IRQ_INTC_ASSERT | IRQ_INTD_ASSERT;
> > -	exynos_pcie_writel(ep->mem_res->elbi_base, val, PCIE_IRQ_EN_PULSE);
> > -}
> > -
> >  static irqreturn_t exynos_pcie_irq_handler(int irq, void *arg)
> >  {
> >  	struct exynos_pcie *ep = arg;
> > @@ -513,9 +601,16 @@ static void exynos_pcie_msi_init(struct exynos_pcie
> > *ep)
> >  	exynos_pcie_writel(ep->mem_res->elbi_base, val, PCIE_IRQ_EN_LEVEL);
> >  }
> >
> > -static void exynos_pcie_enable_interrupts(struct exynos_pcie *ep)
> > +static void exynos_pcie_enable_irq_pulse(struct exynos_pcie *ep)
> >  {
> > -	exynos_pcie_enable_irq_pulse(ep);
> > +	u32 val;
> > +
> > +	val = IRQ_INTA_ASSERT | IRQ_INTB_ASSERT |
> > +		IRQ_INTC_ASSERT | IRQ_INTD_ASSERT;
> > +	exynos_pcie_writel(ep->mem_res->elbi_base, val, PCIE_IRQ_EN_PULSE);
> > +
> > +	exynos_pcie_writel(ep->mem_res->elbi_base, 0, PCIE_IRQ_EN_LEVEL);
> > +	exynos_pcie_writel(ep->mem_res->elbi_base, 0, PCIE_IRQ_EN_SPECIAL);
> 
> Good.
> 
> >
> >  	if (IS_ENABLED(CONFIG_PCI_MSI))
> >  		exynos_pcie_msi_init(ep);
> > @@ -575,10 +670,8 @@ static int exynos_pcie_link_up(struct dw_pcie *pci)
> >  	u32 val;
> >
> >  	val = exynos_pcie_readl(ep->mem_res->elbi_base,
> > PCIE_ELBI_RDLH_LINKUP);
> > -	if (val == PCIE_ELBI_LTSSM_ENABLE)
> > -		return 1;
> 
> Exynos5440 uses 'PCIE_ELBI_LTSSM_ENABLE'.
> Can you keep this code for Exyno5440?
> 
> This register can be added as below.
> 
> /* Exynos5440 PCIe ELBI registers */
> #define EXYNOS5440_PCIE_ELBI_RDLH_LINKUP	0x064
> #define EXYNOS5440_PCIE_ELBI_LTSSM_ENABLE	BIT(0)
> 
> Best regards,
> Jingoo Han
> 
> >
> > -	return 0;
> > +	return (val & PCIE_ELBI_XMLH_LINKUP);
> >  }
> >
> >  static int exynos_pcie_host_init(struct pcie_port *pp)
> > @@ -587,7 +680,7 @@ static int exynos_pcie_host_init(struct pcie_port
> *pp)
> >  	struct exynos_pcie *ep = to_exynos_pcie(pci);
> >
> >  	exynos_pcie_establish_link(ep);
> > -	exynos_pcie_enable_interrupts(ep);
> > +	exynos_pcie_enable_irq_pulse(ep);
> >
> >  	return 0;
> >  }
> > @@ -608,8 +701,11 @@ static int __init exynos_add_pcie_port(struct
> > exynos_pcie *ep,
> >
> >  	pp->irq = platform_get_irq(pdev, 1);
> >  	if (pp->irq < 0) {
> > -		dev_err(dev, "failed to get irq\n");
> > -		return pp->irq;
> > +		pp->irq = platform_get_irq_byname(pdev, "intr");
> > +		if (pp->irq < 0) {
> > +			dev_err(dev, "failed to get irq\n");
> > +			return pp->irq;
> > +		}
> >  	}
> >  	ret = devm_request_irq(dev, pp->irq, exynos_pcie_irq_handler,
> >  				IRQF_SHARED, "exynos-pcie", ep);
> > @@ -678,13 +774,23 @@ static int __init exynos_pcie_probe(struct
> > platform_device *pdev)
> >
> >  	ep->reset_gpio = of_get_named_gpio(np, "reset-gpio", 0);
> >
> > -	/* Assume that controller doesn't use the PHY framework */
> > -	ep->using_phy = false;
> > +	/*
> > +	 * In case of Exynos5440,
> > +	 * Assume that controller doesn't use the PHY frameork.
> > +	 * Other SoCs might be used the PHY framework.
> > +	 */
> > +
> > +	if (of_device_is_compatible(np, "samsung,exynos5440-pcie"))
> > +		ep->using_phy = false;
> >
> > -	ep->phy = devm_of_phy_get(dev, np, NULL);
> > +	ep->phy = devm_of_phy_get(dev, np, "pcie-phy");
> >  	if (IS_ERR(ep->phy)) {
> >  		if (PTR_ERR(ep->phy) == -EPROBE_DEFER)
> >  			return PTR_ERR(ep->phy);
> > +		if (!of_device_is_compatible(np, "samsung,exynos5440-pcie"))
> > {
> > +			dev_err(dev, "Can't find the pcie-phy\n");
> > +			return PTR_ERR(ep->phy);
> > +		}
> >  		dev_warn(dev, "Use the 'phy' property. Current DT of pci-
> > exynos was deprecated!!\n");
> >  	} else
> >  		ep->using_phy = true;
> > @@ -734,23 +840,20 @@ static int __exit exynos_pcie_remove(struct
> > platform_device *pdev)
> >  static const struct of_device_id exynos_pcie_of_match[] = {
> >  	{
> >  		.compatible = "samsung,exynos5440-pcie",
> > -		.data = &exynos5440_pcie_ops
> > +		.data = &exynos5440_pcie_ops,
> > +	}, {
> > +		.compatible = "samsung,exynos5433-pcie",
> > +		.data = &exynos5433_pcie_ops,
> >  	},
> >  	{},
> >  };
> >
> >  static struct platform_driver exynos_pcie_driver = {
> > +	.probe		= exynos_pcie_probe,
> >  	.remove		= __exit_p(exynos_pcie_remove),
> >  	.driver = {
> >  		.name	= "exynos-pcie",
> >  		.of_match_table = exynos_pcie_of_match,
> >  	},
> >  };
> > -
> > -/* Exynos PCIe driver does not allow module unload */
> > -
> > -static int __init exynos_pcie_init(void)
> > -{
> > -	return platform_driver_probe(&exynos_pcie_driver,
> > exynos_pcie_probe);
> > -}
> > -subsys_initcall(exynos_pcie_init);
> > +builtin_platform_driver(exynos_pcie_driver);
> > --
> > 2.15.1
> 


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 2/2] pci: dwc: pci-exynos: add the codes to support the exynos5433

[Jaehoon Chung]

Hi Jingoo,

On 12/22/2017 01:12 AM, Jingoo Han wrote:
> On Thursday, December 21, 2017 7:14 AM, Jaehoon Chung wrote:
>>
>> Exynos5433 has the PCIe for WiFi.
>> Added the codes relevant to PCIe for supporting the exynos5433.
>> Also changed the binding documentation name to
>> 'samsung,exynos-pcie.txt'.
>> (It's not only exynos5440 anymore.)
>>
> 
> I have no objection.
> However, I added some comments about Exynos5440.
> 
>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>> ---
>>  ...exynos5440-pcie.txt => samsung,exynos-pcie.txt} |   2 +-
>>  drivers/pci/dwc/pci-exynos.c                       | 183
> ++++++++++++++++-----
>>  2 files changed, 144 insertions(+), 41 deletions(-)
>>  rename Documentation/devicetree/bindings/pci/{samsung,exynos5440-pcie.txt
>> => samsung,exynos-pcie.txt} (97%)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/samsung,exynos5440-
>> pcie.txt b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.txt
>> similarity index 97%
>> rename from Documentation/devicetree/bindings/pci/samsung,exynos5440-
>> pcie.txt
>> rename to Documentation/devicetree/bindings/pci/samsung,exynos-pcie.txt
>> index 34a11bfbfb60..958dcc150505 100644
>> --- a/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt
>> +++ b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.txt
>> @@ -4,7 +4,7 @@ This PCIe host controller is based on the Synopsys
>> DesignWare PCIe IP
>>  and thus inherits all the common properties defined in designware-
>> pcie.txt.
>>
>>  Required properties:
>> -- compatible: "samsung,exynos5440-pcie"
>> +- compatible: "samsung,exynos5440-pcie" or "samsung,exynos5433-pcie"
>>  - reg: base addresses and lengths of the PCIe controller,
>>  	the PHY controller, additional register for the PHY controller.
>>  	(Registers for the PHY controller are DEPRECATED.
>> diff --git a/drivers/pci/dwc/pci-exynos.c b/drivers/pci/dwc/pci-exynos.c
>> index 5596fdedbb94..8dee2e90347e 100644
>> --- a/drivers/pci/dwc/pci-exynos.c
>> +++ b/drivers/pci/dwc/pci-exynos.c
>> @@ -40,6 +40,8 @@
>>  #define PCIE_IRQ_SPECIAL		0x008
>>  #define PCIE_IRQ_EN_PULSE		0x00c
>>  #define PCIE_IRQ_EN_LEVEL		0x010
>> +#define PCIE_SW_WAKE			0x018
>> +#define PCIE_BUS_EN			BIT(1)
>>  #define IRQ_MSI_ENABLE			BIT(2)
>>  #define PCIE_IRQ_EN_SPECIAL		0x014
>>  #define PCIE_PWR_RESET			0x018
>> @@ -49,7 +51,8 @@
>>  #define PCIE_NONSTICKY_RESET		0x024
>>  #define PCIE_APP_INIT_RESET		0x028
>>  #define PCIE_APP_LTSSM_ENABLE		0x02c
>> -#define PCIE_ELBI_RDLH_LINKUP		0x064
>> +#define PCIE_ELBI_RDLH_LINKUP		0x074
> 
> The address of this register should be 0x064 for exynos5440.
> Howe about the following?
> 
> +#define EXYNOS5440_PCIE_ELBI_RDLH_LINKUP	0x064
> +#define PCIE_ELBI_RDLH_LINKUP		0x074
> 
> Or you can add the following.
> 
> /* Exynos5440 PCIe ELBI registers */
> #define EXYNOS5440_PCIE_ELBI_RDLH_LINKUP	0x064

Maybe, you're right. Because i didn't have Exynos5440 TRM, it's problem to me about updating other SoCs.
I have checked almost all variants Exynos. They are using the LINKUP register as 0x74.

If i can get the exynos5440 TRM, it's much helpful to me. Is it possible?

> 
>> +#define PCIE_ELBI_XMLH_LINKUP		BIT(4)
>>  #define PCIE_ELBI_LTSSM_ENABLE		0x1
>>  #define PCIE_ELBI_SLV_AWMISC		0x11c
>>  #define PCIE_ELBI_SLV_ARMISC		0x120
>> @@ -94,6 +97,10 @@
>>  #define PCIE_PHY_TRSV3_PD_TSV		BIT(7)
>>  #define PCIE_PHY_TRSV3_LVCC		0x31c
>>
>> +/* DBI register */
>> +#define PCIE_MISC_CONTROL_1_OFF		0x8BC
>> +#define DBI_RO_WR_EN			BIT(0)
>> +
>>  struct exynos_pcie_mem_res {
>>  	void __iomem *elbi_base;   /* DT 0th resource: PCIe CTRL */
>>  	void __iomem *phy_base;    /* DT 1st resource: PHY CTRL */
>> @@ -221,6 +228,96 @@ static const struct exynos_pcie_ops
>> exynos5440_pcie_ops = {
>>  	.deinit_clk_resources	= exynos5440_pcie_deinit_clk_resources,
>>  };
>>
>> +static int exynos5433_pcie_get_mem_resources(struct platform_device
> *pdev,
>> +					     struct exynos_pcie *ep)
>> +{
>> +	struct dw_pcie *pci = ep->pci;
>> +	struct device *dev = pci->dev;
>> +	struct resource *res;
>> +
>> +	ep->mem_res = devm_kzalloc(dev, sizeof(*ep->mem_res), GFP_KERNEL);
>> +	if (!ep->mem_res)
>> +		return -ENOMEM;
>> +
>> +	/* External Local Bus interface(ELBI) Register */
>> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "elbi");
>> +	ep->mem_res->elbi_base = devm_ioremap_resource(&pdev->dev, res);
>> +	if (IS_ERR(ep->mem_res->elbi_base))
>> +		return PTR_ERR(ep->mem_res->elbi_base);
>> +
>> +	/* Data Bus Interface(DBI) Register */
>> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
>> +	pci->dbi_base = devm_ioremap_resource(&pdev->dev, res);
>> +	if (IS_ERR(pci->dbi_base))
>> +		return PTR_ERR(pci->dbi_base);
>> +
>> +	return 0;
>> +}
>> +
>> +static int exynos5433_pcie_get_clk_resources(struct exynos_pcie *ep)
>> +{
>> +	struct dw_pcie *pci = ep->pci;
>> +	struct device *dev = pci->dev;
>> +
>> +	ep->clk_res = devm_kzalloc(dev, sizeof(*ep->clk_res), GFP_KERNEL);
>> +	if (!ep->clk_res)
>> +		return -ENOMEM;
>> +
>> +	ep->clk_res->clk = devm_clk_get(dev, "pcie");
>> +	if (IS_ERR(ep->clk_res->clk)) {
>> +		dev_err(dev, "Failed to get pcie rc clock\n");
>> +		return PTR_ERR(ep->clk_res->clk);
>> +	}
>> +
>> +	ep->clk_res->bus_clk = devm_clk_get(dev, "pcie_bus");
>> +	if (IS_ERR(ep->clk_res->bus_clk)) {
>> +		dev_err(dev, "Failed to get pcie bus clock\n");
>> +		return PTR_ERR(ep->clk_res->bus_clk);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static void exynos5433_pcie_deinit_clk_resources(struct exynos_pcie *ep)
>> +{
>> +	clk_disable_unprepare(ep->clk_res->bus_clk);
>> +	clk_disable_unprepare(ep->clk_res->clk);
>> +}
>> +
>> +
>> +static int exynos5433_pcie_init_clk_resources(struct exynos_pcie *ep)
>> +{
>> +	struct dw_pcie *pci = ep->pci;
>> +	struct device *dev = pci->dev;
>> +	int ret;
>> +
>> +	ret = clk_prepare_enable(ep->clk_res->clk);
>> +	if (ret) {
>> +		dev_err(dev, "cannot enable pcie rc clock");
>> +		return ret;
>> +	}
>> +
>> +	ret = clk_prepare_enable(ep->clk_res->bus_clk);
>> +	if (ret) {
>> +		dev_err(dev, "cannot enable pcie bus clock");
>> +		goto err_bus_clk;
>> +	}
>> +
>> +	return 0;
>> +
>> +err_bus_clk:
>> +	clk_disable_unprepare(ep->clk_res->clk);
>> +
>> +	return ret;
>> +}
>> +
>> +static const struct exynos_pcie_ops exynos5433_pcie_ops = {
>> +	.get_mem_resources	= exynos5433_pcie_get_mem_resources,
>> +	.get_clk_resources	= exynos5433_pcie_get_clk_resources,
>> +	.init_clk_resources	= exynos5433_pcie_init_clk_resources,
>> +	.deinit_clk_resources	= exynos5433_pcie_deinit_clk_resources,
>> +};
>> +
>>  static void exynos_pcie_writel(void __iomem *base, u32 val, u32 reg)
>>  {
>>  	writel(val, base + reg);
>> @@ -279,7 +376,9 @@ static void exynos_pcie_deassert_core_reset(struct
>> exynos_pcie *ep)
>>  	exynos_pcie_writel(ep->mem_res->elbi_base, 1, PCIE_NONSTICKY_RESET);
>>  	exynos_pcie_writel(ep->mem_res->elbi_base, 1, PCIE_APP_INIT_RESET);
>>  	exynos_pcie_writel(ep->mem_res->elbi_base, 0, PCIE_APP_INIT_RESET);
>> -	exynos_pcie_writel(ep->mem_res->block_base, 1, PCIE_PHY_MAC_RESET);
>> +	if (ep->mem_res->block_base)
>> +		exynos_pcie_writel(ep->mem_res->block_base, 1,
>> +				PCIE_PHY_MAC_RESET);
> 
> Good.
> 
>>  }
>>
>>  static void exynos_pcie_assert_phy_reset(struct exynos_pcie *ep)
>> @@ -413,9 +512,6 @@ static int exynos_pcie_establish_link(struct
>> exynos_pcie *ep)
>>  	if (ep->using_phy) {
>>  		phy_reset(ep->phy);
>>
>> -		exynos_pcie_writel(ep->mem_res->elbi_base, 1,
>> -				PCIE_PWR_RESET);
>> -
>>  		phy_power_on(ep->phy);
>>  		phy_init(ep->phy);
>>  	} else {
>> @@ -430,14 +526,16 @@ static int exynos_pcie_establish_link(struct
>> exynos_pcie *ep)
>>  		udelay(500);
>>  		exynos_pcie_writel(ep->mem_res->block_base, 0,
>>  					PCIE_PHY_COMMON_RESET);
>> +		exynos_pcie_deassert_core_reset(ep);
>>  	}
>>
>> -	/* pulse for common reset */
>> -	exynos_pcie_writel(ep->mem_res->block_base, 1,
>> PCIE_PHY_COMMON_RESET);
>> -	udelay(500);
>> -	exynos_pcie_writel(ep->mem_res->block_base, 0,
>> PCIE_PHY_COMMON_RESET);
> 
> These codes are also necessary for Exyno5440.
> How about moving these codes instead of removing them?
> 
> @@ -430,14 +526,16 @@ static int exynos_pcie_establish_link(struct
> exynos_pcie *ep)
>  		udelay(500);
>  		exynos_pcie_writel(ep->mem_res->block_base, 0,
>  					PCIE_PHY_COMMON_RESET);
> +		/* pulse for common reset */
> +		exynos_pcie_writel(ep->mem_res->block_base, 1,
> +					PCIE_PHY_COMMON_RESET);
> +		udelay(500);
> +		exynos_pcie_writel(ep->mem_res->block_base, 0,
> +					PCIE_PHY_COMMON_RESET);
> +		exynos_pcie_deassert_core_reset(ep);
>  	}
> 
> 
>> +	/*
>> +	 * Enable DBI_RO_WR_EN bit.
>> +	 * - When set to 1, some RO and HWinit bits are wriatble from
>> +	 *   the local application through the DBI.
>> +	 */
>> +	dw_pcie_writel_dbi(pci, PCIE_MISC_CONTROL_1_OFF, DBI_RO_WR_EN);
>>
>> -	exynos_pcie_deassert_core_reset(ep);
>>  	dw_pcie_setup_rc(pp);
>>  	exynos_pcie_assert_reset(ep);
>>
>> @@ -472,16 +570,6 @@ static void exynos_pcie_clear_irq_pulse(struct
>> exynos_pcie *ep)
>>  	exynos_pcie_writel(ep->mem_res->elbi_base, val, PCIE_IRQ_PULSE);
>>  }
>>
>> -static void exynos_pcie_enable_irq_pulse(struct exynos_pcie *ep)
>> -{
>> -	u32 val;
>> -
>> -	/* enable INTX interrupt */
>> -	val = IRQ_INTA_ASSERT | IRQ_INTB_ASSERT |
>> -		IRQ_INTC_ASSERT | IRQ_INTD_ASSERT;
>> -	exynos_pcie_writel(ep->mem_res->elbi_base, val, PCIE_IRQ_EN_PULSE);
>> -}
>> -
>>  static irqreturn_t exynos_pcie_irq_handler(int irq, void *arg)
>>  {
>>  	struct exynos_pcie *ep = arg;
>> @@ -513,9 +601,16 @@ static void exynos_pcie_msi_init(struct exynos_pcie
>> *ep)
>>  	exynos_pcie_writel(ep->mem_res->elbi_base, val, PCIE_IRQ_EN_LEVEL);
>>  }
>>
>> -static void exynos_pcie_enable_interrupts(struct exynos_pcie *ep)
>> +static void exynos_pcie_enable_irq_pulse(struct exynos_pcie *ep)
>>  {
>> -	exynos_pcie_enable_irq_pulse(ep);
>> +	u32 val;
>> +
>> +	val = IRQ_INTA_ASSERT | IRQ_INTB_ASSERT |
>> +		IRQ_INTC_ASSERT | IRQ_INTD_ASSERT;
>> +	exynos_pcie_writel(ep->mem_res->elbi_base, val, PCIE_IRQ_EN_PULSE);
>> +
>> +	exynos_pcie_writel(ep->mem_res->elbi_base, 0, PCIE_IRQ_EN_LEVEL);
>> +	exynos_pcie_writel(ep->mem_res->elbi_base, 0, PCIE_IRQ_EN_SPECIAL);
> 
> Good.
> 
>>
>>  	if (IS_ENABLED(CONFIG_PCI_MSI))
>>  		exynos_pcie_msi_init(ep);
>> @@ -575,10 +670,8 @@ static int exynos_pcie_link_up(struct dw_pcie *pci)
>>  	u32 val;
>>
>>  	val = exynos_pcie_readl(ep->mem_res->elbi_base,
>> PCIE_ELBI_RDLH_LINKUP);
>> -	if (val == PCIE_ELBI_LTSSM_ENABLE)
>> -		return 1;
> 
> Exynos5440 uses 'PCIE_ELBI_LTSSM_ENABLE'.
> Can you keep this code for Exyno5440?

It's possible to keep, but if it has to keep, then it needs to distinguish between exynos5440 and other exynos.
Although I already mentioned, i needs to get Exynos5440 TRM. :)

Best Regards,
Jaehoon Chung

> 
> This register can be added as below.
> 
> /* Exynos5440 PCIe ELBI registers */
> #define EXYNOS5440_PCIE_ELBI_RDLH_LINKUP	0x064
> #define EXYNOS5440_PCIE_ELBI_LTSSM_ENABLE	BIT(0)
> 
> Best regards,
> Jingoo Han
> 
>>
>> -	return 0;
>> +	return (val & PCIE_ELBI_XMLH_LINKUP);
>>  }
>>
>>  static int exynos_pcie_host_init(struct pcie_port *pp)
>> @@ -587,7 +680,7 @@ static int exynos_pcie_host_init(struct pcie_port *pp)
>>  	struct exynos_pcie *ep = to_exynos_pcie(pci);
>>
>>  	exynos_pcie_establish_link(ep);
>> -	exynos_pcie_enable_interrupts(ep);
>> +	exynos_pcie_enable_irq_pulse(ep);
>>
>>  	return 0;
>>  }
>> @@ -608,8 +701,11 @@ static int __init exynos_add_pcie_port(struct
>> exynos_pcie *ep,
>>
>>  	pp->irq = platform_get_irq(pdev, 1);
>>  	if (pp->irq < 0) {
>> -		dev_err(dev, "failed to get irq\n");
>> -		return pp->irq;
>> +		pp->irq = platform_get_irq_byname(pdev, "intr");
>> +		if (pp->irq < 0) {
>> +			dev_err(dev, "failed to get irq\n");
>> +			return pp->irq;
>> +		}
>>  	}
>>  	ret = devm_request_irq(dev, pp->irq, exynos_pcie_irq_handler,
>>  				IRQF_SHARED, "exynos-pcie", ep);
>> @@ -678,13 +774,23 @@ static int __init exynos_pcie_probe(struct
>> platform_device *pdev)
>>
>>  	ep->reset_gpio = of_get_named_gpio(np, "reset-gpio", 0);
>>
>> -	/* Assume that controller doesn't use the PHY framework */
>> -	ep->using_phy = false;
>> +	/*
>> +	 * In case of Exynos5440,
>> +	 * Assume that controller doesn't use the PHY frameork.
>> +	 * Other SoCs might be used the PHY framework.
>> +	 */
>> +
>> +	if (of_device_is_compatible(np, "samsung,exynos5440-pcie"))
>> +		ep->using_phy = false;
>>
>> -	ep->phy = devm_of_phy_get(dev, np, NULL);
>> +	ep->phy = devm_of_phy_get(dev, np, "pcie-phy");
>>  	if (IS_ERR(ep->phy)) {
>>  		if (PTR_ERR(ep->phy) == -EPROBE_DEFER)
>>  			return PTR_ERR(ep->phy);
>> +		if (!of_device_is_compatible(np, "samsung,exynos5440-pcie"))
>> {
>> +			dev_err(dev, "Can't find the pcie-phy\n");
>> +			return PTR_ERR(ep->phy);
>> +		}
>>  		dev_warn(dev, "Use the 'phy' property. Current DT of pci-
>> exynos was deprecated!!\n");
>>  	} else
>>  		ep->using_phy = true;
>> @@ -734,23 +840,20 @@ static int __exit exynos_pcie_remove(struct
>> platform_device *pdev)
>>  static const struct of_device_id exynos_pcie_of_match[] = {
>>  	{
>>  		.compatible = "samsung,exynos5440-pcie",
>> -		.data = &exynos5440_pcie_ops
>> +		.data = &exynos5440_pcie_ops,
>> +	}, {
>> +		.compatible = "samsung,exynos5433-pcie",
>> +		.data = &exynos5433_pcie_ops,
>>  	},
>>  	{},
>>  };
>>
>>  static struct platform_driver exynos_pcie_driver = {
>> +	.probe		= exynos_pcie_probe,
>>  	.remove		= __exit_p(exynos_pcie_remove),
>>  	.driver = {
>>  		.name	= "exynos-pcie",
>>  		.of_match_table = exynos_pcie_of_match,
>>  	},
>>  };
>> -
>> -/* Exynos PCIe driver does not allow module unload */
>> -
>> -static int __init exynos_pcie_init(void)
>> -{
>> -	return platform_driver_probe(&exynos_pcie_driver,
>> exynos_pcie_probe);
>> -}
>> -subsys_initcall(exynos_pcie_init);
>> +builtin_platform_driver(exynos_pcie_driver);
>> --
>> 2.15.1
> 
> 
> 
> 
> 

Re: [RFC 2/2] pci: dwc: pci-exynos: add the codes to support the exynos5433

[Jingoo Han]

On Thursday, December 21, 2017 9:21 PM, Jaehoon Chung wrote:
> On 12/22/2017 01:12 AM, Jingoo Han wrote:
> > On Thursday, December 21, 2017 7:14 AM, Jaehoon Chung wrote:
> >>
> >> Exynos5433 has the PCIe for WiFi.
> >> Added the codes relevant to PCIe for supporting the exynos5433.
> >> Also changed the binding documentation name to
> >> 'samsung,exynos-pcie.txt'.
> >> (It's not only exynos5440 anymore.)
> >>
> >
> > I have no objection.
> > However, I added some comments about Exynos5440.
> >
> >> Signed-off-by: Jaehoon Chung <jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> >> ---
> >>  ...exynos5440-pcie.txt => samsung,exynos-pcie.txt} |   2 +-
> >>  drivers/pci/dwc/pci-exynos.c                       | 183
> > ++++++++++++++++-----
> >>  2 files changed, 144 insertions(+), 41 deletions(-)
> >>  rename Documentation/devicetree/bindings/pci/{samsung,exynos5440-
> pcie.txt
> >> => samsung,exynos-pcie.txt} (97%)
> >>
> >> diff --git a/Documentation/devicetree/bindings/pci/samsung,exynos5440-
> >> pcie.txt b/Documentation/devicetree/bindings/pci/samsung,exynos-
> pcie.txt
> >> similarity index 97%
> >> rename from Documentation/devicetree/bindings/pci/samsung,exynos5440-
> >> pcie.txt
> >> rename to Documentation/devicetree/bindings/pci/samsung,exynos-pcie.txt
> >> index 34a11bfbfb60..958dcc150505 100644
> >> --- a/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt
> >> +++ b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.txt
> >> @@ -4,7 +4,7 @@ This PCIe host controller is based on the Synopsys
> >> DesignWare PCIe IP
> >>  and thus inherits all the common properties defined in designware-
> >> pcie.txt.
> >>
> >>  Required properties:
> >> -- compatible: "samsung,exynos5440-pcie"
> >> +- compatible: "samsung,exynos5440-pcie" or "samsung,exynos5433-pcie"
> >>  - reg: base addresses and lengths of the PCIe controller,
> >>  	the PHY controller, additional register for the PHY controller.
> >>  	(Registers for the PHY controller are DEPRECATED.
> >> diff --git a/drivers/pci/dwc/pci-exynos.c b/drivers/pci/dwc/pci-
> exynos.c
> >> index 5596fdedbb94..8dee2e90347e 100644
> >> --- a/drivers/pci/dwc/pci-exynos.c
> >> +++ b/drivers/pci/dwc/pci-exynos.c
> >> @@ -40,6 +40,8 @@
> >>  #define PCIE_IRQ_SPECIAL		0x008
> >>  #define PCIE_IRQ_EN_PULSE		0x00c
> >>  #define PCIE_IRQ_EN_LEVEL		0x010
> >> +#define PCIE_SW_WAKE			0x018
> >> +#define PCIE_BUS_EN			BIT(1)
> >>  #define IRQ_MSI_ENABLE			BIT(2)
> >>  #define PCIE_IRQ_EN_SPECIAL		0x014
> >>  #define PCIE_PWR_RESET			0x018
> >> @@ -49,7 +51,8 @@
> >>  #define PCIE_NONSTICKY_RESET		0x024
> >>  #define PCIE_APP_INIT_RESET		0x028
> >>  #define PCIE_APP_LTSSM_ENABLE		0x02c
> >> -#define PCIE_ELBI_RDLH_LINKUP		0x064
> >> +#define PCIE_ELBI_RDLH_LINKUP		0x074
> >
> > The address of this register should be 0x064 for exynos5440.
> > Howe about the following?
> >
> > +#define EXYNOS5440_PCIE_ELBI_RDLH_LINKUP	0x064
> > +#define PCIE_ELBI_RDLH_LINKUP		0x074
> >
> > Or you can add the following.
> >
> > /* Exynos5440 PCIe ELBI registers */
> > #define EXYNOS5440_PCIE_ELBI_RDLH_LINKUP	0x064
> 
> Maybe, you're right. Because i didn't have Exynos5440 TRM, it's problem to
> me about updating other SoCs.
> I have checked almost all variants Exynos. They are using the LINKUP
> register as 0x74.
> 
> If i can get the exynos5440 TRM, it's much helpful to me. Is it possible?

I don't have Exynos5440 TRM.
Please ask other guys.

Best regards,
Jingoo Han

> 
> >
> >> +#define PCIE_ELBI_XMLH_LINKUP		BIT(4)
> >>  #define PCIE_ELBI_LTSSM_ENABLE		0x1
> >>  #define PCIE_ELBI_SLV_AWMISC		0x11c
> >>  #define PCIE_ELBI_SLV_ARMISC		0x120
> >> @@ -94,6 +97,10 @@
> >>  #define PCIE_PHY_TRSV3_PD_TSV		BIT(7)
> >>  #define PCIE_PHY_TRSV3_LVCC		0x31c
> >>
> >> +/* DBI register */
> >> +#define PCIE_MISC_CONTROL_1_OFF		0x8BC
> >> +#define DBI_RO_WR_EN			BIT(0)
> >> +
> >>  struct exynos_pcie_mem_res {
> >>  	void __iomem *elbi_base;   /* DT 0th resource: PCIe CTRL */
> >>  	void __iomem *phy_base;    /* DT 1st resource: PHY CTRL */
> >> @@ -221,6 +228,96 @@ static const struct exynos_pcie_ops
> >> exynos5440_pcie_ops = {
> >>  	.deinit_clk_resources	= exynos5440_pcie_deinit_clk_resources,
> >>  };
> >>
> >> +static int exynos5433_pcie_get_mem_resources(struct platform_device
> > *pdev,
> >> +					     struct exynos_pcie *ep)
> >> +{
> >> +	struct dw_pcie *pci = ep->pci;
> >> +	struct device *dev = pci->dev;
> >> +	struct resource *res;
> >> +
> >> +	ep->mem_res = devm_kzalloc(dev, sizeof(*ep->mem_res), GFP_KERNEL);
> >> +	if (!ep->mem_res)
> >> +		return -ENOMEM;
> >> +
> >> +	/* External Local Bus interface(ELBI) Register */
> >> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "elbi");
> >> +	ep->mem_res->elbi_base = devm_ioremap_resource(&pdev->dev, res);
> >> +	if (IS_ERR(ep->mem_res->elbi_base))
> >> +		return PTR_ERR(ep->mem_res->elbi_base);
> >> +
> >> +	/* Data Bus Interface(DBI) Register */
> >> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
> >> +	pci->dbi_base = devm_ioremap_resource(&pdev->dev, res);
> >> +	if (IS_ERR(pci->dbi_base))
> >> +		return PTR_ERR(pci->dbi_base);
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static int exynos5433_pcie_get_clk_resources(struct exynos_pcie *ep)
> >> +{
> >> +	struct dw_pcie *pci = ep->pci;
> >> +	struct device *dev = pci->dev;
> >> +
> >> +	ep->clk_res = devm_kzalloc(dev, sizeof(*ep->clk_res), GFP_KERNEL);
> >> +	if (!ep->clk_res)
> >> +		return -ENOMEM;
> >> +
> >> +	ep->clk_res->clk = devm_clk_get(dev, "pcie");
> >> +	if (IS_ERR(ep->clk_res->clk)) {
> >> +		dev_err(dev, "Failed to get pcie rc clock\n");
> >> +		return PTR_ERR(ep->clk_res->clk);
> >> +	}
> >> +
> >> +	ep->clk_res->bus_clk = devm_clk_get(dev, "pcie_bus");
> >> +	if (IS_ERR(ep->clk_res->bus_clk)) {
> >> +		dev_err(dev, "Failed to get pcie bus clock\n");
> >> +		return PTR_ERR(ep->clk_res->bus_clk);
> >> +	}
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static void exynos5433_pcie_deinit_clk_resources(struct exynos_pcie
> *ep)
> >> +{
> >> +	clk_disable_unprepare(ep->clk_res->bus_clk);
> >> +	clk_disable_unprepare(ep->clk_res->clk);
> >> +}
> >> +
> >> +
> >> +static int exynos5433_pcie_init_clk_resources(struct exynos_pcie *ep)
> >> +{
> >> +	struct dw_pcie *pci = ep->pci;
> >> +	struct device *dev = pci->dev;
> >> +	int ret;
> >> +
> >> +	ret = clk_prepare_enable(ep->clk_res->clk);
> >> +	if (ret) {
> >> +		dev_err(dev, "cannot enable pcie rc clock");
> >> +		return ret;
> >> +	}
> >> +
> >> +	ret = clk_prepare_enable(ep->clk_res->bus_clk);
> >> +	if (ret) {
> >> +		dev_err(dev, "cannot enable pcie bus clock");
> >> +		goto err_bus_clk;
> >> +	}
> >> +
> >> +	return 0;
> >> +
> >> +err_bus_clk:
> >> +	clk_disable_unprepare(ep->clk_res->clk);
> >> +
> >> +	return ret;
> >> +}
> >> +
> >> +static const struct exynos_pcie_ops exynos5433_pcie_ops = {
> >> +	.get_mem_resources	= exynos5433_pcie_get_mem_resources,
> >> +	.get_clk_resources	= exynos5433_pcie_get_clk_resources,
> >> +	.init_clk_resources	= exynos5433_pcie_init_clk_resources,
> >> +	.deinit_clk_resources	= exynos5433_pcie_deinit_clk_resources,
> >> +};
> >> +
> >>  static void exynos_pcie_writel(void __iomem *base, u32 val, u32 reg)
> >>  {
> >>  	writel(val, base + reg);
> >> @@ -279,7 +376,9 @@ static void exynos_pcie_deassert_core_reset(struct
> >> exynos_pcie *ep)
> >>  	exynos_pcie_writel(ep->mem_res->elbi_base, 1, PCIE_NONSTICKY_RESET);
> >>  	exynos_pcie_writel(ep->mem_res->elbi_base, 1, PCIE_APP_INIT_RESET);
> >>  	exynos_pcie_writel(ep->mem_res->elbi_base, 0, PCIE_APP_INIT_RESET);
> >> -	exynos_pcie_writel(ep->mem_res->block_base, 1, PCIE_PHY_MAC_RESET);
> >> +	if (ep->mem_res->block_base)
> >> +		exynos_pcie_writel(ep->mem_res->block_base, 1,
> >> +				PCIE_PHY_MAC_RESET);
> >
> > Good.
> >
> >>  }
> >>
> >>  static void exynos_pcie_assert_phy_reset(struct exynos_pcie *ep)
> >> @@ -413,9 +512,6 @@ static int exynos_pcie_establish_link(struct
> >> exynos_pcie *ep)
> >>  	if (ep->using_phy) {
> >>  		phy_reset(ep->phy);
> >>
> >> -		exynos_pcie_writel(ep->mem_res->elbi_base, 1,
> >> -				PCIE_PWR_RESET);
> >> -
> >>  		phy_power_on(ep->phy);
> >>  		phy_init(ep->phy);
> >>  	} else {
> >> @@ -430,14 +526,16 @@ static int exynos_pcie_establish_link(struct
> >> exynos_pcie *ep)
> >>  		udelay(500);
> >>  		exynos_pcie_writel(ep->mem_res->block_base, 0,
> >>  					PCIE_PHY_COMMON_RESET);
> >> +		exynos_pcie_deassert_core_reset(ep);
> >>  	}
> >>
> >> -	/* pulse for common reset */
> >> -	exynos_pcie_writel(ep->mem_res->block_base, 1,
> >> PCIE_PHY_COMMON_RESET);
> >> -	udelay(500);
> >> -	exynos_pcie_writel(ep->mem_res->block_base, 0,
> >> PCIE_PHY_COMMON_RESET);
> >
> > These codes are also necessary for Exyno5440.
> > How about moving these codes instead of removing them?
> >
> > @@ -430,14 +526,16 @@ static int exynos_pcie_establish_link(struct
> > exynos_pcie *ep)
> >  		udelay(500);
> >  		exynos_pcie_writel(ep->mem_res->block_base, 0,
> >  					PCIE_PHY_COMMON_RESET);
> > +		/* pulse for common reset */
> > +		exynos_pcie_writel(ep->mem_res->block_base, 1,
> > +					PCIE_PHY_COMMON_RESET);
> > +		udelay(500);
> > +		exynos_pcie_writel(ep->mem_res->block_base, 0,
> > +					PCIE_PHY_COMMON_RESET);
> > +		exynos_pcie_deassert_core_reset(ep);
> >  	}
> >
> >
> >> +	/*
> >> +	 * Enable DBI_RO_WR_EN bit.
> >> +	 * - When set to 1, some RO and HWinit bits are wriatble from
> >> +	 *   the local application through the DBI.
> >> +	 */
> >> +	dw_pcie_writel_dbi(pci, PCIE_MISC_CONTROL_1_OFF, DBI_RO_WR_EN);
> >>
> >> -	exynos_pcie_deassert_core_reset(ep);
> >>  	dw_pcie_setup_rc(pp);
> >>  	exynos_pcie_assert_reset(ep);
> >>
> >> @@ -472,16 +570,6 @@ static void exynos_pcie_clear_irq_pulse(struct
> >> exynos_pcie *ep)
> >>  	exynos_pcie_writel(ep->mem_res->elbi_base, val, PCIE_IRQ_PULSE);
> >>  }
> >>
> >> -static void exynos_pcie_enable_irq_pulse(struct exynos_pcie *ep)
> >> -{
> >> -	u32 val;
> >> -
> >> -	/* enable INTX interrupt */
> >> -	val = IRQ_INTA_ASSERT | IRQ_INTB_ASSERT |
> >> -		IRQ_INTC_ASSERT | IRQ_INTD_ASSERT;
> >> -	exynos_pcie_writel(ep->mem_res->elbi_base, val, PCIE_IRQ_EN_PULSE);
> >> -}
> >> -
> >>  static irqreturn_t exynos_pcie_irq_handler(int irq, void *arg)
> >>  {
> >>  	struct exynos_pcie *ep = arg;
> >> @@ -513,9 +601,16 @@ static void exynos_pcie_msi_init(struct
> exynos_pcie
> >> *ep)
> >>  	exynos_pcie_writel(ep->mem_res->elbi_base, val, PCIE_IRQ_EN_LEVEL);
> >>  }
> >>
> >> -static void exynos_pcie_enable_interrupts(struct exynos_pcie *ep)
> >> +static void exynos_pcie_enable_irq_pulse(struct exynos_pcie *ep)
> >>  {
> >> -	exynos_pcie_enable_irq_pulse(ep);
> >> +	u32 val;
> >> +
> >> +	val = IRQ_INTA_ASSERT | IRQ_INTB_ASSERT |
> >> +		IRQ_INTC_ASSERT | IRQ_INTD_ASSERT;
> >> +	exynos_pcie_writel(ep->mem_res->elbi_base, val, PCIE_IRQ_EN_PULSE);
> >> +
> >> +	exynos_pcie_writel(ep->mem_res->elbi_base, 0, PCIE_IRQ_EN_LEVEL);
> >> +	exynos_pcie_writel(ep->mem_res->elbi_base, 0, PCIE_IRQ_EN_SPECIAL);
> >
> > Good.
> >
> >>
> >>  	if (IS_ENABLED(CONFIG_PCI_MSI))
> >>  		exynos_pcie_msi_init(ep);
> >> @@ -575,10 +670,8 @@ static int exynos_pcie_link_up(struct dw_pcie *pci)
> >>  	u32 val;
> >>
> >>  	val = exynos_pcie_readl(ep->mem_res->elbi_base,
> >> PCIE_ELBI_RDLH_LINKUP);
> >> -	if (val == PCIE_ELBI_LTSSM_ENABLE)
> >> -		return 1;
> >
> > Exynos5440 uses 'PCIE_ELBI_LTSSM_ENABLE'.
> > Can you keep this code for Exyno5440?
> 
> It's possible to keep, but if it has to keep, then it needs to distinguish
> between exynos5440 and other exynos.
> Although I already mentioned, i needs to get Exynos5440 TRM. :)
> 
> Best Regards,
> Jaehoon Chung
> 
> >
> > This register can be added as below.
> >
> > /* Exynos5440 PCIe ELBI registers */
> > #define EXYNOS5440_PCIE_ELBI_RDLH_LINKUP	0x064
> > #define EXYNOS5440_PCIE_ELBI_LTSSM_ENABLE	BIT(0)
> >
> > Best regards,
> > Jingoo Han
> >
> >>
> >> -	return 0;
> >> +	return (val & PCIE_ELBI_XMLH_LINKUP);
> >>  }
> >>
> >>  static int exynos_pcie_host_init(struct pcie_port *pp)
> >> @@ -587,7 +680,7 @@ static int exynos_pcie_host_init(struct pcie_port
> *pp)
> >>  	struct exynos_pcie *ep = to_exynos_pcie(pci);
> >>
> >>  	exynos_pcie_establish_link(ep);
> >> -	exynos_pcie_enable_interrupts(ep);
> >> +	exynos_pcie_enable_irq_pulse(ep);
> >>
> >>  	return 0;
> >>  }
> >> @@ -608,8 +701,11 @@ static int __init exynos_add_pcie_port(struct
> >> exynos_pcie *ep,
> >>
> >>  	pp->irq = platform_get_irq(pdev, 1);
> >>  	if (pp->irq < 0) {
> >> -		dev_err(dev, "failed to get irq\n");
> >> -		return pp->irq;
> >> +		pp->irq = platform_get_irq_byname(pdev, "intr");
> >> +		if (pp->irq < 0) {
> >> +			dev_err(dev, "failed to get irq\n");
> >> +			return pp->irq;
> >> +		}
> >>  	}
> >>  	ret = devm_request_irq(dev, pp->irq, exynos_pcie_irq_handler,
> >>  				IRQF_SHARED, "exynos-pcie", ep);
> >> @@ -678,13 +774,23 @@ static int __init exynos_pcie_probe(struct
> >> platform_device *pdev)
> >>
> >>  	ep->reset_gpio = of_get_named_gpio(np, "reset-gpio", 0);
> >>
> >> -	/* Assume that controller doesn't use the PHY framework */
> >> -	ep->using_phy = false;
> >> +	/*
> >> +	 * In case of Exynos5440,
> >> +	 * Assume that controller doesn't use the PHY frameork.
> >> +	 * Other SoCs might be used the PHY framework.
> >> +	 */
> >> +
> >> +	if (of_device_is_compatible(np, "samsung,exynos5440-pcie"))
> >> +		ep->using_phy = false;
> >>
> >> -	ep->phy = devm_of_phy_get(dev, np, NULL);
> >> +	ep->phy = devm_of_phy_get(dev, np, "pcie-phy");
> >>  	if (IS_ERR(ep->phy)) {
> >>  		if (PTR_ERR(ep->phy) == -EPROBE_DEFER)
> >>  			return PTR_ERR(ep->phy);
> >> +		if (!of_device_is_compatible(np, "samsung,exynos5440-pcie"))
> >> {
> >> +			dev_err(dev, "Can't find the pcie-phy\n");
> >> +			return PTR_ERR(ep->phy);
> >> +		}
> >>  		dev_warn(dev, "Use the 'phy' property. Current DT of pci-
> >> exynos was deprecated!!\n");
> >>  	} else
> >>  		ep->using_phy = true;
> >> @@ -734,23 +840,20 @@ static int __exit exynos_pcie_remove(struct
> >> platform_device *pdev)
> >>  static const struct of_device_id exynos_pcie_of_match[] = {
> >>  	{
> >>  		.compatible = "samsung,exynos5440-pcie",
> >> -		.data = &exynos5440_pcie_ops
> >> +		.data = &exynos5440_pcie_ops,
> >> +	}, {
> >> +		.compatible = "samsung,exynos5433-pcie",
> >> +		.data = &exynos5433_pcie_ops,
> >>  	},
> >>  	{},
> >>  };
> >>
> >>  static struct platform_driver exynos_pcie_driver = {
> >> +	.probe		= exynos_pcie_probe,
> >>  	.remove		= __exit_p(exynos_pcie_remove),
> >>  	.driver = {
> >>  		.name	= "exynos-pcie",
> >>  		.of_match_table = exynos_pcie_of_match,
> >>  	},
> >>  };
> >> -
> >> -/* Exynos PCIe driver does not allow module unload */
> >> -
> >> -static int __init exynos_pcie_init(void)
> >> -{
> >> -	return platform_driver_probe(&exynos_pcie_driver,
> >> exynos_pcie_probe);
> >> -}
> >> -subsys_initcall(exynos_pcie_init);
> >> +builtin_platform_driver(exynos_pcie_driver);
> >> --
> >> 2.15.1
> >
> >
> >
> >
> >


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 2/2] pci: dwc: pci-exynos: add the codes to support the exynos5433

[Rob Herring]

On Thu, Dec 21, 2017 at 09:14:07PM +0900, Jaehoon Chung wrote:
> Exynos5433 has the PCIe for WiFi.
> Added the codes relevant to PCIe for supporting the exynos5433.
> Also changed the binding documentation name to
> 'samsung,exynos-pcie.txt'.
> (It's not only exynos5440 anymore.)
> 
> Signed-off-by: Jaehoon Chung <jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>  ...exynos5440-pcie.txt => samsung,exynos-pcie.txt} |   2 +-
>  drivers/pci/dwc/pci-exynos.c                       | 183 ++++++++++++++++-----
>  2 files changed, 144 insertions(+), 41 deletions(-)
>  rename Documentation/devicetree/bindings/pci/{samsung,exynos5440-pcie.txt => samsung,exynos-pcie.txt} (97%)
> 
> diff --git a/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.txt
> similarity index 97%
> rename from Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt
> rename to Documentation/devicetree/bindings/pci/samsung,exynos-pcie.txt
> index 34a11bfbfb60..958dcc150505 100644
> --- a/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.txt
> @@ -4,7 +4,7 @@ This PCIe host controller is based on the Synopsys DesignWare PCIe IP
>  and thus inherits all the common properties defined in designware-pcie.txt.
>  
>  Required properties:
> -- compatible: "samsung,exynos5440-pcie"
> +- compatible: "samsung,exynos5440-pcie" or "samsung,exynos5433-pcie"

Quite a lot of driver changes for just a new compatible.

>  - reg: base addresses and lengths of the PCIe controller,

For example, you're adding the DBI registers which is not documented 
here. 

Perhaps it is time to remove the old phy support before adding a new 
platform.

>  	the PHY controller, additional register for the PHY controller.
>  	(Registers for the PHY controller are DEPRECATED.
> diff --git a/drivers/pci/dwc/pci-exynos.c b/drivers/pci/dwc/pci-exynos.c
> index 5596fdedbb94..8dee2e90347e 100644
> --- a/drivers/pci/dwc/pci-exynos.c
> +++ b/drivers/pci/dwc/pci-exynos.c
> @@ -40,6 +40,8 @@
>  #define PCIE_IRQ_SPECIAL		0x008
>  #define PCIE_IRQ_EN_PULSE		0x00c
>  #define PCIE_IRQ_EN_LEVEL		0x010
> +#define PCIE_SW_WAKE			0x018
> +#define PCIE_BUS_EN			BIT(1)
>  #define IRQ_MSI_ENABLE			BIT(2)
>  #define PCIE_IRQ_EN_SPECIAL		0x014
>  #define PCIE_PWR_RESET			0x018
> @@ -49,7 +51,8 @@
>  #define PCIE_NONSTICKY_RESET		0x024
>  #define PCIE_APP_INIT_RESET		0x028
>  #define PCIE_APP_LTSSM_ENABLE		0x02c
> -#define PCIE_ELBI_RDLH_LINKUP		0x064
> +#define PCIE_ELBI_RDLH_LINKUP		0x074
> +#define PCIE_ELBI_XMLH_LINKUP		BIT(4)
>  #define PCIE_ELBI_LTSSM_ENABLE		0x1
>  #define PCIE_ELBI_SLV_AWMISC		0x11c
>  #define PCIE_ELBI_SLV_ARMISC		0x120
> @@ -94,6 +97,10 @@
>  #define PCIE_PHY_TRSV3_PD_TSV		BIT(7)
>  #define PCIE_PHY_TRSV3_LVCC		0x31c
>  
> +/* DBI register */
> +#define PCIE_MISC_CONTROL_1_OFF		0x8BC
> +#define DBI_RO_WR_EN			BIT(0)
> +
>  struct exynos_pcie_mem_res {
>  	void __iomem *elbi_base;   /* DT 0th resource: PCIe CTRL */
>  	void __iomem *phy_base;    /* DT 1st resource: PHY CTRL */
> @@ -221,6 +228,96 @@ static const struct exynos_pcie_ops exynos5440_pcie_ops = {
>  	.deinit_clk_resources	= exynos5440_pcie_deinit_clk_resources,
>  };
>  
> +static int exynos5433_pcie_get_mem_resources(struct platform_device *pdev,
> +					     struct exynos_pcie *ep)
> +{
> +	struct dw_pcie *pci = ep->pci;
> +	struct device *dev = pci->dev;
> +	struct resource *res;
> +
> +	ep->mem_res = devm_kzalloc(dev, sizeof(*ep->mem_res), GFP_KERNEL);
> +	if (!ep->mem_res)
> +		return -ENOMEM;
> +
> +	/* External Local Bus interface(ELBI) Register */

These are standard DW registers IIRC. So the DW core should handle this.

> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "elbi");
> +	ep->mem_res->elbi_base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(ep->mem_res->elbi_base))
> +		return PTR_ERR(ep->mem_res->elbi_base);
> +
> +	/* Data Bus Interface(DBI) Register */
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
> +	pci->dbi_base = devm_ioremap_resource(&pdev->dev, res);

This is handled by the DW plat driver. Perhaps the DW core should handle 
it too. 

Does the 5440 really not have DBI registers or you just happen to not 
need to access them?

> +	if (IS_ERR(pci->dbi_base))
> +		return PTR_ERR(pci->dbi_base);
> +
> +	return 0;
> +}
> +
> +static int exynos5433_pcie_get_clk_resources(struct exynos_pcie *ep)
> +{
> +	struct dw_pcie *pci = ep->pci;
> +	struct device *dev = pci->dev;
> +
> +	ep->clk_res = devm_kzalloc(dev, sizeof(*ep->clk_res), GFP_KERNEL);
> +	if (!ep->clk_res)
> +		return -ENOMEM;
> +
> +	ep->clk_res->clk = devm_clk_get(dev, "pcie");
> +	if (IS_ERR(ep->clk_res->clk)) {
> +		dev_err(dev, "Failed to get pcie rc clock\n");
> +		return PTR_ERR(ep->clk_res->clk);
> +	}
> +
> +	ep->clk_res->bus_clk = devm_clk_get(dev, "pcie_bus");
> +	if (IS_ERR(ep->clk_res->bus_clk)) {
> +		dev_err(dev, "Failed to get pcie bus clock\n");
> +		return PTR_ERR(ep->clk_res->bus_clk);
> +	}
> +
> +	return 0;
> +}

Can't you reuse exynos5440_pcie_get_clk_resources? They appear to be the 
same.

> +
> +static void exynos5433_pcie_deinit_clk_resources(struct exynos_pcie *ep)
> +{
> +	clk_disable_unprepare(ep->clk_res->bus_clk);
> +	clk_disable_unprepare(ep->clk_res->clk);
> +}
> +
> +
> +static int exynos5433_pcie_init_clk_resources(struct exynos_pcie *ep)
> +{
> +	struct dw_pcie *pci = ep->pci;
> +	struct device *dev = pci->dev;
> +	int ret;
> +
> +	ret = clk_prepare_enable(ep->clk_res->clk);
> +	if (ret) {
> +		dev_err(dev, "cannot enable pcie rc clock");
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(ep->clk_res->bus_clk);
> +	if (ret) {
> +		dev_err(dev, "cannot enable pcie bus clock");
> +		goto err_bus_clk;
> +	}
> +
> +	return 0;
> +
> +err_bus_clk:
> +	clk_disable_unprepare(ep->clk_res->clk);
> +
> +	return ret;
> +}

Ditto.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 2/2] pci: dwc: pci-exynos: add the codes to support the exynos5433

[Jaehoon Chung]

On 12/27/2017 06:11 AM, Rob Herring wrote:
> On Thu, Dec 21, 2017 at 09:14:07PM +0900, Jaehoon Chung wrote:
>> Exynos5433 has the PCIe for WiFi.
>> Added the codes relevant to PCIe for supporting the exynos5433.
>> Also changed the binding documentation name to
>> 'samsung,exynos-pcie.txt'.
>> (It's not only exynos5440 anymore.)
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> ---
>>  ...exynos5440-pcie.txt => samsung,exynos-pcie.txt} |   2 +-
>>  drivers/pci/dwc/pci-exynos.c                       | 183 ++++++++++++++++-----
>>  2 files changed, 144 insertions(+), 41 deletions(-)
>>  rename Documentation/devicetree/bindings/pci/{samsung,exynos5440-pcie.txt => samsung,exynos-pcie.txt} (97%)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.txt
>> similarity index 97%
>> rename from Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt
>> rename to Documentation/devicetree/bindings/pci/samsung,exynos-pcie.txt
>> index 34a11bfbfb60..958dcc150505 100644
>> --- a/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt
>> +++ b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.txt
>> @@ -4,7 +4,7 @@ This PCIe host controller is based on the Synopsys DesignWare PCIe IP
>>  and thus inherits all the common properties defined in designware-pcie.txt.
>>  
>>  Required properties:
>> -- compatible: "samsung,exynos5440-pcie"
>> +- compatible: "samsung,exynos5440-pcie" or "samsung,exynos5433-pcie"
> 
> Quite a lot of driver changes for just a new compatible.

It needs to distinguish between exynos5440 and other exynos variants.
So needs to add the new common compatible likes "samsung,exynos5-pcie" or "samsung,exynos-pcie".

Actually, i hope that exynos5440 can be removed from mainline kernel.

> 
>>  - reg: base addresses and lengths of the PCIe controller,
> 
> For example, you're adding the DBI registers which is not documented 
> here. 
> 
> Perhaps it is time to remove the old phy support before adding a new 
> platform.

Ok, I will send the patches to remove the old phy.

> 
>>  	the PHY controller, additional register for the PHY controller.
>>  	(Registers for the PHY controller are DEPRECATED.
>> diff --git a/drivers/pci/dwc/pci-exynos.c b/drivers/pci/dwc/pci-exynos.c
>> index 5596fdedbb94..8dee2e90347e 100644
>> --- a/drivers/pci/dwc/pci-exynos.c
>> +++ b/drivers/pci/dwc/pci-exynos.c
>> @@ -40,6 +40,8 @@
>>  #define PCIE_IRQ_SPECIAL		0x008
>>  #define PCIE_IRQ_EN_PULSE		0x00c
>>  #define PCIE_IRQ_EN_LEVEL		0x010
>> +#define PCIE_SW_WAKE			0x018
>> +#define PCIE_BUS_EN			BIT(1)
>>  #define IRQ_MSI_ENABLE			BIT(2)
>>  #define PCIE_IRQ_EN_SPECIAL		0x014
>>  #define PCIE_PWR_RESET			0x018
>> @@ -49,7 +51,8 @@
>>  #define PCIE_NONSTICKY_RESET		0x024
>>  #define PCIE_APP_INIT_RESET		0x028
>>  #define PCIE_APP_LTSSM_ENABLE		0x02c
>> -#define PCIE_ELBI_RDLH_LINKUP		0x064
>> +#define PCIE_ELBI_RDLH_LINKUP		0x074
>> +#define PCIE_ELBI_XMLH_LINKUP		BIT(4)
>>  #define PCIE_ELBI_LTSSM_ENABLE		0x1
>>  #define PCIE_ELBI_SLV_AWMISC		0x11c
>>  #define PCIE_ELBI_SLV_ARMISC		0x120
>> @@ -94,6 +97,10 @@
>>  #define PCIE_PHY_TRSV3_PD_TSV		BIT(7)
>>  #define PCIE_PHY_TRSV3_LVCC		0x31c
>>  
>> +/* DBI register */
>> +#define PCIE_MISC_CONTROL_1_OFF		0x8BC
>> +#define DBI_RO_WR_EN			BIT(0)
>> +
>>  struct exynos_pcie_mem_res {
>>  	void __iomem *elbi_base;   /* DT 0th resource: PCIe CTRL */
>>  	void __iomem *phy_base;    /* DT 1st resource: PHY CTRL */
>> @@ -221,6 +228,96 @@ static const struct exynos_pcie_ops exynos5440_pcie_ops = {
>>  	.deinit_clk_resources	= exynos5440_pcie_deinit_clk_resources,
>>  };
>>  
>> +static int exynos5433_pcie_get_mem_resources(struct platform_device *pdev,
>> +					     struct exynos_pcie *ep)
>> +{
>> +	struct dw_pcie *pci = ep->pci;
>> +	struct device *dev = pci->dev;
>> +	struct resource *res;
>> +
>> +	ep->mem_res = devm_kzalloc(dev, sizeof(*ep->mem_res), GFP_KERNEL);
>> +	if (!ep->mem_res)
>> +		return -ENOMEM;
>> +
>> +	/* External Local Bus interface(ELBI) Register */
> 
> These are standard DW registers IIRC. So the DW core should handle this.
> 
>> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "elbi");
>> +	ep->mem_res->elbi_base = devm_ioremap_resource(&pdev->dev, res);
>> +	if (IS_ERR(ep->mem_res->elbi_base))
>> +		return PTR_ERR(ep->mem_res->elbi_base);
>> +
>> +	/* Data Bus Interface(DBI) Register */
>> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
>> +	pci->dbi_base = devm_ioremap_resource(&pdev->dev, res);
> 
> This is handled by the DW plat driver. Perhaps the DW core should handle 
> it too. 

Will fix.

> 
> Does the 5440 really not have DBI registers or you just happen to not 
> need to access them?

I didn't have exynso5440 TRM, do you means exynos5433? It doesn't have its own DBI register.

> 
>> +	if (IS_ERR(pci->dbi_base))
>> +		return PTR_ERR(pci->dbi_base);
>> +
>> +	return 0;
>> +}
>> +
>> +static int exynos5433_pcie_get_clk_resources(struct exynos_pcie *ep)
>> +{
>> +	struct dw_pcie *pci = ep->pci;
>> +	struct device *dev = pci->dev;
>> +
>> +	ep->clk_res = devm_kzalloc(dev, sizeof(*ep->clk_res), GFP_KERNEL);
>> +	if (!ep->clk_res)
>> +		return -ENOMEM;
>> +
>> +	ep->clk_res->clk = devm_clk_get(dev, "pcie");
>> +	if (IS_ERR(ep->clk_res->clk)) {
>> +		dev_err(dev, "Failed to get pcie rc clock\n");
>> +		return PTR_ERR(ep->clk_res->clk);
>> +	}
>> +
>> +	ep->clk_res->bus_clk = devm_clk_get(dev, "pcie_bus");
>> +	if (IS_ERR(ep->clk_res->bus_clk)) {
>> +		dev_err(dev, "Failed to get pcie bus clock\n");
>> +		return PTR_ERR(ep->clk_res->bus_clk);
>> +	}
>> +
>> +	return 0;
>> +}
> 
> Can't you reuse exynos5440_pcie_get_clk_resources? They appear to be the 
> same.

Will reuse.

> 
>> +
>> +static void exynos5433_pcie_deinit_clk_resources(struct exynos_pcie *ep)
>> +{
>> +	clk_disable_unprepare(ep->clk_res->bus_clk);
>> +	clk_disable_unprepare(ep->clk_res->clk);
>> +}
>> +
>> +
>> +static int exynos5433_pcie_init_clk_resources(struct exynos_pcie *ep)
>> +{
>> +	struct dw_pcie *pci = ep->pci;
>> +	struct device *dev = pci->dev;
>> +	int ret;
>> +
>> +	ret = clk_prepare_enable(ep->clk_res->clk);
>> +	if (ret) {
>> +		dev_err(dev, "cannot enable pcie rc clock");
>> +		return ret;
>> +	}
>> +
>> +	ret = clk_prepare_enable(ep->clk_res->bus_clk);
>> +	if (ret) {
>> +		dev_err(dev, "cannot enable pcie bus clock");
>> +		goto err_bus_clk;
>> +	}
>> +
>> +	return 0;
>> +
>> +err_bus_clk:
>> +	clk_disable_unprepare(ep->clk_res->clk);
>> +
>> +	return ret;
>> +}
> 
> Ditto.

Ok. Will fix.

Best Regards,
Jaehoon Chung

> 
> Rob
> 
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC 2/2] pci: dwc: pci-exynos: add the coedes to support the exynos5433

[Jaehoon Chung]

Exynos5433 has the PCIe for WiFi.
Add the codes relevant to PCIe for supporting the exynos5433.
Also changed the binding documentation name to
'samsung,exynos-pcie.txt'.
(It's not only exynos5440 anymore.)

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
 ...exynos5440-pcie.txt => samsung,exynos-pcie.txt} |   2 +-
 drivers/pci/dwc/pci-exynos.c                       | 183 ++++++++++++++++-----
 2 files changed, 144 insertions(+), 41 deletions(-)
 rename Documentation/devicetree/bindings/pci/{samsung,exynos5440-pcie.txt => samsung,exynos-pcie.txt} (97%)

diff --git a/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.txt
similarity index 97%
rename from Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt
rename to Documentation/devicetree/bindings/pci/samsung,exynos-pcie.txt
index 34a11bfbfb60..958dcc150505 100644
--- a/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.txt
@@ -4,7 +4,7 @@ This PCIe host controller is based on the Synopsys DesignWare PCIe IP
 and thus inherits all the common properties defined in designware-pcie.txt.
 
 Required properties:
-- compatible: "samsung,exynos5440-pcie"
+- compatible: "samsung,exynos5440-pcie" or "samsung,exynos5433-pcie"
 - reg: base addresses and lengths of the PCIe controller,
 	the PHY controller, additional register for the PHY controller.
 	(Registers for the PHY controller are DEPRECATED.
diff --git a/drivers/pci/dwc/pci-exynos.c b/drivers/pci/dwc/pci-exynos.c
index 5596fdedbb94..8dee2e90347e 100644
--- a/drivers/pci/dwc/pci-exynos.c
+++ b/drivers/pci/dwc/pci-exynos.c
@@ -40,6 +40,8 @@
 #define PCIE_IRQ_SPECIAL		0x008
 #define PCIE_IRQ_EN_PULSE		0x00c
 #define PCIE_IRQ_EN_LEVEL		0x010
+#define PCIE_SW_WAKE			0x018
+#define PCIE_BUS_EN			BIT(1)
 #define IRQ_MSI_ENABLE			BIT(2)
 #define PCIE_IRQ_EN_SPECIAL		0x014
 #define PCIE_PWR_RESET			0x018
@@ -49,7 +51,8 @@
 #define PCIE_NONSTICKY_RESET		0x024
 #define PCIE_APP_INIT_RESET		0x028
 #define PCIE_APP_LTSSM_ENABLE		0x02c
-#define PCIE_ELBI_RDLH_LINKUP		0x064
+#define PCIE_ELBI_RDLH_LINKUP		0x074
+#define PCIE_ELBI_XMLH_LINKUP		BIT(4)
 #define PCIE_ELBI_LTSSM_ENABLE		0x1
 #define PCIE_ELBI_SLV_AWMISC		0x11c
 #define PCIE_ELBI_SLV_ARMISC		0x120
@@ -94,6 +97,10 @@
 #define PCIE_PHY_TRSV3_PD_TSV		BIT(7)
 #define PCIE_PHY_TRSV3_LVCC		0x31c
 
+/* DBI register */
+#define PCIE_MISC_CONTROL_1_OFF		0x8BC
+#define DBI_RO_WR_EN			BIT(0)
+
 struct exynos_pcie_mem_res {
 	void __iomem *elbi_base;   /* DT 0th resource: PCIe CTRL */
 	void __iomem *phy_base;    /* DT 1st resource: PHY CTRL */
@@ -221,6 +228,96 @@ static const struct exynos_pcie_ops exynos5440_pcie_ops = {
 	.deinit_clk_resources	= exynos5440_pcie_deinit_clk_resources,
 };
 
+static int exynos5433_pcie_get_mem_resources(struct platform_device *pdev,
+					     struct exynos_pcie *ep)
+{
+	struct dw_pcie *pci = ep->pci;
+	struct device *dev = pci->dev;
+	struct resource *res;
+
+	ep->mem_res = devm_kzalloc(dev, sizeof(*ep->mem_res), GFP_KERNEL);
+	if (!ep->mem_res)
+		return -ENOMEM;
+
+	/* External Local Bus interface(ELBI) Register */
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "elbi");
+	ep->mem_res->elbi_base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(ep->mem_res->elbi_base))
+		return PTR_ERR(ep->mem_res->elbi_base);
+
+	/* Data Bus Interface(DBI) Register */
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
+	pci->dbi_base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(pci->dbi_base))
+		return PTR_ERR(pci->dbi_base);
+
+	return 0;
+}
+
+static int exynos5433_pcie_get_clk_resources(struct exynos_pcie *ep)
+{
+	struct dw_pcie *pci = ep->pci;
+	struct device *dev = pci->dev;
+
+	ep->clk_res = devm_kzalloc(dev, sizeof(*ep->clk_res), GFP_KERNEL);
+	if (!ep->clk_res)
+		return -ENOMEM;
+
+	ep->clk_res->clk = devm_clk_get(dev, "pcie");
+	if (IS_ERR(ep->clk_res->clk)) {
+		dev_err(dev, "Failed to get pcie rc clock\n");
+		return PTR_ERR(ep->clk_res->clk);
+	}
+
+	ep->clk_res->bus_clk = devm_clk_get(dev, "pcie_bus");
+	if (IS_ERR(ep->clk_res->bus_clk)) {
+		dev_err(dev, "Failed to get pcie bus clock\n");
+		return PTR_ERR(ep->clk_res->bus_clk);
+	}
+
+	return 0;
+}
+
+static void exynos5433_pcie_deinit_clk_resources(struct exynos_pcie *ep)
+{
+	clk_disable_unprepare(ep->clk_res->bus_clk);
+	clk_disable_unprepare(ep->clk_res->clk);
+}
+
+
+static int exynos5433_pcie_init_clk_resources(struct exynos_pcie *ep)
+{
+	struct dw_pcie *pci = ep->pci;
+	struct device *dev = pci->dev;
+	int ret;
+
+	ret = clk_prepare_enable(ep->clk_res->clk);
+	if (ret) {
+		dev_err(dev, "cannot enable pcie rc clock");
+		return ret;
+	}
+
+	ret = clk_prepare_enable(ep->clk_res->bus_clk);
+	if (ret) {
+		dev_err(dev, "cannot enable pcie bus clock");
+		goto err_bus_clk;
+	}
+
+	return 0;
+
+err_bus_clk:
+	clk_disable_unprepare(ep->clk_res->clk);
+
+	return ret;
+}
+
+static const struct exynos_pcie_ops exynos5433_pcie_ops = {
+	.get_mem_resources	= exynos5433_pcie_get_mem_resources,
+	.get_clk_resources	= exynos5433_pcie_get_clk_resources,
+	.init_clk_resources	= exynos5433_pcie_init_clk_resources,
+	.deinit_clk_resources	= exynos5433_pcie_deinit_clk_resources,
+};
+
 static void exynos_pcie_writel(void __iomem *base, u32 val, u32 reg)
 {
 	writel(val, base + reg);
@@ -279,7 +376,9 @@ static void exynos_pcie_deassert_core_reset(struct exynos_pcie *ep)
 	exynos_pcie_writel(ep->mem_res->elbi_base, 1, PCIE_NONSTICKY_RESET);
 	exynos_pcie_writel(ep->mem_res->elbi_base, 1, PCIE_APP_INIT_RESET);
 	exynos_pcie_writel(ep->mem_res->elbi_base, 0, PCIE_APP_INIT_RESET);
-	exynos_pcie_writel(ep->mem_res->block_base, 1, PCIE_PHY_MAC_RESET);
+	if (ep->mem_res->block_base)
+		exynos_pcie_writel(ep->mem_res->block_base, 1,
+				PCIE_PHY_MAC_RESET);
 }
 
 static void exynos_pcie_assert_phy_reset(struct exynos_pcie *ep)
@@ -413,9 +512,6 @@ static int exynos_pcie_establish_link(struct exynos_pcie *ep)
 	if (ep->using_phy) {
 		phy_reset(ep->phy);
 
-		exynos_pcie_writel(ep->mem_res->elbi_base, 1,
-				PCIE_PWR_RESET);
-
 		phy_power_on(ep->phy);
 		phy_init(ep->phy);
 	} else {
@@ -430,14 +526,16 @@ static int exynos_pcie_establish_link(struct exynos_pcie *ep)
 		udelay(500);
 		exynos_pcie_writel(ep->mem_res->block_base, 0,
 					PCIE_PHY_COMMON_RESET);
+		exynos_pcie_deassert_core_reset(ep);
 	}
 
-	/* pulse for common reset */
-	exynos_pcie_writel(ep->mem_res->block_base, 1, PCIE_PHY_COMMON_RESET);
-	udelay(500);
-	exynos_pcie_writel(ep->mem_res->block_base, 0, PCIE_PHY_COMMON_RESET);
+	/*
+	 * Enable DBI_RO_WR_EN bit.
+	 * - When set to 1, some RO and HWinit bits are wriatble from
+	 *   the local application through the DBI.
+	 */
+	dw_pcie_writel_dbi(pci, PCIE_MISC_CONTROL_1_OFF, DBI_RO_WR_EN);
 
-	exynos_pcie_deassert_core_reset(ep);
 	dw_pcie_setup_rc(pp);
 	exynos_pcie_assert_reset(ep);
 
@@ -472,16 +570,6 @@ static void exynos_pcie_clear_irq_pulse(struct exynos_pcie *ep)
 	exynos_pcie_writel(ep->mem_res->elbi_base, val, PCIE_IRQ_PULSE);
 }
 
-static void exynos_pcie_enable_irq_pulse(struct exynos_pcie *ep)
-{
-	u32 val;
-
-	/* enable INTX interrupt */
-	val = IRQ_INTA_ASSERT | IRQ_INTB_ASSERT |
-		IRQ_INTC_ASSERT | IRQ_INTD_ASSERT;
-	exynos_pcie_writel(ep->mem_res->elbi_base, val, PCIE_IRQ_EN_PULSE);
-}
-
 static irqreturn_t exynos_pcie_irq_handler(int irq, void *arg)
 {
 	struct exynos_pcie *ep = arg;
@@ -513,9 +601,16 @@ static void exynos_pcie_msi_init(struct exynos_pcie *ep)
 	exynos_pcie_writel(ep->mem_res->elbi_base, val, PCIE_IRQ_EN_LEVEL);
 }
 
-static void exynos_pcie_enable_interrupts(struct exynos_pcie *ep)
+static void exynos_pcie_enable_irq_pulse(struct exynos_pcie *ep)
 {
-	exynos_pcie_enable_irq_pulse(ep);
+	u32 val;
+
+	val = IRQ_INTA_ASSERT | IRQ_INTB_ASSERT |
+		IRQ_INTC_ASSERT | IRQ_INTD_ASSERT;
+	exynos_pcie_writel(ep->mem_res->elbi_base, val, PCIE_IRQ_EN_PULSE);
+
+	exynos_pcie_writel(ep->mem_res->elbi_base, 0, PCIE_IRQ_EN_LEVEL);
+	exynos_pcie_writel(ep->mem_res->elbi_base, 0, PCIE_IRQ_EN_SPECIAL);
 
 	if (IS_ENABLED(CONFIG_PCI_MSI))
 		exynos_pcie_msi_init(ep);
@@ -575,10 +670,8 @@ static int exynos_pcie_link_up(struct dw_pcie *pci)
 	u32 val;
 
 	val = exynos_pcie_readl(ep->mem_res->elbi_base, PCIE_ELBI_RDLH_LINKUP);
-	if (val == PCIE_ELBI_LTSSM_ENABLE)
-		return 1;
 
-	return 0;
+	return (val & PCIE_ELBI_XMLH_LINKUP);
 }
 
 static int exynos_pcie_host_init(struct pcie_port *pp)
@@ -587,7 +680,7 @@ static int exynos_pcie_host_init(struct pcie_port *pp)
 	struct exynos_pcie *ep = to_exynos_pcie(pci);
 
 	exynos_pcie_establish_link(ep);
-	exynos_pcie_enable_interrupts(ep);
+	exynos_pcie_enable_irq_pulse(ep);
 
 	return 0;
 }
@@ -608,8 +701,11 @@ static int __init exynos_add_pcie_port(struct exynos_pcie *ep,
 
 	pp->irq = platform_get_irq(pdev, 1);
 	if (pp->irq < 0) {
-		dev_err(dev, "failed to get irq\n");
-		return pp->irq;
+		pp->irq = platform_get_irq_byname(pdev, "intr");
+		if (pp->irq < 0) {
+			dev_err(dev, "failed to get irq\n");
+			return pp->irq;
+		}
 	}
 	ret = devm_request_irq(dev, pp->irq, exynos_pcie_irq_handler,
 				IRQF_SHARED, "exynos-pcie", ep);
@@ -678,13 +774,23 @@ static int __init exynos_pcie_probe(struct platform_device *pdev)
 
 	ep->reset_gpio = of_get_named_gpio(np, "reset-gpio", 0);
 
-	/* Assume that controller doesn't use the PHY framework */
-	ep->using_phy = false;
+	/*
+	 * In case of Exynos5440,
+	 * Assume that controller doesn't use the PHY frameork.
+	 * Other SoCs might be used the PHY framework.
+	 */
+
+	if (of_device_is_compatible(np, "samsung,exynos5440-pcie"))
+		ep->using_phy = false;
 
-	ep->phy = devm_of_phy_get(dev, np, NULL);
+	ep->phy = devm_of_phy_get(dev, np, "pcie-phy");
 	if (IS_ERR(ep->phy)) {
 		if (PTR_ERR(ep->phy) == -EPROBE_DEFER)
 			return PTR_ERR(ep->phy);
+		if (!of_device_is_compatible(np, "samsung,exynos5440-pcie")) {
+			dev_err(dev, "Can't find the pcie-phy\n");
+			return PTR_ERR(ep->phy);
+		}
 		dev_warn(dev, "Use the 'phy' property. Current DT of pci-exynos was deprecated!!\n");
 	} else
 		ep->using_phy = true;
@@ -734,23 +840,20 @@ static int __exit exynos_pcie_remove(struct platform_device *pdev)
 static const struct of_device_id exynos_pcie_of_match[] = {
 	{
 		.compatible = "samsung,exynos5440-pcie",
-		.data = &exynos5440_pcie_ops
+		.data = &exynos5440_pcie_ops,
+	}, {
+		.compatible = "samsung,exynos5433-pcie",
+		.data = &exynos5433_pcie_ops,
 	},
 	{},
 };
 
 static struct platform_driver exynos_pcie_driver = {
+	.probe		= exynos_pcie_probe,
 	.remove		= __exit_p(exynos_pcie_remove),
 	.driver = {
 		.name	= "exynos-pcie",
 		.of_match_table = exynos_pcie_of_match,
 	},
 };
-
-/* Exynos PCIe driver does not allow module unload */
-
-static int __init exynos_pcie_init(void)
-{
-	return platform_driver_probe(&exynos_pcie_driver, exynos_pcie_probe);
-}
-subsys_initcall(exynos_pcie_init);
+builtin_platform_driver(exynos_pcie_driver);
-- 
2.15.0

Re: [RFC 1/2] pci: dwc: pci-exynos: modify the Kconfig dependency

[Lorenzo Pieralisi]

Hi Jaehoon,

On Thu, Dec 21, 2017 at 09:14:06PM +0900, Jaehoon Chung wrote:
> PCI_EXYNOS has the dependency with SOC_EXYNOS5440.
> It's modified to ARCH_EXYNOS from SOC_EXYNOS5440, because other
> SoCs needs to use this driver.
> 
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
>  drivers/pci/dwc/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/dwc/Kconfig b/drivers/pci/dwc/Kconfig
> index 113e09440f85..0ff9795df8e8 100644
> --- a/drivers/pci/dwc/Kconfig
> +++ b/drivers/pci/dwc/Kconfig
> @@ -65,7 +65,7 @@ config PCIE_DW_PLAT
>  config PCI_EXYNOS
>  	bool "Samsung Exynos PCIe controller"
>  	depends on PCI
> -	depends on SOC_EXYNOS5440
> +	depends on ARCH_EXYNOS
>  	depends on PCI_MSI_IRQ_DOMAIN
>  	select PCIEPORTBUS
>  	select PCIE_DW_HOST

This patch fell through the cracks, is it still needed ? Please let
me know and I will apply it if it is, thank you very much.

Lorenzo

Re: [RFC 1/2] pci: dwc: pci-exynos: modify the Kconfig dependency

[Jaehoon Chung]

Dear Lorenzo,

On 03/13/2018 08:12 PM, Lorenzo Pieralisi wrote:
> Hi Jaehoon,
> 
> On Thu, Dec 21, 2017 at 09:14:06PM +0900, Jaehoon Chung wrote:
>> PCI_EXYNOS has the dependency with SOC_EXYNOS5440.
>> It's modified to ARCH_EXYNOS from SOC_EXYNOS5440, because other
>> SoCs needs to use this driver.
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>> ---
>>  drivers/pci/dwc/Kconfig | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/dwc/Kconfig b/drivers/pci/dwc/Kconfig
>> index 113e09440f85..0ff9795df8e8 100644
>> --- a/drivers/pci/dwc/Kconfig
>> +++ b/drivers/pci/dwc/Kconfig
>> @@ -65,7 +65,7 @@ config PCIE_DW_PLAT
>>  config PCI_EXYNOS
>>  	bool "Samsung Exynos PCIe controller"
>>  	depends on PCI
>> -	depends on SOC_EXYNOS5440
>> +	depends on ARCH_EXYNOS
>>  	depends on PCI_MSI_IRQ_DOMAIN
>>  	select PCIEPORTBUS
>>  	select PCIE_DW_HOST
> 
> This patch fell through the cracks, is it still needed ? Please let
> me know and I will apply it if it is, thank you very much.

I will resend the patches for supporting exynos5433(TM2 board) without RFC.
I have already prepared the patches. now i'm doing some more test and cleaning code.

Best Regards,
Jaehoon Chung

> 
> Lorenzo
> 
> 
>