From: Manivannan Sadhasivam <mani@kernel.org>
To: Alex Elder <elder@riscstar.com>
Cc: robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
bhelgaas@google.com, lpieralisi@kernel.org,
kwilczynski@kernel.org, vkoul@kernel.org, kishon@kernel.org,
dlan@gentoo.org, guodong@riscstar.com, pjw@kernel.org,
palmer@dabbelt.com, aou@eecs.berkeley.edu, alex@ghiti.fr,
p.zabel@pengutronix.de, christian.bruel@foss.st.com,
shradha.t@samsung.com, krishna.chundru@oss.qualcomm.com,
qiang.yu@oss.qualcomm.com, namcao@linutronix.de,
thippeswamy.havalige@amd.com, inochiama@gmail.com,
devicetree@vger.kernel.org, linux-pci@vger.kernel.org,
linux-phy@lists.infradead.org, spacemit@lists.linux.dev,
linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 5/7] PCI: spacemit: introduce SpacemiT PCIe host driver
Date: Sun, 26 Oct 2025 22:25:54 +0530 [thread overview]
Message-ID: <274772thveml3xq2yc7477b7lywzzwelbjtq3hiy4louc6cqom@o5zq66mqa27h> (raw)
In-Reply-To: <20251013153526.2276556-6-elder@riscstar.com>
On Mon, Oct 13, 2025 at 10:35:22AM -0500, Alex Elder wrote:
> Introduce a driver for the PCIe host controller found in the SpacemiT
> K1 SoC. The hardware is derived from the Synopsys DesignWare PCIe IP.
> The driver supports three PCIe ports that operate at PCIe gen2 transfer
> rates (5 GT/sec). The first port uses a combo PHY, which may be
> configured for use for USB 3 instead.
>
> Signed-off-by: Alex Elder <elder@riscstar.com>
> ---
> v2: - Renamed the PCIe driver source file "pcie-spacemit-k1.c"
> - Renamed the PCIe driver Kconfig option PCIE_SPACEMIT_K1; it
> is now tristate rather than Boolean
> - The PCIe host compatible string is now "spacemit,k1-pcie"
> - Renamed the PMU syscon property to be "spacemit,apmu"
> - Renamed the symbols representing the PCI vendor and device IDs
> to align with <linux/pci_ids.h>
> - Use PCIE_T_PVPERL_MS rather than 100 to represent a standard
> delay period.
> - Use platform (not dev) driver-data access functions; assignment
> is done only after the private structure is initialized
> - Deleted some unneeded includes in the PCIe driver.
> - Dropped error checking when operating on MMIO-backed regmaps
> - Added a regmap_read() call in two places, to ensure a specified
> delay occurs *after* the a MMIO write has reached its target.
> - Used ARRAY_SIZE() (not a local variable value) in a few spots
> - Now use readl_relaxed()/writel_relaxed() when operating on
> the "link" I/O memory space in the PCIe driver
> - Updated a few error messages for consistency
> - No longer specify suppress_bind_attrs in the PCIe driver
> - Now specify PCIe driver probe type as PROBE_PREFER_ASYNCHRONOUS
> - No longer use (void) cast to indicate ignored return values
>
> drivers/pci/controller/dwc/Kconfig | 10 +
> drivers/pci/controller/dwc/Makefile | 1 +
> drivers/pci/controller/dwc/pcie-spacemit-k1.c | 319 ++++++++++++++++++
> 3 files changed, 330 insertions(+)
> create mode 100644 drivers/pci/controller/dwc/pcie-spacemit-k1.c
>
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index 349d4657393c9..ede59b34c99ba 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -509,6 +509,16 @@ config PCI_KEYSTONE_EP
> on DesignWare hardware and therefore the driver re-uses the
> DesignWare core functions to implement the driver.
>
> +config PCIE_SPACEMIT_K1
> + tristate "SpacemiT K1 PCIe controller (host mode)"
> + depends on ARCH_SPACEMIT || COMPILE_TEST
> + depends on PCI && OF && HAS_IOMEM
> + select PCIE_DW_HOST
> + default ARCH_SPACEMIT
> + help
> + Enables support for the PCIe controller in the K1 SoC operating
> + in host mode.
> +
> config PCIE_VISCONTI_HOST
> bool "Toshiba Visconti PCIe controller"
> depends on ARCH_VISCONTI || COMPILE_TEST
> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> index 7ae28f3b0fb39..662b0a219ddc4 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -31,6 +31,7 @@ obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o
> obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o
> obj-$(CONFIG_PCIE_VISCONTI_HOST) += pcie-visconti.o
> obj-$(CONFIG_PCIE_RCAR_GEN4) += pcie-rcar-gen4.o
> +obj-$(CONFIG_PCIE_SPACEMIT_K1) += pcie-spacemit-k1.o
> obj-$(CONFIG_PCIE_STM32_HOST) += pcie-stm32.o
> obj-$(CONFIG_PCIE_STM32_EP) += pcie-stm32-ep.o
>
> diff --git a/drivers/pci/controller/dwc/pcie-spacemit-k1.c b/drivers/pci/controller/dwc/pcie-spacemit-k1.c
> new file mode 100644
> index 0000000000000..d58232cbb8a02
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-spacemit-k1.c
> @@ -0,0 +1,319 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * SpacemiT K1 PCIe host driver
> + *
> + * Copyright (C) 2025 by RISCstar Solutions Corporation. All rights reserved.
> + * Copyright (c) 2023, spacemit Corporation.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/gfp.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +#include <linux/types.h>
> +
> +#include "pcie-designware.h"
> +
> +#define PCI_VENDOR_ID_SPACEMIT 0x201f
> +#define PCI_DEVICE_ID_SPACEMIT_K1 0x0001
> +
> +/* Offsets and field definitions for link management registers */
> +
nit: drop the extra newline
> +#define K1_PHY_AHB_IRQ_EN 0x0000
> +#define PCIE_INTERRUPT_EN BIT(0)
> +
> +#define K1_PHY_AHB_LINK_STS 0x0004
> +#define SMLH_LINK_UP BIT(1)
> +#define RDLH_LINK_UP BIT(12)
> +
> +#define INTR_ENABLE 0x0014
> +#define MSI_CTRL_INT BIT(11)
> +
> +/* Some controls require APMU regmap access */
> +#define SYSCON_APMU "spacemit,apmu"
> +
> +/* Offsets and field definitions for APMU registers */
> +
here also
> +#define PCIE_CLK_RESET_CONTROL 0x0000
> +#define LTSSM_EN BIT(6)
> +#define PCIE_AUX_PWR_DET BIT(9)
> +#define PCIE_RC_PERST BIT(12) /* 1: assert PERST# */
> +#define APP_HOLD_PHY_RST BIT(30)
> +#define DEVICE_TYPE_RC BIT(31) /* 0: endpoint; 1: RC */
> +
> +#define PCIE_CONTROL_LOGIC 0x0004
> +#define PCIE_SOFT_RESET BIT(0)
> +
> +struct k1_pcie {
> + struct dw_pcie pci;
> + struct phy *phy;
> + void __iomem *link;
> + struct regmap *pmu; /* Errors ignored; MMIO-backed regmap */
> + u32 pmu_off;
> +};
> +
> +#define to_k1_pcie(dw_pcie) \
> + platform_get_drvdata(to_platform_device((dw_pcie)->dev))
> +
> +static void k1_pcie_toggle_soft_reset(struct k1_pcie *k1)
> +{
> + u32 offset;
> + u32 val;
> +
> + /*
> + * Write, then read back to guarantee it has reached the device
> + * before we start the delay.
> + */
> + offset = k1->pmu_off + PCIE_CONTROL_LOGIC;
> + regmap_set_bits(k1->pmu, offset, PCIE_SOFT_RESET);
> + regmap_read(k1->pmu, offset, &val);
> +
> + mdelay(2);
> +
> + regmap_clear_bits(k1->pmu, offset, PCIE_SOFT_RESET);
> +}
> +
> +/* Enable app clocks, deassert resets */
> +static int k1_pcie_activate(struct k1_pcie *k1)
k1_pcie_enable_resources()?
> +{
> + struct dw_pcie *pci = &k1->pci;
> + int ret;
> +
> + ret = clk_bulk_prepare_enable(ARRAY_SIZE(pci->app_clks), pci->app_clks);
> + if (ret)
> + return ret;
> +
> + ret = reset_control_bulk_deassert(ARRAY_SIZE(pci->app_rsts),
> + pci->app_rsts);
> + if (ret)
> + goto err_disable_clks;
> +
> + ret = reset_control_bulk_deassert(ARRAY_SIZE(pci->core_rsts),
> + pci->core_rsts);
> + if (ret)
> + goto err_assert_resets;
> +
> + return 0;
> +
> +err_assert_resets:
> + reset_control_bulk_assert(ARRAY_SIZE(pci->app_rsts), pci->app_rsts);
> +err_disable_clks:
> + clk_bulk_disable_unprepare(ARRAY_SIZE(pci->app_clks), pci->app_clks);
> +
> + return ret;
> +}
> +
> +/* Assert resets, disable app clocks */
> +static void k1_pcie_deactivate(struct k1_pcie *k1)
k1_pcie_disable_resources()?
> +{
> + struct dw_pcie *pci = &k1->pci;
> +
> + reset_control_bulk_assert(ARRAY_SIZE(pci->core_rsts), pci->core_rsts);
> + reset_control_bulk_assert(ARRAY_SIZE(pci->app_rsts), pci->app_rsts);
> + clk_bulk_disable_unprepare(ARRAY_SIZE(pci->app_clks), pci->app_clks);
> +}
> +
> +static int k1_pcie_init(struct dw_pcie_rp *pp)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> + struct k1_pcie *k1 = to_k1_pcie(pci);
> + u32 offset;
> + u32 mask;
> + u32 val;
> + int ret;
> +
> + k1_pcie_toggle_soft_reset(k1);
> +
> + ret = k1_pcie_activate(k1);
> + if (ret)
> + return ret;
> +
> + ret = phy_init(k1->phy);
> + if (ret) {
> + k1_pcie_deactivate(k1);
> +
> + return ret;
> + }
> +
> + /* Set the PCI vendor and device ID */
> + dw_pcie_dbi_ro_wr_en(pci);
> + dw_pcie_writew_dbi(pci, PCI_VENDOR_ID, PCI_VENDOR_ID_SPACEMIT);
> + dw_pcie_writew_dbi(pci, PCI_DEVICE_ID, PCI_DEVICE_ID_SPACEMIT_K1);
> + dw_pcie_dbi_ro_wr_dis(pci);
> +
> + /*
> + * Assert fundamental reset (drive PERST# low). Put the port in
s/port/controller
> + * root complex mode, and indicate that Vaux (3.3v) is present.
> + */
> + mask = PCIE_RC_PERST;
> + mask |= DEVICE_TYPE_RC | PCIE_AUX_PWR_DET;
> +
> + /*
> + * Write, then read back to guarantee it has reached the device
> + * before we start the delay.
> + */
> + offset = k1->pmu_off + PCIE_CLK_RESET_CONTROL;
> + regmap_set_bits(k1->pmu, offset, mask);
> + regmap_read(k1->pmu, offset, &val);
> +
> + mdelay(PCIE_T_PVPERL_MS);
> +
> + /* Deassert fundamental reset (drive PERST# high) */
> + regmap_clear_bits(k1->pmu, offset, PCIE_RC_PERST);
> +
> + return 0;
> +}
> +
> +static void k1_pcie_deinit(struct dw_pcie_rp *pp)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> + struct k1_pcie *k1 = to_k1_pcie(pci);
> +
> + /* Assert fundamental reset (drive PERST# low) */
> + regmap_set_bits(k1->pmu, k1->pmu_off + PCIE_CLK_RESET_CONTROL,
> + PCIE_RC_PERST);
You need assert PERST# here.
> +
> + phy_exit(k1->phy);
> +
> + k1_pcie_deactivate(k1);
> +}
> +
> +static const struct dw_pcie_host_ops k1_pcie_host_ops = {
> + .init = k1_pcie_init,
> + .deinit = k1_pcie_deinit,
> +};
> +
> +static bool k1_pcie_link_up(struct dw_pcie *pci)
> +{
> + struct k1_pcie *k1 = to_k1_pcie(pci);
> + u32 val;
> +
> + val = readl_relaxed(k1->link + K1_PHY_AHB_LINK_STS);
> +
> + return (val & RDLH_LINK_UP) && (val & SMLH_LINK_UP);
> +}
> +
> +static int k1_pcie_start_link(struct dw_pcie *pci)
> +{
> + struct k1_pcie *k1 = to_k1_pcie(pci);
> + u32 val;
> +
> + /* Stop holding the PHY in reset, and enable link training */
> + regmap_update_bits(k1->pmu, k1->pmu_off + PCIE_CLK_RESET_CONTROL,
> + APP_HOLD_PHY_RST | LTSSM_EN, LTSSM_EN);
> +
> + /* Enable the MSI interrupt */
> + writel_relaxed(MSI_CTRL_INT, k1->link + INTR_ENABLE);
> +
> + /* Top-level interrupt enable */
> + val = readl_relaxed(k1->link + K1_PHY_AHB_IRQ_EN);
> + val |= PCIE_INTERRUPT_EN;
> + writel_relaxed(val, k1->link + K1_PHY_AHB_IRQ_EN);
> +
> + return 0;
> +}
> +
> +static void k1_pcie_stop_link(struct dw_pcie *pci)
> +{
> + struct k1_pcie *k1 = to_k1_pcie(pci);
> + u32 val;
> +
> + /* Disable interrupts */
> + val = readl_relaxed(k1->link + K1_PHY_AHB_IRQ_EN);
> + val &= ~PCIE_INTERRUPT_EN;
> + writel_relaxed(val, k1->link + K1_PHY_AHB_IRQ_EN);
> +
> + writel_relaxed(0, k1->link + INTR_ENABLE);
> +
> + /* Disable the link and hold the PHY in reset */
> + regmap_update_bits(k1->pmu, k1->pmu_off + PCIE_CLK_RESET_CONTROL,
> + APP_HOLD_PHY_RST | LTSSM_EN, APP_HOLD_PHY_RST);
> +}
> +
> +static const struct dw_pcie_ops k1_pcie_ops = {
> + .link_up = k1_pcie_link_up,
> + .start_link = k1_pcie_start_link,
> + .stop_link = k1_pcie_stop_link,
> +};
> +
> +static int k1_pcie_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct k1_pcie *k1;
> + int ret;
> +
> + k1 = devm_kzalloc(dev, sizeof(*k1), GFP_KERNEL);
> + if (!k1)
> + return -ENOMEM;
> +
> + k1->pmu = syscon_regmap_lookup_by_phandle_args(dev_of_node(dev),
> + SYSCON_APMU, 1,
> + &k1->pmu_off);
> + if (IS_ERR(k1->pmu))
> + return dev_err_probe(dev, PTR_ERR(k1->pmu),
> + "failed to lookup PMU registers\n");
> +
> + k1->link = devm_platform_ioremap_resource_byname(pdev, "link");
> + if (!k1->link)
> + return dev_err_probe(dev, -ENOMEM,
> + "failed to map \"link\" registers\n");
> +
> + ret = devm_regulator_get_enable(dev, "vpcie3v3-supply");
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "failed to get \"vpcie3v3\" supply\n");
As mentioned in the bindings patch, you should rely on the PWRCTRL_SLOT driver
to handle the power supplies. It is not yet handling the PERST#, but I have a
series floating for that:
https://lore.kernel.org/linux-pci/20250912-pci-pwrctrl-perst-v3-0-3c0ac62b032c@oss.qualcomm.com/
> +
> + /* Hold the PHY in reset until we start the link */
> + regmap_set_bits(k1->pmu, k1->pmu_off + PCIE_CLK_RESET_CONTROL,
> + APP_HOLD_PHY_RST);
> +
> + k1->phy = devm_phy_get(dev, NULL);
> + if (IS_ERR(k1->phy))
> + return dev_err_probe(dev, PTR_ERR(k1->phy),
> + "failed to get PHY\n");
Once you move these properties to Root Port binding, you need to have per-Root
Port parser. Again, you can refer the STM32 driver.
> +
> + k1->pci.dev = dev;
> + k1->pci.ops = &k1_pcie_ops;
> + dw_pcie_cap_set(&k1->pci, REQ_RES);
> +
> + k1->pci.pp.ops = &k1_pcie_host_ops;
> + k1->pci.pp.num_vectors = MAX_MSI_IRQS;
This driver is just using a single 'msi' vector, which can only support 32 MSIs.
But MAX_MSI_IRQS is 256. So this looks wrong.
> +
> + platform_set_drvdata(pdev, k1);
> +
For setting the correct runtime PM state of the controller, you should do:
pm_runtime_set_active()
pm_runtime_no_callbacks()
devm_pm_runtime_enable()
This will fix the runtime PM hierarchy of PCIe chain (from host controller to
client drivers). Otherwise, it will be broken.
- Mani
--
மணிவண்ணன் சதாசிவம்
next prev parent reply other threads:[~2025-10-26 16:56 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-13 15:35 [PATCH v2 0/7] Introduce SpacemiT K1 PCIe phy and host controller Alex Elder
2025-10-13 15:35 ` [PATCH v2 1/7] dt-bindings: phy: spacemit: add SpacemiT PCIe/combo PHY Alex Elder
2025-10-15 14:52 ` Rob Herring
2025-10-17 16:20 ` Alex Elder
2025-10-13 15:35 ` [PATCH v2 2/7] dt-bindings: phy: spacemit: introduce PCIe PHY Alex Elder
2025-10-15 16:41 ` Rob Herring (Arm)
2025-10-17 16:20 ` Alex Elder
2025-10-13 15:35 ` [PATCH v2 3/7] dt-bindings: pci: spacemit: introduce PCIe host controller Alex Elder
2025-10-14 1:55 ` Yao Zi
2025-10-14 1:57 ` Alex Elder
2025-10-15 16:47 ` Rob Herring
2025-10-17 16:20 ` Alex Elder
2025-10-26 16:38 ` Manivannan Sadhasivam
2025-10-27 22:24 ` Alex Elder
2025-10-28 5:58 ` Manivannan Sadhasivam
2025-10-30 0:10 ` Alex Elder
2025-10-13 15:35 ` [PATCH v2 4/7] phy: spacemit: introduce PCIe/combo PHY Alex Elder
2025-10-15 21:51 ` Aurelien Jarno
2025-10-17 16:21 ` Alex Elder
2025-10-13 15:35 ` [PATCH v2 5/7] PCI: spacemit: introduce SpacemiT PCIe host driver Alex Elder
2025-10-26 16:55 ` Manivannan Sadhasivam [this message]
2025-10-27 22:24 ` Alex Elder
2025-10-28 7:06 ` Manivannan Sadhasivam
2025-10-30 0:10 ` Alex Elder
2025-10-31 6:05 ` Manivannan Sadhasivam
2025-10-31 13:38 ` Alex Elder
2025-10-13 15:35 ` [PATCH v2 6/7] riscv: dts: spacemit: add a PCIe regulator Alex Elder
2025-10-13 15:35 ` [PATCH v2 7/7] riscv: dts: spacemit: PCIe and PHY-related updates Alex Elder
2025-10-16 16:47 ` [PATCH v2 0/7] Introduce SpacemiT K1 PCIe phy and host controller Aurelien Jarno
2025-10-17 16:21 ` Alex Elder
2025-10-28 17:59 ` Aurelien Jarno
2025-10-28 18:42 ` Johannes Erdfelt
2025-10-28 19:10 ` Alex Elder
2025-10-28 20:48 ` Johannes Erdfelt
2025-10-28 20:49 ` Alex Elder
2025-10-30 16:41 ` Manivannan Sadhasivam
2025-10-30 17:49 ` Aurelien Jarno
2025-10-31 6:10 ` Manivannan Sadhasivam
2025-11-03 16:42 ` Alex Elder
2025-10-28 21:08 ` Aurelien Jarno
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=274772thveml3xq2yc7477b7lywzzwelbjtq3hiy4louc6cqom@o5zq66mqa27h \
--to=mani@kernel.org \
--cc=alex@ghiti.fr \
--cc=aou@eecs.berkeley.edu \
--cc=bhelgaas@google.com \
--cc=christian.bruel@foss.st.com \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=dlan@gentoo.org \
--cc=elder@riscstar.com \
--cc=guodong@riscstar.com \
--cc=inochiama@gmail.com \
--cc=kishon@kernel.org \
--cc=krishna.chundru@oss.qualcomm.com \
--cc=krzk+dt@kernel.org \
--cc=kwilczynski@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=linux-phy@lists.infradead.org \
--cc=linux-riscv@lists.infradead.org \
--cc=lpieralisi@kernel.org \
--cc=namcao@linutronix.de \
--cc=p.zabel@pengutronix.de \
--cc=palmer@dabbelt.com \
--cc=pjw@kernel.org \
--cc=qiang.yu@oss.qualcomm.com \
--cc=robh@kernel.org \
--cc=shradha.t@samsung.com \
--cc=spacemit@lists.linux.dev \
--cc=thippeswamy.havalige@amd.com \
--cc=vkoul@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).