On Mon, Nov 11, 2024 at 01:59:56PM +0800, 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.
> +++ 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@xxxxxxx/
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
>
