Skip to contentSkip to conversation

PCI: Add new Unisoc PCIe driver

PATCH RESEND Superseded Hongtao Wu 2 participants 8 messages Jan 14, 2021

Subsystems (4)
Device tree bindings PCI PCI native host bridge and endpoint drivers Arm/spreadtrum SoC support

v5 of 6 Superseded Latest is v6 →

lore
Share & tools
lore

Hongtao Wu

author
#0 lore raw

v5 cover PCI: Add new Unisoc PCIe driver

From: Hongtao Wu <[email protected]>

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

Changes from v1:
1) Test this patch on top of Rob Herring's 40 part series of DWC clean-ups:
https://lore.kernel.org/linux-pci/[email protected]/

2) Delete empty function

3) Document property "sprd,pcie-poweron-syscons" and 'sprd,pcie-poweroff-syscons'

4) Delete runtime suspend/resume function

5) Add COMPILE_TEST which CONFIG_PCIE_SPRD depends on

Changes from v2:
1) Change RC mode to host mode in drivers/pci/controller/dwc/Kconfig

2) Change Signed-off-by from Billows Wu to Hongtao Wu

Changes from v3:
1) Split the property 'sprd,pcie-poweron-syscons' and 'sprd,pcie-poweroff-syscons' into reset, power domains, phy and so on.

2) Delete the function to get resource 'msi' and 'dbi' which were parsed by the DW core.

3) Delete the function 'sprd_pcie_host_init', because the DW core has done it.

Changes from v4:
1) Install 'yamllint' and upgrade dt-schema in order to solve the yamllint and dtschema/dtc warnings/errors.

Hongtao Wu (2):
dt-bindings: PCI: sprd: Document Unisoc PCIe RC host controller
PCI: sprd: Add support for Unisoc SoCs' PCIe controller

 .../devicetree/bindings/pci/sprd-pcie.yaml         |  93 +++++++
 drivers/pci/controller/dwc/Kconfig                 |  12 +
 drivers/pci/controller/dwc/Makefile                |   1 +
 drivers/pci/controller/dwc/pcie-sprd.c             | 293 +++++++++++++++++++++
 4 files changed, 399 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/sprd-pcie.yaml
 create mode 100644 drivers/pci/controller/dwc/pcie-sprd.c

--
2.7.4

author
#1 lore raw

PATCH RESEND v5 1/2 dt-bindings: PCI: sprd: Document Unisoc PCIe RC host controller

From: Hongtao Wu <[email protected]>

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

Reviewed-by Rob Herring <[email protected]>
Signed-off-by Hongtao Wu <[email protected]>
---
 .../devicetree/bindings/pci/sprd-pcie.yaml         | 93 ++++++++++++++++++++++
 1 file changed, 93 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/sprd-pcie.yaml
Documentation/devicetree/bindings/pci/sprd-pcie.yaml new file +93 -0
new file mode 100644
index 0000000..ede06a8
@@ -0,0 +1,93 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/sprd-pcie.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: SoC PCIe Host Controller Device Tree Bindings
+
+maintainers:
+  - Hongtao Wu <[email protected]>
+
+allOf:
+  - $ref: /schemas/pci/pci-bus.yaml#
+
+properties:
+  compatible:
+    items:
+      - const: sprd,ums9520-pcie
+
+  reg:
+    minItems: 2
+    items:
+      - description: Controller control and status registers.
+      - description: PCIe configuration registers.
+
+  reg-names:
+    items:
+      - const: dbi
+      - const: config
+
+  ranges:
+    maxItems: 2
+
+  num-lanes:
+    maximum: 1
+    description: Number of lanes to use for this port.
+
+  interrupts:
+    minItems: 1
+    description: Builtin MSI controller and PCIe host controller.
+
+  interrupt-names:
+    items:
+      - const: msi
+
+  sprd,regmap-aon:
+    $ref: '/schemas/types.yaml#/definitions/phandle'
+    description:
+      Phandle to the AON system controller node (to access the
+      AON_ACCESS_PCIE_EN register on ums9520).
+  sprd,regmap-pmu:
+    $ref: '/schemas/types.yaml#/definitions/phandle'
+    description:
+      Phandle to the PMU system controller node (to access the PERST_N_ASSERT
+      register on ums9520).
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - num-lanes
+  - ranges
+  - interrupts
+  - interrupt-names
+
+additionalProperties: true
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    ipa {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        pcie0: pcie@2b100000 {
+            compatible = "sprd,ums9520-pcie";
+            reg = <0x0 0x2b100000 0x0 0x2000>,
+                  <0x2 0x00000000 0x0 0x2000>;
+            reg-names = "dbi", "config";
+            #address-cells = <3>;
+            #size-cells = <2>;
+            device_type = "pci";
+            ranges = <0x01000000 0x0 0x00000000 0x2 0x00002000 0x0 0x00010000>,
+                     <0x03000000 0x0 0x10000000 0x2 0x10000000 0x1 0xefffffff>;
+            num-lanes = <1>;
+            interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>;
+            interrupt-names = "msi";
+
+            sprd,regmap-aon = <&aon_regs>;
+            sprd,regmap-pmu = <&pmu_regs>;
+        };
+    };

