[PATCH v12 0/9] Add STM32MP25 PCIe drivers

[PATCH v12 0/9] Add STM32MP25 PCIe drivers

[Christian Bruel]

Changes in v12;
   Fix warning reported by kernel test robot <lkp@intel.com>

Changes in v11;
   Address comments from Manivanna:
   - RC driver: Do not call pm_runtime_get_noresume in probe
                More uses of dev_err_probe
   - EP driver: Use level triggered PERST# irq

Changes in v10;
   - Update pcie_ep bindings with dbi2 and atu regs,
     thus remove Reviewed-by and Acked-by.
   
Changes in v9:
   - Describe atu and dbi2 shadowed registers in pcie_ep node
   Address RC and EP drivers comments from Manivanna:
   - Use dev_error_probe() for pm_runtime_enable() calls
   - Reword Kconfig help message
   - Move pm_runtime_get_noresume() before devm_pm_runtime_enable()

Changes in v8:
   - Whitespace in comment
   
Changes in v7:
   - Use device_init_wakeup to enable wakeup
   - Fix comments (Bjorn)

Changes in v6:
   - Call device_wakeup_enable() to fix WAKE# wakeup.
   Address comments from Manivanna:
   - Fix/Add Comments
   - Fix DT indents
   - Remove dw_pcie_ep_linkup() in EP start link
   - Add PCIE_T_PVPERL_MS delay in RC PERST# deassert
   
Changes in v5:
   Address driver comments from Manivanna:
   - Use dw_pcie_{suspend/resume}_noirq instead of private ones.
   - Move dw_pcie_host_init() to probe
   - Add stm32_remove_pcie_port cleanup function
   - Use of_node_put in stm32_pcie_parse_port
   - Remove wakeup-source property
   - Use generic dev_pm_set_dedicated_wake_irq to support wake# irq
   
Changes in v4:
   Address bindings comments Rob Herring
   - Remove phy property form common yaml
   - Remove phy-name property
   - Move wake_gpio and reset_gpio to the host root port
   
Changes in v3:
   Address comments from Manivanna, Rob and Bjorn:
   - Move host wakeup helper to dwc core (Mani)
   - Drop num-lanes=<1> from bindings (Rob)
   - Fix PCI address of I/O region (Mani)
   - Moved PHY to a RC rootport subsection (Bjorn, Mani)
   - Replaced dma-limit quirk by dma-ranges property (Bjorn)
   - Moved out perst assert/deassert from start/stop link (Mani)
   - Drop link_up test optim (Mani)
   - DT and comments rephrasing (Bjorn)
   - Add dts entries now that the combophy entries has landed
   - Drop delaying Configuration Requests

Changes in v2:
   - Fix st,stm32-pcie-common.yaml dt_binding_check	

Changes in v1:
   Address comments from Rob Herring and Bjorn Helgaas:
   - Drop st,limit-mrrs and st,max-payload-size from this patchset
   - Remove single reset and clocks binding names and misc yaml cleanups
   - Split RC/EP common bindings to a separate schema file
   - Use correct PCIE_T_PERST_CLK_US and PCIE_T_RRS_READY_MS defines
   - Use .remove instead of .remove_new
   - Fix bar reset sequence in EP driver
   - Use cleanup blocks for error handling
   - Cosmetic fixes

Christian Bruel (9):
  dt-bindings: PCI: Add STM32MP25 PCIe Root Complex bindings
  PCI: stm32: Add PCIe host support for STM32MP25
  dt-bindings: PCI: Add STM32MP25 PCIe Endpoint bindings
  PCI: stm32: Add PCIe Endpoint support for STM32MP25
  MAINTAINERS: add entry for ST STM32MP25 PCIe drivers
  arm64: dts: st: add PCIe pinctrl entries in stm32mp25-pinctrl.dtsi
  arm64: dts: st: Add PCIe Root Complex mode on stm32mp251
  arm64: dts: st: Add PCIe Endpoint mode on stm32mp251
  arm64: dts: st: Enable PCIe on the stm32mp257f-ev1 board

 .../bindings/pci/st,stm32-pcie-common.yaml    |  33 ++
 .../bindings/pci/st,stm32-pcie-ep.yaml        |  73 ++++
 .../bindings/pci/st,stm32-pcie-host.yaml      | 112 +++++
 MAINTAINERS                                   |   7 +
 arch/arm64/boot/dts/st/stm32mp25-pinctrl.dtsi |  20 +
 arch/arm64/boot/dts/st/stm32mp251.dtsi        |  59 +++
 arch/arm64/boot/dts/st/stm32mp257f-ev1.dts    |  21 +
 drivers/pci/controller/dwc/Kconfig            |  24 ++
 drivers/pci/controller/dwc/Makefile           |   2 +
 drivers/pci/controller/dwc/pcie-stm32-ep.c    | 384 ++++++++++++++++++
 drivers/pci/controller/dwc/pcie-stm32.c       | 368 +++++++++++++++++
 drivers/pci/controller/dwc/pcie-stm32.h       |  16 +
 12 files changed, 1119 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/st,stm32-pcie-common.yaml
 create mode 100644 Documentation/devicetree/bindings/pci/st,stm32-pcie-ep.yaml
 create mode 100644 Documentation/devicetree/bindings/pci/st,stm32-pcie-host.yaml
 create mode 100644 drivers/pci/controller/dwc/pcie-stm32-ep.c
 create mode 100644 drivers/pci/controller/dwc/pcie-stm32.c
 create mode 100644 drivers/pci/controller/dwc/pcie-stm32.h


base-commit: b27cc623e01be9de1580eaa913508b237a7a9673
-- 
2.34.1

[PATCH v12 1/9] dt-bindings: PCI: Add STM32MP25 PCIe Root Complex bindings

[Christian Bruel]

Document the bindings for STM32MP25 PCIe Controller configured in
root complex mode with one root port.

Supports 4 INTx and MSI interrupts from the ARM GICv2m controller.

STM32 PCIe may be in a power domain which is the case for the STM32MP25
based boards.

Supports WAKE# from wake-gpios

Signed-off-by: Christian Bruel <christian.bruel@foss.st.com>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 .../bindings/pci/st,stm32-pcie-common.yaml    |  33 ++++++
 .../bindings/pci/st,stm32-pcie-host.yaml      | 112 ++++++++++++++++++
 2 files changed, 145 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/st,stm32-pcie-common.yaml
 create mode 100644 Documentation/devicetree/bindings/pci/st,stm32-pcie-host.yaml

diff --git a/Documentation/devicetree/bindings/pci/st,stm32-pcie-common.yaml b/Documentation/devicetree/bindings/pci/st,stm32-pcie-common.yaml
new file mode 100644
index 000000000000..5adbff259204
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/st,stm32-pcie-common.yaml
@@ -0,0 +1,33 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/st,stm32-pcie-common.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: STM32MP25 PCIe RC/EP controller
+
+maintainers:
+  - Christian Bruel <christian.bruel@foss.st.com>
+
+description:
+  STM32MP25 PCIe RC/EP common properties
+
+properties:
+  clocks:
+    maxItems: 1
+    description: PCIe system clock
+
+  resets:
+    maxItems: 1
+
+  power-domains:
+    maxItems: 1
+
+  access-controllers:
+    maxItems: 1
+
+required:
+  - clocks
+  - resets
+
+additionalProperties: true
diff --git a/Documentation/devicetree/bindings/pci/st,stm32-pcie-host.yaml b/Documentation/devicetree/bindings/pci/st,stm32-pcie-host.yaml
new file mode 100644
index 000000000000..443bfe2cdc98
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/st,stm32-pcie-host.yaml
@@ -0,0 +1,112 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/st,stm32-pcie-host.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: STMicroelectronics STM32MP25 PCIe Root Complex
+
+maintainers:
+  - Christian Bruel <christian.bruel@foss.st.com>
+
+description:
+  PCIe root complex controller based on the Synopsys DesignWare PCIe core.
+
+allOf:
+  - $ref: /schemas/pci/snps,dw-pcie.yaml#
+  - $ref: /schemas/pci/st,stm32-pcie-common.yaml#
+
+properties:
+  compatible:
+    const: st,stm32mp25-pcie-rc
+
+  reg:
+    items:
+      - description: Data Bus Interface (DBI) registers.
+      - description: PCIe configuration registers.
+
+  reg-names:
+    items:
+      - const: dbi
+      - const: config
+
+  msi-parent:
+    maxItems: 1
+
+patternProperties:
+  '^pcie@[0-2],0$':
+    type: object
+    $ref: /schemas/pci/pci-pci-bridge.yaml#
+
+    properties:
+      reg:
+        maxItems: 1
+
+      phys:
+        maxItems: 1
+
+      reset-gpios:
+        description: GPIO controlled connection to PERST# signal
+        maxItems: 1
+
+      wake-gpios:
+        description: GPIO used as WAKE# input signal
+        maxItems: 1
+
+    required:
+      - phys
+      - ranges
+
+    unevaluatedProperties: false
+
+required:
+  - interrupt-map
+  - interrupt-map-mask
+  - ranges
+  - dma-ranges
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/st,stm32mp25-rcc.h>
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/phy/phy.h>
+    #include <dt-bindings/reset/st,stm32mp25-rcc.h>
+
+    pcie@48400000 {
+        compatible = "st,stm32mp25-pcie-rc";
+        device_type = "pci";
+        reg = <0x48400000 0x400000>,
+              <0x10000000 0x10000>;
+        reg-names = "dbi", "config";
+        #interrupt-cells = <1>;
+        interrupt-map-mask = <0 0 0 7>;
+        interrupt-map = <0 0 0 1 &intc 0 0 GIC_SPI 264 IRQ_TYPE_LEVEL_HIGH>,
+                        <0 0 0 2 &intc 0 0 GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH>,
+                        <0 0 0 3 &intc 0 0 GIC_SPI 266 IRQ_TYPE_LEVEL_HIGH>,
+                        <0 0 0 4 &intc 0 0 GIC_SPI 267 IRQ_TYPE_LEVEL_HIGH>;
+        #address-cells = <3>;
+        #size-cells = <2>;
+        ranges = <0x01000000 0x0 0x00000000 0x10010000 0x0 0x10000>,
+                 <0x02000000 0x0 0x10020000 0x10020000 0x0 0x7fe0000>,
+                 <0x42000000 0x0 0x18000000 0x18000000 0x0 0x8000000>;
+        dma-ranges = <0x42000000 0x0 0x80000000 0x80000000 0x0 0x80000000>;
+        clocks = <&rcc CK_BUS_PCIE>;
+        resets = <&rcc PCIE_R>;
+        msi-parent = <&v2m0>;
+        access-controllers = <&rifsc 68>;
+        power-domains = <&CLUSTER_PD>;
+
+        pcie@0,0 {
+            device_type = "pci";
+            reg = <0x0 0x0 0x0 0x0 0x0>;
+            phys = <&combophy PHY_TYPE_PCIE>;
+            wake-gpios = <&gpioh 5 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>;
+            reset-gpios = <&gpioj 8 GPIO_ACTIVE_LOW>;
+            #address-cells = <3>;
+            #size-cells = <2>;
+            ranges;
+        };
+    };
-- 
2.34.1

[PATCH v12 2/9] PCI: stm32: Add PCIe host support for STM32MP25

[Christian Bruel]

Add driver for the STM32MP25 SoC PCIe Gen1 2.5 GT/s and Gen2 5GT/s
controller based on the DesignWare PCIe core.

Supports MSI via GICv2m, Single Virtual Channel, Single Function

Supports WAKE# GPIO.

Signed-off-by: Christian Bruel <christian.bruel@foss.st.com>
---
 drivers/pci/controller/dwc/Kconfig      |  12 +
 drivers/pci/controller/dwc/Makefile     |   1 +
 drivers/pci/controller/dwc/pcie-stm32.c | 368 ++++++++++++++++++++++++
 drivers/pci/controller/dwc/pcie-stm32.h |  15 +
 4 files changed, 396 insertions(+)
 create mode 100644 drivers/pci/controller/dwc/pcie-stm32.c
 create mode 100644 drivers/pci/controller/dwc/pcie-stm32.h

diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index d9f0386396ed..387151f25f5f 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -410,6 +410,18 @@ config PCIE_SPEAR13XX
 	help
 	  Say Y here if you want PCIe support on SPEAr13XX SoCs.
 
+config PCIE_STM32_HOST
+	tristate "STMicroelectronics STM32MP25 PCIe Controller (host mode)"
+	depends on ARCH_STM32 || COMPILE_TEST
+	depends on PCI_MSI
+	select PCIE_DW_HOST
+	help
+	  Enables Root Complex (RC) support for the DesignWare core based PCIe
+	  controller found in STM32MP25 SoC.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called pcie-stm32.
+
 config PCI_DRA7XX
 	tristate
 
diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
index 908cb7f345db..9d3b43504725 100644
--- a/drivers/pci/controller/dwc/Makefile
+++ b/drivers/pci/controller/dwc/Makefile
@@ -30,6 +30,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_STM32_HOST) += pcie-stm32.o
 
 # The following drivers are for devices that use the generic ACPI
 # pci_root.c driver but don't support standard ECAM config access.
