[RESEND PATCH v5 0/2] PCI: Add new Unisoc PCIe driver

STALE2028d

Revision v5 of 7 in this series.

8 messages, 2 authors, 2021-01-14 · open the first message on its own page

[RESEND PATCH v5 0/2] PCI: Add new Unisoc PCIe driver

From: Hongtao Wu <hidden>
Date: 2021-01-14 08:30:34

From: Hongtao Wu <redacted>

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/20200821035420.380495-1-robh@kernel.org/

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

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

From: Hongtao Wu <hidden>
Date: 2021-01-14 08:30:34

From: Hongtao Wu <redacted>

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

Signed-off-by: Hongtao Wu <redacted>
---
 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
diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index 22c5529..61f0b79 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -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
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..27d7231
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-sprd.c
@@ -0,0 +1,293 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * PCIe host controller driver for Unisoc SoCs
+ *
+ * Copyright (C) 2020-2021 Unisoc, Inc.
+ *
+ * Author: Hongtao Wu <Billows.Wu@unisoc.com>
+ */
+
+#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/property.h>
+#include <linux/regmap.h>
+
+#include "pcie-designware.h"
+
+/* aon apb syscon */
+#define IPA_ACCESS_CFG		0xcd8
+#define  AON_ACCESS_PCIE_EN	BIT(1)
+
+/* pmu apb syscon */
+#define SNPS_PCIE3_SLP_CTRL	0xac
+#define  PERST_N_ASSERT		BIT(1)
+#define  PERST_N_AUTO_EN	BIT(0)
+#define PD_PCIE_CFG_0		0x3e8
+#define  PCIE_FORCE_SHUTDOWN	BIT(25)
+
+#define PCIE_SS_REG_BASE		0xE00
+#define APB_CLKFREQ_TIMEOUT		0x4
+#define  BUSERR_EN			BIT(12)
+#define  APB_TIMER_DIS			BIT(10)
+#define  APB_TIMER_LIMIT		GENMASK(31, 16)
+
+#define PE0_GEN_CTRL_3			0x58
+#define  LTSSM_EN			BIT(0)
+
+struct sprd_pcie_soc_data {
+	u32 syscon_offset;
+};
+
+static const struct sprd_pcie_soc_data ums9520_syscon_data = {
+	.syscon_offset = 0x1000,	/* The offset of set/clear register */
+};
+
+struct sprd_pcie {
+	u32 syscon_offset;
+	struct device	*dev;
+	struct dw_pcie	*pci;
+	struct regmap	*aon_map;
+	struct regmap	*pmu_map;
+	const struct sprd_pcie_soc_data *socdata;
+};
+
+enum sprd_pcie_syscon_type {
+	normal_syscon,		/* it's not a set/clear register */
+	set_syscon,		/* set a set/clear register */
+	clr_syscon,		/* clear a set/clear register */
+};
+
+static void sprd_pcie_buserr_enable(struct dw_pcie *pci)
+{
+	u32 val;
+
+	val = dw_pcie_readl_dbi(pci, PCIE_SS_REG_BASE + APB_CLKFREQ_TIMEOUT);
+	val &= ~APB_TIMER_DIS;
+	val |= BUSERR_EN;
+	val |= APB_TIMER_LIMIT & (0x1f4 << 16);
+	dw_pcie_writel_dbi(pci, PCIE_SS_REG_BASE + APB_CLKFREQ_TIMEOUT, val);
+}
+
+static void sprd_pcie_ltssm_enable(struct dw_pcie *pci, bool enable)
+{
+	u32 val;
+
+	val = dw_pcie_readl_dbi(pci, PCIE_SS_REG_BASE + PE0_GEN_CTRL_3);
+	if (enable)
+		dw_pcie_writel_dbi(pci, PCIE_SS_REG_BASE + PE0_GEN_CTRL_3,
+				   val | LTSSM_EN);
+	else
+		dw_pcie_writel_dbi(pci, PCIE_SS_REG_BASE + PE0_GEN_CTRL_3,
+				   val & ~LTSSM_EN);
+}
+
+static int sprd_pcie_syscon_set(struct sprd_pcie *ctrl, struct regmap *map,
+				u32 reg, u32 mask, u32 val,
+				enum sprd_pcie_syscon_type type)
+{
+	int ret = 0;
+	u32 read_val;
+	u32 offset = ctrl->syscon_offset;
+	struct device *dev = ctrl->pci->dev;
+
+	/*
+	 * Each set/clear register has three registers:
+	 * reg:			base register
+	 * reg + offset:	set register
+	 * reg + offset * 2:	clear register
+	 */
+	switch (type) {
+	case normal_syscon:
+		ret = regmap_read(map, reg, &read_val);
+		if (ret) {
+			dev_err(dev, "failed to read register 0x%x\n", reg);
+			return ret;
+		}
+		read_val &= ~mask;
+		read_val |= (val & mask);
+		ret = regmap_write(map, reg, read_val);
+		break;
+	case set_syscon:
+		reg = reg + offset;
+		ret = regmap_write(map, reg, val);
+		break;
+	case clr_syscon:
+		reg = reg + offset * 2;
+		ret = regmap_write(map, reg, val);
+		break;
+	default:
+		break;
+	}
+
+	if (ret)
+		dev_err(dev, "failed to write register 0x%x\n", reg);
+
+	return ret;
+}
+
+static int sprd_pcie_perst_assert(struct sprd_pcie *ctrl)
+{
+	return sprd_pcie_syscon_set(ctrl, ctrl->pmu_map, SNPS_PCIE3_SLP_CTRL,
+				    PERST_N_ASSERT, PERST_N_ASSERT, set_syscon);
+}
+
+static int sprd_pcie_perst_deassert(struct sprd_pcie *ctrl)
+{
+	int ret;
+
+	ret = sprd_pcie_syscon_set(ctrl, ctrl->pmu_map, SNPS_PCIE3_SLP_CTRL,
+				   PERST_N_ASSERT, 0, clr_syscon);
+	usleep_range(2000, 3000);
+
+	return ret;
+}
+
+static int sprd_pcie_power_on(struct platform_device *pdev)
+{
+	int ret;
+	struct sprd_pcie *ctrl = platform_get_drvdata(pdev);
+	struct dw_pcie *pci = ctrl->pci;
+
+	ret = sprd_pcie_syscon_set(ctrl, ctrl->aon_map, PD_PCIE_CFG_0,
+				   PCIE_FORCE_SHUTDOWN, 0, clr_syscon);
+	if (ret)
+		return ret;
+
+	ret = sprd_pcie_syscon_set(ctrl, ctrl->aon_map, IPA_ACCESS_CFG,
+				   AON_ACCESS_PCIE_EN, AON_ACCESS_PCIE_EN,
+				   set_syscon);
+	if (ret)
+		return ret;
+
+	ret = sprd_pcie_perst_deassert(ctrl);
+	if (ret)
+		return ret;
+
+	sprd_pcie_buserr_enable(pci);
+	sprd_pcie_ltssm_enable(pci, true);
+
+	return ret;
+}
+
+static int sprd_pcie_power_off(struct platform_device *pdev)
+{
+	struct sprd_pcie *ctrl = platform_get_drvdata(pdev);
+	struct dw_pcie *pci = ctrl->pci;
+
+	sprd_pcie_ltssm_enable(pci, false);
+
+	sprd_pcie_perst_assert(ctrl);
+	sprd_pcie_syscon_set(ctrl, ctrl->aon_map, PD_PCIE_CFG_0,
+			     PCIE_FORCE_SHUTDOWN, PCIE_FORCE_SHUTDOWN,
+			     set_syscon);
+	sprd_pcie_syscon_set(ctrl, ctrl->aon_map, IPA_ACCESS_CFG,
+			     AON_ACCESS_PCIE_EN, 0, clr_syscon);
+
+	return 0;
+}
+
+static int sprd_add_pcie_port(struct platform_device *pdev)
+{
+	struct sprd_pcie *ctrl = platform_get_drvdata(pdev);
+	struct dw_pcie *pci = ctrl->pci;
+	struct pcie_port *pp = &pci->pp;
+
+	return dw_pcie_host_init(pp);
+}
+
+static int sprd_pcie_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct sprd_pcie *ctrl;
+	struct dw_pcie *pci;
+	int ret;
+
+	ctrl = devm_kzalloc(dev, sizeof(*ctrl), GFP_KERNEL);
+	if (!ctrl)
+		return -ENOMEM;
+
+	ctrl->socdata =
+		(struct sprd_pcie_soc_data *)of_device_get_match_data(dev);
+	if (!ctrl->socdata) {
+		dev_warn(dev,
+			 "using the default set/clear register offset address");
+		ctrl->syscon_offset = 0x1000;
+	}
+	ctrl->syscon_offset = ctrl->socdata->syscon_offset;
+
+	ctrl->aon_map = syscon_regmap_lookup_by_phandle(dev->of_node,
+							"sprd, regmap-aon");
+	if (IS_ERR(ctrl->aon_map)) {
+		dev_err(dev, "failed to get syscon regmap aon\n");
+		ret = PTR_ERR(ctrl->aon_map);
+		goto err;
+	}
+
+	ctrl->pmu_map = syscon_regmap_lookup_by_phandle(dev->of_node,
+							"sprd, regmap-pmu");
+	if (IS_ERR(ctrl->pmu_map)) {
+		dev_err(dev, "failed to get syscon regmap pmu\n");
+		ret = PTR_ERR(ctrl->pmu_map);
+		goto err;
+	}
+
+	pci = ctrl->pci;
+	pci->dev = dev;
+
+	platform_set_drvdata(pdev, ctrl);
+
+	ret = sprd_pcie_power_on(pdev);
+	if (ret < 0) {
+		dev_err(dev, "failed to power on, 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 ret;
+	}
+
+	return 0;
+
+err_power_off:
+	sprd_pcie_power_off(pdev);
+err:
+	return ret;
+}
+
+static int sprd_pcie_remove(struct platform_device *pdev)
+{
+	sprd_pcie_power_off(pdev);
+
+	return 0;
+}
+
+static const struct of_device_id sprd_pcie_of_match[] = {
+	{
+		.compatible = "sprd,ums9520-pcie",
+		.data  = &ums9520_syscon_data,
+	},
+	{},
+};
+
+static struct platform_driver sprd_pcie_driver = {
+	.probe = sprd_pcie_probe,
+	.remove = __exit_p(sprd_pcie_remove),
+	.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

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

From: Hongtao Wu <hidden>
Date: 2021-01-14 08:30:34

From: Hongtao Wu <redacted>

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

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Hongtao Wu <redacted>
---
 .../devicetree/bindings/pci/sprd-pcie.yaml         | 93 ++++++++++++++++++++++
 1 file changed, 93 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/sprd-pcie.yaml
diff --git a/Documentation/devicetree/bindings/pci/sprd-pcie.yaml b/Documentation/devicetree/bindings/pci/sprd-pcie.yaml
new file mode 100644
index 0000000..ede06a8
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/sprd-pcie.yaml
@@ -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 <billows.wu@unisoc.com>
+
+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

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

From: Leon Romanovsky <leon@kernel.org>
Date: 2021-01-14 08:53:40

On Thu, Jan 14, 2021 at 04:29:28PM +0800, Hongtao Wu wrote:
From: Hongtao Wu <redacted>

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

Signed-off-by: Hongtao Wu <redacted>
---
 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
<...>
+static struct platform_driver sprd_pcie_driver = {
+	.probe = sprd_pcie_probe,
+	.remove = __exit_p(sprd_pcie_remove),
                   ^^^^^^ why is that?
+	.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
--
2.7.4

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

From: Hongtao Wu <hidden>
Date: 2021-01-14 10:50:57

On Thu, Jan 14, 2021 at 4:52 PM Leon Romanovsky [off-list ref] wrote:
On Thu, Jan 14, 2021 at 04:29:28PM +0800, Hongtao Wu wrote:
quoted
From: Hongtao Wu <redacted>

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

Signed-off-by: Hongtao Wu <redacted>
---
 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
+static struct platform_driver sprd_pcie_driver = {
+     .probe = sprd_pcie_probe,
+     .remove = __exit_p(sprd_pcie_remove),
                   ^^^^^^ why is that?
quoted
+     .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
Thanks for the review.
I'll fix it in the next version.
quoted
--
2.7.4

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

From: Hongtao Wu <hidden>
Date: 2021-01-14 12:01:51

On Thu, Jan 14, 2021 at 4:52 PM Leon Romanovsky [off-list ref] wrote:
On Thu, Jan 14, 2021 at 04:29:28PM +0800, Hongtao Wu wrote:
quoted
From: Hongtao Wu <redacted>

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

Signed-off-by: Hongtao Wu <redacted>
---
 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
+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
+     .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".
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?
Thanks
quoted
--
2.7.4

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

From: Leon Romanovsky <leon@kernel.org>
Date: 2021-01-14 13:07:25

On Thu, Jan 14, 2021 at 08:00:50PM +0800, Hongtao Wu wrote:
On Thu, Jan 14, 2021 at 4:52 PM Leon Romanovsky [off-list ref] wrote:
quoted
On Thu, Jan 14, 2021 at 04:29:28PM +0800, Hongtao Wu wrote:
quoted
From: Hongtao Wu <redacted>

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

Signed-off-by: Hongtao Wu <redacted>
---
 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
+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.
If module not defined, these .probe and .remove won't be called.
quoted
quoted
+     .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".
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
Thanks
quoted
--
2.7.4

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

From: Hongtao Wu <hidden>
Date: 2021-01-14 13:31:34

On Thu, Jan 14, 2021 at 9:06 PM Leon Romanovsky [off-list ref] wrote:
On Thu, Jan 14, 2021 at 08:00:50PM +0800, Hongtao Wu wrote:
quoted
On Thu, Jan 14, 2021 at 4:52 PM Leon Romanovsky [off-list ref] wrote:
quoted
On Thu, Jan 14, 2021 at 04:29:28PM +0800, Hongtao Wu wrote:
quoted
From: Hongtao Wu <redacted>

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

Signed-off-by: Hongtao Wu <redacted>
---
 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
+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.
If module not defined, these .probe and .remove won't be called.
quoted
quoted
quoted
+     .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".
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.
Thanks for the explanation!
I'll update “GPL” and ".remove" in the next version.
quoted
quoted
Thanks
quoted
--
2.7.4
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help