[PATCH v1] PCI: mediatek-gen3: Align PERST# sequence with PCIe CEM specification

[PATCH v1] PCI: mediatek-gen3: Align PERST# sequence with PCIe CEM specification

[Jian Yang]

Fix the control sequence of PERST# during system bootup/shutdown to
meet the requirement from PCIe CEM specification. There are two major
changes in this patch:

1. Some of MediaTek's chip will stop generating REFCLK if the
PCIE_PHY_RSTB signal of PCIe controller is asserted. We have to
adjust the startup sequence as follows to ensure that PERST# will be
de-asserted after the REFCLK is stable:
Assert PHY reset and PERST# -> delay 10ms -> De-assert PHY reset ->
delay 100ms -> De-assert PERST#

2. Add 'shutdown' callback to control the timing of PERST# and power
during the system shutdown phase, ensuring that PERST# is active
before the power on connector is removed.

Signed-off-by: Jian Yang <jian.yang@mediatek.com>
---
 drivers/pci/controller/pcie-mediatek-gen3.c | 39 +++++++++++++++++++--
 1 file changed, 36 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
index a94fdbaf47fe..66d177918565 100644
--- a/drivers/pci/controller/pcie-mediatek-gen3.c
+++ b/drivers/pci/controller/pcie-mediatek-gen3.c
@@ -62,6 +62,7 @@
 #define PCIE_PHY_RSTB			BIT(1)
 #define PCIE_BRG_RSTB			BIT(2)
 #define PCIE_PE_RSTB			BIT(3)
+#define PCIE_BRG_RST_RDY_MS		10
 
 #define PCIE_LTSSM_STATUS_REG		0x150
 #define PCIE_LTSSM_STATE_MASK		GENMASK(28, 24)
@@ -133,6 +134,7 @@
 #define MAX_NUM_PHY_RESETS		3
 
 #define PCIE_MTK_RESET_TIME_US		10
+#define PCIE_MTK_PDN_PERST_TIME_MS	5
 
 /* Time in ms needed to complete PCIe reset on EN7581 SoC */
 #define PCIE_EN7581_RESET_TIME_MS	100
@@ -430,6 +432,21 @@ static int mtk_pcie_devices_power_up(struct mtk_gen3_pcie *pcie)
 		return err;
 	}
 
+	/*
+	 * Some of MediaTek's chips won't output REFCLK when PCIE_PHY_RSTB is
+	 * asserted, we have to de-assert MAC & PHY & BRG reset signals first
+	 * to allow the REFCLK to be stable. While PCIE_BRG_RSTB is asserted,
+	 * there is a short period during which the PCIe internal register
+	 * cannot be accessed, so we need to wait 10ms here.
+	 */
+	msleep(PCIE_BRG_RST_RDY_MS);
+
+	if (!(pcie->soc->flags & SKIP_PCIE_RSTB)) {
+		/* De-assert MAC, PHY and BRG reset signals */
+		val &= ~(PCIE_MAC_RSTB | PCIE_PHY_RSTB | PCIE_BRG_RSTB);
+		writel_relaxed(val, pcie->base + PCIE_RST_CTRL_REG);
+	}
+
 	/*
 	 * Described in PCIe CEM specification revision 6.0.
 	 *
@@ -439,9 +456,8 @@ static int mtk_pcie_devices_power_up(struct mtk_gen3_pcie *pcie)
 	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);
+		/* De-assert PERST# signal */
+		val &= ~PCIE_PE_RSTB;
 		writel_relaxed(val, pcie->base + PCIE_RST_CTRL_REG);
 	}
 
@@ -459,6 +475,14 @@ static void mtk_pcie_devices_power_down(struct mtk_gen3_pcie *pcie)
 		writel_relaxed(val, pcie->base + PCIE_RST_CTRL_REG);
 	}
 
+	/*
+	 * Described in PCIe CEM specification revision 6.0.
+	 *
+	 * The PERST# gose active before the power on the connector is removed.
+	 * Wait a while to ensure the voltage transition of PERST# is completed.
+	 */
+	msleep(PCIE_MTK_PDN_PERST_TIME_MS);
+
 	pci_pwrctrl_power_off_devices(pcie->dev);
 }
 