diff --git a/drivers/pci/controller/dwc/pcie-stm32.c b/drivers/pci/controller/dwc/pcie-stm32.c
new file mode 100644
index 000000000000..6bf1b63f88c6
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-stm32.c
@@ -0,0 +1,368 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * STMicroelectronics STM32MP25 PCIe root complex driver.
+ *
+ * Copyright (C) 2025 STMicroelectronics
+ * Author: Christian Bruel <christian.bruel@foss.st.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of_platform.h>
+#include <linux/phy/phy.h>
+#include <linux/pinctrl/devinfo.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/pm_wakeirq.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+#include "pcie-designware.h"
+#include "pcie-stm32.h"
+#include "../../pci.h"
+
+struct stm32_pcie {
+	struct dw_pcie pci;
+	struct regmap *regmap;
+	struct reset_control *rst;
+	struct phy *phy;
+	struct clk *clk;
+	struct gpio_desc *perst_gpio;
+	struct gpio_desc *wake_gpio;
+};
+
+static void stm32_pcie_deassert_perst(struct stm32_pcie *stm32_pcie)
+{
+	/* Delay PERST# de-assertion until the power stabilizes */
+	msleep(PCIE_T_PVPERL_MS);
+
+	gpiod_set_value(stm32_pcie->perst_gpio, 0);
+
+	/* Wait for the REFCLK to stabilize */
+	if (stm32_pcie->perst_gpio)
+		msleep(PCIE_T_RRS_READY_MS);
+}
+
+static void stm32_pcie_assert_perst(struct stm32_pcie *stm32_pcie)
+{
+	gpiod_set_value(stm32_pcie->perst_gpio, 1);
+}
+
+static int stm32_pcie_start_link(struct dw_pcie *pci)
+{
+	struct stm32_pcie *stm32_pcie = to_stm32_pcie(pci);
+
+	return regmap_update_bits(stm32_pcie->regmap, SYSCFG_PCIECR,
+				  STM32MP25_PCIECR_LTSSM_EN,
+				  STM32MP25_PCIECR_LTSSM_EN);
+}
+
+static void stm32_pcie_stop_link(struct dw_pcie *pci)
+{
+	struct stm32_pcie *stm32_pcie = to_stm32_pcie(pci);
+
+	regmap_update_bits(stm32_pcie->regmap, SYSCFG_PCIECR,
+			   STM32MP25_PCIECR_LTSSM_EN, 0);
+}
+
+static int stm32_pcie_suspend_noirq(struct device *dev)
+{
+	struct stm32_pcie *stm32_pcie = dev_get_drvdata(dev);
+	int ret;
+
+	ret = dw_pcie_suspend_noirq(&stm32_pcie->pci);
+	if (ret)
+		return ret;
+
+	stm32_pcie_assert_perst(stm32_pcie);
+
+	clk_disable_unprepare(stm32_pcie->clk);
+
+	if (!device_wakeup_path(dev))
+		phy_exit(stm32_pcie->phy);
+
+	return pinctrl_pm_select_sleep_state(dev);
+}
+
+static int stm32_pcie_resume_noirq(struct device *dev)
+{
+	struct stm32_pcie *stm32_pcie = dev_get_drvdata(dev);
+	int ret;
+
+	/*
+	 * The core clock is gated with CLKREQ# from the COMBOPHY REFCLK,
+	 * thus if no device is present, must force it low with an init pinmux
+	 * to be able to access the DBI registers.
+	 */
+	if (!IS_ERR(dev->pins->init_state))
+		ret = pinctrl_select_state(dev->pins->p, dev->pins->init_state);
+	else
+		ret = pinctrl_pm_select_default_state(dev);
+
+	if (ret) {
+		dev_err(dev, "Failed to activate pinctrl pm state: %d\n", ret);
+		return ret;
+	}
+
+	if (!device_wakeup_path(dev)) {
+		ret = phy_init(stm32_pcie->phy);
+		if (ret) {
+			pinctrl_pm_select_default_state(dev);
+			return ret;
+		}
+	}
+
+	ret = clk_prepare_enable(stm32_pcie->clk);
+	if (ret)
+		goto err_phy_exit;
+
+	stm32_pcie_deassert_perst(stm32_pcie);
+
+	ret = dw_pcie_resume_noirq(&stm32_pcie->pci);
+	if (ret)
+		goto err_disable_clk;
+
+	pinctrl_pm_select_default_state(dev);
+
+	return 0;
+
+err_disable_clk:
+	stm32_pcie_assert_perst(stm32_pcie);
+	clk_disable_unprepare(stm32_pcie->clk);
+
+err_phy_exit:
+	phy_exit(stm32_pcie->phy);
+	pinctrl_pm_select_default_state(dev);
+
+	return ret;
+}
+
+static const struct dev_pm_ops stm32_pcie_pm_ops = {
+	NOIRQ_SYSTEM_SLEEP_PM_OPS(stm32_pcie_suspend_noirq,
+				  stm32_pcie_resume_noirq)
+};
+
+static const struct dw_pcie_host_ops stm32_pcie_host_ops = {
+};
+
+static const struct dw_pcie_ops dw_pcie_ops = {
+	.start_link = stm32_pcie_start_link,
+	.stop_link = stm32_pcie_stop_link
+};
+
+static int stm32_add_pcie_port(struct stm32_pcie *stm32_pcie)
+{
+	struct device *dev = stm32_pcie->pci.dev;
+	unsigned int wake_irq;
+	int ret;
+
+	/* Start to enable resources with PERST# asserted */
+
+	ret = phy_set_mode(stm32_pcie->phy, PHY_MODE_PCIE);
+	if (ret)
+		return ret;
+
+	ret = phy_init(stm32_pcie->phy);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(stm32_pcie->regmap, SYSCFG_PCIECR,
+				 STM32MP25_PCIECR_TYPE_MASK,
+				 STM32MP25_PCIECR_RC);
+	if (ret)
+		goto err_phy_exit;
+
+	stm32_pcie_deassert_perst(stm32_pcie);
+
+	if (stm32_pcie->wake_gpio) {
+		wake_irq = gpiod_to_irq(stm32_pcie->wake_gpio);
+		ret = dev_pm_set_dedicated_wake_irq(dev, wake_irq);
+		if (ret) {
+			dev_err(dev, "Failed to enable wakeup irq %d\n", ret);
+			goto err_assert_perst;
+		}
+		irq_set_irq_type(wake_irq, IRQ_TYPE_EDGE_FALLING);
+	}
+
+	return 0;
+
+err_assert_perst:
+	stm32_pcie_assert_perst(stm32_pcie);
+
+err_phy_exit:
+	phy_exit(stm32_pcie->phy);
+
+	return ret;
+}
+
+static void stm32_remove_pcie_port(struct stm32_pcie *stm32_pcie)
+{
+	dev_pm_clear_wake_irq(stm32_pcie->pci.dev);
+
+	stm32_pcie_assert_perst(stm32_pcie);
+
+	phy_exit(stm32_pcie->phy);
+}
+
+static int stm32_pcie_parse_port(struct stm32_pcie *stm32_pcie)
+{
+	struct device *dev = stm32_pcie->pci.dev;
+	struct device_node *root_port;
+
+	root_port = of_get_next_available_child(dev->of_node, NULL);
+
+	stm32_pcie->phy = devm_of_phy_get(dev, root_port, NULL);
+	if (IS_ERR(stm32_pcie->phy)) {
+		of_node_put(root_port);
+		return dev_err_probe(dev, PTR_ERR(stm32_pcie->phy),
+				     "Failed to get pcie-phy\n");
+	}
+
+	stm32_pcie->perst_gpio = devm_fwnode_gpiod_get(dev, of_fwnode_handle(root_port),
+						       "reset", GPIOD_OUT_HIGH, NULL);
+	if (IS_ERR(stm32_pcie->perst_gpio)) {
+		if (PTR_ERR(stm32_pcie->perst_gpio) != -ENOENT) {
+			of_node_put(root_port);
+			return dev_err_probe(dev, PTR_ERR(stm32_pcie->perst_gpio),
+					     "Failed to get reset GPIO\n");
+		}
+		stm32_pcie->perst_gpio = NULL;
+	}
+
+	stm32_pcie->wake_gpio = devm_fwnode_gpiod_get(dev, of_fwnode_handle(root_port),
+						      "wake", GPIOD_IN, NULL);
+
+	if (IS_ERR(stm32_pcie->wake_gpio)) {
+		if (PTR_ERR(stm32_pcie->wake_gpio) != -ENOENT) {
+			of_node_put(root_port);
+			return dev_err_probe(dev, PTR_ERR(stm32_pcie->wake_gpio),
+					     "Failed to get wake GPIO\n");
+		}
+		stm32_pcie->wake_gpio = NULL;
+	}
+
+	of_node_put(root_port);
+
+	return 0;
+}
+
+static int stm32_pcie_probe(struct platform_device *pdev)
+{
+	struct stm32_pcie *stm32_pcie;
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	stm32_pcie = devm_kzalloc(dev, sizeof(*stm32_pcie), GFP_KERNEL);
+	if (!stm32_pcie)
+		return -ENOMEM;
+
+	stm32_pcie->pci.dev = dev;
+	stm32_pcie->pci.ops = &dw_pcie_ops;
+	stm32_pcie->pci.pp.ops = &stm32_pcie_host_ops;
+
+	stm32_pcie->regmap = syscon_regmap_lookup_by_compatible("st,stm32mp25-syscfg");
+	if (IS_ERR(stm32_pcie->regmap))
+		return dev_err_probe(dev, PTR_ERR(stm32_pcie->regmap),
+				     "No syscfg specified\n");
+
+	stm32_pcie->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(stm32_pcie->clk))
+		return dev_err_probe(dev, PTR_ERR(stm32_pcie->clk),
+				     "Failed to get PCIe clock source\n");
+
+	stm32_pcie->rst = devm_reset_control_get_exclusive(dev, NULL);
+	if (IS_ERR(stm32_pcie->rst))
+		return dev_err_probe(dev, PTR_ERR(stm32_pcie->rst),
+				     "Failed to get PCIe reset\n");
+
+	ret = stm32_pcie_parse_port(stm32_pcie);
+	if (ret)
+		return ret;
+
+	platform_set_drvdata(pdev, stm32_pcie);
+
+	ret = stm32_add_pcie_port(stm32_pcie);
+	if (ret)
+		return ret;
+
+	reset_control_assert(stm32_pcie->rst);
+	reset_control_deassert(stm32_pcie->rst);
+
+	ret = clk_prepare_enable(stm32_pcie->clk);
+	if (ret) {
+		dev_err(dev, "Core clock enable failed %d\n", ret);
+		goto err_remove_port;
+	}
+
+	ret = pm_runtime_set_active(dev);
+	if (ret < 0) {
+		clk_disable_unprepare(stm32_pcie->clk);
+		stm32_remove_pcie_port(stm32_pcie);
+		return dev_err_probe(dev, ret, "Failed to activate runtime PM\n");
+	}
+
+	pm_runtime_no_callbacks(dev);
+
+	ret = devm_pm_runtime_enable(dev);
+	if (ret < 0) {
+		clk_disable_unprepare(stm32_pcie->clk);
+		stm32_remove_pcie_port(stm32_pcie);
+		return dev_err_probe(dev, ret, "Failed to enable runtime PM\n");
+	}
+
+	ret = dw_pcie_host_init(&stm32_pcie->pci.pp);
+	if (ret)
+		goto err_disable_clk;
+
+	if (stm32_pcie->wake_gpio)
+		device_init_wakeup(dev, true);
+
+	return 0;
+
+err_disable_clk:
+	clk_disable_unprepare(stm32_pcie->clk);
+
+err_remove_port:
+	stm32_remove_pcie_port(stm32_pcie);
+
+	return ret;
+}
+
+static void stm32_pcie_remove(struct platform_device *pdev)
+{
+	struct stm32_pcie *stm32_pcie = platform_get_drvdata(pdev);
+	struct dw_pcie_rp *pp = &stm32_pcie->pci.pp;
+
+	if (stm32_pcie->wake_gpio)
+		device_init_wakeup(&pdev->dev, false);
+
+	dw_pcie_host_deinit(pp);
+
+	clk_disable_unprepare(stm32_pcie->clk);
+
+	stm32_remove_pcie_port(stm32_pcie);
+
+	pm_runtime_put_noidle(&pdev->dev);
+}
+
+static const struct of_device_id stm32_pcie_of_match[] = {
+	{ .compatible = "st,stm32mp25-pcie-rc" },
+	{},
+};
+
+static struct platform_driver stm32_pcie_driver = {
+	.probe = stm32_pcie_probe,
+	.remove = stm32_pcie_remove,
+	.driver = {
+		.name = "stm32-pcie",
+		.of_match_table = stm32_pcie_of_match,
+		.pm = &stm32_pcie_pm_ops,
+		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
+	},
+};
+
+module_platform_driver(stm32_pcie_driver);
+
+MODULE_AUTHOR("Christian Bruel <christian.bruel@foss.st.com>");
+MODULE_DESCRIPTION("STM32MP25 PCIe Controller driver");
+MODULE_LICENSE("GPL");
+MODULE_DEVICE_TABLE(of, stm32_pcie_of_match);
diff --git a/drivers/pci/controller/dwc/pcie-stm32.h b/drivers/pci/controller/dwc/pcie-stm32.h
new file mode 100644
index 000000000000..387112c4e42c
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-stm32.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * ST PCIe driver definitions for STM32-MP25 SoC
+ *
+ * Copyright (C) 2025 STMicroelectronics - All Rights Reserved
+ * Author: Christian Bruel <christian.bruel@foss.st.com>
+ */
+
+#define to_stm32_pcie(x)	dev_get_drvdata((x)->dev)
+
+#define STM32MP25_PCIECR_TYPE_MASK	GENMASK(11, 8)
+#define STM32MP25_PCIECR_LTSSM_EN	BIT(2)
+#define STM32MP25_PCIECR_RC		BIT(10)
+
+#define SYSCFG_PCIECR			0x6000
-- 
2.34.1

[PATCH v12 3/9] dt-bindings: PCI: Add STM32MP25 PCIe Endpoint bindings

[Christian Bruel]

STM32MP25 PCIe Controller is based on the DesignWare core configured as
end point mode from the SYSCFG register.

Signed-off-by: Christian Bruel <christian.bruel@foss.st.com>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
---
 .../bindings/pci/st,stm32-pcie-ep.yaml        | 73 +++++++++++++++++++
 1 file changed, 73 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/st,stm32-pcie-ep.yaml

diff --git a/Documentation/devicetree/bindings/pci/st,stm32-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/st,stm32-pcie-ep.yaml
new file mode 100644
index 000000000000..b076ada4f332
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/st,stm32-pcie-ep.yaml
@@ -0,0 +1,73 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/st,stm32-pcie-ep.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: STMicroelectronics STM32MP25 PCIe Endpoint
+
+maintainers:
+  - Christian Bruel <christian.bruel@foss.st.com>
+
+description:
+  PCIe endpoint controller based on the Synopsys DesignWare PCIe core.
+
+allOf:
+  - $ref: /schemas/pci/snps,dw-pcie-ep.yaml#
+  - $ref: /schemas/pci/st,stm32-pcie-common.yaml#
+
+properties:
+  compatible:
+    const: st,stm32mp25-pcie-ep
+
+  reg:
+    items:
+      - description: Data Bus Interface (DBI) registers.
+      - description: Data Bus Interface (DBI) shadow registers.
+      - description: Internal Address Translation Unit (iATU) registers.
+      - description: PCIe configuration registers.
+
+  reg-names:
+    items:
+      - const: dbi
+      - const: dbi2
+      - const: atu
+      - const: addr_space
+
+  reset-gpios:
+    description: GPIO controlled connection to PERST# signal
+    maxItems: 1
+
+  phys:
+    maxItems: 1
+
+required:
+  - phys
+  - reset-gpios
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/st,stm32mp25-rcc.h>
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/phy/phy.h>
+    #include <dt-bindings/reset/st,stm32mp25-rcc.h>
+
+    pcie-ep@48400000 {
+        compatible = "st,stm32mp25-pcie-ep";
+        reg = <0x48400000 0x400000>,
+              <0x48500000 0x100000>,
+              <0x48700000 0x80000>,
+              <0x10000000 0x10000000>;
+        reg-names = "dbi", "dbi2", "atu", "addr_space";
+        clocks = <&rcc CK_BUS_PCIE>;
+        phys = <&combophy PHY_TYPE_PCIE>;
+        resets = <&rcc PCIE_R>;
+        pinctrl-names = "default", "init";
+        pinctrl-0 = <&pcie_pins_a>;
+        pinctrl-1 = <&pcie_init_pins_a>;
+        reset-gpios = <&gpioj 8 GPIO_ACTIVE_LOW>;
+        access-controllers = <&rifsc 68>;
+        power-domains = <&CLUSTER_PD>;
+    };
-- 
2.34.1

[PATCH v12 4/9] PCI: stm32: Add PCIe Endpoint support for STM32MP25

[Christian Bruel]

Add driver to configure the STM32MP25 SoC PCIe Gen1 2.5GT/s or Gen2 5GT/s
controller based on the DesignWare PCIe core in endpoint mode.

Uses the common reference clock provided by the host.

The PCIe core_clk receives the pipe0_clk from the ComboPHY as input,
and the ComboPHY PLL must be locked for pipe0_clk to be ready.
Consequently, PCIe core registers cannot be accessed until the ComboPHY is
fully initialised and REFCLK is enabled and ready.

Signed-off-by: Christian Bruel <christian.bruel@foss.st.com>
---
 drivers/pci/controller/dwc/Kconfig         |  12 +
 drivers/pci/controller/dwc/Makefile        |   1 +
 drivers/pci/controller/dwc/pcie-stm32-ep.c | 384 +++++++++++++++++++++
 drivers/pci/controller/dwc/pcie-stm32.h    |   1 +
 4 files changed, 398 insertions(+)
 create mode 100644 drivers/pci/controller/dwc/pcie-stm32-ep.c

diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index 387151f25f5f..72b9c50c1dfd 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -422,6 +422,18 @@ config PCIE_STM32_HOST
 	  This driver can also be built as a module. If so, the module
 	  will be called pcie-stm32.
 
+config PCIE_STM32_EP
+	tristate "STMicroelectronics STM32MP25 PCIe Controller (endpoint mode)"
+	depends on ARCH_STM32 || COMPILE_TEST
+	depends on PCI_ENDPOINT
+	select PCIE_DW_EP
+	help
+	  Enables Endpoint (EP) support for the DesignWare core based PCIe
+	  controller found in STM32MP25 SoC.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called pcie-stm32-ep.
+
 config PCI_DRA7XX
 	tristate
 
diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
index 9d3b43504725..85ec6804a299 100644
--- a/drivers/pci/controller/dwc/Makefile
+++ b/drivers/pci/controller/dwc/Makefile
@@ -31,6 +31,7 @@ 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_STM32_HOST) += pcie-stm32.o
+obj-$(CONFIG_PCIE_STM32_EP) += pcie-stm32-ep.o
 
 # The following drivers are for devices that use the generic ACPI
 # pci_root.c driver but don't support standard ECAM config access.
