mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/4] PCI: qcom: Align PHY init sequence with HPG
@ 2026-09-16  6:04 Qiang Yu
  2026-09-16  6:04 ` [PATCH 1/4] PCI: qcom: Move PHY init and add PHY reset call to align " Qiang Yu
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Qiang Yu @ 2026-09-16  6:04 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
	Vinod Koul, Neil Armstrong, Philipp Zabel
  Cc: linux-arm-msm, linux-pci, linux-kernel, linux-phy, Qiang Yu

HPG requires the controller/PHY GDSCs to be powered up and all
clocks enabled, then the controller's reset and the PHY's reset
toggled together as one group, with PARF_DEVICE_TYPE set right
after. This series adds the callbacks needed on both sides to
realize that sequence: a phy_reset() call the controller makes as
part of its own reset, and matching phy_ops::init/phy_ops::reset on
the PHY side.

Patches 1-2 move phy_init()/phy_exit() into the RC and EP
controllers' own init/deinit paths, add a phy_reset() call grouped
with each controller's own reset, and move the PARF_DEVICE_TYPE
write to right after that reset group.

Patches 3-4 update the two PHY drivers these controllers can use
(phy-qcom-qmp-pcie[-multiphy].c) to implement phy_ops::init/reset,
matching phy_ops::power_on to what's left once
phy_ops::init/phy_ops::reset are pulled out.

phy_init()/phy_reset() are no-ops against a phy_ops without
phy_ops::init/phy_ops::reset, so patches 1-2 don't change behavior
on any platform by themselves; patches 3-4 are what actually change
the sequence for the platforms using those two PHY drivers. Other
PHY drivers this series doesn't touch (phy-qcom-qmp-pcie-msm8996.c,
phy-qcom-uniphy-pcie-28lp.c, phy-qcom-pcie2.c) are either unaffected
(no phy_ops::init/phy_ops::reset) or only see their
phy_init()/phy_exit() call site moved without a sequence change.

Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
---
Qiang Yu (4):
      PCI: qcom: Move PHY init and add PHY reset call to align with HPG
      PCI: qcom-ep: Move PHY init and drive PHY reset to align with HPG
      phy: qcom-qmp-pcie-multiphy: Split phy_ops into init/exit/reset/power_on
      phy: qcom-qmp-pcie: Split phy_ops into init/exit/reset/power_on

 drivers/pci/controller/dwc/pcie-qcom-ep.c         |  26 +--
 drivers/pci/controller/dwc/pcie-qcom.c            | 165 ++++++++++++++-----
 drivers/phy/qualcomm/phy-qcom-qmp-pcie-multiphy.c |  94 ++++-------
 drivers/phy/qualcomm/phy-qcom-qmp-pcie.c          | 184 ++++++++++------------
 4 files changed, 257 insertions(+), 212 deletions(-)
---
base-commit: e6e35979777d646fe3c7c94dca7dd32fb25d45f4
change-id: 20260915-align_pcie_init_sequence_0916-571190abea56

Best regards,
--  
Qiang Yu <qiang.yu@oss.qualcomm.com>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/4] PCI: qcom: Move PHY init and add PHY reset call to align with HPG
  2026-09-16  6:04 [PATCH 0/4] PCI: qcom: Align PHY init sequence with HPG Qiang Yu
@ 2026-09-16  6:04 ` Qiang Yu
  2026-09-21  9:14   ` Kathiravan Thirumoorthy
  2026-09-16  6:04 ` [PATCH 2/4] PCI: qcom-ep: Move PHY init and drive PHY reset " Qiang Yu
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Qiang Yu @ 2026-09-16  6:04 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
	Vinod Koul, Neil Armstrong, Philipp Zabel
  Cc: linux-arm-msm, linux-pci, linux-kernel, linux-phy, Qiang Yu

HPG requires the controller/PHY GDSCs powered up and all controller/PHY
clocks enabled first, then the controller's reset and the PHY's reset
toggled together as a group, PARF_DEVICE_TYPE set right after, and the
rest of the PARF/DBI config left until the PHY is ready.

Make use of phy_init() and phy_reset(), and change the call flow to:

  qcom_pcie_host_init():
    phy_init()             (PHY GDSCs/regulators/clocks up)
    qcom_pcie_ops::init:
      enable controller clocks
      assert controller reset
      phy_reset()           (toggle the PHY's own reset)
      deassert controller reset
      write PARF_DEVICE_TYPE
    phy_power_on()
    qcom_pcie_ops::post_init  (rest of the PHY/PARF/DBI setup)

phy_init() moves into qcom_pcie_host_init() and phy_exit() into
qcom_pcie_host_deinit(), right before and right after they call
qcom_pcie_ops::init/qcom_pcie_ops::deinit.

Apply this to qcom_pcie_init_2_7_0()/_2_9_0()/_2_3_2()/_2_3_3().
qcom_pcie_init_2_3_3() was enabling clocks after the reset deassert; move
that ahead of the reset assert to match the same ordering. Also add or
move the PARF_DEVICE_TYPE write to right after the reset group in each of
those four functions.

For most platforms phy_init()/phy_exit()/phy_reset() are no-ops against
their phy_ops, so moving the calls around has no effect yet; the PHY's
clocks/reset actually start being driven this way once a later PHY driver
change implements phy_ops::init/phy_ops::exit/phy_ops::reset.
phy-qcom-pcie2.c does implement phy_init()/phy_exit(), but moving the call
site into qcom_pcie_host_init()/qcom_pcie_host_deinit() keeps them at the
same point relative to that platform's own clock/reset programming, so its
init sequence doesn't change either.

Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 165 +++++++++++++++++++++++++--------
 1 file changed, 126 insertions(+), 39 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index ee63a6ec99de..c1792f35f9eb 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -508,6 +508,58 @@ static enum dw_pcie_ltssm qcom_pcie_2_1_0_get_ltssm(struct qcom_pcie *pcie)
 	return (enum dw_pcie_ltssm)FIELD_GET(ELBI_SYS_STTS_LTSSM_STATE_MASK, val);
 }
 
