[PATCH v3 5/7] PCI: mediatek-gen3: Integrate new pwrctrl API
Chen-Yu Tsai
wenst at chromium.org
Sun Mar 8 22:24:34 PDT 2026
On Sat, Mar 7, 2026 at 4:59 AM Bjorn Helgaas <helgaas at 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 at collabora.com>
> > Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski at oss.qualcomm.com>
> > Reviewed-by: Manivannan Sadhasivam <mani at kernel.org>
> > Signed-off-by: Chen-Yu Tsai <wenst at 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
> >
More information about the Linux-mediatek
mailing list