diff --git a/drivers/pci/controller/dwc/pcie-stm32-ep.c b/drivers/pci/controller/dwc/pcie-stm32-ep.c
new file mode 100644
index 000000000000..1f46bcf0c79f
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-stm32-ep.c
@@ -0,0 +1,384 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * STMicroelectronics STM32MP25 PCIe endpoint driver.
+ *
+ * Copyright (C) 2025 STMicroelectronics
+ * Author: Christian Bruel <christian.bruel@foss.st.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of_platform.h>
+#include <linux/of_gpio.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+#include "pcie-designware.h"
+#include "pcie-stm32.h"
+
+enum stm32_pcie_ep_link_status {
+	STM32_PCIE_EP_LINK_DISABLED,
+	STM32_PCIE_EP_LINK_ENABLED,
+};
+
+struct stm32_pcie {
+	struct dw_pcie pci;
+	struct regmap *regmap;
+	struct reset_control *rst;
+	struct phy *phy;
+	struct clk *clk;
+	struct gpio_desc *perst_gpio;
+	enum stm32_pcie_ep_link_status link_status;
+	unsigned int perst_irq;
+};
+
+static void stm32_pcie_ep_init(struct dw_pcie_ep *ep)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+	enum pci_barno bar;
+
+	for (bar = 0; bar < PCI_STD_NUM_BARS; bar++)
+		dw_pcie_ep_reset_bar(pci, bar);
+}
+
+static int stm32_pcie_enable_link(struct dw_pcie *pci)
+{
+	struct stm32_pcie *stm32_pcie = to_stm32_pcie(pci);
+
+	regmap_update_bits(stm32_pcie->regmap, SYSCFG_PCIECR,
+			   STM32MP25_PCIECR_LTSSM_EN,
+			   STM32MP25_PCIECR_LTSSM_EN);
+
+	return dw_pcie_wait_for_link(pci);
+}
+
+static void stm32_pcie_disable_link(struct dw_pcie *pci)
+{
+	struct stm32_pcie *stm32_pcie = to_stm32_pcie(pci);
+
+	regmap_update_bits(stm32_pcie->regmap, SYSCFG_PCIECR, STM32MP25_PCIECR_LTSSM_EN, 0);
+}
+
+static int stm32_pcie_start_link(struct dw_pcie *pci)
+{
+	struct stm32_pcie *stm32_pcie = to_stm32_pcie(pci);
+	int ret;
+
+	if (stm32_pcie->link_status == STM32_PCIE_EP_LINK_ENABLED) {
+		dev_dbg(pci->dev, "Link is already enabled\n");
+		return 0;
+	}
+
+	dev_dbg(pci->dev, "Enable link\n");
+
+	ret = stm32_pcie_enable_link(pci);
+	if (ret) {
+		dev_err(pci->dev, "PCIe cannot establish link: %d\n", ret);
+		return ret;
+	}
+
+	enable_irq(stm32_pcie->perst_irq);
+
+	stm32_pcie->link_status = STM32_PCIE_EP_LINK_ENABLED;
+
+	return 0;
+}
+
+static void stm32_pcie_stop_link(struct dw_pcie *pci)
+{
+	struct stm32_pcie *stm32_pcie = to_stm32_pcie(pci);
+
+	if (stm32_pcie->link_status == STM32_PCIE_EP_LINK_DISABLED) {
+		dev_dbg(pci->dev, "Link is already disabled\n");
+		return;
+	}
+
+	dev_dbg(pci->dev, "Disable link\n");
+
+	disable_irq(stm32_pcie->perst_irq);
+
+	stm32_pcie_disable_link(pci);
+
+	stm32_pcie->link_status = STM32_PCIE_EP_LINK_DISABLED;
+}
+
+static int stm32_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
+				unsigned int type, u16 interrupt_num)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+
+	switch (type) {
+	case PCI_IRQ_INTX:
+		return dw_pcie_ep_raise_intx_irq(ep, func_no);
+	case PCI_IRQ_MSI:
+		return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num);
+	default:
+		dev_err(pci->dev, "UNKNOWN IRQ type\n");
+		return -EINVAL;
+	}
+}
+
+static const struct pci_epc_features stm32_pcie_epc_features = {
+	.msi_capable = true,
+	.align = SZ_64K,
+};
+
+static const struct pci_epc_features*
+stm32_pcie_get_features(struct dw_pcie_ep *ep)
+{
+	return &stm32_pcie_epc_features;
+}
+
+static const struct dw_pcie_ep_ops stm32_pcie_ep_ops = {
+	.init = stm32_pcie_ep_init,
+	.raise_irq = stm32_pcie_raise_irq,
+	.get_features = stm32_pcie_get_features,
+};
+
+static const struct dw_pcie_ops dw_pcie_ops = {
+	.start_link = stm32_pcie_start_link,
+	.stop_link = stm32_pcie_stop_link,
+};
+
+static int stm32_pcie_enable_resources(struct stm32_pcie *stm32_pcie)
+{
+	int ret;
+
+	ret = phy_init(stm32_pcie->phy);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(stm32_pcie->clk);
+	if (ret)
+		phy_exit(stm32_pcie->phy);
+
+	return ret;
+}
+
+static void stm32_pcie_disable_resources(struct stm32_pcie *stm32_pcie)
+{
+	clk_disable_unprepare(stm32_pcie->clk);
+
+	phy_exit(stm32_pcie->phy);
+}
+
+static void stm32_pcie_perst_assert(struct dw_pcie *pci)
+{
+	struct stm32_pcie *stm32_pcie = to_stm32_pcie(pci);
+	struct dw_pcie_ep *ep = &stm32_pcie->pci.ep;
+	struct device *dev = pci->dev;
+
+	dev_dbg(dev, "PERST asserted by host\n");
+
+	pci_epc_deinit_notify(ep->epc);
+
+	stm32_pcie_disable_resources(stm32_pcie);
+
+	pm_runtime_put_sync(dev);
+}
+
+static void stm32_pcie_perst_deassert(struct dw_pcie *pci)
+{
+	struct stm32_pcie *stm32_pcie = to_stm32_pcie(pci);
+	struct device *dev = pci->dev;
+	struct dw_pcie_ep *ep = &pci->ep;
+	int ret;
+
+	dev_dbg(dev, "PERST de-asserted by host\n");
+
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret < 0) {
+		dev_err(dev, "Failed to resume runtime PM: %d\n", ret);
+		return;
+	}
+
+	ret = stm32_pcie_enable_resources(stm32_pcie);
+	if (ret) {
+		dev_err(dev, "Failed to enable resources: %d\n", ret);
+		goto err_pm_put_sync;
+	}
+
+	/*
+	 * Need to reprogram the configuration space registers here because the
+	 * DBI registers were incorrectly reset by the PHY RCC during phy_init().
+	 */
+	ret = dw_pcie_ep_init_registers(ep);
+	if (ret) {
+		dev_err(dev, "Failed to complete initialization: %d\n", ret);
+		goto err_disable_resources;
+	}
+
+	pci_epc_init_notify(ep->epc);
+
+	return;
+
+err_disable_resources:
+	stm32_pcie_disable_resources(stm32_pcie);
+
+err_pm_put_sync:
+	pm_runtime_put_sync(dev);
+}
+
+static irqreturn_t stm32_pcie_ep_perst_irq_thread(int irq, void *data)
+{
+	struct stm32_pcie *stm32_pcie = data;
+	struct dw_pcie *pci = &stm32_pcie->pci;
+	u32 perst;
+
+	perst = gpiod_get_value(stm32_pcie->perst_gpio);
+	if (perst)
+		stm32_pcie_perst_assert(pci);
+	else
+		stm32_pcie_perst_deassert(pci);
+
+	irq_set_irq_type(gpiod_to_irq(stm32_pcie->perst_gpio),
+			 (perst ? IRQF_TRIGGER_HIGH : IRQF_TRIGGER_LOW));
+
+	return IRQ_HANDLED;
+}
+
+static int stm32_add_pcie_ep(struct stm32_pcie *stm32_pcie,
+			     struct platform_device *pdev)
+{
+	struct dw_pcie_ep *ep = &stm32_pcie->pci.ep;
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	ret = regmap_update_bits(stm32_pcie->regmap, SYSCFG_PCIECR,
+				 STM32MP25_PCIECR_TYPE_MASK,
+				 STM32MP25_PCIECR_EP);
+	if (ret)
+		return ret;
+
+	reset_control_assert(stm32_pcie->rst);
+	reset_control_deassert(stm32_pcie->rst);
+
+	ep->ops = &stm32_pcie_ep_ops;
+
+	ret = dw_pcie_ep_init(ep);
+	if (ret) {
+		dev_err(dev, "Failed to initialize ep: %d\n", ret);
+		return ret;
+	}
+
+	ret = stm32_pcie_enable_resources(stm32_pcie);
+	if (ret) {
+		dev_err(dev, "Failed to enable resources: %d\n", ret);
+		dw_pcie_ep_deinit(ep);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int stm32_pcie_probe(struct platform_device *pdev)
+{
+	struct stm32_pcie *stm32_pcie;
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	stm32_pcie = devm_kzalloc(dev, sizeof(*stm32_pcie), GFP_KERNEL);
+	if (!stm32_pcie)
+		return -ENOMEM;
+
+	stm32_pcie->pci.dev = dev;
+	stm32_pcie->pci.ops = &dw_pcie_ops;
+
+	stm32_pcie->regmap = syscon_regmap_lookup_by_compatible("st,stm32mp25-syscfg");
+	if (IS_ERR(stm32_pcie->regmap))
+		return dev_err_probe(dev, PTR_ERR(stm32_pcie->regmap),
+				     "No syscfg specified\n");
+
+	stm32_pcie->phy = devm_phy_get(dev, NULL);
+	if (IS_ERR(stm32_pcie->phy))
+		return dev_err_probe(dev, PTR_ERR(stm32_pcie->phy),
+				     "failed to get pcie-phy\n");
+
+	stm32_pcie->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(stm32_pcie->clk))
+		return dev_err_probe(dev, PTR_ERR(stm32_pcie->clk),
+				     "Failed to get PCIe clock source\n");
+
+	stm32_pcie->rst = devm_reset_control_get_exclusive(dev, NULL);
+	if (IS_ERR(stm32_pcie->rst))
+		return dev_err_probe(dev, PTR_ERR(stm32_pcie->rst),
+				     "Failed to get PCIe reset\n");
+
+	stm32_pcie->perst_gpio = devm_gpiod_get(dev, "reset", GPIOD_IN);
+	if (IS_ERR(stm32_pcie->perst_gpio))
+		return dev_err_probe(dev, PTR_ERR(stm32_pcie->perst_gpio),
+				     "Failed to get reset GPIO\n");
+
+	ret = phy_set_mode(stm32_pcie->phy, PHY_MODE_PCIE);
+	if (ret)
+		return ret;
+
+	platform_set_drvdata(pdev, stm32_pcie);
+
+	pm_runtime_get_noresume(dev);
+
+	ret = devm_pm_runtime_enable(dev);
+	if (ret < 0) {
+		pm_runtime_put_noidle(&pdev->dev);
+		return dev_err_probe(dev, ret, "Failed to enable runtime PM\n");
+	}
+
+	stm32_pcie->perst_irq = gpiod_to_irq(stm32_pcie->perst_gpio);
+
+	/* Will be enabled in start_link when device is initialized. */
+	irq_set_status_flags(stm32_pcie->perst_irq, IRQ_NOAUTOEN);
+
+	ret = devm_request_threaded_irq(dev, stm32_pcie->perst_irq, NULL,
+					stm32_pcie_ep_perst_irq_thread,
+					IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
+					"perst_irq", stm32_pcie);
+	if (ret) {
+		pm_runtime_put_noidle(&pdev->dev);
+		return dev_err_probe(dev, ret, "Failed to request PERST IRQ\n");
+	}
+
+	ret = stm32_add_pcie_ep(stm32_pcie, pdev);
+	if (ret)
+		pm_runtime_put_noidle(&pdev->dev);
+
+	return ret;
+}
+
+static void stm32_pcie_remove(struct platform_device *pdev)
+{
+	struct stm32_pcie *stm32_pcie = platform_get_drvdata(pdev);
+	struct dw_pcie *pci = &stm32_pcie->pci;
+	struct dw_pcie_ep *ep = &pci->ep;
+
+	dw_pcie_stop_link(pci);
+
+	pci_epc_deinit_notify(ep->epc);
+	dw_pcie_ep_deinit(ep);
+
+	stm32_pcie_disable_resources(stm32_pcie);
+
+	pm_runtime_put_sync(&pdev->dev);
+}
+
+static const struct of_device_id stm32_pcie_ep_of_match[] = {
+	{ .compatible = "st,stm32mp25-pcie-ep" },
+	{},
+};
+
+static struct platform_driver stm32_pcie_ep_driver = {
+	.probe = stm32_pcie_probe,
+	.remove = stm32_pcie_remove,
+	.driver = {
+		.name = "stm32-ep-pcie",
+		.of_match_table = stm32_pcie_ep_of_match,
+	},
+};
+
+module_platform_driver(stm32_pcie_ep_driver);
+
+MODULE_AUTHOR("Christian Bruel <christian.bruel@foss.st.com>");
+MODULE_DESCRIPTION("STM32MP25 PCIe Endpoint Controller driver");
+MODULE_LICENSE("GPL");
+MODULE_DEVICE_TABLE(of, stm32_pcie_ep_of_match);
diff --git a/drivers/pci/controller/dwc/pcie-stm32.h b/drivers/pci/controller/dwc/pcie-stm32.h
index 387112c4e42c..09d39f04e469 100644
--- a/drivers/pci/controller/dwc/pcie-stm32.h
+++ b/drivers/pci/controller/dwc/pcie-stm32.h
@@ -9,6 +9,7 @@
 #define to_stm32_pcie(x)	dev_get_drvdata((x)->dev)
 
 #define STM32MP25_PCIECR_TYPE_MASK	GENMASK(11, 8)
+#define STM32MP25_PCIECR_EP		0
 #define STM32MP25_PCIECR_LTSSM_EN	BIT(2)
 #define STM32MP25_PCIECR_RC		BIT(10)
 
-- 
2.34.1

[PATCH v12 5/9] MAINTAINERS: add entry for ST STM32MP25 PCIe drivers

[Christian Bruel]

Add myself as maintainer of STM32MP25 PCIe host and PCIe endpoint drivers

Signed-off-by: Christian Bruel <christian.bruel@foss.st.com>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index a7a147f31468..398260d28bbf 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19068,6 +19068,13 @@ L:	linux-samsung-soc@vger.kernel.org
 S:	Maintained
 F:	drivers/pci/controller/dwc/pci-exynos.c
 
+PCI DRIVER FOR STM32MP25
+M:	Christian Bruel <christian.bruel@foss.st.com>
+L:	linux-pci@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/pci/st,stm32-pcie-*.yaml
+F:	drivers/pci/controller/dwc/*stm32*
+
 PCI DRIVER FOR SYNOPSYS DESIGNWARE
 M:	Jingoo Han <jingoohan1@gmail.com>
 M:	Manivannan Sadhasivam <mani@kernel.org>
-- 
2.34.1

[PATCH v12 6/9] arm64: dts: st: add PCIe pinctrl entries in stm32mp25-pinctrl.dtsi

[Christian Bruel]

Add PCIe pinctrl entries in stm32mp25-pinctrl.dtsi
init: forces GPIO to low while probing so CLKREQ is low for
phy_init
default: restore the AFMUX after controller probe

Add Analog pins of PCIe to perform power cycle

Signed-off-by: Christian Bruel <christian.bruel@foss.st.com>
---
 arch/arm64/boot/dts/st/stm32mp25-pinctrl.dtsi | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm64/boot/dts/st/stm32mp25-pinctrl.dtsi b/arch/arm64/boot/dts/st/stm32mp25-pinctrl.dtsi
index aba90d555f4e..0480b9af00e8 100644
--- a/arch/arm64/boot/dts/st/stm32mp25-pinctrl.dtsi
+++ b/arch/arm64/boot/dts/st/stm32mp25-pinctrl.dtsi
@@ -133,6 +133,26 @@ pins {
 		};
 	};
 
+	pcie_pins_a: pcie-0 {
+		pins {
+			pinmux = <STM32_PINMUX('J', 0, AF4)>;
+			bias-disable;
+		};
+	};
+
+	pcie_init_pins_a: pcie-init-0 {
+		pins {
+			pinmux = <STM32_PINMUX('J', 0, GPIO)>;
+			output-low;
+		};
+	};
+
+	pcie_sleep_pins_a: pcie-sleep-0 {
+		pins {
+			pinmux = <STM32_PINMUX('J', 0, ANALOG)>;
+		};
+	};
+
 	sdmmc1_b4_pins_a: sdmmc1-b4-0 {
 		pins1 {
 			pinmux = <STM32_PINMUX('E', 4, AF10)>, /* SDMMC1_D0 */
-- 
2.34.1

[PATCH v12 7/9] arm64: dts: st: Add PCIe Root Complex mode on stm32mp251

[Christian Bruel]

Add pcie_rc node to support STM32 MP25 PCIe driver based on the
DesignWare PCIe core configured as Root Complex mode

Supports Gen1/Gen2, single lane, MSI interrupts using the ARM GICv2m

Signed-off-by: Christian Bruel <christian.bruel@foss.st.com>
---
 arch/arm64/boot/dts/st/stm32mp251.dtsi | 44 ++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/arch/arm64/boot/dts/st/stm32mp251.dtsi b/arch/arm64/boot/dts/st/stm32mp251.dtsi
index 8d87865850a7..781d0e43ab59 100644
--- a/arch/arm64/boot/dts/st/stm32mp251.dtsi
+++ b/arch/arm64/boot/dts/st/stm32mp251.dtsi
@@ -122,6 +122,15 @@ intc: interrupt-controller@4ac00000 {
 		      <0x0 0x4ac20000 0x0 0x20000>,
 		      <0x0 0x4ac40000 0x0 0x20000>,
 		      <0x0 0x4ac60000 0x0 0x20000>;
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		v2m0: v2m@48090000 {
+			compatible = "arm,gic-v2m-frame";
+			reg = <0x0 0x48090000 0x0 0x1000>;
+			msi-controller;
+		};
 	};
 
 	psci {
@@ -1130,6 +1139,41 @@ stmmac_axi_config_1: stmmac-axi-config {
 					snps,wr_osr_lmt = <0x7>;
 				};
 			};
+
+			pcie_rc: pcie@48400000 {
+				compatible = "st,stm32mp25-pcie-rc";
+				device_type = "pci";
+				reg = <0x48400000 0x400000>,
+				      <0x10000000 0x10000>;
+				reg-names = "dbi", "config";
+				#interrupt-cells = <1>;
+				interrupt-map-mask = <0 0 0 7>;
+				interrupt-map = <0 0 0 1 &intc 0 0 GIC_SPI 264 IRQ_TYPE_LEVEL_HIGH>,
+						<0 0 0 2 &intc 0 0 GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH>,
+						<0 0 0 3 &intc 0 0 GIC_SPI 266 IRQ_TYPE_LEVEL_HIGH>,
+						<0 0 0 4 &intc 0 0 GIC_SPI 267 IRQ_TYPE_LEVEL_HIGH>;
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges = <0x01000000 0x0 0x00000000 0x10010000 0x0 0x10000>,
+					 <0x02000000 0x0 0x10020000 0x10020000 0x0 0x7fe0000>,
+					 <0x42000000 0x0 0x18000000 0x18000000 0x0 0x8000000>;
+				dma-ranges = <0x42000000 0x0 0x80000000 0x80000000 0x0 0x80000000>;
+				clocks = <&rcc CK_BUS_PCIE>;
+				resets = <&rcc PCIE_R>;
+				msi-parent = <&v2m0>;
+				access-controllers = <&rifsc 68>;
+				power-domains = <&CLUSTER_PD>;
+				status = "disabled";
+
+				pcie@0,0 {
+					device_type = "pci";
+					reg = <0x0 0x0 0x0 0x0 0x0>;
+					phys = <&combophy PHY_TYPE_PCIE>;
+					#address-cells = <3>;
+					#size-cells = <2>;
+					ranges;
+				};
+			};
 		};
 
 		bsec: efuse@44000000 {
-- 
2.34.1

[PATCH v12 8/9] arm64: dts: st: Add PCIe Endpoint mode on stm32mp251

[Christian Bruel]

Add pcie_ep node to support STM32 MP25 PCIe driver based on the
DesignWare PCIe core configured as Endpoint mode

Signed-off-by: Christian Bruel <christian.bruel@foss.st.com>
---
 arch/arm64/boot/dts/st/stm32mp251.dtsi | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm64/boot/dts/st/stm32mp251.dtsi b/arch/arm64/boot/dts/st/stm32mp251.dtsi
index 781d0e43ab59..23dcc889c3e8 100644
--- a/arch/arm64/boot/dts/st/stm32mp251.dtsi
+++ b/arch/arm64/boot/dts/st/stm32mp251.dtsi
@@ -1140,6 +1140,21 @@ stmmac_axi_config_1: stmmac-axi-config {
 				};
 			};
 
+			pcie_ep: pcie-ep@48400000 {
+				compatible = "st,stm32mp25-pcie-ep";
+				reg = <0x48400000 0x100000>,
+				      <0x48500000 0x100000>,
+				      <0x48700000 0x80000>,
+				      <0x10000000 0x10000000>;
+				reg-names = "dbi", "dbi2", "atu", "addr_space";
+				clocks = <&rcc CK_BUS_PCIE>;
+				resets = <&rcc PCIE_R>;
+				phys = <&combophy PHY_TYPE_PCIE>;
+				access-controllers = <&rifsc 68>;
+				power-domains = <&CLUSTER_PD>;
+				status = "disabled";
+			};
+
 			pcie_rc: pcie@48400000 {
 				compatible = "st,stm32mp25-pcie-rc";
 				device_type = "pci";
-- 
2.34.1

[PATCH v12 9/9] arm64: dts: st: Enable PCIe on the stm32mp257f-ev1 board

[Christian Bruel]

Add PCIe RC and EP support on stm32mp257f-ev1 board.
Default to RC mode.

Signed-off-by: Christian Bruel <christian.bruel@foss.st.com>
---
 arch/arm64/boot/dts/st/stm32mp257f-ev1.dts | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm64/boot/dts/st/stm32mp257f-ev1.dts b/arch/arm64/boot/dts/st/stm32mp257f-ev1.dts
index 2f561ad40665..f97581aa0841 100644
--- a/arch/arm64/boot/dts/st/stm32mp257f-ev1.dts
+++ b/arch/arm64/boot/dts/st/stm32mp257f-ev1.dts
@@ -265,6 +265,27 @@ scmi_vdd_sdcard: regulator@23 {
 	};
 };
 
