[PATCH v2 12/24] dt-bindings: PCI: Add ZTE ZX279128S host controller

[PATCH v2 12/24] dt-bindings: PCI: Add ZTE ZX279128S host controller

[Navid Ghahremani]

Add Device Tree binding for the DesignWare PCIe host controller on the
ZTE ZX279128S SoC, using standard clocks, resets, and phys properties.

Signed-off-by: Navid Ghahremani <ghahramani.navid@gmail.com>
---
 .../bindings/pci/zte,zx279128s-pcie.yaml      | 121 ++++++++++++++++++
 1 file changed, 121 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/zte,zx279128s-pcie.yaml

diff --git a/Documentation/devicetree/bindings/pci/zte,zx279128s-pcie.yaml b/Documentation/devicetree/bindings/pci/zte,zx279128s-pcie.yaml
new file mode 100644
index 0000000000..dc0214511f
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/zte,zx279128s-pcie.yaml
@@ -0,0 +1,121 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/zte,zx279128s-pcie.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ZTE zx279128s PCIe host controller
+
+maintainers:
+  - Navid Ghahremani <ghahramani.navid@gmail.com>
+
+description:
+  The ZTE zx279128s has two PCIe host controllers based on the Synopsys
+  DesignWare PCIe IP, each with one lane. Their clocks, resets and PHYs are
+  controlled through the SoC's Top CRM blocks.
+
+allOf:
+  - $ref: /schemas/pci/pci-host-bridge.yaml#
+  - $ref: /schemas/pci/snps,dw-pcie-common.yaml#
+
+properties:
+  compatible:
+    const: zte,zx279128s-pcie
+
+  reg:
+    items:
+      - description: Data Bus Interface (DBI) registers
+      - description: Controller wrapper registers
+      - description: PCIe configuration space region
+
+  reg-names:
+    items:
+      - const: dbi
+      - const: ctrl
+      - const: config
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: PCIe controller clock
+
+  clock-names:
+    items:
+      - const: pcie
+
+  resets:
+    items:
+      - description: PCIe controller reset
+
+  reset-names:
+    items:
+      - const: core
+
+  phys:
+    maxItems: 1
+
+  phy-names:
+    items:
+      - const: pcie
+
+  reset-gpios:
+    maxItems: 1
+    description: Optional board GPIO reset line for downstream endpoints.
+
+  enable-gpios:
+    maxItems: 1
+    description: Optional second board GPIO enable line driven together with reset-gpios.
+
+required:
+  - reg
+  - reg-names
+  - interrupts
+  - '#interrupt-cells'
+  - interrupt-map
+  - interrupt-map-mask
+  - clocks
+  - clock-names
+  - resets
+  - reset-names
+  - phys
+  - phy-names
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/zte,zx279128s-crm.h>
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/reset/zte,zx279128s-reset.h>
+
+    pcie@f000000 {
+        compatible = "zte,zx279128s-pcie";
+        reg = <0x0f000000 0x4000>,
+              <0x09500000 0x1000>,
+              <0x1c000000 0x200000>;
+        reg-names = "dbi", "ctrl", "config";
+        #address-cells = <3>;
+        #size-cells = <2>;
+        device_type = "pci";
+        bus-range = <0x00 0xff>;
+        ranges = <0x82000000 0 0x10000000 0x10000000 0 0x08000000>;
+        interrupts = <GIC_SPI 63 IRQ_TYPE_LEVEL_HIGH>;
+        #interrupt-cells = <1>;
+        interrupt-map-mask = <0 0 0 7>;
+        interrupt-map = <0 0 0 1 &gic GIC_SPI 63 IRQ_TYPE_LEVEL_HIGH>,
+                        <0 0 0 2 &gic GIC_SPI 63 IRQ_TYPE_LEVEL_HIGH>,
+                        <0 0 0 3 &gic GIC_SPI 63 IRQ_TYPE_LEVEL_HIGH>,
+                        <0 0 0 4 &gic GIC_SPI 63 IRQ_TYPE_LEVEL_HIGH>;
+        linux,pci-domain = <0>;
+        clocks = <&crm2 ZX279128S_CRM2_PCIE0_CLK>;
+        clock-names = "pcie";
+        resets = <&crm2 ZX279128S_CRM2_RESET_PCIE0>;
+        reset-names = "core";
+        phys = <&pcie0_phy>;
+        phy-names = "pcie";
+        reset-gpios = <&gpio3 5 GPIO_ACTIVE_LOW>;
+        enable-gpios = <&gpio3 7 GPIO_ACTIVE_HIGH>;
+    };
-- 
2.55.0

[PATCH v2 13/24] PCI: dwc: Add ZTE ZX279128S host controller driver

[Navid Ghahremani]

Support the two Synopsys DesignWare PCIe host controllers of the
ZTE ZX279128S SoC. The driver manages the controller clocks via CCF,
asserts/deasserts resets via the Reset framework, and manages the
analog SerDes and PLL via the Generic PHY framework.

Signed-off-by: Navid Ghahremani <ghahramani.navid@gmail.com>
---
 drivers/pci/controller/dwc/Kconfig          |  13 +
 drivers/pci/controller/dwc/Makefile         |   1 +
 drivers/pci/controller/dwc/pcie-zx279128s.c | 273 ++++++++++++++++++++
 3 files changed, 287 insertions(+)
 create mode 100644 drivers/pci/controller/dwc/pcie-zx279128s.c

diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index dcfbe7e229..3a4008be84 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -577,4 +577,17 @@ config PCIE_ULTRARISC
 	  By default, this symbol is enabled when ARCH_ULTRARISC is active,
 	  requiring no further configuration on that platform.
 
+config PCIE_ZX279128S
+	bool "ZTE zx279128s PCIe controller"
+	depends on ARCH_ZTE || COMPILE_TEST
+	depends on PCI_MSI
+	select GENERIC_PHY
+	select PCIE_DW_HOST
+	select RESET_CONTROLLER
+	help
+	  Say Y here to enable support for the two PCIe host controllers of
+	  the ZTE zx279128s SoC. They are based on the Synopsys DesignWare
+	  PCIe IP. On the ZTE ZXHN H3600 they connect the MT7915 Wi-Fi chip.
+	  The driver only supports legacy INTx interrupts.
+
 endmenu
diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
index d4f88cfc62..379198ebd6 100644
--- a/drivers/pci/controller/dwc/Makefile
+++ b/drivers/pci/controller/dwc/Makefile
@@ -35,6 +35,7 @@ obj-$(CONFIG_PCIE_TEGRA194) += pcie-tegra194.o
 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_ZX279128S) += pcie-zx279128s.o
 obj-$(CONFIG_PCIE_RCAR_GEN4) += pcie-rcar-gen4.o
 obj-$(CONFIG_PCIE_SPACEMIT_K1) += pcie-spacemit-k1.o
 obj-$(CONFIG_PCIE_STM32_HOST) += pcie-stm32.o
