RE: [PATCH V14 03/12] PCI: imx6: Assert PERST# before enabling regulators

[Hongxing Zhu]

> -----Original Message-----
> From: Sherry Sun <sherry.sun@xxxxxxx>
> Sent: Wednesday, April 22, 2026 5:36 PM
> To: robh@xxxxxxxxxx; krzk+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx; Frank Li
> <frank.li@xxxxxxx>; s.hauer@xxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx;
> festevam@xxxxxxxxx; lpieralisi@xxxxxxxxxx; kwilczynski@xxxxxxxxxx;
> mani@xxxxxxxxxx; bhelgaas@xxxxxxxxxx; Hongxing Zhu
> <hongxing.zhu@xxxxxxx>; l.stach@xxxxxxxxxxxxxx
> Cc: imx@xxxxxxxxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx; linux-arm-
> kernel@xxxxxxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx
> Subject: [PATCH V14 03/12] PCI: imx6: Assert PERST# before enabling regulators
> 
> The PCIe endpoint may start responding or driving signals as soon as its supply is
> enabled, even before the reference clock is stable.
> Asserting PERST# before enabling the regulator ensures that the endpoint remains
> in reset throughout the entire power-up sequence, until both power and refclk
> are known to be stable and link initialization can safely begin.
> 
> Currently, the driver enables the vpcie3v3aux regulator in
> imx_pcie_probe() before PERST# is asserted in imx_pcie_host_init(), which may
> cause PCIe endpoint undefined behavior during early power-up. However, there is
> no issue so far because PERST# is requested as GPIOD_OUT_HIGH in
> imx_pcie_probe(), which guarantees that PERST# is asserted before enabling the
> vpcie3v3aux regulator.
> 
> This is prepare for the upcoming changes that will parse the reset property using
> the new Root Port binding, which will use GPIOD_ASIS when requesting the reset
> GPIO. With GPIOD_ASIS, the GPIO state is not guaranteed, so explicit sequencing
> is required.
> 
> Fix the power sequencing by:
> 1. Moving vpcie3v3aux regulator enable from probe to
>    imx_pcie_host_init(), where it can be properly sequenced with PERST#.
> 2. Moving imx_pcie_assert_perst() before regulator and clock enable to
>    ensure correct ordering.
> 
> Signed-off-by: Sherry Sun <sherry.sun@xxxxxxx>
Acked-by: Richard Zhu <hongxing.zhu@xxxxxxx>

Best Regards
Richard Zhu
> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 49 +++++++++++++++++++++------
>  1 file changed, 39 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> b/drivers/pci/controller/dwc/pci-imx6.c
> index e35044cc5218..735127ed1455 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -168,6 +168,8 @@ struct imx_pcie {
>  	u32			tx_swing_full;
>  	u32			tx_swing_low;
>  	struct regulator	*vpcie;
> +	struct regulator	*vpcie_aux;
> +	bool			vpcie_aux_enabled;
>  	struct regulator	*vph;
>  	void __iomem		*phy_base;
> 
> @@ -1222,6 +1224,13 @@ static void imx_pcie_disable_device(struct
> pci_host_bridge *bridge,
>  	imx_pcie_remove_lut(imx_pcie, pci_dev_id(pdev));  }
> 
> +static void imx_pcie_vpcie_aux_disable(void *data) {
> +	struct regulator *vpcie_aux = data;
> +
> +	regulator_disable(vpcie_aux);
> +}
> +
>  static void imx_pcie_assert_perst(struct imx_pcie *imx_pcie, bool assert)  {
>  	if (assert) {
> @@ -1242,6 +1251,24 @@ static int imx_pcie_host_init(struct dw_pcie_rp *pp)
>  	struct imx_pcie *imx_pcie = to_imx_pcie(pci);
>  	int ret;
> 
> +	imx_pcie_assert_perst(imx_pcie, true);
> +
> +	/* Keep 3.3Vaux supply enabled for the entire PCIe controller lifecycle */
> +	if (imx_pcie->vpcie_aux && !imx_pcie->vpcie_aux_enabled) {
> +		ret = regulator_enable(imx_pcie->vpcie_aux);
> +		if (ret) {
> +			dev_err(dev, "failed to enable vpcie_aux
> regulator: %d\n",
> +				ret);
> +			return ret;
> +		}
> +		imx_pcie->vpcie_aux_enabled = true;
> +
> +		ret = devm_add_action_or_reset(dev,
> imx_pcie_vpcie_aux_disable,
> +					       imx_pcie->vpcie_aux);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	if (imx_pcie->vpcie) {
>  		ret = regulator_enable(imx_pcie->vpcie);
>  		if (ret) {
> @@ -1251,25 +1278,24 @@ static int imx_pcie_host_init(struct dw_pcie_rp *pp)
>  		}
>  	}
> 
> +	ret = imx_pcie_clk_enable(imx_pcie);
> +	if (ret) {
> +		dev_err(dev, "unable to enable pcie clocks: %d\n", ret);
> +		goto err_reg_disable;
> +	}
> +
>  	if (pp->bridge && imx_check_flag(imx_pcie, IMX_PCIE_FLAG_HAS_LUT))
> {
>  		pp->bridge->enable_device = imx_pcie_enable_device;
>  		pp->bridge->disable_device = imx_pcie_disable_device;
>  	}
> 
>  	imx_pcie_assert_core_reset(imx_pcie);
> -	imx_pcie_assert_perst(imx_pcie, true);
> 
>  	if (imx_pcie->drvdata->init_phy)
>  		imx_pcie->drvdata->init_phy(imx_pcie);
> 
>  	imx_pcie_configure_type(imx_pcie);
> 
> -	ret = imx_pcie_clk_enable(imx_pcie);
> -	if (ret) {
> -		dev_err(dev, "unable to enable pcie clocks: %d\n", ret);
> -		goto err_reg_disable;
> -	}
> -
>  	if (imx_pcie->phy) {
>  		ret = phy_init(imx_pcie->phy);
>  		if (ret) {
> @@ -1782,9 +1808,12 @@ static int imx_pcie_probe(struct platform_device
> *pdev)
>  	of_property_read_u32(node, "fsl,max-link-speed", &pci-
> >max_link_speed);
>  	imx_pcie->supports_clkreq = of_property_read_bool(node, "supports-
> clkreq");
> 
> -	ret = devm_regulator_get_enable_optional(&pdev->dev, "vpcie3v3aux");
> -	if (ret < 0 && ret != -ENODEV)
> -		return dev_err_probe(dev, ret, "failed to enable Vaux supply\n");
> +	imx_pcie->vpcie_aux = devm_regulator_get_optional(&pdev->dev,
> "vpcie3v3aux");
> +	if (IS_ERR(imx_pcie->vpcie_aux)) {
> +		if (PTR_ERR(imx_pcie->vpcie_aux) != -ENODEV)
> +			return PTR_ERR(imx_pcie->vpcie_aux);
> +		imx_pcie->vpcie_aux = NULL;
> +	}
> 
>  	imx_pcie->vpcie = devm_regulator_get_optional(&pdev->dev, "vpcie");
>  	if (IS_ERR(imx_pcie->vpcie)) {
> --
> 2.37.1