+&pcie_ep {
+	pinctrl-names = "default", "init";
+	pinctrl-0 = <&pcie_pins_a>;
+	pinctrl-1 = <&pcie_init_pins_a>;
+	reset-gpios = <&gpioj 8 GPIO_ACTIVE_LOW>;
+	status = "disabled";
+};
+
+&pcie_rc {
+	pinctrl-names = "default", "init", "sleep";
+	pinctrl-0 = <&pcie_pins_a>;
+	pinctrl-1 = <&pcie_init_pins_a>;
+	pinctrl-2 = <&pcie_sleep_pins_a>;
+	status = "okay";
+
+	pcie@0,0 {
+		 reset-gpios = <&gpioj 8 GPIO_ACTIVE_LOW>;
+		 wake-gpios = <&gpioh 5 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>;
+	};
+};
+
 &sdmmc1 {
 	pinctrl-names = "default", "opendrain", "sleep";
 	pinctrl-0 = <&sdmmc1_b4_pins_a>;
-- 
2.34.1

Re: (subset) [PATCH v12 0/9] Add STM32MP25 PCIe drivers

[Manivannan Sadhasivam]

On Tue, 10 Jun 2025 11:07:05 +0200, Christian Bruel wrote:
> Changes in v12;
>    Fix warning reported by kernel test robot <lkp@intel.com>
> 
> Changes in v11;
>    Address comments from Manivanna:
>    - RC driver: Do not call pm_runtime_get_noresume in probe
>                 More uses of dev_err_probe
>    - EP driver: Use level triggered PERST# irq
> 
> [...]

Applied, thanks!

[1/9] dt-bindings: PCI: Add STM32MP25 PCIe Root Complex bindings
      commit: 41d5cfbdda7a61c5d646a54035b697205cff1cf0
[2/9] PCI: stm32: Add PCIe host support for STM32MP25
      commit: f6111bc2d8fe6ffc741661126a2174523124dc11
[3/9] dt-bindings: PCI: Add STM32MP25 PCIe Endpoint bindings
      commit: 203cfc4a23506ffb9c48d1300348c290dbf9368e
[4/9] PCI: stm32: Add PCIe Endpoint support for STM32MP25
      commit: 8869fb36a107a9ff18dab8c224de6afff1e81dec
[5/9] MAINTAINERS: add entry for ST STM32MP25 PCIe drivers
      commit: 003902ed7778d62083120253cd282a9112674986

Best regards,
-- 
Manivannan Sadhasivam <mani@kernel.org>

Re: [PATCH v12 8/9] arm64: dts: st: Add PCIe Endpoint mode on stm32mp251

[Manivannan Sadhasivam]

On Tue, Jun 10, 2025 at 11:07:13AM +0200, Christian Bruel wrote:
> Add pcie_ep node to support STM32 MP25 PCIe driver based on the
> DesignWare PCIe core configured as Endpoint mode
> 
> Signed-off-by: Christian Bruel <christian.bruel@foss.st.com>

Acked-by: Manivannan Sadhasivam <mani@kernel.org>

- Mani

> ---
>  arch/arm64/boot/dts/st/stm32mp251.dtsi | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/st/stm32mp251.dtsi b/arch/arm64/boot/dts/st/stm32mp251.dtsi
> index 781d0e43ab59..23dcc889c3e8 100644
> --- a/arch/arm64/boot/dts/st/stm32mp251.dtsi
> +++ b/arch/arm64/boot/dts/st/stm32mp251.dtsi
> @@ -1140,6 +1140,21 @@ stmmac_axi_config_1: stmmac-axi-config {
>  				};
>  			};
>  
> +			pcie_ep: pcie-ep@48400000 {
> +				compatible = "st,stm32mp25-pcie-ep";
> +				reg = <0x48400000 0x100000>,
> +				      <0x48500000 0x100000>,
> +				      <0x48700000 0x80000>,
> +				      <0x10000000 0x10000000>;
> +				reg-names = "dbi", "dbi2", "atu", "addr_space";
> +				clocks = <&rcc CK_BUS_PCIE>;
> +				resets = <&rcc PCIE_R>;
> +				phys = <&combophy PHY_TYPE_PCIE>;
> +				access-controllers = <&rifsc 68>;
> +				power-domains = <&CLUSTER_PD>;
> +				status = "disabled";
> +			};
> +
>  			pcie_rc: pcie@48400000 {
>  				compatible = "st,stm32mp25-pcie-rc";
>  				device_type = "pci";
> -- 
> 2.34.1
> 

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

Re: [PATCH v12 7/9] arm64: dts: st: Add PCIe Root Complex mode on stm32mp251

[Manivannan Sadhasivam]

On Tue, Jun 10, 2025 at 11:07:12AM +0200, Christian Bruel wrote:
> Add pcie_rc node to support STM32 MP25 PCIe driver based on the
> DesignWare PCIe core configured as Root Complex mode
> 
> Supports Gen1/Gen2, single lane, MSI interrupts using the ARM GICv2m
> 
> Signed-off-by: Christian Bruel <christian.bruel@foss.st.com>

Acked-by: Manivannan Sadhasivam <mani@kernel.org>

- Mani

> ---
>  arch/arm64/boot/dts/st/stm32mp251.dtsi | 44 ++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/st/stm32mp251.dtsi b/arch/arm64/boot/dts/st/stm32mp251.dtsi
> index 8d87865850a7..781d0e43ab59 100644
> --- a/arch/arm64/boot/dts/st/stm32mp251.dtsi
> +++ b/arch/arm64/boot/dts/st/stm32mp251.dtsi
> @@ -122,6 +122,15 @@ intc: interrupt-controller@4ac00000 {
>  		      <0x0 0x4ac20000 0x0 0x20000>,
>  		      <0x0 0x4ac40000 0x0 0x20000>,
>  		      <0x0 0x4ac60000 0x0 0x20000>;
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		v2m0: v2m@48090000 {
> +			compatible = "arm,gic-v2m-frame";
> +			reg = <0x0 0x48090000 0x0 0x1000>;
> +			msi-controller;
> +		};
>  	};
>  
>  	psci {
> @@ -1130,6 +1139,41 @@ stmmac_axi_config_1: stmmac-axi-config {
>  					snps,wr_osr_lmt = <0x7>;
>  				};
>  			};
> +
> +			pcie_rc: pcie@48400000 {
> +				compatible = "st,stm32mp25-pcie-rc";
> +				device_type = "pci";
> +				reg = <0x48400000 0x400000>,
> +				      <0x10000000 0x10000>;
> +				reg-names = "dbi", "config";
> +				#interrupt-cells = <1>;
> +				interrupt-map-mask = <0 0 0 7>;
> +				interrupt-map = <0 0 0 1 &intc 0 0 GIC_SPI 264 IRQ_TYPE_LEVEL_HIGH>,
> +						<0 0 0 2 &intc 0 0 GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH>,
> +						<0 0 0 3 &intc 0 0 GIC_SPI 266 IRQ_TYPE_LEVEL_HIGH>,
> +						<0 0 0 4 &intc 0 0 GIC_SPI 267 IRQ_TYPE_LEVEL_HIGH>;
> +				#address-cells = <3>;
> +				#size-cells = <2>;
> +				ranges = <0x01000000 0x0 0x00000000 0x10010000 0x0 0x10000>,
> +					 <0x02000000 0x0 0x10020000 0x10020000 0x0 0x7fe0000>,
> +					 <0x42000000 0x0 0x18000000 0x18000000 0x0 0x8000000>;
> +				dma-ranges = <0x42000000 0x0 0x80000000 0x80000000 0x0 0x80000000>;
> +				clocks = <&rcc CK_BUS_PCIE>;
> +				resets = <&rcc PCIE_R>;
> +				msi-parent = <&v2m0>;
> +				access-controllers = <&rifsc 68>;
> +				power-domains = <&CLUSTER_PD>;
> +				status = "disabled";
> +
> +				pcie@0,0 {
> +					device_type = "pci";
> +					reg = <0x0 0x0 0x0 0x0 0x0>;
> +					phys = <&combophy PHY_TYPE_PCIE>;
> +					#address-cells = <3>;
> +					#size-cells = <2>;
> +					ranges;
> +				};
> +			};
>  		};
>  
>  		bsec: efuse@44000000 {
> -- 
> 2.34.1
> 

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

Re: (subset) [PATCH v12 0/9] Add STM32MP25 PCIe drivers

[Manivannan Sadhasivam]

On Tue, Jun 24, 2025 at 05:22:06PM -0500, Bjorn Helgaas wrote:
> On Mon, Jun 23, 2025 at 06:13:07AM -0600, Manivannan Sadhasivam wrote:
> > On Tue, 10 Jun 2025 11:07:05 +0200, Christian Bruel wrote:
> > > Changes in v12;
> > >    Fix warning reported by kernel test robot <lkp@intel.com>
> > > 
> > > Changes in v11;
> > >    Address comments from Manivanna:
> > >    - RC driver: Do not call pm_runtime_get_noresume in probe
> > >                 More uses of dev_err_probe
> > >    - EP driver: Use level triggered PERST# irq
> > > 
> > > [...]
> > 
> > Applied, thanks!
> > 
> > [1/9] dt-bindings: PCI: Add STM32MP25 PCIe Root Complex bindings
> >       commit: 41d5cfbdda7a61c5d646a54035b697205cff1cf0
> > [2/9] PCI: stm32: Add PCIe host support for STM32MP25
> >       commit: f6111bc2d8fe6ffc741661126a2174523124dc11
> > [3/9] dt-bindings: PCI: Add STM32MP25 PCIe Endpoint bindings
> >       commit: 203cfc4a23506ffb9c48d1300348c290dbf9368e
> > [4/9] PCI: stm32: Add PCIe Endpoint support for STM32MP25
> >       commit: 8869fb36a107a9ff18dab8c224de6afff1e81dec
> > [5/9] MAINTAINERS: add entry for ST STM32MP25 PCIe drivers
> >       commit: 003902ed7778d62083120253cd282a9112674986
> 
> This doesn't build for me with the attached config:
> 
>   $ make drivers/pci/controller/dwc/pcie-stm32.o
>     CALL    scripts/checksyscalls.sh
>     DESCEND objtool
>     INSTALL libsubcmd_headers
>     CC      drivers/pci/controller/dwc/pcie-stm32.o
>   drivers/pci/controller/dwc/pcie-stm32.c: In function ‘stm32_pcie_suspend_noirq’:
>   drivers/pci/controller/dwc/pcie-stm32.c:83:16: error: implicit declaration of function ‘pinctrl_pm_select_sleep_state’ [-Werror=implicit-function-declaration]
>      83 |         return pinctrl_pm_select_sleep_state(dev);
> 	|                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   drivers/pci/controller/dwc/pcie-stm32.c: In function ‘stm32_pcie_resume_noirq’:
>   drivers/pci/controller/dwc/pcie-stm32.c:96:24: error: ‘struct device’ has no member named ‘pins’
>      96 |         if (!IS_ERR(dev->pins->init_state))
> 	|                        ^~
>   drivers/pci/controller/dwc/pcie-stm32.c:97:23: error: implicit declaration of function ‘pinctrl_select_state’ [-Werror=implicit-function-declaration]
>      97 |                 ret = pinctrl_select_state(dev->pins->p, dev->pins->init_state);
> 	|                       ^~~~~~~~~~~~~~~~~~~~
>   drivers/pci/controller/dwc/pcie-stm32.c:97:47: error: ‘struct device’ has no member named ‘pins’
>      97 |                 ret = pinctrl_select_state(dev->pins->p, dev->pins->init_state);
> 	|                                               ^~
>   drivers/pci/controller/dwc/pcie-stm32.c:97:61: error: ‘struct device’ has no member named ‘pins’
>      97 |                 ret = pinctrl_select_state(dev->pins->p, dev->pins->init_state);
> 	|                                                             ^~
>   drivers/pci/controller/dwc/pcie-stm32.c:99:23: error: implicit declaration of function ‘pinctrl_pm_select_default_state’ [-Werror=implicit-function-declaration]
>      99 |                 ret = pinctrl_pm_select_default_state(dev);
> 	|                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 

Hmm... I see two issues here. First is, wrong pinctrl header used. The correct
one is:

#include <linux/pinctrl/consumer.h>

Second issue is the driver accessing "struct device::pins" directly. The "pins"
member won't be available if CONFIG_PINCTRL is not set (which is what your
.config has). So either the member should not be accessed directly or the
driver has to depend on CONFIG_PINCTRL. The latter one is not acceptable. It
also looks weird that only this driver is accessing the "pins" member directly
apart from the pinctrl core. So I think this part needs a revisit.

Christian?

- Mani

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

Re: (subset) [PATCH v12 0/9] Add STM32MP25 PCIe drivers

[Christian Bruel]

On 6/25/25 06:00, Manivannan Sadhasivam wrote:
> On Tue, Jun 24, 2025 at 05:22:06PM -0500, Bjorn Helgaas wrote:
>> On Mon, Jun 23, 2025 at 06:13:07AM -0600, Manivannan Sadhasivam wrote:
>>> On Tue, 10 Jun 2025 11:07:05 +0200, Christian Bruel wrote:
>>>> Changes in v12;
>>>>     Fix warning reported by kernel test robot <lkp@intel.com>
>>>>
>>>> Changes in v11;
>>>>     Address comments from Manivanna:
>>>>     - RC driver: Do not call pm_runtime_get_noresume in probe
>>>>                  More uses of dev_err_probe
>>>>     - EP driver: Use level triggered PERST# irq
>>>>
>>>> [...]
>>>
>>> Applied, thanks!
>>>
>>> [1/9] dt-bindings: PCI: Add STM32MP25 PCIe Root Complex bindings
>>>        commit: 41d5cfbdda7a61c5d646a54035b697205cff1cf0
>>> [2/9] PCI: stm32: Add PCIe host support for STM32MP25
>>>        commit: f6111bc2d8fe6ffc741661126a2174523124dc11
>>> [3/9] dt-bindings: PCI: Add STM32MP25 PCIe Endpoint bindings
>>>        commit: 203cfc4a23506ffb9c48d1300348c290dbf9368e
>>> [4/9] PCI: stm32: Add PCIe Endpoint support for STM32MP25
>>>        commit: 8869fb36a107a9ff18dab8c224de6afff1e81dec
>>> [5/9] MAINTAINERS: add entry for ST STM32MP25 PCIe drivers
>>>        commit: 003902ed7778d62083120253cd282a9112674986
>>
>> This doesn't build for me with the attached config:
>>
>>    $ make drivers/pci/controller/dwc/pcie-stm32.o
>>      CALL    scripts/checksyscalls.sh
>>      DESCEND objtool
>>      INSTALL libsubcmd_headers
>>      CC      drivers/pci/controller/dwc/pcie-stm32.o
>>    drivers/pci/controller/dwc/pcie-stm32.c: In function ‘stm32_pcie_suspend_noirq’:
>>    drivers/pci/controller/dwc/pcie-stm32.c:83:16: error: implicit declaration of function ‘pinctrl_pm_select_sleep_state’ [-Werror=implicit-function-declaration]
>>       83 |         return pinctrl_pm_select_sleep_state(dev);
>> 	|                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>    drivers/pci/controller/dwc/pcie-stm32.c: In function ‘stm32_pcie_resume_noirq’:
>>    drivers/pci/controller/dwc/pcie-stm32.c:96:24: error: ‘structdevice’ has no member named ‘pins’
>>       96 |         if (!IS_ERR(dev->pins->init_state))
>> 	|                        ^~
>>    drivers/pci/controller/dwc/pcie-stm32.c:97:23: error: implicit declaration of function ‘pinctrl_select_state’ [-Werror=implicit-function-declaration]
>>       97 |                 ret = pinctrl_select_state(dev->pins->p, dev->pins->init_state);
>> 	|                       ^~~~~~~~~~~~~~~~~~~~
>>    drivers/pci/controller/dwc/pcie-stm32.c:97:47: error: ‘structdevice’ has no member named ‘pins’
>>       97 |                 ret = pinctrl_select_state(dev->pins->p, dev->pins->init_state);
>> 	|                                               ^~
>>    drivers/pci/controller/dwc/pcie-stm32.c:97:61: error: ‘structdevice’ has no member named ‘pins’
>>       97 |                 ret = pinctrl_select_state(dev->pins->p, dev->pins->init_state);
>> 	|                                                             ^~
>>    drivers/pci/controller/dwc/pcie-stm32.c:99:23: error: implicit declaration of function ‘pinctrl_pm_select_default_state’ [-Werror=implicit-function-declaration]
>>       99 |                 ret = pinctrl_pm_select_default_state(dev);
>> 	|                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
> 
> Hmm... I see two issues here. First is, wrong pinctrl header used. The correct
> one is:
> 
> #include <linux/pinctrl/consumer.h>

ah yes, the missing pinctrl_pm_select_default_state() should indeed be 
fixed by using the correct header.

> 
> Second issue is the driver accessing "struct device::pins" directly. The "pins"
> member won't be available if CONFIG_PINCTRL is not set (which is what your
> .config has). So either the member should not be accessed directly or the
> driver has to depend on CONFIG_PINCTRL. The latter one is not acceptable.It
> also looks weird that only this driver is accessing the "pins" member directly
> apart from the pinctrl core. So I think this part needs a revisit.
> 
> Christian?
The pinctrl "init" and "default" configurations are managed effectively 
by the probing code. The same approach is required in 
stm32_pcie_resume_noirq().

