diff mbox series

[v7] PCI: rockchip: Add Rockchip RK356X host controller driver

Message ID 20210414070325.924789-1-xxm@rock-chips.com (mailing list archive)
State New
Headers show
Series [v7] PCI: rockchip: Add Rockchip RK356X host controller driver | expand

Commit Message

Simon Xue April 14, 2021, 7:03 a.m. UTC
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.

Signed-off-by: Simon Xue <xxm@rock-chips.com>
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---
 drivers/pci/controller/dwc/Kconfig            |  10 +
 drivers/pci/controller/dwc/Makefile           |   1 +
 drivers/pci/controller/dwc/pcie-dw-rockchip.c | 277 ++++++++++++++++++
 3 files changed, 288 insertions(+)
 create mode 100644 drivers/pci/controller/dwc/pcie-dw-rockchip.c

Comments

Peter Geis April 28, 2021, 4:20 p.m. UTC | #1
On Wed, Apr 14, 2021 at 3:05 AM 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.

Good Afternoon,

I've encountered a bit of an issue with this driver.
Unfortunately it does not support legacy interrupts, meaning any PCIe
device that doesn't support MSIs will fail to enumerate:
[   14.932078] pcieport 0000:00:00.0: of_irq_parse_pci: failed with rc=-22
[   14.932708] snd_hda_intel 0000:01:00.1: assign IRQ: got 0
[   14.933687] snd_hda_intel 0000:01:00.1: enabling device (0000 -> 0002)
[   14.934317] snd_hda_intel 0000:01:00.1: Disabling MSI
[   14.934783] snd_hda_intel 0000:01:00.1: Force to snoop mode by module option
[   14.935534] snd_hda_intel 0000:01:00.1: enabling bus mastering
[   14.939764] snd_hda_intel 0000:01:00.1: unable to grab IRQ 0,
disabling device

Are there plans to support legacy interrupts with the rk3568 controllers?

Thanks,
Peter Geis

>
> Signed-off-by: Simon Xue <xxm@rock-chips.com>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> ---
>  drivers/pci/controller/dwc/Kconfig            |  10 +
>  drivers/pci/controller/dwc/Makefile           |   1 +
>  drivers/pci/controller/dwc/pcie-dw-rockchip.c | 277 ++++++++++++++++++
>  3 files changed, 288 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..8ab027ba8c04 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -214,6 +214,16 @@ 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 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 952d01941f23..0104659dfe88 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);
> +}
> +
> +static void rockchip_pcie_writel_apb(struct rockchip_pcie *rockchip,
> +                                               u32 val, u32 reg)
> +{
> +       writel(val, rockchip->apb_base + reg);
> +}
> +
> +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 & GENMASK(5, 0)) == PCIE_L0S_ENTRY)
> +               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);
> +
> +       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);
> +       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
>
>
>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
Shawn Lin April 29, 2021, 12:46 a.m. UTC | #2
On 2021/4/29 0:20, Peter Geis wrote:
> On Wed, Apr 14, 2021 at 3:05 AM 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.
> 
> Good Afternoon,
> 
> I've encountered a bit of an issue with this driver.

I'm sure your issue is GIC ITS support on RK356x platforms, nothing
really related to this driver itself. You can also sort out arm-gic
workaround patch from downstream vendor tree, if you want MSI support.

> Unfortunately it does not support legacy interrupts, meaning any PCIe
> device that doesn't support MSIs will fail to enumerate:
> [   14.932078] pcieport 0000:00:00.0: of_irq_parse_pci: failed with rc=-22
> [   14.932708] snd_hda_intel 0000:01:00.1: assign IRQ: got 0
> [   14.933687] snd_hda_intel 0000:01:00.1: enabling device (0000 -> 0002)
> [   14.934317] snd_hda_intel 0000:01:00.1: Disabling MSI
> [   14.934783] snd_hda_intel 0000:01:00.1: Force to snoop mode by module option
> [   14.935534] snd_hda_intel 0000:01:00.1: enabling bus mastering
> [   14.939764] snd_hda_intel 0000:01:00.1: unable to grab IRQ 0,
> disabling device
> 
> Are there plans to support legacy interrupts with the rk3568 controllers?

Yes, we did,but not now.

I really don't understand what is blocking this driver from landing
mainline?  I didn't find any comments from maintainers yet since v1. And
now the driver has been iterating for a long time.I believe Simon has
addressed all the review feedback during v1 to v7. Can we please land
this basic support first. If you can test it with arm-gic workaround,
a tested-by tag would be appreciated.

