Message ID | 5051f2375ff6218e7d44ce0c298efd5f9ee56964.1731303328.git.unicorn_wang@outlook.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | Add PCIe support to Sophgo SG2042 SoC | expand |
Hi Chen, kernel test robot noticed the following build warnings: [auto build test WARNING on 2d5404caa8c7bb5c4e0435f94b28834ae5456623] url: https://github.com/intel-lab-lkp/linux/commits/Chen-Wang/dt-bindings-pci-Add-Sophgo-SG2042-PCIe-host/20241111-140237 base: 2d5404caa8c7bb5c4e0435f94b28834ae5456623 patch link: https://lore.kernel.org/r/5051f2375ff6218e7d44ce0c298efd5f9ee56964.1731303328.git.unicorn_wang%40outlook.com patch subject: [PATCH 2/5] PCI: sg2042: Add Sophgo SG2042 PCIe driver config: um-allyesconfig (https://download.01.org/0day-ci/archive/20241111/202411111935.kqORCWuE-lkp@intel.com/config) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241111/202411111935.kqORCWuE-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202411111935.kqORCWuE-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/pci/controller/cadence/pcie-sg2042.c:141:12: warning: 'sg2042_pcie_msi_irq_set_affinity' defined but not used [-Wunused-function] 141 | static int sg2042_pcie_msi_irq_set_affinity(struct irq_data *d, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ vim +/sg2042_pcie_msi_irq_set_affinity +141 drivers/pci/controller/cadence/pcie-sg2042.c 140 > 141 static int sg2042_pcie_msi_irq_set_affinity(struct irq_data *d, 142 const struct cpumask *mask, 143 bool force) 144 { 145 if (d->parent_data) 146 return irq_chip_set_affinity_parent(d, mask, force); 147 148 return -EINVAL; 149 } 150
On Mon, Nov 11, 2024 at 01:59:56PM +0800, Chen Wang wrote: > From: Chen Wang <unicorn_wang@outlook.com> > > 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. > +++ b/drivers/pci/controller/cadence/Kconfig > @@ -67,4 +67,15 @@ config PCI_J721E_EP > Say Y here if you want to support the TI J721E PCIe platform > controller in endpoint mode. TI J721E PCIe controller uses Cadence PCIe > core. > + > +config PCIE_SG2042 > + bool "Sophgo SG2042 PCIe controller (host mode)" > + depends on ARCH_SOPHGO || COMPILE_TEST > + depends on OF > + select PCIE_CADENCE_HOST > + help > + Say Y here if you want to support the Sophgo SG2042 PCIe platform > + controller in host mode. Sophgo SG2042 PCIe controller uses Cadence > + PCIe core. Reorder to keep these menu items in alphabetical order by vendor. > +++ b/drivers/pci/controller/cadence/pcie-sg2042.c > + * SG2042 PCIe controller supports two ways to report MSI: > + * - Method A, the PICe controller implements an MSI interrupt controller inside, > + * and connect to PLIC upward through one interrupt line. Provides > + * memory-mapped msi address, and by programming the upper 32 bits of the > + * address to zero, it can be compatible with old pcie devices that only > + * support 32-bit msi address. > + * - Method B, the PICe controller connects to PLIC upward through an > + * independent MSI controller "sophgo,sg2042-msi" on the SOC. The MSI > + * controller provides multiple(up to 32) interrupt sources to PLIC. > + * Compared with the first method, the advantage is that the interrupt source > + * is expanded, but because for SG2042, the msi address provided by the MSI > + * controller is fixed and only supports 64-bit address(> 2^32), it is not > + * compatible with old pcie devices that only support 32-bit msi address. > + * Method A & B can be configured in DTS with property "sophgo,internal-msi", > + * default is Method B. s/PICe/PCIe/ (multiple) s/msi/MSI/ (multiple) s/pcie/PCIe/ (multiple) Wrap comment (and code below) to fit in 80 columns. Add blank lines between paragraphs. > +#define SG2042_CDNS_PLAT_CPU_TO_BUS_ADDR 0xCFFFFFFFFF Remove (see below). > +static void sg2042_pcie_msi_irq_compose_msi_msg(struct irq_data *d, > + struct msi_msg *msg) > +{ > + struct sg2042_pcie *pcie = irq_data_get_irq_chip_data(d); > + struct device *dev = pcie->cdns_pcie->dev; > + > + msg->address_lo = lower_32_bits(pcie->msi_phys) + BYTE_NUM_PER_MSI_VEC * d->hwirq; > + msg->address_hi = upper_32_bits(pcie->msi_phys); > + msg->data = 1; > + > + pcie->num_applied_vecs = d->hwirq; This looks questionable. How do you know d->hwirq increases every time this is called? > + dev_info(dev, "compose msi msg hwirq[%d] address_hi[%#x] address_lo[%#x]\n", > + (int)d->hwirq, msg->address_hi, msg->address_lo); This seems too verbose to be a dev_info(). Maybe a dev_dbg() or remove it altogether. > + * We use the usual two domain structure, the top one being a generic PCI/MSI > + * domain, the bottom one being SG2042-specific and handling the actual HW > + * interrupt allocation. > + * At the same time, for internal MSI controller(Method A), bottom chip uses a > + * chained handler to handle the controller's MSI IRQ edge triggered. Add blank line between paragraphs. > +static int sg2042_pcie_setup_msi_external(struct sg2042_pcie *pcie) > +{ > + struct device *dev = pcie->cdns_pcie->dev; > + struct device_node *np = dev->of_node; > + struct irq_domain *parent_domain; > + struct device_node *parent_np; > + > + if (!of_find_property(np, "interrupt-parent", NULL)) { > + dev_err(dev, "Can't find interrupt-parent!\n"); > + return -EINVAL; > + } > + > + parent_np = of_irq_find_parent(np); > + if (!parent_np) { > + dev_err(dev, "Can't find node of interrupt-parent!\n"); Can you use some kind of %pOF format to include more information here? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/core-api/printk-formats.rst?id=v6.11#n463 > + return -ENXIO; > + } > + > + parent_domain = irq_find_host(parent_np); > + of_node_put(parent_np); > + if (!parent_domain) { > + dev_err(dev, "Can't find domain of interrupt-parent!\n"); And here? > + return -ENXIO; > + } > + > + return sg2042_pcie_create_msi_domain(pcie, parent_domain); > +} > +static int sg2042_pcie_init_msi_data(struct sg2042_pcie *pcie) > +{ > + struct device *dev = pcie->cdns_pcie->dev; > + u32 value; > + int ret; > + > + raw_spin_lock_init(&pcie->lock); > + > + /* > + * Though the PCIe controller can address >32-bit address space, to > + * facilitate endpoints that support only 32-bit MSI target address, > + * the mask is set to 32-bit to make sure that MSI target address is > + * always a 32-bit address > + */ > + ret = dma_set_coherent_mask(dev, DMA_BIT_MASK(32)); > + if (ret < 0) > + return ret; > + > + pcie->msi_virt = dma_alloc_coherent(dev, BYTE_NUM_PER_MSI_VEC * MAX_MSI_IRQS, > + &pcie->msi_phys, GFP_KERNEL); > + if (!pcie->msi_virt) > + return -ENOMEM; > + > + /* Program the msi address and size */ s/msi/MSI/ > + if (pcie->link_id == 1) { > + regmap_write(pcie->syscon, REG_LINK1_MSI_ADDR_LOW, > + lower_32_bits(pcie->msi_phys)); > + regmap_write(pcie->syscon, REG_LINK1_MSI_ADDR_HIGH, > + upper_32_bits(pcie->msi_phys)); > + > + regmap_read(pcie->syscon, REG_LINK1_MSI_ADDR_SIZE, &value); > + value = (value & REG_LINK1_MSI_ADDR_SIZE_MASK) | MAX_MSI_IRQS; > + regmap_write(pcie->syscon, REG_LINK1_MSI_ADDR_SIZE, value); > + } else { > + regmap_write(pcie->syscon, REG_LINK0_MSI_ADDR_LOW, > + lower_32_bits(pcie->msi_phys)); > + regmap_write(pcie->syscon, REG_LINK0_MSI_ADDR_HIGH, > + upper_32_bits(pcie->msi_phys)); > + > + regmap_read(pcie->syscon, REG_LINK0_MSI_ADDR_SIZE, &value); > + value = (value & REG_LINK0_MSI_ADDR_SIZE_MASK) | (MAX_MSI_IRQS << 16); > + regmap_write(pcie->syscon, REG_LINK0_MSI_ADDR_SIZE, value); > + } Lot of pcie->link_id checking going on here. Consider saving these offsets in the struct sg2042_pcie so you don't need to test everywhere. > + return 0; > +} > +static int sg2042_pcie_setup_msi(struct sg2042_pcie *pcie, struct platform_device *pdev) > +{ > + struct device *dev = pcie->cdns_pcie->dev; > + struct fwnode_handle *fwnode = of_node_to_fwnode(dev->of_node); > + struct irq_domain *parent_domain; > + int ret = 0; > + > + parent_domain = irq_domain_create_linear(fwnode, MSI_DEF_NUM_VECTORS, > + &sg2042_pcie_msi_domain_ops, pcie); > + if (!parent_domain) { > + dev_err(dev, "Failed to create IRQ domain\n"); > + return -ENOMEM; > + } > + irq_domain_update_bus_token(parent_domain, DOMAIN_BUS_NEXUS); > + > + ret = sg2042_pcie_create_msi_domain(pcie, parent_domain); > + if (ret) { > + irq_domain_remove(parent_domain); > + return ret; > + } > + > + ret = sg2042_pcie_init_msi_data(pcie); > + if (ret) { > + dev_err(dev, "Failed to initialize msi data!\n"); s/msi/MSI/ > + return ret; > + } > + > + ret = platform_get_irq_byname(pdev, "msi"); > + if (ret <= 0) { > + dev_err(dev, "failed to get MSI irq\n"); > + return ret; > + } > + pcie->msi_irq = ret; > + > + irq_set_chained_handler_and_data(pcie->msi_irq, > + sg2042_pcie_msi_chained_isr, pcie); > + > + return 0; > +} > + > +static void sg2042_pcie_free_msi(struct sg2042_pcie *pcie) > +{ > + struct device *dev = pcie->cdns_pcie->dev; > + > + if (pcie->msi_irq) > + irq_set_chained_handler_and_data(pcie->msi_irq, NULL, NULL); > + > + if (pcie->msi_virt) > + dma_free_coherent(dev, BYTE_NUM_PER_MSI_VEC * MAX_MSI_IRQS, > + pcie->msi_virt, pcie->msi_phys); > +} > + > +static u64 sg2042_cdns_pcie_cpu_addr_fixup(struct cdns_pcie *pcie, u64 cpu_addr) > +{ > + return cpu_addr & SG2042_CDNS_PLAT_CPU_TO_BUS_ADDR; > +} Remove. This translation between CPU and PCI bus addresses should be described in the DT "ranges" property of the PCI host bridge. See https://lore.kernel.org/linux-pci/20241029-pci_fixup_addr-v7-3-8310dc24fb7c@nxp.com/ for a similar fix. > +static const struct cdns_pcie_ops sg2042_cdns_pcie_ops = { > + .cpu_addr_fixup = sg2042_cdns_pcie_cpu_addr_fixup, > +}; > + > +/* > + * SG2042 only support 4-byte aligned access, so for the rootbus (i.e. to read > + * the PCIe controller itself, read32 is required. For non-rootbus (i.e. to read > + * the PCIe peripheral registers, supports 1/2/4 byte aligned access, so > + * directly use read should be fine. > + * The same is true for write. Add blank line between paragraphs or rewrap into a single paragraph. > +static int sg2042_pcie_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct device_node *np = dev->of_node; > + struct pci_host_bridge *bridge; > + struct device_node *np_syscon; > + struct cdns_pcie *cdns_pcie; > + struct sg2042_pcie *pcie; > + struct cdns_pcie_rc *rc; > + struct regmap *syscon; > + int ret; > + > + if (!IS_ENABLED(CONFIG_PCIE_CADENCE_HOST)) > + return -ENODEV; > + > + pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL); > + if (!pcie) > + return -ENOMEM; > + > + bridge = devm_pci_alloc_host_bridge(dev, sizeof(*rc)); > + if (!bridge) { > + dev_err(dev, "Failed to alloc host bridge!\n"); > + return -ENOMEM; > + } Add blank line. > + bridge->ops = &sg2042_pcie_host_ops; > + > + rc = pci_host_bridge_priv(bridge); > + cdns_pcie = &rc->pcie; > + cdns_pcie->dev = dev; > + cdns_pcie->ops = &sg2042_cdns_pcie_ops; > + pcie->cdns_pcie = cdns_pcie; > + > + np_syscon = of_parse_phandle(np, "sophgo,syscon-pcie-ctrl", 0); > + if (!np_syscon) { > + dev_err(dev, "Failed to get syscon node\n"); > + return -ENOMEM; > + } > + syscon = syscon_node_to_regmap(np_syscon); > + if (IS_ERR(syscon)) { > + dev_err(dev, "Failed to get regmap for syscon\n"); > + return -ENOMEM; > + } > + pcie->syscon = syscon; > + > + if (of_property_read_u32(np, "sophgo,link-id", &pcie->link_id)) { > + dev_err(dev, "Unable to parse link ID\n"); > + return -EINVAL; > + } > + > + pcie->internal_msi = 0; > + if (of_property_read_bool(np, "sophgo,internal-msi")) > + pcie->internal_msi = 1; > + > + 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; > + } > + > + if (pcie->internal_msi) { > + ret = sg2042_pcie_setup_msi(pcie, pdev); > + if (ret < 0) > + goto err_setup_msi; > + } else { > + ret = sg2042_pcie_setup_msi_external(pcie); > + if (ret < 0) > + goto err_setup_msi; > + } > + > + ret = cdns_pcie_init_phy(dev, cdns_pcie); > + if (ret) { > + dev_err(dev, "Failed to init phy!\n"); > + goto err_setup_msi; > + } > + > + 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(cdns_pcie); > + > +err_setup_msi: > + sg2042_pcie_free_msi(pcie); > + > +err_get_sync: > + pm_runtime_put(dev); > + pm_runtime_disable(dev); > + > + return ret; > +} > + > +static void sg2042_pcie_shutdown(struct platform_device *pdev) > +{ > + struct sg2042_pcie *pcie = platform_get_drvdata(pdev); > + struct cdns_pcie *cdns_pcie = pcie->cdns_pcie; > + struct device *dev = &pdev->dev; > + > + sg2042_pcie_free_msi(pcie); > + > + cdns_pcie_disable_phy(cdns_pcie); > + > + pm_runtime_put(dev); > + pm_runtime_disable(dev); > +} > + > +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, > + .shutdown = sg2042_pcie_shutdown, > +}; > +builtin_platform_driver(sg2042_pcie_driver); > -- > 2.34.1 >
Hello~ On 2024/11/13 5:20, Bjorn Helgaas wrote: > On Mon, Nov 11, 2024 at 01:59:56PM +0800, Chen Wang wrote: >> From: Chen Wang <unicorn_wang@outlook.com> >> >> 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. >> +++ b/drivers/pci/controller/cadence/Kconfig >> @@ -67,4 +67,15 @@ config PCI_J721E_EP >> Say Y here if you want to support the TI J721E PCIe platform >> controller in endpoint mode. TI J721E PCIe controller uses Cadence PCIe >> core. >> + >> +config PCIE_SG2042 >> + bool "Sophgo SG2042 PCIe controller (host mode)" >> + depends on ARCH_SOPHGO || COMPILE_TEST >> + depends on OF >> + select PCIE_CADENCE_HOST >> + help >> + Say Y here if you want to support the Sophgo SG2042 PCIe platform >> + controller in host mode. Sophgo SG2042 PCIe controller uses Cadence >> + PCIe core. > Reorder to keep these menu items in alphabetical order by vendor. Sorry, I don't understand your question. I think the menu items in this Kconfig file are already sorted alphabetically. >> +++ b/drivers/pci/controller/cadence/pcie-sg2042.c >> + * SG2042 PCIe controller supports two ways to report MSI: >> + * - Method A, the PICe controller implements an MSI interrupt controller inside, >> + * and connect to PLIC upward through one interrupt line. Provides >> + * memory-mapped msi address, and by programming the upper 32 bits of the >> + * address to zero, it can be compatible with old pcie devices that only >> + * support 32-bit msi address. >> + * - Method B, the PICe controller connects to PLIC upward through an >> + * independent MSI controller "sophgo,sg2042-msi" on the SOC. The MSI >> + * controller provides multiple(up to 32) interrupt sources to PLIC. >> + * Compared with the first method, the advantage is that the interrupt source >> + * is expanded, but because for SG2042, the msi address provided by the MSI >> + * controller is fixed and only supports 64-bit address(> 2^32), it is not >> + * compatible with old pcie devices that only support 32-bit msi address. >> + * Method A & B can be configured in DTS with property "sophgo,internal-msi", >> + * default is Method B. > s/PICe/PCIe/ (multiple) > s/msi/MSI/ (multiple) > s/pcie/PCIe/ (multiple) > > Wrap comment (and code below) to fit in 80 columns. Add blank lines > between paragraphs. Got, will change. > >> +#define SG2042_CDNS_PLAT_CPU_TO_BUS_ADDR 0xCFFFFFFFFF > Remove (see below). Yes, after some testing, seems this address fixup is not need, will remove it. >> +static void sg2042_pcie_msi_irq_compose_msi_msg(struct irq_data *d, >> + struct msi_msg *msg) >> +{ >> + struct sg2042_pcie *pcie = irq_data_get_irq_chip_data(d); >> + struct device *dev = pcie->cdns_pcie->dev; >> + >> + msg->address_lo = lower_32_bits(pcie->msi_phys) + BYTE_NUM_PER_MSI_VEC * d->hwirq; >> + msg->address_hi = upper_32_bits(pcie->msi_phys); >> + msg->data = 1; >> + >> + pcie->num_applied_vecs = d->hwirq; > This looks questionable. How do you know d->hwirq increases every > time this is called? Thanks, it's a bug, I will fix it. >> + dev_info(dev, "compose msi msg hwirq[%d] address_hi[%#x] address_lo[%#x]\n", >> + (int)d->hwirq, msg->address_hi, msg->address_lo); > This seems too verbose to be a dev_info(). Maybe a dev_dbg() or > remove it altogether. Will change it to dev_dbg. >> + * We use the usual two domain structure, the top one being a generic PCI/MSI >> + * domain, the bottom one being SG2042-specific and handling the actual HW >> + * interrupt allocation. >> + * At the same time, for internal MSI controller(Method A), bottom chip uses a >> + * chained handler to handle the controller's MSI IRQ edge triggered. > Add blank line between paragraphs. OK. > >> +static int sg2042_pcie_setup_msi_external(struct sg2042_pcie *pcie) >> +{ >> + struct device *dev = pcie->cdns_pcie->dev; >> + struct device_node *np = dev->of_node; >> + struct irq_domain *parent_domain; >> + struct device_node *parent_np; >> + >> + if (!of_find_property(np, "interrupt-parent", NULL)) { >> + dev_err(dev, "Can't find interrupt-parent!\n"); >> + return -EINVAL; >> + } >> + >> + parent_np = of_irq_find_parent(np); >> + if (!parent_np) { >> + dev_err(dev, "Can't find node of interrupt-parent!\n"); > Can you use some kind of %pOF format to include more information here? > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/core-api/printk-formats.rst?id=v6.11#n463 Thanks, will double check this. [......] >> + if (pcie->link_id == 1) { >> + regmap_write(pcie->syscon, REG_LINK1_MSI_ADDR_LOW, >> + lower_32_bits(pcie->msi_phys)); >> + regmap_write(pcie->syscon, REG_LINK1_MSI_ADDR_HIGH, >> + upper_32_bits(pcie->msi_phys)); >> + >> + regmap_read(pcie->syscon, REG_LINK1_MSI_ADDR_SIZE, &value); >> + value = (value & REG_LINK1_MSI_ADDR_SIZE_MASK) | MAX_MSI_IRQS; >> + regmap_write(pcie->syscon, REG_LINK1_MSI_ADDR_SIZE, value); >> + } else { >> + regmap_write(pcie->syscon, REG_LINK0_MSI_ADDR_LOW, >> + lower_32_bits(pcie->msi_phys)); >> + regmap_write(pcie->syscon, REG_LINK0_MSI_ADDR_HIGH, >> + upper_32_bits(pcie->msi_phys)); >> + >> + regmap_read(pcie->syscon, REG_LINK0_MSI_ADDR_SIZE, &value); >> + value = (value & REG_LINK0_MSI_ADDR_SIZE_MASK) | (MAX_MSI_IRQS << 16); >> + regmap_write(pcie->syscon, REG_LINK0_MSI_ADDR_SIZE, value); >> + } > Lot of pcie->link_id checking going on here. Consider saving these > offsets in the struct sg2042_pcie so you don't need to test > everywhere. Actually, there are not many places in the code to check link_id. If to add storage information in struct sg2042_pcie, at least four u32 are needed. And this logic will only be called one time in the probe. So I think it is better to keep the current method. What do you think? [......]
On Fri, Nov 29, 2024 at 05:51:39PM +0800, Chen Wang wrote: > On 2024/11/13 5:20, Bjorn Helgaas wrote: > > On Mon, Nov 11, 2024 at 01:59:56PM +0800, Chen Wang wrote: > > > From: Chen Wang <unicorn_wang@outlook.com> > > > > > > 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. > > > +++ b/drivers/pci/controller/cadence/Kconfig > > > @@ -67,4 +67,15 @@ config PCI_J721E_EP > > > Say Y here if you want to support the TI J721E PCIe platform > > > controller in endpoint mode. TI J721E PCIe controller uses Cadence PCIe > > > core. > > > + > > > +config PCIE_SG2042 > > > + bool "Sophgo SG2042 PCIe controller (host mode)" > > > + depends on ARCH_SOPHGO || COMPILE_TEST > > > + depends on OF > > > + select PCIE_CADENCE_HOST > > > + help > > > + Say Y here if you want to support the Sophgo SG2042 PCIe platform > > > + controller in host mode. Sophgo SG2042 PCIe controller uses Cadence > > > + PCIe core. > > > > Reorder to keep these menu items in alphabetical order by vendor. > > Sorry, I don't understand your question. I think the menu items in this > Kconfig file are already sorted alphabetically. Here's what menuconfig looks like after this patch: [*] Cadence platform PCIe controller (host mode) [*] Cadence platform PCIe controller (endpoint mode) [*] TI J721E PCIe controller (host mode) [*] TI J721E PCIe controller (endpoint mode) [ ] Sophgo SG2042 PCIe controller (host mode) (NEW) "Sophgo" sorts *before* "TI". > > > + if (pcie->link_id == 1) { > > > + regmap_write(pcie->syscon, REG_LINK1_MSI_ADDR_LOW, > > > + lower_32_bits(pcie->msi_phys)); > > > + regmap_write(pcie->syscon, REG_LINK1_MSI_ADDR_HIGH, > > > + upper_32_bits(pcie->msi_phys)); > > > + > > > + regmap_read(pcie->syscon, REG_LINK1_MSI_ADDR_SIZE, &value); > > > + value = (value & REG_LINK1_MSI_ADDR_SIZE_MASK) | MAX_MSI_IRQS; > > > + regmap_write(pcie->syscon, REG_LINK1_MSI_ADDR_SIZE, value); > > > + } else { > > > + regmap_write(pcie->syscon, REG_LINK0_MSI_ADDR_LOW, > > > + lower_32_bits(pcie->msi_phys)); > > > + regmap_write(pcie->syscon, REG_LINK0_MSI_ADDR_HIGH, > > > + upper_32_bits(pcie->msi_phys)); > > > + > > > + regmap_read(pcie->syscon, REG_LINK0_MSI_ADDR_SIZE, &value); > > > + value = (value & REG_LINK0_MSI_ADDR_SIZE_MASK) | (MAX_MSI_IRQS << 16); > > > + regmap_write(pcie->syscon, REG_LINK0_MSI_ADDR_SIZE, value); > > > + } > > > > Lot of pcie->link_id checking going on here. Consider saving these > > offsets in the struct sg2042_pcie so you don't need to test > > everywhere. > > Actually, there are not many places in the code to check link_id. If to add > storage information in struct sg2042_pcie, at least four u32 are needed. > And this logic will only be called one time in the probe. So I think it is > better to keep the current method. What do you think? 1) I don't think "link_id" is a very good name since it appears to refer to one of two PCIe Root Ports. mvebu uses "marvell,pcie-port" which looks like a similar usage, although unnecessarily Marvell-specific. And arch/arm/boot/dts/marvell/armada-370-db.dts has separate stanzas for two Root Ports, without needing a property to distinguish them, which would be even better. I'm not sure why arch/arm/boot/dts/marvell/armada-370.dtsi needs "marvell,pcie-port" even though it also has separate stanzas. 2) I think checking pcie->link_id is likely to be harder to maintain in the future, e.g., if a new chip adds more Root Ports, you'll have to touch each use. Bjorn
On 2024/11/30 3:50, Bjorn Helgaas wrote: > On Fri, Nov 29, 2024 at 05:51:39PM +0800, Chen Wang wrote: >> On 2024/11/13 5:20, Bjorn Helgaas wrote: >>> On Mon, Nov 11, 2024 at 01:59:56PM +0800, Chen Wang wrote: >>>> From: Chen Wang <unicorn_wang@outlook.com> >>>> >>>> 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. >>>> +++ b/drivers/pci/controller/cadence/Kconfig >>>> @@ -67,4 +67,15 @@ config PCI_J721E_EP >>>> Say Y here if you want to support the TI J721E PCIe platform >>>> controller in endpoint mode. TI J721E PCIe controller uses Cadence PCIe >>>> core. >>>> + >>>> +config PCIE_SG2042 >>>> + bool "Sophgo SG2042 PCIe controller (host mode)" >>>> + depends on ARCH_SOPHGO || COMPILE_TEST >>>> + depends on OF >>>> + select PCIE_CADENCE_HOST >>>> + help >>>> + Say Y here if you want to support the Sophgo SG2042 PCIe platform >>>> + controller in host mode. Sophgo SG2042 PCIe controller uses Cadence >>>> + PCIe core. >>> Reorder to keep these menu items in alphabetical order by vendor. >> Sorry, I don't understand your question. I think the menu items in this >> Kconfig file are already sorted alphabetically. > Here's what menuconfig looks like after this patch: > > [*] Cadence platform PCIe controller (host mode) > [*] Cadence platform PCIe controller (endpoint mode) > [*] TI J721E PCIe controller (host mode) > [*] TI J721E PCIe controller (endpoint mode) > [ ] Sophgo SG2042 PCIe controller (host mode) (NEW) > > "Sophgo" sorts *before* "TI". I see what you mean. I thought we just had to make sure the item IDs in the Kconfig file were in alphabetical order. I will make changes in the next version based on your suggestions. >>>> + if (pcie->link_id == 1) { >>>> + regmap_write(pcie->syscon, REG_LINK1_MSI_ADDR_LOW, >>>> + lower_32_bits(pcie->msi_phys)); >>>> + regmap_write(pcie->syscon, REG_LINK1_MSI_ADDR_HIGH, >>>> + upper_32_bits(pcie->msi_phys)); >>>> + >>>> + regmap_read(pcie->syscon, REG_LINK1_MSI_ADDR_SIZE, &value); >>>> + value = (value & REG_LINK1_MSI_ADDR_SIZE_MASK) | MAX_MSI_IRQS; >>>> + regmap_write(pcie->syscon, REG_LINK1_MSI_ADDR_SIZE, value); >>>> + } else { >>>> + regmap_write(pcie->syscon, REG_LINK0_MSI_ADDR_LOW, >>>> + lower_32_bits(pcie->msi_phys)); >>>> + regmap_write(pcie->syscon, REG_LINK0_MSI_ADDR_HIGH, >>>> + upper_32_bits(pcie->msi_phys)); >>>> + >>>> + regmap_read(pcie->syscon, REG_LINK0_MSI_ADDR_SIZE, &value); >>>> + value = (value & REG_LINK0_MSI_ADDR_SIZE_MASK) | (MAX_MSI_IRQS << 16); >>>> + regmap_write(pcie->syscon, REG_LINK0_MSI_ADDR_SIZE, value); >>>> + } >>> Lot of pcie->link_id checking going on here. Consider saving these >>> offsets in the struct sg2042_pcie so you don't need to test >>> everywhere. >> Actually, there are not many places in the code to check link_id. If to add >> storage information in struct sg2042_pcie, at least four u32 are needed. >> And this logic will only be called one time in the probe. So I think it is >> better to keep the current method. What do you think? > 1) I don't think "link_id" is a very good name since it appears to > refer to one of two PCIe Root Ports. mvebu uses "marvell,pcie-port" > which looks like a similar usage, although unnecessarily > Marvell-specific. And arch/arm/boot/dts/marvell/armada-370-db.dts has > separate stanzas for two Root Ports, without needing a property to > distinguish them, which would be even better. I'm not sure why > arch/arm/boot/dts/marvell/armada-370.dtsi needs "marvell,pcie-port" > even though it also has separate stanzas. > > 2) I think checking pcie->link_id is likely to be harder to maintain > in the future, e.g., if a new chip adds more Root Ports, you'll have > to touch each use. > > Bjorn Thanks for your input. I looked at the Marvell solution you recommended. It seems that it is a relatively complex IP design to support mvebu, which can flexibly support the export of 1 to 4 variable root ports. The Cadence IP used by SG2042 is much simpler, and it is fixed to export up to two root ports. So I plan to implement it in a simple way. Changing "link_id" to "pcie-port" will look better. The writing of link actually refers to the terminology in the Cadence manual, which is actually the concept of port. By the way, there is no demand for scalability at present, because it is said that Sophgo currently only uses Cadence IP for the SG2042 SoC, and subsequent products should switch to solutions from other suppliers. Any question please let me know. Regards, Chen
diff --git a/drivers/pci/controller/cadence/Kconfig b/drivers/pci/controller/cadence/Kconfig index 8a0044bb3989..45a16215ea94 100644 --- a/drivers/pci/controller/cadence/Kconfig +++ b/drivers/pci/controller/cadence/Kconfig @@ -67,4 +67,15 @@ config PCI_J721E_EP Say Y here if you want to support the TI J721E PCIe platform controller in endpoint mode. TI J721E PCIe controller uses Cadence PCIe core. + +config PCIE_SG2042 + bool "Sophgo SG2042 PCIe controller (host mode)" + depends on ARCH_SOPHGO || COMPILE_TEST + depends on OF + select PCIE_CADENCE_HOST + help + Say Y here if you want to support the Sophgo SG2042 PCIe platform + controller in host mode. Sophgo SG2042 PCIe controller uses Cadence + PCIe core. + endmenu diff --git a/drivers/pci/controller/cadence/Makefile b/drivers/pci/controller/cadence/Makefile index 9bac5fb2f13d..89aa316f54ac 100644 --- a/drivers/pci/controller/cadence/Makefile +++ b/drivers/pci/controller/cadence/Makefile @@ -4,3 +4,4 @@ obj-$(CONFIG_PCIE_CADENCE_HOST) += pcie-cadence-host.o obj-$(CONFIG_PCIE_CADENCE_EP) += pcie-cadence-ep.o obj-$(CONFIG_PCIE_CADENCE_PLAT) += pcie-cadence-plat.o obj-$(CONFIG_PCI_J721E) += pci-j721e.o +obj-$(CONFIG_PCIE_SG2042) += pcie-sg2042.o \ No newline at end of file diff --git a/drivers/pci/controller/cadence/pcie-sg2042.c b/drivers/pci/controller/cadence/pcie-sg2042.c new file mode 100644 index 000000000000..809edb8e7259 --- /dev/null +++ b/drivers/pci/controller/cadence/pcie-sg2042.c @@ -0,0 +1,611 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * pcie-sg2042 - PCIe controller driver for Sophgo SG2042 SoC + * + * Copyright (C) 2024 Sophgo Technology Inc. + * Copyright (C) 2024 Chen Wang <unicorn_wang@outlook.com> + */ + +#include <linux/bits.h> +#include <linux/irq.h> +#include <linux/irqchip/chained_irq.h> +#include <linux/irqdomain.h> +#include <linux/kernel.h> +#include <linux/mfd/syscon.h> +#include <linux/msi.h> +#include <linux/of_address.h> +#include <linux/of_irq.h> +#include <linux/of_pci.h> +#include <linux/platform_device.h> +#include <linux/pm_runtime.h> +#include <linux/regmap.h> + +#include "pcie-cadence.h" + +/* + * SG2042 PCIe controller supports two ways to report MSI: + * - Method A, the PICe controller implements an MSI interrupt controller inside, + * and connect to PLIC upward through one interrupt line. Provides + * memory-mapped msi address, and by programming the upper 32 bits of the + * address to zero, it can be compatible with old pcie devices that only + * support 32-bit msi address. + * - Method B, the PICe controller connects to PLIC upward through an + * independent MSI controller "sophgo,sg2042-msi" on the SOC. The MSI + * controller provides multiple(up to 32) interrupt sources to PLIC. + * Compared with the first method, the advantage is that the interrupt source + * is expanded, but because for SG2042, the msi address provided by the MSI + * controller is fixed and only supports 64-bit address(> 2^32), it is not + * compatible with old pcie devices that only support 32-bit msi address. + * Method A & B can be configured in DTS with property "sophgo,internal-msi", + * default is Method B. + */ + +#define MAX_MSI_IRQS 8 +#define MAX_MSI_IRQS_PER_CTRL 1 +#define MAX_MSI_CTRLS (MAX_MSI_IRQS / MAX_MSI_IRQS_PER_CTRL) +#define MSI_DEF_NUM_VECTORS MAX_MSI_IRQS +#define BYTE_NUM_PER_MSI_VEC 4 + +#define REG_CLEAR 0x0804 +#define REG_STATUS 0x0810 +#define REG_LINK0_MSI_ADDR_SIZE 0x085C +#define REG_LINK1_MSI_ADDR_SIZE 0x080C +#define REG_LINK0_MSI_ADDR_LOW 0x0860 +#define REG_LINK0_MSI_ADDR_HIGH 0x0864 +#define REG_LINK1_MSI_ADDR_LOW 0x0868 +#define REG_LINK1_MSI_ADDR_HIGH 0x086C + +#define REG_CLEAR_LINK0_BIT 2 +#define REG_CLEAR_LINK1_BIT 3 +#define REG_STATUS_LINK0_BIT 2 +#define REG_STATUS_LINK1_BIT 3 + +#define REG_LINK0_MSI_ADDR_SIZE_MASK GENMASK(15, 0) +#define REG_LINK1_MSI_ADDR_SIZE_MASK GENMASK(31, 16) + +#define SG2042_CDNS_PLAT_CPU_TO_BUS_ADDR 0xCFFFFFFFFF + +struct sg2042_pcie { + struct cdns_pcie *cdns_pcie; + + struct regmap *syscon; + + u32 link_id; + u32 internal_msi; /* Flag if use internal MSI controller, default external */ + + struct irq_domain *msi_domain; + + int msi_irq; + + dma_addr_t msi_phys; + void *msi_virt; + + u32 num_applied_vecs; /* number of applied vectors, used to speed up ISR */ + + raw_spinlock_t lock; + DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS); +}; + +static void sg2042_pcie_msi_irq_mask_external(struct irq_data *d) +{ + pci_msi_mask_irq(d); + irq_chip_mask_parent(d); +} + +static void sg2042_pcie_msi_irq_unmask_external(struct irq_data *d) +{ + pci_msi_unmask_irq(d); + irq_chip_unmask_parent(d); +} + +static struct irq_chip sg2042_pcie_msi_chip_external = { + .name = "SG2042 PCIe MSI External", + .irq_ack = irq_chip_ack_parent, + .irq_mask = sg2042_pcie_msi_irq_mask_external, + .irq_unmask = sg2042_pcie_msi_irq_unmask_external, +}; + +static struct msi_domain_info sg2042_pcie_msi_domain_info_external = { + .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS), + .chip = &sg2042_pcie_msi_chip_external, +}; + +static struct irq_chip sg2042_pcie_msi_chip = { + .name = "SG2042 PCIe MSI", + .irq_ack = irq_chip_ack_parent, +}; + +static struct msi_domain_info sg2042_pcie_msi_domain_info = { + .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS), + .chip = &sg2042_pcie_msi_chip, +}; + +static void sg2042_pcie_msi_clear_status(struct sg2042_pcie *pcie) +{ + u32 status, clr_msi_in_bit; + + if (pcie->link_id == 1) + clr_msi_in_bit = BIT(REG_CLEAR_LINK1_BIT); + else + clr_msi_in_bit = BIT(REG_CLEAR_LINK0_BIT); + + regmap_read(pcie->syscon, REG_CLEAR, &status); + status |= clr_msi_in_bit; + regmap_write(pcie->syscon, REG_CLEAR, status); + + /* need write 0 to reset, hardware can not reset automatically */ + status &= ~clr_msi_in_bit; + regmap_write(pcie->syscon, REG_CLEAR, status); +} + +static int sg2042_pcie_msi_irq_set_affinity(struct irq_data *d, + const struct cpumask *mask, + bool force) +{ + if (d->parent_data) + return irq_chip_set_affinity_parent(d, mask, force); + + return -EINVAL; +} + +static void sg2042_pcie_msi_irq_compose_msi_msg(struct irq_data *d, + struct msi_msg *msg) +{ + struct sg2042_pcie *pcie = irq_data_get_irq_chip_data(d); + struct device *dev = pcie->cdns_pcie->dev; + + msg->address_lo = lower_32_bits(pcie->msi_phys) + BYTE_NUM_PER_MSI_VEC * d->hwirq; + msg->address_hi = upper_32_bits(pcie->msi_phys); + msg->data = 1; + + pcie->num_applied_vecs = d->hwirq; + + dev_info(dev, "compose msi msg hwirq[%d] address_hi[%#x] address_lo[%#x]\n", + (int)d->hwirq, msg->address_hi, msg->address_lo); +} + +static void sg2042_pcie_msi_irq_ack(struct irq_data *d) +{ + struct sg2042_pcie *pcie = irq_data_get_irq_chip_data(d); + + sg2042_pcie_msi_clear_status(pcie); +} + +static struct irq_chip sg2042_pcie_msi_bottom_chip = { + .name = "SG2042 PCIe PLIC-MSI translator", + .irq_ack = sg2042_pcie_msi_irq_ack, + .irq_compose_msi_msg = sg2042_pcie_msi_irq_compose_msi_msg, +#ifdef CONFIG_SMP + .irq_set_affinity = sg2042_pcie_msi_irq_set_affinity, +#endif +}; + +static int sg2042_pcie_irq_domain_alloc(struct irq_domain *domain, + unsigned int virq, unsigned int nr_irqs, + void *args) +{ + struct sg2042_pcie *pcie = domain->host_data; + unsigned long flags; + u32 i; + int bit; + + raw_spin_lock_irqsave(&pcie->lock, flags); + + bit = bitmap_find_free_region(pcie->msi_irq_in_use, MSI_DEF_NUM_VECTORS, + order_base_2(nr_irqs)); + + raw_spin_unlock_irqrestore(&pcie->lock, flags); + + if (bit < 0) + return -ENOSPC; + + for (i = 0; i < nr_irqs; i++) + irq_domain_set_info(domain, virq + i, bit + i, + &sg2042_pcie_msi_bottom_chip, + pcie, handle_edge_irq, + NULL, NULL); + + return 0; +} + +static void sg2042_pcie_irq_domain_free(struct irq_domain *domain, + unsigned int virq, unsigned int nr_irqs) +{ + struct irq_data *d = irq_domain_get_irq_data(domain, virq); + struct sg2042_pcie *pcie = irq_data_get_irq_chip_data(d); + unsigned long flags; + + raw_spin_lock_irqsave(&pcie->lock, flags); + + bitmap_release_region(pcie->msi_irq_in_use, d->hwirq, + order_base_2(nr_irqs)); + + raw_spin_unlock_irqrestore(&pcie->lock, flags); +} + +static const struct irq_domain_ops sg2042_pcie_msi_domain_ops = { + .alloc = sg2042_pcie_irq_domain_alloc, + .free = sg2042_pcie_irq_domain_free, +}; + +/* + * We use the usual two domain structure, the top one being a generic PCI/MSI + * domain, the bottom one being SG2042-specific and handling the actual HW + * interrupt allocation. + * At the same time, for internal MSI controller(Method A), bottom chip uses a + * chained handler to handle the controller's MSI IRQ edge triggered. + */ +static int sg2042_pcie_create_msi_domain(struct sg2042_pcie *pcie, + struct irq_domain *parent) +{ + struct device *dev = pcie->cdns_pcie->dev; + struct fwnode_handle *fwnode = of_node_to_fwnode(dev->of_node); + + if (pcie->internal_msi) + pcie->msi_domain = pci_msi_create_irq_domain(fwnode, + &sg2042_pcie_msi_domain_info, + parent); + + else + pcie->msi_domain = pci_msi_create_irq_domain(fwnode, + &sg2042_pcie_msi_domain_info_external, + parent); + + if (!pcie->msi_domain) { + dev_err(dev, "Failed to create MSI domain\n"); + return -ENOMEM; + } + + return 0; +} + +static int sg2042_pcie_setup_msi_external(struct sg2042_pcie *pcie) +{ + struct device *dev = pcie->cdns_pcie->dev; + struct device_node *np = dev->of_node; + struct irq_domain *parent_domain; + struct device_node *parent_np; + + if (!of_find_property(np, "interrupt-parent", NULL)) { + dev_err(dev, "Can't find interrupt-parent!\n"); + return -EINVAL; + } + + parent_np = of_irq_find_parent(np); + if (!parent_np) { + dev_err(dev, "Can't find node of interrupt-parent!\n"); + return -ENXIO; + } + + parent_domain = irq_find_host(parent_np); + of_node_put(parent_np); + if (!parent_domain) { + dev_err(dev, "Can't find domain of interrupt-parent!\n"); + return -ENXIO; + } + + return sg2042_pcie_create_msi_domain(pcie, parent_domain); +} + +static int sg2042_pcie_init_msi_data(struct sg2042_pcie *pcie) +{ + struct device *dev = pcie->cdns_pcie->dev; + u32 value; + int ret; + + raw_spin_lock_init(&pcie->lock); + + /* + * Though the PCIe controller can address >32-bit address space, to + * facilitate endpoints that support only 32-bit MSI target address, + * the mask is set to 32-bit to make sure that MSI target address is + * always a 32-bit address + */ + ret = dma_set_coherent_mask(dev, DMA_BIT_MASK(32)); + if (ret < 0) + return ret; + + pcie->msi_virt = dma_alloc_coherent(dev, BYTE_NUM_PER_MSI_VEC * MAX_MSI_IRQS, + &pcie->msi_phys, GFP_KERNEL); + if (!pcie->msi_virt) + return -ENOMEM; + + /* Program the msi address and size */ + if (pcie->link_id == 1) { + regmap_write(pcie->syscon, REG_LINK1_MSI_ADDR_LOW, + lower_32_bits(pcie->msi_phys)); + regmap_write(pcie->syscon, REG_LINK1_MSI_ADDR_HIGH, + upper_32_bits(pcie->msi_phys)); + + regmap_read(pcie->syscon, REG_LINK1_MSI_ADDR_SIZE, &value); + value = (value & REG_LINK1_MSI_ADDR_SIZE_MASK) | MAX_MSI_IRQS; + regmap_write(pcie->syscon, REG_LINK1_MSI_ADDR_SIZE, value); + } else { + regmap_write(pcie->syscon, REG_LINK0_MSI_ADDR_LOW, + lower_32_bits(pcie->msi_phys)); + regmap_write(pcie->syscon, REG_LINK0_MSI_ADDR_HIGH, + upper_32_bits(pcie->msi_phys)); + + regmap_read(pcie->syscon, REG_LINK0_MSI_ADDR_SIZE, &value); + value = (value & REG_LINK0_MSI_ADDR_SIZE_MASK) | (MAX_MSI_IRQS << 16); + regmap_write(pcie->syscon, REG_LINK0_MSI_ADDR_SIZE, value); + } + + return 0; +} + +static irqreturn_t sg2042_pcie_msi_handle_irq(struct sg2042_pcie *pcie) +{ + u32 i, pos; + unsigned long val; + u32 status, num_vectors; + irqreturn_t ret = IRQ_NONE; + + num_vectors = pcie->num_applied_vecs; + for (i = 0; i <= num_vectors; i++) { + status = readl((void *)(pcie->msi_virt + i * BYTE_NUM_PER_MSI_VEC)); + if (!status) + continue; + + ret = IRQ_HANDLED; + val = status; + pos = 0; + while ((pos = find_next_bit(&val, MAX_MSI_IRQS_PER_CTRL, + pos)) != MAX_MSI_IRQS_PER_CTRL) { + generic_handle_domain_irq(pcie->msi_domain->parent, + (i * MAX_MSI_IRQS_PER_CTRL) + + pos); + pos++; + } + writel(0, ((void *)(pcie->msi_virt) + i * BYTE_NUM_PER_MSI_VEC)); + } + return ret; +} + +static void sg2042_pcie_msi_chained_isr(struct irq_desc *desc) +{ + struct irq_chip *chip = irq_desc_get_chip(desc); + u32 status, st_msi_in_bit; + struct sg2042_pcie *pcie; + + chained_irq_enter(chip, desc); + + pcie = irq_desc_get_handler_data(desc); + if (pcie->link_id == 1) + st_msi_in_bit = REG_STATUS_LINK1_BIT; + else + st_msi_in_bit = REG_STATUS_LINK0_BIT; + + regmap_read(pcie->syscon, REG_STATUS, &status); + if ((status >> st_msi_in_bit) & 0x1) { + sg2042_pcie_msi_clear_status(pcie); + + sg2042_pcie_msi_handle_irq(pcie); + } + + chained_irq_exit(chip, desc); +} + +static int sg2042_pcie_setup_msi(struct sg2042_pcie *pcie, struct platform_device *pdev) +{ + struct device *dev = pcie->cdns_pcie->dev; + struct fwnode_handle *fwnode = of_node_to_fwnode(dev->of_node); + struct irq_domain *parent_domain; + int ret = 0; + + parent_domain = irq_domain_create_linear(fwnode, MSI_DEF_NUM_VECTORS, + &sg2042_pcie_msi_domain_ops, pcie); + if (!parent_domain) { + dev_err(dev, "Failed to create IRQ domain\n"); + return -ENOMEM; + } + irq_domain_update_bus_token(parent_domain, DOMAIN_BUS_NEXUS); + + ret = sg2042_pcie_create_msi_domain(pcie, parent_domain); + if (ret) { + irq_domain_remove(parent_domain); + return ret; + } + + ret = sg2042_pcie_init_msi_data(pcie); + if (ret) { + dev_err(dev, "Failed to initialize msi data!\n"); + return ret; + } + + ret = platform_get_irq_byname(pdev, "msi"); + if (ret <= 0) { + dev_err(dev, "failed to get MSI irq\n"); + return ret; + } + pcie->msi_irq = ret; + + irq_set_chained_handler_and_data(pcie->msi_irq, + sg2042_pcie_msi_chained_isr, pcie); + + return 0; +} + +static void sg2042_pcie_free_msi(struct sg2042_pcie *pcie) +{ + struct device *dev = pcie->cdns_pcie->dev; + + if (pcie->msi_irq) + irq_set_chained_handler_and_data(pcie->msi_irq, NULL, NULL); + + if (pcie->msi_virt) + dma_free_coherent(dev, BYTE_NUM_PER_MSI_VEC * MAX_MSI_IRQS, + pcie->msi_virt, pcie->msi_phys); +} + +static u64 sg2042_cdns_pcie_cpu_addr_fixup(struct cdns_pcie *pcie, u64 cpu_addr) +{ + return cpu_addr & SG2042_CDNS_PLAT_CPU_TO_BUS_ADDR; +} + +static const struct cdns_pcie_ops sg2042_cdns_pcie_ops = { + .cpu_addr_fixup = sg2042_cdns_pcie_cpu_addr_fixup, +}; + +/* + * SG2042 only support 4-byte aligned access, so for the rootbus (i.e. to read + * the PCIe controller itself, read32 is required. For non-rootbus (i.e. to read + * the PCIe peripheral registers, supports 1/2/4 byte aligned access, so + * directly use read should be fine. + * The same is true for write. + */ +static int sg2042_pcie_config_read(struct pci_bus *bus, unsigned int devfn, + int where, int size, u32 *value) +{ + if (pci_is_root_bus(bus)) + return pci_generic_config_read32(bus, devfn, where, size, + value); + + return pci_generic_config_read(bus, devfn, where, size, value); +} + +static int sg2042_pcie_config_write(struct pci_bus *bus, unsigned int devfn, + int where, int size, u32 value) +{ + if (pci_is_root_bus(bus)) + return pci_generic_config_write32(bus, devfn, where, size, + value); + + return pci_generic_config_write(bus, devfn, where, size, value); +} + +static struct pci_ops sg2042_pcie_host_ops = { + .map_bus = cdns_pci_map_bus, + .read = sg2042_pcie_config_read, + .write = sg2042_pcie_config_write, +}; + +static int sg2042_pcie_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct device_node *np = dev->of_node; + struct pci_host_bridge *bridge; + struct device_node *np_syscon; + struct cdns_pcie *cdns_pcie; + struct sg2042_pcie *pcie; + struct cdns_pcie_rc *rc; + struct regmap *syscon; + int ret; + + if (!IS_ENABLED(CONFIG_PCIE_CADENCE_HOST)) + return -ENODEV; + + pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL); + if (!pcie) + return -ENOMEM; + + bridge = devm_pci_alloc_host_bridge(dev, sizeof(*rc)); + if (!bridge) { + dev_err(dev, "Failed to alloc host bridge!\n"); + return -ENOMEM; + } + bridge->ops = &sg2042_pcie_host_ops; + + rc = pci_host_bridge_priv(bridge); + cdns_pcie = &rc->pcie; + cdns_pcie->dev = dev; + cdns_pcie->ops = &sg2042_cdns_pcie_ops; + pcie->cdns_pcie = cdns_pcie; + + np_syscon = of_parse_phandle(np, "sophgo,syscon-pcie-ctrl", 0); + if (!np_syscon) { + dev_err(dev, "Failed to get syscon node\n"); + return -ENOMEM; + } + syscon = syscon_node_to_regmap(np_syscon); + if (IS_ERR(syscon)) { + dev_err(dev, "Failed to get regmap for syscon\n"); + return -ENOMEM; + } + pcie->syscon = syscon; + + if (of_property_read_u32(np, "sophgo,link-id", &pcie->link_id)) { + dev_err(dev, "Unable to parse link ID\n"); + return -EINVAL; + } + + pcie->internal_msi = 0; + if (of_property_read_bool(np, "sophgo,internal-msi")) + pcie->internal_msi = 1; + + 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; + } + + if (pcie->internal_msi) { + ret = sg2042_pcie_setup_msi(pcie, pdev); + if (ret < 0) + goto err_setup_msi; + } else { + ret = sg2042_pcie_setup_msi_external(pcie); + if (ret < 0) + goto err_setup_msi; + } + + ret = cdns_pcie_init_phy(dev, cdns_pcie); + if (ret) { + dev_err(dev, "Failed to init phy!\n"); + goto err_setup_msi; + } + + 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(cdns_pcie); + +err_setup_msi: + sg2042_pcie_free_msi(pcie); + +err_get_sync: + pm_runtime_put(dev); + pm_runtime_disable(dev); + + return ret; +} + +static void sg2042_pcie_shutdown(struct platform_device *pdev) +{ + struct sg2042_pcie *pcie = platform_get_drvdata(pdev); + struct cdns_pcie *cdns_pcie = pcie->cdns_pcie; + struct device *dev = &pdev->dev; + + sg2042_pcie_free_msi(pcie); + + cdns_pcie_disable_phy(cdns_pcie); + + pm_runtime_put(dev); + pm_runtime_disable(dev); +} + +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, + .shutdown = sg2042_pcie_shutdown, +}; +builtin_platform_driver(sg2042_pcie_driver);