[PATCH v3 0/7] PCI: mediatek-gen3: add power control support

[PATCH v3 0/7] PCI: mediatek-gen3: add power control support

[Chen-Yu Tsai]

Hi folks,

This is v3 of my MediaTek PCIe gen3 controller driver power control
support series.

Changes since v2:
- Link to v2: https://lore.kernel.org/all/20260226092234.3859740-1-wenst@chromium.org/
- Made PCIE_MEDIATEK_GEN3 select PCI_PWRCTRL_SLOT, following existing
  examples

I do wonder why the existing ones don't select PCI_PWRCTRL instead.
As there are multiple providers, and now even the M.2 power sequencing
driver, I think either we enable the common ones by default, or let
the user pick and choose.

Changes since v1:
- Link to v1: https://lore.kernel.org/all/20260224071258.2654521-1-wenst@chromium.org/
- commit message for patch 3 was rewritten
Jianjun Wang was dropped from the recipients as the email was
bouncing.

This series adds power control support to the MediaTek PCIe gen3
controller driver. This allows proper modeling of WiFi and NVMe
adapters in the device tree and control over their power supplies.

Patch 1 through 4 are cleanups and minor improvements to the driver.

Patch 5 adds power control support using the new pwrctrl API to the
PCIe controller driver.

Patch 6 adds the WiFi and BT power supplies for the MT8195 Cherry design.
This is actually a M.2 E-key slot, but support for that is still WIP [1].
And even with it merged, support for the USB side still needs to be
figured out.

We can either merge this as is for now, and do another conversion later,
or just keep this one out. However this is still an improvement over
the current device tree, in which the power for the slot is always on
using a pinctrl setting.

Patch 7 adds the M.2 M-key NVMe slot found on MT8195 Dojo Chromebooks.
This change actually makes use of the M.2 pwrseq driver.


Please have a look.


Thanks
ChenYu

[1] https://lore.kernel.org/linux-pci/20260224-pci-m2-e-v5-0-dd9b9501d33c@oss.qualcomm.com/

Chen-Yu Tsai (7):
  PCI: mediatek-gen3: Clean up mtk_pcie_parse_port() with
    dev_err_probe()
  PCI: mediatek-gen3: Add error path for probe and resume driver
    callbacks
  PCI: mediatek-gen3: Split out device power helpers
  PCI: mediatek-gen3: Disable device if further setup fails
  PCI: mediatek-gen3: Integrate new pwrctrl API
  arm64: dts: mediatek: mt8195-cherry: add WiFi PCIe and BT USB power
    supplies
  arm64: dts: mediatek: mt8195-cherry-dojo: Describe M.2 M-key NVMe slot

 .../dts/mediatek/mt8195-cherry-dojo-r1.dts    |  38 ++++
 .../boot/dts/mediatek/mt8195-cherry.dtsi      |  47 +++--
 drivers/pci/controller/Kconfig                |   1 +
 drivers/pci/controller/pcie-mediatek-gen3.c   | 185 +++++++++++-------
 4 files changed, 184 insertions(+), 87 deletions(-)

-- 
2.53.0.473.g4a7958ca14-goog

[PATCH v3 1/7] PCI: mediatek-gen3: Clean up mtk_pcie_parse_port() with dev_err_probe()

[Chen-Yu Tsai]

mtk_pcie_parse_port() in the pcie-mediatek-gen driver has a bunch of

    if (err) {
    	dev_err(dev, "error message\n");
	return err; # or goto
    }

patterns.

Simplify these with dev_err_probe(). The system also gains proper
deferred probe messages that can be seen in

    /sys/kernel/debug/devices_deferred

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 drivers/pci/controller/pcie-mediatek-gen3.c | 36 ++++++---------------
 1 file changed, 10 insertions(+), 26 deletions(-)

diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
index 75ddb8bee168..1939cac995b5 100644
--- a/drivers/pci/controller/pcie-mediatek-gen3.c
+++ b/drivers/pci/controller/pcie-mediatek-gen3.c
@@ -876,10 +876,8 @@ static int mtk_pcie_parse_port(struct mtk_gen3_pcie *pcie)
 	if (!regs)
 		return -EINVAL;
 	pcie->base = devm_ioremap_resource(dev, regs);
-	if (IS_ERR(pcie->base)) {
-		dev_err(dev, "failed to map register base\n");
-		return PTR_ERR(pcie->base);
-	}
+	if (IS_ERR(pcie->base))
+		return dev_err_probe(dev, PTR_ERR(pcie->base), "failed to map register base\n");
 
 	pcie->reg_base = regs->start;
 
@@ -888,34 +886,20 @@ static int mtk_pcie_parse_port(struct mtk_gen3_pcie *pcie)
 
 	ret = devm_reset_control_bulk_get_optional_shared(dev, num_resets,
 							  pcie->phy_resets);
-	if (ret) {
-		dev_err(dev, "failed to get PHY bulk reset\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to get PHY bulk reset\n");
 
 	pcie->mac_reset = devm_reset_control_get_optional_exclusive(dev, "mac");
-	if (IS_ERR(pcie->mac_reset)) {
-		ret = PTR_ERR(pcie->mac_reset);
-		if (ret != -EPROBE_DEFER)
-			dev_err(dev, "failed to get MAC reset\n");
-
-		return ret;
-	}
+	if (IS_ERR(pcie->mac_reset))
+		return dev_err_probe(dev, PTR_ERR(pcie->mac_reset), "failed to get MAC reset\n");
 
 	pcie->phy = devm_phy_optional_get(dev, "pcie-phy");
-	if (IS_ERR(pcie->phy)) {
-		ret = PTR_ERR(pcie->phy);
-		if (ret != -EPROBE_DEFER)
-			dev_err(dev, "failed to get PHY\n");
-
-		return ret;
-	}
+	if (IS_ERR(pcie->phy))
+		return dev_err_probe(dev, PTR_ERR(pcie->phy), "failed to get PHY\n");
 
 	pcie->num_clks = devm_clk_bulk_get_all(dev, &pcie->clks);
-	if (pcie->num_clks < 0) {
-		dev_err(dev, "failed to get clocks\n");
-		return pcie->num_clks;
-	}
+	if (pcie->num_clks < 0)
+		return dev_err_probe(dev, pcie->num_clks, "failed to get clocks\n");
 
 	ret = of_property_read_u32(dev->of_node, "num-lanes", &num_lanes);
 	if (ret == 0) {
-- 
2.53.0.473.g4a7958ca14-goog

[PATCH v3 2/7] PCI: mediatek-gen3: Add error path for probe and resume driver callbacks

[Chen-Yu Tsai]

The probe and resume callbacks currently do teardown in the conditional
block directly. This is going to get ugly when the pwrctrl calls are
added.

Move the teardown to proper error cleanup paths.

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 drivers/pci/controller/pcie-mediatek-gen3.c | 22 ++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
index 1939cac995b5..f2bf4afbdc2f 100644
--- a/drivers/pci/controller/pcie-mediatek-gen3.c
+++ b/drivers/pci/controller/pcie-mediatek-gen3.c
@@ -1189,13 +1189,15 @@ static int mtk_pcie_probe(struct platform_device *pdev)
 	host->sysdata = pcie;
 
 	err = pci_host_probe(host);
-	if (err) {
-		mtk_pcie_irq_teardown(pcie);
-		mtk_pcie_power_down(pcie);
-		return err;
-	}
+	if (err)
+		goto err_teardown_irq_and_power_down;
 
 	return 0;
+
+err_teardown_irq_and_power_down:
+	mtk_pcie_irq_teardown(pcie);
+	mtk_pcie_power_down(pcie);
+	return err;
 }
 
 static void mtk_pcie_remove(struct platform_device *pdev)
@@ -1301,14 +1303,16 @@ static int mtk_pcie_resume_noirq(struct device *dev)
 		return err;
 
 	err = mtk_pcie_startup_port(pcie);
-	if (err) {
-		mtk_pcie_power_down(pcie);
-		return err;
-	}
+	if (err)
+		goto err_power_down;
 
 	mtk_pcie_irq_restore(pcie);
 
 	return 0;
+
+err_power_down:
+	mtk_pcie_power_down(pcie);
+	return err;
 }
 
 static const struct dev_pm_ops mtk_pcie_pm_ops = {
-- 
2.53.0.473.g4a7958ca14-goog

[PATCH v3 3/7] PCI: mediatek-gen3: Split out device power helpers

[Chen-Yu Tsai]

In preparation for adding full power on/off control with the pwrctrl
API, split out the existing code that only partially deals with device
power sequencing into separate helper functions. The existing code only
handles PERST#.

This is purely moving code around, and brings no functional changes.

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
Changes since v1:
- Updated commit message
---
 drivers/pci/controller/pcie-mediatek-gen3.c | 87 ++++++++++++---------
 1 file changed, 52 insertions(+), 35 deletions(-)

diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
index f2bf4afbdc2f..870d3b3db11d 100644
--- a/drivers/pci/controller/pcie-mediatek-gen3.c
+++ b/drivers/pci/controller/pcie-mediatek-gen3.c
@@ -403,6 +403,54 @@ static void mtk_pcie_enable_msi(struct mtk_gen3_pcie *pcie)
 	writel_relaxed(val, pcie->base + PCIE_INT_ENABLE_REG);
 }
 
+static int mtk_pcie_device_power_up(struct mtk_gen3_pcie *pcie)
+{
+	int err;
+	u32 val;
+
+	/*
+	 * Airoha EN7581 has a hw bug asserting/releasing PCIE_PE_RSTB signal
+	 * causing occasional PCIe link down. In order to overcome the issue,
+	 * PCIE_RSTB signals are not asserted/released at this stage and the
+	 * PCIe block is reset using en7523_reset_assert() and
+	 * en7581_pci_enable().
+	 */
+	if (!(pcie->soc->flags & SKIP_PCIE_RSTB)) {
+		/* Assert all reset signals */
+		val = readl_relaxed(pcie->base + PCIE_RST_CTRL_REG);
+		val |= PCIE_MAC_RSTB | PCIE_PHY_RSTB | PCIE_BRG_RSTB |
+		       PCIE_PE_RSTB;
+		writel_relaxed(val, pcie->base + PCIE_RST_CTRL_REG);
+
+		/*
+		 * Described in PCIe CEM specification revision 6.0.
+		 *
+		 * The deassertion of PERST# should be delayed 100ms (TPVPERL)
+		 * for the power and clock to become stable.
+		 */
+		msleep(PCIE_T_PVPERL_MS);
+
+		/* De-assert reset signals */
+		val &= ~(PCIE_MAC_RSTB | PCIE_PHY_RSTB | PCIE_BRG_RSTB |
+			 PCIE_PE_RSTB);
+		writel_relaxed(val, pcie->base + PCIE_RST_CTRL_REG);
+	}
+
+	return 0;
+}
+
+static void mtk_pcie_device_power_down(struct mtk_gen3_pcie *pcie)
+{
+	u32 val;
+
+	if (!(pcie->soc->flags & SKIP_PCIE_RSTB)) {
+		/* Assert the PERST# pin */
+		val = readl_relaxed(pcie->base + PCIE_RST_CTRL_REG);
+		val |= PCIE_PE_RSTB;
+		writel_relaxed(val, pcie->base + PCIE_RST_CTRL_REG);
+	}
+}
+
 static int mtk_pcie_startup_port(struct mtk_gen3_pcie *pcie)
 {
 	struct resource_entry *entry;
@@ -464,33 +512,9 @@ static int mtk_pcie_startup_port(struct mtk_gen3_pcie *pcie)
 	val |= PCIE_DISABLE_DVFSRC_VLT_REQ;
 	writel_relaxed(val, pcie->base + PCIE_MISC_CTRL_REG);
 
-	/*
-	 * Airoha EN7581 has a hw bug asserting/releasing PCIE_PE_RSTB signal
-	 * causing occasional PCIe link down. In order to overcome the issue,
-	 * PCIE_RSTB signals are not asserted/released at this stage and the
-	 * PCIe block is reset using en7523_reset_assert() and
-	 * en7581_pci_enable().
-	 */
-	if (!(pcie->soc->flags & SKIP_PCIE_RSTB)) {
-		/* Assert all reset signals */
-		val = readl_relaxed(pcie->base + PCIE_RST_CTRL_REG);
-		val |= PCIE_MAC_RSTB | PCIE_PHY_RSTB | PCIE_BRG_RSTB |
-		       PCIE_PE_RSTB;
-		writel_relaxed(val, pcie->base + PCIE_RST_CTRL_REG);
-
-		/*
-		 * Described in PCIe CEM specification revision 6.0.
-		 *
-		 * The deassertion of PERST# should be delayed 100ms (TPVPERL)
-		 * for the power and clock to become stable.
-		 */
-		msleep(PCIE_T_PVPERL_MS);
-
-		/* De-assert reset signals */
-		val &= ~(PCIE_MAC_RSTB | PCIE_PHY_RSTB | PCIE_BRG_RSTB |
-			 PCIE_PE_RSTB);
-		writel_relaxed(val, pcie->base + PCIE_RST_CTRL_REG);
-	}
+	err = mtk_pcie_device_power_up(pcie);
+	if (err)
+		return err;
 
 	/* Check if the link is up or not */
 	err = readl_poll_timeout(pcie->base + PCIE_LINK_STATUS_REG, val,
@@ -1269,7 +1293,6 @@ static int mtk_pcie_suspend_noirq(struct device *dev)
 {
 	struct mtk_gen3_pcie *pcie = dev_get_drvdata(dev);
 	int err;
-	u32 val;
 
 	/* Trigger link to L2 state */
 	err = mtk_pcie_turn_off_link(pcie);
@@ -1278,13 +1301,7 @@ static int mtk_pcie_suspend_noirq(struct device *dev)
 		return err;
 	}
 
-	if (!(pcie->soc->flags & SKIP_PCIE_RSTB)) {
-		/* Assert the PERST# pin */
-		val = readl_relaxed(pcie->base + PCIE_RST_CTRL_REG);
-		val |= PCIE_PE_RSTB;
-		writel_relaxed(val, pcie->base + PCIE_RST_CTRL_REG);
-	}
-
+	mtk_pcie_device_power_down(pcie);
 	dev_dbg(pcie->dev, "entered L2 states successfully");
 
 	mtk_pcie_irq_save(pcie);
-- 
2.53.0.473.g4a7958ca14-goog

[PATCH v3 4/7] PCI: mediatek-gen3: Disable device if further setup fails

[Chen-Yu Tsai]

If further setup fails after the device is powered on and link training
succeeds, we want to place the device back in a quiescence state to
avoid unintended activity and save power. This also helps with power
state tracking and balancing once pwrctrl API is integrated.

Power down the device in the error paths of mtk_pcie_startup_port() and
mtk_pcie_setup().

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 drivers/pci/controller/pcie-mediatek-gen3.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
index 870d3b3db11d..7459e1c1899d 100644
--- a/drivers/pci/controller/pcie-mediatek-gen3.c
+++ b/drivers/pci/controller/pcie-mediatek-gen3.c
@@ -531,7 +531,7 @@ static int mtk_pcie_startup_port(struct mtk_gen3_pcie *pcie)
 		dev_err(pcie->dev,
 			"PCIe link down, current LTSSM state: %s (%#x)\n",
 			ltssm_state, val);
-		return err;
+		goto err_power_down_device;
 	}
 
 	mtk_pcie_enable_msi(pcie);
@@ -556,10 +556,14 @@ static int mtk_pcie_startup_port(struct mtk_gen3_pcie *pcie)
 		err = mtk_pcie_set_trans_table(pcie, cpu_addr, pci_addr, size,
 					       type, &table_index);
 		if (err)
-			return err;
+			goto err_power_down_device;
 	}
 
 	return 0;
+
+err_power_down_device:
+	mtk_pcie_device_power_down(pcie);
+	return err;
 }
 
 #define MTK_MSI_FLAGS_REQUIRED (MSI_FLAG_USE_DEF_DOM_OPS	| \
@@ -1174,15 +1178,17 @@ static int mtk_pcie_setup(struct mtk_gen3_pcie *pcie)
 	/* Try link up */
 	err = mtk_pcie_startup_port(pcie);
 	if (err)
-		goto err_setup;
+		goto err_power_down;
 
 	err = mtk_pcie_setup_irq(pcie);
 	if (err)
-		goto err_setup;
+		goto err_device_power_off;
 
 	return 0;
 
