2020-08-21 09:53:22

by Hongtao Wu

[permalink] [raw]
Subject: [PATCH 2/2] PCI: sprd: Add support for Unisoc SoCs' PCIe controller

From: Billows Wu <[email protected]>

This series adds PCIe controller driver for Unisoc SoCs.
This controller is based on DesignWare PCIe IP.

Signed-off-by: Billows Wu <[email protected]>
---
drivers/pci/controller/dwc/Kconfig | 12 ++
drivers/pci/controller/dwc/Makefile | 1 +
drivers/pci/controller/dwc/pcie-sprd.c | 256 +++++++++++++++++++++++++++++++++
3 files changed, 269 insertions(+)
create mode 100644 drivers/pci/controller/dwc/pcie-sprd.c

diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index 044a376..d26ce94 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -311,4 +311,16 @@ config PCIE_AL
required only for DT-based platforms. ACPI platforms with the
Annapurna Labs PCIe controller don't need to enable this.

+config PCIE_SPRD
+ tristate "Unisoc PCIe controller - RC mode"
+ depends on ARCH_SPRD
+ depends on PCI_MSI_IRQ_DOMAIN
+ select PCIE_DW_HOST
+ help
+ Some Uisoc SoCs contain two PCIe controllers as RC: One is gen2,
+ and the other is gen3. While other Unisoc SoCs may have only one
+ PCIe controller which can be configured as an Endpoint(EP) or a Root
+ complex(RC). In order to enable host-specific features PCIE_SPRD must
+ be selected, which uses the Designware core.
+
endmenu
diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
index a751553..eb546e9 100644
--- a/drivers/pci/controller/dwc/Makefile
+++ b/drivers/pci/controller/dwc/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_PCI_MESON) += pci-meson.o
obj-$(CONFIG_PCIE_TEGRA194) += pcie-tegra194.o
obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o
obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o
+obj-$(CONFIG_PCIE_SPRD) += pcie-sprd.o

