On Wed, Aug 20, 2025 at 07:18:42PM +0800, Randolph Lin wrote:
> Add driver support for DesignWare based PCIe controller in Andes
> QiLai SoC. The driver only supports the Root Complex mode.
>
> Signed-off-by: Randolph Lin <randolph@xxxxxxxxxxxxx>
> ---
> drivers/pci/controller/dwc/Kconfig | 13 +
> drivers/pci/controller/dwc/Makefile | 1 +
> drivers/pci/controller/dwc/pcie-andes-qilai.c | 227 ++++++++++++++++++
> 3 files changed, 241 insertions(+)
> create mode 100644 drivers/pci/controller/dwc/pcie-andes-qilai.c
>
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index ff6b6d9e18ec..a9c5a43f648b 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -49,6 +49,19 @@ config PCIE_AMD_MDB
> DesignWare IP and therefore the driver re-uses the DesignWare
> core functions to implement the driver.
>
> +config PCIE_ANDES_QILAI
> + bool "ANDES QiLai PCIe controller"
> + depends on OF && (RISCV || COMPILE_TEST)
> + depends on PCI_MSI
> + depends on ARCH_ANDES
> + select PCIE_DW_HOST
> + default y if ARCH_ANDES
> + help
> + Say Y here if you want to enable PCIe controller support on Andes
> + QiLai SoCs. The Andes QiLai SoCs PCIe controller is based on
> + DesignWare IP and therefore the driver re-uses the DesignWare
> + core functions to implement the driver.
> +
> config PCI_MESON
> tristate "Amlogic Meson PCIe controller"
> default m if ARCH_MESON
> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> index 6919d27798d1..de9583cbd675 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -5,6 +5,7 @@ obj-$(CONFIG_PCIE_DW_HOST) += pcie-designware-host.o
> obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
> obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
> obj-$(CONFIG_PCIE_AMD_MDB) += pcie-amd-mdb.o
> +obj-$(CONFIG_PCIE_ANDES_QILAI) += pcie-andes-qilai.o
> obj-$(CONFIG_PCIE_BT1) += pcie-bt1.o
> obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
> obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
> diff --git a/drivers/pci/controller/dwc/pcie-andes-qilai.c b/drivers/pci/controller/dwc/pcie-andes-qilai.c
> new file mode 100644
> index 000000000000..dd06eee82cac
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-andes-qilai.c
> @@ -0,0 +1,227 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for the PCIe Controller in QiLai from Andes
> + *
> + * Copyright (C) 2025 Andes Technology Corporation
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/platform_device.h>
> +#include <linux/types.h>
> +
> +#include "pcie-designware.h"
> +
> +#define PCIE_INTR_CONTROL1 0x15c
> +#define PCIE_MSI_CTRL_INT_EN BIT(28)
> +
> +#define PCIE_LOGIC_COHERENCY_CONTROL3 0x8e8
> +/* Write-Back, Read and Write Allocate */
> +#define IOCP_ARCACHE 0xf
> +/* Write-Back, Read and Write Allocate */
> +#define IOCP_AWCACHE 0xf
Are IOCP_ARCACHE and IOCP_AWCACHE supposed to be identical values with
identical comments?
> +#define PCIE_CFG_MSTR_ARCACHE_MODE GENMASK(6, 3)
> +#define PCIE_CFG_MSTR_AWCACHE_MODE GENMASK(14, 11)
> +#define PCIE_CFG_MSTR_ARCACHE_VALUE GENMASK(22, 19)
> +#define PCIE_CFG_MSTR_AWCACHE_VALUE GENMASK(30, 27)
> +
> +#define PCIE_GEN_CONTROL2 0x54
> +#define PCIE_CFG_LTSSM_EN BIT(0)
> +
> +#define PCIE_REGS_PCIE_SII_PM_STATE 0xc0
> +#define SMLH_LINK_UP BIT(6)
> +#define RDLH_LINK_UP BIT(7)
> +#define PCIE_REGS_PCIE_SII_LINK_UP (SMLH_LINK_UP | RDLH_LINK_UP)
> +
> +struct qilai_pcie {
> + struct dw_pcie dw;
> + struct platform_device *pdev;
> + void __iomem *apb_base;
> +};
> +
> +#define to_qilai_pcie(_dw) container_of(_dw, struct qilai_pcie, dw)
> +
> +static u64 qilai_pcie_cpu_addr_fixup(struct dw_pcie *pci, u64 cpu_addr)
> +{
> + struct dw_pcie_rp *pp = &pci->pp;
> +
> + return cpu_addr - pp->cfg0_base;
Sorry, we can't do this. We're removing .cpu_addr_fixup() because
it's a workaround for defects in the DT description. See these
commits, for example:
befc86a0b354 ("PCI: dwc: Use parent_bus_offset to remove need for .cpu_addr_fixup()")
b9812179f601 ("PCI: imx6: Remove imx_pcie_cpu_addr_fixup()")
07ae413e169d ("PCI: intel-gw: Remove intel_pcie_cpu_addr()")
> +}
> +
> +static u32 qilai_pcie_outbound_atu_check(struct dw_pcie *pci,
> + const struct dw_pcie_ob_atu_cfg *atu,
> + u64 *limit_addr)
> +{
> + u64 parent_bus_addr = atu->parent_bus_addr;
> +
> + *limit_addr = parent_bus_addr + atu->size - 1;
> +
> + /*
> + * Addresses below 4 GB are not 1:1 mapped; therefore, range checks
> + * only need to ensure addresses below 4 GB match pci->region_limit.
> + */
> + if (lower_32_bits(*limit_addr & ~pci->region_limit) !=
> + lower_32_bits(parent_bus_addr & ~pci->region_limit) ||
> + !IS_ALIGNED(parent_bus_addr, pci->region_align) ||
> + !IS_ALIGNED(atu->pci_addr, pci->region_align) || !atu->size)
> + return -EINVAL;
> + else
> + return 0;
I'm a little dubious about this because the knowledge about 1:1
mapping feels like something we should know from devicetree. But I'm
not an expert in this area.
If you do need this, write it like this, which I think is slightly
simpler than the if/else:
if (...)
return -EINVAL;
return 0;
> +}
> +
> +static bool qilai_pcie_link_up(struct dw_pcie *pci)
> +{
> + struct qilai_pcie *qlpci = to_qilai_pcie(pci);
Suggest using "pcie" as the name for "struct qilai_pcie *" pointers
because that's a common pattern in other drivers.
> + u32 val;
> +
> + /* Read smlh & rdlh link up by checking debug port */
> + dw_pcie_read(qlpci->apb_base + PCIE_REGS_PCIE_SII_PM_STATE, 0x4, &val);
> +
> + return (val & PCIE_REGS_PCIE_SII_LINK_UP) == PCIE_REGS_PCIE_SII_LINK_UP;
> +}
> +
> +static int qilai_pcie_start_link(struct dw_pcie *pci)
> +{
> + struct qilai_pcie *qlpci = to_qilai_pcie(pci);
> + u32 val;
> +
> + /* Do phy link up */
> + dw_pcie_read(qlpci->apb_base + PCIE_GEN_CONTROL2, 0x4, &val);
> + val |= PCIE_CFG_LTSSM_EN;
> + dw_pcie_write(qlpci->apb_base + PCIE_GEN_CONTROL2, 0x4, val);
> +
> + return 0;
> +}
> +
> +static const struct dw_pcie_ops qilai_pcie_ops = {
> + .cpu_addr_fixup = qilai_pcie_cpu_addr_fixup,
> + .outbound_atu_check = qilai_pcie_outbound_atu_check,
> + .link_up = qilai_pcie_link_up,
> + .start_link = qilai_pcie_start_link,
> +};
> +
> +static struct qilai_pcie *qilai_pcie_create_data(struct platform_device *pdev)
> +{
> + struct qilai_pcie *qlpci;
> +
> + qlpci = devm_kzalloc(&pdev->dev, sizeof(*qlpci), GFP_KERNEL);
> + if (!qlpci)
> + return ERR_PTR(-ENOMEM);
> +
> + qlpci->pdev = pdev;
> + platform_set_drvdata(pdev, qlpci);
> +
> + return qlpci;
Doesn't seem worth a separate function to me. I see this is copied
from bt1_pcie_create_data(), and kudos for paying attention to how
other drivers do things. But I don't think it's really worth it
there, either, and it's the only driver that does that.
Most drivers do it directly in *_pcie_probe(), which also makes it
possible to delay the platform_set_drvdata() until after qlpci is
initialized (and also makes it obvious that you do the
platform_set_drvdata() *twice* :)).
> +}
> +
> +/*
> + * Setup the Qilai PCIe IOCP (IO Coherence Port) Read/Write Behaviors to the
> + * Write-Back, Read and Write Allocate mode.
> + */
> +static void qilai_pcie_iocp_cache_setup(struct dw_pcie_rp *pp)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> + u32 val;
> +
> + dw_pcie_dbi_ro_wr_en(pci);
> +
> + dw_pcie_read(pci->dbi_base + PCIE_LOGIC_COHERENCY_CONTROL3,
> + sizeof(val), &val);
> + val |= FIELD_PREP(PCIE_CFG_MSTR_ARCACHE_MODE, IOCP_ARCACHE);
> + val |= FIELD_PREP(PCIE_CFG_MSTR_AWCACHE_MODE, IOCP_AWCACHE);
> + val |= FIELD_PREP(PCIE_CFG_MSTR_ARCACHE_VALUE, IOCP_ARCACHE);
> + val |= FIELD_PREP(PCIE_CFG_MSTR_AWCACHE_VALUE, IOCP_AWCACHE);
Since you never clear anything in "val", it looks like this assumes
the value you read from PCIE_LOGIC_COHERENCY_CONTROL3 contains zeroes
where you insert the new values. Possibly use FIELD_MODIFY() instead?
> + dw_pcie_write(pci->dbi_base + PCIE_LOGIC_COHERENCY_CONTROL3,
> + sizeof(val), val);
> +
> + dw_pcie_dbi_ro_wr_dis(pci);
> +}
> +
> +static void qilai_pcie_enable_msi(struct qilai_pcie *qlpci)
> +{
> + u32 val;
> +
> + dw_pcie_read(qlpci->apb_base + PCIE_INTR_CONTROL1,
> + sizeof(val), &val);
> + val |= PCIE_MSI_CTRL_INT_EN;
> + dw_pcie_write(qlpci->apb_base + PCIE_INTR_CONTROL1,
> + sizeof(val), val);
Apparently you don't need dw_pcie_dbi_ro_wr_en() and
dw_pcie_dbi_ro_wr_dis() around this? Or maybe this relies on the DWC
core taking care of this, I dunno.
> +}
> +
> +static int qilai_pcie_host_init(struct dw_pcie_rp *pp)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> + struct qilai_pcie *qlpci = to_qilai_pcie(pci);
> +
> + qilai_pcie_enable_msi(qlpci);
> +
> + return 0;
> +}
> +
> +static const struct dw_pcie_host_ops qilai_pcie_host_ops = {
> + .init = qilai_pcie_host_init,
> +};
> +
> +static int qilai_pcie_add_port(struct qilai_pcie *qlpci)
> +{
> + struct device *dev = &qlpci->pdev->dev;
> + struct platform_device *pdev = qlpci->pdev;
> + int ret;
> +
> + qlpci->dw.dev = dev;
> + qlpci->dw.ops = &qilai_pcie_ops;
> + qlpci->dw.pp.num_vectors = MAX_MSI_IRQS;
> + qlpci->dw.pp.ops = &qilai_pcie_host_ops;
> +
> + dw_pcie_cap_set(&qlpci->dw, REQ_RES);
> +
> + qlpci->apb_base = devm_platform_ioremap_resource_byname(pdev, "apb");
> + if (IS_ERR(qlpci->apb_base))
> + return PTR_ERR(qlpci->apb_base);
> +
> + ret = dw_pcie_host_init(&qlpci->dw.pp);
> + if (ret) {
> + dev_err_probe(dev, ret, "Failed to initialize PCIe host\n");
> + return ret;
> + }
> +
> + qilai_pcie_iocp_cache_setup(&qlpci->dw.pp);
> +
> + return 0;
> +}
> +
> +static int qilai_pcie_probe(struct platform_device *pdev)
> +{
> + struct qilai_pcie *qlpci;
> +
> + qlpci = qilai_pcie_create_data(pdev);
> + if (IS_ERR(qlpci))
> + return PTR_ERR(qlpci);
> +
> + platform_set_drvdata(pdev, qlpci);
> +
> + return qilai_pcie_add_port(qlpci);
> +}
> +
> +static const struct of_device_id qilai_pcie_of_match[] = {
> + { .compatible = "andestech,qilai-pcie" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, qilai_pcie_of_match);
> +
> +static struct platform_driver qilai_pcie_driver = {
> + .probe = qilai_pcie_probe,
> + .driver = {
> + .name = "qilai-pcie",
> + .of_match_table = qilai_pcie_of_match,
> + },
> +};
> +
> +module_platform_driver(qilai_pcie_driver);
> +
> +MODULE_AUTHOR("Randolph Lin <randolph@xxxxxxxxxxxxx>");
> +MODULE_DESCRIPTION("Andes Qilai PCIe driver");
> +MODULE_LICENSE("GPL");
> --
> 2.34.1
>