In this case, would introducing a new helper function, 
pinctrl_pm_select_init_state(), be preferable, even if we are the only 
consumer?

Thank you


> 
> - Mani
> 

Re: (subset) [PATCH v12 0/9] Add STM32MP25 PCIe drivers

[Manivannan Sadhasivam]

On Wed, Jun 25, 2025 at 12:18:16PM +0200, Christian Bruel wrote:
> 
> 
> On 6/25/25 06:00, Manivannan Sadhasivam wrote:
> > On Tue, Jun 24, 2025 at 05:22:06PM -0500, Bjorn Helgaas wrote:
> > > On Mon, Jun 23, 2025 at 06:13:07AM -0600, Manivannan Sadhasivam wrote:
> > > > On Tue, 10 Jun 2025 11:07:05 +0200, Christian Bruel wrote:
> > > > > Changes in v12;
> > > > >     Fix warning reported by kernel test robot <lkp@intel.com>
> > > > > 
> > > > > Changes in v11;
> > > > >     Address comments from Manivanna:
> > > > >     - RC driver: Do not call pm_runtime_get_noresume in probe
> > > > >                  More uses of dev_err_probe
> > > > >     - EP driver: Use level triggered PERST# irq
> > > > > 
> > > > > [...]
> > > > 
> > > > Applied, thanks!
> > > > 
> > > > [1/9] dt-bindings: PCI: Add STM32MP25 PCIe Root Complex bindings
> > > >        commit: 41d5cfbdda7a61c5d646a54035b697205cff1cf0
> > > > [2/9] PCI: stm32: Add PCIe host support for STM32MP25
> > > >        commit: f6111bc2d8fe6ffc741661126a2174523124dc11
> > > > [3/9] dt-bindings: PCI: Add STM32MP25 PCIe Endpoint bindings
> > > >        commit: 203cfc4a23506ffb9c48d1300348c290dbf9368e
> > > > [4/9] PCI: stm32: Add PCIe Endpoint support for STM32MP25
> > > >        commit: 8869fb36a107a9ff18dab8c224de6afff1e81dec
> > > > [5/9] MAINTAINERS: add entry for ST STM32MP25 PCIe drivers
> > > >        commit: 003902ed7778d62083120253cd282a9112674986
> > > 
> > > This doesn't build for me with the attached config:
> > > 
> > >    $ make drivers/pci/controller/dwc/pcie-stm32.o
> > >      CALL    scripts/checksyscalls.sh
> > >      DESCEND objtool
> > >      INSTALL libsubcmd_headers
> > >      CC      drivers/pci/controller/dwc/pcie-stm32.o
> > >    drivers/pci/controller/dwc/pcie-stm32.c: In function ‘stm32_pcie_suspend_noirq’:
> > >    drivers/pci/controller/dwc/pcie-stm32.c:83:16: error: implicit declaration of function ‘pinctrl_pm_select_sleep_state’ [-Werror=implicit-function-declaration]
> > >       83 |         return pinctrl_pm_select_sleep_state(dev);
> > > 	|                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > >    drivers/pci/controller/dwc/pcie-stm32.c: In function ‘stm32_pcie_resume_noirq’:
> > >    drivers/pci/controller/dwc/pcie-stm32.c:96:24: error: ‘structdevice’ has no member named ‘pins’
> > >       96 |         if (!IS_ERR(dev->pins->init_state))
> > > 	|                        ^~
> > >    drivers/pci/controller/dwc/pcie-stm32.c:97:23: error: implicit declaration of function ‘pinctrl_select_state’ [-Werror=implicit-function-declaration]
> > >       97 |                 ret = pinctrl_select_state(dev->pins->p, dev->pins->init_state);
> > > 	|                       ^~~~~~~~~~~~~~~~~~~~
> > >    drivers/pci/controller/dwc/pcie-stm32.c:97:47: error: ‘structdevice’ has no member named ‘pins’
> > >       97 |                 ret = pinctrl_select_state(dev->pins->p, dev->pins->init_state);
> > > 	|                                               ^~
> > >    drivers/pci/controller/dwc/pcie-stm32.c:97:61: error: ‘structdevice’ has no member named ‘pins’
> > >       97 |                 ret = pinctrl_select_state(dev->pins->p, dev->pins->init_state);
> > > 	|                                                             ^~
> > >    drivers/pci/controller/dwc/pcie-stm32.c:99:23: error: implicit declaration of function ‘pinctrl_pm_select_default_state’ [-Werror=implicit-function-declaration]
> > >       99 |                 ret = pinctrl_pm_select_default_state(dev);
> > > 	|                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > 
> > 
> > Hmm... I see two issues here. First is, wrong pinctrl header used. The correct
> > one is:
> > 
> > #include <linux/pinctrl/consumer.h>
> 
> ah yes, the missing pinctrl_pm_select_default_state() should indeed be fixed
> by using the correct header.
> 

I've fixed it in the branch.

> > 
> > Second issue is the driver accessing "struct device::pins" directly. The "pins"
> > member won't be available if CONFIG_PINCTRL is not set (which is what your
> > .config has). So either the member should not be accessed directly or the
> > driver has to depend on CONFIG_PINCTRL. The latter one is not acceptable.It
> > also looks weird that only this driver is accessing the "pins" member directly
> > apart from the pinctrl core. So I think this part needs a revisit.
> > 
> > Christian?
> The pinctrl "init" and "default" configurations are managed effectively by
> the probing code. The same approach is required in
> stm32_pcie_resume_noirq().
> 
> In this case, would introducing a new helper function,
> pinctrl_pm_select_init_state(), be preferable, even if we are the only
> consumer?
> 

Sounds reasonable. If you end up creating an API, get it acked by the pinctrl
maintainer so that I can merge it (and the use of it in this driver) through
the PCI tree to avoid cross tree dependency.

- Mani

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

Re: [PATCH v12 2/9] PCI: stm32: Add PCIe host support for STM32MP25

[Bjorn Helgaas]

[+to Linus for pinctrl usage question below]

On Tue, Jun 10, 2025 at 11:07:07AM +0200, Christian Bruel wrote:
> Add driver for the STM32MP25 SoC PCIe Gen1 2.5 GT/s and Gen2 5GT/s
> controller based on the DesignWare PCIe core.
> 
> Supports MSI via GICv2m, Single Virtual Channel, Single Function
> 
> Supports WAKE# GPIO.
> 
> Signed-off-by: Christian Bruel <christian.bruel@foss.st.com>
> ---
>  drivers/pci/controller/dwc/Kconfig      |  12 +
>  drivers/pci/controller/dwc/Makefile     |   1 +
>  drivers/pci/controller/dwc/pcie-stm32.c | 368 ++++++++++++++++++++++++
>  drivers/pci/controller/dwc/pcie-stm32.h |  15 +
>  4 files changed, 396 insertions(+)
>  create mode 100644 drivers/pci/controller/dwc/pcie-stm32.c
>  create mode 100644 drivers/pci/controller/dwc/pcie-stm32.h
> 
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index d9f0386396ed..387151f25f5f 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -410,6 +410,18 @@ config PCIE_SPEAR13XX
>  	help
>  	  Say Y here if you want PCIe support on SPEAr13XX SoCs.
>  
> +config PCIE_STM32_HOST
> +	tristate "STMicroelectronics STM32MP25 PCIe Controller (host mode)"
> +	depends on ARCH_STM32 || COMPILE_TEST
> +	depends on PCI_MSI
> +	select PCIE_DW_HOST
> +	help
> +	  Enables Root Complex (RC) support for the DesignWare core based PCIe
> +	  controller found in STM32MP25 SoC.
> +
> +	  This driver can also be built as a module. If so, the module
> +	  will be called pcie-stm32.
> +
>  config PCI_DRA7XX
>  	tristate
>  
> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> index 908cb7f345db..9d3b43504725 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -30,6 +30,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_STM32_HOST) += pcie-stm32.o
>  
>  # The following drivers are for devices that use the generic ACPI
>  # pci_root.c driver but don't support standard ECAM config access.
> diff --git a/drivers/pci/controller/dwc/pcie-stm32.c b/drivers/pci/controller/dwc/pcie-stm32.c
> new file mode 100644
> index 000000000000..6bf1b63f88c6
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-stm32.c
> @@ -0,0 +1,368 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * STMicroelectronics STM32MP25 PCIe root complex driver.
> + *
> + * Copyright (C) 2025 STMicroelectronics
> + * Author: Christian Bruel <christian.bruel@foss.st.com>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/of_platform.h>
> +#include <linux/phy/phy.h>
> +#include <linux/pinctrl/devinfo.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/pm_wakeirq.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +#include "pcie-designware.h"
> +#include "pcie-stm32.h"
> +#include "../../pci.h"
> +
> +struct stm32_pcie {
> +	struct dw_pcie pci;
> +	struct regmap *regmap;
> +	struct reset_control *rst;
> +	struct phy *phy;
> +	struct clk *clk;
> +	struct gpio_desc *perst_gpio;
> +	struct gpio_desc *wake_gpio;
> +};
> +
> +static void stm32_pcie_deassert_perst(struct stm32_pcie *stm32_pcie)
> +{
> +	/* Delay PERST# de-assertion until the power stabilizes */
> +	msleep(PCIE_T_PVPERL_MS);
> +
> +	gpiod_set_value(stm32_pcie->perst_gpio, 0);
> +
> +	/* Wait for the REFCLK to stabilize */
> +	if (stm32_pcie->perst_gpio)
> +		msleep(PCIE_T_RRS_READY_MS);
> +}
> +
> +static void stm32_pcie_assert_perst(struct stm32_pcie *stm32_pcie)
> +{
> +	gpiod_set_value(stm32_pcie->perst_gpio, 1);
> +}
> +
> +static int stm32_pcie_start_link(struct dw_pcie *pci)
> +{
> +	struct stm32_pcie *stm32_pcie = to_stm32_pcie(pci);
> +
> +	return regmap_update_bits(stm32_pcie->regmap, SYSCFG_PCIECR,
> +				  STM32MP25_PCIECR_LTSSM_EN,
> +				  STM32MP25_PCIECR_LTSSM_EN);
> +}
> +
> +static void stm32_pcie_stop_link(struct dw_pcie *pci)
> +{
> +	struct stm32_pcie *stm32_pcie = to_stm32_pcie(pci);
> +
> +	regmap_update_bits(stm32_pcie->regmap, SYSCFG_PCIECR,
> +			   STM32MP25_PCIECR_LTSSM_EN, 0);
> +}
> +
> +static int stm32_pcie_suspend_noirq(struct device *dev)
> +{
> +	struct stm32_pcie *stm32_pcie = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = dw_pcie_suspend_noirq(&stm32_pcie->pci);
> +	if (ret)
> +		return ret;
> +
> +	stm32_pcie_assert_perst(stm32_pcie);
> +
> +	clk_disable_unprepare(stm32_pcie->clk);
> +
> +	if (!device_wakeup_path(dev))
> +		phy_exit(stm32_pcie->phy);
> +
> +	return pinctrl_pm_select_sleep_state(dev);

Isn't there some setup required before we can use
pinctrl_select_state(), pinctrl_pm_select_sleep_state(),
pinctrl_pm_select_default_state(), etc?

I expected something like devm_pinctrl_get() in the .probe() path, but
I don't see anything.  I don't know how pinctrl works, but I don't see
how dev->pins gets set up.

> +}
> +
> +static int stm32_pcie_resume_noirq(struct device *dev)
> +{
> +	struct stm32_pcie *stm32_pcie = dev_get_drvdata(dev);
> +	int ret;
> +
> +	/*
> +	 * The core clock is gated with CLKREQ# from the COMBOPHY REFCLK,
> +	 * thus if no device is present, must force it low with an init pinmux
> +	 * to be able to access the DBI registers.
> +	 */
> +	if (!IS_ERR(dev->pins->init_state))
> +		ret = pinctrl_select_state(dev->pins->p, dev->pins->init_state);
> +	else
> +		ret = pinctrl_pm_select_default_state(dev);
> +
> +	if (ret) {
> +		dev_err(dev, "Failed to activate pinctrl pm state: %d\n", ret);
> +		return ret;
> +	}
> +
> +	if (!device_wakeup_path(dev)) {
> +		ret = phy_init(stm32_pcie->phy);
> +		if (ret) {
> +			pinctrl_pm_select_default_state(dev);
> +			return ret;
> +		}
> +	}
> +
> +	ret = clk_prepare_enable(stm32_pcie->clk);
> +	if (ret)
> +		goto err_phy_exit;
> +
> +	stm32_pcie_deassert_perst(stm32_pcie);
> +
> +	ret = dw_pcie_resume_noirq(&stm32_pcie->pci);
> +	if (ret)
> +		goto err_disable_clk;
> +
> +	pinctrl_pm_select_default_state(dev);
> +
> +	return 0;
> +
> +err_disable_clk:
> +	stm32_pcie_assert_perst(stm32_pcie);
> +	clk_disable_unprepare(stm32_pcie->clk);
> +
> +err_phy_exit:
> +	phy_exit(stm32_pcie->phy);
> +	pinctrl_pm_select_default_state(dev);
> +
> +	return ret;
> +}
> +
> +static const struct dev_pm_ops stm32_pcie_pm_ops = {
> +	NOIRQ_SYSTEM_SLEEP_PM_OPS(stm32_pcie_suspend_noirq,
> +				  stm32_pcie_resume_noirq)
> +};
> +
> +static const struct dw_pcie_host_ops stm32_pcie_host_ops = {
> +};
> +
> +static const struct dw_pcie_ops dw_pcie_ops = {
> +	.start_link = stm32_pcie_start_link,
> +	.stop_link = stm32_pcie_stop_link
> +};
> +
> +static int stm32_add_pcie_port(struct stm32_pcie *stm32_pcie)
> +{
> +	struct device *dev = stm32_pcie->pci.dev;
> +	unsigned int wake_irq;
> +	int ret;
> +
> +	/* Start to enable resources with PERST# asserted */
> +
> +	ret = phy_set_mode(stm32_pcie->phy, PHY_MODE_PCIE);
> +	if (ret)
> +		return ret;
> +
> +	ret = phy_init(stm32_pcie->phy);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_update_bits(stm32_pcie->regmap, SYSCFG_PCIECR,
> +				 STM32MP25_PCIECR_TYPE_MASK,
> +				 STM32MP25_PCIECR_RC);
> +	if (ret)
> +		goto err_phy_exit;
> +
> +	stm32_pcie_deassert_perst(stm32_pcie);
> +
> +	if (stm32_pcie->wake_gpio) {
> +		wake_irq = gpiod_to_irq(stm32_pcie->wake_gpio);
> +		ret = dev_pm_set_dedicated_wake_irq(dev, wake_irq);
> +		if (ret) {
> +			dev_err(dev, "Failed to enable wakeup irq %d\n", ret);
> +			goto err_assert_perst;
> +		}
> +		irq_set_irq_type(wake_irq, IRQ_TYPE_EDGE_FALLING);
> +	}
> +
> +	return 0;
> +
> +err_assert_perst:
> +	stm32_pcie_assert_perst(stm32_pcie);
> +
> +err_phy_exit:
> +	phy_exit(stm32_pcie->phy);
> +
> +	return ret;
> +}
> +
> +static void stm32_remove_pcie_port(struct stm32_pcie *stm32_pcie)
> +{
> +	dev_pm_clear_wake_irq(stm32_pcie->pci.dev);
> +
> +	stm32_pcie_assert_perst(stm32_pcie);
> +
> +	phy_exit(stm32_pcie->phy);
> +}
> +
> +static int stm32_pcie_parse_port(struct stm32_pcie *stm32_pcie)
> +{
> +	struct device *dev = stm32_pcie->pci.dev;
> +	struct device_node *root_port;
> +
> +	root_port = of_get_next_available_child(dev->of_node, NULL);
> +
> +	stm32_pcie->phy = devm_of_phy_get(dev, root_port, NULL);
> +	if (IS_ERR(stm32_pcie->phy)) {
> +		of_node_put(root_port);
> +		return dev_err_probe(dev, PTR_ERR(stm32_pcie->phy),
> +				     "Failed to get pcie-phy\n");
> +	}
> +
> +	stm32_pcie->perst_gpio = devm_fwnode_gpiod_get(dev, of_fwnode_handle(root_port),
> +						       "reset", GPIOD_OUT_HIGH, NULL);
> +	if (IS_ERR(stm32_pcie->perst_gpio)) {
> +		if (PTR_ERR(stm32_pcie->perst_gpio) != -ENOENT) {
> +			of_node_put(root_port);
> +			return dev_err_probe(dev, PTR_ERR(stm32_pcie->perst_gpio),
> +					     "Failed to get reset GPIO\n");
> +		}
> +		stm32_pcie->perst_gpio = NULL;
> +	}
> +
> +	stm32_pcie->wake_gpio = devm_fwnode_gpiod_get(dev, of_fwnode_handle(root_port),
> +						      "wake", GPIOD_IN, NULL);
> +
> +	if (IS_ERR(stm32_pcie->wake_gpio)) {
> +		if (PTR_ERR(stm32_pcie->wake_gpio) != -ENOENT) {
> +			of_node_put(root_port);
> +			return dev_err_probe(dev, PTR_ERR(stm32_pcie->wake_gpio),
> +					     "Failed to get wake GPIO\n");
> +		}
> +		stm32_pcie->wake_gpio = NULL;
> +	}
> +
> +	of_node_put(root_port);
> +
> +	return 0;
> +}
> +
> +static int stm32_pcie_probe(struct platform_device *pdev)
> +{
> +	struct stm32_pcie *stm32_pcie;
> +	struct device *dev = &pdev->dev;
> +	int ret;
> +
> +	stm32_pcie = devm_kzalloc(dev, sizeof(*stm32_pcie), GFP_KERNEL);
> +	if (!stm32_pcie)
> +		return -ENOMEM;
> +
> +	stm32_pcie->pci.dev = dev;
> +	stm32_pcie->pci.ops = &dw_pcie_ops;
> +	stm32_pcie->pci.pp.ops = &stm32_pcie_host_ops;
> +
> +	stm32_pcie->regmap = syscon_regmap_lookup_by_compatible("st,stm32mp25-syscfg");
> +	if (IS_ERR(stm32_pcie->regmap))
> +		return dev_err_probe(dev, PTR_ERR(stm32_pcie->regmap),
> +				     "No syscfg specified\n");
> +
> +	stm32_pcie->clk = devm_clk_get(dev, NULL);
> +	if (IS_ERR(stm32_pcie->clk))
> +		return dev_err_probe(dev, PTR_ERR(stm32_pcie->clk),
> +				     "Failed to get PCIe clock source\n");
> +
> +	stm32_pcie->rst = devm_reset_control_get_exclusive(dev, NULL);
> +	if (IS_ERR(stm32_pcie->rst))
> +		return dev_err_probe(dev, PTR_ERR(stm32_pcie->rst),
> +				     "Failed to get PCIe reset\n");
> +
> +	ret = stm32_pcie_parse_port(stm32_pcie);
> +	if (ret)
> +		return ret;
> +
> +	platform_set_drvdata(pdev, stm32_pcie);
> +
> +	ret = stm32_add_pcie_port(stm32_pcie);
> +	if (ret)
> +		return ret;
> +
> +	reset_control_assert(stm32_pcie->rst);
> +	reset_control_deassert(stm32_pcie->rst);
> +
> +	ret = clk_prepare_enable(stm32_pcie->clk);
> +	if (ret) {
> +		dev_err(dev, "Core clock enable failed %d\n", ret);
> +		goto err_remove_port;
> +	}
> +
> +	ret = pm_runtime_set_active(dev);
> +	if (ret < 0) {
> +		clk_disable_unprepare(stm32_pcie->clk);
> +		stm32_remove_pcie_port(stm32_pcie);
> +		return dev_err_probe(dev, ret, "Failed to activate runtime PM\n");
> +	}
> +
> +	pm_runtime_no_callbacks(dev);
> +
> +	ret = devm_pm_runtime_enable(dev);
> +	if (ret < 0) {
> +		clk_disable_unprepare(stm32_pcie->clk);
> +		stm32_remove_pcie_port(stm32_pcie);
> +		return dev_err_probe(dev, ret, "Failed to enable runtime PM\n");
> +	}
> +
> +	ret = dw_pcie_host_init(&stm32_pcie->pci.pp);
> +	if (ret)
> +		goto err_disable_clk;
> +
> +	if (stm32_pcie->wake_gpio)
> +		device_init_wakeup(dev, true);
> +
> +	return 0;
> +
> +err_disable_clk:
> +	clk_disable_unprepare(stm32_pcie->clk);
> +
> +err_remove_port:
> +	stm32_remove_pcie_port(stm32_pcie);
> +
> +	return ret;
> +}
> +
> +static void stm32_pcie_remove(struct platform_device *pdev)
> +{
> +	struct stm32_pcie *stm32_pcie = platform_get_drvdata(pdev);
> +	struct dw_pcie_rp *pp = &stm32_pcie->pci.pp;
> +
> +	if (stm32_pcie->wake_gpio)
> +		device_init_wakeup(&pdev->dev, false);
> +
> +	dw_pcie_host_deinit(pp);
> +
> +	clk_disable_unprepare(stm32_pcie->clk);
> +
> +	stm32_remove_pcie_port(stm32_pcie);
> +
> +	pm_runtime_put_noidle(&pdev->dev);
> +}
> +
> +static const struct of_device_id stm32_pcie_of_match[] = {
> +	{ .compatible = "st,stm32mp25-pcie-rc" },
> +	{},
> +};
> +
> +static struct platform_driver stm32_pcie_driver = {
> +	.probe = stm32_pcie_probe,
> +	.remove = stm32_pcie_remove,
> +	.driver = {
> +		.name = "stm32-pcie",
> +		.of_match_table = stm32_pcie_of_match,
> +		.pm = &stm32_pcie_pm_ops,
> +		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
> +	},
> +};
> +
> +module_platform_driver(stm32_pcie_driver);
> +
> +MODULE_AUTHOR("Christian Bruel <christian.bruel@foss.st.com>");
> +MODULE_DESCRIPTION("STM32MP25 PCIe Controller driver");
> +MODULE_LICENSE("GPL");
> +MODULE_DEVICE_TABLE(of, stm32_pcie_of_match);
> diff --git a/drivers/pci/controller/dwc/pcie-stm32.h b/drivers/pci/controller/dwc/pcie-stm32.h
> new file mode 100644
> index 000000000000..387112c4e42c
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-stm32.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * ST PCIe driver definitions for STM32-MP25 SoC
> + *
> + * Copyright (C) 2025 STMicroelectronics - All Rights Reserved
> + * Author: Christian Bruel <christian.bruel@foss.st.com>
> + */
> +
> +#define to_stm32_pcie(x)	dev_get_drvdata((x)->dev)
> +
> +#define STM32MP25_PCIECR_TYPE_MASK	GENMASK(11, 8)
> +#define STM32MP25_PCIECR_LTSSM_EN	BIT(2)
> +#define STM32MP25_PCIECR_RC		BIT(10)
> +
> +#define SYSCFG_PCIECR			0x6000
> -- 
> 2.34.1
> 