> 
> Thanks,
> Peter Geis
> 
>>
>> Signed-off-by: Simon Xue <xxm@rock-chips.com>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>> ---
>>   drivers/pci/controller/dwc/Kconfig            |  10 +
>>   drivers/pci/controller/dwc/Makefile           |   1 +
>>   drivers/pci/controller/dwc/pcie-dw-rockchip.c | 277 ++++++++++++++++++
>>   3 files changed, 288 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..8ab027ba8c04 100644
>> --- a/drivers/pci/controller/dwc/Kconfig
>> +++ b/drivers/pci/controller/dwc/Kconfig
>> @@ -214,6 +214,16 @@ 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 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 952d01941f23..0104659dfe88 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);
>> +}
>> +
>> +static void rockchip_pcie_writel_apb(struct rockchip_pcie *rockchip,
>> +                                               u32 val, u32 reg)
>> +{
>> +       writel(val, rockchip->apb_base + reg);
>> +}
>> +
>> +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 & GENMASK(5, 0)) == PCIE_L0S_ENTRY)
>> +               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);
>> +
>> +       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);
>> +       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
>>
>>
>>
>>
>> _______________________________________________
>> Linux-rockchip mailing list
>> Linux-rockchip@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
> 
> 
>
Peter Geis April 29, 2021, 1:42 a.m. UTC | #3
On Wed, Apr 28, 2021 at 8:46 PM Shawn Lin <shawn.lin@rock-chips.com> wrote:
>
> On 2021/4/29 0:20, Peter Geis wrote:
> > On Wed, Apr 14, 2021 at 3:05 AM 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.
> >
> > Good Afternoon,
> >
> > I've encountered a bit of an issue with this driver.
>
> I'm sure your issue is GIC ITS support on RK356x platforms, nothing
> really related to this driver itself. You can also sort out arm-gic
> workaround patch from downstream vendor tree, if you want MSI support.

I have functional MSI support, some devices do not support MSIs
however and need legacy INTx.
I'd like to point out that the downstream patch does not actually work
on downstream.
The GFP_DMA32 flag is discarded by the slab allocator, this breaks MSI
allocation when the PCIe driver probes.
I hacked together my own version which works but would definitely not
be accepted for submission.

>
> > Unfortunately it does not support legacy interrupts, meaning any PCIe
> > device that doesn't support MSIs will fail to enumerate:
> > [   14.932078] pcieport 0000:00:00.0: of_irq_parse_pci: failed with rc=-22
> > [   14.932708] snd_hda_intel 0000:01:00.1: assign IRQ: got 0
> > [   14.933687] snd_hda_intel 0000:01:00.1: enabling device (0000 -> 0002)
> > [   14.934317] snd_hda_intel 0000:01:00.1: Disabling MSI
> > [   14.934783] snd_hda_intel 0000:01:00.1: Force to snoop mode by module option
> > [   14.935534] snd_hda_intel 0000:01:00.1: enabling bus mastering
> > [   14.939764] snd_hda_intel 0000:01:00.1: unable to grab IRQ 0,
> > disabling device
> >
> > Are there plans to support legacy interrupts with the rk3568 controllers?
>
> Yes, we did,but not now.

This will significantly limit the devices that can work with the controller.
Is there any functional reason preventing legacy interrupts from working?
I've so far not been successful plugging in support.

>
> I really don't understand what is blocking this driver from landing
> mainline?  I didn't find any comments from maintainers yet since v1. And
> now the driver has been iterating for a long time.I believe Simon has
> addressed all the review feedback during v1 to v7. Can we please land
> this basic support first. If you can test it with arm-gic workaround,
> a tested-by tag would be appreciated.

Do you have the DT bindings for this patch available?

Yes, at this stage what is implemented appears to function correctly, so:
Tested-by: Peter Geis <pgwipeout@gmail.com>
on the rk3566-quartz64 prototype development board.

Dependent on the following patches:
GICv3-ITS official errata for 32bit limitation and shareability limitation.
Combo-phy support, no patches have been released to mainline yet.

