On 8/31/2025 12:45 PM, Manivannan Sadhasivam wrote:
On Thu, Aug 28, 2025 at 10:17:40AM GMT, Chen Wang wrote:
From: Chen Wang <unicorn_wang@xxxxxxxxxxx>
Add support for PCIe controller in SG2042 SoC. The controller
uses the Cadence PCIe core programmed by pcie-cadence*.c. The
PCIe controller will work in host mode only.
Supported data rate, lanes etc...
OK, I will add these descriptions in next revision.
Signed-off-by: Chen Wang <unicorn_wang@xxxxxxxxxxx>
---
drivers/pci/controller/cadence/Kconfig | 12 ++
drivers/pci/controller/cadence/Makefile | 1 +
drivers/pci/controller/cadence/pcie-sg2042.c | 134 +++++++++++++++++++
3 files changed, 147 insertions(+)
create mode 100644 drivers/pci/controller/cadence/pcie-sg2042.c
diff --git a/drivers/pci/controller/cadence/Kconfig b/drivers/pci/controller/cadence/Kconfig
index 666e16b6367f..b1f1941d5208 100644
--- a/drivers/pci/controller/cadence/Kconfig
+++ b/drivers/pci/controller/cadence/Kconfig
@@ -42,6 +42,17 @@ config PCIE_CADENCE_PLAT_EP
endpoint mode. This PCIe controller may be embedded into many
different vendors SoCs.
+config PCIE_SG2042
PCIE_SG2042_HOST
ok
+ bool "Sophgo SG2042 PCIe controller (host mode)"
Since this driver doesn't implement irqchip, you should make it tristate and as
a module.
Yes, I will implement it as a module.
+ depends on ARCH_SOPHGO || COMPILE_TEST
+ depends on OF
depends on OF && (ARCH_SOPHGO || COMPILE_TEST)
OK
[......]
+
+ bridge = devm_pci_alloc_host_bridge(dev, sizeof(*rc));
+ if (!bridge) {
+ dev_err(dev, "Failed to alloc host bridge!\n");
Use dev_err_probe() here and below.
OK
+ return -ENOMEM;
+ }
+
+ bridge->ops = &sg2042_pcie_host_ops;
+
+ rc = pci_host_bridge_priv(bridge);
+ pcie = &rc->pcie;
You are setting drvdata only below.
Sorry, I don't understand your question here. I guess you are confused
about the statement `platform_set_drvdata(pdev, pcie);`. Let me explain
why we need to do like this.
Originally, I defined a drvdata stucture:
struct sg2042_pcie {
struct cdns_pcie *cdns_pcie;
// other properties
};
But after code cleanup, I find there is only "cdns_pcie" left, so I
remove the `struct sg2042_pcie` and directy set `cdns_pcie` (in new
version, it is renamed to pcie) as drvdata.
+ pcie->dev = dev;
+
+ platform_set_drvdata(pdev, pcie);
+
+ pm_runtime_enable(dev);
+
+ ret = pm_runtime_get_sync(dev);
+ if (ret < 0) {
+ dev_err(dev, "pm_runtime_get_sync failed\n");
+ goto err_get_sync;
+ }
+
Why do you need pm_runtime_get_sync()? DT binding doesn't provide a
power-domain, so you just need:
pm_runtime_set_active()
pm_runtime_no_callbacks()
devm_pm_runtime_enable()
OK, I will improve this.
+ ret = cdns_pcie_init_phy(dev, pcie);
+ if (ret) {
+ dev_err(dev, "Failed to init phy!\n");
+ goto err_get_sync;
+ }
+
+ ret = cdns_pcie_host_setup(rc);
+ if (ret < 0) {
+ dev_err(dev, "Failed to setup host!\n");
+ goto err_host_setup;
+ }
+
+ return 0;
+
+err_host_setup:
+ cdns_pcie_disable_phy(pcie);
+
+err_get_sync:
+ pm_runtime_put(dev);
+ pm_runtime_disable(dev);
+
+ return ret;
+}
+
+static void sg2042_pcie_shutdown(struct platform_device *pdev)
+{
+ struct cdns_pcie *pcie = platform_get_drvdata(pdev);
+ struct device *dev = &pdev->dev;
+
+ cdns_pcie_disable_phy(pcie);
+
+ pm_runtime_put(dev);
+ pm_runtime_disable(dev);
You don't need these as per my above suggestion.
OK.
+}
+
+static const struct of_device_id sg2042_pcie_of_match[] = {
+ { .compatible = "sophgo,sg2042-pcie-host" },
+ {},
+};
+
+static struct platform_driver sg2042_pcie_driver = {
+ .driver = {
+ .name = "sg2042-pcie",
+ .of_match_table = sg2042_pcie_of_match,
+ .pm = &cdns_pcie_pm_ops,
+ },
+ .probe = sg2042_pcie_probe,
Why no remove()?
OK, I will replace shutdown with remove when I implement it as a module
in next revision.
Thanks,
Chen
- Mani