Re: [PATCH v12 2/9] PCI: stm32: Add PCIe host support for STM32MP25

[Christian Bruel]

On 8/7/25 20:09, Bjorn Helgaas wrote:
> [+to Linus for pinctrl usage question below]
> 
> On Tue, Jun 10, 2025 at 11:07:07AM +0200, Christian Bruel wrote:
>> Add driver for the STM32MP25 SoC PCIe Gen1 2.5 GT/s and Gen2 5GT/s
>> controller based on the DesignWare PCIe core.
>>

>> +
>> +	return pinctrl_pm_select_sleep_state(dev);
> 
> Isn't there some setup required before we can use
> pinctrl_select_state(), pinctrl_pm_select_sleep_state(),
> pinctrl_pm_select_default_state(), etc?
> 
> I expected something like devm_pinctrl_get() in the .probe() path, but
> I don't see anything.  I don't know how pinctrl works, but I don't see
> how dev->pins gets set up.

Linus knows better, but the dev->pins states are attached to the dev 
struct before probe by the pinctrl driver

/**
  * pinctrl_bind_pins() - called by the device core before probe
  * @dev: the device that is just about to probe
  */
int pinctrl_bind_pins(struct device *dev)

Christian

Re: [PATCH v12 2/9] PCI: stm32: Add PCIe host support for STM32MP25

[Bjorn Helgaas]

On Fri, Aug 08, 2025 at 04:55:52PM +0200, Christian Bruel wrote:
> On 8/7/25 20:09, Bjorn Helgaas wrote:
> > [+to Linus for pinctrl usage question below]
> > 
> > On Tue, Jun 10, 2025 at 11:07:07AM +0200, Christian Bruel wrote:
> > > Add driver for the STM32MP25 SoC PCIe Gen1 2.5 GT/s and Gen2 5GT/s
> > > controller based on the DesignWare PCIe core.

> > > +	return pinctrl_pm_select_sleep_state(dev);
> > 
> > Isn't there some setup required before we can use
> > pinctrl_select_state(), pinctrl_pm_select_sleep_state(),
> > pinctrl_pm_select_default_state(), etc?
> > 
> > I expected something like devm_pinctrl_get() in the .probe() path, but
> > I don't see anything.  I don't know how pinctrl works, but I don't see
> > how dev->pins gets set up.
> 
> Linus knows better, but the dev->pins states are attached to the dev struct
> before probe by the pinctrl driver
> 
> /**
>  * pinctrl_bind_pins() - called by the device core before probe
>  * @dev: the device that is just about to probe
>  */
> int pinctrl_bind_pins(struct device *dev)

Thanks for the pointer.  Might be worthy of a mention in
Documentation/driver-api/pin-control.rst.  Maybe pinctrl/consumer.h
could even have a bread crumb to that effect since drivers use all
those interfaces that rely in the implicit initialization done before
their .probe().

pin-control.rst mentions pinctrl_get_select_default() being called
just before the driver probe, but that's now unused and it looks like
pinctrl_bind_pins() does something similar:

  really_probe
    pinctrl_bind_pins
      dev->pins = devm_kzalloc()
      devm_pinctrl_get
      pinctrl_lookup_state(PINCTRL_STATE_DEFAULT)
      pinctrl_lookup_state(PINCTRL_STATE_INIT)
      pinctrl_select_state(init)      # if present, else default
    call_driver_probe

Bjorn

Re: [PATCH v12 2/9] PCI: stm32: Add PCIe host support for STM32MP25

[Christian Bruel]

On 8/8/25 18:45, Bjorn Helgaas wrote:
> On Fri, Aug 08, 2025 at 04:55:52PM +0200, Christian Bruel wrote:
>> On 8/7/25 20:09, Bjorn Helgaas wrote:
>>> [+to Linus for pinctrl usage question below]
>>>
>>> On Tue, Jun 10, 2025 at 11:07:07AM +0200, Christian Bruel wrote:
>>>> Add driver for the STM32MP25 SoC PCIe Gen1 2.5 GT/s and Gen2 5GT/s
>>>> controller based on the DesignWare PCIe core.
> 
>>>> +	return pinctrl_pm_select_sleep_state(dev);
>>>
>>> Isn't there some setup required before we can use
>>> pinctrl_select_state(), pinctrl_pm_select_sleep_state(),
>>> pinctrl_pm_select_default_state(), etc?
>>>
>>> I expected something like devm_pinctrl_get() in the .probe() path, but
>>> I don't see anything.  I don't know how pinctrl works, but I don't see
>>> how dev->pins gets set up.
>>
>> Linus knows better, but the dev->pins states are attached to the dev struct
>> before probe by the pinctrl driver
>>
>> /**
>>   * pinctrl_bind_pins() - called by the device core before probe
>>   * @dev: the device that is just about to probe
>>   */
>> int pinctrl_bind_pins(struct device *dev)
> 
> Thanks for the pointer.  Might be worthy of a mention in
> Documentation/driver-api/pin-control.rst.  Maybe pinctrl/consumer.h
> could even have a bread crumb to that effect since drivers use all
> those interfaces that rely in the implicit initialization done before
> their .probe().
> 
> pin-control.rst mentions pinctrl_get_select_default() being called
> just before the driver probe, but that's now unused and it looks like
> pinctrl_bind_pins() does something similar:
> 
>    really_probe
>      pinctrl_bind_pins
>        dev->pins = devm_kzalloc()
>        devm_pinctrl_get
>        pinctrl_lookup_state(PINCTRL_STATE_DEFAULT)
>        pinctrl_lookup_state(PINCTRL_STATE_INIT)
>        pinctrl_select_state(init)      # if present, else default
>      call_driver_probe

Yeah, and state_init is not mentioned in the documentation even for the 
'normal probe'. The only doc I see from the original commit 
ef0eebc05130b0d22b0ea65c0cd014ee16fc89c7

" 

     Let's introudce a new "init" state.  If this is defined we'll set
     pinctrl to this state before probe and then "default" after probe
     (unless the driver explicitly changed states already). 

"

I will propose something in pin-control.rst, with maybe some code-block 
for the pm part and respin [PATCH 0/2] Add pinctrl_pm_select_init_state 
helper function...

Christian

> 
> Bjorn

Re: [PATCH v12 2/9] PCI: stm32: Add PCIe host support for STM32MP25

[Bjorn Helgaas]

On Tue, Jun 10, 2025 at 11:07:07AM +0200, Christian Bruel wrote:
> Add driver for the STM32MP25 SoC PCIe Gen1 2.5 GT/s and Gen2 5GT/s
> controller based on the DesignWare PCIe core.
> ...

> +struct stm32_pcie {
> +	struct dw_pcie pci;
> +	struct regmap *regmap;
> +	struct reset_control *rst;
> +	struct phy *phy;
> +	struct clk *clk;
> +	struct gpio_desc *perst_gpio;
> +	struct gpio_desc *wake_gpio;
> +};
> +
> +static void stm32_pcie_deassert_perst(struct stm32_pcie *stm32_pcie)
> +{
> +	/* Delay PERST# de-assertion until the power stabilizes */
> +	msleep(PCIE_T_PVPERL_MS);
> +
> +	gpiod_set_value(stm32_pcie->perst_gpio, 0);
> +
> +	/* Wait for the REFCLK to stabilize */
> +	if (stm32_pcie->perst_gpio)
> +		msleep(PCIE_T_RRS_READY_MS);

This obviously relies on gpiod_set_value(stm32_pcie->perst_gpio, 0)
being a no-op when perst_gpio == NULL.  Since we're testing perst_gpio
anyway, I think it might be more readable to avoid relying on
gpiod_set_value() being a no-op:

  msleep(PCIE_T_PVPERL_MS);

  if (stm32_pcie->perst_gpio) {
    gpiod_set_value(stm32_pcie->perst_gpio, 0);
    msleep(PCIE_T_RRS_READY_MS);
  }

(And a similar change in stm32_pcie_assert_perst())

But this seems wrong because PCIE_T_PVPERL_MS is a delay before PERST#
is deasserted, but when perst_gpio == NULL, PERST# has already *been*
deasserted.

So it seems like it would make more sense as:

  if (stm32_pcie->perst_gpio) {
    msleep(PCIE_T_PVPERL_MS);
    gpiod_set_value(stm32_pcie->perst_gpio, 0);
  }

  msleep(PCIE_T_RRS_READY_MS);

> +}
> +
> +static void stm32_pcie_assert_perst(struct stm32_pcie *stm32_pcie)
> +{
> +	gpiod_set_value(stm32_pcie->perst_gpio, 1);
> +}
> +
> +static int stm32_pcie_start_link(struct dw_pcie *pci)
> +{
> +	struct stm32_pcie *stm32_pcie = to_stm32_pcie(pci);
> +
> +	return regmap_update_bits(stm32_pcie->regmap, SYSCFG_PCIECR,
> +				  STM32MP25_PCIECR_LTSSM_EN,
> +				  STM32MP25_PCIECR_LTSSM_EN);
> +}
> +
> +static void stm32_pcie_stop_link(struct dw_pcie *pci)
> +{
> +	struct stm32_pcie *stm32_pcie = to_stm32_pcie(pci);
> +
> +	regmap_update_bits(stm32_pcie->regmap, SYSCFG_PCIECR,
> +			   STM32MP25_PCIECR_LTSSM_EN, 0);
> +}
> +
> +static int stm32_pcie_suspend_noirq(struct device *dev)
> +{
> +	struct stm32_pcie *stm32_pcie = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = dw_pcie_suspend_noirq(&stm32_pcie->pci);
> +	if (ret)
> +		return ret;
> +
> +	stm32_pcie_assert_perst(stm32_pcie);
> +
> +	clk_disable_unprepare(stm32_pcie->clk);
> +
> +	if (!device_wakeup_path(dev))
> +		phy_exit(stm32_pcie->phy);
> +
> +	return pinctrl_pm_select_sleep_state(dev);
> +}
> +
> +static int stm32_pcie_resume_noirq(struct device *dev)
> +{
> +	struct stm32_pcie *stm32_pcie = dev_get_drvdata(dev);
> +	int ret;
> +
> +	/*
> +	 * The core clock is gated with CLKREQ# from the COMBOPHY REFCLK,
> +	 * thus if no device is present, must force it low with an init pinmux
> +	 * to be able to access the DBI registers.

What happens on initial probe if no device is present?  I assume we
access DBI registers in the dw_pcie_host_init() path, and it seems
like we'd have the same issue with DBI not being accessible when no
device is present.

> +	if (!IS_ERR(dev->pins->init_state))
> +		ret = pinctrl_select_state(dev->pins->p, dev->pins->init_state);
> +	else
> +		ret = pinctrl_pm_select_default_state(dev);
> +
> +	if (ret) {
> +		dev_err(dev, "Failed to activate pinctrl pm state: %d\n", ret);
> +		return ret;
> +	}
> +
> +	if (!device_wakeup_path(dev)) {
> +		ret = phy_init(stm32_pcie->phy);
> +		if (ret) {
> +			pinctrl_pm_select_default_state(dev);
> +			return ret;
> +		}
> +	}
> +
> +	ret = clk_prepare_enable(stm32_pcie->clk);
> +	if (ret)
> +		goto err_phy_exit;
> +
> +	stm32_pcie_deassert_perst(stm32_pcie);
> +
> +	ret = dw_pcie_resume_noirq(&stm32_pcie->pci);
> +	if (ret)
> +		goto err_disable_clk;
> +
> +	pinctrl_pm_select_default_state(dev);
> +
> +	return 0;
> +
> +err_disable_clk:
> +	stm32_pcie_assert_perst(stm32_pcie);
> +	clk_disable_unprepare(stm32_pcie->clk);
> +
> +err_phy_exit:
> +	phy_exit(stm32_pcie->phy);
> +	pinctrl_pm_select_default_state(dev);
> +
> +	return ret;
> +}
> +
> +static const struct dev_pm_ops stm32_pcie_pm_ops = {
> +	NOIRQ_SYSTEM_SLEEP_PM_OPS(stm32_pcie_suspend_noirq,
> +				  stm32_pcie_resume_noirq)
> +};
> +
> +static const struct dw_pcie_host_ops stm32_pcie_host_ops = {
> +};
> +
> +static const struct dw_pcie_ops dw_pcie_ops = {
> +	.start_link = stm32_pcie_start_link,
> +	.stop_link = stm32_pcie_stop_link
> +};
> +
> +static int stm32_add_pcie_port(struct stm32_pcie *stm32_pcie)
> +{
> +	struct device *dev = stm32_pcie->pci.dev;
> +	unsigned int wake_irq;
> +	int ret;
> +
> +	/* Start to enable resources with PERST# asserted */

I guess if device tree doesn't describe PERST#, we assume PERST# is
actually *deasserted* already at this point (because
stm32_pcie_deassert_perst() does nothing other than the delay)?

> +	ret = phy_set_mode(stm32_pcie->phy, PHY_MODE_PCIE);
> +	if (ret)
> +		return ret;
> +
> +	ret = phy_init(stm32_pcie->phy);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_update_bits(stm32_pcie->regmap, SYSCFG_PCIECR,
> +				 STM32MP25_PCIECR_TYPE_MASK,
> +				 STM32MP25_PCIECR_RC);
> +	if (ret)
> +		goto err_phy_exit;
> +
> +	stm32_pcie_deassert_perst(stm32_pcie);
> +
> +	if (stm32_pcie->wake_gpio) {
> +		wake_irq = gpiod_to_irq(stm32_pcie->wake_gpio);
> +		ret = dev_pm_set_dedicated_wake_irq(dev, wake_irq);
> +		if (ret) {
> +			dev_err(dev, "Failed to enable wakeup irq %d\n", ret);
> +			goto err_assert_perst;
> +		}
> +		irq_set_irq_type(wake_irq, IRQ_TYPE_EDGE_FALLING);
> +	}
> +
> +	return 0;
> +
> +err_assert_perst:
> +	stm32_pcie_assert_perst(stm32_pcie);
> +
> +err_phy_exit:
> +	phy_exit(stm32_pcie->phy);
> +
> +	return ret;
> +}
> +
> +static void stm32_remove_pcie_port(struct stm32_pcie *stm32_pcie)
> +{
> +	dev_pm_clear_wake_irq(stm32_pcie->pci.dev);
> +
> +	stm32_pcie_assert_perst(stm32_pcie);
> +
> +	phy_exit(stm32_pcie->phy);
> +}
> +
> +static int stm32_pcie_parse_port(struct stm32_pcie *stm32_pcie)
> +{
> +	struct device *dev = stm32_pcie->pci.dev;
> +	struct device_node *root_port;
> +
> +	root_port = of_get_next_available_child(dev->of_node, NULL);
> +
> +	stm32_pcie->phy = devm_of_phy_get(dev, root_port, NULL);
> +	if (IS_ERR(stm32_pcie->phy)) {
> +		of_node_put(root_port);
> +		return dev_err_probe(dev, PTR_ERR(stm32_pcie->phy),
> +				     "Failed to get pcie-phy\n");
> +	}
> +
> +	stm32_pcie->perst_gpio = devm_fwnode_gpiod_get(dev, of_fwnode_handle(root_port),
> +						       "reset", GPIOD_OUT_HIGH, NULL);
> +	if (IS_ERR(stm32_pcie->perst_gpio)) {
> +		if (PTR_ERR(stm32_pcie->perst_gpio) != -ENOENT) {
> +			of_node_put(root_port);
> +			return dev_err_probe(dev, PTR_ERR(stm32_pcie->perst_gpio),
> +					     "Failed to get reset GPIO\n");
> +		}
> +		stm32_pcie->perst_gpio = NULL;

This looks like perst_gpio is optional in device tree?  Is that really
the case?  It seems hard to ensure we have the PCIE_T_PVPERL_MS and
PCIE_T_RRS_READY_MS delays in the right place if we don't have
perst_gpio.

> +	}
> +
> +	stm32_pcie->wake_gpio = devm_fwnode_gpiod_get(dev, of_fwnode_handle(root_port),
> +						      "wake", GPIOD_IN, NULL);
> +
> +	if (IS_ERR(stm32_pcie->wake_gpio)) {
> +		if (PTR_ERR(stm32_pcie->wake_gpio) != -ENOENT) {
> +			of_node_put(root_port);
> +			return dev_err_probe(dev, PTR_ERR(stm32_pcie->wake_gpio),
> +					     "Failed to get wake GPIO\n");
> +		}
> +		stm32_pcie->wake_gpio = NULL;
> +	}
> +
> +	of_node_put(root_port);
> +
> +	return 0;
> +}
> +
> +static int stm32_pcie_probe(struct platform_device *pdev)
> +{
> +	struct stm32_pcie *stm32_pcie;
> +	struct device *dev = &pdev->dev;
> +	int ret;
> +
> +	stm32_pcie = devm_kzalloc(dev, sizeof(*stm32_pcie), GFP_KERNEL);
> +	if (!stm32_pcie)
> +		return -ENOMEM;
> +
> +	stm32_pcie->pci.dev = dev;
> +	stm32_pcie->pci.ops = &dw_pcie_ops;
> +	stm32_pcie->pci.pp.ops = &stm32_pcie_host_ops;
> +
> +	stm32_pcie->regmap = syscon_regmap_lookup_by_compatible("st,stm32mp25-syscfg");
> +	if (IS_ERR(stm32_pcie->regmap))
> +		return dev_err_probe(dev, PTR_ERR(stm32_pcie->regmap),
> +				     "No syscfg specified\n");
> +
> +	stm32_pcie->clk = devm_clk_get(dev, NULL);
> +	if (IS_ERR(stm32_pcie->clk))
> +		return dev_err_probe(dev, PTR_ERR(stm32_pcie->clk),
> +				     "Failed to get PCIe clock source\n");
> +
> +	stm32_pcie->rst = devm_reset_control_get_exclusive(dev, NULL);
> +	if (IS_ERR(stm32_pcie->rst))
> +		return dev_err_probe(dev, PTR_ERR(stm32_pcie->rst),
> +				     "Failed to get PCIe reset\n");
> +
> +	ret = stm32_pcie_parse_port(stm32_pcie);
> +	if (ret)
> +		return ret;
> +
> +	platform_set_drvdata(pdev, stm32_pcie);
> +
> +	ret = stm32_add_pcie_port(stm32_pcie);
> +	if (ret)
> +		return ret;
> +
> +	reset_control_assert(stm32_pcie->rst);
> +	reset_control_deassert(stm32_pcie->rst);
> +
> +	ret = clk_prepare_enable(stm32_pcie->clk);
> +	if (ret) {
> +		dev_err(dev, "Core clock enable failed %d\n", ret);
> +		goto err_remove_port;
> +	}
> +
> +	ret = pm_runtime_set_active(dev);
> +	if (ret < 0) {
> +		clk_disable_unprepare(stm32_pcie->clk);
> +		stm32_remove_pcie_port(stm32_pcie);
> +		return dev_err_probe(dev, ret, "Failed to activate runtime PM\n");
> +	}
> +
> +	pm_runtime_no_callbacks(dev);
> +
> +	ret = devm_pm_runtime_enable(dev);
> +	if (ret < 0) {
> +		clk_disable_unprepare(stm32_pcie->clk);
> +		stm32_remove_pcie_port(stm32_pcie);
> +		return dev_err_probe(dev, ret, "Failed to enable runtime PM\n");
> +	}
> +
> +	ret = dw_pcie_host_init(&stm32_pcie->pci.pp);
> +	if (ret)
> +		goto err_disable_clk;
> +
> +	if (stm32_pcie->wake_gpio)
> +		device_init_wakeup(dev, true);
> +
> +	return 0;
> +
> +err_disable_clk:
> +	clk_disable_unprepare(stm32_pcie->clk);
> +
> +err_remove_port:
> +	stm32_remove_pcie_port(stm32_pcie);
> +
> +	return ret;
> +}
> +
> +static void stm32_pcie_remove(struct platform_device *pdev)
> +{
> +	struct stm32_pcie *stm32_pcie = platform_get_drvdata(pdev);
> +	struct dw_pcie_rp *pp = &stm32_pcie->pci.pp;
> +
> +	if (stm32_pcie->wake_gpio)
> +		device_init_wakeup(&pdev->dev, false);
> +
> +	dw_pcie_host_deinit(pp);
> +
> +	clk_disable_unprepare(stm32_pcie->clk);
> +
> +	stm32_remove_pcie_port(stm32_pcie);
> +
> +	pm_runtime_put_noidle(&pdev->dev);
> +}
> +
> +static const struct of_device_id stm32_pcie_of_match[] = {
> +	{ .compatible = "st,stm32mp25-pcie-rc" },
> +	{},
> +};
> +
> +static struct platform_driver stm32_pcie_driver = {
> +	.probe = stm32_pcie_probe,
> +	.remove = stm32_pcie_remove,
> +	.driver = {
> +		.name = "stm32-pcie",
> +		.of_match_table = stm32_pcie_of_match,
> +		.pm = &stm32_pcie_pm_ops,
> +		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
> +	},
> +};
> +
> +module_platform_driver(stm32_pcie_driver);
> +
> +MODULE_AUTHOR("Christian Bruel <christian.bruel@foss.st.com>");
> +MODULE_DESCRIPTION("STM32MP25 PCIe Controller driver");
> +MODULE_LICENSE("GPL");
> +MODULE_DEVICE_TABLE(of, stm32_pcie_of_match);
> diff --git a/drivers/pci/controller/dwc/pcie-stm32.h b/drivers/pci/controller/dwc/pcie-stm32.h
> new file mode 100644
> index 000000000000..387112c4e42c
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-stm32.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * ST PCIe driver definitions for STM32-MP25 SoC
> + *
> + * Copyright (C) 2025 STMicroelectronics - All Rights Reserved
> + * Author: Christian Bruel <christian.bruel@foss.st.com>
> + */
> +
> +#define to_stm32_pcie(x)	dev_get_drvdata((x)->dev)
> +
> +#define STM32MP25_PCIECR_TYPE_MASK	GENMASK(11, 8)
> +#define STM32MP25_PCIECR_LTSSM_EN	BIT(2)
> +#define STM32MP25_PCIECR_RC		BIT(10)
> +
> +#define SYSCFG_PCIECR			0x6000
> -- 
> 2.34.1
> 

Re: [PATCH v12 2/9] PCI: stm32: Add PCIe host support for STM32MP25

[Christian Bruel]

On 8/13/25 21:29, Bjorn Helgaas wrote:
> On Tue, Jun 10, 2025 at 11:07:07AM +0200, Christian Bruel wrote:
>> Add driver for the STM32MP25 SoC PCIe Gen1 2.5 GT/s and Gen2 5GT/s
>> controller based on the DesignWare PCIe core.
>> ...
> 
>> +struct stm32_pcie {
>> +	struct dw_pcie pci;
>> +	struct regmap *regmap;
>> +	struct reset_control *rst;
>> +	struct phy *phy;
>> +	struct clk *clk;
>> +	struct gpio_desc *perst_gpio;
>> +	struct gpio_desc *wake_gpio;
>> +};
>> +
>> +static void stm32_pcie_deassert_perst(struct stm32_pcie *stm32_pcie)
>> +{
>> +	/* Delay PERST# de-assertion until the power stabilizes */
>> +	msleep(PCIE_T_PVPERL_MS);
>> +
>> +	gpiod_set_value(stm32_pcie->perst_gpio, 0);
>> +
>> +	/* Wait for the REFCLK to stabilize */
>> +	if (stm32_pcie->perst_gpio)
>> +		msleep(PCIE_T_RRS_READY_MS);
> 
> This obviously relies on gpiod_set_value(stm32_pcie->perst_gpio, 0)
> being a no-op when perst_gpio == NULL.  Since we're testing perst_gpio
> anyway, I think it might be more readable to avoid relying on
> gpiod_set_value() being a no-op:
> 
>    msleep(PCIE_T_PVPERL_MS);
> 
>    if (stm32_pcie->perst_gpio) {
>      gpiod_set_value(stm32_pcie->perst_gpio, 0);
>      msleep(PCIE_T_RRS_READY_MS);
>    }
> 
> (And a similar change in stm32_pcie_assert_perst())
> 
> But this seems wrong because PCIE_T_PVPERL_MS is a delay before PERST#
> is deasserted, but when perst_gpio == NULL, PERST# has already *been*
> deasserted.
> 
> So it seems like it would make more sense as:
> 
>    if (stm32_pcie->perst_gpio) {
>      msleep(PCIE_T_PVPERL_MS);
>      gpiod_set_value(stm32_pcie->perst_gpio, 0);
>    }
> 
>    msleep(PCIE_T_RRS_READY_MS);

I agree, grouping pvperl_ms and the PERST# deassertion together makes 
sense. thank you.

> 
>> +}
>> +
>> +static void stm32_pcie_assert_perst(struct stm32_pcie *stm32_pcie)
>> +{
>> +	gpiod_set_value(stm32_pcie->perst_gpio, 1);
>> +}
>> +
>> +static int stm32_pcie_start_link(struct dw_pcie *pci)
>> +{
>> +	struct stm32_pcie *stm32_pcie = to_stm32_pcie(pci);
>> +
>> +	return regmap_update_bits(stm32_pcie->regmap, SYSCFG_PCIECR,
>> +				  STM32MP25_PCIECR_LTSSM_EN,
>> +				  STM32MP25_PCIECR_LTSSM_EN);
>> +}
>> +
>> +static void stm32_pcie_stop_link(struct dw_pcie *pci)
>> +{
>> +	struct stm32_pcie *stm32_pcie = to_stm32_pcie(pci);
>> +
>> +	regmap_update_bits(stm32_pcie->regmap, SYSCFG_PCIECR,
>> +			   STM32MP25_PCIECR_LTSSM_EN, 0);
>> +}
>> +
>> +static int stm32_pcie_suspend_noirq(struct device *dev)
>> +{
>> +	struct stm32_pcie *stm32_pcie = dev_get_drvdata(dev);
>> +	int ret;
>> +
>> +	ret = dw_pcie_suspend_noirq(&stm32_pcie->pci);
>> +	if (ret)
>> +		return ret;
>> +
>> +	stm32_pcie_assert_perst(stm32_pcie);
>> +
>> +	clk_disable_unprepare(stm32_pcie->clk);
>> +
>> +	if (!device_wakeup_path(dev))
>> +		phy_exit(stm32_pcie->phy);
>> +
>> +	return pinctrl_pm_select_sleep_state(dev);
>> +}
>> +
>> +static int stm32_pcie_resume_noirq(struct device *dev)
>> +{
>> +	struct stm32_pcie *stm32_pcie = dev_get_drvdata(dev);
>> +	int ret;
>> +
>> +	/*
>> +	 * The core clock is gated with CLKREQ# from the COMBOPHY REFCLK,
>> +	 * thus if no device is present, must force it low with an init pinmux
>> +	 * to be able to access the DBI registers.
> 
> What happens on initial probe if no device is present?  I assume we
> access DBI registers in the dw_pcie_host_init() path, and it seems
> like we'd have the same issue with DBI not being accessible when no
> device is present.

Correct, same issue. The magic happens with the PINCTRL init state that 
is automatically called before probe. As I tried to explain in the 
Documentation in the pinctrl patch:

- if ``init`` and ``default`` are defined in the device tree, the "init"
   state is selected before the driver probe and the "default" state is
   selected after the driver probe.

> 
>> +	if (!IS_ERR(dev->pins->init_state))
>> +		ret = pinctrl_select_state(dev->pins->p, dev->pins->init_state);
>> +	else
>> +		ret = pinctrl_pm_select_default_state(dev);
>> +
>> +	if (ret) {
>> +		dev_err(dev, "Failed to activate pinctrl pm state: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	if (!device_wakeup_path(dev)) {
>> +		ret = phy_init(stm32_pcie->phy);
>> +		if (ret) {
>> +			pinctrl_pm_select_default_state(dev);
>> +			return ret;
>> +		}
>> +	}
>> +
>> +	ret = clk_prepare_enable(stm32_pcie->clk);
>> +	if (ret)
>> +		goto err_phy_exit;
>> +
>> +	stm32_pcie_deassert_perst(stm32_pcie);
>> +
>> +	ret = dw_pcie_resume_noirq(&stm32_pcie->pci);
>> +	if (ret)
>> +		goto err_disable_clk;
>> +
>> +	pinctrl_pm_select_default_state(dev);
>> +
>> +	return 0;
>> +
>> +err_disable_clk:
>> +	stm32_pcie_assert_perst(stm32_pcie);
>> +	clk_disable_unprepare(stm32_pcie->clk);
>> +
>> +err_phy_exit:
>> +	phy_exit(stm32_pcie->phy);
>> +	pinctrl_pm_select_default_state(dev);
>> +
>> +	return ret;
>> +}
>> +
>> +static const struct dev_pm_ops stm32_pcie_pm_ops = {
>> +	NOIRQ_SYSTEM_SLEEP_PM_OPS(stm32_pcie_suspend_noirq,
>> +				  stm32_pcie_resume_noirq)
>> +};
>> +
>> +static const struct dw_pcie_host_ops stm32_pcie_host_ops = {
>> +};
>> +
>> +static const struct dw_pcie_ops dw_pcie_ops = {
>> +	.start_link = stm32_pcie_start_link,
>> +	.stop_link = stm32_pcie_stop_link
>> +};
>> +
>> +static int stm32_add_pcie_port(struct stm32_pcie *stm32_pcie)
>> +{
>> +	struct device *dev = stm32_pcie->pci.dev;
>> +	unsigned int wake_irq;
>> +	int ret;
>> +
>> +	/* Start to enable resources with PERST# asserted */
> 
> I guess if device tree doesn't describe PERST#, we assume PERST# is
> actually *deasserted* already at this point (because
> stm32_pcie_deassert_perst() does nothing other than the delay)?

yes, this also implies that PV is stable

> 
>> +	ret = phy_set_mode(stm32_pcie->phy, PHY_MODE_PCIE);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = phy_init(stm32_pcie->phy);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = regmap_update_bits(stm32_pcie->regmap, SYSCFG_PCIECR,
>> +				 STM32MP25_PCIECR_TYPE_MASK,
>> +				 STM32MP25_PCIECR_RC);
>> +	if (ret)
>> +		goto err_phy_exit;
>> +
>> +	stm32_pcie_deassert_perst(stm32_pcie);
>> +
>> +	if (stm32_pcie->wake_gpio) {
>> +		wake_irq = gpiod_to_irq(stm32_pcie->wake_gpio);
>> +		ret = dev_pm_set_dedicated_wake_irq(dev, wake_irq);
>> +		if (ret) {
>> +			dev_err(dev, "Failed to enable wakeup irq %d\n", ret);
>> +			goto err_assert_perst;
>> +		}
>> +		irq_set_irq_type(wake_irq, IRQ_TYPE_EDGE_FALLING);
>> +	}
>> +
>> +	return 0;
>> +
>> +err_assert_perst:
>> +	stm32_pcie_assert_perst(stm32_pcie);
>> +
>> +err_phy_exit:
>> +	phy_exit(stm32_pcie->phy);
>> +
>> +	return ret;
>> +}
>> +
>> +static void stm32_remove_pcie_port(struct stm32_pcie *stm32_pcie)
>> +{
>> +	dev_pm_clear_wake_irq(stm32_pcie->pci.dev);
>> +
>> +	stm32_pcie_assert_perst(stm32_pcie);
>> +
>> +	phy_exit(stm32_pcie->phy);
>> +}
>> +
>> +static int stm32_pcie_parse_port(struct stm32_pcie *stm32_pcie)
>> +{
>> +	struct device *dev = stm32_pcie->pci.dev;
>> +	struct device_node *root_port;
>> +
>> +	root_port = of_get_next_available_child(dev->of_node, NULL);
>> +
>> +	stm32_pcie->phy = devm_of_phy_get(dev, root_port, NULL);
>> +	if (IS_ERR(stm32_pcie->phy)) {
>> +		of_node_put(root_port);
>> +		return dev_err_probe(dev, PTR_ERR(stm32_pcie->phy),
>> +				     "Failed to get pcie-phy\n");
>> +	}
>> +
>> +	stm32_pcie->perst_gpio = devm_fwnode_gpiod_get(dev, of_fwnode_handle(root_port),
>> +						       "reset", GPIOD_OUT_HIGH, NULL);
>> +	if (IS_ERR(stm32_pcie->perst_gpio)) {
>> +		if (PTR_ERR(stm32_pcie->perst_gpio) != -ENOENT) {
>> +			of_node_put(root_port);
>> +			return dev_err_probe(dev, PTR_ERR(stm32_pcie->perst_gpio),
>> +					     "Failed to get reset GPIO\n");
>> +		}
>> +		stm32_pcie->perst_gpio = NULL;
> 
> This looks like perst_gpio is optional in device tree?  Is that really
> the case?  It seems hard to ensure we have the PCIE_T_PVPERL_MS and
> PCIE_T_RRS_READY_MS delays in the right place if we don't have
> perst_gpio.

perst_gpio was initially made optional for pre-silicon platforms with 
hooked PHY functions (or no PHY).
It is assumed that the PV and REFCLK signals are already stable, I 
suppose that without perst_gpio, the system must rely on these 
assumptions. But my experience with other models without perst as gpio 
is limited to this.

> 
>> +	}
>> +
>> +	stm32_pcie->wake_gpio = devm_fwnode_gpiod_get(dev, of_fwnode_handle(root_port),
>> +						      "wake", GPIOD_IN, NULL);
>> +
>> +	if (IS_ERR(stm32_pcie->wake_gpio)) {
>> +		if (PTR_ERR(stm32_pcie->wake_gpio) != -ENOENT) {
>> +			of_node_put(root_port);
>> +			return dev_err_probe(dev, PTR_ERR(stm32_pcie->wake_gpio),
>> +					     "Failed to get wake GPIO\n");
>> +		}
>> +		stm32_pcie->wake_gpio = NULL;
>> +	}
>> +
>> +	of_node_put(root_port);
>> +
>> +	return 0;
>> +}
>> +
>> +static int stm32_pcie_probe(struct platform_device *pdev)
>> +{
>> +	struct stm32_pcie *stm32_pcie;
>> +	struct device *dev = &pdev->dev;
>> +	int ret;
>> +
>> +	stm32_pcie = devm_kzalloc(dev, sizeof(*stm32_pcie), GFP_KERNEL);
>> +	if (!stm32_pcie)
>> +		return -ENOMEM;
>> +
>> +	stm32_pcie->pci.dev = dev;
>> +	stm32_pcie->pci.ops = &dw_pcie_ops;
>> +	stm32_pcie->pci.pp.ops = &stm32_pcie_host_ops;
>> +
>> +	stm32_pcie->regmap = syscon_regmap_lookup_by_compatible("st,stm32mp25-syscfg");
>> +	if (IS_ERR(stm32_pcie->regmap))
>> +		return dev_err_probe(dev, PTR_ERR(stm32_pcie->regmap),
>> +				     "No syscfg specified\n");
>> +
>> +	stm32_pcie->clk = devm_clk_get(dev, NULL);
>> +	if (IS_ERR(stm32_pcie->clk))
>> +		return dev_err_probe(dev, PTR_ERR(stm32_pcie->clk),
>> +				     "Failed to get PCIe clock source\n");
>> +
>> +	stm32_pcie->rst = devm_reset_control_get_exclusive(dev, NULL);
>> +	if (IS_ERR(stm32_pcie->rst))
>> +		return dev_err_probe(dev, PTR_ERR(stm32_pcie->rst),
>> +				     "Failed to get PCIe reset\n");
>> +
>> +	ret = stm32_pcie_parse_port(stm32_pcie);
>> +	if (ret)
>> +		return ret;
>> +
>> +	platform_set_drvdata(pdev, stm32_pcie);
>> +
>> +	ret = stm32_add_pcie_port(stm32_pcie);
>> +	if (ret)
>> +		return ret;
>> +
>> +	reset_control_assert(stm32_pcie->rst);
>> +	reset_control_deassert(stm32_pcie->rst);
>> +
>> +	ret = clk_prepare_enable(stm32_pcie->clk);
>> +	if (ret) {
>> +		dev_err(dev, "Core clock enable failed %d\n", ret);
>> +		goto err_remove_port;
>> +	}
>> +
>> +	ret = pm_runtime_set_active(dev);
>> +	if (ret < 0) {
>> +		clk_disable_unprepare(stm32_pcie->clk);
>> +		stm32_remove_pcie_port(stm32_pcie);
>> +		return dev_err_probe(dev, ret, "Failed to activate runtime PM\n");
>> +	}
>> +
>> +	pm_runtime_no_callbacks(dev);
>> +
>> +	ret = devm_pm_runtime_enable(dev);
>> +	if (ret < 0) {
>> +		clk_disable_unprepare(stm32_pcie->clk);
>> +		stm32_remove_pcie_port(stm32_pcie);
>> +		return dev_err_probe(dev, ret, "Failed to enable runtime PM\n");
>> +	}
>> +
>> +	ret = dw_pcie_host_init(&stm32_pcie->pci.pp);
>> +	if (ret)
>> +		goto err_disable_clk;
>> +
>> +	if (stm32_pcie->wake_gpio)
>> +		device_init_wakeup(dev, true);
>> +
>> +	return 0;
>> +
>> +err_disable_clk:
>> +	clk_disable_unprepare(stm32_pcie->clk);
>> +
>> +err_remove_port:
>> +	stm32_remove_pcie_port(stm32_pcie);
>> +
>> +	return ret;
>> +}
>> +
>> +static void stm32_pcie_remove(struct platform_device *pdev)
>> +{
>> +	struct stm32_pcie *stm32_pcie = platform_get_drvdata(pdev);
>> +	struct dw_pcie_rp *pp = &stm32_pcie->pci.pp;
>> +
>> +	if (stm32_pcie->wake_gpio)
>> +		device_init_wakeup(&pdev->dev, false);
>> +
>> +	dw_pcie_host_deinit(pp);
>> +
>> +	clk_disable_unprepare(stm32_pcie->clk);
>> +
>> +	stm32_remove_pcie_port(stm32_pcie);
>> +
>> +	pm_runtime_put_noidle(&pdev->dev);
>> +}
>> +
>> +static const struct of_device_id stm32_pcie_of_match[] = {
>> +	{ .compatible = "st,stm32mp25-pcie-rc" },
>> +	{},
>> +};
>> +
>> +static struct platform_driver stm32_pcie_driver = {
>> +	.probe = stm32_pcie_probe,
>> +	.remove = stm32_pcie_remove,
>> +	.driver = {
>> +		.name = "stm32-pcie",
>> +		.of_match_table = stm32_pcie_of_match,
>> +		.pm = &stm32_pcie_pm_ops,
>> +		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
>> +	},
>> +};
>> +
>> +module_platform_driver(stm32_pcie_driver);
>> +
>> +MODULE_AUTHOR("Christian Bruel <christian.bruel@foss.st.com>");
>> +MODULE_DESCRIPTION("STM32MP25 PCIe Controller driver");
>> +MODULE_LICENSE("GPL");
>> +MODULE_DEVICE_TABLE(of, stm32_pcie_of_match);
>> diff --git a/drivers/pci/controller/dwc/pcie-stm32.h b/drivers/pci/controller/dwc/pcie-stm32.h
>> new file mode 100644
>> index 000000000000..387112c4e42c
>> --- /dev/null
>> +++ b/drivers/pci/controller/dwc/pcie-stm32.h
>> @@ -0,0 +1,15 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * ST PCIe driver definitions for STM32-MP25 SoC
>> + *
>> + * Copyright (C) 2025 STMicroelectronics - All Rights Reserved
>> + * Author: Christian Bruel <christian.bruel@foss.st.com>
>> + */
>> +
>> +#define to_stm32_pcie(x)	dev_get_drvdata((x)->dev)
>> +
>> +#define STM32MP25_PCIECR_TYPE_MASK	GENMASK(11, 8)
>> +#define STM32MP25_PCIECR_LTSSM_EN	BIT(2)
>> +#define STM32MP25_PCIECR_RC		BIT(10)
>> +
>> +#define SYSCFG_PCIECR			0x6000
>> -- 
>> 2.34.1
>>

Re: [PATCH v12 2/9] PCI: stm32: Add PCIe host support for STM32MP25

[Bjorn Helgaas]

[+cc Linus]

On Mon, Aug 18, 2025 at 12:50:19PM +0200, Christian Bruel wrote:
> On 8/13/25 21:29, Bjorn Helgaas wrote:
> > On Tue, Jun 10, 2025 at 11:07:07AM +0200, Christian Bruel wrote:
> > > Add driver for the STM32MP25 SoC PCIe Gen1 2.5 GT/s and Gen2 5GT/s
> > > controller based on the DesignWare PCIe core.
> > > ...

> > > +static int stm32_pcie_resume_noirq(struct device *dev)
> > > +{
> > > +	struct stm32_pcie *stm32_pcie = dev_get_drvdata(dev);
> > > +	int ret;
> > > +
> > > +	/*
> > > +	 * The core clock is gated with CLKREQ# from the COMBOPHY REFCLK,
> > > +	 * thus if no device is present, must force it low with an init pinmux
> > > +	 * to be able to access the DBI registers.
> > 
> > What happens on initial probe if no device is present?  I assume we
> > access DBI registers in the dw_pcie_host_init() path, and it seems
> > like we'd have the same issue with DBI not being accessible when no
> > device is present.
> 
> Correct, same issue. The magic happens with the PINCTRL init state that is
> automatically called before probe. As I tried to explain in the
> Documentation in the pinctrl patch:
> 
> - if ``init`` and ``default`` are defined in the device tree, the "init"
>   state is selected before the driver probe and the "default" state is
>   selected after the driver probe.

OK, thanks for that reminder!

The fact that the pinctrl init state is set implicitly before .probe(),
but we have to do it explicitly in .stm32_pcie_resume_noirq() seems a
*little* bit weird and makes the driver harder to review.  But ... I
guess that's out of scope for this series.

I see that Linus has given his approval to merge the new
pinctrl_pm_select_init_state() along with this series.  Would you mind
pulling those changes [1] and the use [2] into a v13 of this series?
That way I won't have to collect up all the pieces and risk missing
something.

BTW, I noticed a s/platformm/platform/ typo in the "[PATCH v1 2/2]
pinctrl: Add pinctrl_pm_select_init_state helper function" patch.

> > > +	if (!IS_ERR(dev->pins->init_state))
> > > +		ret = pinctrl_select_state(dev->pins->p, dev->pins->init_state);
> > > +	else
> > > +		ret = pinctrl_pm_select_default_state(dev);
> > > +
> > > +	if (ret) {
> > > +		dev_err(dev, "Failed to activate pinctrl pm state: %d\n", ret);
> > > +		return ret;
> > > +	}

> > > +static int stm32_add_pcie_port(struct stm32_pcie *stm32_pcie)
> > > +{
> > > +	struct device *dev = stm32_pcie->pci.dev;
> > > +	unsigned int wake_irq;
> > > +	int ret;
> > > +
> > > +	/* Start to enable resources with PERST# asserted */
> > 
> > I guess if device tree doesn't describe PERST#, we assume PERST# is
> > actually *deasserted* already at this point (because
> > stm32_pcie_deassert_perst() does nothing other than the delay)?
> 
> yes, this also implies that PV is stable

Maybe we could update the comment somehow?  Or maybe even just remove
it, since we actually don't know the state of PERST# at this point?

It sounds like we don't know the PERST# state until after we call
stm32_pcie_deassert_perst() below.

> > > +	ret = phy_set_mode(stm32_pcie->phy, PHY_MODE_PCIE);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	ret = phy_init(stm32_pcie->phy);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	ret = regmap_update_bits(stm32_pcie->regmap, SYSCFG_PCIECR,
> > > +				 STM32MP25_PCIECR_TYPE_MASK,
> > > +				 STM32MP25_PCIECR_RC);
> > > +	if (ret)
> > > +		goto err_phy_exit;
> > > +
> > > +	stm32_pcie_deassert_perst(stm32_pcie);

Bjorn

[1] https://lore.kernel.org/r/20250813081139.93201-1-christian.bruel@foss.st.com
[2] https://lore.kernel.org/r/20250813115319.212721-1-christian.bruel@foss.st.com

Re: [PATCH v12 2/9] PCI: stm32: Add PCIe host support for STM32MP25

[Christian Bruel]

On 8/19/25 01:06, Bjorn Helgaas wrote:
> [+cc Linus]
> 
> On Mon, Aug 18, 2025 at 12:50:19PM +0200, Christian Bruel wrote:
>> On 8/13/25 21:29, Bjorn Helgaas wrote:
>>> On Tue, Jun 10, 2025 at 11:07:07AM +0200, Christian Bruel wrote:
>>>> Add driver for the STM32MP25 SoC PCIe Gen1 2.5 GT/s and Gen2 5GT/s
>>>> controller based on the DesignWare PCIe core.
>>>> ...
> 
>>>> +static int stm32_pcie_resume_noirq(struct device *dev)
>>>> +{
>>>> +	struct stm32_pcie *stm32_pcie = dev_get_drvdata(dev);
>>>> +	int ret;
>>>> +
>>>> +	/*
>>>> +	 * The core clock is gated with CLKREQ# from the COMBOPHY REFCLK,
>>>> +	 * thus if no device is present, must force it low with an init pinmux
>>>> +	 * to be able to access the DBI registers.
>>>
>>> What happens on initial probe if no device is present?  I assume we
>>> access DBI registers in the dw_pcie_host_init() path, and it seems
>>> like we'd have the same issue with DBI not being accessible when no
>>> device is present.
>>
>> Correct, same issue. The magic happens with the PINCTRL init state that is
>> automatically called before probe. As I tried to explain in the
>> Documentation in the pinctrl patch:
>>
>> - if ``init`` and ``default`` are defined in the device tree, the "init"
>>    state is selected before the driver probe and the "default" state is
>>    selected after the driver probe.
> 
> OK, thanks for that reminder!
> 
> The fact that the pinctrl init state is set implicitly before .probe(),
> but we have to do it explicitly in .stm32_pcie_resume_noirq() seems a
> *little* bit weird and makes the driver harder to review.  But ... I
> guess that's out of scope for this series.
> 
> I see that Linus has given his approval to merge the new
> pinctrl_pm_select_init_state() along with this series.  Would you mind
> pulling those changes [1] and the use [2] into a v13 of this series?
> That way I won't have to collect up all the pieces and risk missing
> something.

sure, I will rebase and re-spin with squashed last bits.

> 
> BTW, I noticed a s/platformm/platform/ typo in the "[PATCH v1 2/2]
> pinctrl: Add pinctrl_pm_select_init_state helper function" patch.

thank you,

> 
>>>> +	if (!IS_ERR(dev->pins->init_state))
>>>> +		ret = pinctrl_select_state(dev->pins->p, dev->pins->init_state);
>>>> +	else
>>>> +		ret = pinctrl_pm_select_default_state(dev);
>>>> +
>>>> +	if (ret) {
>>>> +		dev_err(dev, "Failed to activate pinctrl pm state: %d\n", ret);
>>>> +		return ret;
>>>> +	}
> 
>>>> +static int stm32_add_pcie_port(struct stm32_pcie *stm32_pcie)
>>>> +{
>>>> +	struct device *dev = stm32_pcie->pci.dev;
>>>> +	unsigned int wake_irq;
>>>> +	int ret;
>>>> +
>>>> +	/* Start to enable resources with PERST# asserted */
>>>
>>> I guess if device tree doesn't describe PERST#, we assume PERST# is
>>> actually *deasserted* already at this point (because
>>> stm32_pcie_deassert_perst() does nothing other than the delay)?
>>
>> yes, this also implies that PV is stable
> 
> Maybe we could update the comment somehow?  Or maybe even just remove
> it, since we actually don't know the state of PERST# at this point?


> 
> It sounds like we don't know the PERST# state until after we call
> stm32_pcie_deassert_perst() 

If perst_gpio is defined in the DT, PERST# is asserted (GPIOD_OUT_HIGH) 
in stm32_pcie_parse_port().
If it is not, we must assume PERST# was already started de-asserted from 
the environment along with the REFCLK

So locally I agree, we don't know the PERST# state.
I will remove this useless ambiguous comment

Christian

> 
>>>> +	ret = phy_set_mode(stm32_pcie->phy, PHY_MODE_PCIE);
>>>> +	if (ret)
>>>> +		return ret;
>>>> +
>>>> +	ret = phy_init(stm32_pcie->phy);
>>>> +	if (ret)
>>>> +		return ret;
>>>> +
>>>> +	ret = regmap_update_bits(stm32_pcie->regmap, SYSCFG_PCIECR,
>>>> +				 STM32MP25_PCIECR_TYPE_MASK,
>>>> +				 STM32MP25_PCIECR_RC);
>>>> +	if (ret)
>>>> +		goto err_phy_exit;
>>>> +
>>>> +	stm32_pcie_deassert_perst(stm32_pcie);
> 
> Bjorn
> 
> [1] https://lore.kernel.org/r/20250813081139.93201-1-christian.bruel@foss.st.com
> [2] https://lore.kernel.org/r/20250813115319.212721-1-christian.bruel@foss.st.com
>