On 13/06/2025 05:30, Jacky Chou wrote:
> Introduce PCIe Root Complex driver for ASPEED SoCs. Support RC
> initialization, reset, clock, IRQ domain, and MSI domain setup.
> Implement platform-specific setup and register configuration for
> ASPEED. And provide PCI config space read/write and INTx/MSI
> interrupt handling.
>
> Signed-off-by: Jacky Chou <jacky_chou@xxxxxxxxxxxxxx>
> ---
> drivers/pci/controller/Kconfig | 13 +
> drivers/pci/controller/Makefile | 1 +
> drivers/pci/controller/pcie-aspeed.c | 1039 ++++++++++++++++++++++++++
> 3 files changed, 1053 insertions(+)
> create mode 100644 drivers/pci/controller/pcie-aspeed.c
>
> diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> index 886f6f43a895..f6b5eea3b570 100644
> --- a/drivers/pci/controller/Kconfig
> +++ b/drivers/pci/controller/Kconfig
> @@ -216,6 +216,19 @@ config PCIE_MT7621
> help
> This selects a driver for the MediaTek MT7621 PCIe Controller.
>
> +config PCIE_ASPEED
> + bool "ASPEED PCIe controller"
> + depends on PCI
depends ARCH_ASPEED || COMPILE_TEST
> + depends on OF || COMPILE_TEST
> + select PCI_MSI_ARCH_FALLBACKS
> + help
> + Enable this option to add support for the PCIe controller
> + found on ASPEED SoCs.
> + This driver provides initialization and management for PCIe
> + Root Complex functionality, including interrupt and MSI support.
> + Select Y if your platform uses an ASPEED SoC and requires PCIe
> + connectivity.
> +
> config PCI_HYPERV_INTERFACE
> tristate "Microsoft Hyper-V PCI Interface"
> depends on ((X86 && X86_64) || ARM64) && HYPERV && PCI_MSI
> diff --git a/drivers/pci/controller/Makefile b/drivers/pci/controller/Makefile
> index 038ccbd9e3ba..1339f88e153d 100644
> --- a/drivers/pci/controller/Makefile
> +++ b/drivers/pci/controller/Makefile
> @@ -39,6 +39,7 @@ obj-$(CONFIG_PCI_LOONGSON) += pci-loongson.o
> obj-$(CONFIG_PCIE_HISI_ERR) += pcie-hisi-error.o
> obj-$(CONFIG_PCIE_APPLE) += pcie-apple.o
> obj-$(CONFIG_PCIE_MT7621) += pcie-mt7621.o
> +obj-$(CONFIG_PCIE_ASPEED) += pcie-aspeed.o
>
> # pcie-hisi.o quirks are needed even without CONFIG_PCIE_DW
> obj-y += dwc/
> diff --git a/drivers/pci/controller/pcie-aspeed.c b/drivers/pci/controller/pcie-aspeed.c
> new file mode 100644
> index 000000000000..c745684a7f9b
> --- /dev/null
> +++ b/drivers/pci/controller/pcie-aspeed.c
> @@ -0,0 +1,1039 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2025 Aspeed Technology Inc.
> + */
> +#include <linux/irqchip/chained_irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/kernel.h>
> +#include <linux/msi.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/of_platform.h>
Where do you use it?
> +#include <linux/of_address.h>
Where do you use it?
> +#include <linux/of_irq.h>
Where do you use it?
> +#include <linux/of_pci.h>
Where do you use it?
> +#include <linux/pci.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +#include <linux/irq.h>
> +#include <linux/interrupt.h>
> +#include <linux/workqueue.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +
...
> +
> +static int aspeed_pcie_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct pci_host_bridge *host;
> + struct aspeed_pcie *pcie;
> + struct device_node *node = dev->of_node;
> + const void *md = of_device_get_match_data(dev);
Not void, but specific type. This is not Javascript, we have here types.
> + int irq, ret;
> +
> + if (!md)
> + return -ENODEV;
> +
> + host = devm_pci_alloc_host_bridge(dev, sizeof(*pcie));
> + if (!host)
> + return -ENOMEM;
> +
> + pcie = pci_host_bridge_priv(host);
> + pcie->dev = dev;
> + pcie->tx_tag = 0;
> + platform_set_drvdata(pdev, pcie);
> +
> + pcie->platform = md;
> + pcie->host = host;
> +
> + pcie->reg = devm_platform_ioremap_resource(pdev, 0);
> +
> + of_property_read_u32(node, "msi_address", &pcie->msi_address);
> + of_property_read_u32(node, "linux,pci-domain", &pcie->domain);
> +
> + pcie->cfg = syscon_regmap_lookup_by_phandle(dev->of_node, "aspeed,pciecfg");
> + if (IS_ERR(pcie->cfg))
> + return dev_err_probe(dev, PTR_ERR(pcie->cfg), "Failed to map pciecfg base\n");
> +
> + pcie->pciephy = syscon_regmap_lookup_by_phandle(node, "aspeed,pciephy");
> + if (IS_ERR(pcie->pciephy))
> + return dev_err_probe(dev, PTR_ERR(pcie->pciephy), "Failed to map pciephy base\n");
> +
> + pcie->h2xrst = devm_reset_control_get_exclusive(dev, "h2x");
> + if (IS_ERR(pcie->h2xrst))
> + return dev_err_probe(dev, PTR_ERR(pcie->h2xrst), "Failed to get h2x reset\n");
> +
> + pcie->perst = devm_reset_control_get_exclusive(dev, "perst");
> + if (IS_ERR(pcie->perst))
> + return dev_err_probe(dev, PTR_ERR(pcie->perst), "Failed to get perst reset\n");
> +
> + ret = pcie->platform->setup(pdev);
> + if (ret)
> + goto err_setup;
> +
> + host->sysdata = pcie;
> +
> + ret = aspeed_pcie_init_irq_domain(pcie);
> + if (ret)
> + goto err_irq_init;
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0)
> + goto err_irq;
> +
> + ret = devm_request_irq(dev, irq, aspeed_pcie_intr_handler, IRQF_SHARED, dev_name(dev),
> + pcie);
> + if (ret)
> + goto err_irq;
> +
> + pcie->clock = clk_get(dev, NULL);
Huh...
> + if (IS_ERR(pcie->clock))
> + goto err_clk;
> + ret = clk_prepare_enable(pcie->clock);
devm_clk_get_enabled.
> + if (ret)
> + goto err_clk_enable;
> +
> + ret = pci_host_probe(host);
> + if (ret)
> + goto err_clk_enable;
> +
> + return 0;
> +
> +err_clk_enable:
> + clk_put(pcie->clock);
> +err_clk:
> +err_irq:
> + aspeed_pcie_irq_domain_free(pcie);
> +err_irq_init:
> +err_setup:
> + return dev_err_probe(dev, ret, "Failed to setup PCIe RC\n");
> +}
> +
> +static void aspeed_pcie_remove(struct platform_device *pdev)
> +{
> + struct aspeed_pcie *pcie = platform_get_drvdata(pdev);
> +
> + if (pcie->clock) {
> + clk_disable_unprepare(pcie->clock);
> + clk_put(pcie->clock);
> + }
> +
> + pci_stop_root_bus(pcie->host->bus);
> + pci_remove_root_bus(pcie->host->bus);
> + aspeed_pcie_irq_domain_free(pcie);
> +}
> +
> +static struct aspeed_pcie_rc_platform pcie_rc_ast2600 = {
This should be const. Why it cannot?
> + .setup = aspeed_ast2600_setup,
> + .reg_intx_en = 0x04,
> + .reg_intx_sts = 0x08,
> + .reg_msi_en = 0x20,
> + .reg_msi_sts = 0x28,
> +};
> +
> +static struct aspeed_pcie_rc_platform pcie_rc_ast2700 = {
This should be const. Why it cannot?
> + .setup = aspeed_ast2700_setup,
> + .reg_intx_en = 0x40,
> + .reg_intx_sts = 0x48,
> + .reg_msi_en = 0x50,
> + .reg_msi_sts = 0x58,
> +};
> +
> +static const struct of_device_id aspeed_pcie_of_match[] = {
> + { .compatible = "aspeed,ast2600-pcie", .data = &pcie_rc_ast2600 },
> + { .compatible = "aspeed,ast2700-pcie", .data = &pcie_rc_ast2700 },
> + {}
> +};
> +
> +static struct platform_driver aspeed_pcie_driver = {
> + .driver = {
> + .name = "aspeed-pcie",
> + .suppress_bind_attrs = true,
Why?
> + .of_match_table = aspeed_pcie_of_match,
> + },
> + .probe = aspeed_pcie_probe,
> + .remove = aspeed_pcie_remove,
So how exactly remove can be triggered?
> +};
> +
> +module_platform_driver(aspeed_pcie_driver);
> +
> +MODULE_AUTHOR("Jacky Chou <jacky_chou@xxxxxxxxxxxxxx>");
> +MODULE_DESCRIPTION("ASPEED PCIe Root Complex");
> +MODULE_LICENSE("GPL");
Best regards,
Krzysztof