@@ -1266,6 +1290,14 @@ static void mtk_pcie_remove(struct platform_device *pdev)
 	mtk_pcie_irq_teardown(pcie);
 }
 
+static void mtk_pcie_shutdown(struct platform_device *pdev)
+{
+	struct mtk_gen3_pcie *pcie = platform_get_drvdata(pdev);
+
+	mtk_pcie_devices_power_down(pcie);
+	mtk_pcie_power_down(pcie);
+}
+
 static void mtk_pcie_irq_save(struct mtk_gen3_pcie *pcie)
 {
 	int i;
@@ -1404,6 +1436,7 @@ MODULE_DEVICE_TABLE(of, mtk_pcie_of_match);
 static struct platform_driver mtk_pcie_driver = {
 	.probe = mtk_pcie_probe,
 	.remove = mtk_pcie_remove,
+	.shutdown = mtk_pcie_shutdown,
 	.driver = {
 		.name = "mtk-pcie-gen3",
 		.of_match_table = mtk_pcie_of_match,
-- 
2.45.2

Re: [PATCH v1] PCI: mediatek-gen3: Align PERST# sequence with PCIe CEM specification

[Chen-Yu Tsai]

On Wed, Apr 1, 2026 at 11:19 AM Jian Yang <jian.yang@mediatek.com> wrote:
>
> Fix the control sequence of PERST# during system bootup/shutdown to
> meet the requirement from PCIe CEM specification. There are two major
> changes in this patch:
>
> 1. Some of MediaTek's chip will stop generating REFCLK if the
> PCIE_PHY_RSTB signal of PCIe controller is asserted. We have to
> adjust the startup sequence as follows to ensure that PERST# will be
> de-asserted after the REFCLK is stable:
> Assert PHY reset and PERST# -> delay 10ms -> De-assert PHY reset ->
> delay 100ms -> De-assert PERST#
>
> 2. Add 'shutdown' callback to control the timing of PERST# and power
> during the system shutdown phase, ensuring that PERST# is active
> before the power on connector is removed.
>
> Signed-off-by: Jian Yang <jian.yang@mediatek.com>

Seems correct to me.

Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>

If possible, I think we would want this merged this cycle together
with the other pwrctrl changes.

Re: [PATCH v1] PCI: mediatek-gen3: Align PERST# sequence with PCIe CEM specification

[Manivannan Sadhasivam]

On Wed, Apr 01, 2026 at 11:16:42AM +0800, Jian Yang wrote:
> Fix the control sequence of PERST# during system bootup/shutdown to
> meet the requirement from PCIe CEM specification. There are two major
> changes in this patch:
> 

You are fixing multiple issues in one patch, which is not acceptable.

> 1. Some of MediaTek's chip will stop generating REFCLK if the
> PCIE_PHY_RSTB signal of PCIe controller is asserted. We have to
> adjust the startup sequence as follows to ensure that PERST# will be
> de-asserted after the REFCLK is stable:
> Assert PHY reset and PERST# -> delay 10ms -> De-assert PHY reset ->
> delay 100ms -> De-assert PERST#
> 

This issue is separate and not related to the below two issues.

> 2. Add 'shutdown' callback to control the timing of PERST# and power
> during the system shutdown phase, ensuring that PERST# is active
> before the power on connector is removed.
> 

Adding 'shutdown()' callback should belong to a separate patch with its own
motivation.

> Signed-off-by: Jian Yang <jian.yang@mediatek.com>
> ---
>  drivers/pci/controller/pcie-mediatek-gen3.c | 39 +++++++++++++++++++--
>  1 file changed, 36 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
> index a94fdbaf47fe..66d177918565 100644
> --- a/drivers/pci/controller/pcie-mediatek-gen3.c
> +++ b/drivers/pci/controller/pcie-mediatek-gen3.c
> @@ -62,6 +62,7 @@
>  #define PCIE_PHY_RSTB			BIT(1)
>  #define PCIE_BRG_RSTB			BIT(2)
>  #define PCIE_PE_RSTB			BIT(3)
> +#define PCIE_BRG_RST_RDY_MS		10
>  
>  #define PCIE_LTSSM_STATUS_REG		0x150
>  #define PCIE_LTSSM_STATE_MASK		GENMASK(28, 24)
> @@ -133,6 +134,7 @@
>  #define MAX_NUM_PHY_RESETS		3
>  
>  #define PCIE_MTK_RESET_TIME_US		10
> +#define PCIE_MTK_PDN_PERST_TIME_MS	5
>  
>  /* Time in ms needed to complete PCIe reset on EN7581 SoC */
>  #define PCIE_EN7581_RESET_TIME_MS	100
> @@ -430,6 +432,21 @@ static int mtk_pcie_devices_power_up(struct mtk_gen3_pcie *pcie)
>  		return err;
>  	}
>  
> +	/*
> +	 * Some of MediaTek's chips won't output REFCLK when PCIE_PHY_RSTB is
> +	 * asserted, we have to de-assert MAC & PHY & BRG reset signals first
> +	 * to allow the REFCLK to be stable. While PCIE_BRG_RSTB is asserted,
> +	 * there is a short period during which the PCIe internal register
> +	 * cannot be accessed, so we need to wait 10ms here.
> +	 */
> +	msleep(PCIE_BRG_RST_RDY_MS);
> +
> +	if (!(pcie->soc->flags & SKIP_PCIE_RSTB)) {
> +		/* De-assert MAC, PHY and BRG reset signals */
> +		val &= ~(PCIE_MAC_RSTB | PCIE_PHY_RSTB | PCIE_BRG_RSTB);
> +		writel_relaxed(val, pcie->base + PCIE_RST_CTRL_REG);
> +	}
> +
>  	/*
>  	 * Described in PCIe CEM specification revision 6.0.
>  	 *
> @@ -439,9 +456,8 @@ static int mtk_pcie_devices_power_up(struct mtk_gen3_pcie *pcie)
>  	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);
> +		/* De-assert PERST# signal */
> +		val &= ~PCIE_PE_RSTB;
>  		writel_relaxed(val, pcie->base + PCIE_RST_CTRL_REG);
>  	}
>  
> @@ -459,6 +475,14 @@ static void mtk_pcie_devices_power_down(struct mtk_gen3_pcie *pcie)
>  		writel_relaxed(val, pcie->base + PCIE_RST_CTRL_REG);
>  	}
>  
> +	/*
> +	 * Described in PCIe CEM specification revision 6.0.
> +	 *
> +	 * The PERST# gose active before the power on the connector is removed.
> +	 * Wait a while to ensure the voltage transition of PERST# is completed.
> +	 */
> +	msleep(PCIE_MTK_PDN_PERST_TIME_MS);