-err_setup:
+err_device_power_off:
+	mtk_pcie_device_power_down(pcie);
+err_power_down:
 	mtk_pcie_power_down(pcie);
 
 	return err;
-- 
2.53.0.473.g4a7958ca14-goog

[PATCH v3 5/7] PCI: mediatek-gen3: Integrate new pwrctrl API

[Chen-Yu Tsai]

With the new PCI pwrctrl API and PCI slot binding and power drivers, we
now have a way to describe and power up WiFi/BT adapters connected
through a PCIe or M.2 slot, or exploded onto the mainboard itself.

Integrate the PCI pwrctrl API into the PCIe driver, so that power is
properly enabled before PCIe link training is done, allowing the
card to successfully be detected.

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
Changes since v2
- Added "select PCI_PWRCTRL_SLOT" to Kconfig to fix kernel test robot
  compilation error

I'm wondering why the two existing uses select PCI_PWRCTRL_SLOT and not
PCI_PWRCTRL though.
---
 drivers/pci/controller/Kconfig              |  1 +
 drivers/pci/controller/pcie-mediatek-gen3.c | 38 ++++++++++++++++-----
 2 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index 5aaed8ac6e44..e72ac6934379 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -222,6 +222,7 @@ config PCIE_MEDIATEK_GEN3
 	depends on ARCH_AIROHA || ARCH_MEDIATEK || COMPILE_TEST
 	depends on PCI_MSI
 	select IRQ_MSI_LIB
+	select PCI_PWRCTRL_SLOT
 	help
 	  Adds support for PCIe Gen3 MAC controller for MediaTek SoCs.
 	  This PCIe controller is compatible with Gen3, Gen2 and Gen1 speed,
diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
index 7459e1c1899d..93e591f788f7 100644
--- a/drivers/pci/controller/pcie-mediatek-gen3.c
+++ b/drivers/pci/controller/pcie-mediatek-gen3.c
@@ -22,6 +22,7 @@
 #include <linux/of_device.h>
 #include <linux/of_pci.h>
 #include <linux/pci.h>
+#include <linux/pci-pwrctrl.h>
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
 #include <linux/pm_domain.h>
@@ -421,15 +422,23 @@ static int mtk_pcie_device_power_up(struct mtk_gen3_pcie *pcie)
 		val |= PCIE_MAC_RSTB | PCIE_PHY_RSTB | PCIE_BRG_RSTB |
 		       PCIE_PE_RSTB;
 		writel_relaxed(val, pcie->base + PCIE_RST_CTRL_REG);
+	}
+
+	err = pci_pwrctrl_power_on_devices(pcie->dev);
+	if (err) {
+		dev_err(pcie->dev, "Failed to power on devices: %pe\n", ERR_PTR(err));
+		return err;
+	}
 
-		/*
-		 * Described in PCIe CEM specification revision 6.0.
-		 *
-		 * The deassertion of PERST# should be delayed 100ms (TPVPERL)
-		 * for the power and clock to become stable.
-		 */
-		msleep(PCIE_T_PVPERL_MS);
+	/*
+	 * Described in PCIe CEM specification revision 6.0.
+	 *
+	 * The deassertion of PERST# should be delayed 100ms (TPVPERL)
+	 * for the power and clock to become stable.
+	 */
+	msleep(PCIE_T_PVPERL_MS);
 
+	if (!(pcie->soc->flags & SKIP_PCIE_RSTB)) {
 		/* De-assert reset signals */
 		val &= ~(PCIE_MAC_RSTB | PCIE_PHY_RSTB | PCIE_BRG_RSTB |
 			 PCIE_PE_RSTB);
@@ -449,6 +458,8 @@ static void mtk_pcie_device_power_down(struct mtk_gen3_pcie *pcie)
 		val |= PCIE_PE_RSTB;
 		writel_relaxed(val, pcie->base + PCIE_RST_CTRL_REG);
 	}
+
+	pci_pwrctrl_power_off_devices(pcie->dev);
 }
 
 static int mtk_pcie_startup_port(struct mtk_gen3_pcie *pcie)
@@ -1211,9 +1222,13 @@ static int mtk_pcie_probe(struct platform_device *pdev)
 	pcie->soc = device_get_match_data(dev);
 	platform_set_drvdata(pdev, pcie);
 
+	err = pci_pwrctrl_create_devices(pcie->dev);
+	if (err)
+		return dev_err_probe(dev, err, "failed to create pwrctrl devices\n");
+
 	err = mtk_pcie_setup(pcie);
 	if (err)
-		return err;
+		goto err_destroy_pwrctrl;
 
 	host->ops = &mtk_pcie_ops;
 	host->sysdata = pcie;
@@ -1226,7 +1241,12 @@ static int mtk_pcie_probe(struct platform_device *pdev)
 
 err_teardown_irq_and_power_down:
 	mtk_pcie_irq_teardown(pcie);
+	mtk_pcie_device_power_down(pcie);
 	mtk_pcie_power_down(pcie);
+err_destroy_pwrctrl:
+	if (err != -EPROBE_DEFER)
+		pci_pwrctrl_destroy_devices(pcie->dev);
+
 	return err;
 }
 
@@ -1241,7 +1261,9 @@ static void mtk_pcie_remove(struct platform_device *pdev)
 	pci_unlock_rescan_remove();
 
 	mtk_pcie_irq_teardown(pcie);
+	pci_pwrctrl_power_off_devices(pcie->dev);
 	mtk_pcie_power_down(pcie);
+	pci_pwrctrl_destroy_devices(pcie->dev);
 }
 
 static void mtk_pcie_irq_save(struct mtk_gen3_pcie *pcie)
-- 
2.53.0.473.g4a7958ca14-goog

Re: [PATCH v3 5/7] PCI: mediatek-gen3: Integrate new pwrctrl API

[Bjorn Helgaas]

On Mon, Mar 02, 2026 at 01:31:05PM +0800, Chen-Yu Tsai wrote:
> With the new PCI pwrctrl API and PCI slot binding and power drivers, we
> now have a way to describe and power up WiFi/BT adapters connected
> through a PCIe or M.2 slot, or exploded onto the mainboard itself.
> 
> Integrate the PCI pwrctrl API into the PCIe driver, so that power is
> properly enabled before PCIe link training is done, allowing the
> card to successfully be detected.
> ...

> @@ -1211,9 +1222,13 @@ static int mtk_pcie_probe(struct platform_device *pdev)
>  	pcie->soc = device_get_match_data(dev);
>  	platform_set_drvdata(pdev, pcie);
>  
> +	err = pci_pwrctrl_create_devices(pcie->dev);

AFAICT this has nothing at all to do with mediatek-gen3; it only has
to do with the fact that the platform provides a way to control the
main power to endpoints that happen to be connected to it.  Are we
going to have to add this in every single native host controller
driver?

Re: [PATCH v3 5/7] PCI: mediatek-gen3: Integrate new pwrctrl API

[Manivannan Sadhasivam]

On Fri, Mar 06, 2026 at 01:21:34PM -0600, Bjorn Helgaas wrote:
> On Mon, Mar 02, 2026 at 01:31:05PM +0800, Chen-Yu Tsai wrote:
> > With the new PCI pwrctrl API and PCI slot binding and power drivers, we
> > now have a way to describe and power up WiFi/BT adapters connected
> > through a PCIe or M.2 slot, or exploded onto the mainboard itself.
> > 
> > Integrate the PCI pwrctrl API into the PCIe driver, so that power is
> > properly enabled before PCIe link training is done, allowing the
> > card to successfully be detected.
> > ...
> 
> > @@ -1211,9 +1222,13 @@ static int mtk_pcie_probe(struct platform_device *pdev)
> >  	pcie->soc = device_get_match_data(dev);
> >  	platform_set_drvdata(pdev, pcie);
> >  
> > +	err = pci_pwrctrl_create_devices(pcie->dev);
> 
> AFAICT this has nothing at all to do with mediatek-gen3; it only has
> to do with the fact that the platform provides a way to control the
> main power to endpoints that happen to be connected to it.  Are we
> going to have to add this in every single native host controller
> driver?

Yes, I want this feature to be opt-in because, a lot of controller drivers
handle the device power themselves. We can convert these drivers one-by-one and
when atleast 50% of them are converted, we can move these calls to generic APIs
such as pci_host_probe().

- Mani

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

Re: [PATCH v3 5/7] PCI: mediatek-gen3: Integrate new pwrctrl API

[Bjorn Helgaas]

On Mon, Mar 02, 2026 at 01:31:05PM +0800, Chen-Yu Tsai wrote:
> With the new PCI pwrctrl API and PCI slot binding and power drivers, we
> now have a way to describe and power up WiFi/BT adapters connected
> through a PCIe or M.2 slot, or exploded onto the mainboard itself.
> 
> Integrate the PCI pwrctrl API into the PCIe driver, so that power is
> properly enabled before PCIe link training is done, allowing the
> card to successfully be detected.
> 
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> ---
> Changes since v2
> - Added "select PCI_PWRCTRL_SLOT" to Kconfig to fix kernel test robot
>   compilation error
> 
> I'm wondering why the two existing uses select PCI_PWRCTRL_SLOT and not
> PCI_PWRCTRL though.
> ---
>  drivers/pci/controller/Kconfig              |  1 +
>  drivers/pci/controller/pcie-mediatek-gen3.c | 38 ++++++++++++++++-----
>  2 files changed, 31 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> index 5aaed8ac6e44..e72ac6934379 100644
> --- a/drivers/pci/controller/Kconfig
> +++ b/drivers/pci/controller/Kconfig
> @@ -222,6 +222,7 @@ config PCIE_MEDIATEK_GEN3
>  	depends on ARCH_AIROHA || ARCH_MEDIATEK || COMPILE_TEST
>  	depends on PCI_MSI
>  	select IRQ_MSI_LIB
> +	select PCI_PWRCTRL_SLOT
>  	help
>  	  Adds support for PCIe Gen3 MAC controller for MediaTek SoCs.
>  	  This PCIe controller is compatible with Gen3, Gen2 and Gen1 speed,
> diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
> index 7459e1c1899d..93e591f788f7 100644
> --- a/drivers/pci/controller/pcie-mediatek-gen3.c
> +++ b/drivers/pci/controller/pcie-mediatek-gen3.c
> @@ -22,6 +22,7 @@
>  #include <linux/of_device.h>
>  #include <linux/of_pci.h>
>  #include <linux/pci.h>
> +#include <linux/pci-pwrctrl.h>
>  #include <linux/phy/phy.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_domain.h>
> @@ -421,15 +422,23 @@ static int mtk_pcie_device_power_up(struct mtk_gen3_pcie *pcie)
>  		val |= PCIE_MAC_RSTB | PCIE_PHY_RSTB | PCIE_BRG_RSTB |
>  		       PCIE_PE_RSTB;
>  		writel_relaxed(val, pcie->base + PCIE_RST_CTRL_REG);
> +	}
> +
> +	err = pci_pwrctrl_power_on_devices(pcie->dev);
> +	if (err) {
> +		dev_err(pcie->dev, "Failed to power on devices: %pe\n", ERR_PTR(err));
> +		return err;
> +	}
>  
> -		/*
> -		 * Described in PCIe CEM specification revision 6.0.
> -		 *
> -		 * The deassertion of PERST# should be delayed 100ms (TPVPERL)
> -		 * for the power and clock to become stable.
> -		 */
> -		msleep(PCIE_T_PVPERL_MS);
> +	/*
> +	 * Described in PCIe CEM specification revision 6.0.
> +	 *
> +	 * The deassertion of PERST# should be delayed 100ms (TPVPERL)
> +	 * for the power and clock to become stable.
> +	 */
> +	msleep(PCIE_T_PVPERL_MS);
>  
> +	if (!(pcie->soc->flags & SKIP_PCIE_RSTB)) {
>  		/* De-assert reset signals */
>  		val &= ~(PCIE_MAC_RSTB | PCIE_PHY_RSTB | PCIE_BRG_RSTB |
>  			 PCIE_PE_RSTB);
> @@ -449,6 +458,8 @@ static void mtk_pcie_device_power_down(struct mtk_gen3_pcie *pcie)
>  		val |= PCIE_PE_RSTB;
>  		writel_relaxed(val, pcie->base + PCIE_RST_CTRL_REG);
>  	}
> +
> +	pci_pwrctrl_power_off_devices(pcie->dev);
>  }
>  
>  static int mtk_pcie_startup_port(struct mtk_gen3_pcie *pcie)
> @@ -1211,9 +1222,13 @@ static int mtk_pcie_probe(struct platform_device *pdev)
>  	pcie->soc = device_get_match_data(dev);
>  	platform_set_drvdata(pdev, pcie);
>  
> +	err = pci_pwrctrl_create_devices(pcie->dev);
> +	if (err)
> +		return dev_err_probe(dev, err, "failed to create pwrctrl devices\n");
> +
>  	err = mtk_pcie_setup(pcie);
>  	if (err)
> -		return err;
> +		goto err_destroy_pwrctrl;
>  
>  	host->ops = &mtk_pcie_ops;
>  	host->sysdata = pcie;
> @@ -1226,7 +1241,12 @@ static int mtk_pcie_probe(struct platform_device *pdev)
>  
>  err_teardown_irq_and_power_down:
>  	mtk_pcie_irq_teardown(pcie);
> +	mtk_pcie_device_power_down(pcie);

So now we have:

  mtk_pcie_probe
    mtk_pcie_setup
      mtk_pcie_startup_port
        mtk_pcie_device_power_up      <-- power up
        mtk_pcie_device_power_down    # error path
      mtk_pcie_setup_irq              # set up controller IRQ
      mtk_pcie_device_power_down      # if mtk_pcie_setup_irq() failed
    pci_host_probe                    # enumerate downstream devices
    mtk_pcie_device_power_down        # if pci_host_probe() failed

I think this is kind of a mess because mtk_pcie_device_power_down() is
called from so many places, and some of them aren't connected to the
mtk_pcie_device_power_up().

In mtk_pcie_setup(), mtk_pcie_setup_irq() only deals with the
*controller* IRQ and has nothing to do with the downstream devices.  I
think mtk_pcie_setup_irq() should be done before
mtk_pcie_startup_port() so we can abort before even powering up those
devices.

In mtk_pcie_startup_port(), mtk_pcie_enable_msi() and the PCIe
translation window setup also don't have anything to do with the
downstream devices, so I think they should be done before 
calling mtk_pcie_device_power_up().  The only thing there that needs
the downstream devices powered up is waiting for the link to come up.

>  	mtk_pcie_power_down(pcie);
> +err_destroy_pwrctrl:
> +	if (err != -EPROBE_DEFER)
> +		pci_pwrctrl_destroy_devices(pcie->dev);
> +
>  	return err;
>  }
>  
> @@ -1241,7 +1261,9 @@ static void mtk_pcie_remove(struct platform_device *pdev)
>  	pci_unlock_rescan_remove();
>  
>  	mtk_pcie_irq_teardown(pcie);
> +	pci_pwrctrl_power_off_devices(pcie->dev);
>  	mtk_pcie_power_down(pcie);
> +	pci_pwrctrl_destroy_devices(pcie->dev);
>  }
>  
>  static void mtk_pcie_irq_save(struct mtk_gen3_pcie *pcie)
> -- 
> 2.53.0.473.g4a7958ca14-goog
> 

Re: [PATCH v3 5/7] PCI: mediatek-gen3: Integrate new pwrctrl API

[Chen-Yu Tsai]