+static void qcom_pcie_phy_exit(struct qcom_pcie *pcie)
+{
+	struct qcom_pcie_port *port;
+
+	list_for_each_entry(port, &pcie->ports, list)
+		phy_exit(port->phy);
+}
+
+static int qcom_pcie_phy_init(struct qcom_pcie *pcie)
+{
+	struct qcom_pcie_port *port, *failed_port = NULL;
+	struct device *dev = pcie->pci->dev;
+	int ret;
+
+	list_for_each_entry(port, &pcie->ports, list) {
+		ret = phy_init(port->phy);
+		if (ret) {
+			dev_err(dev, "phy init failed (%d)\n", ret);
+			failed_port = port;
+			goto err_exit_phy;
+		}
+	}
+
+	return 0;
+
+err_exit_phy:
+	list_for_each_entry(port, &pcie->ports, list) {
+		if (port == failed_port)
+			break;
+		phy_exit(port->phy);
+	}
+
+	return ret;
+}
+
+static int qcom_pcie_phy_reset(struct qcom_pcie *pcie)
+{
+	struct qcom_pcie_port *port;
+	struct device *dev = pcie->pci->dev;
+	int ret;
+
+	list_for_each_entry(port, &pcie->ports, list) {
+		ret = phy_reset(port->phy);
+		if (ret) {
+			dev_err(dev, "phy reset failed (%d)\n", ret);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
 static int qcom_pcie_get_resources_2_1_0(struct qcom_pcie *pcie)
 {
 	struct qcom_pcie_resources_2_1_0 *res = &pcie->res.v2_1_0;
@@ -791,11 +843,23 @@ static int qcom_pcie_init_2_3_2(struct qcom_pcie *pcie)
 	ret = clk_bulk_prepare_enable(res->num_clks, res->clks);
 	if (ret) {
 		dev_err(dev, "cannot prepare/enable clocks\n");
-		regulator_bulk_disable(ARRAY_SIZE(res->supplies), res->supplies);
-		return ret;
+		goto err_disable_regulators;
 	}
 
+	ret = qcom_pcie_phy_reset(pcie);
+	if (ret)
+		goto err_disable_clocks;
+
+	writel(DEVICE_TYPE_RC, pcie->parf + PARF_DEVICE_TYPE);
+
 	return 0;
+
+err_disable_clocks:
+	clk_bulk_disable_unprepare(res->num_clks, res->clks);
+err_disable_regulators:
+	regulator_bulk_disable(ARRAY_SIZE(res->supplies), res->supplies);
+
+	return ret;
 }
 
 static int qcom_pcie_post_init_2_3_2(struct qcom_pcie *pcie)
@@ -957,18 +1021,28 @@ static int qcom_pcie_init_2_3_3(struct qcom_pcie *pcie)
 	struct device *dev = pci->dev;
 	int ret;
 
+	ret = clk_bulk_prepare_enable(res->num_clks, res->clks);
+	if (ret) {
+		dev_err(dev, "cannot prepare/enable clocks\n");
+		return ret;
+	}
+
 	ret = reset_control_bulk_assert(ARRAY_SIZE(res->rst), res->rst);
 	if (ret < 0) {
 		dev_err(dev, "cannot assert resets\n");
-		return ret;
+		goto err_disable_clocks;
 	}
 
 	usleep_range(2000, 2500);
 
+	ret = qcom_pcie_phy_reset(pcie);
+	if (ret)
+		goto err_assert_resets;
+
 	ret = reset_control_bulk_deassert(ARRAY_SIZE(res->rst), res->rst);
 	if (ret < 0) {
 		dev_err(dev, "cannot deassert resets\n");
-		return ret;
+		goto err_assert_resets;
 	}
 
 	/*
@@ -977,11 +1051,7 @@ static int qcom_pcie_init_2_3_3(struct qcom_pcie *pcie)
 	 */
 	usleep_range(2000, 2500);
 
-	ret = clk_bulk_prepare_enable(res->num_clks, res->clks);
-	if (ret) {
-		dev_err(dev, "cannot prepare/enable clocks\n");
-		goto err_assert_resets;
-	}
+	writel(DEVICE_TYPE_RC, pcie->parf + PARF_DEVICE_TYPE);
 
 	return 0;
 
@@ -991,6 +1061,8 @@ static int qcom_pcie_init_2_3_3(struct qcom_pcie *pcie)
 	 * the original failure in 'ret'.
 	 */
 	reset_control_bulk_assert(ARRAY_SIZE(res->rst), res->rst);
+err_disable_clocks:
+	clk_bulk_disable_unprepare(res->num_clks, res->clks);
 
 	return ret;
 }
@@ -1066,7 +1138,6 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
 	struct qcom_pcie_resources_2_7_0 *res = &pcie->res.v2_7_0;
 	struct dw_pcie *pci = pcie->pci;
 	struct device *dev = pci->dev;
-	u32 val;
 	int ret;
 
 	ret = regulator_bulk_enable(ARRAY_SIZE(res->supplies), res->supplies);
@@ -1085,6 +1156,10 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
 		goto err_disable_clocks;
 	}
 
+	ret = qcom_pcie_phy_reset(pcie);
+	if (ret)
+		goto err_disable_clocks;
+
 	usleep_range(1000, 1500);
 
 	ret = reset_control_deassert(res->rst);
@@ -1099,6 +1174,21 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
 	/* configure PCIe to RC mode */
 	writel(DEVICE_TYPE_RC, pcie->parf + PARF_DEVICE_TYPE);
 
+	return 0;
+err_disable_clocks:
+	clk_bulk_disable_unprepare(res->num_clks, res->clks);
+err_disable_regulators:
+	regulator_bulk_disable(ARRAY_SIZE(res->supplies), res->supplies);
+
+	return ret;
+}
+
+static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie)
+{
+	const struct qcom_pcie_cfg *pcie_cfg = pcie->cfg;
+	struct dw_pcie *pci = pcie->pci;
+	u32 val;
+
 	/* Force PHY out of lowest power state */
 	val = readl(pcie->parf + PARF_PHY_CTRL);
 	val &= ~PHY_TEST_PWR_DOWN;
@@ -1126,19 +1216,6 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
 	val |= EN;
 	writel(val, pcie->parf + PARF_AXI_MSTR_WR_ADDR_HALT_V2);
 
-	return 0;
-err_disable_clocks:
-	clk_bulk_disable_unprepare(res->num_clks, res->clks);
-err_disable_regulators:
-	regulator_bulk_disable(ARRAY_SIZE(res->supplies), res->supplies);
-
-	return ret;
-}
-
-static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie)
-{
-	const struct qcom_pcie_cfg *pcie_cfg = pcie->cfg;
-
 	if (pcie_cfg->override_no_snoop)
 		writel(WR_NO_SNOOP_OVERRIDE_EN | RD_NO_SNOOP_OVERRIDE_EN,
 				pcie->parf + PARF_NO_SNOOP_OVERRIDE);
@@ -1298,10 +1375,14 @@ static int qcom_pcie_init_2_9_0(struct qcom_pcie *pcie)
 	struct device *dev = pcie->pci->dev;
 	int ret;
 
+	ret = clk_bulk_prepare_enable(res->num_clks, res->clks);
+	if (ret)
+		return ret;
+
 	ret = reset_control_assert(res->rst);
 	if (ret) {
 		dev_err(dev, "reset assert failed (%d)\n", ret);
-		return ret;
+		goto err_disable_clocks;
 	}
 
 	/*
@@ -1310,15 +1391,26 @@ static int qcom_pcie_init_2_9_0(struct qcom_pcie *pcie)
 	 */
 	usleep_range(2000, 2500);
 
+	ret = qcom_pcie_phy_reset(pcie);
+	if (ret)
+		goto err_disable_clocks;
+
 	ret = reset_control_deassert(res->rst);
 	if (ret) {
 		dev_err(dev, "reset deassert failed (%d)\n", ret);
-		return ret;
+		goto err_disable_clocks;
 	}
 
 	usleep_range(2000, 2500);
 
-	return clk_bulk_prepare_enable(res->num_clks, res->clks);
+	writel(DEVICE_TYPE_RC, pcie->parf + PARF_DEVICE_TYPE);
+
+	return 0;
+
+err_disable_clocks:
+	clk_bulk_disable_unprepare(res->num_clks, res->clks);
+
+	return ret;
 }
 
 static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