diff --git a/drivers/pci/controller/dwc/pcie-zx279128s.c b/drivers/pci/controller/dwc/pcie-zx279128s.c
new file mode 100644
index 0000000000..592a7f7c4e
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-zx279128s.c
@@ -0,0 +1,273 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * ZTE zx279128s PCIe host controller
+ *
+ * The SoC has two DesignWare PCIe controllers with one lane each. Their
+ * clocks, resets and PHYs are managed through the CCF, Reset and Generic PHY
+ * frameworks.
+ *
+ * Copyright (C) 2026 Navid Ghahremani <ghahramani.navid@gmail.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/pci.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+
+#include "pcie-designware.h"
+
+/* Controller wrapper ("ctrl" registers) */
+#define CTRL_CFG		0x000
+#define CTRL_CFG_EN		BIT(5)
+#define CTRL_STATUS		0x0dc
+#define CTRL_STATUS_LTSSM	GENMASK(22, 17)
+#define CTRL_LTSSM_L0		0x11
+
+struct zx279128s_pcie {
+	struct dw_pcie pci;
+	void __iomem *ctrl;
+	struct clk *clk;
+	struct reset_control *rst;
+	struct phy *phy;
+	struct gpio_desc *reset;
+	struct gpio_desc *enable;
+};
+
+#define to_zx279128s_pcie(x)	container_of(x, struct zx279128s_pcie, pci)
+
+/* The board reset lines are shared by both controllers, see host_init */
+static DEFINE_MUTEX(zx279128s_board_lock);
+static bool zx279128s_board_released;
+
+/* Limit the link to 2.5 GT/s and retrain it, as the vendor firmware does */
+static void zx279128s_pcie_force_gen1(struct dw_pcie *pci)
+{
+	u8 cap = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+	u32 val;
+	u16 val16;
+
+	if (WARN_ON(!cap))
+		return;
+
+	dw_pcie_dbi_ro_wr_en(pci);
+
+	/*
+	 * With L0s/L1 enabled the H3600 endpoints stop responding, so do not
+	 * advertise those states or clock power management until their wakeup
+	 * sequence is understood.
+	 */
+	val = dw_pcie_readl_dbi(pci, cap + PCI_EXP_LNKCAP);
+	val &= ~(PCI_EXP_LNKCAP_SLS | PCI_EXP_LNKCAP_ASPMS |
+		 PCI_EXP_LNKCAP_CLKPM);
+	val |= PCI_EXP_LNKCAP_SLS_2_5GB;
+	dw_pcie_writel_dbi(pci, cap + PCI_EXP_LNKCAP, val);
+
+	val16 = dw_pcie_readw_dbi(pci, cap + PCI_EXP_LNKCTL2);
+	val16 &= ~PCI_EXP_LNKCTL2_TLS;
+	val16 |= PCI_EXP_LNKCTL2_TLS_2_5GT;
+	dw_pcie_writew_dbi(pci, cap + PCI_EXP_LNKCTL2, val16);
+
+	val16 = dw_pcie_readw_dbi(pci, cap + PCI_EXP_LNKCTL);
+	val16 &= ~(PCI_EXP_LNKCTL_ASPMC | PCI_EXP_LNKCTL_CLKREQ_EN);
+	dw_pcie_writew_dbi(pci, cap + PCI_EXP_LNKCTL, val16 | PCI_EXP_LNKCTL_RL);
+
+	dw_pcie_dbi_ro_wr_dis(pci);
+}
+
+static int zx279128s_pcie_host_init(struct dw_pcie_rp *pp)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct zx279128s_pcie *pcie = to_zx279128s_pcie(pci);
+	int ret;
+
+	/*
+	 * The board lines reset a device that can sit behind both controllers
+	 * (the MT7915 of the H3600 uses both). The first controller to start
+	 * resets and releases it; the second must not reset it again.
+	 */
+	mutex_lock(&zx279128s_board_lock);
+	if (!zx279128s_board_released && (pcie->reset || pcie->enable)) {
+		ret = gpiod_direction_output(pcie->reset, 1);
+		if (ret)
+			goto err_unlock;
+		ret = gpiod_direction_output(pcie->enable, 0);
+		if (ret)
+			goto err_unlock;
+		msleep(100);
+		ret = gpiod_set_value_cansleep(pcie->reset, 0);
+		if (ret)
+			goto err_unlock;
+		ret = gpiod_set_value_cansleep(pcie->enable, 1);
+		if (ret)
+			goto err_unlock;
+		msleep(200);
+		zx279128s_board_released = true;
+	}
+
+	ret = phy_init(pcie->phy);
+	if (ret)
+		goto err_unlock;
+
+	ret = phy_power_on(pcie->phy);
+	if (ret)
+		goto err_phy_exit;
+
+	ret = reset_control_assert(pcie->rst);
+	if (ret)
+		goto err_phy_off;
+	fsleep(50);
+	ret = reset_control_deassert(pcie->rst);
+	if (ret)
+		goto err_phy_off;
+
+	mutex_unlock(&zx279128s_board_lock);
+
+	/* Enable the controller, which starts link training */
+	writel(readl(pcie->ctrl + CTRL_CFG) | CTRL_CFG_EN, pcie->ctrl + CTRL_CFG);
+
+	zx279128s_pcie_force_gen1(pci);
+
+	return 0;
+
+err_phy_off:
+	phy_power_off(pcie->phy);
+err_phy_exit:
+	phy_exit(pcie->phy);
+err_unlock:
+	mutex_unlock(&zx279128s_board_lock);
+	return dev_err_probe(pci->dev, ret, "failed to initialize PCIe\n");
+}
+
+static void zx279128s_pcie_host_deinit(struct dw_pcie_rp *pp)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct zx279128s_pcie *pcie = to_zx279128s_pcie(pci);
+
+	mutex_lock(&zx279128s_board_lock);
+	reset_control_assert(pcie->rst);
+	phy_power_off(pcie->phy);
+	phy_exit(pcie->phy);
+	mutex_unlock(&zx279128s_board_lock);
+}
+
+/*
+ * The Wi-Fi on the H3600 uses legacy INTx interrupts. Providing msi_init
+ * keeps the DWC core from setting up its internal MSI controller, which has
+ * not been tested on this SoC.
+ */
+static int zx279128s_pcie_msi_init(struct dw_pcie_rp *pp)
+{
+	return 0;
+}
+
+static const struct dw_pcie_host_ops zx279128s_pcie_host_ops = {
+	.init = zx279128s_pcie_host_init,
+	.deinit = zx279128s_pcie_host_deinit,
+	.msi_init = zx279128s_pcie_msi_init,
+};
+
+static int zx279128s_pcie_start_link(struct dw_pcie *pci)
+{
+	zx279128s_pcie_force_gen1(pci);
+
+	return 0;
+}
+
+static bool zx279128s_pcie_link_up(struct dw_pcie *pci)
+{
+	struct zx279128s_pcie *pcie = to_zx279128s_pcie(pci);
+	u32 val = readl(pcie->ctrl + CTRL_STATUS);
+
+	return FIELD_GET(CTRL_STATUS_LTSSM, val) == CTRL_LTSSM_L0;
+}
+
+static const struct dw_pcie_ops zx279128s_pcie_ops = {
+	.link_up = zx279128s_pcie_link_up,
+	.start_link = zx279128s_pcie_start_link,
+};
+
+static int zx279128s_pcie_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct zx279128s_pcie *pcie;
+	struct resource *res;
+
+	pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
+	if (!pcie)
+		return -ENOMEM;
+
+	pcie->pci.dev = dev;
+	pcie->pci.ops = &zx279128s_pcie_ops;
+	pcie->pci.pp.ops = &zx279128s_pcie_host_ops;
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ctrl");
+	pcie->ctrl = devm_ioremap_resource(dev, res);
+	if (IS_ERR(pcie->ctrl))
+		return PTR_ERR(pcie->ctrl);
+
+	pcie->clk = devm_clk_get_enabled(dev, "pcie");
+	if (IS_ERR(pcie->clk))
+		return dev_err_probe(dev, PTR_ERR(pcie->clk),
+				     "failed to get pcie clock\n");
+
+	pcie->rst = devm_reset_control_get_exclusive(dev, "core");
+	if (IS_ERR(pcie->rst))
+		pcie->rst = devm_reset_control_get_exclusive(dev, "pcie");
+	if (IS_ERR(pcie->rst))
+		return dev_err_probe(dev, PTR_ERR(pcie->rst),
+				     "failed to get pcie reset\n");
+
+	pcie->phy = devm_phy_get(dev, "pcie");
+	if (IS_ERR(pcie->phy))
+		return dev_err_probe(dev, PTR_ERR(pcie->phy),
+				     "failed to get pcie phy\n");
+
+	/*
+	 * Optional board lines to reset and enable endpoints. If already
+	 * claimed by another controller, treat -EBUSY as already managed.
+	 */
+	pcie->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_ASIS);
+	if (IS_ERR(pcie->reset)) {
+		if (PTR_ERR(pcie->reset) == -EBUSY)
+			pcie->reset = NULL;
+		else
+			return dev_err_probe(dev, PTR_ERR(pcie->reset),
+					     "failed to get the reset GPIO\n");
+	}
+
+	pcie->enable = devm_gpiod_get_optional(dev, "enable", GPIOD_ASIS);
+	if (IS_ERR(pcie->enable)) {
+		if (PTR_ERR(pcie->enable) == -EBUSY)
+			pcie->enable = NULL;
+		else
+			return dev_err_probe(dev, PTR_ERR(pcie->enable),
+					     "failed to get the enable GPIO\n");
+	}
+
+	return dw_pcie_host_init(&pcie->pci.pp);
+}
+
+static const struct of_device_id zx279128s_pcie_of_match[] = {
+	{ .compatible = "zte,zx279128s-pcie" },
+	{ }
+};
+
+static struct platform_driver zx279128s_pcie_driver = {
+	.probe = zx279128s_pcie_probe,
+	.driver = {
+		.name = "zx279128s-pcie",
+		.of_match_table = zx279128s_pcie_of_match,
+		.suppress_bind_attrs = true,
+	},
+};
+builtin_platform_driver(zx279128s_pcie_driver);
-- 
2.55.0

Re: [PATCH v2 13/24] PCI: dwc: Add ZTE ZX279128S host controller driver

[Bjorn Helgaas]

On Wed, Oct 14, 2026 at 08:20:53AM +0100, Navid Ghahremani wrote:
> Support the two Synopsys DesignWare PCIe host controllers of the
> ZTE ZX279128S SoC. The driver manages the controller clocks via CCF,
> asserts/deasserts resets via the Reset framework, and manages the
> analog SerDes and PLL via the Generic PHY framework.

Thanks for making the commit log complete.

Suggest making the subject line:

  PCI: zx: Add ...

or whatever tag you plan to use for the family of hardware this driver
will support over time.  Future changes to a driver typically don't
include "dwc" because the name of the driver itself is more useful.

I see "zx" used in the spi patch and "zte" used in the clk patch.
"zte" might be too generic; there may be future hardware that requires
a different driver.  "zx279128s" might be too specific; there will
likely be more SoCs that share this driver.  Same consideration
applies to filename, config symbol, struct and function names, etc.

> Signed-off-by: Navid Ghahremani <ghahramani.navid@gmail.com>
> ---
>  drivers/pci/controller/dwc/Kconfig          |  13 +
>  drivers/pci/controller/dwc/Makefile         |   1 +
>  drivers/pci/controller/dwc/pcie-zx279128s.c | 273 ++++++++++++++++++++
>  3 files changed, 287 insertions(+)
>  create mode 100644 drivers/pci/controller/dwc/pcie-zx279128s.c
> 
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index dcfbe7e229..3a4008be84 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -577,4 +577,17 @@ config PCIE_ULTRARISC
>  	  By default, this symbol is enabled when ARCH_ULTRARISC is active,
>  	  requiring no further configuration on that platform.
>  
> +config PCIE_ZX279128S
> +	bool "ZTE zx279128s PCIe controller"
> +	depends on ARCH_ZTE || COMPILE_TEST
> +	depends on PCI_MSI
> +	select GENERIC_PHY
> +	select PCIE_DW_HOST
> +	select RESET_CONTROLLER
> +	help
> +	  Say Y here to enable support for the two PCIe host controllers of
> +	  the ZTE zx279128s SoC. They are based on the Synopsys DesignWare
> +	  PCIe IP. On the ZTE ZXHN H3600 they connect the MT7915 Wi-Fi chip.
> +	  The driver only supports legacy INTx interrupts.

