| Message ID | 20201023075744.26200-1-m.szyprowski@samsung.com |
|---|---|
| Headers | show |
| Series | Add DW PCIe support for Exynos5433 SoCs | expand |
On Fri, Oct 23, 2020 at 09:57:40AM +0200, Marek Szyprowski wrote: > Add dt-bindings for the Samsung Exynos PCIe controller (Exynos5433 > variant). Based on the text dt-binding posted by Jaehoon Chung. > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > .../bindings/pci/samsung,exynos-pcie.yaml | 114 ++++++++++++++++++ > 1 file changed, 114 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml > Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org> Best regards, Krzysztof
Hi Rob, On 26.10.2020 20:14, Rob Herring wrote: > On Fri, Oct 23, 2020 at 2:58 AM Marek Szyprowski > <m.szyprowski@samsung.com> wrote: >> From: Jaehoon Chung <jh80.chung@samsung.com> >> >> Exynos5440 SoC support has been dropped since commit 8c83315da1cf ("ARM: >> dts: exynos: Remove Exynos5440"). Rework this driver to support DWC PCIe >> variant found in the Exynos5433 SoCs. >> >> The main difference in Exynos5433 variant is lack of the MSI support >> (the MSI interrupt is not even routed to the CPU). >> >> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com> >> [mszyprow: reworked the driver to support only Exynos5433 variant, >> simplified code, rebased onto current kernel code, added >> regulator support, converted to the regular platform driver, >> removed MSI related code, rewrote commit message] >> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> >> Acked-by: Krzysztof Kozlowski <krzk@kernel.org> >> --- >> drivers/pci/controller/dwc/Kconfig | 3 +- >> drivers/pci/controller/dwc/pci-exynos.c | 358 ++++++++++-------------- >> drivers/pci/quirks.c | 1 + >> 3 files changed, 145 insertions(+), 217 deletions(-) >> >> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig >> index bc049865f8e0..ade07abd23c9 100644 >> --- a/drivers/pci/controller/dwc/Kconfig >> +++ b/drivers/pci/controller/dwc/Kconfig >> @@ -84,8 +84,7 @@ config PCIE_DW_PLAT_EP >> >> config PCI_EXYNOS >> bool "Samsung Exynos PCIe controller" >> - depends on SOC_EXYNOS5440 || COMPILE_TEST >> - depends on PCI_MSI_IRQ_DOMAIN >> + depends on ARCH_EXYNOS || COMPILE_TEST >> select PCIE_DW_HOST >> >> config PCI_IMX6 >> diff --git a/drivers/pci/controller/dwc/pci-exynos.c b/drivers/pci/controller/dwc/pci-exynos.c >> index 242683cde04a..58056fbdc2fa 100644 >> --- a/drivers/pci/controller/dwc/pci-exynos.c >> +++ b/drivers/pci/controller/dwc/pci-exynos.c >> @@ -2,26 +2,23 @@ >> /* >> * PCIe host controller driver for Samsung Exynos SoCs >> * >> - * Copyright (C) 2013 Samsung Electronics Co., Ltd. >> + * Copyright (C) 2013-2020 Samsung Electronics Co., Ltd. >> * https://www.samsung.com >> * >> * Author: Jingoo Han <jg1.han@samsung.com> >> + * Jaehoon Chung <jh80.chung@samsung.com> >> */ >> >> #include <linux/clk.h> >> #include <linux/delay.h> >> -#include <linux/gpio.h> >> #include <linux/interrupt.h> >> #include <linux/kernel.h> >> #include <linux/init.h> >> #include <linux/of_device.h> >> -#include <linux/of_gpio.h> >> #include <linux/pci.h> >> #include <linux/platform_device.h> >> #include <linux/phy/phy.h> >> -#include <linux/resource.h> >> -#include <linux/signal.h> >> -#include <linux/types.h> >> +#include <linux/regulator/consumer.h> >> >> #include "pcie-designware.h" >> >> @@ -37,102 +34,47 @@ >> #define PCIE_IRQ_SPECIAL 0x008 >> #define PCIE_IRQ_EN_PULSE 0x00c >> #define PCIE_IRQ_EN_LEVEL 0x010 >> -#define IRQ_MSI_ENABLE BIT(2) >> #define PCIE_IRQ_EN_SPECIAL 0x014 >> -#define PCIE_PWR_RESET 0x018 >> +#define PCIE_SW_WAKE 0x018 >> +#define PCIE_BUS_EN BIT(1) >> #define PCIE_CORE_RESET 0x01c >> #define PCIE_CORE_RESET_ENABLE BIT(0) >> #define PCIE_STICKY_RESET 0x020 >> #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 >> #define PCIE_ELBI_SLV_DBI_ENABLE BIT(21) >> >> -struct exynos_pcie_mem_res { >> - void __iomem *elbi_base; /* DT 0th resource: PCIe CTRL */ >> -}; >> - >> -struct exynos_pcie_clk_res { >> - struct clk *clk; >> - struct clk *bus_clk; >> -}; >> +/* DBI register */ >> +#define PCIE_MISC_CONTROL_1_OFF 0x8BC >> +#define DBI_RO_WR_EN BIT(0) > Standard DWC port logic register. The core already handles this > mostly. And provides a function to it where it doesn't. Looking at > your use, I think you can drop the access. > >> ... >> @@ -243,19 +168,25 @@ static int exynos_pcie_establish_link(struct exynos_pcie *ep) >> exynos_pcie_assert_core_reset(ep); >> >> 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); >> >> exynos_pcie_deassert_core_reset(ep); >> + >> + val = exynos_pcie_readl(ep->elbi_base, PCIE_SW_WAKE); >> + val &= ~PCIE_BUS_EN; >> + exynos_pcie_writel(ep->elbi_base, val, PCIE_SW_WAKE); >> + >> + /* >> + * 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); >> dw_pcie_setup_rc(pp); > First thing this function does is set DBI_RO_WR_EN. Indeed, this has been added to dw_pcie_setup_rc() in commit 3924bc2fd1b6 ("PCI: dwc: Group DBI registers writes requiring unlocking"), after initial version of this patchset. Thanks for pointing this out. I will remove this. >> ... >> @@ -450,42 +347,49 @@ static int __init exynos_pcie_probe(struct platform_device *pdev) >> if (!ep) >> return -ENOMEM; >> >> - pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL); >> - if (!pci) >> - return -ENOMEM; >> - >> - pci->dev = dev; >> - pci->ops = &dw_pcie_ops; >> + ep->pci.dev = dev; >> + ep->pci.ops = &dw_pcie_ops; >> >> - ep->pci = pci; >> - ep->ops = (const struct exynos_pcie_ops *) >> - of_device_get_match_data(dev); >> + ep->phy = devm_of_phy_get(dev, np, NULL); >> + if (IS_ERR(ep->phy)) >> + return PTR_ERR(ep->phy); >> >> - ep->reset_gpio = of_get_named_gpio(np, "reset-gpio", 0); >> + /* External Local Bus interface (ELBI) registers */ >> + ep->elbi_base = devm_platform_ioremap_resource_byname(pdev, "elbi"); >> + if (IS_ERR(ep->elbi_base)) >> + return PTR_ERR(ep->elbi_base); >> >> - ep->phy = devm_of_phy_get(dev, np, NULL); >> - if (IS_ERR(ep->phy)) { >> - if (PTR_ERR(ep->phy) != -ENODEV) >> - return PTR_ERR(ep->phy); >> + /* Data Bus Interface (DBI) registers */ >> + ep->pci.dbi_base = devm_platform_ioremap_resource_byname(pdev, "dbi"); >> + if (IS_ERR(ep->pci.dbi_base)) >> + return PTR_ERR(ep->pci.dbi_base); > This is going to get moved to the DWC core code. Well, so far it is not there yet and other dw-pci drivers do it on their own. Could you point a patch that does this, so I can rebase onto it? > >> - ep->phy = NULL; >> + ep->clk = devm_clk_get(dev, "pcie"); >> + if (IS_ERR(ep->clk)) { >> + dev_err(dev, "Failed to get pcie rc clock\n"); >> + return PTR_ERR(ep->clk); >> } >> >> - if (ep->ops && ep->ops->get_mem_resources) { >> - ret = ep->ops->get_mem_resources(pdev, ep); >> - if (ret) >> - return ret; >> + ep->bus_clk = devm_clk_get(dev, "pcie_bus"); >> + if (IS_ERR(ep->bus_clk)) { >> + dev_err(dev, "Failed to get pcie bus clock\n"); >> + return PTR_ERR(ep->bus_clk); >> } >> >> - if (ep->ops && ep->ops->get_clk_resources && >> - ep->ops->init_clk_resources) { >> - ret = ep->ops->get_clk_resources(ep); >> - if (ret) >> - return ret; >> - ret = ep->ops->init_clk_resources(ep); >> - if (ret) >> - return ret; >> - } >> + ep->supplies[0].supply = "vdd18"; >> + ep->supplies[1].supply = "vdd10"; >> + ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ep->supplies), >> + ep->supplies); >> + if (ret) >> + return ret; >> + >> + ret = exynos_pcie_init_clk_resources(ep); >> + if (ret) >> + return ret; >> + >> + ret = regulator_bulk_enable(ARRAY_SIZE(ep->supplies), ep->supplies); >> + if (ret) >> + return ret; >> >> platform_set_drvdata(pdev, ep); >> >> @@ -497,9 +401,9 @@ static int __init exynos_pcie_probe(struct platform_device *pdev) >> >> fail_probe: >> phy_exit(ep->phy); >> + exynos_pcie_deinit_clk_resources(ep); >> + regulator_bulk_disable(ARRAY_SIZE(ep->supplies), ep->supplies); >> >> - if (ep->ops && ep->ops->deinit_clk_resources) >> - ep->ops->deinit_clk_resources(ep); >> return ret; >> } >> >> @@ -507,32 +411,56 @@ static int __exit exynos_pcie_remove(struct platform_device *pdev) >> { >> struct exynos_pcie *ep = platform_get_drvdata(pdev); >> >> - if (ep->ops && ep->ops->deinit_clk_resources) >> - ep->ops->deinit_clk_resources(ep); >> + phy_power_off(ep->phy); >> + phy_exit(ep->phy); >> + exynos_pcie_deinit_clk_resources(ep); >> + regulator_bulk_disable(ARRAY_SIZE(ep->supplies), ep->supplies); >> >> return 0; >> } >> >> +static int __maybe_unused exynos_pcie_suspend_noirq(struct device *dev) >> +{ > Why noirq variant needed? Lot's of PCI host drivers do this and I've > yet to get a reason... Frankly, I have no idea, but switching to SET_LATE_SYSTEM_SLEEP_PM_OPS breaks system suspend/resume operation - the board doesn't resume from suspend. If this is really important I will add some more logs and try to find what happens between late/early and noirq phases. > Adding suspend/resume should probably be a separate patch. What I'd > like to do here is have common DWC suspend/resume functions that the > platform drivers can use or wrap. Okay, I can move adding suspend/resume to the separate patch if You want. However I probably know too little about PCI to extract some common dwc suspend/resume functions. >> + struct exynos_pcie *ep = dev_get_drvdata(dev); >> + >> + phy_power_off(ep->phy); >> + phy_exit(ep->phy); >> + regulator_bulk_disable(ARRAY_SIZE(ep->supplies), ep->supplies); >> + >> + return 0; >> +} >> + >> +static int __maybe_unused exynos_pcie_resume_noirq(struct device *dev) >> +{ >> + struct exynos_pcie *ep = dev_get_drvdata(dev); >> + struct dw_pcie *pci = &ep->pci; >> + struct pcie_port *pp = &pci->pp; >> + int ret; >> + >> + ret = regulator_bulk_enable(ARRAY_SIZE(ep->supplies), ep->supplies); >> + if (ret) >> + return ret; >> + /* exynos_pcie_host_init controls ep->phy */ >> + return exynos_pcie_host_init(pp); >> +} >> + >> +static const struct dev_pm_ops exynos_pcie_pm_ops = { >> + SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(exynos_pcie_suspend_noirq, >> + exynos_pcie_resume_noirq) >> +}; >> + >> static const struct of_device_id exynos_pcie_of_match[] = { >> - { >> - .compatible = "samsung,exynos5440-pcie", >> - .data = &exynos5440_pcie_ops >> - }, >> - {}, >> + { .compatible = "samsung,exynos5433-pcie", }, >> + { }, >> }; >> >> 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, >> + .pm = &exynos_pcie_pm_ops, >> }, >> }; >> - >> -/* 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); > Good that this is gone, but... > >> +builtin_platform_driver(exynos_pcie_driver); > I would like to make all the host drivers modules. I can check if this can be easily done. If not, I would like to keep it builtin in this patch and leave modularization for the future. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland
Dear All, This patchset is a resurrection of the DW PCIe support for the Exynos5433 SoCs posted long time ago here: https://lkml.org/lkml/2016/12/26/6 and later here: https://lkml.org/lkml/2017/12/21/296 . In meantime the support for the Exynos5440 SoCs has been completely dropped from mainline kernel, as those SoCs never reached the market. The PCIe driver for Exynos5440 variant however has not been removed yet. This patchset simply reworks it to support the Exynos5433 variant. The lack of the need to support both variants significantly simplifies the driver code. Best regards, Marek Szyprowski v2: - fixed issues in dt-bindings pointed by Krzysztof and Rob v1: https://lore.kernel.org/linux-samsung-soc/20201019094715.15343-1-m.szyprowski@samsung.com/ - initial version of this resurrected patchset Patch summary: Jaehoon Chung (3): phy: samsung: phy-exynos-pcie: rework driver to support Exynos5433 PCIe PHY pci: dwc: pci-exynos: rework the driver to support Exynos5433 variant arm64: dts: exynos: add the WiFi/PCIe support to TM2(e) boards Marek Szyprowski (3): dt-bindings: pci: drop samsung,exynos5440-pcie binding dt-bindings: pci: add the samsung,exynos-pcie binding dt-bindings: phy: add the samsung,exynos-pcie-phy binding .../bindings/pci/samsung,exynos-pcie.yaml | 114 ++++++ .../bindings/pci/samsung,exynos5440-pcie.txt | 58 --- .../bindings/phy/samsung,exynos-pcie-phy.yaml | 51 +++ .../boot/dts/exynos/exynos5433-pinctrl.dtsi | 2 +- .../dts/exynos/exynos5433-tm2-common.dtsi | 24 +- arch/arm64/boot/dts/exynos/exynos5433.dtsi | 35 ++ drivers/pci/controller/dwc/Kconfig | 3 +- drivers/pci/controller/dwc/pci-exynos.c | 358 +++++++----------- drivers/pci/quirks.c | 1 + drivers/phy/samsung/phy-exynos-pcie.c | 304 ++++++--------- 10 files changed, 481 insertions(+), 469 deletions(-) create mode 100644 Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml delete mode 100644 Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt create mode 100644 Documentation/devicetree/bindings/phy/samsung,exynos-pcie-phy.yaml -- 2.17.1