# The following drivers are for devices that use the generic ACPI
# pci_root.c driver but don't support standard ECAM config access.
diff --git a/drivers/pci/controller/dwc/pcie-sprd.c b/drivers/pci/controller/dwc/pcie-sprd.c
new file mode 100644
index 0000000..cda812d
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-sprd.c
@@ -0,0 +1,256 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * PCIe host controller driver for Unisoc SoCs
+ *
+ * Copyright (C) 2020 Unisoc corporation. http://www.unisoc.com
+ *
+ * Author: Billows Wu <[email protected]>
+ */
+
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+
+#include "pcie-designware.h"
+
+#define NUM_OF_ARGS 5
+
+struct sprd_pcie {
+ struct dw_pcie *pci;
+};
+
+struct sprd_pcie_of_data {
+ enum dw_pcie_device_mode mode;
+};
+
+static int sprd_pcie_establish_link(struct dw_pcie *pci)
+{
+ return 0;
+}
+
+static const struct dw_pcie_ops sprd_pcie_ops = {
+ .start_link = sprd_pcie_establish_link,
+};
+
+int sprd_pcie_syscon_setting(struct platform_device *pdev, char *env)
+{
+ struct device_node *np = pdev->dev.of_node;
+ int i, count, err;
+ u32 type, delay, reg, mask, val, tmp_val;
+ struct of_phandle_args out_args;
+ struct regmap *iomap;
+ struct device *dev = &pdev->dev;
+
+ if (!of_find_property(np, env, NULL)) {
+ dev_info(dev, "There isn't property %s in dts\n", env);
+ return 0;
+ }
+
+ count = of_property_count_elems_of_size(np, env,
+ (NUM_OF_ARGS + 1) * sizeof(u32));
+ dev_info(dev, "Property (%s) reg count is %d :\n", env, count);
+
+ for (i = 0; i < count; i++) {
+ err = of_parse_phandle_with_fixed_args(np, env, NUM_OF_ARGS,
+ i, &out_args);
+ if (err < 0)
+ return err;
+
+ type = out_args.args[0];
+ delay = out_args.args[1];
+ reg = out_args.args[2];
+ mask = out_args.args[3];
+ val = out_args.args[4];
+
+ iomap = syscon_node_to_regmap(out_args.np);
+
+ switch (type) {
+ case 0:
+ regmap_update_bits(iomap, reg, mask, val);
+ break;
+
+ case 1:
+ regmap_read(iomap, reg, &tmp_val);
+ tmp_val &= (~mask);
+ tmp_val |= (val & mask);
+ regmap_write(iomap, reg, tmp_val);
+ break;
+ default:
+ break;
+ }
+
+ if (delay)
+ usleep_range(delay, delay + 10);
+
+ regmap_read(iomap, reg, &tmp_val);
+ dev_dbg(&pdev->dev,
+ "%2d:reg[0x%8x] mask[0x%8x] val[0x%8x] result[0x%8x]\n",
+ i, reg, mask, val, tmp_val);
+ }
+
+ return i;
+}
+
+static void sprd_pcie_perst_assert(struct platform_device *pdev)
+{
+ sprd_pcie_syscon_setting(pdev, "sprd,pcie-perst-assert");
+}
+
+static void sprd_pcie_perst_deassert(struct platform_device *pdev)
+{
+ sprd_pcie_syscon_setting(pdev, "sprd,pcie-perst-deassert");
+}
+
+static int sprd_pcie_host_shutdown(struct platform_device *pdev)
+{
+ int ret;
+ struct device *dev = &pdev->dev;
+
+ ret = sprd_pcie_syscon_setting(pdev, "sprd,pcie-shutdown-syscons");
+ if (ret < 0)
+ dev_err(dev,
+ "Failed to set pcie shutdown syscons, return %d\n",
+ ret);
+
+ sprd_pcie_perst_assert(pdev);
+
+ ret = pm_runtime_put(&pdev->dev);
+ if (ret < 0)
+ dev_warn(&pdev->dev,
+ "Failed to put runtime,return %d\n", ret);
+
+ return ret;
+}
+
+static int sprd_pcie_host_init(struct pcie_port *pp)
+{
+ struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+ struct platform_device *pdev = to_platform_device(pci->dev);
+
+ sprd_pcie_perst_deassert(pdev);
+
+ dw_pcie_setup_rc(pp);
+
+ if (IS_ENABLED(CONFIG_PCI_MSI))
+ dw_pcie_msi_init(pp);
+
+ if (dw_pcie_wait_for_link(pci)) {
+ dev_warn(pci->dev,
+ "pcie ep may has not been powered on yet\n");
+ sprd_pcie_host_shutdown(pdev);
+ }
+
+ return 0;
+}
+
+static const struct dw_pcie_host_ops sprd_pcie_host_ops = {
+ .host_init = sprd_pcie_host_init,
+};
+
+static int sprd_add_pcie_port(struct platform_device *pdev)
+{
+ struct resource *res;
+ struct device *dev = &pdev->dev;
+ struct sprd_pcie *ctrl = platform_get_drvdata(pdev);
+ struct dw_pcie *pci = ctrl->pci;
+ struct pcie_port *pp = &pci->pp;
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
+ if (!res)
+ return -EINVAL;
+
+ pci->dbi_base = devm_ioremap(dev, res->start, resource_size(res));
+ if (!pci->dbi_base)
+ return -ENOMEM;
+
+ pp->ops = &sprd_pcie_host_ops;
+
+ if (IS_ENABLED(CONFIG_PCI_MSI)) {
+ pp->msi_irq = platform_get_irq_byname(pdev, "msi");
+ if (pp->msi_irq < 0) {
+ dev_err(dev, "Failed to get msi, return %d\n",
+ pp->msi_irq);
+ return pp->msi_irq;
+ }
+ }
+
+ return dw_pcie_host_init(pp);
+}
+
+static int sprd_pcie_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct dw_pcie *pci;
+ struct sprd_pcie *ctrl;
+ int ret;
+
+ ctrl = devm_kzalloc(dev, sizeof(*ctrl), GFP_KERNEL);
+ if (!ctrl)
+ return -ENOMEM;
+
+ pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
+ if (!pci)
+ return -ENOMEM;
+
+ pci->dev = dev;
+ pci->ops = &sprd_pcie_ops;
+ ctrl->pci = pci;
+
+ platform_set_drvdata(pdev, ctrl);
+
+ pm_runtime_enable(dev);
+ ret = pm_runtime_get_sync(dev);
+ if (ret < 0) {
+ dev_err(dev, "Fialed to get runtime sync, return %d\n", ret);
+ goto err_get_sync;
+ }
+
+ ret = sprd_pcie_syscon_setting(pdev, "sprd,pcie-startup-syscons");
+ if (ret < 0) {
+ dev_err(dev, "Failed to get pcie syscons, return %d\n", ret);
+ goto err_power_off;
+ }
+
+ ret = sprd_add_pcie_port(pdev);
+ if (ret)
+ dev_warn(dev, "Failed to initialize RC controller\n");
+
+ return 0;
+
+err_power_off:
+ sprd_pcie_syscon_setting(pdev, "sprd,pcie-shutdown-syscons");
+
+err_get_sync:
+ pm_runtime_put(&pdev->dev);
+ pm_runtime_disable(dev);
+
+ return ret;
+}
+
+static const struct of_device_id sprd_pcie_of_match[] = {
+ {
+ .compatible = "sprd,pcie",
+ },
+ {},
+};
+
+static struct platform_driver sprd_pcie_driver = {
+ .probe = sprd_pcie_probe,
+ .driver = {
+ .name = "sprd-pcie",
+ .of_match_table = sprd_pcie_of_match,
+ },
+};
+
+module_platform_driver(sprd_pcie_driver);
+
+MODULE_DESCRIPTION("Unisoc PCIe host controller driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4


2020-08-21 13:12:27

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 2/2] PCI: sprd: Add support for Unisoc SoCs' PCIe controller