On Sat, Mar 7, 2026 at 4:59 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Mon, Mar 02, 2026 at 01:31:05PM +0800, Chen-Yu Tsai wrote:
> > With the new PCI pwrctrl API and PCI slot binding and power drivers, we
> > now have a way to describe and power up WiFi/BT adapters connected
> > through a PCIe or M.2 slot, or exploded onto the mainboard itself.
> >
> > Integrate the PCI pwrctrl API into the PCIe driver, so that power is
> > properly enabled before PCIe link training is done, allowing the
> > card to successfully be detected.
> >
> > Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> > Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> > Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
> > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> > ---
> > Changes since v2
> > - Added "select PCI_PWRCTRL_SLOT" to Kconfig to fix kernel test robot
> >   compilation error
> >
> > I'm wondering why the two existing uses select PCI_PWRCTRL_SLOT and not
> > PCI_PWRCTRL though.
> > ---
> >  drivers/pci/controller/Kconfig              |  1 +
> >  drivers/pci/controller/pcie-mediatek-gen3.c | 38 ++++++++++++++++-----
> >  2 files changed, 31 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> > index 5aaed8ac6e44..e72ac6934379 100644
> > --- a/drivers/pci/controller/Kconfig
> > +++ b/drivers/pci/controller/Kconfig
> > @@ -222,6 +222,7 @@ config PCIE_MEDIATEK_GEN3
> >       depends on ARCH_AIROHA || ARCH_MEDIATEK || COMPILE_TEST
> >       depends on PCI_MSI
> >       select IRQ_MSI_LIB
> > +     select PCI_PWRCTRL_SLOT
> >       help
> >         Adds support for PCIe Gen3 MAC controller for MediaTek SoCs.
> >         This PCIe controller is compatible with Gen3, Gen2 and Gen1 speed,
> > diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
> > index 7459e1c1899d..93e591f788f7 100644
> > --- a/drivers/pci/controller/pcie-mediatek-gen3.c
> > +++ b/drivers/pci/controller/pcie-mediatek-gen3.c
> > @@ -22,6 +22,7 @@
> >  #include <linux/of_device.h>
> >  #include <linux/of_pci.h>
> >  #include <linux/pci.h>
> > +#include <linux/pci-pwrctrl.h>
> >  #include <linux/phy/phy.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/pm_domain.h>
> > @@ -421,15 +422,23 @@ static int mtk_pcie_device_power_up(struct mtk_gen3_pcie *pcie)
> >               val |= PCIE_MAC_RSTB | PCIE_PHY_RSTB | PCIE_BRG_RSTB |
> >                      PCIE_PE_RSTB;
> >               writel_relaxed(val, pcie->base + PCIE_RST_CTRL_REG);
> > +     }
> > +
> > +     err = pci_pwrctrl_power_on_devices(pcie->dev);
> > +     if (err) {
> > +             dev_err(pcie->dev, "Failed to power on devices: %pe\n", ERR_PTR(err));
> > +             return err;
> > +     }
> >
> > -             /*
> > -              * Described in PCIe CEM specification revision 6.0.
> > -              *
> > -              * The deassertion of PERST# should be delayed 100ms (TPVPERL)
> > -              * for the power and clock to become stable.
> > -              */
> > -             msleep(PCIE_T_PVPERL_MS);
> > +     /*
> > +      * Described in PCIe CEM specification revision 6.0.
> > +      *
> > +      * The deassertion of PERST# should be delayed 100ms (TPVPERL)
> > +      * for the power and clock to become stable.
> > +      */
> > +     msleep(PCIE_T_PVPERL_MS);
> >
> > +     if (!(pcie->soc->flags & SKIP_PCIE_RSTB)) {
> >               /* De-assert reset signals */
> >               val &= ~(PCIE_MAC_RSTB | PCIE_PHY_RSTB | PCIE_BRG_RSTB |
> >                        PCIE_PE_RSTB);
> > @@ -449,6 +458,8 @@ static void mtk_pcie_device_power_down(struct mtk_gen3_pcie *pcie)
> >               val |= PCIE_PE_RSTB;
> >               writel_relaxed(val, pcie->base + PCIE_RST_CTRL_REG);
> >       }
> > +
> > +     pci_pwrctrl_power_off_devices(pcie->dev);
> >  }
> >
> >  static int mtk_pcie_startup_port(struct mtk_gen3_pcie *pcie)
> > @@ -1211,9 +1222,13 @@ static int mtk_pcie_probe(struct platform_device *pdev)
> >       pcie->soc = device_get_match_data(dev);
> >       platform_set_drvdata(pdev, pcie);
> >
> > +     err = pci_pwrctrl_create_devices(pcie->dev);
> > +     if (err)
> > +             return dev_err_probe(dev, err, "failed to create pwrctrl devices\n");
> > +
> >       err = mtk_pcie_setup(pcie);
> >       if (err)
> > -             return err;
> > +             goto err_destroy_pwrctrl;
> >
> >       host->ops = &mtk_pcie_ops;
> >       host->sysdata = pcie;
> > @@ -1226,7 +1241,12 @@ static int mtk_pcie_probe(struct platform_device *pdev)
> >
> >  err_teardown_irq_and_power_down:
> >       mtk_pcie_irq_teardown(pcie);
> > +     mtk_pcie_device_power_down(pcie);
>
> So now we have:
>
>   mtk_pcie_probe
>     mtk_pcie_setup
>       mtk_pcie_startup_port
>         mtk_pcie_device_power_up      <-- power up
>         mtk_pcie_device_power_down    # error path
>       mtk_pcie_setup_irq              # set up controller IRQ
>       mtk_pcie_device_power_down      # if mtk_pcie_setup_irq() failed
>     pci_host_probe                    # enumerate downstream devices
>     mtk_pcie_device_power_down        # if pci_host_probe() failed
>
> I think this is kind of a mess because mtk_pcie_device_power_down() is
> called from so many places, and some of them aren't connected to the
> mtk_pcie_device_power_up().
>
> In mtk_pcie_setup(), mtk_pcie_setup_irq() only deals with the
> *controller* IRQ and has nothing to do with the downstream devices.  I
> think mtk_pcie_setup_irq() should be done before
> mtk_pcie_startup_port() so we can abort before even powering up those
> devices.

Makes sense to me. I think it can even be moved outside of mtk_pcie_setup()?
That way we don't have mtk_pcie_irq_teardown() in two error paths?

I can send a follow up patch for that.

> In mtk_pcie_startup_port(), mtk_pcie_enable_msi() and the PCIe
> translation window setup also don't have anything to do with the
> downstream devices, so I think they should be done before
> calling mtk_pcie_device_power_up().  The only thing there that needs
> the downstream devices powered up is waiting for the link to come up.

My guess is that if the link up fails, then mtk_pcie_startup_port() is
going to error out anyway, so it maybe made sense to make sure a device
is actually present before doing any more work.

We will probably have to ask MediaTek what the original intent was.


ChenYu

> >       mtk_pcie_power_down(pcie);
> > +err_destroy_pwrctrl:
> > +     if (err != -EPROBE_DEFER)
> > +             pci_pwrctrl_destroy_devices(pcie->dev);
> > +
> >       return err;
> >  }
> >
> > @@ -1241,7 +1261,9 @@ static void mtk_pcie_remove(struct platform_device *pdev)
> >       pci_unlock_rescan_remove();
> >
> >       mtk_pcie_irq_teardown(pcie);
> > +     pci_pwrctrl_power_off_devices(pcie->dev);
> >       mtk_pcie_power_down(pcie);
> > +     pci_pwrctrl_destroy_devices(pcie->dev);
> >  }
> >
> >  static void mtk_pcie_irq_save(struct mtk_gen3_pcie *pcie)
> > --
> > 2.53.0.473.g4a7958ca14-goog
> >

Re: [PATCH v3 5/7] PCI: mediatek-gen3: Integrate new pwrctrl API

[Bjorn Helgaas]

On Mon, Mar 09, 2026 at 01:24:34PM +0800, Chen-Yu Tsai wrote:
> On Sat, Mar 7, 2026 at 4:59 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > On Mon, Mar 02, 2026 at 01:31:05PM +0800, Chen-Yu Tsai wrote:
> > > With the new PCI pwrctrl API and PCI slot binding and power drivers, we
> > > now have a way to describe and power up WiFi/BT adapters connected
> > > through a PCIe or M.2 slot, or exploded onto the mainboard itself.
> ...

> > So now we have:
> >
> >   mtk_pcie_probe
> >     mtk_pcie_setup
> >       mtk_pcie_startup_port
> >         mtk_pcie_device_power_up      <-- power up
> >         mtk_pcie_device_power_down    # error path
> >       mtk_pcie_setup_irq              # set up controller IRQ
> >       mtk_pcie_device_power_down      # if mtk_pcie_setup_irq() failed
> >     pci_host_probe                    # enumerate downstream devices
> >     mtk_pcie_device_power_down        # if pci_host_probe() failed
> >
> > I think this is kind of a mess because mtk_pcie_device_power_down() is
> > called from so many places, and some of them aren't connected to the
> > mtk_pcie_device_power_up().
> >
> > In mtk_pcie_setup(), mtk_pcie_setup_irq() only deals with the
> > *controller* IRQ and has nothing to do with the downstream devices.  I
> > think mtk_pcie_setup_irq() should be done before
> > mtk_pcie_startup_port() so we can abort before even powering up those
> > devices.
> 
> Makes sense to me. I think it can even be moved outside of mtk_pcie_setup()?
> That way we don't have mtk_pcie_irq_teardown() in two error paths?
> 
> I can send a follow up patch for that.

