[PATCH v3 0/4] PCI: qcom: Add link retention support

[PATCH v3 0/4] PCI: qcom: Add link retention support

[Krishna Chaitanya Chundru]

This patch series introduces support for retaining the PCIe link across
bootloader and kernel handoff on Qualcomm platforms, specifically
X1E80100. The goal is to reduce boot time and avoid unnecessary link
reinitialization  when the link is already up.

We are not enabling link retantion support for all the targets, as there
is no guarantee that the bootloader on all targets has initialized the
PCIe link in max supported speed. So we are enabling for hamoa & glymur
target only for now based on the config flag.

If the link is up and has link_retain is set to true in the
ithe driver config data then enable retain logic in the controller.

In phy as we already have skip init logic, the phy patch uses same
assumption that if there is phy no csr and bootloader has done the init
then driver can skip resetting the phy when phy status indicates it is
up.

Problem:-
1) As part of late init calls of clock the framework is disabling all the
unvoted resources by that time and also there is no sync state to keep
them enabled till the probe is completed. Due to dependencies with
regulators and phy, qcom pcie probe is happening after late init which is
causing the resources(clocks) to be off which causes the link to go down.
To avoid this we need to use this kernel command line argument
(clk_ignore_unused) to skip disabling clocks as part of late init for
initial version. Once it is resolved we can avoid those kernel command
line argument.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
Changes in v3:
- Updated the max link speed and current link speed logic for link
  retain.
- Fixed l1ss_support & probe deffer fix as pointed by sashiko.
- Link to v2: https://patch.msgid.link/20260521-link_retain-v2-0-08ed448b081c@oss.qualcomm.com

Changes in v2:
- Rebased with latest changes
- GENPD/power domains are not longer getting turned off with the latest
  kernel, only issue is with the clocks.
- Removed the patch [2/5] PCI: dwc: Add support for retaining link during host init
  as we are not seeing much difference with this or without this (Bjorn).
- couple of nits in commit & prints (Mani).
- Remove skip_reset for the long term (Dmitry).
- Link to v1: https://lore.kernel.org/r/20260109-link_retain-v1-0-7e6782230f4b@oss.qualcomm.com

---
Krishna Chaitanya Chundru (4):
      phy: qcom: qmp-pcie: Skip PHY reset if already up
      PCI: qcom: Keep PERST# GPIO state as-is during probe
      PCI: qcom: Add link retention support
      PCI: qcom: enable Link retain logic for Hamoa

 drivers/pci/controller/dwc/pcie-designware.h |  1 +
 drivers/pci/controller/dwc/pcie-qcom.c       | 80 +++++++++++++++++++++++++---
 drivers/phy/qualcomm/phy-qcom-qmp-pcie.c     | 31 +++++++----
 3 files changed, 96 insertions(+), 16 deletions(-)
---
base-commit: ce1f20ceb245f81cc147bf4fd570c09f9f720ab2
change-id: 20251001-link_retain-f181307947e4

Best regards,
--  
Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>

[PATCH v3 1/4] phy: qcom: qmp-pcie: Skip PHY reset if already up

[Krishna Chaitanya Chundru]

If the bootloader has already powered up the PCIe PHY, performing a
full reset and waiting for the PHY to come up again adds unnecessary
delay during boot.

Extend the existing skip_init handling by introducing a skip_reset
condition. When skip_init is active and the PHY status indicates that
the PHY is already operational, skip asserting and deasserting the
no-csr reset while still enabling the required resources during
power-on.

This allows reusing the bootloader-initialized PHY state and avoids
redundant PHY reinitialization and PCIe link retraining, which can
add hundred's of milliseconds of delay.