@@ -1335,7 +1427,6 @@ static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
 
 	qcom_pcie_configure_dbi_atu_base(pcie);
 
-	writel(DEVICE_TYPE_RC, pcie->parf + PARF_DEVICE_TYPE);
 	writel(BYPASS | MSTR_AXI_CLK_EN | AHB_CLK_EN,
 		pcie->parf + PARF_MHI_CLOCK_RESET_CTRL);
 	writel(GEN3_RELATED_OFF_RXEQ_RGRDLESS_RXTS |
@@ -1447,10 +1538,14 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
 
 	qcom_pcie_perst_assert(pcie);
 
-	ret = pcie->cfg->ops->init(pcie);
+	ret = qcom_pcie_phy_init(pcie);
 	if (ret)
 		return ret;
 
+	ret = pcie->cfg->ops->init(pcie);
+	if (ret)
+		goto err_exit_phy;
+
 	ret = qcom_pcie_phy_power_on(pcie);
 	if (ret)
 		goto err_deinit;
@@ -1503,6 +1598,8 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
 	qcom_pcie_phy_power_off(pcie);
 err_deinit:
 	pcie->cfg->ops->deinit(pcie);
+err_exit_phy:
+	qcom_pcie_phy_exit(pcie);
 
 	return ret;
 }
@@ -1524,6 +1621,7 @@ static void qcom_pcie_host_deinit(struct dw_pcie_rp *pp)
 
 	qcom_pcie_phy_power_off(pcie);
 	pcie->cfg->ops->deinit(pcie);
+	qcom_pcie_phy_exit(pcie);
 }
 
 static void qcom_pcie_host_post_init(struct dw_pcie_rp *pp)
@@ -2109,10 +2207,6 @@ static int qcom_pcie_parse_port(struct qcom_pcie *pcie, struct device_node *node
 	if (!port)
 		return -ENOMEM;
 
-	ret = phy_init(phy);
-	if (ret)
-		return ret;
-
 	INIT_LIST_HEAD(&port->perst);
 
 	ret = qcom_pcie_parse_perst(pcie, port, node);
@@ -2157,7 +2251,6 @@ static int qcom_pcie_parse_ports(struct qcom_pcie *pcie)
 	list_for_each_entry_safe(port, tmp_port, &pcie->ports, list) {
 		list_for_each_entry_safe(perst, tmp_perst, &port->perst, list)
 			list_del(&perst->list);
-		phy_exit(port->phy);
 		list_del(&port->list);
 	}
 
@@ -2170,16 +2263,11 @@ static int qcom_pcie_parse_legacy_binding(struct qcom_pcie *pcie)
 	struct qcom_pcie_perst *perst;
 	struct qcom_pcie_port *port;
 	struct phy *phy;
-	int ret;
 
 	phy = devm_phy_optional_get(dev, "pciephy");
 	if (IS_ERR(phy))
 		return PTR_ERR(phy);
 
-	ret = phy_init(phy);
-	if (ret)
-		return ret;
-
 	port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
 	if (!port)
 		return -ENOMEM;
@@ -2398,7 +2486,6 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 	list_for_each_entry_safe(port, tmp_port, &pcie->ports, list) {
 		list_for_each_entry_safe(perst, tmp_perst, &port->perst, list)
 			list_del(&perst->list);
-		phy_exit(port->phy);
 		list_del(&port->list);
 	}
 err_pm_runtime_put:

-- 
2.34.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/4] PCI: qcom-ep: Move PHY init and drive PHY reset to align with HPG
  2026-09-16  6:04 [PATCH 0/4] PCI: qcom: Align PHY init sequence with HPG Qiang Yu
  2026-09-16  6:04 ` [PATCH 1/4] PCI: qcom: Move PHY init and add PHY reset call to align " Qiang Yu
@ 2026-09-16  6:04 ` Qiang Yu
  2026-09-16  6:04 ` [PATCH 3/4] phy: qcom-qmp-pcie-multiphy: Split phy_ops into init/exit/reset/power_on Qiang Yu
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Qiang Yu @ 2026-09-16  6:04 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
	Vinod Koul, Neil Armstrong, Philipp Zabel
  Cc: linux-arm-msm, linux-pci, linux-kernel, linux-phy, Qiang Yu

HPG requires the GDSC to be powered up and all PHY/controller clocks
enabled before the controller's core reset and the PHY's reset are toggled
together as one atomic group, with PARF_DEVICE_TYPE set right after that
reset group, same as the RC-mode change.

Move phy_init() ahead of the controller's own clock enable, and fold
phy_reset() into the core reset assert/deassert. phy_power_on() keeps
running afterwards. Also move the PARF_DEVICE_TYPE write out of
qcom_pcie_perst_deassert() to right after the reset group, in
qcom_pcie_enable_resources().

phy_reset() and phy_init are currently a no-op against the PHY this driver
uses, so the init sequence doesn't change yet; it actually changes once
the PHY driver later implements .reset.

Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-qcom-ep.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
index 8364696a1b98..cd7725d277c5 100644
--- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
+++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
@@ -228,7 +228,7 @@ struct qcom_pcie_ep {
 	int perst_irq;
 };
 
-static int qcom_pcie_ep_core_reset(struct qcom_pcie_ep *pcie_ep)
+static int qcom_pcie_ep_phy_core_reset(struct qcom_pcie_ep *pcie_ep)
 {
 	struct dw_pcie *pci = &pcie_ep->pci;
 	struct device *dev = pci->dev;
@@ -242,6 +242,12 @@ static int qcom_pcie_ep_core_reset(struct qcom_pcie_ep *pcie_ep)
 
 	usleep_range(CORE_RESET_TIME_US_MIN, CORE_RESET_TIME_US_MAX);
 
+	ret = phy_reset(pcie_ep->phy);
+	if (ret) {
+		dev_err(dev, "Cannot reset phy\n");
+		return ret;
+	}
+
 	ret = reset_control_deassert(pcie_ep->core_reset);
 	if (ret) {
 		dev_err(dev, "Cannot de-assert core reset\n");
@@ -332,25 +338,25 @@ static int qcom_pcie_enable_resources(struct qcom_pcie_ep *pcie_ep)
 	struct dw_pcie *pci = &pcie_ep->pci;
 	int ret;
 
-	ret = clk_bulk_prepare_enable(pcie_ep->num_clks, pcie_ep->clks);
+	ret = phy_init(pcie_ep->phy);
 	if (ret)
 		return ret;
 
-	ret = qcom_pcie_ep_core_reset(pcie_ep);
+	ret = clk_bulk_prepare_enable(pcie_ep->num_clks, pcie_ep->clks);
 	if (ret)
-		goto err_disable_clk;
+		goto err_phy_exit;
 
-	ret = phy_init(pcie_ep->phy);
+	ret = qcom_pcie_ep_phy_core_reset(pcie_ep);
 	if (ret)
 		goto err_disable_clk;
 
 	ret = phy_set_mode_ext(pcie_ep->phy, PHY_MODE_PCIE, PHY_MODE_PCIE_EP);
 	if (ret)
-		goto err_phy_exit;
+		goto err_disable_clk;
 
 	ret = phy_power_on(pcie_ep->phy);
 	if (ret)
-		goto err_phy_exit;
+		goto err_disable_clk;
 
 	/*
 	 * Some Qualcomm platforms require interconnect bandwidth constraints
@@ -370,10 +376,10 @@ static int qcom_pcie_enable_resources(struct qcom_pcie_ep *pcie_ep)
 
 err_phy_off:
 	phy_power_off(pcie_ep->phy);
-err_phy_exit:
-	phy_exit(pcie_ep->phy);
 err_disable_clk:
 	clk_bulk_disable_unprepare(pcie_ep->num_clks, pcie_ep->clks);
+err_phy_exit:
+	phy_exit(pcie_ep->phy);
 
 	return ret;
 }
@@ -390,8 +396,8 @@ static void qcom_pcie_disable_resources(struct qcom_pcie_ep *pcie_ep)
 
 	icc_set_bw(pcie_ep->icc_mem, 0, 0);
 	phy_power_off(pcie_ep->phy);
-	phy_exit(pcie_ep->phy);
 	clk_bulk_disable_unprepare(pcie_ep->num_clks, pcie_ep->clks);
+	phy_exit(pcie_ep->phy);
 }
 
 static int qcom_pcie_perst_deassert(struct dw_pcie *pci)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 3/4] phy: qcom-qmp-pcie-multiphy: Split phy_ops into init/exit/reset/power_on
  2026-09-16  6:04 [PATCH 0/4] PCI: qcom: Align PHY init sequence with HPG Qiang Yu
  2026-09-16  6:04 ` [PATCH 1/4] PCI: qcom: Move PHY init and add PHY reset call to align " Qiang Yu
  2026-09-16  6:04 ` [PATCH 2/4] PCI: qcom-ep: Move PHY init and drive PHY reset " Qiang Yu
@ 2026-09-16  6:04 ` Qiang Yu
  2026-09-16  6:04 ` [PATCH 4/4] phy: qcom-qmp-pcie: " Qiang Yu
  2026-09-16 15:55 ` [PATCH 0/4] PCI: qcom: Align PHY init sequence with HPG Bjorn Helgaas
  4 siblings, 0 replies; 9+ messages in thread
