> > 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
>
Agreed.
> > + 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?
No, I will remove it in next version.
>
> > +#include <linux/of_address.h>
>
> Where do you use it?
>
No, I will remove it in next version.
>
> > +#include <linux/of_irq.h>
>
> Where do you use it?
>
No, I will remove it in next version.
>
> > +#include <linux/of_pci.h>
>
> Where do you use it?
>
No, I will remove it in next version.
> > +#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.
>
Agreed.
I will add this type in next version.
> > + int irq, ret;
> > +
> > + if (!md)
> > + return -ENODEV
...
> > +
> > + 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.
>
I will change it in next version.
> > + 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?
>
Agreed.
> > + .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?
>
Agreed.
> > + .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?
>
I will remove it in next version.
> > + .of_match_table = aspeed_pcie_of_match,
> > + },
> > + .probe = aspeed_pcie_probe,
> > + .remove = aspeed_pcie_remove,
>
> So how exactly remove can be triggered?
>
Agreed.
I think it may be triggered when rebooting.
I will remove it in next version.
Thanks,
Jacky