This relies on the assumption that when skip_init is enabled and the
PHY is reported as up, the bootloader has already configured the PHY
correctly and the link is in a usable state.

Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Tested-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
index d3effad7a074..49e10151cb1e 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
@@ -4862,6 +4862,7 @@ 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;
 
 	/*
@@ -4877,6 +4878,9 @@ 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],
+							    cfg->phy_status);
+
 	if (!qmp->skip_init && !cfg->tbls.serdes_num) {
 		dev_err(qmp->dev, "Init sequence not available\n");
 		return -ENODATA;
@@ -4900,13 +4904,15 @@ static int qmp_pcie_init(struct phy *phy)
 		}
 	}
 
-	ret = reset_control_assert(qmp->nocsr_reset);
-	if (ret) {
-		dev_err(qmp->dev, "no-csr reset assert failed\n");
-		goto err_assert_reset;
-	}
+	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);
+		usleep_range(200, 300);
+	}
 
 	if (!qmp->skip_init) {
 		ret = reset_control_bulk_deassert(cfg->num_resets, qmp->resets);
@@ -4956,8 +4962,11 @@ 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.
@@ -4981,10 +4990,12 @@ static int qmp_pcie_power_on(struct phy *phy)
 	if (ret)
 		return ret;
 
-	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 (!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)

-- 
2.34.1

[PATCH v3 2/4] PCI: qcom: Keep PERST# GPIO state as-is during probe

[Krishna Chaitanya Chundru]

The PERST# signal is used to reset PCIe devices. Requesting the GPIO with
GPIOD_OUT_HIGH forces the line high during probe, which can unintentionally
assert reset on devices already out of reset and break proper link
sequencing.

Change the request to use GPIOD_ASIS so the driver preserves the existing
GPIO state configured by the bootloader or firmware. This allows platforms
that manage PERST# externally to maintain correct reset sequencing. PERST#
is asserted explicitly later during qcom_pcie_host_init(), so forcing it
high at probe time is unnecessary.

Tested-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index d8eb52857f69..9ca620d4746a 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1841,7 +1841,7 @@ static int qcom_pcie_parse_perst(struct qcom_pcie *pcie,
 		goto parse_child_node;
 
 	reset = devm_fwnode_gpiod_get(dev, of_fwnode_handle(np), "reset",
-				      GPIOD_OUT_HIGH, "PERST#");
+				      GPIOD_ASIS, "PERST#");
 	if (IS_ERR(reset)) {
 		/*
 		 * FIXME: GPIOLIB currently supports exclusive GPIO access only.
@@ -1918,7 +1918,7 @@ static int qcom_pcie_parse_ports(struct qcom_pcie *pcie)
 
 	if (of_find_property(dev->of_node, "perst-gpios", NULL)) {
 		pcie->reset = devm_gpiod_get_optional(dev, "perst",
-						      GPIOD_OUT_HIGH);
+						      GPIOD_ASIS);
 		if (IS_ERR(pcie->reset))
 			return PTR_ERR(pcie->reset);
 	}

-- 
2.34.1

[PATCH v3 3/4] PCI: qcom: Add link retention support

[Krishna Chaitanya Chundru]

Some platforms keep the PCIe link active across bootloader and kernel
handoff. Reinitializing the controller and toggling PERST# in such cases is
unnecessary when the driver does not need to retrain the link.

Introduce link_retain in both qcom_pcie_cfg and qcom_pcie to indicate when
link retention is supported. During initialization, check the LTSSM state;
if the link is already in L0 or L1 idle and LTSSM is enabled, set
link_retain and skip controller reset, PERST# toggling, and other post-
init steps.

The max-link-speed and num-lanes properties in Device Tree cap the link's
maximum capability. Retrain the link if the controller's max link speed or
width exceeds the Device Tree cap, unless the currently negotiated speed
or width is already within that cap, in which case the existing link can
still be retained.

Configure the DBI and ATU base addresses in the retention path, since the
bootloader may use different base addresses than those provided by the
device tree.

Set l1ss_support in the retention path as well, since it is otherwise only
set later in the normal init flow which is skipped when the link is
retained.

Tested-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-designware.h |  1 +
 drivers/pci/controller/dwc/pcie-qcom.c       | 68 ++++++++++++++++++++++++++--
 2 files changed, 66 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index de4b245b1758..0c98b914eb52 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -471,6 +471,7 @@ struct dw_pcie_rp {
 	bool			native_ecam;
 	bool                    skip_l23_ready;
 	bool			skip_pwrctrl_off;
+	bool			link_retain;
 };
 
 struct dw_pcie_ep_ops {
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 9ca620d4746a..77d2d989e9da 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -260,12 +260,14 @@ struct qcom_pcie_ops {
   * @override_no_snoop: Override NO_SNOOP attribute in TLP to enable cache
   * snooping
   * @firmware_managed: Set if the Root Complex is firmware managed
+  * @link_retain: Set if controller supports retaining link from bootloader
   */
 struct qcom_pcie_cfg {
 	const struct qcom_pcie_ops *ops;
 	bool override_no_snoop;
 	bool firmware_managed;
 	bool no_l0s;
+	bool link_retain;
 };
 
 struct qcom_pcie_perst {
@@ -996,6 +998,50 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie)
 	return 0;
 }
 
+/*
+ * Determine whether the link established by the bootloader can be reused.
+ *
+ * The max-link-speed and num-lanes specified in Device Tree are meant to cap
+ * the link's maximum capability. Retrain the link if the controller's max
+ * link speed/width exceeds the Device Tree cap, unless the currently
+ * negotiated speed/width is already within that cap, in which case the
+ * existing link can be retained as-is.
+ */
+static bool qcom_pcie_check_link_retain(struct qcom_pcie *pcie)
+{
+	u32 cap, speed, cur_speed, val, ltssm, width, cur_width;
+	struct dw_pcie *pci = pcie->pci;
+	u8 offset;
+
+	val = readl(pcie->parf + PARF_LTSSM);
+	ltssm = val & 0x1f;
+	if ((val & LTSSM_EN) &&
+	    (ltssm == DW_PCIE_LTSSM_L0 || ltssm == DW_PCIE_LTSSM_L1_IDLE)) {
+		qcom_pcie_configure_dbi_atu_base(pcie);
+
+		offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+		cap = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);
+		speed = FIELD_GET(PCI_EXP_LNKCAP_SLS, cap);
+		width = dw_pcie_link_get_max_link_width(pci);
+
+		val = dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKSTA);
+		cur_speed = FIELD_GET(PCI_EXP_LNKSTA_CLS, val);
+		cur_width = FIELD_GET(PCI_EXP_LNKSTA_NLW, val);
+
+		if (pci->max_link_speed > 0 && speed > pci->max_link_speed &&
+		    cur_speed > pci->max_link_speed)
+			return false;
+
+		if (pci->num_lanes > 0 && width > pci->num_lanes &&
+		    cur_width > pci->num_lanes)
+			return false;
+
+		return true;
+	}
+
+	return false;
+}
+
 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;
@@ -1014,6 +1060,15 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
 	if (ret < 0)
 		goto err_disable_regulators;
 