From: Qiang Yu @ 2026-09-16  6:04 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
	Vinod Koul, Neil Armstrong, Philipp Zabel
  Cc: linux-arm-msm, linux-pci, linux-kernel, linux-phy, Qiang Yu

Per HPG, the controller now calls phy_init() from
qcom_pcie_host_init() and phy_reset() from inside its own core reset
assert/deassert. Making that actually happen here needs phy_ops::init
and phy_ops::reset implemented, and phy_ops::power_on adjusted to
match. Split the combined phy_ops::power_on/power_off into
phy_ops::init/exit/reset/power_on to do that.

phy_ops::init enables power domains, regulators, and all PHY clocks
(pipe clock first). phy_ops::reset does the no-CSR reset assert/
delay/deassert/delay. phy_ops::power_on is left with just the PCS
status poll. phy_ops::exit mirrors phy_ops::init in reverse;
phy_ops::power_off is dropped since nothing remains for it to do.

Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
---
 drivers/phy/qualcomm/phy-qcom-qmp-pcie-multiphy.c | 94 +++++++++--------------
 1 file changed, 35 insertions(+), 59 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie-multiphy.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie-multiphy.c
index e93cba4369fb..aa7130efca45 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie-multiphy.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie-multiphy.c
@@ -280,20 +280,18 @@ static int qmp_pcie_init(struct phy *phy)
 		goto err_pd_power_off;
 	}
 
-	ret = reset_control_bulk_assert(qmp->cfg->num_nocsr_resets, qmp->nocsr_resets);
-	if (ret) {
-		dev_err(qmp->dev, "no-csr reset assert failed: %d\n", ret);
+	ret = clk_bulk_prepare_enable(qmp->cfg->num_pipe_clks, qmp->pipe_clks);
+	if (ret)
 		goto err_disable_regulators;
-	}
-
-	usleep_range(200, 300);
 
 	ret = clk_bulk_prepare_enable(qmp->cfg->num_clks, qmp->clks);
 	if (ret)
-		goto err_disable_regulators;
+		goto err_disable_pipe_clks;
 
 	return 0;
 
+err_disable_pipe_clks:
+	clk_bulk_disable_unprepare(qmp->cfg->num_pipe_clks, qmp->pipe_clks);
 err_disable_regulators:
 	regulator_bulk_disable(cfg->num_vregs, qmp->vregs);
 err_pd_power_off:
@@ -310,31 +308,47 @@ static int qmp_pcie_exit(struct phy *phy)
 	reset_control_bulk_assert(qmp->cfg->num_nocsr_resets, qmp->nocsr_resets);
 
 	clk_bulk_disable_unprepare(qmp->cfg->num_clks, qmp->clks);
+	clk_bulk_disable_unprepare(qmp->cfg->num_pipe_clks, qmp->pipe_clks);
 	regulator_bulk_disable(cfg->num_vregs, qmp->vregs);
 	qmp_pcie_pd_power_off(qmp);
 
 	return 0;
 }
 