Capitalization of "zx279128s" here (menu item, help text, comment
below) should probably match the commit log and marketing materials
for this SoC.

The config symbol should use the same tag as the subject line.
PCIE_ZX279128S might be too specific if more SoCs might share this
driver.

>  endmenu
> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> index d4f88cfc62..379198ebd6 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -35,6 +35,7 @@ obj-$(CONFIG_PCIE_TEGRA194) += pcie-tegra194.o
>  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_ZX279128S) += pcie-zx279128s.o
>  obj-$(CONFIG_PCIE_RCAR_GEN4) += pcie-rcar-gen4.o
>  obj-$(CONFIG_PCIE_SPACEMIT_K1) += pcie-spacemit-k1.o
>  obj-$(CONFIG_PCIE_STM32_HOST) += pcie-stm32.o
> diff --git a/drivers/pci/controller/dwc/pcie-zx279128s.c b/drivers/pci/controller/dwc/pcie-zx279128s.c
> new file mode 100644
> index 0000000000..592a7f7c4e
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-zx279128s.c
> @@ -0,0 +1,273 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * ZTE zx279128s PCIe host controller

Re: [PATCH v2 13/24] PCI: dwc: Add ZTE ZX279128S host controller driver

[Stefan Dösinger]

[-- Attachment #1: Type: text/plain, Size: 987 bytes --]

Hi,

This is mostly a reply for Navid, not Bjorn, but since the topic of 
capitalization came up 

Am Mittwoch, 14. Oktober 2026, 15:19:54 Mitteleuropäische Sommerzeit schrieb 
Bjorn Helgaas:
> Capitalization of "zx279128s" here (menu item, help text, comment
> below) should probably match the commit log and marketing materials
> for this SoC.
> 
> The config symbol should use the same tag as the subject line.
> PCIE_ZX279128S might be too specific if more SoCs might share this
> driver.

For my little pet chip I decided to keep the letters lowercase because the SoC 
name is borderline unreadable. At least zx297520v3 has some minor visual 
distinction whereas ZX297520V3 does not. If somebody had asked me why I would 
have said the above and that Linux generally uses snake_case not CamelCase.

So I'd suggest to zse zx279128s rather than ZX279128S unless the symbol is 
capitalized anyway.

Now just wait for the zx234502 charger and zx234290 PMIC...
 

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 870 bytes --]

