On 2/12/2025 8:13 AM, Dmitry Baryshkov wrote:
On Tue, Feb 11, 2025 at 05:42:31PM +0800, Wenbin Yao wrote:From: Qiang Yu <quic_qianyu@xxxxxxxxxxx> Some QCOM PCIe PHYs support no_csr reset. Unlike BCR reset which resets the whole PHY (hardware and register), no_csr reset only resets PHY hardware but retains register values, which means PHY setting can be skipped during PHY init if PCIe link is enabled in booltloader and only no_csr is toggled after that. Hence, determine whether the PHY has been enabled in bootloader by verifying QPHY_START_CTRL register. If it's programmed and no_csr reset is available, skip BCR reset and PHY register setting to establish the PCIe link with bootloader - programmed PHY settings. Signed-off-by: Qiang Yu <quic_qianyu@xxxxxxxxxxx> Signed-off-by: Wenbin Yao <quic_wenbyao@xxxxxxxxxxx> --- drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 91 +++++++++++++++--------- 1 file changed, 58 insertions(+), 33 deletions(-) diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c index ac42e4b01065..7f0802d09812 100644 --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c @@ -2805,6 +2805,7 @@ struct qmp_pcie {const struct qmp_phy_cfg *cfg;bool tcsr_4ln_config; + bool phy_initialized;void __iomem *serdes;void __iomem *pcs; @@ -3976,6 +3977,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; int ret;ret = regulator_bulk_enable(cfg->num_vregs, qmp->vregs);@@ -3984,10 +3986,17 @@ static int qmp_pcie_init(struct phy *phy) return ret; }- ret = reset_control_bulk_assert(cfg->num_resets, qmp->resets);- if (ret) { - dev_err(qmp->dev, "reset assert failed\n"); - goto err_disable_regulators; + qmp->phy_initialized = !!(readl(pcs + cfg->regs[QPHY_START_CTRL])); + /* + * Toggle BCR reset for phy that doesn't support no_csr + * reset or has not been initialized + */ + if (!qmp->nocsr_reset || !qmp->phy_initialized) {Instead of having phy_initialized please add another boolean field, qmp->skip_init = !!qmp->nocsr_reset && !!phy_initialized; Use qmp->skip_init through the code.
In qmp_pcie_power_off and qmp_pcie_exit, we only check qmp->nocsr_reset. It seems unnecessary to combine qmp->nocsr_reset with phy_initialized.
+ ret = reset_control_bulk_assert(cfg->num_resets, qmp->resets); + if (ret) { + dev_err(qmp->dev, "reset assert failed\n"); + goto err_disable_regulators; + } }ret = reset_control_assert(qmp->nocsr_reset);@@ -3998,10 +4007,12 @@ static int qmp_pcie_init(struct phy *phy)usleep_range(200, 300); - ret = reset_control_bulk_deassert(cfg->num_resets, qmp->resets);- if (ret) { - dev_err(qmp->dev, "reset deassert failed\n"); - goto err_assert_reset; + if (!qmp->nocsr_reset || !qmp->phy_initialized) { + 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 = clk_bulk_prepare_enable(ARRAY_SIZE(qmp_pciephy_clk_l), qmp->clks);@@ -4011,7 +4022,8 @@ static int qmp_pcie_init(struct phy *phy) return 0;err_assert_reset:- reset_control_bulk_assert(cfg->num_resets, qmp->resets); + if (!qmp->nocsr_reset || !qmp->phy_initialized) + reset_control_bulk_assert(cfg->num_resets, qmp->resets); err_disable_regulators: regulator_bulk_disable(cfg->num_vregs, qmp->vregs);@@ -4023,7 +4035,10 @@ static int qmp_pcie_exit(struct phy *phy)struct qmp_pcie *qmp = phy_get_drvdata(phy); const struct qmp_phy_cfg *cfg = qmp->cfg;- reset_control_bulk_assert(cfg->num_resets, qmp->resets);+ if (!qmp->nocsr_reset) + reset_control_bulk_assert(cfg->num_resets, qmp->resets); + else + reset_control_assert(qmp->nocsr_reset);clk_bulk_disable_unprepare(ARRAY_SIZE(qmp_pciephy_clk_l), qmp->clks); @@ -4042,16 +4057,22 @@ static int qmp_pcie_power_on(struct phy *phy)unsigned int mask, val; int ret;- qphy_setbits(pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL],- cfg->pwrdn_ctrl); + /* + * Write CSR register for phy that doesn't support no_csrwhat is CSR register?
The registers of PHY.
+ * reset or has not been initialized + */ + if (!qmp->nocsr_reset || !qmp->phy_initialized) { + qphy_setbits(pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL], + cfg->pwrdn_ctrl);- if (qmp->mode == PHY_MODE_PCIE_RC)- mode_tbls = cfg->tbls_rc; - else - mode_tbls = cfg->tbls_ep; + if (qmp->mode == PHY_MODE_PCIE_RC) + mode_tbls = cfg->tbls_rc; + else + mode_tbls = cfg->tbls_ep;- qmp_pcie_init_registers(qmp, &cfg->tbls);- qmp_pcie_init_registers(qmp, mode_tbls); + qmp_pcie_init_registers(qmp, &cfg->tbls); + qmp_pcie_init_registers(qmp, mode_tbls); + }ret = clk_bulk_prepare_enable(qmp->num_pipe_clks, qmp->pipe_clks);if (ret) @@ -4063,15 +4084,16 @@ static int qmp_pcie_power_on(struct phy *phy) goto err_disable_pipe_clk; }- /* Pull PHY out of reset state */- qphy_clrbits(pcs, cfg->regs[QPHY_SW_RESET], SW_RESET); + if (!qmp->nocsr_reset || !qmp->phy_initialized) { + /* Pull PHY out of reset state */ + qphy_clrbits(pcs, cfg->regs[QPHY_SW_RESET], SW_RESET);- /* start SerDes and Phy-Coding-Sublayer */- qphy_setbits(pcs, cfg->regs[QPHY_START_CTRL], SERDES_START | PCS_START); - - if (!cfg->skip_start_delay) - usleep_range(1000, 1200); + /* start SerDes and Phy-Coding-Sublayer */ + qphy_setbits(pcs, cfg->regs[QPHY_START_CTRL], SERDES_START | PCS_START);+ if (!cfg->skip_start_delay)+ usleep_range(1000, 1200); + } status = pcs + cfg->regs[QPHY_PCS_STATUS]; mask = cfg->phy_status; ret = readl_poll_timeout(status, val, !(val & mask), 200, @@ -4096,16 +4118,19 @@ static int qmp_pcie_power_off(struct phy *phy)clk_bulk_disable_unprepare(qmp->num_pipe_clks, qmp->pipe_clks); - /* PHY reset */- qphy_setbits(qmp->pcs, cfg->regs[QPHY_SW_RESET], SW_RESET);- /* stop SerDes and Phy-Coding-Sublayer */- qphy_clrbits(qmp->pcs, cfg->regs[QPHY_START_CTRL], - SERDES_START | PCS_START); + if (!qmp->nocsr_reset) {Why this one doesn't check for the qmp->phy_initialized?
If a PHY supports no_csr reset but isn't enabled in the bootloader, we still need to program the phy settings only once so that we can reuse them during the D3cold -> D0 cycle. Therefore, we don't check qmp->phy_initialized here.
+ /* PHY reset */ + qphy_setbits(qmp->pcs, cfg->regs[QPHY_SW_RESET], SW_RESET);- /* Put PHY into POWER DOWN state: active low */- qphy_clrbits(qmp->pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL], - cfg->pwrdn_ctrl); + /* stop SerDes and Phy-Coding-Sublayer */ + qphy_clrbits(qmp->pcs, cfg->regs[QPHY_START_CTRL], + SERDES_START | PCS_START); + + /* Put PHY into POWER DOWN state: active low */ + qphy_clrbits(qmp->pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL], + cfg->pwrdn_ctrl); + }return 0;} -- 2.34.1
-- With best wishes Wenbin