-static int qmp_pcie_power_on(struct phy *phy)
+static int qmp_pcie_reset(struct phy *phy)
 {
 	struct qmp_pcie *qmp = phy_get_drvdata(phy);
 	const struct qmp_phy_cfg *cfg = qmp->cfg;
-	const struct qmp_pcie_offsets *offs = cfg->offsets;
-	void __iomem *status;
-	unsigned int val;
-	int i, ret;
+	int ret;
 
-	ret = clk_bulk_prepare_enable(qmp->cfg->num_pipe_clks, qmp->pipe_clks);
-	if (ret)
+	ret = reset_control_bulk_assert(cfg->num_nocsr_resets, qmp->nocsr_resets);
+	if (ret) {
+		dev_err(qmp->dev, "no-csr reset assert failed: %d\n", ret);
 		return ret;
+	}
 
-	ret = reset_control_bulk_deassert(qmp->cfg->num_nocsr_resets, qmp->nocsr_resets);
+	udelay(5);
+
+	ret = reset_control_bulk_deassert(cfg->num_nocsr_resets, qmp->nocsr_resets);
 	if (ret) {
 		dev_err(qmp->dev, "no-csr reset deassert failed: %d\n", ret);
-		goto err_disable_pipe_clk;
+		return ret;
 	}
 
+	udelay(5);
+
+	return 0;
+}
+
+static int qmp_pcie_power_on(struct phy *phy)
+{
+	struct qmp_pcie *qmp = phy_get_drvdata(phy);
+	const struct qmp_phy_cfg *cfg = qmp->cfg;
+	const struct qmp_pcie_offsets *offs = cfg->offsets;
+	void __iomem *status;
+	unsigned int val;
+	int i, ret;
+
 	for (i = 0; i < cfg->num_regs; i++) {
 		status = qmp->base[i] + offs->pcs + cfg->regs[QPHY_PCS_STATUS];
 		ret = readl_poll_timeout(status, val, !(val & cfg->phy_status), 200,
@@ -342,56 +356,18 @@ static int qmp_pcie_power_on(struct phy *phy)
 		if (ret) {
 			dev_err(qmp->dev, "PHY power on timed-out (%s): %d\n",
 				cfg->reg_names[i], ret);
-			goto err_disable_pipe_clk;
+			return ret;
 		}
 	}
 
 	return 0;
-
-err_disable_pipe_clk:
-	clk_bulk_disable_unprepare(qmp->cfg->num_pipe_clks, qmp->pipe_clks);
-
-	return ret;
-}
-
-static int qmp_pcie_power_off(struct phy *phy)
-{
-	struct qmp_pcie *qmp = phy_get_drvdata(phy);
-
-	clk_bulk_disable_unprepare(qmp->cfg->num_pipe_clks, qmp->pipe_clks);
-
-	return 0;
-}
-
-static int qmp_pcie_enable(struct phy *phy)
-{
-	int ret;
-
-	ret = qmp_pcie_init(phy);
-	if (ret)
-		return ret;
-
-	ret = qmp_pcie_power_on(phy);
-	if (ret)
-		qmp_pcie_exit(phy);
-
-	return ret;
-}
-
-static int qmp_pcie_disable(struct phy *phy)
-{
-	int ret;
-
-	ret = qmp_pcie_power_off(phy);
-	if (ret)
-		return ret;
-
-	return qmp_pcie_exit(phy);
 }
 
 static const struct phy_ops qmp_pcie_phy_ops = {
-	.power_on	= qmp_pcie_enable,
-	.power_off	= qmp_pcie_disable,
+	.init		= qmp_pcie_init,
+	.exit		= qmp_pcie_exit,
+	.power_on	= qmp_pcie_power_on,
+	.reset		= qmp_pcie_reset,
 	.owner		= THIS_MODULE,
 };
 

-- 
2.34.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 4/4] phy: qcom-qmp-pcie: Split phy_ops into init/exit/reset/power_on
  2026-09-16  6:04 [PATCH 0/4] PCI: qcom: Align PHY init sequence with HPG Qiang Yu
                   ` (2 preceding siblings ...)
  2026-09-16  6:04 ` [PATCH 3/4] phy: qcom-qmp-pcie-multiphy: Split phy_ops into init/exit/reset/power_on Qiang Yu
