Re: [PATCH V7 04/13] PCI: imx6: Assert PERST# before enabling regulators
From: Frank Li <Frank.li@nxp.com>
Date: 2026-03-10 15:23:55
Also in:
imx, linux-devicetree, linux-pci, lkml
On Tue, Mar 10, 2026 at 09:54:17AM +0800, Sherry Sun wrote:
According to the PCIe initialization requirements, PERST# signal should be asserted before applying power to the PCIe device, and deasserted after power and reference clock are stable. Currently, the driver enables the vpcie3v3aux regulator in imx_pcie_probe() before PERST# is asserted in imx_pcie_host_init(), which violates the PCIe power sequencing requirements. 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 a preparation patch for the upcoming changes that will parse the
Nit: This is prepare for ...
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. The vpcie3v3aux regulator is kept enabled for the entire PCIe controller lifecycle and automatically disabled on device removal via devm cleanup. Signed-off-by: Sherry Sun <redacted> ---
...
quoted hunk ↗ jump to hunk
static void imx_pcie_assert_perst(struct imx_pcie *imx_pcie, bool assert) { if (assert) {@@ -1240,6 +1249,29 @@ static int imx_pcie_host_init(struct dw_pcie_rp *pp) struct imx_pcie *imx_pcie = to_imx_pcie(pci); int ret; + 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; + }
are you sure need move this part? it is not related preset and vaux.
+
+ 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) {How about two pcie shared one vaux regulator? ...
quoted hunk ↗ jump to hunk
if (ret) {@@ -1790,9 +1816,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);
keep old dev_err_probe(), just message change to "failed to get Vaux supply". Frnak
+ imx_pcie->vpcie_aux = NULL;
+ }
imx_pcie->vpcie = devm_regulator_get_optional(&pdev->dev, "vpcie");
if (IS_ERR(imx_pcie->vpcie)) {
--
2.37.1