Hi Hongtao,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on pci/next]
[also build test WARNING on robh/for-next v5.9-rc1 next-20200821]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Hongtao-Wu/PCI-Add-new-Unisoc-PCIe-driver/20200821-175351
base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: arm64-allyesconfig (attached as .config)
compiler: aarch64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm64

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

>> drivers/pci/controller/dwc/pcie-sprd.c:43:5: warning: no previous prototype for 'sprd_pcie_syscon_setting' [-Wmissing-prototypes]
43 | int sprd_pcie_syscon_setting(struct platform_device *pdev, char *env)
| ^~~~~~~~~~~~~~~~~~~~~~~~

# https://github.com/0day-ci/linux/commit/b82baaf6b6b4a04032d29dd7b749d1f9c4a23104
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Hongtao-Wu/PCI-Add-new-Unisoc-PCIe-driver/20200821-175351
git checkout b82baaf6b6b4a04032d29dd7b749d1f9c4a23104
vim +/sprd_pcie_syscon_setting +43 drivers/pci/controller/dwc/pcie-sprd.c

42
> 43 int sprd_pcie_syscon_setting(struct platform_device *pdev, char *env)
44 {
45 struct device_node *np = pdev->dev.of_node;
46 int i, count, err;
47 u32 type, delay, reg, mask, val, tmp_val;
48 struct of_phandle_args out_args;
49 struct regmap *iomap;
50 struct device *dev = &pdev->dev;
51
52 if (!of_find_property(np, env, NULL)) {
53 dev_info(dev, "There isn't property %s in dts\n", env);
54 return 0;
55 }
56
57 count = of_property_count_elems_of_size(np, env,
58 (NUM_OF_ARGS + 1) * sizeof(u32));
59 dev_info(dev, "Property (%s) reg count is %d :\n", env, count);
60
61 for (i = 0; i < count; i++) {
62 err = of_parse_phandle_with_fixed_args(np, env, NUM_OF_ARGS,
63 i, &out_args);
64 if (err < 0)
65 return err;
66
67 type = out_args.args[0];
68 delay = out_args.args[1];
69 reg = out_args.args[2];
70 mask = out_args.args[3];
71 val = out_args.args[4];
72
73 iomap = syscon_node_to_regmap(out_args.np);
74
75 switch (type) {
76 case 0:
77 regmap_update_bits(iomap, reg, mask, val);
78 break;
79
80 case 1:
81 regmap_read(iomap, reg, &tmp_val);
82 tmp_val &= (~mask);
83 tmp_val |= (val & mask);
84 regmap_write(iomap, reg, tmp_val);
85 break;
86 default:
87 break;
88 }
89
90 if (delay)
91 usleep_range(delay, delay + 10);
92
93 regmap_read(iomap, reg, &tmp_val);
94 dev_dbg(&pdev->dev,
95 "%2d:reg[0x%8x] mask[0x%8x] val[0x%8x] result[0x%8x]\n",
96 i, reg, mask, val, tmp_val);
97 }
98
99 return i;
100 }
101

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (4.22 kB)
.config.gz (72.41 kB)
Download all attachments

