Re: [PATCH v10 3/4] PCI: qilai: Add Andes QiLai SoC PCIe host driver support

[Manivannan Sadhasivam]

On Fri, Jan 16, 2026 at 07:02:33PM +0800, Randolph wrote:
> From: Randolph Lin <randolph@xxxxxxxxxxxxx>
> 
> 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>

Just a couple of nits. Overall this driver LGTM!

> ---
>  drivers/pci/controller/dwc/Kconfig            |  13 ++
>  drivers/pci/controller/dwc/Makefile           |   1 +
>  drivers/pci/controller/dwc/pcie-andes-qilai.c | 198 ++++++++++++++++++
>  3 files changed, 212 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 519b59422b47..de5ff883e2ba 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -61,6 +61,19 @@ config PCI_MESON
>  	  and therefore the driver re-uses the DesignWare core functions to
>  	  implement the driver.
>  
> +config PCIE_ANDES_QILAI
> +	tristate "Andes QiLai PCIe controller"
> +	depends on ARCH_ANDES || COMPILE_TEST
> +	depends on PCI_MSI
> +	select PCIE_DW_HOST
> +	help
> +	  Say Y here to enable PCIe controller support on Andes QiLai SoCs,
> +	  which operate in Root Complex mode. The Andes QiLai SoC PCIe
> +	  controller is based on DesignWare IP (5.97a version) and therefore

Avoid mentioning IP revision here as this driver may evolve to support multiple
IP versions.

> +	  the driver re-uses the DesignWare core functions to implement the
> +	  driver. The Andes QiLai SoC features three Root Complexes, each
> +	  operating on PCIe 4.0.
> +
>  config PCIE_ARTPEC6
>  	bool
>  
> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> index 67ba59c02038..9e61458dff00 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..daa0281abd24
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-andes-qilai.c
> @@ -0,0 +1,198 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for the PCIe Controller in QiLai from Andes
> + *
> + * Copyright (C) 2025 Andes Technology Corporation

2026

> + */
> +
> +#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
> +
> +/*
> + * Refer to Table A4-5 (Memory type encoding) in the
> + * AMBA AXI and ACE Protocol Specification.
> + *
> + * The selected value corresponds to the Memory type field:
> + * "Write-back, Read and Write-allocate".
> + *
> + * The last three rows in the table A4-5 in
> + * AMBA AXI and ACE Protocol Specification:
> + * ARCACHE        AWCACHE        Memory type
> + * ------------------------------------------------------------------
> + * 1111 (0111)    0111           Write-back Read-allocate
> + * 1011           1111 (1011)    Write-back Write-allocate
> + * 1111           1111           Write-back Read and Write-allocate (selected)
> + */
> +#define IOCP_ARCACHE				0b1111
> +#define IOCP_AWCACHE				0b1111
> +
> +#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 pci;
> +	void __iomem *apb_base;
> +};
> +
> +#define to_qilai_pcie(_pci) container_of(_pci, struct qilai_pcie, pci)
> +
> +static bool qilai_pcie_link_up(struct dw_pcie *pci)
> +{
> +	struct qilai_pcie *pcie = to_qilai_pcie(pci);
> +	u32 val;
> +
> +	/* Read smlh & rdlh link up by checking debug port */

No need of this comment.

> +	val = readl(pcie->apb_base + PCIE_REGS_PCIE_SII_PM_STATE);
> +
> +	return (val & PCIE_REGS_PCIE_SII_LINK_UP) == PCIE_REGS_PCIE_SII_LINK_UP;

Use FIELD_ macros throughout the driver.

> +}
> +
> +static int qilai_pcie_start_link(struct dw_pcie *pci)
> +{
> +	struct qilai_pcie *pcie = to_qilai_pcie(pci);
> +	u32 val;
> +
> +	val = readl(pcie->apb_base + PCIE_GEN_CONTROL2);
> +	val |= PCIE_CFG_LTSSM_EN;
> +	writel(val, pcie->apb_base + PCIE_GEN_CONTROL2);
> +
> +	return 0;
> +}
> +
> +static const struct dw_pcie_ops qilai_pcie_ops = {
> +	.link_up = qilai_pcie_link_up,
> +	.start_link = qilai_pcie_start_link,
> +};
> +
> +/*
> + * Set up the QiLai PCIe IOCP (IO Coherence Port) Read/Write Behaviors to the
> + * Write-Back, Read and Write Allocate mode.
> + *
> + * The IOCP HW target is SoC last-level cache (L2 Cache), which serves as the
> + * system cache. The IOCP HW helps maintain cache monitoring, ensuring that
> + * the device can snoop data from/to the cache.

If the IP allows device to snoop data to/from the cache, the controller should
be marked as 'dma-coherent' in devicetree. And you need to add this property in
the binding too.

> + */
> +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);
> +	FIELD_MODIFY(PCIE_CFG_MSTR_ARCACHE_MODE, &val, IOCP_ARCACHE);
> +	FIELD_MODIFY(PCIE_CFG_MSTR_AWCACHE_MODE, &val, IOCP_AWCACHE);
> +	FIELD_MODIFY(PCIE_CFG_MSTR_ARCACHE_VALUE, &val, IOCP_ARCACHE);
> +	FIELD_MODIFY(PCIE_CFG_MSTR_AWCACHE_VALUE, &val, IOCP_AWCACHE);
> +	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 *pcie)
> +{
> +	u32 val;
> +
> +	val = readl(pcie->apb_base + PCIE_INTR_CONTROL1);
> +	val |= PCIE_MSI_CTRL_INT_EN;
> +	writel(val, pcie->apb_base + PCIE_INTR_CONTROL1);
> +}
> +
> +static int qilai_pcie_host_init(struct dw_pcie_rp *pp)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +	struct qilai_pcie *pcie = to_qilai_pcie(pci);
> +
> +	qilai_pcie_enable_msi(pcie);
> +
> +	return 0;
> +}
> +
> +static void qilai_pcie_host_post_init(struct dw_pcie_rp *pp)
> +{
> +	qilai_pcie_iocp_cache_setup(pp);
> +}
> +
> +static const struct dw_pcie_host_ops qilai_pcie_host_ops = {
> +	.init = qilai_pcie_host_init,
> +	.post_init = qilai_pcie_host_post_init,
> +};
> +
> +static int qilai_pcie_probe(struct platform_device *pdev)
> +{
> +	struct qilai_pcie *pcie;
> +	struct dw_pcie *pci;
> +	struct device *dev = &pdev->dev;
> +	int ret;
> +
> +	pcie = devm_kzalloc(&pdev->dev, sizeof(*pcie), GFP_KERNEL);
> +	if (!pcie)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, pcie);
> +
> +	pci = &pcie->pci;
> +	pcie->pci.dev = dev;
> +	pcie->pci.ops = &qilai_pcie_ops;
> +	pcie->pci.pp.ops = &qilai_pcie_host_ops;
> +	pci->use_parent_dt_ranges = true;
> +
> +	dw_pcie_cap_set(&pcie->pci, REQ_RES);
> +
> +	pcie->apb_base = devm_platform_ioremap_resource_byname(pdev, "apb");
> +	if (IS_ERR(pcie->apb_base))
> +		return PTR_ERR(pcie->apb_base);
> +

Mark controller PM runtime status as enabled:

        pm_runtime_set_active(dev);
        pm_runtime_no_callbacks(dev);
        devm_pm_runtime_enable(dev);

> +	ret = dw_pcie_host_init(&pcie->pci.pp);
> +	if (ret) {
> +		dev_err_probe(dev, ret, "Failed to initialize PCIe host\n");

return dev_err_probe()...

- Mani

-- 
மணிவண்ணன் சதாசிவம்