>
> >
> > Thanks,
> > Peter Geis
> >
> >>
> >> Signed-off-by: Simon Xue <xxm@rock-chips.com>
> >> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> >> ---
> >>   drivers/pci/controller/dwc/Kconfig            |  10 +
> >>   drivers/pci/controller/dwc/Makefile           |   1 +
> >>   drivers/pci/controller/dwc/pcie-dw-rockchip.c | 277 ++++++++++++++++++
> >>   3 files changed, 288 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..8ab027ba8c04 100644
> >> --- a/drivers/pci/controller/dwc/Kconfig
> >> +++ b/drivers/pci/controller/dwc/Kconfig
> >> @@ -214,6 +214,16 @@ 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 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 952d01941f23..0104659dfe88 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);
> >> +}
> >> +
> >> +static void rockchip_pcie_writel_apb(struct rockchip_pcie *rockchip,
> >> +                                               u32 val, u32 reg)
> >> +{
> >> +       writel(val, rockchip->apb_base + reg);
> >> +}
> >> +
> >> +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 & GENMASK(5, 0)) == PCIE_L0S_ENTRY)
> >> +               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);
> >> +
> >> +       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);
> >> +       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
> >>
> >>
> >>
> >>
> >> _______________________________________________
> >> Linux-rockchip mailing list
> >> Linux-rockchip@lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/linux-rockchip
> >
> >
> >
>
>
Shawn Lin April 29, 2021, 6:32 a.m. UTC | #4
On 2021/4/29 9:42, Peter Geis wrote:
> On Wed, Apr 28, 2021 at 8:46 PM Shawn Lin <shawn.lin@rock-chips.com> wrote:
>>
>> On 2021/4/29 0:20, Peter Geis wrote:
>>> On Wed, Apr 14, 2021 at 3:05 AM 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.
>>>
>>> Good Afternoon,
>>>
>>> I've encountered a bit of an issue with this driver.
>>
>> I'm sure your issue is GIC ITS support on RK356x platforms, nothing
>> really related to this driver itself. You can also sort out arm-gic
>> workaround patch from downstream vendor tree, if you want MSI support.
> 
> I have functional MSI support, some devices do not support MSIs
> however and need legacy INTx.
> I'd like to point out that the downstream patch does not actually work
> on downstream.
> The GFP_DMA32 flag is discarded by the slab allocator, this breaks MSI
> allocation when the PCIe driver probes.
> I hacked together my own version which works but would definitely not
> be accepted for submission.
> 
>>
>>> Unfortunately it does not support legacy interrupts, meaning any PCIe
>>> device that doesn't support MSIs will fail to enumerate:
>>> [   14.932078] pcieport 0000:00:00.0: of_irq_parse_pci: failed with rc=-22
>>> [   14.932708] snd_hda_intel 0000:01:00.1: assign IRQ: got 0
>>> [   14.933687] snd_hda_intel 0000:01:00.1: enabling device (0000 -> 0002)
>>> [   14.934317] snd_hda_intel 0000:01:00.1: Disabling MSI
>>> [   14.934783] snd_hda_intel 0000:01:00.1: Force to snoop mode by module option
>>> [   14.935534] snd_hda_intel 0000:01:00.1: enabling bus mastering
>>> [   14.939764] snd_hda_intel 0000:01:00.1: unable to grab IRQ 0,
>>> disabling device
>>>
>>> Are there plans to support legacy interrupts with the rk3568 controllers?
>>
>> Yes, we did,but not now.
> 
> This will significantly limit the devices that can work with the controller.
> Is there any functional reason preventing legacy interrupts from working?

Just because this $subject patch is still in-flight and we are going to
add it later. Still some features are developed but not ready yet.


> I've so far not been successful plugging in support.
> 
>>
>> I really don't understand what is blocking this driver from landing
>> mainline?  I didn't find any comments from maintainers yet since v1. And
>> now the driver has been iterating for a long time.I believe Simon has
>> addressed all the review feedback during v1 to v7. Can we please land
>> this basic support first. If you can test it with arm-gic workaround,
>> a tested-by tag would be appreciated.
> 
> Do you have the DT bindings for this patch available?

Yes we have, and you can also find it from vendor tree.

> 
> Yes, at this stage what is implemented appears to function correctly, so:
> Tested-by: Peter Geis <pgwipeout@gmail.com>
> on the rk3566-quartz64 prototype development board.

Thanks for testing.

