* [PATCH v5 0/7] Introduce SpacemiT K1 PCIe phy and host controller
@ 2025-11-07 19:15 Alex Elder
2025-11-07 19:15 ` [PATCH v5 3/7] dt-bindings: pci: spacemit: Introduce PCIe " Alex Elder
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Alex Elder @ 2025-11-07 19:15 UTC (permalink / raw)
To: dlan, robh, krzk+dt, conor+dt, vkoul, kishon, bhelgaas,
lpieralisi, kwilczynski, mani
Cc: ziyao, aurelien, johannes, mayank.rana, qiang.yu, shradha.t,
inochiama, pjw, palmer, aou, alex, p.zabel, christian.bruel,
thippeswamy.havalige, krishna.chundru, guodong, devicetree,
linux-pci, linux-phy, spacemit, linux-riscv, linux-kernel
This series introduces a PHY driver and a PCIe driver to support PCIe
on the SpacemiT K1 SoC. The PCIe implementation is derived from a
Synopsys DesignWare PCIe IP. The PHY driver supports one combination
PCIe/USB PHY as well as two PCIe-only PHYs. The combo PHY port uses
one PCIe lane, and the other two ports each have two lanes. All PCIe
ports operate at 5 GT/second.
The PCIe PHYs must be configured using a value that can only be
determined using the combo PHY, operating in PCIe mode. To allow
that PHY to be used for USB, the needed calibration step is performed
by the PHY driver automatically at probe time. Once this step is done,
the PHY can be used for either PCIe or USB.
This initial version of the driver supports 32 MSIs, and does not
support PCI INTx interrupts. The hardware does not support MSI-X.
Version 5 of this series incorporates suggestions made during the
review of version 4. Specific highlights are detailed below.
Note:
Aurelien Jarno and Johannes Erdfelt have reported seeing ASPM errors
accessing NVMe drives when using earlier versions of this series.
The Kconfig files they used were very different from the RISC-V
default configuration.
Aurelien has since reported the errors do not occur when using
defconfig. Johannes has not reported back about this.
I do not claim these issues are resolved, however this version
of the series does address all other feedback received to date.
-Alex
This series is available here:
https://github.com/riscstar/linux/tree/outgoing/pcie-v5
Between version 3 and version 4:
- Clarify that INTx interrupts are not currently supported
- Add Rob Herring's Reviewed-by on patch 3
- The name of the PCIe root port will always begin with "pcie"
- Lines in the bindings are now wrapped at 80 columns
- Subject lines are all captialized (after subsystem tags)
- Place the PCIe Kconfig option in the proper location based on
vendor name (not Kconfig symbol); expand its description
- Drop two PCIe controller Kconfig dependencies
- Use dw_pcie_readl_dbi() and dw_pcie_writel_dbi() when turning
off ASPM L1
- The dw_pcie_host_ops->init callback has been rearranged a bit:
- The vendor and device IDs are now set early
- PERST# is asserted separate from putting the controller in RC mode
and indicating power is detected
- phy_init() is now called later, just before deasserting PERST#
- Because of timing issues involved in having the root port enable power,
getting and enabling the regulator is back to being done in the PCIe
controller probe function
- The regulator definition is moved back to the PCIe controller DT node,
out of the root port sub-node (in "k1-bananapi-f3.dts")
Here is version 4 of this series:
https://lore.kernel.org/lkml/20251030220259.1063792-1-elder@riscstar.com/
Between version 3 and version 4:
- In the DT binding for the PCIe host controlloller, add a new
sub-node representing the root port
- Move the phys and supply properties out of the PCIe host controller
and into the root port node
- Define the spacemit,apmu property later in the binding and DTS files
- Define the device_type property first in the binding examples and
DTS files
- Add root port sub-nodes in the examples and the DTS files
- Select the PCI_PWRCTRL_SLOT config option when PCIE_SPACEMIT_K1 is
enabled
- Parse the root port node in the driver, and get the PHY
- Leverage the PCI pwrctrl slot driver to get and enable the regulator
- Don't set num_vectors to 256; just use the default (32)
- Cleaned up some comments, white space, and symbol names based on
feedback from Mani
- Add some runtime PM calls to ensure it works propertly
- Add a new post_init callback, which disables ASPM L1 for the link
Here is version 3 of this series:
https://lore.kernel.org/lkml/20251017190740.306780-1-elder@riscstar.com/
Between version 2 and version 3:
- Reviewed-by from Rob added to the first two patches
- The "num-viewport" property has been removed
- The "phy" reset is listed first in the combo PHY binding
- The PHY now requires a resets property to specify the "phy" reset
- The PCIe driver no longer requires a "phy" reset
- The PHY driver now gets and deasserts the reset for all PHYs
- Error handling and "put" of clocks in the PHY driver has been
corrected (for clk_bulk_get() rather than clk_bulk_get_all())
Here is version 2 of this series:
https://lore.kernel.org/lkml/20251013153526.2276556-1-elder@riscstar.com/
Alex Elder (7):
dt-bindings: phy: spacemit: Add SpacemiT PCIe/combo PHY
dt-bindings: phy: spacemit: Introduce PCIe PHY
dt-bindings: pci: spacemit: Introduce PCIe host controller
phy: spacemit: Introduce PCIe/combo PHY
PCI: spacemit: Add SpacemiT PCIe host driver
riscv: dts: spacemit: Add a PCIe regulator
riscv: dts: spacemit: PCIe and PHY-related updates
.../bindings/pci/spacemit,k1-pcie-host.yaml | 157 ++++
.../bindings/phy/spacemit,k1-combo-phy.yaml | 114 +++
.../bindings/phy/spacemit,k1-pcie-phy.yaml | 71 ++
.../boot/dts/spacemit/k1-bananapi-f3.dts | 44 ++
arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi | 33 +
arch/riscv/boot/dts/spacemit/k1.dtsi | 176 +++++
drivers/pci/controller/dwc/Kconfig | 13 +
drivers/pci/controller/dwc/Makefile | 1 +
drivers/pci/controller/dwc/pcie-spacemit-k1.c | 353 +++++++++
drivers/phy/Kconfig | 11 +
drivers/phy/Makefile | 1 +
drivers/phy/phy-spacemit-k1-pcie.c | 670 ++++++++++++++++++
12 files changed, 1644 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pci/spacemit,k1-pcie-host.yaml
create mode 100644 Documentation/devicetree/bindings/phy/spacemit,k1-combo-phy.yaml
create mode 100644 Documentation/devicetree/bindings/phy/spacemit,k1-pcie-phy.yaml
create mode 100644 drivers/pci/controller/dwc/pcie-spacemit-k1.c
create mode 100644 drivers/phy/phy-spacemit-k1-pcie.c
base-commit: 9c0826a5d9aa4d52206dd89976858457a2a8a7ed
--
2.48.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v5 3/7] dt-bindings: pci: spacemit: Introduce PCIe host controller
2025-11-07 19:15 [PATCH v5 0/7] Introduce SpacemiT K1 PCIe phy and host controller Alex Elder
@ 2025-11-07 19:15 ` Alex Elder
2025-11-07 19:15 ` [PATCH v5 5/7] PCI: spacemit: Add SpacemiT PCIe host driver Alex Elder
2025-11-08 11:08 ` [PATCH v5 0/7] Introduce SpacemiT K1 PCIe phy and host controller Aurelien Jarno
2 siblings, 0 replies; 7+ messages in thread
From: Alex Elder @ 2025-11-07 19:15 UTC (permalink / raw)
To: robh, krzk+dt, conor+dt, bhelgaas, lpieralisi, kwilczynski, mani
Cc: dlan, guodong, devicetree, linux-pci, spacemit, linux-riscv,
linux-kernel
Add the Device Tree binding for the PCIe root complex found on the
SpacemiT K1 SoC. This device is derived from the Synopsys Designware
PCIe IP. It supports up to three PCIe ports operating at PCIe gen 2
link speeds (5 GT/sec). One of the ports uses a combo PHY, which is
typically used to support a USB 3 port.
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Alex Elder <elder@riscstar.com>
---
v5: - Add Rob Herring's Reviewed-by tag
- Wrap lines at 80 columns
- Root port nodes will begin with pcie ('e' is not optional)
.../bindings/pci/spacemit,k1-pcie-host.yaml | 157 ++++++++++++++++++
1 file changed, 157 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pci/spacemit,k1-pcie-host.yaml
diff --git a/Documentation/devicetree/bindings/pci/spacemit,k1-pcie-host.yaml b/Documentation/devicetree/bindings/pci/spacemit,k1-pcie-host.yaml
new file mode 100644
index 0000000000000..c4c00b5fcdc0c
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/spacemit,k1-pcie-host.yaml
@@ -0,0 +1,157 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/spacemit,k1-pcie-host.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: SpacemiT K1 PCI Express Host Controller
+
+maintainers:
+ - Alex Elder <elder@riscstar.com>
+
+description: >
+ The SpacemiT K1 SoC PCIe host controller is based on the Synopsys DesignWare
+ PCIe IP. The controller uses the DesignWare built-in MSI interrupt
+ controller, and supports 256 MSIs.
+
+allOf:
+ - $ref: /schemas/pci/snps,dw-pcie.yaml#
+
+properties:
+ compatible:
+ const: spacemit,k1-pcie
+
+ reg:
+ items:
+ - description: DesignWare PCIe registers
+ - description: ATU address space
+ - description: PCIe configuration space
+ - description: Link control registers
+
+ reg-names:
+ items:
+ - const: dbi
+ - const: atu
+ - const: config
+ - const: link
+
+ clocks:
+ items:
+ - description: DWC PCIe Data Bus Interface (DBI) clock
+ - description: DWC PCIe application AXI-bus master interface clock
+ - description: DWC PCIe application AXI-bus slave interface clock
+
+ clock-names:
+ items:
+ - const: dbi
+ - const: mstr
+ - const: slv
+
+ resets:
+ items:
+ - description: DWC PCIe Data Bus Interface (DBI) reset
+ - description: DWC PCIe application AXI-bus master interface reset
+ - description: DWC PCIe application AXI-bus slave interface reset
+
+ reset-names:
+ items:
+ - const: dbi
+ - const: mstr
+ - const: slv
+
+ interrupts:
+ items:
+ - description: Interrupt used for MSIs
+
+ interrupt-names:
+ const: msi
+
+ spacemit,apmu:
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+ description:
+ A phandle that refers to the APMU system controller, whose regmap is
+ used in managing resets and link state, along with and offset of its
+ reset control register.
+ items:
+ - items:
+ - description: phandle to APMU system controller
+ - description: register offset
+
+patternProperties:
+ '^pcie@':
+ type: object
+ $ref: /schemas/pci/pci-pci-bridge.yaml#
+
+ properties:
+ phys:
+ maxItems: 1
+
+ vpcie3v3-supply:
+ description:
+ A phandle for 3.3v regulator to use for PCIe
+
+ required:
+ - phys
+ - vpcie3v3-supply
+
+ unevaluatedProperties: false
+
+required:
+ - clocks
+ - clock-names
+ - resets
+ - reset-names
+ - interrupts
+ - interrupt-names
+ - spacemit,apmu
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/spacemit,k1-syscon.h>
+ pcie@ca400000 {
+ device_type = "pci";
+ compatible = "spacemit,k1-pcie";
+ reg = <0xca400000 0x00001000>,
+ <0xca700000 0x0001ff24>,
+ <0x9f000000 0x00002000>,
+ <0xc0c20000 0x00001000>;
+ reg-names = "dbi",
+ "atu",
+ "config",
+ "link";
+ #address-cells = <3>;
+ #size-cells = <2>;
+ ranges = <0x01000000 0x0 0x00000000 0x9f002000 0x0 0x00100000>,
+ <0x02000000 0x0 0x90000000 0x90000000 0x0 0x0f000000>;
+ interrupts = <142>;
+ interrupt-names = "msi";
+ clocks = <&syscon_apmu CLK_PCIE1_DBI>,
+ <&syscon_apmu CLK_PCIE1_MASTER>,
+ <&syscon_apmu CLK_PCIE1_SLAVE>;
+ clock-names = "dbi",
+ "mstr",
+ "slv";
+ resets = <&syscon_apmu RESET_PCIE1_DBI>,
+ <&syscon_apmu RESET_PCIE1_MASTER>,
+ <&syscon_apmu RESET_PCIE1_SLAVE>;
+ reset-names = "dbi",
+ "mstr",
+ "slv";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pcie1_3_cfg>;
+ spacemit,apmu = <&syscon_apmu 0x3d4>;
+
+ pcie@0 {
+ device_type = "pci";
+ compatible = "pciclass,0604";
+ reg = <0x0 0x0 0x0 0x0 0x0>;
+ bus-range = <0x01 0xff>;
+ #address-cells = <3>;
+ #size-cells = <2>;
+ ranges;
+ phys = <&pcie1_phy>;
+ vpcie3v3-supply = <&pcie_vcc_3v3>;
+ };
+ };
--
2.48.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v5 5/7] PCI: spacemit: Add SpacemiT PCIe host driver
2025-11-07 19:15 [PATCH v5 0/7] Introduce SpacemiT K1 PCIe phy and host controller Alex Elder
2025-11-07 19:15 ` [PATCH v5 3/7] dt-bindings: pci: spacemit: Introduce PCIe " Alex Elder
@ 2025-11-07 19:15 ` Alex Elder
2025-11-08 13:00 ` Christophe JAILLET
2025-11-08 11:08 ` [PATCH v5 0/7] Introduce SpacemiT K1 PCIe phy and host controller Aurelien Jarno
2 siblings, 1 reply; 7+ messages in thread
From: Alex Elder @ 2025-11-07 19:15 UTC (permalink / raw)
To: lpieralisi, kwilczynski, mani, robh, bhelgaas
Cc: dlan, aurelien, johannes, p.zabel, christian.bruel,
thippeswamy.havalige, krishna.chundru, mayank.rana, qiang.yu,
shradha.t, inochiama, guodong, linux-pci, spacemit, linux-riscv,
linux-kernel
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>
---
v5: - Kconfig option now positioned based on vendor name sort
- Kconfig option description has been expanded a bit
- Kconfig option does not depend on PCI or OF
- dw_pcie_readl_dbi() and dw_pcie_writel_dbi() are now used when
turning off ASPM L1
- In k1_pcie_phy_init():
- Vendor and device IDs are set earlier
- PERST# is now asserted separately
- phy_init() is now called later
- Getting and enabling the regulator is done in the controller
probe function, rather than relying on the root port driver
doing that
drivers/pci/controller/dwc/Kconfig | 13 +
drivers/pci/controller/dwc/Makefile | 1 +
drivers/pci/controller/dwc/pcie-spacemit-k1.c | 353 ++++++++++++++++++
3 files changed, 367 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..718bb54e943f6 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -416,6 +416,19 @@ config PCIE_SOPHGO_DW
Say Y here if you want PCIe host controller support on
Sophgo SoCs.
+config PCIE_SPACEMIT_K1
+ tristate "SpacemiT K1 PCIe controller (host mode)"
+ depends on ARCH_SPACEMIT || COMPILE_TEST
+ depends on HAS_IOMEM
+ select PCIE_DW_HOST
+ select PCI_PWRCTRL_SLOT
+ default ARCH_SPACEMIT
+ help
+ Enables support for the DesignWare based PCIe controller in
+ the SpacemiT K1 SoC operating in host mode. Three controllers
+ are available on the K1 SoC; the first of these shares a PHY
+ with a USB 3.0 host controller (one or the other can be used).
+
config PCIE_SPEAR13XX
bool "STMicroelectronics SPEAr PCIe controller"
depends on ARCH_SPEAR13XX || 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..fd428a39b83cd
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-spacemit-k1.c
@@ -0,0 +1,353 @@
+// 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 */
+#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 */
+#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_enable_resources(struct k1_pcie *k1)
+{
+ 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;
+
+ return 0;
+
+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_disable_resources(struct k1_pcie *k1)
+{
+ struct dw_pcie *pci = &k1->pci;
+
+ 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 reset_ctrl;
+ u32 val;
+ int ret;
+
+ k1_pcie_toggle_soft_reset(k1);
+
+ ret = k1_pcie_enable_resources(k1);
+ if (ret)
+ 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);
+
+ /*
+ * Start by asserting fundamental reset (drive PERST# low). The
+ * PCI CEM spec says that PERST# should be deasserted at least
+ * 100ms after the power becomes stable, so we'll insert that
+ * delay first. Write, then read it back to guarantee the write
+ * reaches the device before we start the delay.
+ */
+ reset_ctrl = k1->pmu_off + PCIE_CLK_RESET_CONTROL;
+ regmap_set_bits(k1->pmu, reset_ctrl, PCIE_RC_PERST);
+ regmap_read(k1->pmu, reset_ctrl, &val);
+ mdelay(PCIE_T_PVPERL_MS);
+
+ /*
+ * Put the controller in root complex mode, and indicate that
+ * Vaux (3.3v) is present.
+ */
+ regmap_set_bits(k1->pmu, reset_ctrl, DEVICE_TYPE_RC | PCIE_AUX_PWR_DET);
+
+ ret = phy_init(k1->phy);
+ if (ret) {
+ k1_pcie_disable_resources(k1);
+
+ return ret;
+ }
+
+ /* Finally deassert fundamental reset (drive PERST# high) */
+ regmap_clear_bits(k1->pmu, reset_ctrl, PCIE_RC_PERST);
+
+ return 0;
+}
+
+/* Disable ASPM L1 for now, until reported errors can be reproduced */
+static void k1_pcie_post_init(struct dw_pcie_rp *pp)
+{
+ struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+ u8 offset;
+ u32 val;
+
+ offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+ offset += PCI_EXP_LNKCAP;
+
+ /* Turn off ASPM L1 for the link */
+ dw_pcie_dbi_ro_wr_en(pci);
+ val = dw_pcie_readl_dbi(pci, offset);
+ val &= ~PCI_EXP_LNKCAP_ASPM_L1;
+ dw_pcie_writel_dbi(pci, offset, val);
+ dw_pcie_dbi_ro_wr_dis(pci);
+}
+
+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);
+
+ phy_exit(k1->phy);
+
+ k1_pcie_disable_resources(k1);
+}
+
+static const struct dw_pcie_host_ops k1_pcie_host_ops = {
+ .init = k1_pcie_init,
+ .post_init = k1_pcie_post_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_parse_port(struct k1_pcie *k1)
+{
+ struct device *dev = k1->pci.dev;
+ struct device_node *root_port;
+ struct phy *phy;
+
+ /* We assume only one root port */
+ root_port = of_get_next_available_child(dev_of_node(dev), NULL);
+ if (!root_port)
+ return -EINVAL;
+
+ phy = devm_of_phy_get(dev, root_port, NULL);
+
+ of_node_put(root_port);
+
+ if (IS_ERR(phy))
+ return PTR_ERR(phy);
+
+ k1->phy = phy;
+
+ return 0;
+}
+
+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");
+
+ 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;
+
+ /* 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);
+
+ ret = devm_regulator_get_enable(dev, "vpcie3v3");
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "failed to get \"vpcie3v3\" supply\n");
+
+ pm_runtime_set_active(dev);
+ pm_runtime_no_callbacks(dev);
+ devm_pm_runtime_enable(dev);
+
+ platform_set_drvdata(pdev, k1);
+
+ ret = k1_pcie_parse_port(k1);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to parse root port\n");
+
+ ret = dw_pcie_host_init(&k1->pci.pp);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to initialize host\n");
+
+ return 0;
+}
+
+static void k1_pcie_remove(struct platform_device *pdev)
+{
+ struct k1_pcie *k1 = platform_get_drvdata(pdev);
+
+ dw_pcie_host_deinit(&k1->pci.pp);
+}
+
+static const struct of_device_id k1_pcie_of_match_table[] = {
+ { .compatible = "spacemit,k1-pcie", },
+ { },
+};
+
+static struct platform_driver k1_pcie_driver = {
+ .probe = k1_pcie_probe,
+ .remove = k1_pcie_remove,
+ .driver = {
+ .name = "spacemit-k1-pcie",
+ .of_match_table = k1_pcie_of_match_table,
+ .probe_type = PROBE_PREFER_ASYNCHRONOUS,
+ },
+};
+module_platform_driver(k1_pcie_driver);
--
2.48.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v5 0/7] Introduce SpacemiT K1 PCIe phy and host controller
2025-11-07 19:15 [PATCH v5 0/7] Introduce SpacemiT K1 PCIe phy and host controller Alex Elder
2025-11-07 19:15 ` [PATCH v5 3/7] dt-bindings: pci: spacemit: Introduce PCIe " Alex Elder
2025-11-07 19:15 ` [PATCH v5 5/7] PCI: spacemit: Add SpacemiT PCIe host driver Alex Elder
@ 2025-11-08 11:08 ` Aurelien Jarno
2025-11-10 15:20 ` Alex Elder
2 siblings, 1 reply; 7+ messages in thread
From: Aurelien Jarno @ 2025-11-08 11:08 UTC (permalink / raw)
To: Alex Elder
Cc: dlan, robh, krzk+dt, conor+dt, vkoul, kishon, bhelgaas,
lpieralisi, kwilczynski, mani, ziyao, johannes, mayank.rana,
qiang.yu, shradha.t, inochiama, pjw, palmer, aou, alex, p.zabel,
christian.bruel, thippeswamy.havalige, krishna.chundru, guodong,
devicetree, linux-pci, linux-phy, spacemit, linux-riscv,
linux-kernel
Hi Alex,
Thanks for this new version.
On 2025-11-07 13:15, Alex Elder wrote:
> This series introduces a PHY driver and a PCIe driver to support PCIe
> on the SpacemiT K1 SoC. The PCIe implementation is derived from a
> Synopsys DesignWare PCIe IP. The PHY driver supports one combination
> PCIe/USB PHY as well as two PCIe-only PHYs. The combo PHY port uses
> one PCIe lane, and the other two ports each have two lanes. All PCIe
> ports operate at 5 GT/second.
>
> The PCIe PHYs must be configured using a value that can only be
> determined using the combo PHY, operating in PCIe mode. To allow
> that PHY to be used for USB, the needed calibration step is performed
> by the PHY driver automatically at probe time. Once this step is done,
> the PHY can be used for either PCIe or USB.
>
> This initial version of the driver supports 32 MSIs, and does not
> support PCI INTx interrupts. The hardware does not support MSI-X.
>
> Version 5 of this series incorporates suggestions made during the
> review of version 4. Specific highlights are detailed below.
>
> Note:
> Aurelien Jarno and Johannes Erdfelt have reported seeing ASPM errors
> accessing NVMe drives when using earlier versions of this series.
> The Kconfig files they used were very different from the RISC-V
> default configuration.
>
> Aurelien has since reported the errors do not occur when using
> defconfig. Johannes has not reported back about this.
Unfortunately, while it is true with v4, this is not the case with v5
anymore :(
Fundamentally in the generic designware driver, post_init (which is used
to disable L1 support on the controller side) is called after starting
the link. The comparison of the capabilities is done in
pcie_aspm_cap_init when the link is up, which happens a tiny bit after
starting it.
In practice with v4, the link is started, ASPM L1 is disabled and the
link becomes up. With v5, the move of the code getting and enabling the
regulator changed the timing, and ASPM L1 is now disabled on the
controller 2-3 ms after the link is up, which is too late.
I have added a call to pci_info to display the moment where ASPM is
disabled. This is without the regulator change:
[ 0.386730] spacemit-k1-pcie ca400000.pcie: host bridge /soc/pcie-bus/pcie@ca400000 ranges:
[ 0.386970] spacemit-k1-pcie ca800000.pcie: host bridge /soc/pcie-bus/pcie@ca800000 ranges:
[ 0.387017] spacemit-k1-pcie ca800000.pcie: IO 0x00b7002000..0x00b7101fff -> 0x0000000000
[ 0.387047] spacemit-k1-pcie ca800000.pcie: MEM 0x00a0000000..0x00afffffff -> 0x00a0000000
[ 0.387062] spacemit-k1-pcie ca800000.pcie: MEM 0x00b0000000..0x00b6ffffff -> 0x00b0000000
[ 0.400109] spacemit-k1-pcie ca400000.pcie: IO 0x009f002000..0x009f101fff -> 0x0000000000
[ 0.490101] spacemit-k1-pcie ca800000.pcie: iATU: unroll T, 8 ob, 8 ib, align 4K, limit 4G
[ 0.494195] spacemit-k1-pcie ca400000.pcie: MEM 0x0090000000..0x009effffff -> 0x0090000000
[ 0.850344] spacemit-k1-pcie ca400000.pcie: iATU: unroll T, 8 ob, 8 ib, align 4K, limit 4G
[ 0.950133] spacemit-k1-pcie ca400000.pcie: PCIe Gen.1 x2 link up
[ 1.129988] spacemit-k1-pcie ca400000.pcie: PCI host bridge to bus 0000:00
[ 1.335482] pci_bus 0000:00: root bus resource [bus 00-ff]
[ 1.340946] pci_bus 0000:00: root bus resource [io 0x100000-0x1fffff] (bus address [0x0000-0xfffff])
[ 1.350181] pci_bus 0000:00: root bus resource [mem 0x90000000-0x9effffff]
[ 1.358734] pci_bus 0000:00: resource 4 [io 0x100000-0x1fffff]
[ 1.362033] pci_bus 0000:00: resource 5 [mem 0x90000000-0x9effffff]
[ 1.368289] spacemit-k1-pcie ca400000.pcie: pcie_aspm_override_default_link_state
[ 1.375967] pci 0000:00:00.0: [1e5d:3003] type 01 class 0x060400 PCIe Root Port
[ 1.383043] pci 0000:00:00.0: BAR 0 [mem 0x00000000-0x000fffff]
[ 1.388927] pci 0000:00:00.0: BAR 1 [mem 0x00000000-0x000fffff]
[ 1.394826] pci 0000:00:00.0: PCI bridge to [bus 01-ff]
[ 1.400061] pci 0000:00:00.0: bridge window [io 0x100000-0x100fff]
[ 1.406460] pci 0000:00:00.0: bridge window [mem 0x00000000-0x000fffff]
[ 1.413245] pci 0000:00:00.0: bridge window [mem 0x00000000-0x000fffff 64bit pref]
[ 1.421012] pci 0000:00:00.0: supports D1
[ 1.424948] pci 0000:00:00.0: PME# supported from D0 D1 D3hot D3cold
[ 1.432718] pci 0000:01:00.0: [1987:5015] type 00 class 0x010802 PCIe Endpoint
[ 1.438698] pci 0000:01:00.0: BAR 0 [mem 0x00000000-0x00003fff 64bit]
[ 1.445426] pci 0000:01:00.0: 4.000 Gb/s available PCIe bandwidth, limited by 2.5 GT/s PCIe x2 link at 0000:00:00.0 (capable of 31.504 Gb/s with 8.0 GT/s PCIe x4 link)
[ 1.464897] pci_bus 0000:01: busn_res: [bus 01-ff] end is updated to 01
And this is with the regulator change:
[ 0.410796] spacemit-k1-pcie ca400000.pcie: host bridge /soc/pcie-bus/pcie@ca400000 ranges:
[ 0.410836] spacemit-k1-pcie ca800000.pcie: host bridge /soc/pcie-bus/pcie@ca800000 ranges:
[ 0.410889] spacemit-k1-pcie ca800000.pcie: IO 0x00b7002000..0x00b7101fff -> 0x0000000000
[ 0.410917] spacemit-k1-pcie ca800000.pcie: MEM 0x00a0000000..0x00afffffff -> 0x00a0000000
[ 0.410932] spacemit-k1-pcie ca800000.pcie: MEM 0x00b0000000..0x00b6ffffff -> 0x00b0000000
[ 0.424651] spacemit-k1-pcie ca400000.pcie: IO 0x009f002000..0x009f101fff -> 0x0000000000
[ 0.436446] spacemit-k1-pcie ca400000.pcie: MEM 0x0090000000..0x009effffff -> 0x0090000000
[ 0.513897] spacemit-k1-pcie ca800000.pcie: iATU: unroll T, 8 ob, 8 ib, align 4K, limit 4G
[ 0.559595] spacemit-k1-pcie ca400000.pcie: iATU: unroll T, 8 ob, 8 ib, align 4K, limit 4G
[ 0.839412] spacemit-k1-pcie ca400000.pcie: PCIe Gen.1 x2 link up
[ 0.847078] spacemit-k1-pcie ca400000.pcie: PCI host bridge to bus 0000:00
[ 0.857600] pci_bus 0000:00: root bus resource [bus 00-ff]
[ 0.868702] pci_bus 0000:00: root bus resource [io 0x100000-0x1fffff] (bus address [0x0000-0xfffff])
[ 1.146409] pci_bus 0000:00: root bus resource [mem 0x90000000-0x9effffff]
[ 1.373742] pci 0000:00:00.0: [1e5d:3003] type 01 class 0x060400 PCIe Root Port
[ 1.380963] pci 0000:00:00.0: BAR 0 [mem 0x00000000-0x000fffff]
[ 1.386883] pci 0000:00:00.0: BAR 1 [mem 0x00000000-0x000fffff]
[ 1.392808] pci 0000:00:00.0: PCI bridge to [bus 01-ff]
[ 1.395394] pci 0000:00:00.0: bridge window [io 0x100000-0x100fff]
[ 1.401811] pci 0000:00:00.0: bridge window [mem 0x00000000-0x000fffff]
[ 1.408583] pci 0000:00:00.0: bridge window [mem 0x00000000-0x000fffff 64bit pref]
[ 1.416354] pci 0000:00:00.0: supports D1
[ 1.420294] pci 0000:00:00.0: PME# supported from D0 D1 D3hot D3cold
[ 1.428220] pci 0000:01:00.0: [1987:5015] type 00 class 0x010802 PCIe Endpoint
[ 1.434034] pci 0000:01:00.0: BAR 0 [mem 0x00000000-0x00003fff 64bit]
[ 1.440772] pci 0000:01:00.0: 4.000 Gb/s available PCIe bandwidth, limited by 2.5 GT/s PCIe x2 link at 0000:00:00.0 (capable of 31.504 Gb/s with 8.0 GT/s PCIe x4 link)
[ 1.463390] pci 0000:01:00.0: pcie_aspm_override_default_link_state
[ 1.467000] pci 0000:01:00.0: ASPM: default states L1
[ 1.472093] pci_bus 0000:01: busn_res: [bus 01-ff] end is updated to 01
Note how the line pcie_aspm_override_default_link_state arrives too
late.
Regards
Aurelien
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
aurelien@aurel32.net http://aurel32.net
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v5 5/7] PCI: spacemit: Add SpacemiT PCIe host driver
2025-11-07 19:15 ` [PATCH v5 5/7] PCI: spacemit: Add SpacemiT PCIe host driver Alex Elder
@ 2025-11-08 13:00 ` Christophe JAILLET
2025-11-10 15:05 ` Alex Elder
0 siblings, 1 reply; 7+ messages in thread
From: Christophe JAILLET @ 2025-11-08 13:00 UTC (permalink / raw)
To: Alex Elder, lpieralisi, kwilczynski, mani, robh, bhelgaas
Cc: dlan, aurelien, johannes, p.zabel, christian.bruel,
thippeswamy.havalige, krishna.chundru, mayank.rana, qiang.yu,
shradha.t, inochiama, guodong, linux-pci, spacemit, linux-riscv,
linux-kernel
Le 07/11/2025 à 20:15, Alex Elder a écrit :
> 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>
> ---
...
> +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)
if (IS_ERR(k1->link)) ?
> + return dev_err_probe(dev, -ENOMEM,
> + "failed to map \"link\" registers\n");
Message with -ENOMEM are ignored, so a direct return -ENOMEM is less
verbose and will bhave the same. See [1].
But in this case, I think it should be PTR_ERR(k1->link).
[1]:
https://elixir.bootlin.com/linux/v6.18-rc2/source/drivers/base/core.c#L5015
> +
> + 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;
> +
> + /* 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);
> +
> + ret = devm_regulator_get_enable(dev, "vpcie3v3");
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "failed to get \"vpcie3v3\" supply\n");
> +
> + pm_runtime_set_active(dev);
> + pm_runtime_no_callbacks(dev);
> + devm_pm_runtime_enable(dev);
> +
> + platform_set_drvdata(pdev, k1);
> +
> + ret = k1_pcie_parse_port(k1);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to parse root port\n");
> +
> + ret = dw_pcie_host_init(&k1->pci.pp);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to initialize host\n");
> +
> + return 0;
> +}
...
> +static const struct of_device_id k1_pcie_of_match_table[] = {
> + { .compatible = "spacemit,k1-pcie", },
> + { },
Unneeded trainling comma after a terminator.
> +};
> +
> +static struct platform_driver k1_pcie_driver = {
> + .probe = k1_pcie_probe,
> + .remove = k1_pcie_remove,
> + .driver = {
> + .name = "spacemit-k1-pcie",
> + .of_match_table = k1_pcie_of_match_table,
> + .probe_type = PROBE_PREFER_ASYNCHRONOUS,
> + },
> +};
> +module_platform_driver(k1_pcie_driver);
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v5 5/7] PCI: spacemit: Add SpacemiT PCIe host driver
2025-11-08 13:00 ` Christophe JAILLET
@ 2025-11-10 15:05 ` Alex Elder
0 siblings, 0 replies; 7+ messages in thread
From: Alex Elder @ 2025-11-10 15:05 UTC (permalink / raw)
To: Christophe JAILLET, lpieralisi, kwilczynski, mani, robh, bhelgaas
Cc: dlan, aurelien, johannes, p.zabel, christian.bruel,
thippeswamy.havalige, krishna.chundru, mayank.rana, qiang.yu,
shradha.t, inochiama, guodong, linux-pci, spacemit, linux-riscv,
linux-kernel
On 11/8/25 7:00 AM, Christophe JAILLET wrote:
> Le 07/11/2025 à 20:15, Alex Elder a écrit :
>> 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>
>> ---
>
> ...
>
>> +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)
>
> if (IS_ERR(k1->link)) ?
Yes, you're right. I'll fix this in the next version.
>
>> + return dev_err_probe(dev, -ENOMEM,
>> + "failed to map \"link\" registers\n");
>
> Message with -ENOMEM are ignored, so a direct return -ENOMEM is less
> verbose and will bhave the same. See [1].
>
> But in this case, I think it should be PTR_ERR(k1->link).
Yes, that's what it will be.
> [1]: https://elixir.bootlin.com/linux/v6.18-rc2/source/drivers/base/
> core.c#L5015
>
>> +
>> + 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;
>> +
>> + /* 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);
>> +
>> + ret = devm_regulator_get_enable(dev, "vpcie3v3");
>> + if (ret)
>> + return dev_err_probe(dev, ret,
>> + "failed to get \"vpcie3v3\" supply\n");
>> +
>> + pm_runtime_set_active(dev);
>> + pm_runtime_no_callbacks(dev);
>> + devm_pm_runtime_enable(dev);
>> +
>> + platform_set_drvdata(pdev, k1);
>> +
>> + ret = k1_pcie_parse_port(k1);
>> + if (ret)
>> + return dev_err_probe(dev, ret, "failed to parse root port\n");
>> +
>> + ret = dw_pcie_host_init(&k1->pci.pp);
>> + if (ret)
>> + return dev_err_probe(dev, ret, "failed to initialize host\n");
>> +
>> + return 0;
>> +}
>
> ...
>
>> +static const struct of_device_id k1_pcie_of_match_table[] = {
>> + { .compatible = "spacemit,k1-pcie", },
>> + { },
>
> Unneeded trainling comma after a terminator.
Unneeded, but not erroneous. In any case, I'll drop it based
on your preference.
Thank you for your review.
-Alex
>
>> +};
>> +
>> +static struct platform_driver k1_pcie_driver = {
>> + .probe = k1_pcie_probe,
>> + .remove = k1_pcie_remove,
>> + .driver = {
>> + .name = "spacemit-k1-pcie",
>> + .of_match_table = k1_pcie_of_match_table,
>> + .probe_type = PROBE_PREFER_ASYNCHRONOUS,
>> + },
>> +};
>> +module_platform_driver(k1_pcie_driver);
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v5 0/7] Introduce SpacemiT K1 PCIe phy and host controller
2025-11-08 11:08 ` [PATCH v5 0/7] Introduce SpacemiT K1 PCIe phy and host controller Aurelien Jarno
@ 2025-11-10 15:20 ` Alex Elder
0 siblings, 0 replies; 7+ messages in thread
From: Alex Elder @ 2025-11-10 15:20 UTC (permalink / raw)
To: dlan, robh, krzk+dt, conor+dt, vkoul, kishon, bhelgaas,
lpieralisi, kwilczynski, mani, ziyao, johannes, mayank.rana,
qiang.yu, shradha.t, inochiama, pjw, palmer, aou, alex, p.zabel,
christian.bruel, thippeswamy.havalige, krishna.chundru, guodong,
devicetree, linux-pci, linux-phy, spacemit, linux-riscv,
linux-kernel
On 11/8/25 5:08 AM, Aurelien Jarno wrote:
> Hi Alex,
>
> Thanks for this new version.
>
> On 2025-11-07 13:15, Alex Elder wrote:
>> This series introduces a PHY driver and a PCIe driver to support PCIe
>> on the SpacemiT K1 SoC. The PCIe implementation is derived from a
>> Synopsys DesignWare PCIe IP. The PHY driver supports one combination
>> PCIe/USB PHY as well as two PCIe-only PHYs. The combo PHY port uses
>> one PCIe lane, and the other two ports each have two lanes. All PCIe
>> ports operate at 5 GT/second.
>>
>> The PCIe PHYs must be configured using a value that can only be
>> determined using the combo PHY, operating in PCIe mode. To allow
>> that PHY to be used for USB, the needed calibration step is performed
>> by the PHY driver automatically at probe time. Once this step is done,
>> the PHY can be used for either PCIe or USB.
>>
>> This initial version of the driver supports 32 MSIs, and does not
>> support PCI INTx interrupts. The hardware does not support MSI-X.
>>
>> Version 5 of this series incorporates suggestions made during the
>> review of version 4. Specific highlights are detailed below.
>>
>> Note:
>> Aurelien Jarno and Johannes Erdfelt have reported seeing ASPM errors
>> accessing NVMe drives when using earlier versions of this series.
>> The Kconfig files they used were very different from the RISC-V
>> default configuration.
>>
>> Aurelien has since reported the errors do not occur when using
>> defconfig. Johannes has not reported back about this.
>
> Unfortunately, while it is true with v4, this is not the case with v5
> anymore :(
That's too bad, but thank you for reporting it.
> Fundamentally in the generic designware driver, post_init (which is used
> to disable L1 support on the controller side) is called after starting
> the link. The comparison of the capabilities is done in
> pcie_aspm_cap_init when the link is up, which happens a tiny bit after
> starting it.
>
> In practice with v4, the link is started, ASPM L1 is disabled and the
> link becomes up. With v5, the move of the code getting and enabling the
> regulator changed the timing, and ASPM L1 is now disabled on the
> controller 2-3 ms after the link is up, which is too late.
Yes in v4, we relied on the root port driver to enable the
regulator, but (on my system anyway) that happened too late,
*after* the PCIe controller driver held PERST# asserted for
100 msec. PERST# is not supposed to be de-asserted until
power is known to be stable. So v5 went back to having
the controller get the regulator in k1_pcie_probe().
I am supposed to receive the WD Blue SN570 on Wednesday, and
when I get that I'll have a chance to try to reproduce at
least one of these problems, and can ensure there are no
timing-related issues like this.
Thank you for your continued testing and feedback about this.
-Alex
> I have added a call to pci_info to display the moment where ASPM is
> disabled. This is without the regulator change:
>
> [ 0.386730] spacemit-k1-pcie ca400000.pcie: host bridge /soc/pcie-bus/pcie@ca400000 ranges:
> [ 0.386970] spacemit-k1-pcie ca800000.pcie: host bridge /soc/pcie-bus/pcie@ca800000 ranges:
> [ 0.387017] spacemit-k1-pcie ca800000.pcie: IO 0x00b7002000..0x00b7101fff -> 0x0000000000
> [ 0.387047] spacemit-k1-pcie ca800000.pcie: MEM 0x00a0000000..0x00afffffff -> 0x00a0000000
> [ 0.387062] spacemit-k1-pcie ca800000.pcie: MEM 0x00b0000000..0x00b6ffffff -> 0x00b0000000
> [ 0.400109] spacemit-k1-pcie ca400000.pcie: IO 0x009f002000..0x009f101fff -> 0x0000000000
> [ 0.490101] spacemit-k1-pcie ca800000.pcie: iATU: unroll T, 8 ob, 8 ib, align 4K, limit 4G
> [ 0.494195] spacemit-k1-pcie ca400000.pcie: MEM 0x0090000000..0x009effffff -> 0x0090000000
> [ 0.850344] spacemit-k1-pcie ca400000.pcie: iATU: unroll T, 8 ob, 8 ib, align 4K, limit 4G
> [ 0.950133] spacemit-k1-pcie ca400000.pcie: PCIe Gen.1 x2 link up
> [ 1.129988] spacemit-k1-pcie ca400000.pcie: PCI host bridge to bus 0000:00
> [ 1.335482] pci_bus 0000:00: root bus resource [bus 00-ff]
> [ 1.340946] pci_bus 0000:00: root bus resource [io 0x100000-0x1fffff] (bus address [0x0000-0xfffff])
> [ 1.350181] pci_bus 0000:00: root bus resource [mem 0x90000000-0x9effffff]
> [ 1.358734] pci_bus 0000:00: resource 4 [io 0x100000-0x1fffff]
> [ 1.362033] pci_bus 0000:00: resource 5 [mem 0x90000000-0x9effffff]
> [ 1.368289] spacemit-k1-pcie ca400000.pcie: pcie_aspm_override_default_link_state
> [ 1.375967] pci 0000:00:00.0: [1e5d:3003] type 01 class 0x060400 PCIe Root Port
> [ 1.383043] pci 0000:00:00.0: BAR 0 [mem 0x00000000-0x000fffff]
> [ 1.388927] pci 0000:00:00.0: BAR 1 [mem 0x00000000-0x000fffff]
> [ 1.394826] pci 0000:00:00.0: PCI bridge to [bus 01-ff]
> [ 1.400061] pci 0000:00:00.0: bridge window [io 0x100000-0x100fff]
> [ 1.406460] pci 0000:00:00.0: bridge window [mem 0x00000000-0x000fffff]
> [ 1.413245] pci 0000:00:00.0: bridge window [mem 0x00000000-0x000fffff 64bit pref]
> [ 1.421012] pci 0000:00:00.0: supports D1
> [ 1.424948] pci 0000:00:00.0: PME# supported from D0 D1 D3hot D3cold
> [ 1.432718] pci 0000:01:00.0: [1987:5015] type 00 class 0x010802 PCIe Endpoint
> [ 1.438698] pci 0000:01:00.0: BAR 0 [mem 0x00000000-0x00003fff 64bit]
> [ 1.445426] pci 0000:01:00.0: 4.000 Gb/s available PCIe bandwidth, limited by 2.5 GT/s PCIe x2 link at 0000:00:00.0 (capable of 31.504 Gb/s with 8.0 GT/s PCIe x4 link)
> [ 1.464897] pci_bus 0000:01: busn_res: [bus 01-ff] end is updated to 01
>
> And this is with the regulator change:
>
> [ 0.410796] spacemit-k1-pcie ca400000.pcie: host bridge /soc/pcie-bus/pcie@ca400000 ranges:
> [ 0.410836] spacemit-k1-pcie ca800000.pcie: host bridge /soc/pcie-bus/pcie@ca800000 ranges:
> [ 0.410889] spacemit-k1-pcie ca800000.pcie: IO 0x00b7002000..0x00b7101fff -> 0x0000000000
> [ 0.410917] spacemit-k1-pcie ca800000.pcie: MEM 0x00a0000000..0x00afffffff -> 0x00a0000000
> [ 0.410932] spacemit-k1-pcie ca800000.pcie: MEM 0x00b0000000..0x00b6ffffff -> 0x00b0000000
> [ 0.424651] spacemit-k1-pcie ca400000.pcie: IO 0x009f002000..0x009f101fff -> 0x0000000000
> [ 0.436446] spacemit-k1-pcie ca400000.pcie: MEM 0x0090000000..0x009effffff -> 0x0090000000
> [ 0.513897] spacemit-k1-pcie ca800000.pcie: iATU: unroll T, 8 ob, 8 ib, align 4K, limit 4G
> [ 0.559595] spacemit-k1-pcie ca400000.pcie: iATU: unroll T, 8 ob, 8 ib, align 4K, limit 4G
> [ 0.839412] spacemit-k1-pcie ca400000.pcie: PCIe Gen.1 x2 link up
> [ 0.847078] spacemit-k1-pcie ca400000.pcie: PCI host bridge to bus 0000:00
> [ 0.857600] pci_bus 0000:00: root bus resource [bus 00-ff]
> [ 0.868702] pci_bus 0000:00: root bus resource [io 0x100000-0x1fffff] (bus address [0x0000-0xfffff])
> [ 1.146409] pci_bus 0000:00: root bus resource [mem 0x90000000-0x9effffff]
> [ 1.373742] pci 0000:00:00.0: [1e5d:3003] type 01 class 0x060400 PCIe Root Port
> [ 1.380963] pci 0000:00:00.0: BAR 0 [mem 0x00000000-0x000fffff]
> [ 1.386883] pci 0000:00:00.0: BAR 1 [mem 0x00000000-0x000fffff]
> [ 1.392808] pci 0000:00:00.0: PCI bridge to [bus 01-ff]
> [ 1.395394] pci 0000:00:00.0: bridge window [io 0x100000-0x100fff]
> [ 1.401811] pci 0000:00:00.0: bridge window [mem 0x00000000-0x000fffff]
> [ 1.408583] pci 0000:00:00.0: bridge window [mem 0x00000000-0x000fffff 64bit pref]
> [ 1.416354] pci 0000:00:00.0: supports D1
> [ 1.420294] pci 0000:00:00.0: PME# supported from D0 D1 D3hot D3cold
> [ 1.428220] pci 0000:01:00.0: [1987:5015] type 00 class 0x010802 PCIe Endpoint
> [ 1.434034] pci 0000:01:00.0: BAR 0 [mem 0x00000000-0x00003fff 64bit]
> [ 1.440772] pci 0000:01:00.0: 4.000 Gb/s available PCIe bandwidth, limited by 2.5 GT/s PCIe x2 link at 0000:00:00.0 (capable of 31.504 Gb/s with 8.0 GT/s PCIe x4 link)
> [ 1.463390] pci 0000:01:00.0: pcie_aspm_override_default_link_state
> [ 1.467000] pci 0000:01:00.0: ASPM: default states L1
> [ 1.472093] pci_bus 0000:01: busn_res: [bus 01-ff] end is updated to 01
>
> Note how the line pcie_aspm_override_default_link_state arrives too
> late.
>
> Regards
> Aurelien
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-11-10 15:20 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-07 19:15 [PATCH v5 0/7] Introduce SpacemiT K1 PCIe phy and host controller Alex Elder
2025-11-07 19:15 ` [PATCH v5 3/7] dt-bindings: pci: spacemit: Introduce PCIe " Alex Elder
2025-11-07 19:15 ` [PATCH v5 5/7] PCI: spacemit: Add SpacemiT PCIe host driver Alex Elder
2025-11-08 13:00 ` Christophe JAILLET
2025-11-10 15:05 ` Alex Elder
2025-11-08 11:08 ` [PATCH v5 0/7] Introduce SpacemiT K1 PCIe phy and host controller Aurelien Jarno
2025-11-10 15:20 ` Alex Elder
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox