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@xxxxxxxxxxx> >> --- >> ...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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html