> 
> Dependent on the following patches:
> GICv3-ITS official errata for 32bit limitation and shareability limitation.
> Combo-phy support, no patches have been released to mainline yet >
>>
>>>
>>> Thanks,
>>> Peter Geis
>>>
>>>>
>>>> Signed-off-by: Simon Xue <xxm@rock-chips.com>
>>>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>>>> ---
>>>>    drivers/pci/controller/dwc/Kconfig            |  10 +
>>>>    drivers/pci/controller/dwc/Makefile           |   1 +
>>>>    drivers/pci/controller/dwc/pcie-dw-rockchip.c | 277 ++++++++++++++++++
>>>>    3 files changed, 288 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..8ab027ba8c04 100644
>>>> --- a/drivers/pci/controller/dwc/Kconfig
>>>> +++ b/drivers/pci/controller/dwc/Kconfig
>>>> @@ -214,6 +214,16 @@ 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 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 952d01941f23..0104659dfe88 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);
>>>> +}
>>>> +
>>>> +static void rockchip_pcie_writel_apb(struct rockchip_pcie *rockchip,
>>>> +                                               u32 val, u32 reg)
>>>> +{
>>>> +       writel(val, rockchip->apb_base + reg);
>>>> +}
>>>> +
>>>> +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 & GENMASK(5, 0)) == PCIE_L0S_ENTRY)
>>>> +               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);
>>>> +
>>>> +       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);
>>>> +       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
>>>>
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> Linux-rockchip mailing list
>>>> Linux-rockchip@lists.infradead.org
>>>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>>>
>>>
>>>
>>
>>
> 
> 
>
Christoph Hellwig April 29, 2021, 6:41 a.m. UTC | #5
On Wed, Apr 28, 2021 at 09:42:37PM -0400, Peter Geis wrote:
> I have functional MSI support, some devices do not support MSIs
> however and need legacy INTx.
> I'd like to point out that the downstream patch does not actually work
> on downstream.
> The GFP_DMA32 flag is discarded by the slab allocator, this breaks MSI
> allocation when the PCIe driver probes.
> I hacked together my own version which works but would definitely not
> be accepted for submission.

Seriously folks.  Never, ever use GFP_DMA or GFP_DMA32 in actual driver
code.  They fundamentally don't do what you want.  Devices as a rule of
thumb do care about DMA addressability, not CPU addressability, so they
must use the DMA API to allocate the memory, and the dma mask to control
addressability.
Peter Geis April 29, 2021, 11:21 a.m. UTC | #6
On Thu, Apr 29, 2021 at 2:32 AM Shawn Lin <shawn.lin@rock-chips.com> wrote:
>
> On 2021/4/29 9:42, Peter Geis wrote:
> > On Wed, Apr 28, 2021 at 8:46 PM Shawn Lin <shawn.lin@rock-chips.com> wrote:
> >>
> >> On 2021/4/29 0:20, Peter Geis wrote:
> >>> On Wed, Apr 14, 2021 at 3:05 AM 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.
> >>>
> >>> Good Afternoon,
> >>>
> >>> I've encountered a bit of an issue with this driver.
> >>
> >> I'm sure your issue is GIC ITS support on RK356x platforms, nothing
> >> really related to this driver itself. You can also sort out arm-gic
> >> workaround patch from downstream vendor tree, if you want MSI support.
> >
> > I have functional MSI support, some devices do not support MSIs
> > however and need legacy INTx.
> > I'd like to point out that the downstream patch does not actually work
> > on downstream.
> > The GFP_DMA32 flag is discarded by the slab allocator, this breaks MSI
> > allocation when the PCIe driver probes.
> > I hacked together my own version which works but would definitely not
> > be accepted for submission.
> >
> >>
> >>> Unfortunately it does not support legacy interrupts, meaning any PCIe
> >>> device that doesn't support MSIs will fail to enumerate:
> >>> [   14.932078] pcieport 0000:00:00.0: of_irq_parse_pci: failed with rc=-22
> >>> [   14.932708] snd_hda_intel 0000:01:00.1: assign IRQ: got 0
> >>> [   14.933687] snd_hda_intel 0000:01:00.1: enabling device (0000 -> 0002)
> >>> [   14.934317] snd_hda_intel 0000:01:00.1: Disabling MSI
> >>> [   14.934783] snd_hda_intel 0000:01:00.1: Force to snoop mode by module option
> >>> [   14.935534] snd_hda_intel 0000:01:00.1: enabling bus mastering
> >>> [   14.939764] snd_hda_intel 0000:01:00.1: unable to grab IRQ 0,
> >>> disabling device
> >>>
> >>> Are there plans to support legacy interrupts with the rk3568 controllers?
> >>
> >> Yes, we did,but not now.
> >
> > This will significantly limit the devices that can work with the controller.
> > Is there any functional reason preventing legacy interrupts from working?
>
> Just because this $subject patch is still in-flight and we are going to
> add it later. Still some features are developed but not ready yet.

I now have partial INTx functionality, but it seems there is no way to
acknowledge the interrupt only mask and unmask them.
Is there a register to acknowledge the legacy interrupts?