What if we do those cleanups first?  That would make this patch quite
a bit simpler and we wouldn't have to undo things.  It would be a
shame to have a complicated patch to add this stuff, then another
complicated patch to clean it up.

> > In mtk_pcie_startup_port(), mtk_pcie_enable_msi() and the PCIe
> > translation window setup also don't have anything to do with the
> > downstream devices, so I think they should be done before
> > calling mtk_pcie_device_power_up().  The only thing there that needs
> > the downstream devices powered up is waiting for the link to come up.
> 
> My guess is that if the link up fails, then mtk_pcie_startup_port() is
> going to error out anyway, so it maybe made sense to make sure a device
> is actually present before doing any more work.

Doesn't sound very convincing to me.  mtk_pcie_set_trans_table() and
the window setup are local things with no timeouts or anything.  I
think the logical structure is way more important than some kind of
hand-crafted work avoidance.

Separating the PCIe controller setup from things related to the link
and downstream devices is pretty high on my list.

Bjorn

Re: [PATCH v3 5/7] PCI: mediatek-gen3: Integrate new pwrctrl API

[Chen-Yu Tsai]

On Tue, Mar 10, 2026 at 5:50 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Mon, Mar 09, 2026 at 01:24:34PM +0800, Chen-Yu Tsai wrote:
> > On Sat, Mar 7, 2026 at 4:59 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > > On Mon, Mar 02, 2026 at 01:31:05PM +0800, Chen-Yu Tsai wrote:
> > > > With the new PCI pwrctrl API and PCI slot binding and power drivers, we
> > > > now have a way to describe and power up WiFi/BT adapters connected
> > > > through a PCIe or M.2 slot, or exploded onto the mainboard itself.
> > ...
>
> > > So now we have:
> > >
> > >   mtk_pcie_probe
> > >     mtk_pcie_setup
> > >       mtk_pcie_startup_port
> > >         mtk_pcie_device_power_up      <-- power up
> > >         mtk_pcie_device_power_down    # error path
> > >       mtk_pcie_setup_irq              # set up controller IRQ
> > >       mtk_pcie_device_power_down      # if mtk_pcie_setup_irq() failed
> > >     pci_host_probe                    # enumerate downstream devices
> > >     mtk_pcie_device_power_down        # if pci_host_probe() failed
> > >
> > > I think this is kind of a mess because mtk_pcie_device_power_down() is
> > > called from so many places, and some of them aren't connected to the
> > > mtk_pcie_device_power_up().
> > >
> > > In mtk_pcie_setup(), mtk_pcie_setup_irq() only deals with the
> > > *controller* IRQ and has nothing to do with the downstream devices.  I
> > > think mtk_pcie_setup_irq() should be done before
> > > mtk_pcie_startup_port() so we can abort before even powering up those
> > > devices.
> >
> > Makes sense to me. I think it can even be moved outside of mtk_pcie_setup()?
> > That way we don't have mtk_pcie_irq_teardown() in two error paths?
> >
> > I can send a follow up patch for that.
>
> What if we do those cleanups first?  That would make this patch quite
> a bit simpler and we wouldn't have to undo things.  It would be a
> shame to have a complicated patch to add this stuff, then another
> complicated patch to clean it up.

Sure.

> > > In mtk_pcie_startup_port(), mtk_pcie_enable_msi() and the PCIe
> > > translation window setup also don't have anything to do with the
> > > downstream devices, so I think they should be done before
> > > calling mtk_pcie_device_power_up().  The only thing there that needs
> > > the downstream devices powered up is waiting for the link to come up.
> >
> > My guess is that if the link up fails, then mtk_pcie_startup_port() is
> > going to error out anyway, so it maybe made sense to make sure a device
> > is actually present before doing any more work.
>
> Doesn't sound very convincing to me.  mtk_pcie_set_trans_table() and
> the window setup are local things with no timeouts or anything.  I
> think the logical structure is way more important than some kind of
> hand-crafted work avoidance.

I understand. I merely meant that I have no actual knowledge of why it
happened, or if there are any concerns from the hardware side.

> Separating the PCIe controller setup from things related to the link
> and downstream devices is pretty high on my list.

Makes sense to me.

I'll send an updated version.


Thanks
ChenYu

Re: [PATCH v3 5/7] PCI: mediatek-gen3: Integrate new pwrctrl API

[Chen-Yu Tsai]

On Tue, Mar 10, 2026 at 12:49 PM Chen-Yu Tsai <wenst@chromium.org> wrote:
>
> On Tue, Mar 10, 2026 at 5:50 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> >
> > On Mon, Mar 09, 2026 at 01:24:34PM +0800, Chen-Yu Tsai wrote:
> > > On Sat, Mar 7, 2026 at 4:59 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > > > On Mon, Mar 02, 2026 at 01:31:05PM +0800, Chen-Yu Tsai wrote:
> > > > > With the new PCI pwrctrl API and PCI slot binding and power drivers, we
> > > > > now have a way to describe and power up WiFi/BT adapters connected
> > > > > through a PCIe or M.2 slot, or exploded onto the mainboard itself.
> > > ...
> >
> > > > So now we have:
> > > >
> > > >   mtk_pcie_probe
> > > >     mtk_pcie_setup
> > > >       mtk_pcie_startup_port
> > > >         mtk_pcie_device_power_up      <-- power up
> > > >         mtk_pcie_device_power_down    # error path
> > > >       mtk_pcie_setup_irq              # set up controller IRQ
> > > >       mtk_pcie_device_power_down      # if mtk_pcie_setup_irq() failed
> > > >     pci_host_probe                    # enumerate downstream devices
> > > >     mtk_pcie_device_power_down        # if pci_host_probe() failed
> > > >
> > > > I think this is kind of a mess because mtk_pcie_device_power_down() is
> > > > called from so many places, and some of them aren't connected to the
> > > > mtk_pcie_device_power_up().
> > > >
> > > > In mtk_pcie_setup(), mtk_pcie_setup_irq() only deals with the
> > > > *controller* IRQ and has nothing to do with the downstream devices.  I
> > > > think mtk_pcie_setup_irq() should be done before
> > > > mtk_pcie_startup_port() so we can abort before even powering up those
> > > > devices.
> > >
> > > Makes sense to me. I think it can even be moved outside of mtk_pcie_setup()?
> > > That way we don't have mtk_pcie_irq_teardown() in two error paths?
> > >
> > > I can send a follow up patch for that.
> >
> > What if we do those cleanups first?  That would make this patch quite
> > a bit simpler and we wouldn't have to undo things.  It would be a
> > shame to have a complicated patch to add this stuff, then another
> > complicated patch to clean it up.
>
> Sure.
>
> > > > In mtk_pcie_startup_port(), mtk_pcie_enable_msi() and the PCIe
> > > > translation window setup also don't have anything to do with the
> > > > downstream devices, so I think they should be done before
> > > > calling mtk_pcie_device_power_up().  The only thing there that needs
> > > > the downstream devices powered up is waiting for the link to come up.
> > >
> > > My guess is that if the link up fails, then mtk_pcie_startup_port() is
> > > going to error out anyway, so it maybe made sense to make sure a device
> > > is actually present before doing any more work.
> >
> > Doesn't sound very convincing to me.  mtk_pcie_set_trans_table() and
> > the window setup are local things with no timeouts or anything.  I
> > think the logical structure is way more important than some kind of
> > hand-crafted work avoidance.
>
> I understand. I merely meant that I have no actual knowledge of why it
> happened, or if there are any concerns from the hardware side.
>
> > Separating the PCIe controller setup from things related to the link
> > and downstream devices is pretty high on my list.
>
> Makes sense to me.
>
> I'll send an updated version.

I'm confused. I see this version in -next with the minor fixups from
Bjorn. Do you want me to do the cleanups on top of them?


ChenYu

Re: [PATCH v3 5/7] PCI: mediatek-gen3: Integrate new pwrctrl API

[Bjorn Helgaas]

