Re: [PATCH v3 3/4] PCI: qcom: Add link retention support
From: Manivannan Sadhasivam
Date: Wed Jul 29 2026 - 05:05:09 EST
On Wed, Jul 29, 2026 at 09:37:47AM +0530, Krishna Chaitanya Chundru wrote:
>
>
> On 7/13/2026 10:20 AM, Manivannan Sadhasivam wrote:
> > On Thu, Jul 09, 2026 at 12:05:18PM +0530, Krishna Chaitanya Chundru wrote:
> >> 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.
> >>
> > Where is this base address change?
> It is being covered as part of dw_pcie_setup_rc(), this part can be removed
> as this being called unconditionally in this patch.
> >> 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@xxxxxxxxxxxxxxxx>
> >> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@xxxxxxxxxxxxxxxx>
> >> ---
> >> 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);
> >> -
> > This is problematic. Most of the init callbacks do operations like reset the
> > PHY, enable clocks... If those operations affect the link state and the device
> > is not reset using PERST#, it will cause the device to crash.
> PERST# is only for endpoint, not for internal controller so it will not cause
> any crash.
If those operations affect the PCIe link state and REFCLK (most likely yes),
then it may end up crashing the device.
> More over the perst default state can be set be assert through pincntrl if you
> still
> suspect this can cause some issue on the pcie controller side.
>
'can be set' is not a guarantee.
- Mani
--
மணிவண்ணன் சதாசிவம்