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

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

[Chen-Yu Tsai]

Hi folks,

This is v6 of my MediaTek PCIe gen3 controller driver power control
support series. This series is based on next-20260323, with the
commit a2b2ca0c2477 ("Merge branch 'pci/controller/mediatek-gen3'")
reverted to drop the older version.

This series is ready to be merged. Please drop the older version and use
this one instead.

Changes since v5:
- Link to v5: https://lore.kernel.org/all/20260311075223.3303497-1-wenst@chromium.org/
- Adapt to PCI_PWRCTRL_SLOT -> PCI_PWRCTRL_GENERIC Kconfig symbol name change

Changes since v4:
- Link to v4: https://lore.kernel.org/all/20260310091947.2742004-1-wenst@chromium.org/
- Patch 1
  - Expanded tabs in commit message
- Patch 5
  - s/mtk_pcie_device_power_(up|down)/mtk_pcie_devices_power_(up|down)/
- Patch 6
  - Adapted to mtk_pcie_devices_power_down() name change
- Patch 7
  - Fixed label typo causing build break
  - Replaced "exploded" with "populated" in commit message, and added
    more explanation about what "populated onto the mainboard" means.

Changes since v3:
- Link to v3: https://lore.kernel.org/all/20260302053109.1117091-1-wenst@chromium.org/
- Added two patches to move kernel setup code before controller setup
  code, and controller setup code before device setup code, as requested
  by Bjorn
- Dropped dts patches as they are already merged

I kept all the existing reviewed-by tags, since the changes to the
existing patches aren't that big.

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 6 are cleanups and minor improvements to the driver.

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


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: Move mtk_pcie_setup_irq() out of mtk_pcie_setup()
  PCI: mediatek-gen3: Move controller setup steps before PERST# control
  PCI: mediatek-gen3: Add error path for 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

 drivers/pci/controller/Kconfig              |   1 +
 drivers/pci/controller/pcie-mediatek-gen3.c | 223 +++++++++++---------
 2 files changed, 129 insertions(+), 95 deletions(-)

-- 
2.53.0.983.g0bb29b3bc5-goog

[PATCH v6 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.983.g0bb29b3bc5-goog

[PATCH v6 2/7] PCI: mediatek-gen3: Move mtk_pcie_setup_irq() out of mtk_pcie_setup()

[Chen-Yu Tsai]

mtk_pcie_setup_irq() just sets up the IRQ domains for PCI INTx and MSI,
and chains them to the controller's interrupt. It's not really related
to the setup of the actual PCIe controller.

Move the mtk_pcie_setup_irq() call out of and before mtk_pcie_setup(),
and add a proper error message for when it fails. Reorder
mtk_pcie_irq_teardown() in the remove callback to follow. Also create
an error path in the probe function.

Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
Link: https://lore.kernel.org/all/20260309215056.GA603013@bhelgaas/
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 drivers/pci/controller/pcie-mediatek-gen3.c | 25 ++++++++++++---------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
index 1939cac995b5..04ae195d36c2 100644
--- a/drivers/pci/controller/pcie-mediatek-gen3.c
+++ b/drivers/pci/controller/pcie-mediatek-gen3.c
@@ -1152,10 +1152,6 @@ static int mtk_pcie_setup(struct mtk_gen3_pcie *pcie)
 	if (err)
 		goto err_setup;
 
-	err = mtk_pcie_setup_irq(pcie);
-	if (err)
-		goto err_setup;
-
 	return 0;
 
 err_setup:
@@ -1181,21 +1177,28 @@ static int mtk_pcie_probe(struct platform_device *pdev)
 	pcie->soc = device_get_match_data(dev);
 	platform_set_drvdata(pdev, pcie);
 
+	err = mtk_pcie_setup_irq(pcie);
+	if (err)
+		return dev_err_probe(dev, err, "Failed to setup IRQ domains\n");
+
 	err = mtk_pcie_setup(pcie);
 	if (err)
-		return err;
+		goto err_tear_down_irq;
 
 	host->ops = &mtk_pcie_ops;
 	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_power_down_pcie;
 
 	return 0;
+
+err_power_down_pcie:
+	mtk_pcie_power_down(pcie);
+err_tear_down_irq:
+	mtk_pcie_irq_teardown(pcie);
+	return err;
 }
 
 static void mtk_pcie_remove(struct platform_device *pdev)