--
2.7.4

author
#2 lore raw

PATCH RESEND v5 2/2 PCI: sprd: Add support for Unisoc SoCs' PCIe controller

In reply to Hongtao Wu

From: Hongtao Wu <[email protected]>

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

Signed-off-by Hongtao Wu <[email protected]>
---
 drivers/pci/controller/dwc/Kconfig     |  12 ++
 drivers/pci/controller/dwc/Makefile    |   1 +
 drivers/pci/controller/dwc/pcie-sprd.c | 293 +++++++++++++++++++++++++++++++++
 3 files changed, 306 insertions(+)
 create mode 100644 drivers/pci/controller/dwc/pcie-sprd.c
drivers/pci/controller/dwc/Kconfig +12 -0
index 22c5529..61f0b79 100644
@@ -318,4 +318,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 - Host Mode"
+	depends on ARCH_SPRD || COMPILE_TEST
+	depends on PCI_MSI_IRQ_DOMAIN
+	select PCIE_DW_HOST
+	help
+	  Unisoc PCIe controller uses the DesignWare core. It can be configured
+	  as an Endpoint (EP) or a Root complex (RC). In order to enable host
+	  mode (the controller works as RC), PCIE_SPRD must be selected.
+	  Say Y or M here if you want to PCIe RC controller support on Unisoc
+	  SoCs.
+
 endmenu
drivers/pci/controller/dwc/Makefile +1 -0
index a751553..eb546e9 100644
@@ -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.
drivers/pci/controller/dwc/pcie-sprd.c new file +293 -0

293 lines not on this page · raw message

--
2.7.4

#3 lore raw

Re: [RESEND PATCH v5 2/2] PCI: sprd: Add support for Unisoc SoCs' PCIe controller

Quoted 12 lines · Hongtao Wu

From: Hongtao Wu <[email protected]>

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

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

<...>

drivers/pci/controller/dwc/pcie-sprd.c line 283