@ 2026-09-16  6:04 ` Qiang Yu
  2026-09-21  9:15   ` Kathiravan Thirumoorthy
  2026-09-16 15:55 ` [PATCH 0/4] PCI: qcom: Align PHY init sequence with HPG Bjorn Helgaas
  4 siblings, 1 reply; 9+ messages in thread
From: Qiang Yu @ 2026-09-16  6:04 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
	Vinod Koul, Neil Armstrong, Philipp Zabel
  Cc: linux-arm-msm, linux-pci, linux-kernel, linux-phy, Qiang Yu

The controller now calls phy_init() from
qcom_pcie_host_init()/qcom_pcie_enable_resources() and phy_reset()
from inside its own core reset assert/deassert, so the PHY's reset
toggles alongside the controller's core reset as HPG requires.
Making that actually happen here needs phy_ops::init and
phy_ops::reset implemented, and phy_ops::power_on adjusted to match.
Split the combined phy_ops::power_on/power_off into
phy_ops::init/exit/reset/power_on to do that.

phy_ops::init enables regulators and all clocks (pipe clock first)
and determines skip_init/skip_reset. phy_ops::reset toggles the
reset that skip_init picked. Register-table programming used to run
while the no-CSR reset was still asserted; now it runs afterwards,
in phy_ops::power_on.

Adds skip_reset to struct qmp_pcie so phy_ops::reset and
phy_ops::power_on share the value phy_ops::init determined instead
of re-reading PCS registers.

Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
---
 drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 184 ++++++++++++++-----------------
 1 file changed, 80 insertions(+), 104 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
index d30194c6ed17..2b6741a4120c 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
@@ -4058,6 +4058,7 @@ struct qmp_pcie {
 	const struct qmp_phy_cfg *cfg;
 	bool tcsr_4ln_config;
 	bool skip_init;
+	bool skip_reset;
 
 	void __iomem *serdes;
 	void __iomem *pcs;
@@ -5747,9 +5748,22 @@ static int qmp_pcie_init(struct phy *phy)
 	struct qmp_pcie *qmp = phy_get_drvdata(phy);
 	const struct qmp_phy_cfg *cfg = qmp->cfg;
 	void __iomem *pcs = qmp->pcs;
-	bool skip_reset;
 	int ret;
 
+	ret = regulator_bulk_enable(cfg->num_vregs, qmp->vregs);
+	if (ret) {
+		dev_err(qmp->dev, "failed to enable regulators, err=%d\n", ret);
+		return ret;
+	}
+
+	ret = clk_bulk_prepare_enable(qmp->num_pipe_clks, qmp->pipe_clks);
+	if (ret)
+		goto err_disable_regulators;
+
+	ret = clk_bulk_prepare_enable(ARRAY_SIZE(qmp_pciephy_clk_l), qmp->clks);
+	if (ret)
+		goto err_disable_pipe_clks;
+
 	/*
 	 * We can skip PHY initialization if all of the following conditions
 	 * are met:
@@ -5763,59 +5777,21 @@ static int qmp_pcie_init(struct phy *phy)
 		qphy_checkbits(pcs, cfg->regs[QPHY_START_CTRL], SERDES_START | PCS_START) &&
 		qphy_checkbits(pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL], cfg->pwrdn_ctrl);
 
-	skip_reset = qmp->skip_init && !qphy_checkbits(pcs, cfg->regs[QPHY_PCS_STATUS],
+	qmp->skip_reset = qmp->skip_init && !qphy_checkbits(pcs, cfg->regs[QPHY_PCS_STATUS],
 							    cfg->phy_status);
 
 	if (!qmp->skip_init && !cfg->tbls.serdes_num) {
 		dev_err(qmp->dev, "Init sequence not available\n");
-		return -ENODATA;
-	}
-
-	ret = regulator_bulk_enable(cfg->num_vregs, qmp->vregs);
-	if (ret) {
-		dev_err(qmp->dev, "failed to enable regulators, err=%d\n", ret);
-		return ret;
-	}
-
-	/*
-	 * Toggle BCR reset for PHY that doesn't support no_csr reset or has not
-	 * been initialized.
-	 */
-	if (!qmp->skip_init) {
-		ret = reset_control_bulk_assert(cfg->num_resets, qmp->resets);
-		if (ret) {
-			dev_err(qmp->dev, "reset assert failed\n");
-			goto err_disable_regulators;
-		}
-	}
-
-	if (!skip_reset) {
-		ret = reset_control_assert(qmp->nocsr_reset);
-		if (ret) {
-			dev_err(qmp->dev, "no-csr reset assert failed\n");
-			goto err_assert_reset;
-		}
-
-		usleep_range(200, 300);
-	}
-
-	if (!qmp->skip_init) {
-		ret = reset_control_bulk_deassert(cfg->num_resets, qmp->resets);
-		if (ret) {
-			dev_err(qmp->dev, "reset deassert failed\n");
-			goto err_assert_reset;
-		}
+		ret = -ENODATA;
+		goto err_disable_clks;
 	}
 
-	ret = clk_bulk_prepare_enable(ARRAY_SIZE(qmp_pciephy_clk_l), qmp->clks);
-	if (ret)
-		goto err_assert_reset;
-
 	return 0;
 
-err_assert_reset:
-	if (!qmp->skip_init)
-		reset_control_bulk_assert(cfg->num_resets, qmp->resets);
+err_disable_clks:
+	clk_bulk_disable_unprepare(ARRAY_SIZE(qmp_pciephy_clk_l), qmp->clks);
+err_disable_pipe_clks:
+	clk_bulk_disable_unprepare(qmp->num_pipe_clks, qmp->pipe_clks);
 err_disable_regulators:
 	regulator_bulk_disable(cfg->num_vregs, qmp->vregs);
 
@@ -5833,12 +5809,62 @@ static int qmp_pcie_exit(struct phy *phy)
 		reset_control_bulk_assert(cfg->num_resets, qmp->resets);
 
 	clk_bulk_disable_unprepare(ARRAY_SIZE(qmp_pciephy_clk_l), qmp->clks);
+	clk_bulk_disable_unprepare(qmp->num_pipe_clks, qmp->pipe_clks);
 
 	regulator_bulk_disable(cfg->num_vregs, qmp->vregs);
 
 	return 0;
 }
 
+static int qmp_pcie_reset(struct phy *phy)
+{
+	struct qmp_pcie *qmp = phy_get_drvdata(phy);
+	const struct qmp_phy_cfg *cfg = qmp->cfg;
+	int ret;
+
+	if (qmp->skip_reset)
+		return 0;
+
+	if (qmp->skip_init) {
+		ret = reset_control_assert(qmp->nocsr_reset);
+		if (ret) {
+			dev_err(qmp->dev, "no-csr reset assert failed\n");
+			return ret;
+		}
+
+		usleep_range(200, 300);
+
+		ret = reset_control_deassert(qmp->nocsr_reset);
+		if (ret) {
+			dev_err(qmp->dev, "no-csr reset deassert failed\n");
+			return ret;
+		}
+
+		return 0;
+	}
+
+	/*
+	 * Toggle BCR reset for PHY that doesn't support no_csr reset or has not
+	 * been initialized.
+	 */
+	ret = reset_control_bulk_assert(cfg->num_resets, qmp->resets);
+	if (ret) {
+		dev_err(qmp->dev, "reset assert failed\n");
+		return ret;
+	}
+
+	usleep_range(200, 300);
+
+	ret = reset_control_bulk_deassert(cfg->num_resets, qmp->resets);
+	if (ret) {
+		dev_err(qmp->dev, "reset deassert failed\n");
+		reset_control_bulk_assert(cfg->num_resets, qmp->resets);
+		return ret;
+	}
+
+	return 0;
+}
+
 static int qmp_pcie_power_on(struct phy *phy)
 {
 	struct qmp_pcie *qmp = phy_get_drvdata(phy);
@@ -5847,17 +5873,14 @@ static int qmp_pcie_power_on(struct phy *phy)
 	void __iomem *pcs = qmp->pcs;
 	void __iomem *status;
 	unsigned int mask, val;
-	bool skip_reset;
 	int ret;
 
-	skip_reset = qmp->skip_init && !qphy_checkbits(pcs, cfg->regs[QPHY_PCS_STATUS],
-							    cfg->phy_status);
 	/*
 	 * Write CSR register for PHY that doesn't support no_csr reset or has not
 	 * been initialized.
 	 */
 	if (qmp->skip_init)
-		goto skip_tbls_init;
+		goto skip_serdes_start;
 
 	qphy_setbits(pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL],
 			cfg->pwrdn_ctrl);
@@ -5870,22 +5893,6 @@ static int qmp_pcie_power_on(struct phy *phy)
 	qmp_pcie_init_registers(qmp, &cfg->tbls);
 	qmp_pcie_init_registers(qmp, mode_tbls);
 
-skip_tbls_init:
-	ret = clk_bulk_prepare_enable(qmp->num_pipe_clks, qmp->pipe_clks);
-	if (ret)
-		return ret;
-
-	if (!skip_reset) {
-		ret = reset_control_deassert(qmp->nocsr_reset);
-		if (ret) {
-			dev_err(qmp->dev, "no-csr reset deassert failed\n");
-			goto err_disable_pipe_clk;
-		}
-	}
-
-	if (qmp->skip_init)
-		goto skip_serdes_start;
-
 	/* Pull PHY out of reset state */
 	qphy_clrbits(pcs, cfg->regs[QPHY_SW_RESET], SW_RESET);
 
@@ -5902,15 +5909,10 @@ static int qmp_pcie_power_on(struct phy *phy)
 				 PHY_INIT_COMPLETE_TIMEOUT);
 	if (ret) {
 		dev_err(qmp->dev, "phy initialization timed-out\n");
-		goto err_disable_pipe_clk;
+		return ret;
 	}
 
 	return 0;
-
-err_disable_pipe_clk:
-	clk_bulk_disable_unprepare(qmp->num_pipe_clks, qmp->pipe_clks);
-
-	return ret;
 }
 
 static int qmp_pcie_power_off(struct phy *phy)
@@ -5918,8 +5920,6 @@ static int qmp_pcie_power_off(struct phy *phy)
 	struct qmp_pcie *qmp = phy_get_drvdata(phy);
 	const struct qmp_phy_cfg *cfg = qmp->cfg;
 
-	clk_bulk_disable_unprepare(qmp->num_pipe_clks, qmp->pipe_clks);
-
 	/*
 	 * While powering off the PHY, only qmp->nocsr_reset needs to be checked. In
 	 * this way, no matter whether the PHY settings were initially programmed by
@@ -5927,7 +5927,7 @@ static int qmp_pcie_power_off(struct phy *phy)
 	 * next time.
 	 */
 	if (qmp->nocsr_reset)
-		goto skip_phy_deinit;
+		return 0;
 
 	/* PHY reset */
 	qphy_setbits(qmp->pcs, cfg->regs[QPHY_SW_RESET], SW_RESET);
@@ -5940,36 +5940,9 @@ static int qmp_pcie_power_off(struct phy *phy)
 	qphy_clrbits(qmp->pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL],
 			cfg->pwrdn_ctrl);
 
-skip_phy_deinit:
 	return 0;
 }
 
-static int qmp_pcie_enable(struct phy *phy)
-{
-	int ret;
-
-	ret = qmp_pcie_init(phy);
-	if (ret)
-		return ret;
-
-	ret = qmp_pcie_power_on(phy);
-	if (ret)
-		qmp_pcie_exit(phy);
-
-	return ret;
-}
-
-static int qmp_pcie_disable(struct phy *phy)
-{
-	int ret;
-
-	ret = qmp_pcie_power_off(phy);
-	if (ret)
-		return ret;
-
-	return qmp_pcie_exit(phy);
-}
-
 static int qmp_pcie_set_mode(struct phy *phy, enum phy_mode mode, int submode)
 {
 	struct qmp_pcie *qmp = phy_get_drvdata(phy);
@@ -5988,8 +5961,11 @@ static int qmp_pcie_set_mode(struct phy *phy, enum phy_mode mode, int submode)
 }
 
 static const struct phy_ops qmp_pcie_phy_ops = {
-	.power_on	= qmp_pcie_enable,
-	.power_off	= qmp_pcie_disable,
+	.init		= qmp_pcie_init,
+	.exit		= qmp_pcie_exit,
+	.power_on	= qmp_pcie_power_on,
+	.power_off	= qmp_pcie_power_off,
+	.reset		= qmp_pcie_reset,
 	.set_mode	= qmp_pcie_set_mode,
 	.owner		= THIS_MODULE,
 };

-- 
2.34.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/4] PCI: qcom: Align PHY init sequence with HPG
  2026-09-16  6:04 [PATCH 0/4] PCI: qcom: Align PHY init sequence with HPG Qiang Yu
                   ` (3 preceding siblings ...)
  2026-09-16  6:04 ` [PATCH 4/4] phy: qcom-qmp-pcie: " Qiang Yu
@ 2026-09-16 15:55 ` Bjorn Helgaas
  2026-09-21 11:16   ` Manivannan Sadhasivam
  4 siblings, 1 reply; 9+ messages in thread
From: Bjorn Helgaas @ 2026-09-16 15:55 UTC (permalink / raw)
  To: Qiang Yu
  Cc: Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
	Vinod Koul, Neil Armstrong, Philipp Zabel, linux-arm-msm,
	linux-pci, linux-kernel, linux-phy

On Tue, Sep 15, 2026 at 11:04:20PM -0700, Qiang Yu wrote:
> HPG requires the controller/PHY GDSCs to be powered up and all
> clocks enabled, then the controller's reset and the PHY's reset
> toggled together as one group, with PARF_DEVICE_TYPE set right
> after. This series adds the callbacks needed on both sides to
> realize that sequence: a phy_reset() call the controller makes as
> part of its own reset, and matching phy_ops::init/phy_ops::reset on
> the PHY side.

This observation is not related to this series; it's a question about
PCI host controller initialization ordering in general because it
seems like this is a perennial issue.  Is there anything we can do to
converge on some sort of generic standard or typical ordering across
drivers?

For example, while looking at a patch that changed ordering in a
different driver (not qcom), I asked Gemini for some guidance and it
came up with the following ordering recommendations, which seem
pretty generic:

  - assert downstream PERST# so physical lanes remain quiet while
    setting up the host

  - assert local resets (core & PHY)

  - enable and stabilize power (IP core, I/O rails, PHYs)

  - enable and stabilize clocks, including refclk and T_PERST_CLK

  - initialize PHYs

    - deassert PHY reset or allow register access to PHY

    - wait for PHY PLL lock to refclk

    - program PHY parameters & calibration

    - verify PHY ready

  - deassert local core reset

  - configure RC (device type, MPS, max link speed, width, etc)

  - deassert downstream PERST#

  - enable LTSSM

If this could be made generic enough, maybe something like it could be
included in Documentation/PCI/controller/pci-controller-drivers.rst?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/4] PCI: qcom: Move PHY init and add PHY reset call to align with HPG
  2026-09-16  6:04 ` [PATCH 1/4] PCI: qcom: Move PHY init and add PHY reset call to align " Qiang Yu
