[<prev] [next>] [day] [month] [year] [list]
Message-ID: <20250923155443.GA2041202@bhelgaas>
Date: Tue, 23 Sep 2025 10:54:43 -0500
From: Bjorn Helgaas <helgaas@...nel.org>
To: Randolph Lin <randolph@...estech.com>
Cc: linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
linux-riscv@...ts.infradead.org, devicetree@...r.kernel.org,
jingoohan1@...il.com, mani@...nel.org, lpieralisi@...nel.org,
kwilczynski@...nel.org, robh@...nel.org, bhelgaas@...gle.com,
krzk+dt@...nel.org, conor+dt@...nel.org, alex@...ti.fr,
aou@...s.berkeley.edu, palmer@...belt.com, paul.walmsley@...ive.com,
ben717@...estech.com, inochiama@...il.com,
thippeswamy.havalige@....com, namcao@...utronix.de,
shradha.t@...sung.com, randolph.sklin@...il.com,
tim609@...estech.com
Subject: Re: [PATCH v3 4/5] PCI: andes: Add Andes QiLai SoC PCIe host driver
support
On Tue, Sep 23, 2025 at 07:36:46PM +0800, Randolph Lin wrote:
> Add driver support for DesignWare based PCIe controller in Andes
> QiLai SoC. The driver only supports the Root Complex mode.
> +++ 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 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 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 features three Root Complexes, each
> + operating on PCIe 4.0.
Sort these by vendor name:
AMD MDB Versal2 PCIe controller
Amlogic Meson PCIe controller
ANDES QiLai PCIe controller
Axis ARTPEC-6 PCIe controller (host mode)
> config PCI_MESON
> tristate "Amlogic Meson PCIe controller"
> + * 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
Deserves a note about why these values are identical.
> +struct qilai_pcie {
> + struct dw_pcie pci;
> + struct platform_device *pdev;
"pdev" appears to be set but never used; drop it if you don't need it.
> +/*
> + * Setup the Qilai PCIe IOCP (IO Coherence Port) Read/Write Behaviors to the
> + * Write-Back, Read and Write Allocate mode.
Add blank line or rewrap into single paragraph.
> + * 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 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))
> + 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);
I don't think we should be doing anything after dw_pcie_host_init()
because by the time we get here, we've already enumerated downstream
devices and potentially bound drivers to them.
If you need things done in dw_pcie_host_init() before enumeration,
qilai_pcie_host_init() and similar hooks are possibilities.
> + return 0;
> +}
Powered by blists - more mailing lists