Message ID | 20210506023448.169146-1-xxm@rock-chips.com |
---|---|
State | Not Applicable, archived |
Headers | show |
Series | [v9,1/2] dt-bindings: rockchip: Add DesignWare based PCIe controller | expand |
Context | Check | Description |
---|---|---|
robh/checkpatch | success | |
robh/dt-meta-schema | success | |
robh/dtbs-check | success |
[+Pali] On Thu, May 06, 2021 at 10:35:44AM +0800, Simon Xue wrote: > Add a driver for the DesignWare-based PCIe controller found on > RK356X. The existing pcie-rockchip-host driver is only used for > the Rockchip-designed IP found on RK3399. > > Tested-by: Peter Geis <pgwipeout@gmail.com> > Reviewed-by: Kever Yang <kever.yang@rock-chips.com> > Reported-by: kernel test robot <lkp@intel.com> I will remove this tag - it does not make sense on a patch adding a new driver. [...] > +static int rockchip_pcie_start_link(struct dw_pcie *pci) > +{ > + struct rockchip_pcie *rockchip = to_rockchip_pcie(pci); > + > + /* Reset device */ > + gpiod_set_value_cansleep(rockchip->rst_gpio, 0); > + > + rockchip_pcie_enable_ltssm(rockchip); > + > + /* > + * PCIe requires the refclk to be stable for 100µs prior to releasing > + * PERST. See table 2-4 in section 2.6.2 AC Specifications of the PCI > + * Express Card Electromechanical Specification, 1.1. However, we don't > + * know if the refclk is coming from RC's PHY or external OSC. If it's > + * from RC, so enabling LTSSM is the just right place to release #PERST. > + * We need more extra time as before, rather than setting just > + * 100us as we don't know how long should the device need to reset. > + */ > + msleep(100); Any rationale behind the time chosen ? Ongoing discussion: https://lore.kernel.org/linux-pci/20210531090540.2663171-1-luca@lucaceresoli.net Lorenzo > + gpiod_set_value_cansleep(rockchip->rst_gpio, 1); > + > + return 0; > +} > + > +static int rockchip_pcie_host_init(struct pcie_port *pp) > +{ > + struct dw_pcie *pci = to_dw_pcie_from_pp(pp); > + struct rockchip_pcie *rockchip = to_rockchip_pcie(pci); > + u32 val; > + > + /* LTSSM enable control mode */ > + val = rockchip_pcie_readl_apb(rockchip, PCIE_CLIENT_HOT_RESET_CTRL); > + val |= PCIE_LTSSM_ENABLE_ENHANCE | (PCIE_LTSSM_ENABLE_ENHANCE << 16); > + rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_HOT_RESET_CTRL); > + > + rockchip_pcie_writel_apb(rockchip, PCIE_CLIENT_RC_MODE, > + PCIE_CLIENT_GENERAL_CONTROL); > + > + return 0; > +} > + > +static const struct dw_pcie_host_ops rockchip_pcie_host_ops = { > + .host_init = rockchip_pcie_host_init, > +}; > + > +static int rockchip_pcie_clk_init(struct rockchip_pcie *rockchip) > +{ > + struct device *dev = rockchip->pci.dev; > + int ret; > + > + ret = devm_clk_bulk_get_all(dev, &rockchip->clks); > + if (ret < 0) > + return ret; > + > + rockchip->clk_cnt = ret; > + > + return clk_bulk_prepare_enable(rockchip->clk_cnt, rockchip->clks); > +} > + > +static int rockchip_pcie_resource_get(struct platform_device *pdev, > + struct rockchip_pcie *rockchip) > +{ > + rockchip->apb_base = devm_platform_ioremap_resource_byname(pdev, "apb"); > + if (IS_ERR(rockchip->apb_base)) > + return PTR_ERR(rockchip->apb_base); > + > + rockchip->rst_gpio = devm_gpiod_get_optional(&pdev->dev, "reset", > + GPIOD_OUT_HIGH); > + if (IS_ERR(rockchip->rst_gpio)) > + return PTR_ERR(rockchip->rst_gpio); > + > + return 0; > +} > + > +static int rockchip_pcie_phy_init(struct rockchip_pcie *rockchip) > +{ > + struct device *dev = rockchip->pci.dev; > + int ret; > + > + rockchip->phy = devm_phy_get(dev, "pcie-phy"); > + if (IS_ERR(rockchip->phy)) > + return dev_err_probe(dev, PTR_ERR(rockchip->phy), > + "missing PHY\n"); > + > + ret = phy_init(rockchip->phy); > + if (ret < 0) > + return ret; > + > + ret = phy_power_on(rockchip->phy); > + if (ret) > + phy_exit(rockchip->phy); > + > + return ret; > +} > + > +static void rockchip_pcie_phy_deinit(struct rockchip_pcie *rockchip) > +{ > + phy_exit(rockchip->phy); > + phy_power_off(rockchip->phy); > +} > + > +static int rockchip_pcie_reset_control_release(struct rockchip_pcie *rockchip) > +{ > + struct device *dev = rockchip->pci.dev; > + > + rockchip->rst = devm_reset_control_array_get_exclusive(dev); > + if (IS_ERR(rockchip->rst)) > + return dev_err_probe(dev, PTR_ERR(rockchip->rst), > + "failed to get reset lines\n"); > + > + return reset_control_deassert(rockchip->rst); > +} > + > +static const struct dw_pcie_ops dw_pcie_ops = { > + .link_up = rockchip_pcie_link_up, > + .start_link = rockchip_pcie_start_link, > +}; > + > +static int rockchip_pcie_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct rockchip_pcie *rockchip; > + struct pcie_port *pp; > + int ret; > + > + rockchip = devm_kzalloc(dev, sizeof(*rockchip), GFP_KERNEL); > + if (!rockchip) > + return -ENOMEM; > + > + platform_set_drvdata(pdev, rockchip); > + > + rockchip->pci.dev = dev; > + rockchip->pci.ops = &dw_pcie_ops; > + > + pp = &rockchip->pci.pp; > + pp->ops = &rockchip_pcie_host_ops; > + > + ret = rockchip_pcie_resource_get(pdev, rockchip); > + if (ret) > + return ret; > + > + /* DON'T MOVE ME: must be enable before PHY init */ > + rockchip->vpcie3v3 = devm_regulator_get_optional(dev, "vpcie3v3"); > + if (IS_ERR(rockchip->vpcie3v3)) > + if (PTR_ERR(rockchip->vpcie3v3) != -ENODEV) > + return dev_err_probe(dev, PTR_ERR(rockchip->vpcie3v3), > + "failed to get vpcie3v3 regulator\n"); > + > + ret = regulator_enable(rockchip->vpcie3v3); > + if (ret) { > + dev_err(dev, "failed to enable vpcie3v3 regulator\n"); > + return ret; > + } > + > + ret = rockchip_pcie_phy_init(rockchip); > + if (ret) > + goto disable_regulator; > + > + ret = rockchip_pcie_reset_control_release(rockchip); > + if (ret) > + goto deinit_phy; > + > + ret = rockchip_pcie_clk_init(rockchip); > + if (ret) > + goto deinit_phy; > + > + ret = dw_pcie_host_init(pp); > + if (!ret) > + return 0; > + > + clk_bulk_disable_unprepare(rockchip->clk_cnt, rockchip->clks); > +deinit_phy: > + rockchip_pcie_phy_deinit(rockchip); > +disable_regulator: > + regulator_disable(rockchip->vpcie3v3); > + > + return ret; > +} > + > +static const struct of_device_id rockchip_pcie_of_match[] = { > + { .compatible = "rockchip,rk3568-pcie", }, > + {}, > +}; > + > +static struct platform_driver rockchip_pcie_driver = { > + .driver = { > + .name = "rockchip-dw-pcie", > + .of_match_table = rockchip_pcie_of_match, > + .suppress_bind_attrs = true, > + }, > + .probe = rockchip_pcie_probe, > +}; > +builtin_platform_driver(rockchip_pcie_driver); > -- > 2.25.1 > > >
== On Wed, May 5, 2021 at 8:35 PM Simon Xue <xxm@rock-chips.com> wrote: > > Add a driver for the DesignWare-based PCIe controller found on > RK356X. The existing pcie-rockchip-host driver is only used for > the Rockchip-designed IP found on RK3399. > > Tested-by: Peter Geis <pgwipeout@gmail.com> > Reviewed-by: Kever Yang <kever.yang@rock-chips.com> > Reported-by: kernel test robot <lkp@intel.com> > Signed-off-by: Simon Xue <xxm@rock-chips.com> > Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> > --- > drivers/pci/controller/dwc/Kconfig | 11 + > drivers/pci/controller/dwc/Makefile | 1 + > drivers/pci/controller/dwc/pcie-dw-rockchip.c | 277 ++++++++++++++++++ > 3 files changed, 289 insertions(+) > create mode 100644 drivers/pci/controller/dwc/pcie-dw-rockchip.c > > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig > index 423d35872ce4..60d3dde9ca39 100644 > --- a/drivers/pci/controller/dwc/Kconfig > +++ b/drivers/pci/controller/dwc/Kconfig > @@ -214,6 +214,17 @@ config PCIE_ARTPEC6_EP > Enables support for the PCIe controller in the ARTPEC-6 SoC to work in > endpoint mode. This uses the DesignWare core. > > +config PCIE_ROCKCHIP_DW_HOST > + bool "Rockchip DesignWare PCIe controller" > + select PCIE_DW > + select PCIE_DW_HOST > + depends on PCI_MSI_IRQ_DOMAIN > + depends on ARCH_ROCKCHIP || COMPILE_TEST > + depends on OF > + help > + Enables support for the DesignWare PCIe controller in the > + Rockchip SoC except RK3399. > + > config PCIE_INTEL_GW > bool "Intel Gateway PCIe host controller support" > depends on OF && (X86 || COMPILE_TEST) > diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile > index eca805c1a023..689b06a7d334 100644 > --- a/drivers/pci/controller/dwc/Makefile > +++ b/drivers/pci/controller/dwc/Makefile > @@ -14,6 +14,7 @@ obj-$(CONFIG_PCI_LAYERSCAPE_EP) += pci-layerscape-ep.o > obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o > obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o > obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o > +obj-$(CONFIG_PCIE_ROCKCHIP_DW_HOST) += pcie-dw-rockchip.o > obj-$(CONFIG_PCIE_INTEL_GW) += pcie-intel-gw.o > obj-$(CONFIG_PCIE_KIRIN) += pcie-kirin.o > obj-$(CONFIG_PCIE_HISI_STB) += pcie-histb.o > diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c > new file mode 100644 > index 000000000000..3f060144eeab > --- /dev/null > +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c > @@ -0,0 +1,277 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * PCIe host controller driver for Rockchip SoCs. > + * > + * Copyright (C) 2021 Rockchip Electronics Co., Ltd. > + * http://www.rock-chips.com > + * > + * Author: Simon Xue <xxm@rock-chips.com> > + */ > + > +#include <linux/clk.h> > +#include <linux/gpio/consumer.h> > +#include <linux/mfd/syscon.h> > +#include <linux/module.h> > +#include <linux/of_device.h> > +#include <linux/phy/phy.h> > +#include <linux/platform_device.h> > +#include <linux/regmap.h> > +#include <linux/reset.h> > + > +#include "pcie-designware.h" > + > +/* > + * The upper 16 bits of PCIE_CLIENT_CONFIG are a write > + * mask for the lower 16 bits. > + */ > +#define HIWORD_UPDATE(mask, val) (((mask) << 16) | (val)) > +#define HIWORD_UPDATE_BIT(val) HIWORD_UPDATE(val, val) > + > +#define to_rockchip_pcie(x) dev_get_drvdata((x)->dev) > + > +#define PCIE_CLIENT_RC_MODE HIWORD_UPDATE_BIT(0x40) > +#define PCIE_CLIENT_ENABLE_LTSSM HIWORD_UPDATE_BIT(0xc) > +#define PCIE_SMLH_LINKUP BIT(16) > +#define PCIE_RDLH_LINKUP BIT(17) > +#define PCIE_LINKUP (PCIE_SMLH_LINKUP | PCIE_RDLH_LINKUP) > +#define PCIE_L0S_ENTRY 0x11 > +#define PCIE_CLIENT_GENERAL_CONTROL 0x0 > +#define PCIE_CLIENT_GENERAL_DEBUG 0x104 > +#define PCIE_CLIENT_HOT_RESET_CTRL 0x180 > +#define PCIE_CLIENT_LTSSM_STATUS 0x300 > +#define PCIE_LTSSM_ENABLE_ENHANCE BIT(4) > + > +struct rockchip_pcie { > + struct dw_pcie pci; > + void __iomem *apb_base; > + struct phy *phy; > + struct clk_bulk_data *clks; > + unsigned int clk_cnt; > + struct reset_control *rst; > + struct gpio_desc *rst_gpio; > + struct regulator *vpcie3v3; > +}; > + > +static int rockchip_pcie_readl_apb(struct rockchip_pcie *rockchip, > + u32 reg) > +{ > + return readl(rockchip->apb_base + reg); Use _relaxed variant. > +} > + > +static void rockchip_pcie_writel_apb(struct rockchip_pcie *rockchip, > + u32 val, u32 reg) > +{ > + writel(val, rockchip->apb_base + reg); Use _relaxed variant. > +} > + > +static void rockchip_pcie_enable_ltssm(struct rockchip_pcie *rockchip) > +{ > + rockchip_pcie_writel_apb(rockchip, PCIE_CLIENT_ENABLE_LTSSM, > + PCIE_CLIENT_GENERAL_CONTROL); > +} > + > +static int rockchip_pcie_link_up(struct dw_pcie *pci) > +{ > + struct rockchip_pcie *rockchip = to_rockchip_pcie(pci); > + u32 val = rockchip_pcie_readl_apb(rockchip, PCIE_CLIENT_LTSSM_STATUS); > + > + if ((val & (PCIE_RDLH_LINKUP | PCIE_SMLH_LINKUP)) == PCIE_LINKUP && ((val & PCIE_LINKUP) == PCIE_LINKUP) > + (val & GENMASK(5, 0)) == PCIE_L0S_ENTRY) Perhaps a define for GENMASK(5, 0) > + return 1; > + > + return 0; > +} > + > +static int rockchip_pcie_start_link(struct dw_pcie *pci) > +{ > + struct rockchip_pcie *rockchip = to_rockchip_pcie(pci); > + > + /* Reset device */ > + gpiod_set_value_cansleep(rockchip->rst_gpio, 0); With gpiod, 0 means inactive which is reset deasserted. Your DT should have an ACTIVE_LOW flag. > + > + rockchip_pcie_enable_ltssm(rockchip); > + > + /* > + * PCIe requires the refclk to be stable for 100µs prior to releasing > + * PERST. See table 2-4 in section 2.6.2 AC Specifications of the PCI > + * Express Card Electromechanical Specification, 1.1. However, we don't > + * know if the refclk is coming from RC's PHY or external OSC. If it's > + * from RC, so enabling LTSSM is the just right place to release #PERST. > + * We need more extra time as before, rather than setting just > + * 100us as we don't know how long should the device need to reset. > + */ > + msleep(100); > + gpiod_set_value_cansleep(rockchip->rst_gpio, 1); > + > + return 0; > +} > + > +static int rockchip_pcie_host_init(struct pcie_port *pp) > +{ > + struct dw_pcie *pci = to_dw_pcie_from_pp(pp); > + struct rockchip_pcie *rockchip = to_rockchip_pcie(pci); > + u32 val; > + > + /* LTSSM enable control mode */ > + val = rockchip_pcie_readl_apb(rockchip, PCIE_CLIENT_HOT_RESET_CTRL); > + val |= PCIE_LTSSM_ENABLE_ENHANCE | (PCIE_LTSSM_ENABLE_ENHANCE << 16); Since you have a write mask, why do you need to do a read-mod-write? And why not use HIWORD_UPDATE_BIT? > + rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_HOT_RESET_CTRL); > + > + rockchip_pcie_writel_apb(rockchip, PCIE_CLIENT_RC_MODE, > + PCIE_CLIENT_GENERAL_CONTROL); > + > + return 0; > +}
Hi, 在 2021/6/23 22:33, Lorenzo Pieralisi 写道: > [+Pali] > > On Thu, May 06, 2021 at 10:35:44AM +0800, Simon Xue wrote: >> Add a driver for the DesignWare-based PCIe controller found on >> RK356X. The existing pcie-rockchip-host driver is only used for >> the Rockchip-designed IP found on RK3399. >> >> Tested-by: Peter Geis <pgwipeout@gmail.com> >> Reviewed-by: Kever Yang <kever.yang@rock-chips.com> >> Reported-by: kernel test robot <lkp@intel.com> > I will remove this tag - it does not make sense on a patch adding > a new driver. > > [...] > >> +static int rockchip_pcie_start_link(struct dw_pcie *pci) >> +{ >> + struct rockchip_pcie *rockchip = to_rockchip_pcie(pci); >> + >> + /* Reset device */ >> + gpiod_set_value_cansleep(rockchip->rst_gpio, 0); >> + >> + rockchip_pcie_enable_ltssm(rockchip); >> + >> + /* >> + * PCIe requires the refclk to be stable for 100µs prior to releasing >> + * PERST. See table 2-4 in section 2.6.2 AC Specifications of the PCI >> + * Express Card Electromechanical Specification, 1.1. However, we don't >> + * know if the refclk is coming from RC's PHY or external OSC. If it's >> + * from RC, so enabling LTSSM is the just right place to release #PERST. >> + * We need more extra time as before, rather than setting just >> + * 100us as we don't know how long should the device need to reset. >> + */ >> + msleep(100); > Any rationale behind the time chosen ? We found some device need about 30ms, so 100ms here just leave more room for other devices. > Ongoing discussion: > > https://lore.kernel.org/linux-pci/20210531090540.2663171-1-luca@lucaceresoli.net > > Lorenzo > >> + gpiod_set_value_cansleep(rockchip->rst_gpio, 1); >> + >> + return 0; >> +} >> + >> +static int rockchip_pcie_host_init(struct pcie_port *pp) >> +{ >> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp); >> + struct rockchip_pcie *rockchip = to_rockchip_pcie(pci); >> + u32 val; >> + >> + /* LTSSM enable control mode */ >> + val = rockchip_pcie_readl_apb(rockchip, PCIE_CLIENT_HOT_RESET_CTRL); >> + val |= PCIE_LTSSM_ENABLE_ENHANCE | (PCIE_LTSSM_ENABLE_ENHANCE << 16); >> + rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_HOT_RESET_CTRL); >> + >> + rockchip_pcie_writel_apb(rockchip, PCIE_CLIENT_RC_MODE, >> + PCIE_CLIENT_GENERAL_CONTROL); >> + >> + return 0; >> +} >> + >> +static const struct dw_pcie_host_ops rockchip_pcie_host_ops = { >> + .host_init = rockchip_pcie_host_init, >> +}; >> + >> +static int rockchip_pcie_clk_init(struct rockchip_pcie *rockchip) >> +{ >> + struct device *dev = rockchip->pci.dev; >> + int ret; >> + >> + ret = devm_clk_bulk_get_all(dev, &rockchip->clks); >> + if (ret < 0) >> + return ret; >> + >> + rockchip->clk_cnt = ret; >> + >> + return clk_bulk_prepare_enable(rockchip->clk_cnt, rockchip->clks); >> +} >> + >> +static int rockchip_pcie_resource_get(struct platform_device *pdev, >> + struct rockchip_pcie *rockchip) >> +{ >> + rockchip->apb_base = devm_platform_ioremap_resource_byname(pdev, "apb"); >> + if (IS_ERR(rockchip->apb_base)) >> + return PTR_ERR(rockchip->apb_base); >> + >> + rockchip->rst_gpio = devm_gpiod_get_optional(&pdev->dev, "reset", >> + GPIOD_OUT_HIGH); >> + if (IS_ERR(rockchip->rst_gpio)) >> + return PTR_ERR(rockchip->rst_gpio); >> + >> + return 0; >> +} >> + >> +static int rockchip_pcie_phy_init(struct rockchip_pcie *rockchip) >> +{ >> + struct device *dev = rockchip->pci.dev; >> + int ret; >> + >> + rockchip->phy = devm_phy_get(dev, "pcie-phy"); >> + if (IS_ERR(rockchip->phy)) >> + return dev_err_probe(dev, PTR_ERR(rockchip->phy), >> + "missing PHY\n"); >> + >> + ret = phy_init(rockchip->phy); >> + if (ret < 0) >> + return ret; >> + >> + ret = phy_power_on(rockchip->phy); >> + if (ret) >> + phy_exit(rockchip->phy); >> + >> + return ret; >> +} >> + >> +static void rockchip_pcie_phy_deinit(struct rockchip_pcie *rockchip) >> +{ >> + phy_exit(rockchip->phy); >> + phy_power_off(rockchip->phy); >> +} >> + >> +static int rockchip_pcie_reset_control_release(struct rockchip_pcie *rockchip) >> +{ >> + struct device *dev = rockchip->pci.dev; >> + >> + rockchip->rst = devm_reset_control_array_get_exclusive(dev); >> + if (IS_ERR(rockchip->rst)) >> + return dev_err_probe(dev, PTR_ERR(rockchip->rst), >> + "failed to get reset lines\n"); >> + >> + return reset_control_deassert(rockchip->rst); >> +} >> + >> +static const struct dw_pcie_ops dw_pcie_ops = { >> + .link_up = rockchip_pcie_link_up, >> + .start_link = rockchip_pcie_start_link, >> +}; >> + >> +static int rockchip_pcie_probe(struct platform_device *pdev) >> +{ >> + struct device *dev = &pdev->dev; >> + struct rockchip_pcie *rockchip; >> + struct pcie_port *pp; >> + int ret; >> + >> + rockchip = devm_kzalloc(dev, sizeof(*rockchip), GFP_KERNEL); >> + if (!rockchip) >> + return -ENOMEM; >> + >> + platform_set_drvdata(pdev, rockchip); >> + >> + rockchip->pci.dev = dev; >> + rockchip->pci.ops = &dw_pcie_ops; >> + >> + pp = &rockchip->pci.pp; >> + pp->ops = &rockchip_pcie_host_ops; >> + >> + ret = rockchip_pcie_resource_get(pdev, rockchip); >> + if (ret) >> + return ret; >> + >> + /* DON'T MOVE ME: must be enable before PHY init */ >> + rockchip->vpcie3v3 = devm_regulator_get_optional(dev, "vpcie3v3"); >> + if (IS_ERR(rockchip->vpcie3v3)) >> + if (PTR_ERR(rockchip->vpcie3v3) != -ENODEV) >> + return dev_err_probe(dev, PTR_ERR(rockchip->vpcie3v3), >> + "failed to get vpcie3v3 regulator\n"); >> + >> + ret = regulator_enable(rockchip->vpcie3v3); >> + if (ret) { >> + dev_err(dev, "failed to enable vpcie3v3 regulator\n"); >> + return ret; >> + } >> + >> + ret = rockchip_pcie_phy_init(rockchip); >> + if (ret) >> + goto disable_regulator; >> + >> + ret = rockchip_pcie_reset_control_release(rockchip); >> + if (ret) >> + goto deinit_phy; >> + >> + ret = rockchip_pcie_clk_init(rockchip); >> + if (ret) >> + goto deinit_phy; >> + >> + ret = dw_pcie_host_init(pp); >> + if (!ret) >> + return 0; >> + >> + clk_bulk_disable_unprepare(rockchip->clk_cnt, rockchip->clks); >> +deinit_phy: >> + rockchip_pcie_phy_deinit(rockchip); >> +disable_regulator: >> + regulator_disable(rockchip->vpcie3v3); >> + >> + return ret; >> +} >> + >> +static const struct of_device_id rockchip_pcie_of_match[] = { >> + { .compatible = "rockchip,rk3568-pcie", }, >> + {}, >> +}; >> + >> +static struct platform_driver rockchip_pcie_driver = { >> + .driver = { >> + .name = "rockchip-dw-pcie", >> + .of_match_table = rockchip_pcie_of_match, >> + .suppress_bind_attrs = true, >> + }, >> + .probe = rockchip_pcie_probe, >> +}; >> +builtin_platform_driver(rockchip_pcie_driver); >> -- >> 2.25.1 >> >> >> > >
On Thursday 24 June 2021 10:08:54 xxm wrote: > 在 2021/6/23 22:33, Lorenzo Pieralisi 写道: > > On Thu, May 06, 2021 at 10:35:44AM +0800, Simon Xue wrote: > > > +static int rockchip_pcie_start_link(struct dw_pcie *pci) > > > +{ > > > + struct rockchip_pcie *rockchip = to_rockchip_pcie(pci); > > > + > > > + /* Reset device */ > > > + gpiod_set_value_cansleep(rockchip->rst_gpio, 0); > > > + > > > + rockchip_pcie_enable_ltssm(rockchip); > > > + > > > + /* > > > + * PCIe requires the refclk to be stable for 100µs prior to releasing > > > + * PERST. See table 2-4 in section 2.6.2 AC Specifications of the PCI > > > + * Express Card Electromechanical Specification, 1.1. However, we don't > > > + * know if the refclk is coming from RC's PHY or external OSC. If it's > > > + * from RC, so enabling LTSSM is the just right place to release #PERST. > > > + * We need more extra time as before, rather than setting just > > > + * 100us as we don't know how long should the device need to reset. > > > + */ > > > + msleep(100); > > Any rationale behind the time chosen ? > We found some device need about 30ms, so 100ms here just leave more room for > other devices. Can you share information which PCIe card needs 30ms? Last year I did tests with more WiFi AC cards and "the slowest" one was Compex WLE1216 which needed about 11ms (more than 10ms). All other cards were happy with just 1-2ms.
Hi, 在 2021/6/24 16:23, Pali Rohár 写道: > On Thursday 24 June 2021 10:08:54 xxm wrote: >> 在 2021/6/23 22:33, Lorenzo Pieralisi 写道: >>> On Thu, May 06, 2021 at 10:35:44AM +0800, Simon Xue wrote: >>>> +static int rockchip_pcie_start_link(struct dw_pcie *pci) >>>> +{ >>>> + struct rockchip_pcie *rockchip = to_rockchip_pcie(pci); >>>> + >>>> + /* Reset device */ >>>> + gpiod_set_value_cansleep(rockchip->rst_gpio, 0); >>>> + >>>> + rockchip_pcie_enable_ltssm(rockchip); >>>> + >>>> + /* >>>> + * PCIe requires the refclk to be stable for 100µs prior to releasing >>>> + * PERST. See table 2-4 in section 2.6.2 AC Specifications of the PCI >>>> + * Express Card Electromechanical Specification, 1.1. However, we don't >>>> + * know if the refclk is coming from RC's PHY or external OSC. If it's >>>> + * from RC, so enabling LTSSM is the just right place to release #PERST. >>>> + * We need more extra time as before, rather than setting just >>>> + * 100us as we don't know how long should the device need to reset. >>>> + */ >>>> + msleep(100); >>> Any rationale behind the time chosen ? >> We found some device need about 30ms, so 100ms here just leave more room for >> other devices. > Can you share information which PCIe card needs 30ms? > > Last year I did tests with more WiFi AC cards and "the slowest" one was > Compex WLE1216 which needed about 11ms (more than 10ms). All other cards > were happy with just 1-2ms. Sorry, it was about 5 years ago, I can't find the specific card now. By the way, we also take other's upstream code "msleep(100)" as a reference. > >
On Thu, May 06, 2021 at 10:34:48AM +0800, Simon Xue wrote: > Document DT bindings for PCIe controller found on Rockchip SoC. > > Reviewed-by: Rob Herring <robh@kernel.org> > Reviewed-by: Kever Yang <kever.yang@rock-chips.com> > Signed-off-by: Simon Xue <xxm@rock-chips.com> > --- > .../bindings/pci/rockchip-dw-pcie.yaml | 141 ++++++++++++++++++ > 1 file changed, 141 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml I don't know why this didn't get sent with subsequent versions, but it should have. I've applied it now. Rob
diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml new file mode 100644 index 000000000000..142bbe577763 --- /dev/null +++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml @@ -0,0 +1,141 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pci/rockchip-dw-pcie.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: DesignWare based PCIe controller on Rockchip SoCs + +maintainers: + - Shawn Lin <shawn.lin@rock-chips.com> + - Simon Xue <xxm@rock-chips.com> + - Heiko Stuebner <heiko@sntech.de> + +description: |+ + RK3568 SoC PCIe host controller is based on the Synopsys DesignWare + PCIe IP and thus inherits all the common properties defined in + designware-pcie.txt. + +allOf: + - $ref: /schemas/pci/pci-bus.yaml# + +# We need a select here so we don't match all nodes with 'snps,dw-pcie' +select: + properties: + compatible: + contains: + const: rockchip,rk3568-pcie + required: + - compatible + +properties: + compatible: + items: + - const: rockchip,rk3568-pcie + - const: snps,dw-pcie + + reg: + items: + - description: Data Bus Interface (DBI) registers + - description: Rockchip designed configuration registers + - description: Config registers + + reg-names: + items: + - const: dbi + - const: apb + - const: config + + clocks: + items: + - description: AHB clock for PCIe master + - description: AHB clock for PCIe slave + - description: AHB clock for PCIe dbi + - description: APB clock for PCIe + - description: Auxiliary clock for PCIe + + clock-names: + items: + - const: aclk_mst + - const: aclk_slv + - const: aclk_dbi + - const: pclk + - const: aux + + msi-map: true + + num-lanes: true + + phys: + maxItems: 1 + + phy-names: + const: pcie-phy + + power-domains: + maxItems: 1 + + ranges: + maxItems: 2 + + resets: + maxItems: 1 + + reset-names: + const: pipe + + vpcie3v3-supply: true + +required: + - compatible + - reg + - reg-names + - clocks + - clock-names + - msi-map + - num-lanes + - phys + - phy-names + - power-domains + - resets + - reset-names + +unevaluatedProperties: false + +examples: + - | + + bus { + #address-cells = <2>; + #size-cells = <2>; + + pcie3x2: pcie@fe280000 { + compatible = "rockchip,rk3568-pcie", "snps,dw-pcie"; + reg = <0x3 0xc0800000 0x0 0x390000>, + <0x0 0xfe280000 0x0 0x10000>, + <0x3 0x80000000 0x0 0x100000>; + reg-names = "dbi", "apb", "config"; + bus-range = <0x20 0x2f>; + clocks = <&cru 143>, <&cru 144>, + <&cru 145>, <&cru 146>, + <&cru 147>; + clock-names = "aclk_mst", "aclk_slv", + "aclk_dbi", "pclk", + "aux"; + device_type = "pci"; + linux,pci-domain = <2>; + max-link-speed = <2>; + msi-map = <0x2000 &its 0x2000 0x1000>; + num-lanes = <2>; + phys = <&pcie30phy>; + phy-names = "pcie-phy"; + power-domains = <&power 15>; + ranges = <0x81000000 0x0 0x80800000 0x3 0x80800000 0x0 0x100000>, + <0x83000000 0x0 0x80900000 0x3 0x80900000 0x0 0x3f700000>; + resets = <&cru 193>; + reset-names = "pipe"; + #address-cells = <3>; + #size-cells = <2>; + }; + }; +...