Re: [PATCH v2 12/24] dt-bindings: PCI: Add ZTE ZX279128S host controller

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] dt-bindings: pci: custom enable-gpios bypasses regulator framework
--

commit 4e7065d52cc5c001e2bd9b479582baa2dabea3c6
Author: Navid Ghahremani <ghahramani.navid@gmail.com>

dt-bindings: PCI: Add ZTE ZX279128S host controller

This commit adds the Device Tree binding for the DesignWare PCIe host
controller on the ZTE ZX279128S SoC. It defines the required registers,
interrupts, clocks, resets, and PHYs. It also introduces optional board-level
GPIOs for resets and enable lines.

> diff --git a/Documentation/devicetree/bindings/pci/zte,zx279128s-pcie.yaml b/Documentation/devicetree/bindings/pci/zte,zx279128s-pcie.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/zte,zx279128s-pcie.yaml
[ ... ]
> +  reset-gpios:
> +    maxItems: 1
> +    description: Optional board GPIO reset line for downstream endpoints.
> +
> +  enable-gpios:
> +    maxItems: 1
> +    description: Optional second board GPIO enable line driven together with reset-gpios.

[Severity: Low]
Does this custom enable-gpios property bypass the regulator framework?

In Device Tree, board power or enable lines should typically be modeled using
standard regulator bindings (such as vpcie-supply) linked to a regulator-fixed
node, rather than using hardcoded raw GPIO properties directly in the PCIe
controller node.

A review of existing PCI bindings shows no other uses of enable-gpios. Could
this be modeled with a standard regulator instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014072109.1759502-1-ghahramani.navid@gmail.com?part=12

Re: [PATCH v2 13/24] PCI: dwc: Add ZTE ZX279128S host controller driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] pci: dwc: zx279128s: -EPROBE_DEFER masked when requesting core reset
- [Medium] pci: dwc: zx279128s link training failure on concurrent probe
--