@@ -1208,8 +1211,8 @@ static void mtk_pcie_remove(struct platform_device *pdev)
 	pci_remove_root_bus(host->bus);
 	pci_unlock_rescan_remove();
 
-	mtk_pcie_irq_teardown(pcie);
 	mtk_pcie_power_down(pcie);
+	mtk_pcie_irq_teardown(pcie);
 }
 
 static void mtk_pcie_irq_save(struct mtk_gen3_pcie *pcie)
-- 
2.53.0.983.g0bb29b3bc5-goog

[PATCH v6 3/7] PCI: mediatek-gen3: Move controller setup steps before PERST# control

[Chen-Yu Tsai]

Setting up the translation windows and enabling MSI involve only
configuring the controller, not the device. These can be done before the
device is enabled.

Move these steps before the existing PERST# control. This provides a
cleaner separation of controller vs device setup. This also allows the
later patches that split out PERST# control and add device power
control to have cleaner teardown.

This change only moves code. No functional change is expected.

Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
Link: https://lore.kernel.org/all/20260309215056.GA603013@bhelgaas/
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 drivers/pci/controller/pcie-mediatek-gen3.c | 50 ++++++++++-----------
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
index 04ae195d36c2..1b6290f2c360 100644
--- a/drivers/pci/controller/pcie-mediatek-gen3.c
+++ b/drivers/pci/controller/pcie-mediatek-gen3.c
@@ -464,6 +464,31 @@ 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);
 
+	mtk_pcie_enable_msi(pcie);
+
+	/* Set PCIe translation windows */
+	resource_list_for_each_entry(entry, &host->windows) {
+		struct resource *res = entry->res;
+		unsigned long type = resource_type(res);
+		resource_size_t cpu_addr;
+		resource_size_t pci_addr;
+		resource_size_t size;
+
+		if (type == IORESOURCE_IO)
+			cpu_addr = pci_pio_to_address(res->start);
+		else if (type == IORESOURCE_MEM)
+			cpu_addr = res->start;
+		else
+			continue;
+
+		pci_addr = res->start - entry->offset;
+		size = resource_size(res);
+		err = mtk_pcie_set_trans_table(pcie, cpu_addr, pci_addr, size,
+					       type, &table_index);
+		if (err)
+			return err;
+	}
+
 	/*
 	 * Airoha EN7581 has a hw bug asserting/releasing PCIE_PE_RSTB signal
 	 * causing occasional PCIe link down. In order to overcome the issue,
@@ -510,31 +535,6 @@ static int mtk_pcie_startup_port(struct mtk_gen3_pcie *pcie)
 		return err;
 	}
 
-	mtk_pcie_enable_msi(pcie);
-
-	/* Set PCIe translation windows */
-	resource_list_for_each_entry(entry, &host->windows) {
-		struct resource *res = entry->res;
-		unsigned long type = resource_type(res);
-		resource_size_t cpu_addr;
-		resource_size_t pci_addr;
-		resource_size_t size;
-
-		if (type == IORESOURCE_IO)
-			cpu_addr = pci_pio_to_address(res->start);
-		else if (type == IORESOURCE_MEM)
-			cpu_addr = res->start;
-		else
-			continue;
-
-		pci_addr = res->start - entry->offset;
-		size = resource_size(res);
-		err = mtk_pcie_set_trans_table(pcie, cpu_addr, pci_addr, size,
-					       type, &table_index);
-		if (err)
-			return err;
-	}
-
 	return 0;
 }
 
-- 
2.53.0.983.g0bb29b3bc5-goog

[PATCH v6 4/7] PCI: mediatek-gen3: Add error path for resume driver callbacks

[Chen-Yu Tsai]

The resume callback currently does teardown in the conditional block
directly. This is going to get ugly when the pwrctrl calls are added.

