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@andestech.com>
---
 drivers/pci/controller/dwc/Kconfig            |  16 ++
 drivers/pci/controller/dwc/Makefile           |   1 +
 drivers/pci/controller/dwc/pcie-andes-qilai.c | 214 ++++++++++++++++++
 3 files changed, 231 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..6dc7af3dcea9 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -49,6 +49,22 @@ 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
+	help
+          Say Y here to enable PCIe controller support on Andes QiLai SoCs,
+	  which operate in Root Complex mode. The Andes QiLai SoCs PCIe
+	  controller is based on DesignWare IP (5.97a version) and therefore
+	  the driver re-uses the DesignWare core functions to implement the
+	  driver. The Andes QiLai SoC has three Root Complexes (RCs): one
+	  operates on PCIe 4.0 with 4 lanes at 0x80000000, while the other
+	  two operate on PCIe 4.0 with 2 lanes at 0xA0000000 and 0xC0000000,
+	  respectively.
+
 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..c7f5dffa3e20
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-andes-qilai.c
@@ -0,0 +1,214 @@
+// 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
+
+/*
+ * 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".
+ */
+#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;
+	struct platform_device *pdev;
+	void __iomem *apb_base;
+};
+
+#define to_qilai_pcie(_pci) container_of(_pci, struct qilai_pcie, pci)
+
+static
+bool qilai_pcie_outbound_atu_addr_valid(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 false;
+
+	return true;
+}
+
+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 */
+	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;
+}
+
+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 = {
+	.outbound_atu_addr_valid = qilai_pcie_outbound_atu_addr_valid,
+	.link_up = qilai_pcie_link_up,
+	.start_link = qilai_pcie_start_link,
+};
+
+/*
+ * Setup 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.
+ */
+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 const struct dw_pcie_host_ops qilai_pcie_host_ops = {
+	.init = qilai_pcie_host_init,
+};
+
+static int qilai_pcie_probe(struct platform_device *pdev)
+{
+	struct qilai_pcie *pcie;
+	struct dw_pcie *pci;
+	struct device *dev;
+	int ret;
+
+	pcie = devm_kzalloc(&pdev->dev, sizeof(*pcie), GFP_KERNEL);
+	if (!pcie)
+		return -ENOMEM;
+
+	pcie->pdev = pdev;
+	platform_set_drvdata(pdev, pcie);
+
+	pci = &pcie->pci;
+	dev = &pcie->pdev->dev;
+	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)) {
+		dev_err_probe(dev, ret, "Failed to Get APB registers.\n");
+		return PTR_ERR(pcie->apb_base);
+	}
+
+	ret = dw_pcie_host_init(&pcie->pci.pp);
+	if (ret) {
+		dev_err_probe(dev, ret, "Failed to initialize PCIe host\n");
+		return ret;
+	}
+
+	qilai_pcie_iocp_cache_setup(&pcie->pci.pp);
+
+	return 0;
+}
+
+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,
+		/* only test passed at PROBE_DEFAULT_STRATEGY */
+		.probe_type = PROBE_DEFAULT_STRATEGY,
+	},
+};
+
+builtin_platform_driver(qilai_pcie_driver);
+
+MODULE_AUTHOR("Randolph Lin <randolph@andestech.com>");
+MODULE_DESCRIPTION("Andes Qilai PCIe driver");
+MODULE_LICENSE("GPL");
-- 
2.34.1

Hi Randolph,

kernel test robot noticed the following build warnings:

[auto build test WARNING on pci/next]
[also build test WARNING on pci/for-linus robh/for-next linus/master v6.17-rc6 next-20250916]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Randolph-Lin/PCI-dwc-Add-outbound-ATU-address-range-validation-callback/20250916-180841
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link:    https://lore.kernel.org/r/20250916100417.3036847-5-randolph%40andestech.com
patch subject: [PATCH v2 4/5] PCI: andes: Add Andes QiLai SoC PCIe host driver support
config: riscv-allmodconfig (https://download.01.org/0day-ci/archive/20250917/202509171747.yJ9wsIkH-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 7c861bcedf61607b6c087380ac711eb7ff918ca6)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250917/202509171747.yJ9wsIkH-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202509171747.yJ9wsIkH-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from <built-in>:3:
   In file included from include/linux/compiler_types.h:171:
   include/linux/compiler-clang.h:28:9: warning: '__SANITIZE_ADDRESS__' macro redefined [-Wmacro-redefined]
      28 | #define __SANITIZE_ADDRESS__
         |         ^
   <built-in>:371:9: note: previous definition is here
     371 | #define __SANITIZE_ADDRESS__ 1
         |         ^
>> drivers/pci/controller/dwc/pcie-andes-qilai.c:179:22: warning: variable 'ret' is uninitialized when used here [-Wuninitialized]
     179 |                 dev_err_probe(dev, ret, "Failed to Get APB registers.\n");
         |                                    ^~~
   drivers/pci/controller/dwc/pcie-andes-qilai.c:159:9: note: initialize the variable 'ret' to silence this warning
     159 |         int ret;
         |                ^
         |                 = 0
   2 warnings generated.


vim +/ret +179 drivers/pci/controller/dwc/pcie-andes-qilai.c

   153	
   154	static int qilai_pcie_probe(struct platform_device *pdev)
   155	{
   156		struct qilai_pcie *pcie;
   157		struct dw_pcie *pci;
   158		struct device *dev;
   159		int ret;
   160	
   161		pcie = devm_kzalloc(&pdev->dev, sizeof(*pcie), GFP_KERNEL);
   162		if (!pcie)
   163			return -ENOMEM;
   164	
   165		pcie->pdev = pdev;
   166		platform_set_drvdata(pdev, pcie);
   167	
   168		pci = &pcie->pci;
   169		dev = &pcie->pdev->dev;
   170		pcie->pci.dev = dev;
   171		pcie->pci.ops = &qilai_pcie_ops;
   172		pcie->pci.pp.ops = &qilai_pcie_host_ops;
   173		pci->use_parent_dt_ranges = true;
   174	
   175		dw_pcie_cap_set(&pcie->pci, REQ_RES);
   176	
   177		pcie->apb_base = devm_platform_ioremap_resource_byname(pdev, "apb");
   178		if (IS_ERR(pcie->apb_base)) {
 > 179			dev_err_probe(dev, ret, "Failed to Get APB registers.\n");
   180			return PTR_ERR(pcie->apb_base);
   181		}
   182	
   183		ret = dw_pcie_host_init(&pcie->pci.pp);
   184		if (ret) {
   185			dev_err_probe(dev, ret, "Failed to initialize PCIe host\n");
   186			return ret;
   187		}
   188	
   189		qilai_pcie_iocp_cache_setup(&pcie->pci.pp);
   190	
   191		return 0;
   192	}
   193	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

On Tue, Sep 16, 2025 at 06:04:16PM +0800, Randolph Lin wrote:
> Add driver support for DesignWare based PCIe controller in Andes
> QiLai SoC. The driver only supports the Root Complex mode.

> +config PCIE_ANDES_QILAI
> +	bool "ANDES QiLai PCIe controller"
> +	depends on OF && (RISCV || COMPILE_TEST)
> +	depends on PCI_MSI
> +	depends on ARCH_ANDES

This prevents a lot of compile testing.  AFAICT, no other controller
depends directly on the arch.  Most do something like these:

  depends on MACH_ARTPEC6 || COMPILE_TEST
  depends on ARCH_MXC || COMPILE_TEST
  depends on OF && (ARM || ARCH_LAYERSCAPE || COMPILE_TEST)

> +	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 SoCs PCIe
> +	  controller is based on DesignWare IP (5.97a version) and therefore
> +	  the driver re-uses the DesignWare core functions to implement the
> +	  driver. The Andes QiLai SoC has three Root Complexes (RCs): one
> +	  operates on PCIe 4.0 with 4 lanes at 0x80000000, while the other
> +	  two operate on PCIe 4.0 with 2 lanes at 0xA0000000 and 0xC0000000,
> +	  respectively.

I assume these addresses come from devicetree, so I don't think
there's any need to include them here.

Fix space/tab indentation issue on first line of help text.  Do the
indentation the same way as the rest of the file.

> + * 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".

Add blank line between paragraphs or rewrap into a single paragraph.

> +static
> +bool qilai_pcie_outbound_atu_addr_valid(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 false;

Seems a little bit strange.  Is this something that could be expressed
via devicetree?  Or something peculiar about QiLai that's different
from all the other DWC-based controllers?

> + * Setup 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.

Add blank lines between paragraphs (or rewrap into a single paragraph
if that's what you intend).

> +static struct platform_driver qilai_pcie_driver = {
> +	.probe = qilai_pcie_probe,
> +	.driver = {
> +		.name	= "qilai-pcie",
> +		.of_match_table = qilai_pcie_of_match,
> +		/* only test passed at PROBE_DEFAULT_STRATEGY */
> +		.probe_type = PROBE_DEFAULT_STRATEGY,

This is the only use of PROBE_DEFAULT_STRATEGY in the entire tree, so
I doubt you need it.  If you do, please explain why in more detail.

Bjorn

Hi Bjorn,

On Tue, Sep 16, 2025 at 09:46:52AM -0500, Bjorn Helgaas wrote:
> [EXTERNAL MAIL]
> 
> On Tue, Sep 16, 2025 at 06:04:16PM +0800, Randolph Lin wrote:
> > Add driver support for DesignWare based PCIe controller in Andes
> > QiLai SoC. The driver only supports the Root Complex mode.
> 
> > +config PCIE_ANDES_QILAI
> > +     bool "ANDES QiLai PCIe controller"
> > +     depends on OF && (RISCV || COMPILE_TEST)
> > +     depends on PCI_MSI
> > +     depends on ARCH_ANDES
> 
> This prevents a lot of compile testing.  AFAICT, no other controller
> depends directly on the arch.  Most do something like these:
> 
>   depends on MACH_ARTPEC6 || COMPILE_TEST
>   depends on ARCH_MXC || COMPILE_TEST
>   depends on OF && (ARM || ARCH_LAYERSCAPE || COMPILE_TEST)
> 

ok.

> > +     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 SoCs PCIe
> > +       controller is based on DesignWare IP (5.97a version) and therefore
> > +       the driver re-uses the DesignWare core functions to implement the
> > +       driver. The Andes QiLai SoC has three Root Complexes (RCs): one
> > +       operates on PCIe 4.0 with 4 lanes at 0x80000000, while the other
> > +       two operate on PCIe 4.0 with 2 lanes at 0xA0000000 and 0xC0000000,
> > +       respectively.
> 
> I assume these addresses come from devicetree, so I don't think
> there's any need to include them here.
> 

I will add num-lanes property in the devicetree.

> Fix space/tab indentation issue on first line of help text.  Do the
> indentation the same way as the rest of the file.
> 

I'm sorry for making this mistake.

> > + * 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".
> 
> Add blank line between paragraphs or rewrap into a single paragraph.
> 

Ok.

> > +static
> > +bool qilai_pcie_outbound_atu_addr_valid(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 false;
> 
> Seems a little bit strange.  Is this something that could be expressed
> via devicetree?  Or something peculiar about QiLai that's different
> from all the other DWC-based controllers?
> 

After reviewing both the code history and the bug tracking system, it turns out
that this code doesn't even qualify as a valid workaround.
Apologies for having submitted it as a patch.

The root cause is that the iATU limits were not configured correctly.
The original design assumed at least 32GB or 128GB of BAR resource assignment,
but the actual chip sets the iATU limit to only 4GB.
As a result, region_limit is always constrained by this 4GB boundary.

The correct workaround should be to program the iATU only for the 32-bit address
space and skip iATU programming for the 64-bit space. A simple way to implement
this workaround is to parse the num-viewport property from the devicetree and
use this value directly, instead of relying on the result of reading
PCIE_ATU_VIEWPORT.

I will attempt to implement it this way, but the correct method is not yet
well-defined. Do you have any suggestions on how to modify the num-viewport
property from the devicetree for use in the driver?
It seems it will be modified in pcie-designware.c.

> > + * Setup 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.
> 
> Add blank lines between paragraphs (or rewrap into a single paragraph
> if that's what you intend).
> 

Ok.

> > +static struct platform_driver qilai_pcie_driver = {
> > +     .probe = qilai_pcie_probe,
> > +     .driver = {
> > +             .name   = "qilai-pcie",
> > +             .of_match_table = qilai_pcie_of_match,
> > +             /* only test passed at PROBE_DEFAULT_STRATEGY */
> > +             .probe_type = PROBE_DEFAULT_STRATEGY,
> 
> This is the only use of PROBE_DEFAULT_STRATEGY in the entire tree, so
> I doubt you need it.  If you do, please explain why in more detail.
> 

In the V1 patch, the reviewer, Manivannan, suggested:
"You should start using PROBE_PREFER_ASYNCHRONOUS."
However, after setting up PROBE_PREFER_ASYNCHRONOUS, numerous errors
were encountered during the EP device probe flow.
Therefore, we would prefer to continue using PROBE_DEFAULT_STRATEGY.

> Bjorn

Sincerely,
Randolph

On Wed, Sep 17, 2025 at 08:16:25PM +0800, Randolph Lin wrote:
> On Tue, Sep 16, 2025 at 09:46:52AM -0500, Bjorn Helgaas wrote:
> > On Tue, Sep 16, 2025 at 06:04:16PM +0800, Randolph Lin wrote:
> > > Add driver support for DesignWare based PCIe controller in Andes
> > > QiLai SoC. The driver only supports the Root Complex mode.

> > > +          Say Y here to enable PCIe controller support on Andes QiLai SoCs,
> > > +       which operate in Root Complex mode. The Andes QiLai SoCs PCIe
> > > +       controller is based on DesignWare IP (5.97a version) and therefore
> > > +       the driver re-uses the DesignWare core functions to implement the
> > > +       driver. The Andes QiLai SoC has three Root Complexes (RCs): one
> > > +       operates on PCIe 4.0 with 4 lanes at 0x80000000, while the other
> > > +       two operate on PCIe 4.0 with 2 lanes at 0xA0000000 and 0xC0000000,
> > > +       respectively.
> > 
> > I assume these addresses come from devicetree, so I don't think
> > there's any need to include them here.
> 
> I will add num-lanes property in the devicetree.

Don't add num-lanes to devicetree unless your driver requires it.
dw_pcie_host_init() uses dw_pcie_link_get_max_link_width() try to get
the lane width from PCI_EXP_LNKCAP.

> > > +static
> > > +bool qilai_pcie_outbound_atu_addr_valid(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 false;
> > 
> > Seems a little bit strange.  Is this something that could be expressed
> > via devicetree?  Or something peculiar about QiLai that's different
> > from all the other DWC-based controllers?
> 
> After reviewing both the code history and the bug tracking system,
> it turns out that this code doesn't even qualify as a valid
> workaround.  Apologies for having submitted it as a patch.
> 
> The root cause is that the iATU limits were not configured
> correctly.  The original design assumed at least 32GB or 128GB of
> BAR resource assignment, but the actual chip sets the iATU limit to
> only 4GB.  As a result, region_limit is always constrained by this
> 4GB boundary.
> 
> The correct workaround should be to program the iATU only for the
> 32-bit address space and skip iATU programming for the 64-bit space.
> A simple way to implement this workaround is to parse the
> num-viewport property from the devicetree and use this value
> directly, instead of relying on the result of reading
> PCIE_ATU_VIEWPORT.
> 
> I will attempt to implement it this way, but the correct method is
> not yet well-defined. Do you have any suggestions on how to modify
> the num-viewport property from the devicetree for use in the driver?
> It seems it will be modified in pcie-designware.c.

Nope, I don't know enough about DWC to give any advice, sorry!

> > > +static struct platform_driver qilai_pcie_driver = {
> > > +     .probe = qilai_pcie_probe,
> > > +     .driver = {
> > > +             .name   = "qilai-pcie",
> > > +             .of_match_table = qilai_pcie_of_match,
> > > +             /* only test passed at PROBE_DEFAULT_STRATEGY */
> > > +             .probe_type = PROBE_DEFAULT_STRATEGY,
> > 
> > This is the only use of PROBE_DEFAULT_STRATEGY in the entire tree, so
> > I doubt you need it.  If you do, please explain why in more detail.
> 
> In the V1 patch, the reviewer, Manivannan, suggested:
> "You should start using PROBE_PREFER_ASYNCHRONOUS."
> However, after setting up PROBE_PREFER_ASYNCHRONOUS, numerous errors
> were encountered during the EP device probe flow.
> Therefore, we would prefer to continue using PROBE_DEFAULT_STRATEGY.

I don't really know the details of probe_type.  But it sounds like the
errors with PROBE_PREFER_ASYNCHRONOUS should probably be debugged and
fixed.

If PROBE_PREFER_ASYNCHRONOUS can't be made to work, it sounds like you
should specify PROBE_FORCE_SYNCHRONOUS, because the comments suggest
that using PROBE_DEFAULT_STRATEGY means the driver should work with
either PROBE_FORCE_SYNCHRONOUS or PROBE_PREFER_ASYNCHRONOUS:

  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/device/driver.h?id=v6.16#n24

Hi Bjorn,

On Wed, Sep 17, 2025 at 04:57:22PM -0500, Bjorn Helgaas wrote:
> [EXTERNAL MAIL]
> 
> On Wed, Sep 17, 2025 at 08:16:25PM +0800, Randolph Lin wrote:
> > On Tue, Sep 16, 2025 at 09:46:52AM -0500, Bjorn Helgaas wrote:
> > > On Tue, Sep 16, 2025 at 06:04:16PM +0800, Randolph Lin wrote:
> > > > Add driver support for DesignWare based PCIe controller in Andes
> > > > QiLai SoC. The driver only supports the Root Complex mode.
> 
> > > > +          Say Y here to enable PCIe controller support on Andes QiLai SoCs,
> > > > +       which operate in Root Complex mode. The Andes QiLai SoCs PCIe
> > > > +       controller is based on DesignWare IP (5.97a version) and therefore
> > > > +       the driver re-uses the DesignWare core functions to implement the
> > > > +       driver. The Andes QiLai SoC has three Root Complexes (RCs): one
> > > > +       operates on PCIe 4.0 with 4 lanes at 0x80000000, while the other
> > > > +       two operate on PCIe 4.0 with 2 lanes at 0xA0000000 and 0xC0000000,
> > > > +       respectively.
> > >
> > > I assume these addresses come from devicetree, so I don't think
> > > there's any need to include them here.
> >
> > I will add num-lanes property in the devicetree.
> 
> Don't add num-lanes to devicetree unless your driver requires it.
> dw_pcie_host_init() uses dw_pcie_link_get_max_link_width() try to get
> the lane width from PCI_EXP_LNKCAP.
>

ok.

> > > > +static
> > > > +bool qilai_pcie_outbound_atu_addr_valid(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 false;
> > >
> > > Seems a little bit strange.  Is this something that could be expressed
> > > via devicetree?  Or something peculiar about QiLai that's different
> > > from all the other DWC-based controllers?
> >
> > After reviewing both the code history and the bug tracking system,
> > it turns out that this code doesn't even qualify as a valid
> > workaround.  Apologies for having submitted it as a patch.
> >
> > The root cause is that the iATU limits were not configured
> > correctly.  The original design assumed at least 32GB or 128GB of
> > BAR resource assignment, but the actual chip sets the iATU limit to
> > only 4GB.  As a result, region_limit is always constrained by this
> > 4GB boundary.
> >
> > The correct workaround should be to program the iATU only for the
> > 32-bit address space and skip iATU programming for the 64-bit space.
> > A simple way to implement this workaround is to parse the
> > num-viewport property from the devicetree and use this value
> > directly, instead of relying on the result of reading
> > PCIE_ATU_VIEWPORT.
> >
> > I will attempt to implement it this way, but the correct method is
> > not yet well-defined. Do you have any suggestions on how to modify
> > the num-viewport property from the devicetree for use in the driver?
> > It seems it will be modified in pcie-designware.c.
> 
> Nope, I don't know enough about DWC to give any advice, sorry!
> 
> > > > +static struct platform_driver qilai_pcie_driver = {
> > > > +     .probe = qilai_pcie_probe,
> > > > +     .driver = {
> > > > +             .name   = "qilai-pcie",
> > > > +             .of_match_table = qilai_pcie_of_match,
> > > > +             /* only test passed at PROBE_DEFAULT_STRATEGY */
> > > > +             .probe_type = PROBE_DEFAULT_STRATEGY,
> > >
> > > This is the only use of PROBE_DEFAULT_STRATEGY in the entire tree, so
> > > I doubt you need it.  If you do, please explain why in more detail.
> >
> > In the V1 patch, the reviewer, Manivannan, suggested:
> > "You should start using PROBE_PREFER_ASYNCHRONOUS."
> > However, after setting up PROBE_PREFER_ASYNCHRONOUS, numerous errors
> > were encountered during the EP device probe flow.
> > Therefore, we would prefer to continue using PROBE_DEFAULT_STRATEGY.
> 
> I don't really know the details of probe_type.  But it sounds like the
> errors with PROBE_PREFER_ASYNCHRONOUS should probably be debugged and
> fixed.
> 
> If PROBE_PREFER_ASYNCHRONOUS can't be made to work, it sounds like you
> should specify PROBE_FORCE_SYNCHRONOUS, because the comments suggest
> that using PROBE_DEFAULT_STRATEGY means the driver should work with
> either PROBE_FORCE_SYNCHRONOUS or PROBE_PREFER_ASYNCHRONOUS:
> 
>   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/device/driver.h?id=v6.16#n24

refer to the following patches:
    91703041697c ("PCI: Allow built-in drivers to use async initial probing")
    8e77d3d59d7b ("Revert "usb: xhci-pci: Set PROBE_PREFER_ASYNCHRONOUS"")

We can continue using PROBE_PREFER_ASYNCHRONOUS.
When used with drivers that support it, such as NVMe, the system behaves as
expected. In the past, we primarily used Renesas xHCI controller.

Sincerely,
Randolph