PERST# is asserted by setting the 'PCIE_PE_RSTB' bit. So if that controls the
actual voltage transition of PERST# signal, will reading it back ensure that the
transition has been completed without waiting for an arbitrary delay?

Nevertheless, this also belongs to a separate patch.

- Mani

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

Re: [PATCH v1] PCI: mediatek-gen3: Align PERST# sequence with PCIe CEM specification

[Jian Yang (杨戬)]

Hi Mani,

On Wed, 2026-04-01 at 17:49 +0530, Manivannan Sadhasivam wrote:
> On Wed, Apr 01, 2026 at 11:16:42AM +0800, Jian Yang wrote:
> > Fix the control sequence of PERST# during system bootup/shutdown to
> > meet the requirement from PCIe CEM specification. There are two
> > major
> > changes in this patch:
> > 
> 
> You are fixing multiple issues in one patch, which is not acceptable.
> 
> > 1. Some of MediaTek's chip will stop generating REFCLK if the
> > PCIE_PHY_RSTB signal of PCIe controller is asserted. We have to
> > adjust the startup sequence as follows to ensure that PERST# will
> > be
> > de-asserted after the REFCLK is stable:
> > Assert PHY reset and PERST# -> delay 10ms -> De-assert PHY reset ->
> > delay 100ms -> De-assert PERST#
> > 
> 
> This issue is separate and not related to the below two issues.
> 
> > 2. Add 'shutdown' callback to control the timing of PERST# and
> > power
> > during the system shutdown phase, ensuring that PERST# is active
> > before the power on connector is removed.
> > 
> 
> Adding 'shutdown()' callback should belong to a separate patch with
> its own
> motivation.