>
>
> > I've so far not been successful plugging in support.
> >
> >>
> >> I really don't understand what is blocking this driver from landing
> >> mainline?  I didn't find any comments from maintainers yet since v1. And
> >> now the driver has been iterating for a long time.I believe Simon has
> >> addressed all the review feedback during v1 to v7. Can we please land
> >> this basic support first. If you can test it with arm-gic workaround,
> >> a tested-by tag would be appreciated.
> >
> > Do you have the DT bindings for this patch available?
>
> Yes we have, and you can also find it from vendor tree.

New drivers should include .yaml documentation files.
Downstream does not have one, and one was not included with this patch.
The designware-pcie.txt doesn't cover vendor specific implementations.

See Documentation/devicetree/bindings/pci/intel-gw-pcie.yaml for example.
>
> >
> > Yes, at this stage what is implemented appears to function correctly, so:
> > Tested-by: Peter Geis <pgwipeout@gmail.com>
> > on the rk3566-quartz64 prototype development board.
>
> Thanks for testing.
>
> >
> > Dependent on the following patches:
> > GICv3-ITS official errata for 32bit limitation and shareability limitation.
> > Combo-phy support, no patches have been released to mainline yet >
> >>
> >>>
> >>> Thanks,
> >>> Peter Geis
> >>>
> >>>>
> >>>> Signed-off-by: Simon Xue <xxm@rock-chips.com>
> >>>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> >>>> ---
> >>>>    drivers/pci/controller/dwc/Kconfig            |  10 +
> >>>>    drivers/pci/controller/dwc/Makefile           |   1 +
> >>>>    drivers/pci/controller/dwc/pcie-dw-rockchip.c | 277 ++++++++++++++++++
> >>>>    3 files changed, 288 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..8ab027ba8c04 100644
> >>>> --- a/drivers/pci/controller/dwc/Kconfig
> >>>> +++ b/drivers/pci/controller/dwc/Kconfig
> >>>> @@ -214,6 +214,16 @@ 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 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 952d01941f23..0104659dfe88 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);
> >>>> +}
> >>>> +
> >>>> +static void rockchip_pcie_writel_apb(struct rockchip_pcie *rockchip,
> >>>> +                                               u32 val, u32 reg)
> >>>> +{
> >>>> +       writel(val, rockchip->apb_base + reg);
> >>>> +}
> >>>> +
> >>>> +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 & GENMASK(5, 0)) == PCIE_L0S_ENTRY)
> >>>> +               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);
> >>>> +
> >>>> +       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);
> >>>> +       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
> >>>>
> >>>>
> >>>>
> >>>>
> >>>> _______________________________________________
> >>>> Linux-rockchip mailing list
> >>>> Linux-rockchip@lists.infradead.org
> >>>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
> >>>
> >>>
> >>>
> >>
> >>
> >
> >
> >
>
>
Peter Geis April 29, 2021, 12:16 p.m. UTC | #7
On Thu, Apr 29, 2021 at 7:55 AM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Thu, Apr 29, 2021 at 07:26:40AM -0400, Peter Geis wrote:
> > Good Morning! I'm trying to implement the MSI workaround in as sane a
> > method as possible.
> > Do you have a better way to have the kernel only allocate memory in
> > the lower regions?
> > (Without disabling everything above 4G altogether)
> > This chip only supports a maximum of 8G.
> > (I know, why they did it while maintaining a 32bit bus is beyond me).
>
>
> Please use dma_alloc_coherent to allocate the memory, the dma_addr_t
> return value is the address to be fed to the hardware.  Before doing
> that set the desired mask.  DMA_BIT_MASK(33) would get your 8GB.

Thanks!
Unfortunately this isn't actually DMA allocation, they were using
GFP_DMA32 as a hack to allocate regular kernel memory in the 32 bit
range.
It's in drivers/irqchip/irq-gic-v3-its.c for reference.
The functions are simply kcalloc(), kzalloc(), kzalloc_node(),
alloc_pages_node(), and alloc_pages().
I'd prefer not to have to rewrite this driver's entire memory
allocation system for one errata.

I'm following the code path for dma_alloc_coherent to see if anything
sticks out to me though.
diff mbox series

Patch

diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index 423d35872ce4..8ab027ba8c04 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -214,6 +214,16 @@  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 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 952d01941f23..0104659dfe88 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);
+}
+
+static void rockchip_pcie_writel_apb(struct rockchip_pcie *rockchip,
+						u32 val, u32 reg)
+{
+	writel(val, rockchip->apb_base + reg);
+}
+
+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 & GENMASK(5, 0)) == PCIE_L0S_ENTRY)
+		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);
+
+	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);
+	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);