On Fri, Mar 20, 2026 at 02:10:03PM +0800, Chen-Yu Tsai wrote:
> On Tue, Mar 10, 2026 at 12:49 PM Chen-Yu Tsai <wenst@chromium.org> wrote:
> > On Tue, Mar 10, 2026 at 5:50 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > > On Mon, Mar 09, 2026 at 01:24:34PM +0800, Chen-Yu Tsai wrote:
> > > > On Sat, Mar 7, 2026 at 4:59 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > > > > On Mon, Mar 02, 2026 at 01:31:05PM +0800, Chen-Yu Tsai wrote:
> > > > > > With the new PCI pwrctrl API and PCI slot binding and power drivers, we
> > > > > > now have a way to describe and power up WiFi/BT adapters connected
> > > > > > through a PCIe or M.2 slot, or exploded onto the mainboard itself.
> > > > ...
> > >
> > > > > So now we have:
> > > > >
> > > > >   mtk_pcie_probe
> > > > >     mtk_pcie_setup
> > > > >       mtk_pcie_startup_port
> > > > >         mtk_pcie_device_power_up      <-- power up
> > > > >         mtk_pcie_device_power_down    # error path
> > > > >       mtk_pcie_setup_irq              # set up controller IRQ
> > > > >       mtk_pcie_device_power_down      # if mtk_pcie_setup_irq() failed
> > > > >     pci_host_probe                    # enumerate downstream devices
> > > > >     mtk_pcie_device_power_down        # if pci_host_probe() failed
> > > > >
> > > > > I think this is kind of a mess because mtk_pcie_device_power_down() is
> > > > > called from so many places, and some of them aren't connected to the
> > > > > mtk_pcie_device_power_up().
> > > > >
> > > > > In mtk_pcie_setup(), mtk_pcie_setup_irq() only deals with the
> > > > > *controller* IRQ and has nothing to do with the downstream devices.  I
> > > > > think mtk_pcie_setup_irq() should be done before
> > > > > mtk_pcie_startup_port() so we can abort before even powering up those
> > > > > devices.
> > > >
> > > > Makes sense to me. I think it can even be moved outside of mtk_pcie_setup()?
> > > > That way we don't have mtk_pcie_irq_teardown() in two error paths?
> > > >
> > > > I can send a follow up patch for that.
> > >
> > > What if we do those cleanups first?  That would make this patch quite
> > > a bit simpler and we wouldn't have to undo things.  It would be a
> > > shame to have a complicated patch to add this stuff, then another
> > > complicated patch to clean it up.
> >
> > Sure.
> >
> > > > > In mtk_pcie_startup_port(), mtk_pcie_enable_msi() and the PCIe
> > > > > translation window setup also don't have anything to do with the
> > > > > downstream devices, so I think they should be done before
> > > > > calling mtk_pcie_device_power_up().  The only thing there that needs
> > > > > the downstream devices powered up is waiting for the link to come up.
> > > >
> > > > My guess is that if the link up fails, then mtk_pcie_startup_port() is
> > > > going to error out anyway, so it maybe made sense to make sure a device
> > > > is actually present before doing any more work.
> > >
> > > Doesn't sound very convincing to me.  mtk_pcie_set_trans_table() and
> > > the window setup are local things with no timeouts or anything.  I
> > > think the logical structure is way more important than some kind of
> > > hand-crafted work avoidance.
> >
> > I understand. I merely meant that I have no actual knowledge of why it
> > happened, or if there are any concerns from the hardware side.
> >
> > > Separating the PCIe controller setup from things related to the link
> > > and downstream devices is pretty high on my list.
> >
> > Makes sense to me.
> >
> > I'll send an updated version.
> 
> I'm confused. I see this version in -next with the minor fixups from
> Bjorn. Do you want me to do the cleanups on top of them?

I had intended to defer merging pci/controller/mediatek-gen3 to
pci/next and wait for your updated version.  But I see that I forgot
about that and included the original version.  I rebuilt pci/next
without pci/controller/mediatek-gen3.

Bjorn

Re: [PATCH v3 5/7] PCI: mediatek-gen3: Integrate new pwrctrl API

[Chen-Yu Tsai]

On Tue, Mar 24, 2026 at 3:08 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Fri, Mar 20, 2026 at 02:10:03PM +0800, Chen-Yu Tsai wrote:
> > On Tue, Mar 10, 2026 at 12:49 PM Chen-Yu Tsai <wenst@chromium.org> wrote:
> > > On Tue, Mar 10, 2026 at 5:50 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > > > On Mon, Mar 09, 2026 at 01:24:34PM +0800, Chen-Yu Tsai wrote:
> > > > > On Sat, Mar 7, 2026 at 4:59 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > > > > > On Mon, Mar 02, 2026 at 01:31:05PM +0800, Chen-Yu Tsai wrote:
> > > > > > > With the new PCI pwrctrl API and PCI slot binding and power drivers, we
> > > > > > > now have a way to describe and power up WiFi/BT adapters connected
> > > > > > > through a PCIe or M.2 slot, or exploded onto the mainboard itself.
> > > > > ...
> > > >
> > > > > > So now we have:
> > > > > >
> > > > > >   mtk_pcie_probe
> > > > > >     mtk_pcie_setup
> > > > > >       mtk_pcie_startup_port
> > > > > >         mtk_pcie_device_power_up      <-- power up
> > > > > >         mtk_pcie_device_power_down    # error path
> > > > > >       mtk_pcie_setup_irq              # set up controller IRQ
> > > > > >       mtk_pcie_device_power_down      # if mtk_pcie_setup_irq() failed
> > > > > >     pci_host_probe                    # enumerate downstream devices
> > > > > >     mtk_pcie_device_power_down        # if pci_host_probe() failed
> > > > > >
> > > > > > I think this is kind of a mess because mtk_pcie_device_power_down() is
> > > > > > called from so many places, and some of them aren't connected to the
> > > > > > mtk_pcie_device_power_up().
> > > > > >
> > > > > > In mtk_pcie_setup(), mtk_pcie_setup_irq() only deals with the
> > > > > > *controller* IRQ and has nothing to do with the downstream devices.  I
> > > > > > think mtk_pcie_setup_irq() should be done before
> > > > > > mtk_pcie_startup_port() so we can abort before even powering up those
> > > > > > devices.
> > > > >
> > > > > Makes sense to me. I think it can even be moved outside of mtk_pcie_setup()?
> > > > > That way we don't have mtk_pcie_irq_teardown() in two error paths?
> > > > >
> > > > > I can send a follow up patch for that.
> > > >
> > > > What if we do those cleanups first?  That would make this patch quite
> > > > a bit simpler and we wouldn't have to undo things.  It would be a
> > > > shame to have a complicated patch to add this stuff, then another
> > > > complicated patch to clean it up.
> > >
> > > Sure.
> > >
> > > > > > In mtk_pcie_startup_port(), mtk_pcie_enable_msi() and the PCIe
> > > > > > translation window setup also don't have anything to do with the
> > > > > > downstream devices, so I think they should be done before
> > > > > > calling mtk_pcie_device_power_up().  The only thing there that needs
> > > > > > the downstream devices powered up is waiting for the link to come up.
> > > > >
> > > > > My guess is that if the link up fails, then mtk_pcie_startup_port() is
> > > > > going to error out anyway, so it maybe made sense to make sure a device
> > > > > is actually present before doing any more work.
> > > >
> > > > Doesn't sound very convincing to me.  mtk_pcie_set_trans_table() and
> > > > the window setup are local things with no timeouts or anything.  I
> > > > think the logical structure is way more important than some kind of
> > > > hand-crafted work avoidance.
> > >
> > > I understand. I merely meant that I have no actual knowledge of why it
> > > happened, or if there are any concerns from the hardware side.
> > >
> > > > Separating the PCIe controller setup from things related to the link
> > > > and downstream devices is pretty high on my list.
> > >
> > > Makes sense to me.
> > >
> > > I'll send an updated version.
> >
> > I'm confused. I see this version in -next with the minor fixups from
> > Bjorn. Do you want me to do the cleanups on top of them?
>
> I had intended to defer merging pci/controller/mediatek-gen3 to
> pci/next and wait for your updated version.  But I see that I forgot
> about that and included the original version.  I rebuilt pci/next
> without pci/controller/mediatek-gen3.

I'll send v6 with the slot driver symbol name fixed.