Move the teardown to a proper error cleanup path.

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 | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
index 1b6290f2c360..22a16e4ebc76 100644
--- a/drivers/pci/controller/pcie-mediatek-gen3.c
+++ b/drivers/pci/controller/pcie-mediatek-gen3.c
@@ -1304,14 +1304,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.983.g0bb29b3bc5-goog

[PATCH v6 5/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>
---
 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 22a16e4ebc76..526db8815401 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_devices_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_devices_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;
@@ -489,33 +537,9 @@ static int mtk_pcie_startup_port(struct mtk_gen3_pcie *pcie)
 			return err;
 	}
 
-	/*
-	 * 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_devices_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,
@@ -1270,7 +1294,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);
@@ -1279,13 +1302,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_devices_power_down(pcie);
 	dev_dbg(pcie->dev, "entered L2 states successfully");
 
 	mtk_pcie_irq_save(pcie);
-- 
2.53.0.983.g0bb29b3bc5-goog

Re: [PATCH v6 5/7] PCI: mediatek-gen3: Split out device power helpers

[Chen-Yu Tsai]

On Tue, Mar 24, 2026 at 1:20 PM Chen-Yu Tsai <wenst@chromium.org> wrote:
>
> 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>
> ---
>  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 22a16e4ebc76..526db8815401 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_devices_power_up(struct mtk_gen3_pcie *pcie)
> +{
> +       int err;

Sashiko pointed out that this addition would cause an unused variable
warning. This line should be added in the last patch instead.

Do you want me to respin? Or is it fine since the end result is the same?

Sorry about this.


ChenYu


> +       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_devices_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;
> @@ -489,33 +537,9 @@ static int mtk_pcie_startup_port(struct mtk_gen3_pcie *pcie)
>                         return err;
>         }
>
> -       /*
> -        * 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_devices_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,
> @@ -1270,7 +1294,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);
> @@ -1279,13 +1302,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_devices_power_down(pcie);
>         dev_dbg(pcie->dev, "entered L2 states successfully");
>
>         mtk_pcie_irq_save(pcie);
> --
> 2.53.0.983.g0bb29b3bc5-goog
>

Re: [PATCH v6 5/7] PCI: mediatek-gen3: Split out device power helpers

[Manivannan Sadhasivam]

On Tue, Mar 24, 2026 at 04:55:47PM +0800, Chen-Yu Tsai wrote:
> On Tue, Mar 24, 2026 at 1:20 PM Chen-Yu Tsai <wenst@chromium.org> wrote:
> >
> > 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>
> > ---
> >  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 22a16e4ebc76..526db8815401 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_devices_power_up(struct mtk_gen3_pcie *pcie)
> > +{
> > +       int err;
> 
> Sashiko pointed out that this addition would cause an unused variable
> warning. This line should be added in the last patch instead.
> 
> Do you want me to respin? Or is it fine since the end result is the same?
> 

I fixed it in the branch as we try not to introduce any warning even between the
commits.

> Sorry about this.
> 

Np!

- Mani

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

[PATCH v6 6/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_probe().

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 | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
index 526db8815401..208866d33c77 100644
--- a/drivers/pci/controller/pcie-mediatek-gen3.c
+++ b/drivers/pci/controller/pcie-mediatek-gen3.c
@@ -556,10 +556,14 @@ 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;
 	}
 
 	return 0;
+
+err_power_down_device:
+	mtk_pcie_devices_power_down(pcie);
+	return err;
 }
 
 #define MTK_MSI_FLAGS_REQUIRED (MSI_FLAG_USE_DEF_DOM_OPS	| \
@@ -1219,6 +1223,7 @@ static int mtk_pcie_probe(struct platform_device *pdev)
 	return 0;
 
 err_power_down_pcie:
+	mtk_pcie_devices_power_down(pcie);
 	mtk_pcie_power_down(pcie);
 err_tear_down_irq:
 	mtk_pcie_irq_teardown(pcie);
-- 
2.53.0.983.g0bb29b3bc5-goog

[PATCH v6 7/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 populated onto the mainboard itself.
The latter case has the adapter layout or design copied verbatim,
replacing the slot with direct connections.

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 v5:
- Adapt to PCI_PWRCTRL_SLOT -> PCI_PWRCTRL_GENERIC Kconfig symbol namechange
---
 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..686349e09cd3 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_GENERIC
 	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 208866d33c77..a94fdbaf47fe 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_devices_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_devices_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)
@@ -1209,9 +1220,15 @@ static int mtk_pcie_probe(struct platform_device *pdev)
 	if (err)
 		return dev_err_probe(dev, err, "Failed to setup IRQ domains\n");
 
+	err = pci_pwrctrl_create_devices(pcie->dev);
+	if (err) {
+		goto err_tear_down_irq;
+		dev_err_probe(dev, err, "failed to create pwrctrl devices\n");
+	}
+
 	err = mtk_pcie_setup(pcie);
 	if (err)
-		goto err_tear_down_irq;
+		goto err_destroy_pwrctrl;
 
 	host->ops = &mtk_pcie_ops;
 	host->sysdata = pcie;
@@ -1225,6 +1242,9 @@ static int mtk_pcie_probe(struct platform_device *pdev)
 err_power_down_pcie:
 	mtk_pcie_devices_power_down(pcie);
 	mtk_pcie_power_down(pcie);
+err_destroy_pwrctrl:
+	if (err != -EPROBE_DEFER)
+		pci_pwrctrl_destroy_devices(pcie->dev);
 err_tear_down_irq:
 	mtk_pcie_irq_teardown(pcie);
 	return err;
@@ -1240,7 +1260,9 @@ static void mtk_pcie_remove(struct platform_device *pdev)
 	pci_remove_root_bus(host->bus);
 	pci_unlock_rescan_remove();
 
+	pci_pwrctrl_power_off_devices(pcie->dev);
 	mtk_pcie_power_down(pcie);
+	pci_pwrctrl_destroy_devices(pcie->dev);
 	mtk_pcie_irq_teardown(pcie);
 }
 
-- 
2.53.0.983.g0bb29b3bc5-goog

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

[Manivannan Sadhasivam]

On Tue, Mar 24, 2026 at 01:19:52PM +0800, Chen-Yu Tsai wrote:
> Hi folks,
> 
> This is v6 of my MediaTek PCIe gen3 controller driver power control
> support series. This series is based on next-20260323, with the
> commit a2b2ca0c2477 ("Merge branch 'pci/controller/mediatek-gen3'")
> reverted to drop the older version.
> 
> This series is ready to be merged. Please drop the older version and use
> this one instead.
> 
> Changes since v5:
> - Link to v5: https://lore.kernel.org/all/20260311075223.3303497-1-wenst@chromium.org/
> - Adapt to PCI_PWRCTRL_SLOT -> PCI_PWRCTRL_GENERIC Kconfig symbol name change
> 
> Changes since v4:
> - Link to v4: https://lore.kernel.org/all/20260310091947.2742004-1-wenst@chromium.org/
> - Patch 1
>   - Expanded tabs in commit message
> - Patch 5
>   - s/mtk_pcie_device_power_(up|down)/mtk_pcie_devices_power_(up|down)/
> - Patch 6
>   - Adapted to mtk_pcie_devices_power_down() name change
> - Patch 7
>   - Fixed label typo causing build break
>   - Replaced "exploded" with "populated" in commit message, and added
>     more explanation about what "populated onto the mainboard" means.
> 
> Changes since v3:
> - Link to v3: https://lore.kernel.org/all/20260302053109.1117091-1-wenst@chromium.org/
> - Added two patches to move kernel setup code before controller setup
>   code, and controller setup code before device setup code, as requested
>   by Bjorn
> - Dropped dts patches as they are already merged
> 
> I kept all the existing reviewed-by tags, since the changes to the
> existing patches aren't that big.
> 
> 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.
> 

I agree with you. I've sent a patch [1] to select these drivers in defconfig to
make the users life easier. But in the meantime, let's have this dependency
as-is, otherwise, until my defconfig patch lands, there is no guarantee that
these drivers will get selected by default.

I'll change these dependencies after v7.1-rc1.

- Mani

[1] https://lore.kernel.org/linux-arm-msm/20260324073931.23739-1-manivannan.sadhasivam@oss.qualcomm.com

> 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 6 are cleanups and minor improvements to the driver.
> 
> Patch 7 adds power control support using the new pwrctrl API to the
> PCIe controller driver.
> 
> 
> 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: Move mtk_pcie_setup_irq() out of mtk_pcie_setup()
>   PCI: mediatek-gen3: Move controller setup steps before PERST# control
>   PCI: mediatek-gen3: Add error path for 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
> 
>  drivers/pci/controller/Kconfig              |   1 +
>  drivers/pci/controller/pcie-mediatek-gen3.c | 223 +++++++++++---------
>  2 files changed, 129 insertions(+), 95 deletions(-)
> 
> -- 
> 2.53.0.983.g0bb29b3bc5-goog
> 

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

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

[Chen-Yu Tsai]

On Tue, Mar 24, 2026 at 3:44 PM Manivannan Sadhasivam <mani@kernel.org> wrote:
>
> On Tue, Mar 24, 2026 at 01:19:52PM +0800, Chen-Yu Tsai wrote:
> > Hi folks,
> >
> > This is v6 of my MediaTek PCIe gen3 controller driver power control
> > support series. This series is based on next-20260323, with the
> > commit a2b2ca0c2477 ("Merge branch 'pci/controller/mediatek-gen3'")
> > reverted to drop the older version.
> >
> > This series is ready to be merged. Please drop the older version and use
> > this one instead.
> >
> > Changes since v5:
> > - Link to v5: https://lore.kernel.org/all/20260311075223.3303497-1-wenst@chromium.org/
> > - Adapt to PCI_PWRCTRL_SLOT -> PCI_PWRCTRL_GENERIC Kconfig symbol name change
> >
> > Changes since v4:
> > - Link to v4: https://lore.kernel.org/all/20260310091947.2742004-1-wenst@chromium.org/
> > - Patch 1
> >   - Expanded tabs in commit message
> > - Patch 5
> >   - s/mtk_pcie_device_power_(up|down)/mtk_pcie_devices_power_(up|down)/
> > - Patch 6
> >   - Adapted to mtk_pcie_devices_power_down() name change
> > - Patch 7
> >   - Fixed label typo causing build break
> >   - Replaced "exploded" with "populated" in commit message, and added
> >     more explanation about what "populated onto the mainboard" means.
> >
> > Changes since v3:
> > - Link to v3: https://lore.kernel.org/all/20260302053109.1117091-1-wenst@chromium.org/
> > - Added two patches to move kernel setup code before controller setup
> >   code, and controller setup code before device setup code, as requested
> >   by Bjorn
> > - Dropped dts patches as they are already merged
> >
> > I kept all the existing reviewed-by tags, since the changes to the
> > existing patches aren't that big.
> >
> > 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.
> >
>
> I agree with you. I've sent a patch [1] to select these drivers in defconfig to
> make the users life easier. But in the meantime, let's have this dependency
> as-is, otherwise, until my defconfig patch lands, there is no guarantee that
> these drivers will get selected by default.

I would probably just make PCI_PWRCTRL_GENERIC default m or y.

Or make CONFIG_PCI (and probably CONFIG_USB later on) imply
CONFIG_POWER_SEQUENCING_PCIE_M2.

ChenYu


> I'll change these dependencies after v7.1-rc1.
>
> - Mani
>
> [1] https://lore.kernel.org/linux-arm-msm/20260324073931.23739-1-manivannan.sadhasivam@oss.qualcomm.com
>
> > 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 6 are cleanups and minor improvements to the driver.
> >
> > Patch 7 adds power control support using the new pwrctrl API to the
> > PCIe controller driver.
> >
> >
> > 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: Move mtk_pcie_setup_irq() out of mtk_pcie_setup()
> >   PCI: mediatek-gen3: Move controller setup steps before PERST# control
> >   PCI: mediatek-gen3: Add error path for 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
> >
> >  drivers/pci/controller/Kconfig              |   1 +
> >  drivers/pci/controller/pcie-mediatek-gen3.c | 223 +++++++++++---------
> >  2 files changed, 129 insertions(+), 95 deletions(-)
> >
> > --
> > 2.53.0.983.g0bb29b3bc5-goog
> >
>
> --
> மணிவண்ணன் சதாசிவம்

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

[Manivannan Sadhasivam]

On Tue, Mar 24, 2026 at 03:52:33PM +0800, Chen-Yu Tsai wrote:
> On Tue, Mar 24, 2026 at 3:44 PM Manivannan Sadhasivam <mani@kernel.org> wrote:
> >
> > On Tue, Mar 24, 2026 at 01:19:52PM +0800, Chen-Yu Tsai wrote:
> > > Hi folks,
> > >
> > > This is v6 of my MediaTek PCIe gen3 controller driver power control
> > > support series. This series is based on next-20260323, with the
> > > commit a2b2ca0c2477 ("Merge branch 'pci/controller/mediatek-gen3'")
> > > reverted to drop the older version.
> > >
> > > This series is ready to be merged. Please drop the older version and use
> > > this one instead.
> > >
> > > Changes since v5:
> > > - Link to v5: https://lore.kernel.org/all/20260311075223.3303497-1-wenst@chromium.org/
> > > - Adapt to PCI_PWRCTRL_SLOT -> PCI_PWRCTRL_GENERIC Kconfig symbol name change
> > >
> > > Changes since v4:
> > > - Link to v4: https://lore.kernel.org/all/20260310091947.2742004-1-wenst@chromium.org/
> > > - Patch 1
> > >   - Expanded tabs in commit message
> > > - Patch 5
> > >   - s/mtk_pcie_device_power_(up|down)/mtk_pcie_devices_power_(up|down)/
> > > - Patch 6
> > >   - Adapted to mtk_pcie_devices_power_down() name change
> > > - Patch 7
> > >   - Fixed label typo causing build break
> > >   - Replaced "exploded" with "populated" in commit message, and added
> > >     more explanation about what "populated onto the mainboard" means.
> > >
> > > Changes since v3:
> > > - Link to v3: https://lore.kernel.org/all/20260302053109.1117091-1-wenst@chromium.org/
> > > - Added two patches to move kernel setup code before controller setup
> > >   code, and controller setup code before device setup code, as requested
> > >   by Bjorn
> > > - Dropped dts patches as they are already merged
> > >
> > > I kept all the existing reviewed-by tags, since the changes to the
> > > existing patches aren't that big.
> > >
> > > 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.
> > >
> >
> > I agree with you. I've sent a patch [1] to select these drivers in defconfig to
> > make the users life easier. But in the meantime, let's have this dependency
> > as-is, otherwise, until my defconfig patch lands, there is no guarantee that
> > these drivers will get selected by default.
> 
> I would probably just make PCI_PWRCTRL_GENERIC default m or y.
> 
> Or make CONFIG_PCI (and probably CONFIG_USB later on) imply
> CONFIG_POWER_SEQUENCING_PCIE_M2.
> 

The problem is, these drivers are mostly targeted for DT systems as of now. If
we select it by default for all archs, it will bloat the image size for other
archs that don't need these drivers. That's why I wanted to enable these in arch
defconfig.

- Mani

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

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

[Chen-Yu Tsai]

On Tue, Mar 24, 2026 at 4:01 PM Manivannan Sadhasivam <mani@kernel.org> wrote:
>
> On Tue, Mar 24, 2026 at 03:52:33PM +0800, Chen-Yu Tsai wrote:
> > On Tue, Mar 24, 2026 at 3:44 PM Manivannan Sadhasivam <mani@kernel.org> wrote:
> > >
> > > On Tue, Mar 24, 2026 at 01:19:52PM +0800, Chen-Yu Tsai wrote:
> > > > Hi folks,
> > > >
> > > > This is v6 of my MediaTek PCIe gen3 controller driver power control
> > > > support series. This series is based on next-20260323, with the
> > > > commit a2b2ca0c2477 ("Merge branch 'pci/controller/mediatek-gen3'")
> > > > reverted to drop the older version.
> > > >
> > > > This series is ready to be merged. Please drop the older version and use
> > > > this one instead.
> > > >
> > > > Changes since v5:
> > > > - Link to v5: https://lore.kernel.org/all/20260311075223.3303497-1-wenst@chromium.org/
> > > > - Adapt to PCI_PWRCTRL_SLOT -> PCI_PWRCTRL_GENERIC Kconfig symbol name change
> > > >
> > > > Changes since v4:
> > > > - Link to v4: https://lore.kernel.org/all/20260310091947.2742004-1-wenst@chromium.org/
> > > > - Patch 1
> > > >   - Expanded tabs in commit message
> > > > - Patch 5
> > > >   - s/mtk_pcie_device_power_(up|down)/mtk_pcie_devices_power_(up|down)/
> > > > - Patch 6
> > > >   - Adapted to mtk_pcie_devices_power_down() name change
> > > > - Patch 7
> > > >   - Fixed label typo causing build break
> > > >   - Replaced "exploded" with "populated" in commit message, and added
> > > >     more explanation about what "populated onto the mainboard" means.
> > > >
> > > > Changes since v3:
> > > > - Link to v3: https://lore.kernel.org/all/20260302053109.1117091-1-wenst@chromium.org/
> > > > - Added two patches to move kernel setup code before controller setup
> > > >   code, and controller setup code before device setup code, as requested
> > > >   by Bjorn
> > > > - Dropped dts patches as they are already merged
> > > >
> > > > I kept all the existing reviewed-by tags, since the changes to the
> > > > existing patches aren't that big.
> > > >
> > > > 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.
> > > >
> > >
> > > I agree with you. I've sent a patch [1] to select these drivers in defconfig to
> > > make the users life easier. But in the meantime, let's have this dependency
> > > as-is, otherwise, until my defconfig patch lands, there is no guarantee that
> > > these drivers will get selected by default.
> >
> > I would probably just make PCI_PWRCTRL_GENERIC default m or y.
> >
> > Or make CONFIG_PCI (and probably CONFIG_USB later on) imply
> > CONFIG_POWER_SEQUENCING_PCIE_M2.
> >
>
> The problem is, these drivers are mostly targeted for DT systems as of now. If
> we select it by default for all archs, it will bloat the image size for other
> archs that don't need these drivers. That's why I wanted to enable these in arch
> defconfig.

How about

    imply CONFIG_POWER_SEQUENCING_PCIE_M2 if OF

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

[Manivannan Sadhasivam]

On Tue, Mar 24, 2026 at 04:26:22PM +0800, Chen-Yu Tsai wrote:
> On Tue, Mar 24, 2026 at 4:01 PM Manivannan Sadhasivam <mani@kernel.org> wrote:
> >
> > On Tue, Mar 24, 2026 at 03:52:33PM +0800, Chen-Yu Tsai wrote:
> > > On Tue, Mar 24, 2026 at 3:44 PM Manivannan Sadhasivam <mani@kernel.org> wrote:
> > > >
> > > > On Tue, Mar 24, 2026 at 01:19:52PM +0800, Chen-Yu Tsai wrote:
> > > > > Hi folks,
> > > > >
> > > > > This is v6 of my MediaTek PCIe gen3 controller driver power control
> > > > > support series. This series is based on next-20260323, with the
> > > > > commit a2b2ca0c2477 ("Merge branch 'pci/controller/mediatek-gen3'")
> > > > > reverted to drop the older version.
> > > > >
> > > > > This series is ready to be merged. Please drop the older version and use
> > > > > this one instead.
> > > > >
> > > > > Changes since v5:
> > > > > - Link to v5: https://lore.kernel.org/all/20260311075223.3303497-1-wenst@chromium.org/
> > > > > - Adapt to PCI_PWRCTRL_SLOT -> PCI_PWRCTRL_GENERIC Kconfig symbol name change
> > > > >
> > > > > Changes since v4:
> > > > > - Link to v4: https://lore.kernel.org/all/20260310091947.2742004-1-wenst@chromium.org/
> > > > > - Patch 1
> > > > >   - Expanded tabs in commit message
> > > > > - Patch 5
> > > > >   - s/mtk_pcie_device_power_(up|down)/mtk_pcie_devices_power_(up|down)/
> > > > > - Patch 6
> > > > >   - Adapted to mtk_pcie_devices_power_down() name change
> > > > > - Patch 7
> > > > >   - Fixed label typo causing build break
> > > > >   - Replaced "exploded" with "populated" in commit message, and added
> > > > >     more explanation about what "populated onto the mainboard" means.
> > > > >
> > > > > Changes since v3:
> > > > > - Link to v3: https://lore.kernel.org/all/20260302053109.1117091-1-wenst@chromium.org/
> > > > > - Added two patches to move kernel setup code before controller setup
> > > > >   code, and controller setup code before device setup code, as requested
> > > > >   by Bjorn
> > > > > - Dropped dts patches as they are already merged
> > > > >
> > > > > I kept all the existing reviewed-by tags, since the changes to the
> > > > > existing patches aren't that big.
> > > > >
> > > > > 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.
> > > > >
> > > >
> > > > I agree with you. I've sent a patch [1] to select these drivers in defconfig to
> > > > make the users life easier. But in the meantime, let's have this dependency
> > > > as-is, otherwise, until my defconfig patch lands, there is no guarantee that
> > > > these drivers will get selected by default.
> > >
> > > I would probably just make PCI_PWRCTRL_GENERIC default m or y.
> > >
> > > Or make CONFIG_PCI (and probably CONFIG_USB later on) imply
> > > CONFIG_POWER_SEQUENCING_PCIE_M2.
> > >
> >
> > The problem is, these drivers are mostly targeted for DT systems as of now. If
> > we select it by default for all archs, it will bloat the image size for other
> > archs that don't need these drivers. That's why I wanted to enable these in arch
> > defconfig.
> 
> How about
> 
>     imply CONFIG_POWER_SEQUENCING_PCIE_M2 if OF

I'm not sure if PCI Kconfig is the right place to put it. We don't select any
drivers using 'imply' as of now and OF conditional also means that this driver
could be selected for PowerPC archs which don't use this driver at all.

Same argument could be used for ARM defconfig as well as we have server based
ARM64 SoCs, but I don't have a better choice atm.

- Mani

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

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

[Manivannan Sadhasivam]

On Tue, 24 Mar 2026 13:19:52 +0800, Chen-Yu Tsai wrote:
> This is v6 of my MediaTek PCIe gen3 controller driver power control
> support series. This series is based on next-20260323, with the
> commit a2b2ca0c2477 ("Merge branch 'pci/controller/mediatek-gen3'")
> reverted to drop the older version.
> 
> This series is ready to be merged. Please drop the older version and use
> this one instead.
> 
> [...]

Applied, thanks!

[1/7] PCI: mediatek-gen3: Clean up mtk_pcie_parse_port() with dev_err_probe()
      commit: 71f526831e9d7db53a7b01717bfb725759b1d06f
[2/7] PCI: mediatek-gen3: Move mtk_pcie_setup_irq() out of mtk_pcie_setup()
      commit: 0c7398cbd5d0c0d9952b6bcf1fb738e252ea0f50
[3/7] PCI: mediatek-gen3: Move controller setup steps before PERST# control
      commit: 67b1ff043b0ddd88e0a6af6c4c3590a54b60eb93
[4/7] PCI: mediatek-gen3: Add error path for resume driver callbacks
      commit: 76190e45623aefd6a84450af5c84e793f4f6a796
[5/7] PCI: mediatek-gen3: Split out device power helpers
      commit: b8cbed2f4d783f4d9f3edc5dc6d2aac848de3709
[6/7] PCI: mediatek-gen3: Disable device if further setup fails
      commit: fded9d718161f1c8da7a8464b87a028292ad5573
[7/7] PCI: mediatek-gen3: Integrate new pwrctrl API
      commit: edb2f4b495fbf1b148df0deee147ea5c320f56a3

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