[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f44beed6-72c3-4e86-834b-ac522c786bc7@oss.qualcomm.com>
Date: Wed, 13 Aug 2025 12:13:31 +0200
From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
To: Stephan Gerhold <stephan.gerhold@...aro.org>,
Vinod Koul <vkoul@...nel.org>
Cc: Kishon Vijay Abraham I <kishon@...nel.org>,
Wenbin Yao <quic_wenbyao@...cinc.com>,
Qiang Yu <qiang.yu@....qualcomm.com>,
Manivannan Sadhasivam <mani@...nel.org>, linux-arm-msm@...r.kernel.org,
linux-phy@...ts.infradead.org, linux-kernel@...r.kernel.org,
Konrad Dybcio <konradybcio@...nel.org>
Subject: Re: [PATCH] phy: qcom: qmp-pcie: Fix PHY initialization when powered
down by firmware
On 8/12/25 6:30 PM, Stephan Gerhold wrote:
> Commit 0cc22f5a861c ("phy: qcom: qmp-pcie: Add PHY register retention
> support") added support for using the "no_csr" reset to skip configuration
> of the PHY if the init sequence was already applied by the boot firmware.
> The expectation is that the PHY is only turned on/off by using the "no_csr"
> reset, instead of powering it down and re-programming it after a full
> reset.
>
> The boot firmware on X1E does not fully conform to this expectation: If the
> PCIe3 link fails to come up (e.g. because no PCIe card is inserted), the
> firmware powers down the PHY using the QPHY_PCS_POWER_DOWN_CONTROL
> register. The QPHY_START_CTRL register is kept as-is, so the driver assumes
> the PHY is already initialized and skips the configuration/power up
> sequence. The PHY won't come up again without clearing the
> QPHY_PCS_POWER_DOWN_CONTROL, so eventually initialization fails:
>
> qcom-qmp-pcie-phy 1be0000.phy: phy initialization timed-out
> phy phy-1be0000.phy.0: phy poweron failed --> -110
> qcom-pcie 1bd0000.pcie: cannot initialize host
> qcom-pcie 1bd0000.pcie: probe with driver qcom-pcie failed with error -110
>
> This can be reliably reproduced on the X1E CRD, QCP and Devkit when no card
> is inserted for PCIe3.
>
> Fix this by checking the QPHY_PCS_POWER_DOWN_CONTROL register in addition
> to QPHY_START_CTRL. If the PHY is powered down with the register, it
> doesn't conform to the expectations for using the "no_csr" reset, so we
> fully re-initialize with the normal reset sequence.
>
> Cc: stable@...r.kernel.org
> Fixes: 0cc22f5a861c ("phy: qcom: qmp-pcie: Add PHY register retention support")
> Signed-off-by: Stephan Gerhold <stephan.gerhold@...aro.org>
> ---
> drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> index 95830dcfdec9b1f68fd55d1cc3c102985cfafcc1..6a469a8f5ae7eba6e4d1d702efaae1c658c4321e 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> @@ -4339,10 +4339,12 @@ 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 phy_initialized = !!(readl(pcs + cfg->regs[QPHY_START_CTRL]));
> int ret;
>
> - qmp->skip_init = qmp->nocsr_reset && phy_initialized;
> + qmp->skip_init = qmp->nocsr_reset &&
> + readl(pcs + cfg->regs[QPHY_START_CTRL]) &&
> + readl(pcs + cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL]);
I think it would be good to ensure the value matches platform config
expectations, i.e. !(val & cfg->pwrdn_ctrl)
Konrad
Powered by blists - more mailing lists