commit 31ef562bd53943175554ff3b4004b8759b588b24
Author: Navid Ghahremani <ghahramani.navid@gmail.com>
Subject: PCI: dwc: Add ZTE ZX279128S host controller driver

This commit adds support for the two Synopsys DesignWare PCIe host controllers
found on the ZTE ZX279128S SoC. It manages the clocks, resets, and PHYs using
standard kernel frameworks. The driver limits the link speed to Gen1 as done
by the vendor firmware.

> diff --git a/drivers/pci/controller/dwc/pcie-zx279128s.c b/drivers/pci/controller/dwc/pcie-zx279128s.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-zx279128s.c
[ ... ]
> +static int zx279128s_pcie_host_init(struct dw_pcie_rp *pp)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +	struct zx279128s_pcie *pcie = to_zx279128s_pcie(pci);
> +	int ret;
> +
> +	/*
> +	 * The board lines reset a device that can sit behind both controllers
> +	 * (the MT7915 of the H3600 uses both). The first controller to start
> +	 * resets and releases it; the second must not reset it again.
> +	 */
> +	mutex_lock(&zx279128s_board_lock);
> +	if (!zx279128s_board_released && (pcie->reset || pcie->enable)) {

[Severity: Medium]
If both PCIe controllers are probed concurrently (for example, via
asynchronous probing), could the second controller skip this reset sequence
and fail link training?

If the second controller requests the shared reset GPIO in
zx279128s_pcie_probe() and receives -EBUSY, its GPIO pointers will be set to
NULL. When it reaches this critical section before the first controller has
executed the reset toggle, it will evaluate pcie->reset || pcie->enable as
false.

This would cause it to skip the wait block, proceeding immediately to link
training while the endpoints are still in reset.

> +		ret = gpiod_direction_output(pcie->reset, 1);
> +		if (ret)
> +			goto err_unlock;
[ ... ]
> +static int zx279128s_pcie_probe(struct platform_device *pdev)
> +{
[ ... ]
> +	pcie->clk = devm_clk_get_enabled(dev, "pcie");
> +	if (IS_ERR(pcie->clk))
> +		return dev_err_probe(dev, PTR_ERR(pcie->clk),
> +				     "failed to get pcie clock\n");
> +
> +	pcie->rst = devm_reset_control_get_exclusive(dev, "core");
> +	if (IS_ERR(pcie->rst))
> +		pcie->rst = devm_reset_control_get_exclusive(dev, "pcie");
> +	if (IS_ERR(pcie->rst))
> +		return dev_err_probe(dev, PTR_ERR(pcie->rst),
> +				     "failed to get pcie reset\n");

[Severity: High]
Does this unconditional fallback logic mask probe deferrals?

If the reset controller is not yet ready, the initial request for the core
reset line returns -EPROBE_DEFER.

The fallback unconditionally requests the pcie reset line. If the device
tree only specifies core, the second request will return -ENOENT or -EINVAL.

This overwrites the original -EPROBE_DEFER, causing the controller to
permanently fail probing instead of deferring.

> +
> +	pcie->phy = devm_phy_get(dev, "pcie");
> +	if (IS_ERR(pcie->phy))
> +		return dev_err_probe(dev, PTR_ERR(pcie->phy),
> +				     "failed to get pcie phy\n");
> +
> +	/*
> +	 * Optional board lines to reset and enable endpoints. If already
> +	 * claimed by another controller, treat -EBUSY as already managed.
> +	 */
> +	pcie->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_ASIS);
> +	if (IS_ERR(pcie->reset)) {
> +		if (PTR_ERR(pcie->reset) == -EBUSY)
> +			pcie->reset = NULL;
> +		else
> +			return dev_err_probe(dev, PTR_ERR(pcie->reset),
> +					     "failed to get the reset GPIO\n");
> +	}
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014072109.1759502-1-ghahramani.navid@gmail.com?part=13