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

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