Quoted 3 lines
drivers/pci/controller/dwc/pcie-sprd.c new file +3 -0
+static struct platform_driver sprd_pcie_driver = {
+	.probe = sprd_pcie_probe,
+	.remove = __exit_p(sprd_pcie_remove),

                   ^^^^^^ why is that?

drivers/pci/controller/dwc/pcie-sprd.c line 293

Quoted 10 lines
drivers/pci/controller/dwc/pcie-sprd.c new file +10 -0
+	.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");

I think that it needs to be "GPL" and not "GPL v2".

Thanks

Quoted 3 lines

--
2.7.4

author
#4 lore raw

Re: [RESEND PATCH v5 2/2] PCI: sprd: Add support for Unisoc SoCs' PCIe controller

Quoted 35 lines · Leon Romanovsky
Quoted 12 lines · Hongtao Wu

From: Hongtao Wu <[email protected]>

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

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

<...>

drivers/pci/controller/dwc/pcie-sprd.c line 283

Quoted 3 lines
drivers/pci/controller/dwc/pcie-sprd.c new file +3 -0
+static struct platform_driver sprd_pcie_driver = {
+     .probe = sprd_pcie_probe,
+     .remove = __exit_p(sprd_pcie_remove),

                   ^^^^^^ why is that?

drivers/pci/controller/dwc/pcie-sprd.c line 293

Quoted 10 lines
drivers/pci/controller/dwc/pcie-sprd.c new file +10 -0
+     .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");

I think that it needs to be "GPL" and not "GPL v2".

Quoted 1 line

Thanks

Thanks for the review.
I'll fix it in the next version.

Quoted 4 lines
Quoted 4 lines

--
2.7.4

author
#5 lore raw

Re: [RESEND PATCH v5 2/2] PCI: sprd: Add support for Unisoc SoCs' PCIe controller

In reply to Leon Romanovsky
Quoted 15 lines · Leon Romanovsky
Quoted 5 lines · Hongtao Wu

From: Hongtao Wu <[email protected]>

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

Quoted 7 lines
Signed-off-by Hongtao Wu <[email protected]>
---
 drivers/pci/controller/dwc/Kconfig     |  12 ++
 drivers/pci/controller/dwc/Makefile    |   1 +
 drivers/pci/controller/dwc/pcie-sprd.c | 293 +++++++++++++++++++++++++++++++++
 3 files changed, 306 insertions(+)
 create mode 100644 drivers/pci/controller/dwc/pcie-sprd.c
Quoted 7 lines

<...>

drivers/pci/controller/dwc/pcie-sprd.c line 283

Quoted 3 lines
drivers/pci/controller/dwc/pcie-sprd.c new file +3 -0
+static struct platform_driver sprd_pcie_driver = {
+     .probe = sprd_pcie_probe,
+     .remove = __exit_p(sprd_pcie_remove),

                   ^^^^^^ why is that?

Thanks for the review.

I think that if 'MODULE' is defined, '.remove = sprd_pcie_remove', else '.remove = NULL'.
I would appreciate hearing your opinion about this.

Quoted 4 lines
Quoted 4 lines
drivers/pci/controller/dwc/pcie-sprd.c new file +4 -0
+     .driver = {
+             .name = "sprd-pcie",
+             .of_match_table = sprd_pcie_of_match,
+     },
Quoted 9 lines

drivers/pci/controller/dwc/pcie-sprd.c line 293

Quoted 6 lines
drivers/pci/controller/dwc/pcie-sprd.c new file +6 -0
+};
+
+module_platform_driver(sprd_pcie_driver);
+
+MODULE_DESCRIPTION("Unisoc PCIe host controller driver");
+MODULE_LICENSE("GPL v2");

I think that it needs to be "GPL" and not "GPL v2".

Many platform drivers use 'GPL v2', but others use 'GPL'. I am not sure whether to use 'GPL' or 'GPL v2'. Could you tell me why ‘GPL’ is needed here?

Quoted 5 lines

Thanks

Quoted 3 lines

--
2.7.4

#6 lore raw

Re: [RESEND PATCH v5 2/2] PCI: sprd: Add support for Unisoc SoCs' PCIe controller

Quoted 21 lines · Hongtao Wu
Quoted 20 lines · Leon Romanovsky
Quoted 12 lines · Hongtao Wu

From: Hongtao Wu <[email protected]>

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

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

<...>

Quoted 3 lines
drivers/pci/controller/dwc/pcie-sprd.c new file +3 -0
+static struct platform_driver sprd_pcie_driver = {
+     .probe = sprd_pcie_probe,
+     .remove = __exit_p(sprd_pcie_remove),
Quoted 8 lines
Quoted 2 lines
                   ^^^^^^ why is that?

Thanks for the review.

I think that if 'MODULE' is defined, '.remove = sprd_pcie_remove', else '.remove = NULL'.
I would appreciate hearing your opinion about this.

If module not defined, these .probe and .remove won't be called.

Quoted 15 lines
Quoted 14 lines

drivers/pci/controller/dwc/pcie-sprd.c line 293

Quoted 11 lines
drivers/pci/controller/dwc/pcie-sprd.c new file +10 -0
+     .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");

I think that it needs to be "GPL" and not "GPL v2".

Quoted 3 lines

Many platform drivers use 'GPL v2', but others use 'GPL'. I am not sure whether to use 'GPL' or 'GPL v2'. Could you tell me why ‘GPL’ is needed here?

Because GPL already means v2, see Documentation/process/license-rules.rst

447
448 "GPL v2" Same as "GPL". It exists for historic
449 reasons.

Quoted 6 lines
Quoted 6 lines

Thanks

Quoted 3 lines

--
2.7.4

author
#7 lore raw

Re: [RESEND PATCH v5 2/2] PCI: sprd: Add support for Unisoc SoCs' PCIe controller

Quoted 53 lines · Leon Romanovsky
Quoted 21 lines · Hongtao Wu
Quoted 20 lines · Leon Romanovsky
Quoted 12 lines · Hongtao Wu

From: Hongtao Wu <[email protected]>

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

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

<...>

Quoted 3 lines
drivers/pci/controller/dwc/pcie-sprd.c new file +3 -0
+static struct platform_driver sprd_pcie_driver = {
+     .probe = sprd_pcie_probe,
+     .remove = __exit_p(sprd_pcie_remove),
Quoted 8 lines
Quoted 2 lines
                   ^^^^^^ why is that?

Thanks for the review.

I think that if 'MODULE' is defined, '.remove = sprd_pcie_remove', else '.remove = NULL'.
I would appreciate hearing your opinion about this.

If module not defined, these .probe and .remove won't be called.

Quoted 15 lines
Quoted 14 lines

drivers/pci/controller/dwc/pcie-sprd.c line 293

Quoted 11 lines
drivers/pci/controller/dwc/pcie-sprd.c new file +10 -0
+     .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");

I think that it needs to be "GPL" and not "GPL v2".

Quoted 3 lines

Many platform drivers use 'GPL v2', but others use 'GPL'. I am not sure whether to use 'GPL' or 'GPL v2'. Could you tell me why ‘GPL’ is needed here?

Quoted 6 lines

Because GPL already means v2, see Documentation/process/license-rules.rst

447
448 "GPL v2" Same as "GPL". It exists for historic
449 reasons.

Thanks for the explanation!
I'll update “GPL” and ".remove" in the next version.

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

Thanks

Quoted 3 lines

--
2.7.4