On 18/03/2025 15:49, Nitin Rawat wrote:
Refactor the UFS PHY reset handling to parse the reset logic only once
during probe, instead of every resume.
Move the UFS PHY reset parsing logic from qmp_phy_power_on to
qmp_ufs_probe to avoid unnecessary parsing during resume.
Co-developed-by: Ram Kumar Dwivedi <quic_rdwivedi@xxxxxxxxxxx>
Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@xxxxxxxxxxx>
Signed-off-by: Nitin Rawat <quic_nitirawa@xxxxxxxxxxx>
---
drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 104 ++++++++++++------------
1 file changed, 50 insertions(+), 54 deletions(-)
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
index 0089ee80f852..3a80c2c110d2 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
@@ -1757,32 +1757,6 @@ static void qmp_ufs_init_registers(struct qmp_ufs *qmp, const struct qmp_phy_cfg
qmp_ufs_init_all(qmp, &cfg->tbls_hs_b);
}
-static int qmp_ufs_com_init(struct qmp_ufs *qmp)
-{
- const struct qmp_phy_cfg *cfg = qmp->cfg;
- void __iomem *pcs = qmp->pcs;
- 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_clks, qmp->clks);
- if (ret)
- goto err_disable_regulators;
-
- qphy_setbits(pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL], SW_PWRDN);
-
- return 0;
-
-err_disable_regulators:
- regulator_bulk_disable(cfg->num_vregs, qmp->vregs);
-
- return ret;
-}
-
static int qmp_ufs_com_exit(struct qmp_ufs *qmp)
{
const struct qmp_phy_cfg *cfg = qmp->cfg;
@@ -1800,41 +1774,27 @@ static int qmp_ufs_power_on(struct phy *phy)
{
struct qmp_ufs *qmp = phy_get_drvdata(phy);
const struct qmp_phy_cfg *cfg = qmp->cfg;
+ void __iomem *pcs = qmp->pcs;
int ret;
- dev_vdbg(qmp->dev, "Initializing QMP phy\n");
-
- if (cfg->no_pcs_sw_reset) {
- /*
- * Get UFS reset, which is delayed until now to avoid a
- * circular dependency where UFS needs its PHY, but the PHY
- * needs this UFS reset.
- */
- if (!qmp->ufs_reset) {
- qmp->ufs_reset =
- devm_reset_control_get_exclusive(qmp->dev,
- "ufsphy");
-
- if (IS_ERR(qmp->ufs_reset)) {
- ret = PTR_ERR(qmp->ufs_reset);
- dev_err(qmp->dev,
- "failed to get UFS reset: %d\n",
- ret);
-
- qmp->ufs_reset = NULL;
- return ret;
- }
- }
- ret = reset_control_assert(qmp->ufs_reset);
- if (ret)
- return 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 = qmp_ufs_com_init(qmp);
+ ret = clk_bulk_prepare_enable(qmp->num_clks, qmp->clks);
if (ret)
- return ret;
+ goto err_disable_regulators;
+
+ qphy_setbits(pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL], SW_PWRDN);
return 0;
+
+err_disable_regulators:
+ regulator_bulk_disable(cfg->num_vregs, qmp->vregs);
+
+ return ret;
}
This change is too fuzzy, please introduce qmp_ufs_get_phy_reset()
in a patch, move qmp_ufs_com_init() inline in qmp_ufs_power_on()
in a second time, and finally move reset_control_assert() to
calibrate in a third patch (and explain why).
Thanks,
Neil
static int qmp_ufs_phy_calibrate(struct phy *phy)
@@ -1846,6 +1806,10 @@ static int qmp_ufs_phy_calibrate(struct phy *phy)
unsigned int val;
int ret;
+ ret = reset_control_assert(qmp->ufs_reset);
+ if (ret)
+ return ret;
+
qmp_ufs_init_registers(qmp, cfg);
ret = reset_control_deassert(qmp->ufs_reset);
@@ -2088,6 +2052,34 @@ static int qmp_ufs_parse_dt(struct qmp_ufs *qmp)
return 0;
}
+static int qmp_ufs_get_phy_reset(struct qmp_ufs *qmp)
+{
+ const struct qmp_phy_cfg *cfg = qmp->cfg;
+ int ret;
+
+ if (!cfg->no_pcs_sw_reset)
+ return 0;
+
+ /*
+ * Get UFS reset, which is delayed until now to avoid a
+ * circular dependency where UFS needs its PHY, but the PHY
+ * needs this UFS reset.
+ */
+ if (!qmp->ufs_reset) {
+ qmp->ufs_reset =
+ devm_reset_control_get_exclusive(qmp->dev, "ufsphy");
+
+ if (IS_ERR(qmp->ufs_reset)) {
+ ret = PTR_ERR(qmp->ufs_reset);
+ dev_err(qmp->dev, "failed to get PHY reset: %d\n", ret);
+ qmp->ufs_reset = NULL;
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
static int qmp_ufs_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -2114,6 +2106,10 @@ static int qmp_ufs_probe(struct platform_device *pdev)
if (ret)
return ret;
+ ret = qmp_ufs_get_phy_reset(qmp);
+ if (ret)
+ return ret;
+
/* Check for legacy binding with child node. */
np = of_get_next_available_child(dev->of_node, NULL);
if (np) {
--
2.48.1