2020-08-21 15:19:32

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH 2/2] PCI: sprd: Add support for Unisoc SoCs' PCIe controller

On 8/21/20 2:51 AM, Hongtao Wu wrote:
> From: Billows Wu <[email protected]>
>
> This series adds PCIe controller driver for Unisoc SoCs.
> This controller is based on DesignWare PCIe IP.
>
> Signed-off-by: Billows Wu <[email protected]>
> ---
> drivers/pci/controller/dwc/Kconfig | 12 ++
> drivers/pci/controller/dwc/Makefile | 1 +
> drivers/pci/controller/dwc/pcie-sprd.c | 256 +++++++++++++++++++++++++++++++++
> 3 files changed, 269 insertions(+)
> create mode 100644 drivers/pci/controller/dwc/pcie-sprd.c

Hi,

> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index 044a376..d26ce94 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -311,4 +311,16 @@ config PCIE_AL
> required only for DT-based platforms. ACPI platforms with the
> Annapurna Labs PCIe controller don't need to enable this.
>
> +config PCIE_SPRD
> + tristate "Unisoc PCIe controller - RC mode"
> + depends on ARCH_SPRD
> + depends on PCI_MSI_IRQ_DOMAIN
> + select PCIE_DW_HOST
> + help
> + Some Uisoc SoCs contain two PCIe controllers as RC: One is gen2,

Unisoc

> + and the other is gen3. While other Unisoc SoCs may have only one
> + PCIe controller which can be configured as an Endpoint(EP) or a Root
> + complex(RC). In order to enable host-specific features PCIE_SPRD must

complex (RC).

> + be selected, which uses the Designware core.
> +
> endmenu

Also, please follow Documentation/process/coding-style.rst for
Kconfig entries:

For all of the Kconfig* configuration files throughout the source tree,
the indentation is somewhat different. Lines under a ``config`` definition
are indented with one tab, while help text is indented an additional two
spaces.


--
~Randy

2020-08-21 22:25:05

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH 2/2] PCI: sprd: Add support for Unisoc SoCs' PCIe controller

On Fri, Aug 21, 2020 at 3:52 AM Hongtao Wu <[email protected]> wrote:
>
> From: Billows Wu <[email protected]>
>
> This series adds PCIe controller driver for Unisoc SoCs.
> This controller is based on DesignWare PCIe IP.

Please test this on top of my 40 part series of DWC clean-ups:

https://lore.kernel.org/linux-pci/[email protected]/

> Signed-off-by: Billows Wu <[email protected]>
> ---
> drivers/pci/controller/dwc/Kconfig | 12 ++
> drivers/pci/controller/dwc/Makefile | 1 +
> drivers/pci/controller/dwc/pcie-sprd.c | 256 +++++++++++++++++++++++++++++++++
> 3 files changed, 269 insertions(+)
> create mode 100644 drivers/pci/controller/dwc/pcie-sprd.c
>
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index 044a376..d26ce94 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -311,4 +311,16 @@ config PCIE_AL
> required only for DT-based platforms. ACPI platforms with the
> Annapurna Labs PCIe controller don't need to enable this.
>
> +config PCIE_SPRD
> + tristate "Unisoc PCIe controller - RC mode"
> + depends on ARCH_SPRD

|| COMPILE_TEST

> + depends on PCI_MSI_IRQ_DOMAIN
> + select PCIE_DW_HOST
> + help
> + Some Uisoc SoCs contain two PCIe controllers as RC: One is gen2,
> + and the other is gen3. While other Unisoc SoCs may have only one
> + PCIe controller which can be configured as an Endpoint(EP) or a Root
> + complex(RC). In order to enable host-specific features PCIE_SPRD must
> + be selected, which uses the Designware core.
> +
> endmenu
> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> index a751553..eb546e9 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -20,6 +20,7 @@ obj-$(CONFIG_PCI_MESON) += pci-meson.o
> obj-$(CONFIG_PCIE_TEGRA194) += pcie-tegra194.o
> obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o
> obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o
> +obj-$(CONFIG_PCIE_SPRD) += pcie-sprd.o
>
> # The following drivers are for devices that use the generic ACPI
> # pci_root.c driver but don't support standard ECAM config access.
> diff --git a/drivers/pci/controller/dwc/pcie-sprd.c b/drivers/pci/controller/dwc/pcie-sprd.c
> new file mode 100644
> index 0000000..cda812d
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-sprd.c
> @@ -0,0 +1,256 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * PCIe host controller driver for Unisoc SoCs
> + *
> + * Copyright (C) 2020 Unisoc corporation. http://www.unisoc.com
> + *
> + * Author: Billows Wu <[email protected]>
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/interrupt.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/of_irq.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +
> +#include "pcie-designware.h"
> +
> +#define NUM_OF_ARGS 5
> +
> +struct sprd_pcie {
> + struct dw_pcie *pci;
> +};
> +
> +struct sprd_pcie_of_data {
> + enum dw_pcie_device_mode mode;
> +};
> +
> +static int sprd_pcie_establish_link(struct dw_pcie *pci)
> +{
> + return 0;
> +}
> +
> +static const struct dw_pcie_ops sprd_pcie_ops = {
> + .start_link = sprd_pcie_establish_link,

I don't think you need an empty function here.

> +};
> +
> +int sprd_pcie_syscon_setting(struct platform_device *pdev, char *env)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + int i, count, err;
> + u32 type, delay, reg, mask, val, tmp_val;
> + struct of_phandle_args out_args;
> + struct regmap *iomap;
> + struct device *dev = &pdev->dev;
> +
> + if (!of_find_property(np, env, NULL)) {
> + dev_info(dev, "There isn't property %s in dts\n", env);
> + return 0;
> + }
> +
> + count = of_property_count_elems_of_size(np, env,
> + (NUM_OF_ARGS + 1) * sizeof(u32));
> + dev_info(dev, "Property (%s) reg count is %d :\n", env, count);
> +
> + for (i = 0; i < count; i++) {
> + err = of_parse_phandle_with_fixed_args(np, env, NUM_OF_ARGS,
> + i, &out_args);
> + if (err < 0)
> + return err;
> +
> + type = out_args.args[0];
> + delay = out_args.args[1];
> + reg = out_args.args[2];
> + mask = out_args.args[3];
> + val = out_args.args[4];
> +
> + iomap = syscon_node_to_regmap(out_args.np);
> +
> + switch (type) {
> + case 0:
> + regmap_update_bits(iomap, reg, mask, val);
> + break;
> +
> + case 1:
> + regmap_read(iomap, reg, &tmp_val);
> + tmp_val &= (~mask);
> + tmp_val |= (val & mask);
> + regmap_write(iomap, reg, tmp_val);
> + break;
> + default:
> + break;
> + }
> +
> + if (delay)
> + usleep_range(delay, delay + 10);
> +
> + regmap_read(iomap, reg, &tmp_val);
> + dev_dbg(&pdev->dev,
> + "%2d:reg[0x%8x] mask[0x%8x] val[0x%8x] result[0x%8x]\n",
> + i, reg, mask, val, tmp_val);
> + }
> +
> + return i;
> +}
> +
> +static void sprd_pcie_perst_assert(struct platform_device *pdev)
> +{
> + sprd_pcie_syscon_setting(pdev, "sprd,pcie-perst-assert");

Not documented.

> +}
> +
> +static void sprd_pcie_perst_deassert(struct platform_device *pdev)
> +{
> + sprd_pcie_syscon_setting(pdev, "sprd,pcie-perst-deassert");

These 2 can be a single property.

> +}
> +
> +static int sprd_pcie_host_shutdown(struct platform_device *pdev)
> +{
> + int ret;
> + struct device *dev = &pdev->dev;
> +
> + ret = sprd_pcie_syscon_setting(pdev, "sprd,pcie-shutdown-syscons");

Not documented.

> + if (ret < 0)
> + dev_err(dev,
> + "Failed to set pcie shutdown syscons, return %d\n",
> + ret);
> +
> + sprd_pcie_perst_assert(pdev);
> +
> + ret = pm_runtime_put(&pdev->dev);
> + if (ret < 0)
> + dev_warn(&pdev->dev,
> + "Failed to put runtime,return %d\n", ret);
> +
> + return ret;
> +}
> +
> +static int sprd_pcie_host_init(struct pcie_port *pp)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> + struct platform_device *pdev = to_platform_device(pci->dev);
> +
> + sprd_pcie_perst_deassert(pdev);
> +
> + dw_pcie_setup_rc(pp);
> +
> + if (IS_ENABLED(CONFIG_PCI_MSI))

You won't need this check anymore with my series.

> + dw_pcie_msi_init(pp);
> +
> + if (dw_pcie_wait_for_link(pci)) {
> + dev_warn(pci->dev,
> + "pcie ep may has not been powered on yet\n");
> + sprd_pcie_host_shutdown(pdev);
> + }
> +
> + return 0;
> +}
> +
> +static const struct dw_pcie_host_ops sprd_pcie_host_ops = {
> + .host_init = sprd_pcie_host_init,
> +};
> +
> +static int sprd_add_pcie_port(struct platform_device *pdev)
> +{
> + struct resource *res;
> + struct device *dev = &pdev->dev;
> + struct sprd_pcie *ctrl = platform_get_drvdata(pdev);
> + struct dw_pcie *pci = ctrl->pci;
> + struct pcie_port *pp = &pci->pp;
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");

The core will do this for you.

> + if (!res)
> + return -EINVAL;
> +
> + pci->dbi_base = devm_ioremap(dev, res->start, resource_size(res));
> + if (!pci->dbi_base)
> + return -ENOMEM;
> +
> + pp->ops = &sprd_pcie_host_ops;
> +
> + if (IS_ENABLED(CONFIG_PCI_MSI)) {
> + pp->msi_irq = platform_get_irq_byname(pdev, "msi");
> + if (pp->msi_irq < 0) {
> + dev_err(dev, "Failed to get msi, return %d\n",
> + pp->msi_irq);
> + return pp->msi_irq;
> + }
> + }
> +
> + return dw_pcie_host_init(pp);
> +}
> +
> +static int sprd_pcie_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct dw_pcie *pci;
> + struct sprd_pcie *ctrl;
> + int ret;
> +
> + ctrl = devm_kzalloc(dev, sizeof(*ctrl), GFP_KERNEL);
> + if (!ctrl)
> + return -ENOMEM;
> +
> + pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);

Embed dw_pcie in sprd_pcie and then have a single alloc.

> + if (!pci)
> + return -ENOMEM;
> +
> + pci->dev = dev;
> + pci->ops = &sprd_pcie_ops;
> + ctrl->pci = pci;
> +
> + platform_set_drvdata(pdev, ctrl);
> +
> + pm_runtime_enable(dev);
> + ret = pm_runtime_get_sync(dev);

I don't think these do anything because you don't have any
suspend/resume callbacks.

> + if (ret < 0) {
> + dev_err(dev, "Fialed to get runtime sync, return %d\n", ret);
> + goto err_get_sync;
> + }
> +
> + ret = sprd_pcie_syscon_setting(pdev, "sprd,pcie-startup-syscons");

I don't like all these properties. Do they point to different blocks?
If all the same block, then 1 property please.

Maybe you need a reset controller?

> + if (ret < 0) {
> + dev_err(dev, "Failed to get pcie syscons, return %d\n", ret);
> + goto err_power_off;
> + }
> +
> + ret = sprd_add_pcie_port(pdev);
> + if (ret)
> + dev_warn(dev, "Failed to initialize RC controller\n");
> +
> + return 0;
> +
> +err_power_off:
> + sprd_pcie_syscon_setting(pdev, "sprd,pcie-shutdown-syscons");
> +
> +err_get_sync:
> + pm_runtime_put(&pdev->dev);
> + pm_runtime_disable(dev);
> +
> + return ret;
> +}
> +
> +static const struct of_device_id sprd_pcie_of_match[] = {
> + {
> + .compatible = "sprd,pcie",
> + },
> + {},
> +};
> +
> +static struct platform_driver sprd_pcie_driver = {
> + .probe = sprd_pcie_probe,
> + .driver = {
> + .name = "sprd-pcie",
> + .of_match_table = sprd_pcie_of_match,
> + },
> +};
> +
> +module_platform_driver(sprd_pcie_driver);
> +
> +MODULE_DESCRIPTION("Unisoc PCIe host controller driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.7.4
>