PCI: qcom: Add support for modular builds

Implement the new host_deinit() callback so that the PHY is powered off and regulators and clocks are disabled also on late host-init errors.

Fixes 82a823833f4e ("PCI: qcom: Add Qualcomm PCIe controller driver")

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/pci/controller/dwc/pcie-qcom.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

drivers/pci/controller/dwc/pcie-qcom.c +12 -1

index 0c13f976626f..417be4d225ed 100644
@@ -1534,8 +1534,19 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
 	return ret;
 }
 
+static void qcom_pcie_host_deinit(struct dw_pcie_rp *pp)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct qcom_pcie *pcie = to_qcom_pcie(pci);
+
+	qcom_ep_reset_assert(pcie);
+	phy_power_off(pcie->phy);
+	pcie->cfg->ops->deinit(pcie);
+}
+
 static const struct dw_pcie_host_ops qcom_pcie_dw_ops = {
-	.host_init = qcom_pcie_host_init,
+	.host_init	= qcom_pcie_host_init,
+	.host_deinit	= qcom_pcie_host_deinit,
 };
 
 /* Qcom IP rev.: 2.1.0	Synopsys IP rev.: 4.01a */

Signature · 1 line

2.37.3

Allow the Qualcomm PCIe controller driver to be built as a module, which is useful for multi-platform kernels as well as during development.

Reviewed-by Rob Herring <[email protected]>

Reviewed-by Manivannan Sadhasivam <[email protected]>

Acked-by Stanimir Varbanov <[email protected]>

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/pci/controller/dwc/Kconfig     |  2 +-
 drivers/pci/controller/dwc/pcie-qcom.c | 26 +++++++++++++++++++++++---
 2 files changed, 24 insertions(+), 4 deletions(-)

drivers/pci/controller/dwc/Kconfig +1 -1

index 62ce3abf0f19..230f56d1a268 100644
@@ -168,7 +168,7 @@ config PCI_HISI
 	  Hip05 and Hip06 SoCs
 
 config PCIE_QCOM
-	bool "Qualcomm PCIe controller"
+	tristate "Qualcomm PCIe controller"
 	depends on OF && (ARCH_QCOM || COMPILE_TEST)
 	depends on PCI_MSI_IRQ_DOMAIN
 	select PCIE_DW_HOST

drivers/pci/controller/dwc/pcie-qcom.c +23 -3

index 417be4d225ed..699172c22ed4 100644
@@ -17,7 +17,7 @@
 #include <linux/io.h>
 #include <linux/iopoll.h>
 #include <linux/kernel.h>
-#include <linux/init.h>
+#include <linux/module.h>
 #include <linux/of_device.h>
 #include <linux/of_gpio.h>
 #include <linux/pci.h>

@@ -1820,6 +1820,21 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 	return ret;
 }
 
+static int qcom_pcie_remove(struct platform_device *pdev)
+{
+	struct qcom_pcie *pcie = platform_get_drvdata(pdev);
+	struct device *dev = &pdev->dev;
+
+	dw_pcie_host_deinit(&pcie->pci->pp);
+
+	phy_exit(pcie->phy);
+
+	pm_runtime_put_sync(dev);
+	pm_runtime_disable(dev);
+
+	return 0;
+}
+
 static const struct of_device_id qcom_pcie_match[] = {
 	{ .compatible = "qcom,pcie-apq8064", .data = &cfg_2_1_0 },
 	{ .compatible = "qcom,pcie-apq8084", .data = &cfg_1_0_0 },

@@ -1841,6 +1856,7 @@ static const struct of_device_id qcom_pcie_match[] = {
 	{ .compatible = "qcom,pcie-sm8450-pcie1", .data = &cfg_1_9_0 },
 	{ }
 };
+MODULE_DEVICE_TABLE(of, qcom_pcie_match);
 
 static void qcom_fixup_class(struct pci_dev *dev)
 {

@@ -1856,10 +1872,14 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_QCOM, 0x1001, qcom_fixup_class);
 
 static struct platform_driver qcom_pcie_driver = {
 	.probe = qcom_pcie_probe,
+	.remove = qcom_pcie_remove,
 	.driver = {
 		.name = "qcom-pcie",
-		.suppress_bind_attrs = true,
 		.of_match_table = qcom_pcie_match,
 	},
 };
-builtin_platform_driver(qcom_pcie_driver);
+module_platform_driver(qcom_pcie_driver);
+
+MODULE_AUTHOR("Stanimir Varbanov <[email protected]>");
+MODULE_DESCRIPTION("Qualcomm PCIe root complex driver");
+MODULE_LICENSE("GPL");

Signature · 1 line

2.37.3

Quoted 2 lines · Johan Hovold

Allow the Qualcomm PCIe controller driver to be built as a module, which is useful for multi-platform kernels as well as during development.

There are two different goals here, and there's no real reason to bundle them together:

1) Make qcom a loadable module. This is a hard requirement so multi-platform kernels don't need to build in all drivers statically.

2) Make qcom unloadable. This is a high want, possibly even a requirement for developers, but is not really a big issue for users.

There are different changes required: 1) requires the Kconfig change;
2) requires .remove() to be implemented. Since there's no requirement that these be done together, let's split them into separate patches.

Then we can make sure that at least 1) gets done, and if for any reason 2) isn't safe or breaks something, we can at least bisect and if necessary revert it without losing 1).

Quoted 19 lines · Bjorn Helgaas

Quoted 2 lines · Johan Hovold

Allow the Qualcomm PCIe controller driver to be built as a module, which is useful for multi-platform kernels as well as during development.

There are two different goals here, and there's no real reason to bundle them together:

1) Make qcom a loadable module. This is a hard requirement so multi-platform kernels don't need to build in all drivers statically.

2) Make qcom unloadable. This is a high want, possibly even a requirement for developers, but is not really a big issue for users.

There are different changes required: 1) requires the Kconfig change;
2) requires .remove() to be implemented. Since there's no requirement that these be done together, let's split them into separate patches.

Quoted 3 lines

Then we can make sure that at least 1) gets done, and if for any reason 2) isn't safe or breaks something, we can at least bisect and if necessary revert it without losing 1).

Implementing 1) in itself requires more than simply splitting this patch. And I don't think we should be making life harder for developers, as well as users assisting during debugging, by going in that direction.

We have tons of modules in the kernel and very few that cannot be unloaded. Anyone who doesn't trust root to not unload modules can always disable unloading completely using CONFIG_MODULE_UNLOAD.

Allow the Qualcomm PCIe controller driver to be built as a module, which is useful for multi-platform kernels as well as during development.

There's no rush with this, but I figured I'd send an updated version that has been rebased on 6.1-rc1 (where post_deinit() has been removed).

I also broke out the qcom_pcie_host_deinit() handler in a separate patch as the host_deinit() callback has now been added to dwc core and can be used to fixes some late-probe error handling.

Johan

Changes in v3
- rebase on 6.1-rc1 where post_deinit() has been removed
- split out host-init error handling
- add Stan's ack

Changes in v2
- rebase on next-20220720 (adjust context)
- add Rob and Mani's reviewed-by tags

Johan Hovold (2):
PCI: qcom: Fix host-init error handling
PCI: qcom: Add support for modular builds

 drivers/pci/controller/dwc/Kconfig     |  2 +-
 drivers/pci/controller/dwc/pcie-qcom.c | 39 +++++++++++++++++++++++---
 2 files changed, 36 insertions(+), 5 deletions(-)

Signature · 1 line

2.37.3

Quoted 6 lines · Johan Hovold

Allow the Qualcomm PCIe controller driver to be built as a module, which is useful for multi-platform kernels as well as during development.

There's no rush with this, but I figured I'd send an updated version that has been rebased on 6.1-rc1 (where post_deinit() has been removed).

Quoted 3 lines

I also broke out the qcom_pcie_host_deinit() handler in a separate patch as the host_deinit() callback has now been added to dwc core and can be used to fixes some late-probe error handling.

Waiting for the dust to settle on patch 2, does it make sense to merge patch 1 on its own ?

Thanks,
Lorenzo

Quoted 24 lines

Johan

Changes in v3
- rebase on 6.1-rc1 where post_deinit() has been removed
- split out host-init error handling
- add Stan's ack

Changes in v2
- rebase on next-20220720 (adjust context)
- add Rob and Mani's reviewed-by tags

Johan Hovold (2):
PCI: qcom: Fix host-init error handling
PCI: qcom: Add support for modular builds

 drivers/pci/controller/dwc/Kconfig     |  2 +-
 drivers/pci/controller/dwc/pcie-qcom.c | 39 +++++++++++++++++++++++---
 2 files changed, 36 insertions(+), 5 deletions(-)

Signature · 1 line

2.37.3

Quoted 6 lines · Johan Hovold

Allow the Qualcomm PCIe controller driver to be built as a module, which is useful for multi-platform kernels as well as during development.

There's no rush with this, but I figured I'd send an updated version that has been rebased on 6.1-rc1 (where post_deinit() has been removed).

Quoted 3 lines

I also broke out the qcom_pcie_host_deinit() handler in a separate patch as the host_deinit() callback has now been added to dwc core and can be used to fixes some late-probe error handling.

Waiting for the dust to settle on patch 2, does it make sense to merge patch 1 on its own ?

Quoted 21 lines

Changes in v3
- rebase on 6.1-rc1 where post_deinit() has been removed
- split out host-init error handling
- add Stan's ack

Changes in v2
- rebase on next-20220720 (adjust context)
- add Rob and Mani's reviewed-by tags

Johan Hovold (2):
PCI: qcom: Fix host-init error handling
PCI: qcom: Add support for modular builds

 drivers/pci/controller/dwc/Kconfig     |  2 +-
 drivers/pci/controller/dwc/pcie-qcom.c | 39 +++++++++++++++++++++++---
 2 files changed, 36 insertions(+), 5 deletions(-)

Signature · 1 line

2.37.3

Allow the Qualcomm PCIe controller driver to be built as a module, which is useful for multi-platform kernels as well as during development.

There's no rush with this, but I figured I'd send an updated version that has been rebased on 6.1-rc1 (where post_deinit() has been removed).

I also broke out the qcom_pcie_host_deinit() handler in a separate patch as the host_deinit() callback has now been added to dwc core and can be used to fixes some late-probe error handling.

[...]