ChenYu

[PATCH v3 6/7] arm64: dts: mediatek: mt8195-cherry: add WiFi PCIe and BT USB power supplies

[Chen-Yu Tsai]

The MT8195 Cherry design features an M.2 E-key slot wired up with PCIe
and USB for a WiFi+BT adapter. Previously the power was just enabled
all the time with a default pinctrl setting that set the GPIO pin high.

With the PCIe slot description DT binding in place, the power supplies
can at least be added and tied to the PCIe and USB hosts. Once the
M.2 E-key binding is merged, this description can be further converted
to an M.2 E-key.

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 .../boot/dts/mediatek/mt8195-cherry.dtsi      | 47 ++++++++++++++-----
 1 file changed, 36 insertions(+), 11 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8195-cherry.dtsi b/arch/arm64/boot/dts/mediatek/mt8195-cherry.dtsi
index 6e99122c65ac..f1ff64a84267 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195-cherry.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8195-cherry.dtsi
@@ -83,6 +83,17 @@ pp3300_s3: regulator-pp3300-s3 {
 		vin-supply = <&pp3300_z2>;
 	};
 
+	pp3300_wlan: regulator-pp3300-wlan {
+		compatible = "regulator-fixed";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pp3300_wlan_en_pin>;
+		regulator-name = "pp3300_wlan";
+		/* load switch */
+		enable-active-high;
+		gpio = <&pio 58 GPIO_ACTIVE_HIGH>;
+		vin-supply = <&pp3300_z2>;
+	};
+
 	/* system wide 3.3V power rail */
 	pp3300_z2: regulator-pp3300-z2 {
 		compatible = "regulator-fixed";
@@ -760,10 +771,25 @@ &ovl0_in {
 };
 
 &pcie1 {
-	status = "okay";
-
 	pinctrl-names = "default";
 	pinctrl-0 = <&pcie1_pins_default>;
+	status = "okay";
+
+	pcie@0 {
+		compatible = "pciclass,0604";
+		reg = <0 0 0 0 0>;
+		device_type = "pci";
+		num-lanes = <1>;
+		vpcie3v3-supply = <&pp3300_wlan>;
+		#address-cells = <3>;
+		#size-cells = <2>;
+		ranges;
+
+		wifi@0 {
+			reg = <0 0 0 0 0>;
+			wakeup-source;
+		};
+	};
 };
 
 &pio {
@@ -1179,12 +1205,6 @@ pins-vreg-en {
 	};
 
 	pio_default: pio-default-pins {
-		pins-wifi-enable {
-			pinmux = <PINMUX_GPIO58__FUNC_GPIO58>;
-			output-high;
-			drive-strength = <14>;
-		};
-
 		pins-low-power-pd {
 			pinmux = <PINMUX_GPIO25__FUNC_GPIO25>,
 				 <PINMUX_GPIO26__FUNC_GPIO26>,
@@ -1222,6 +1242,12 @@ pins-low-power-pupd {
 		};
 	};
 
+	pp3300_wlan_en_pin: pp3300-wlan-en-pins {
+		pins-en {
+			pinmux = <PINMUX_GPIO58__FUNC_GPIO58>;
+		};
+	};
+
 	rt1019p_pins_default: rt1019p-default-pins {
 		pins-amp-sdb {
 			pinmux = <PINMUX_GPIO100__FUNC_GPIO100>;
@@ -1570,11 +1596,10 @@ &xhci2 {
 };
 
 &xhci3 {
-	status = "okay";
-
 	/* MT7921's USB Bluetooth has issues with USB2 LPM */
 	usb2-lpm-disable;
-	vbus-supply = <&usb_vbus>;
+	vbus-supply = <&pp3300_wlan>;
+	status = "okay";
 };
 
 #include <arm/cros-ec-keyboard.dtsi>
-- 
2.53.0.473.g4a7958ca14-goog

[PATCH v3 7/7] arm64: dts: mediatek: mt8195-cherry-dojo: Describe M.2 M-key NVMe slot

[Chen-Yu Tsai]

The Dojo device has a M.2 M-key slot for an included NVMe on some
models.

Add a proper device tree description based on the new M.2 M-key binding.
Power for the slot is controlled by the embedded controller. As far as
the main SoC is concerned, it is always on.

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 .../dts/mediatek/mt8195-cherry-dojo-r1.dts    | 38 +++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8195-cherry-dojo-r1.dts b/arch/arm64/boot/dts/mediatek/mt8195-cherry-dojo-r1.dts
index 49664de99b88..57cc329f49c4 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195-cherry-dojo-r1.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8195-cherry-dojo-r1.dts
@@ -11,6 +11,28 @@ / {
 	compatible = "google,dojo-sku7", "google,dojo-sku5",
 		     "google,dojo-sku3", "google,dojo-sku1",
 		     "google,dojo", "mediatek,mt8195";
+
+	nvme-connector {
+		compatible = "pcie-m2-m-connector";
+		/* power is controlled by EC */
+		vpcie3v3-supply = <&pp3300_z2>;
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				reg = <0>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				nvme_ep: endpoint@0 {
+					reg = <0>;
+					remote-endpoint = <&pcie0_ep>;
+				};
+			};
+		};
+	};
 };
 
 &audio_codec {
@@ -72,6 +94,22 @@ &pcie0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pcie0_pins_default>;
 	status = "okay";
+
+	pcie@0 {
+		compatible = "pciclass,0604";
+		reg = <0 0 0 0 0>;
+		device_type = "pci";
+		num-lanes = <2>;
+		#address-cells = <3>;
+		#size-cells = <2>;
+		ranges;
+
+		port {
+			pcie0_ep: endpoint {
+				remote-endpoint = <&nvme_ep>;
+			};
+		};
+	};
 };
 
 &pciephy {
-- 
2.53.0.473.g4a7958ca14-goog

Re: (subset) [PATCH v3 0/7] PCI: mediatek-gen3: add power control support

[Manivannan Sadhasivam]

On Mon, 02 Mar 2026 13:31:00 +0800, Chen-Yu Tsai wrote:
> This is v3 of my MediaTek PCIe gen3 controller driver power control
> support series.
> 
> Changes since v2:
> - Link to v2: https://lore.kernel.org/all/20260226092234.3859740-1-wenst@chromium.org/
> - Made PCIE_MEDIATEK_GEN3 select PCI_PWRCTRL_SLOT, following existing
>   examples
> 
> [...]

Applied, thanks!

[1/7] PCI: mediatek-gen3: Clean up mtk_pcie_parse_port() with dev_err_probe()
      commit: accd5483cf12fe30b99f3d8585921b34c71c0a76
[2/7] PCI: mediatek-gen3: Add error path for probe and resume driver callbacks
      commit: 26781cd7ac6cedb7c669d8ddd384dfd914b8ac4a
[3/7] PCI: mediatek-gen3: Split out device power helpers
      commit: 01ddd5ab62127aeaf05a4ef99a1f566ed739de59
[4/7] PCI: mediatek-gen3: Disable device if further setup fails
      commit: c0275030fb4030be4cb78b460af5bd819d2d8b91
[5/7] PCI: mediatek-gen3: Integrate new pwrctrl API
      commit: 614bfdbba1e224f91affe0e1e71d5672d3807a2e

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

Re: (subset) [PATCH v3 0/7] PCI: mediatek-gen3: add power control support

[AngeloGioacchino Del Regno]

On Mon, 02 Mar 2026 13:31:00 +0800, Chen-Yu Tsai wrote:
> This is v3 of my MediaTek PCIe gen3 controller driver power control
> support series.
> 
> Changes since v2:
> - Link to v2: https://lore.kernel.org/all/20260226092234.3859740-1-wenst@chromium.org/
> - Made PCIE_MEDIATEK_GEN3 select PCI_PWRCTRL_SLOT, following existing
>   examples
> 
> [...]

Applied to v7.0-next/dts64, thanks!

[6/7] arm64: dts: mediatek: mt8195-cherry: add WiFi PCIe and BT USB power supplies
      commit: 0c5d91b3d0eeed3925dd43f834afcc19a11aa9fd
[7/7] arm64: dts: mediatek: mt8195-cherry-dojo: Describe M.2 M-key NVMe slot
      commit: 4c434585ce6d485ee12ed6becb1524f933454aba

Cheers,
Angelo