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@xxxxxxxxxxxx>
> ---
>  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

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