+	if (pcie->cfg->link_retain) {
+		pci->pp.link_retain = qcom_pcie_check_link_retain(pcie);
+		pci->l1ss_support = true;
+		if (pci->pp.link_retain) {
+			dev_info(dev, "Retaining PCIe link\n");
+			return 0;
+		}
+	}
+
 	ret = reset_control_assert(res->rst);
 	if (ret) {
 		dev_err(dev, "reset assert failed (%d)\n", ret);
@@ -1074,6 +1129,9 @@ static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie)
 {
 	const struct qcom_pcie_cfg *pcie_cfg = pcie->cfg;
 
+	if (pcie->pci->pp.link_retain)
+		return 0;
+
 	if (pcie_cfg->override_no_snoop)
 		writel(WR_NO_SNOOP_OVERRIDE_EN | RD_NO_SNOOP_OVERRIDE_EN,
 				pcie->parf + PARF_NO_SNOOP_OVERRIDE);
@@ -1364,12 +1422,13 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
 	struct qcom_pcie *pcie = to_qcom_pcie(pci);
 	int ret;
 
-	qcom_pcie_perst_assert(pcie);
-
 	ret = pcie->cfg->ops->init(pcie);
 	if (ret)
 		return ret;
 
+	if (!pp->link_retain)
+		qcom_pcie_perst_assert(pcie);
+
 	ret = qcom_pcie_phy_power_on(pcie);
 	if (ret)
 		goto err_deinit;
@@ -1398,7 +1457,8 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
 
 	qcom_pcie_configure_ports(pcie);
 
-	qcom_pcie_perst_deassert(pcie);
+	if (!pp->link_retain)
+		qcom_pcie_perst_deassert(pcie);
 
 	if (pcie->cfg->ops->config_sid) {
 		ret = pcie->cfg->ops->config_sid(pcie);
@@ -1414,6 +1474,8 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
 	if (!pp->skip_pwrctrl_off)
 		pci_pwrctrl_power_off_devices(pci->dev);
 err_pwrctrl_destroy:
+	if (pp->link_retain && ret == -EPROBE_DEFER)
+		return ret;
 	if (ret != -EPROBE_DEFER && !pci->suspended)
 		pci_pwrctrl_destroy_devices(pci->dev);
 err_disable_phy:

-- 
2.34.1

[PATCH v3 4/4] PCI: qcom: enable Link retain logic for Hamoa

[Krishna Chaitanya Chundru]

The Hamoa platform supports keeping the PCIe link active across
bootloader and kernel handoff. To take advantage of this, introduce a
specific configuration (cfg_x1e80100) with link_retain = true and
update the device match table to use it.

Tested-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 77d2d989e9da..0b345a16f4cf 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1671,6 +1671,12 @@ static const struct qcom_pcie_cfg cfg_sc8280xp = {
 	.no_l0s = true,
 };
 
+static const struct qcom_pcie_cfg cfg_x1e80100 = {
+	.ops = &ops_1_21_0,
+	.no_l0s = true,
+	.link_retain = true,
+};
+
 static const struct qcom_pcie_cfg cfg_fw_managed = {
 	.firmware_managed = true,
 };
@@ -2370,7 +2376,7 @@ static const struct of_device_id qcom_pcie_match[] = {
 	{ .compatible = "qcom,pcie-sm8450-pcie0", .data = &cfg_1_9_0 },
 	{ .compatible = "qcom,pcie-sm8450-pcie1", .data = &cfg_1_9_0 },
 	{ .compatible = "qcom,pcie-sm8550", .data = &cfg_1_9_0 },
-	{ .compatible = "qcom,pcie-x1e80100", .data = &cfg_sc8280xp },
+	{ .compatible = "qcom,pcie-x1e80100", .data = &cfg_x1e80100 },
 	{ }
 };
 

-- 
2.34.1

Re: [PATCH v3 2/4] PCI: qcom: Keep PERST# GPIO state as-is during probe

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Requesting the PERST# GPIO with GPIOD_ASIS leaves it as an input if not configured by the bootloader, causing subsequent reset toggles to silently fail.
--

--- Patch [2]: [PATCH v3 2/4] PCI: qcom: Keep PERST# GPIO state as-is during probe ---
commit 9e74f48a37d93ad22a1ec71470af58b05c07c912
Author: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>

PCI: qcom: Keep PERST# GPIO state as-is during probe

This commit changes the PERST# GPIO request from GPIOD_OUT_HIGH to GPIOD_ASIS
in order to preserve the existing GPIO state configured by the bootloader or
firmware during probe.

> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1841,7 +1841,7 @@ static int qcom_pcie_parse_perst(struct qcom_pcie *pcie,
>  		goto parse_child_node;
>  
>  	reset = devm_fwnode_gpiod_get(dev, of_fwnode_handle(np), "reset",
> -				      GPIOD_OUT_HIGH, "PERST#");
> +				      GPIOD_ASIS, "PERST#");

[Severity: High]
If the bootloader does not configure the PERST# GPIO as an output, does
requesting it with GPIOD_ASIS leave the pin configured as an input?

If so, later when __qcom_pcie_perst_assert() attempts to toggle the reset:

	gpiod_set_value_cansleep(perst->desc, val);

Will it silently fail to drive the pin, preventing the PCIe link from being
established?

>  	if (IS_ERR(reset)) {
>  		/*
>  		 * FIXME: GPIOLIB currently supports exclusive GPIO access only.

[ ... ]

> @@ -1918,7 +1918,7 @@ static int qcom_pcie_parse_ports(struct qcom_pcie *pcie)
>  
>  	if (of_find_property(dev->of_node, "perst-gpios", NULL)) {
>  		pcie->reset = devm_gpiod_get_optional(dev, "perst",
> -						      GPIOD_OUT_HIGH);
> +						      GPIOD_ASIS);

[Severity: High]
This appears to have the same issue. If the GPIO defaults to an input and
is not configured by the bootloader, will gpiod_set_value_cansleep()
silently fail to drive the line later?

>  		if (IS_ERR(pcie->reset))
>  			return PTR_ERR(pcie->reset);
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-link_retain-v3-0-81a9d187bb61@oss.qualcomm.com?part=2

Re: [PATCH v3 1/4] phy: qcom: qmp-pcie: Skip PHY reset if already up

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The `skip_reset` condition is evaluated independently in both `qmp_pcie_init()` and `qmp_pcie_power_on()`, which can lead to unbalanced reset operations and leave the PHY permanently in reset.
--

--- Patch [1]: [PATCH v3 1/4] phy: qcom: qmp-pcie: Skip PHY reset if already up ---
commit 3002076fcac7c2d6108104d2911e0cbb6c6bf224
Author: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>

phy: qcom: qmp-pcie: Skip PHY reset if already up

This patch extends the skip_init handling by introducing a skip_reset
condition for the qmp-pcie phy. It skips asserting and deasserting the
no-csr reset if the bootloader has already powered up the PCIe PHY and
it is operational. This reuses the bootloader-initialized PHY state to
save boot time and avoid redundant PCIe link retraining.

> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> index d3effad7a074b..49e10151cb1e4 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> @@ -4862,6 +4862,7 @@ 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;
>  
>  	/*
> @@ -4877,6 +4878,9 @@ 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],
> +							    cfg->phy_status);
> +
>  	if (!qmp->skip_init && !cfg->tbls.serdes_num) {
>  		dev_err(qmp->dev, "Init sequence not available\n");
>  		return -ENODATA;

[ ... ]

> @@ -4900,13 +4904,15 @@ static int qmp_pcie_init(struct phy *phy)
>  		}
>  	}
>  
> -	ret = reset_control_assert(qmp->nocsr_reset);
> -	if (ret) {
> -		dev_err(qmp->dev, "no-csr reset assert failed\n");
> -		goto err_assert_reset;
> -	}
> +	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);
> +		usleep_range(200, 300);
> +	}
>  
>  	if (!qmp->skip_init) {
>  		ret = reset_control_bulk_deassert(cfg->num_resets, qmp->resets);

[ ... ]

> @@ -4956,8 +4962,11 @@ 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);

[Severity: High]
Will this lead to unbalanced reset operations if the hardware state changes
between qmp_pcie_init() and qmp_pcie_power_on()?

In qmp_pcie_init(), skip_reset is computed by reading the live PHY status
register. If it evaluates to false, nocsr_reset is asserted. However,
asserting the reset could alter the state of the PHY status register.

When qmp_pcie_power_on() subsequently executes, it reads the status register
again here. If the register now indicates the PHY is ready, skip_reset would
evaluate to true, and the driver would skip reset_control_deassert() below,
leaving the PHY permanently in reset.

Could we cache the skip_reset decision in struct qmp_pcie during init rather
than re-evaluating the live register here?

>  	/*
>  	 * Write CSR register for PHY that doesn't support no_csr reset or has not
>  	 * been initialized.
> @@ -4981,10 +4990,12 @@ static int qmp_pcie_power_on(struct phy *phy)
>  	if (ret)
>  		return ret;
>  
> -	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 (!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)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-link_retain-v3-0-81a9d187bb61@oss.qualcomm.com?part=1

Re: [PATCH v3 3/4] PCI: qcom: Add link retention support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Probe deferral causes reference count and resource leaks for clocks, regulators, and PHY components when link retention is active.
- [Medium] Bypassing `post_init` during link retention skips critical controller quirks and DMA coherency settings.
- [Medium] `qcom_pcie_check_link_retain()` uses an incorrect hardcoded mask (`0x1f`) to read the LTSSM state, risking state aliasing.
--

--- Patch [3]: [PATCH v3 3/4] PCI: qcom: Add link retention support ---
commit 48e4b763c5282c1440201e104587c3e6e2342ab6
Author: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
PCI: qcom: Add link retention support

This patch adds link retention support for Qualcomm PCIe controllers. It
skips controller reset and PERST# toggling if the bootloader has already
trained the link and left it in L0 or L1 idle. It checks device tree caps
and configures base addresses in the retention path.

> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c

[ ... ]

> @@ -996,6 +998,50 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie)
>  	return 0;
>  }
>  
> +/*
> + * Determine whether the link established by the bootloader can be reused.
> + *
> + * The max-link-speed and num-lanes specified in Device Tree are meant to cap
> + * the link's maximum capability. Retrain the link if the controller's max
> + * link speed/width exceeds the Device Tree cap, unless the currently
> + * negotiated speed/width is already within that cap, in which case the
> + * existing link can be retained as-is.
> + */
> +static bool qcom_pcie_check_link_retain(struct qcom_pcie *pcie)
> +{
> +	u32 cap, speed, cur_speed, val, ltssm, width, cur_width;
> +	struct dw_pcie *pci = pcie->pci;
> +	u8 offset;
> +
> +	val = readl(pcie->parf + PARF_LTSSM);
> +	ltssm = val & 0x1f;

[Severity: Medium]
Does this hardcoded 0x1f mask truncate the LTSSM state?

The standard LTSSM state mask (PARF_LTSSM_STATE_MASK) is defined as
GENMASK(5, 0), which is 0x3f.

By discarding bit 5, higher-order states like 0x31 could erroneously alias
to DW_PCIE_LTSSM_L0 (0x11).

This might cause the driver to incorrectly assume the link is active and
retain it, skipping initialization and leaving the link broken.

[ ... ]

> @@ -1074,6 +1129,9 @@ static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie)
>  {
>  	const struct qcom_pcie_cfg *pcie_cfg = pcie->cfg;
>  
> +	if (pcie->pci->pp.link_retain)
> +		return 0;
> +

[Severity: Medium]
Does bypassing the rest of qcom_pcie_post_init_2_7_0() skip critical OS quirks?

By returning early, this bypasses the configuration of NO_SNOOP_OVERRIDE and
the NCCS hot-plug quirk.

Missing NO_SNOOP_OVERRIDE can cause DMA cache coherency issues and data
corruption on ARM.

Missing the NCCS quirk can cause the OS pciehp driver to wait for
non-existent command completions, leading to hot-plug timeouts.

>  	if (pcie_cfg->override_no_snoop)
>  		writel(WR_NO_SNOOP_OVERRIDE_EN | RD_NO_SNOOP_OVERRIDE_EN,
>  				pcie->parf + PARF_NO_SNOOP_OVERRIDE);

[ ... ]

> @@ -1414,6 +1474,8 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
>  	if (!pp->skip_pwrctrl_off)
>  		pci_pwrctrl_power_off_devices(pci->dev);
>  err_pwrctrl_destroy:
> +	if (pp->link_retain && ret == -EPROBE_DEFER)
> +		return ret;

[Severity: Medium]
Could this early return leak hardware enable counts during probe deferral?

If a downstream function like pci_pwrctrl_power_on_devices() returns
-EPROBE_DEFER, this early return explicitly bypasses the err_disable_phy
and err_deinit cleanup paths.

The driver core's devres cleanup drops the resource references but does not
manually disable them.

On the next probe attempt, qcom_pcie_init_2_7_0() will re-enable the
regulators and clocks:

	ret = regulator_bulk_enable(ARRAY_SIZE(res->supplies), res->supplies);
...
	ret = clk_bulk_prepare_enable(res->num_clks, res->clks);

This compounds the leaked reference counts, preventing the resources from
ever being powered off upon driver unbind or system shutdown.

>  	if (ret != -EPROBE_DEFER && !pci->suspended)
>  		pci_pwrctrl_destroy_devices(pci->dev);
>  err_disable_phy:
>  	qcom_pcie_phy_power_off(pcie);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-link_retain-v3-0-81a9d187bb61@oss.qualcomm.com?part=3

Re: [PATCH v3 4/4] PCI: qcom: enable Link retain logic for Hamoa

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Probe deferral leaks PHY power-on, clock, and regulator enable counts when `link_retain` is active.
--

--- Patch [4]: [PATCH v3 4/4] PCI: qcom: enable Link retain logic for Hamoa ---
commit a80ea22accc5e92e309b80e7933e0f0bac88c9ec
Author: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>

PCI: qcom: enable Link retain logic for Hamoa

The Hamoa platform supports keeping the PCIe link active across
bootloader and kernel handoff. To take advantage of this, introduce a
specific configuration (cfg_x1e80100) with link_retain = true and
update the device match table to use it.

> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 77d2d989e9da5..0b345a16f4cf6 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1671,6 +1671,12 @@ static const struct qcom_pcie_cfg cfg_sc8280xp = {
>  	.no_l0s = true,
>  };
>  
> +static const struct qcom_pcie_cfg cfg_x1e80100 = {
> +	.ops = &ops_1_21_0,
> +	.no_l0s = true,
> +	.link_retain = true,

[Severity: High]
Does enabling link_retain here cause a resource leak during probe deferral?

In qcom_pcie_host_init(), if an initialization step returns -EPROBE_DEFER and
link_retain is true, the error path returns early:

drivers/pci/controller/dwc/pcie-qcom.c:qcom_pcie_host_init() {
    ...
err_pwrctrl_destroy:
    if (pp->link_retain && ret == -EPROBE_DEFER)
        return ret;
    ...
}

This bypasses err_disable_phy and err_deinit which would balance the
resource counts.

Since qcom_pcie_init_2_7_0() unconditionally enables the regulators and
clocks, and qcom_pcie_phy_power_on() enables the PHYs, won't these reference
counts be leaked on each deferral retry? When the driver is eventually
removed, deinit() will only decrement them once, leaving the resources
permanently enabled.

Is there a way to balance these reference counts during a probe deferral
when link_retain is active?

> +};
> +

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-link_retain-v3-0-81a9d187bb61@oss.qualcomm.com?part=4

Re: [PATCH v3 4/4] PCI: qcom: enable Link retain logic for Hamoa

[Konrad Dybcio]

On 7/9/26 8:35 AM, Krishna Chaitanya Chundru wrote:
> The Hamoa platform supports keeping the PCIe link active across
> bootloader and kernel handoff. To take advantage of this, introduce a
> specific configuration (cfg_x1e80100) with link_retain = true and
> update the device match table to use it.
> 
> Tested-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad