[{"id":3673215,"web_url":"http://patchwork.ozlabs.org/comment/3673215/","msgid":"<jwazawvhuoafkhfwpjfgccc3hz6kej7i6iwkh5be2qena2b4di@yzv6e75zezfu>","list_archive_url":null,"date":"2026-04-03T16:29:46","subject":"Re: [PATCH V10 04/13] PCI: imx6: Assert PERST# before enabling\n regulators","submitter":{"id":78905,"url":"http://patchwork.ozlabs.org/api/people/78905/","name":"Manivannan Sadhasivam","email":"mani@kernel.org"},"content":"On Thu, Apr 02, 2026 at 05:50:58PM +0800, Sherry Sun wrote:\n> According to the PCIe initialization requirements, PERST# signal should\n> be asserted before applying power to the PCIe device, and deasserted\n> after power and reference clock are stable.\n> \n\nSpec wording is not quite like this. Spec mandates asserting PERST# *before*\nstopping refclk and powering down the device and deasserting it *after* applying\npower and refclk stable.\n\nI believe you want to assert PERST# before enabling regulator to prevent the\nendpoint from functioning? If so, is it due to refclk not available yet or some\nother reason?\n\n> Currently, the driver enables the vpcie3v3aux regulator in\n> imx_pcie_probe() before PERST# is asserted in imx_pcie_host_init(),\n> which violates the PCIe power sequencing requirements. However, there\n> is no issue so far because PERST# is requested as GPIOD_OUT_HIGH in\n> imx_pcie_probe(), which guarantees that PERST# is asserted before\n> enabling the vpcie3v3aux regulator.\n> \n> This is prepare for the upcoming changes that will parse the reset\n> property using the new Root Port binding, which will use GPIOD_ASIS\n> when requesting the reset GPIO. With GPIOD_ASIS, the GPIO state is not\n> guaranteed, so explicit sequencing is required.\n> \n> Fix the power sequencing by:\n> 1. Moving vpcie3v3aux regulator enable from probe to\n>    imx_pcie_host_init(), where it can be properly sequenced with PERST#.\n> 2. Moving imx_pcie_assert_perst() before regulator and clock enable to\n>    ensure correct ordering.\n> \n> The vpcie3v3aux regulator is kept enabled for the entire PCIe controller\n> lifecycle and automatically disabled on device removal via devm cleanup.\n> \n\nvpcie3v3aux handling should be in a separate patch.\n\n- Mani\n\n> Signed-off-by: Sherry Sun <sherry.sun@nxp.com>\n> ---\n>  drivers/pci/controller/dwc/pci-imx6.c | 49 +++++++++++++++++++++------\n>  1 file changed, 39 insertions(+), 10 deletions(-)\n> \n> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c\n> index 45d70ae7e04f..948ffb75d122 100644\n> --- a/drivers/pci/controller/dwc/pci-imx6.c\n> +++ b/drivers/pci/controller/dwc/pci-imx6.c\n> @@ -166,6 +166,8 @@ struct imx_pcie {\n>  \tu32\t\t\ttx_swing_full;\n>  \tu32\t\t\ttx_swing_low;\n>  \tstruct regulator\t*vpcie;\n> +\tstruct regulator\t*vpcie_aux;\n> +\tbool\t\t\tvpcie_aux_enabled;\n>  \tstruct regulator\t*vph;\n>  \tvoid __iomem\t\t*phy_base;\n>  \n> @@ -1220,6 +1222,13 @@ static void imx_pcie_disable_device(struct pci_host_bridge *bridge,\n>  \timx_pcie_remove_lut(imx_pcie, pci_dev_id(pdev));\n>  }\n>  \n> +static void imx_pcie_vpcie_aux_disable(void *data)\n> +{\n> +\tstruct regulator *vpcie_aux = data;\n> +\n> +\tregulator_disable(vpcie_aux);\n> +}\n> +\n>  static void imx_pcie_assert_perst(struct imx_pcie *imx_pcie, bool assert)\n>  {\n>  \tif (assert) {\n> @@ -1240,6 +1249,24 @@ static int imx_pcie_host_init(struct dw_pcie_rp *pp)\n>  \tstruct imx_pcie *imx_pcie = to_imx_pcie(pci);\n>  \tint ret;\n>  \n> +\timx_pcie_assert_perst(imx_pcie, true);\n> +\n> +\t/* Keep 3.3Vaux supply enabled for the entire PCIe controller lifecycle */\n> +\tif (imx_pcie->vpcie_aux && !imx_pcie->vpcie_aux_enabled) {\n> +\t\tret = regulator_enable(imx_pcie->vpcie_aux);\n> +\t\tif (ret) {\n> +\t\t\tdev_err(dev, \"failed to enable vpcie_aux regulator: %d\\n\",\n> +\t\t\t\tret);\n> +\t\t\treturn ret;\n> +\t\t}\n> +\t\timx_pcie->vpcie_aux_enabled = true;\n> +\n> +\t\tret = devm_add_action_or_reset(dev, imx_pcie_vpcie_aux_disable,\n> +\t\t\t\t\t       imx_pcie->vpcie_aux);\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n> +\t}\n> +\n>  \tif (imx_pcie->vpcie) {\n>  \t\tret = regulator_enable(imx_pcie->vpcie);\n>  \t\tif (ret) {\n> @@ -1249,25 +1276,24 @@ static int imx_pcie_host_init(struct dw_pcie_rp *pp)\n>  \t\t}\n>  \t}\n>  \n> +\tret = imx_pcie_clk_enable(imx_pcie);\n> +\tif (ret) {\n> +\t\tdev_err(dev, \"unable to enable pcie clocks: %d\\n\", ret);\n> +\t\tgoto err_reg_disable;\n> +\t}\n> +\n>  \tif (pp->bridge && imx_check_flag(imx_pcie, IMX_PCIE_FLAG_HAS_LUT)) {\n>  \t\tpp->bridge->enable_device = imx_pcie_enable_device;\n>  \t\tpp->bridge->disable_device = imx_pcie_disable_device;\n>  \t}\n>  \n>  \timx_pcie_assert_core_reset(imx_pcie);\n> -\timx_pcie_assert_perst(imx_pcie, true);\n>  \n>  \tif (imx_pcie->drvdata->init_phy)\n>  \t\timx_pcie->drvdata->init_phy(imx_pcie);\n>  \n>  \timx_pcie_configure_type(imx_pcie);\n>  \n> -\tret = imx_pcie_clk_enable(imx_pcie);\n> -\tif (ret) {\n> -\t\tdev_err(dev, \"unable to enable pcie clocks: %d\\n\", ret);\n> -\t\tgoto err_reg_disable;\n> -\t}\n> -\n>  \tif (imx_pcie->phy) {\n>  \t\tret = phy_init(imx_pcie->phy);\n>  \t\tif (ret) {\n> @@ -1780,9 +1806,12 @@ static int imx_pcie_probe(struct platform_device *pdev)\n>  \tof_property_read_u32(node, \"fsl,max-link-speed\", &pci->max_link_speed);\n>  \timx_pcie->supports_clkreq = of_property_read_bool(node, \"supports-clkreq\");\n>  \n> -\tret = devm_regulator_get_enable_optional(&pdev->dev, \"vpcie3v3aux\");\n> -\tif (ret < 0 && ret != -ENODEV)\n> -\t\treturn dev_err_probe(dev, ret, \"failed to enable Vaux supply\\n\");\n> +\timx_pcie->vpcie_aux = devm_regulator_get_optional(&pdev->dev, \"vpcie3v3aux\");\n> +\tif (IS_ERR(imx_pcie->vpcie_aux)) {\n> +\t\tif (PTR_ERR(imx_pcie->vpcie_aux) != -ENODEV)\n> +\t\t\treturn PTR_ERR(imx_pcie->vpcie_aux);\n> +\t\timx_pcie->vpcie_aux = NULL;\n> +\t}\n>  \n>  \timx_pcie->vpcie = devm_regulator_get_optional(&pdev->dev, \"vpcie\");\n>  \tif (IS_ERR(imx_pcie->vpcie)) {\n> -- \n> 2.37.1\n>","headers":{"Return-Path":"\n <linux-pci+bounces-51819-incoming=patchwork.ozlabs.org@vger.kernel.org>","X-Original-To":["incoming@patchwork.ozlabs.org","linux-pci@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256\n header.s=k20201202 header.b=d3t2+OAH;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=172.105.105.114; helo=tor.lore.kernel.org;\n envelope-from=linux-pci+bounces-51819-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n\tdkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org\n header.b=\"d3t2+OAH\"","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=10.30.226.201"],"Received":["from tor.lore.kernel.org (tor.lore.kernel.org [172.105.105.114])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519 server-signature ECDSA (secp384r1) server-digest SHA384)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4fnPTT009lz1yCs\n\tfor <incoming@patchwork.ozlabs.org>; Sat, 04 Apr 2026 03:35:08 +1100 (AEDT)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby tor.lore.kernel.org (Postfix) with ESMTP id C22C4301B4C0\n\tfor <incoming@patchwork.ozlabs.org>; Fri,  3 Apr 2026 16:30:03 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id B619E3CCA0C;\n\tFri,  3 Apr 2026 16:30:00 +0000 (UTC)","from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org\n [10.30.226.201])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id 8B5983CA4B3;\n\tFri,  3 Apr 2026 16:30:00 +0000 (UTC)","by smtp.kernel.org (Postfix) with ESMTPSA id B757CC4CEF7;\n\tFri,  3 Apr 2026 16:29:54 +0000 (UTC)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1775233800; cv=none;\n b=tXrGWlzfLopREZ53SL1U/lHfbfYHqdJcaQUGGM5APPi2JLOT9k5JvEDNiunXwWVdaZGrLnqyFkp8167ailjHNA2o802Gs4S7vyW5drv4P+1sQjO6eZUYlEVrbmJoC5DGjXfvZAxXbJMfOp4RWjLDwVXC4Agp6v0SdxBUOxcQxnU=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1775233800; c=relaxed/simple;\n\tbh=uQCRAOjiV5WXPzI6jcTO694gx4cH1dlQgpFKmqcXA5w=;\n\th=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version:\n\t Content-Type:Content-Disposition:In-Reply-To;\n b=BPO0fj67kA8dUIl7EbB5cAEEhoiwvzJw4FZAhbLWarVVdVEswmO9ZoqDM21k7HtvZtOjH2Y3mfSuV8SHcE0DjmYIJJRXpf5YNhG4DfwCdH24ztJha7ilCbVlQU9nVuFKW5RYJ0wlkVFPBAAARD4scB85XUPuRK6gYcGzeYieK0Q=","ARC-Authentication-Results":"i=1; smtp.subspace.kernel.org;\n dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org\n header.b=d3t2+OAH; arc=none smtp.client-ip=10.30.226.201","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org;\n\ts=k20201202; t=1775233800;\n\tbh=uQCRAOjiV5WXPzI6jcTO694gx4cH1dlQgpFKmqcXA5w=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=d3t2+OAHZo0jkm8hzozNa4QRBweKwXcljiFW5lnb6ImG2r0kh/bxvTFk5MGH9pdHQ\n\t y0OmYJ54QNe/FAIJtrxCmSu/AjdYl1AWsXJtdyU8kOuQvesdIxwEfTdO3WZCXxmAXe\n\t 2dA2z5XGivG0D0urzxBuNu5sKF/8YawL8d5IDyVD2XI9nhFftRgsDzOf3FVldR9vOF\n\t 44tDIAykbbaKNmfzBMTDa6uiAot2E+jOcuaIaFFTWEb6oFFG3yM1hjN/R98jhkWr10\n\t KMqyDUeMPLa0ekmOXVPnfeU23W3Aq/xOeKiDxU7PfV0EkTO8zxhR+z3jql6Oyo317h\n\t FQickyvwtnYaA==","Date":"Fri, 3 Apr 2026 21:59:46 +0530","From":"Manivannan Sadhasivam <mani@kernel.org>","To":"Sherry Sun <sherry.sun@nxp.com>","Cc":"robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,\n\tFrank.Li@nxp.com, s.hauer@pengutronix.de, kernel@pengutronix.de,\n\tfestevam@gmail.com, lpieralisi@kernel.org, kwilczynski@kernel.org,\n\tbhelgaas@google.com, hongxing.zhu@nxp.com, l.stach@pengutronix.de,\n\timx@lists.linux.dev, linux-pci@vger.kernel.org,\n linux-arm-kernel@lists.infradead.org,\n\tdevicetree@vger.kernel.org, linux-kernel@vger.kernel.org","Subject":"Re: [PATCH V10 04/13] PCI: imx6: Assert PERST# before enabling\n regulators","Message-ID":"<jwazawvhuoafkhfwpjfgccc3hz6kej7i6iwkh5be2qena2b4di@yzv6e75zezfu>","References":"<20260402095107.205439-1-sherry.sun@nxp.com>\n <20260402095107.205439-5-sherry.sun@nxp.com>","Precedence":"bulk","X-Mailing-List":"linux-pci@vger.kernel.org","List-Id":"<linux-pci.vger.kernel.org>","List-Subscribe":"<mailto:linux-pci+subscribe@vger.kernel.org>","List-Unsubscribe":"<mailto:linux-pci+unsubscribe@vger.kernel.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20260402095107.205439-5-sherry.sun@nxp.com>"}}]