@ 2026-09-21  9:14   ` Kathiravan Thirumoorthy
  0 siblings, 0 replies; 9+ messages in thread
From: Kathiravan Thirumoorthy @ 2026-09-21  9:14 UTC (permalink / raw)
  To: Qiang Yu, Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
	Vinod Koul, Neil Armstrong, Philipp Zabel
  Cc: linux-arm-msm, linux-pci, linux-kernel, linux-phy


On 9/16/2026 11:34 AM, Qiang Yu wrote:
> HPG requires the controller/PHY GDSCs powered up and all controller/PHY
> clocks enabled first, then the controller's reset and the PHY's reset
> toggled together as a group, PARF_DEVICE_TYPE set right after, and the
> rest of the PARF/DBI config left until the PHY is ready.
>
> Make use of phy_init() and phy_reset(), and change the call flow to:
>
>    qcom_pcie_host_init():
>      phy_init()             (PHY GDSCs/regulators/clocks up)
>      qcom_pcie_ops::init:
>        enable controller clocks
>        assert controller reset
>        phy_reset()           (toggle the PHY's own reset)
>        deassert controller reset
>        write PARF_DEVICE_TYPE
>      phy_power_on()
>      qcom_pcie_ops::post_init  (rest of the PHY/PARF/DBI setup)
>
> phy_init() moves into qcom_pcie_host_init() and phy_exit() into
> qcom_pcie_host_deinit(), right before and right after they call
> qcom_pcie_ops::init/qcom_pcie_ops::deinit.
>
> Apply this to qcom_pcie_init_2_7_0()/_2_9_0()/_2_3_2()/_2_3_3().
> qcom_pcie_init_2_3_3() was enabling clocks after the reset deassert; move
> that ahead of the reset assert to match the same ordering. Also add or
> move the PARF_DEVICE_TYPE write to right after the reset group in each of
> those four functions.
>
> For most platforms phy_init()/phy_exit()/phy_reset() are no-ops against
> their phy_ops, so moving the calls around has no effect yet; the PHY's
> clocks/reset actually start being driven this way once a later PHY driver
> change implements phy_ops::init/phy_ops::exit/phy_ops::reset.
> phy-qcom-pcie2.c does implement phy_init()/phy_exit(), but moving the call
> site into qcom_pcie_host_init()/qcom_pcie_host_deinit() keeps them at the
> same point relative to that platform's own clock/reset programming, so its
> init sequence doesn't change either.
>
> Signed-off-by: Qiang Yu<qiang.yu@oss.qualcomm.com>
> ---
>   drivers/pci/controller/dwc/pcie-qcom.c | 165 +++++++++++++++++++++++++--------
>   1 file changed, 126 insertions(+), 39 deletions(-)

Verified the PCIe enumeration in IPQ9574 and IPQ5424 and it works fine. So,

Tested-by: Kathiravan Thirumoorthy 
<kathiravan.thirumoorthy@oss.qualcomm.com> #IPQ9574-RDP433, IPQ5424-RDP466


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 4/4] phy: qcom-qmp-pcie: Split phy_ops into init/exit/reset/power_on
  2026-09-16  6:04 ` [PATCH 4/4] phy: qcom-qmp-pcie: " Qiang Yu