Thanks for your comments. I will send a new patch series.

> 
> > Signed-off-by: Jian Yang <jian.yang@mediatek.com>
> > ---
> >  drivers/pci/controller/pcie-mediatek-gen3.c | 39
> > +++++++++++++++++++--
> >  1 file changed, 36 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c
> > b/drivers/pci/controller/pcie-mediatek-gen3.c
> > index a94fdbaf47fe..66d177918565 100644
> > --- a/drivers/pci/controller/pcie-mediatek-gen3.c
> > +++ b/drivers/pci/controller/pcie-mediatek-gen3.c
> > @@ -62,6 +62,7 @@
> >  #define PCIE_PHY_RSTB			BIT(1)
> >  #define PCIE_BRG_RSTB			BIT(2)
> >  #define PCIE_PE_RSTB			BIT(3)
> > +#define PCIE_BRG_RST_RDY_MS		10
> >  
> >  #define PCIE_LTSSM_STATUS_REG		0x150
> >  #define PCIE_LTSSM_STATE_MASK		GENMASK(28, 24)
> > @@ -133,6 +134,7 @@
> >  #define MAX_NUM_PHY_RESETS		3
> >  
> >  #define PCIE_MTK_RESET_TIME_US		10
> > +#define PCIE_MTK_PDN_PERST_TIME_MS	5
> >  
> >  /* Time in ms needed to complete PCIe reset on EN7581 SoC */
> >  #define PCIE_EN7581_RESET_TIME_MS	100
> > @@ -430,6 +432,21 @@ static int mtk_pcie_devices_power_up(struct
> > mtk_gen3_pcie *pcie)
> >  		return err;
> >  	}
> >  
> > +	/*
> > +	 * Some of MediaTek's chips won't output REFCLK when
> > PCIE_PHY_RSTB is
> > +	 * asserted, we have to de-assert MAC & PHY & BRG reset signals
> > first
> > +	 * to allow the REFCLK to be stable. While PCIE_BRG_RSTB is
> > asserted,
> > +	 * there is a short period during which the PCIe internal
> > register
> > +	 * cannot be accessed, so we need to wait 10ms here.
> > +	 */
> > +	msleep(PCIE_BRG_RST_RDY_MS);
> > +
> > +	if (!(pcie->soc->flags & SKIP_PCIE_RSTB)) {
> > +		/* De-assert MAC, PHY and BRG reset signals */
> > +		val &= ~(PCIE_MAC_RSTB | PCIE_PHY_RSTB |
> > PCIE_BRG_RSTB);
> > +		writel_relaxed(val, pcie->base + PCIE_RST_CTRL_REG);
> > +	}
> > +
> >  	/*
> >  	 * Described in PCIe CEM specification revision 6.0.
> >  	 *
> > @@ -439,9 +456,8 @@ static int mtk_pcie_devices_power_up(struct
> > mtk_gen3_pcie *pcie)
> >  	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);
> > +		/* De-assert PERST# signal */
> > +		val &= ~PCIE_PE_RSTB;
> >  		writel_relaxed(val, pcie->base + PCIE_RST_CTRL_REG);
> >  	}
> >  
> > @@ -459,6 +475,14 @@ static void mtk_pcie_devices_power_down(struct
> > mtk_gen3_pcie *pcie)
> >  		writel_relaxed(val, pcie->base + PCIE_RST_CTRL_REG);
> >  	}
> >  
> > +	/*
> > +	 * Described in PCIe CEM specification revision 6.0.
> > +	 *
> > +	 * The PERST# gose active before the power on the connector is
> > removed.
> > +	 * Wait a while to ensure the voltage transition of PERST# is
> > completed.
> > +	 */
> > +	msleep(PCIE_MTK_PDN_PERST_TIME_MS);
> 
> PERST# is asserted by setting the 'PCIE_PE_RSTB' bit. So if that
> controls the
> actual voltage transition of PERST# signal, will reading it back
> ensure that the
> transition has been completed without waiting for an arbitrary delay?

You're right. I believe this delay is unnecessary. I will remove it in
the next version.

> 
> Nevertheless, this also belongs to a separate patch.
> 
> - Mani
> 

Best regards,
Jian Yang