@ 2026-09-21  9:15   ` Kathiravan Thirumoorthy
  0 siblings, 0 replies; 9+ messages in thread
From: Kathiravan Thirumoorthy @ 2026-09-21  9:15 UTC (permalink / raw)
  To: Qiang Yu, Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
	Vinod Koul, Neil Armstrong, Philipp Zabel
  Cc: linux-arm-msm, linux-pci, linux-kernel, linux-phy


On 9/16/2026 11:34 AM, Qiang Yu wrote:
> The controller now calls phy_init() from
> qcom_pcie_host_init()/qcom_pcie_enable_resources() and phy_reset()
> from inside its own core reset assert/deassert, so the PHY's reset
> toggles alongside the controller's core reset as HPG requires.
> Making that actually happen here needs phy_ops::init and
> phy_ops::reset implemented, and phy_ops::power_on adjusted to match.
> Split the combined phy_ops::power_on/power_off into
> phy_ops::init/exit/reset/power_on to do that.
>
> phy_ops::init enables regulators and all clocks (pipe clock first)
> and determines skip_init/skip_reset. phy_ops::reset toggles the
> reset that skip_init picked. Register-table programming used to run
> while the no-CSR reset was still asserted; now it runs afterwards,
> in phy_ops::power_on.
>
> Adds skip_reset to struct qmp_pcie so phy_ops::reset and
> phy_ops::power_on share the value phy_ops::init determined instead
> of re-reading PCS registers.
>
> Signed-off-by: Qiang Yu<qiang.yu@oss.qualcomm.com>
> ---
>   drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 184 ++++++++++++++-----------------
>   1 file changed, 80 insertions(+), 104 deletions(-)

Verified the PCIe enumeration in IPQ9574 and IPQ5424 and it works fine. So,

Tested-by: Kathiravan Thirumoorthy 
<kathiravan.thirumoorthy@oss.qualcomm.com> #IPQ9574-RDP433, IPQ5424-RDP466



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/4] PCI: qcom: Align PHY init sequence with HPG
  2026-09-16 15:55 ` [PATCH 0/4] PCI: qcom: Align PHY init sequence with HPG Bjorn Helgaas
@ 2026-09-21 11:16   ` Manivannan Sadhasivam
  0 siblings, 0 replies; 9+ messages in thread
From: Manivannan Sadhasivam @ 2026-09-21 11:16 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Qiang Yu, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Rob Herring, Bjorn Helgaas, Vinod Koul, Neil Armstrong,
	Philipp Zabel, linux-arm-msm, linux-pci, linux-kernel, linux-phy

On Wed, Sep 16, 2026 at 10:55:04AM -0500, Bjorn Helgaas wrote:
> On Tue, Sep 15, 2026 at 11:04:20PM -0700, Qiang Yu wrote:
> > HPG requires the controller/PHY GDSCs to be powered up and all
> > clocks enabled, then the controller's reset and the PHY's reset
> > toggled together as one group, with PARF_DEVICE_TYPE set right
> > after. This series adds the callbacks needed on both sides to
> > realize that sequence: a phy_reset() call the controller makes as
> > part of its own reset, and matching phy_ops::init/phy_ops::reset on
> > the PHY side.
> 
> This observation is not related to this series; it's a question about
> PCI host controller initialization ordering in general because it
> seems like this is a perennial issue.  Is there anything we can do to
> converge on some sort of generic standard or typical ordering across
> drivers?
> 
> For example, while looking at a patch that changed ordering in a
> different driver (not qcom), I asked Gemini for some guidance and it
> came up with the following ordering recommendations, which seem
> pretty generic:
> 
>   - assert downstream PERST# so physical lanes remain quiet while
>     setting up the host
> 
>   - assert local resets (core & PHY)
> 
>   - enable and stabilize power (IP core, I/O rails, PHYs)
> 
>   - enable and stabilize clocks, including refclk and T_PERST_CLK
> 
>   - initialize PHYs
> 
>     - deassert PHY reset or allow register access to PHY
> 
>     - wait for PHY PLL lock to refclk
> 
>     - program PHY parameters & calibration
> 
>     - verify PHY ready
> 
>   - deassert local core reset
> 
>   - configure RC (device type, MPS, max link speed, width, etc)
> 
>   - deassert downstream PERST#
> 
>   - enable LTSSM
> 
> If this could be made generic enough, maybe something like it could be
> included in Documentation/PCI/controller/pci-controller-drivers.rst?

Some of these sequences could be generalised, but the problem is, pretty much
each controller sequence follow a hardware reference manual and those
recommend a specific sequence. So if we diverge from that recommended sequence,
then getting help from the hardware team for any issue would be tricky. Atleast,
this is the case with Qcom controllers.

- Mani

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-09-21 11:17 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-16  6:04 [PATCH 0/4] PCI: qcom: Align PHY init sequence with HPG Qiang Yu
2026-09-16  6:04 ` [PATCH 1/4] PCI: qcom: Move PHY init and add PHY reset call to align " Qiang Yu
2026-09-21  9:14   ` Kathiravan Thirumoorthy
2026-09-16  6:04 ` [PATCH 2/4] PCI: qcom-ep: Move PHY init and drive PHY reset " Qiang Yu
2026-09-16  6:04 ` [PATCH 3/4] phy: qcom-qmp-pcie-multiphy: Split phy_ops into init/exit/reset/power_on Qiang Yu
2026-09-16  6:04 ` [PATCH 4/4] phy: qcom-qmp-pcie: " Qiang Yu
2026-09-21  9:15   ` Kathiravan Thirumoorthy
2026-09-16 15:55 ` [PATCH 0/4] PCI: qcom: Align PHY init sequence with HPG Bjorn Helgaas
2026-09-21 11:16   ` Manivannan Sadhasivam

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®