[PATCH v8 phy-next 11/31] scsi: ufs: qcom: call phy_init() before phy_power_on()

Vladimir Oltean
2026-05-05T10:07:16Z

The Qualcomm UFS host controller interacts with the QMP PHY in a way
which violates the Generic PHY API expectation, documented in section
"Order of API calls" from Documentation/driver-api/phy/phy.rst, and then
tries to hide it.

Namely, calls must be made in the phy_init() -> phy_power_on() ->
phy_power_off() -> phy_exit() sequence.

What we actually have is:

ufshcd_init()
-> ufshcd_hba_init()
-> ufshcd_setup_clocks(hba, true)
-> ufshcd_vops_setup_clocks(hba, true, POST_CHANGE)
-> ufs_qcom_setup_clocks(hba, true, POST_CHANGE)
-> ufs_qcom_init() has not run, simply ignore
-> ufshcd_variant_hba_init()
-> ufs_qcom_init()
-> ufs_qcom_setup_clocks(hba, true, POST_CHANGE)
-> phy_power_on(phy)
-> ufshcd_hba_enable()
-> ufshcd_vops_hce_enable_notify()
-> ufs_qcom_hce_enable_notify()
-> ufs_qcom_power_up_sequence()
-> if (phy->power_count) phy_power_off(phy)
-> phy_init(phy)

This "works" because the way that the "phy_power_on was called before
phy_init\n" warning condition is detected in phy-core.c is if the
power_count is positive at the phy_init() call time.

By having that "if (phy->power_count) phy_power_off(phy)" logic, the
ufs-qcom.c technically sidesteps the test, but actually violates the
Generic PHY API even more (calls phy_power_on() *and* phy_power_off()
before phy_init()).

The reason why I stumbled upon this was that I was trying to remove
dereferences of phy->power_count (a PHY internal field) from consumer
drivers.

phy_init(), implemented as qmp_ufs_phy_init(), calls qmp->ufs_reset =
devm_reset_control_get_exclusive(), so my understanding is that it needs
to be called:
- no earlier than ufs_qcom_init() -> devm_reset_controller_register()
which makes qmp->ufs_reset available
- no later than ufs_qcom_power_up_sequence() -> phy_calibrate() ->
qmp_ufs_phy_calibrate() where the qmp->ufs_reset is needed; although
phy_init() should be the first PHY API call made.

The only mystery is why is the current phy_init() placement so late, in
ufs_qcom_power_up_sequence(), but I guess the answer is that the
placement is vestigial. After the incremental work of commit
c9b589791fc1 ("phy: qcom: Utilize UFS reset controller") from
Evan Green and commit cbfd6c124f27 ("phy: qcom-qmp-ufs: Refactor
phy_power_on and phy_calibrate callbacks") from Nitin Rawat, the entire
multi-stage PHY init procedure was moved to phy_power_on(), but nobody
bothered to move phy_init() anywhere else more natural.

So hopefully if the calculations are right, any placement within that
bounding box should be good, and I'm picking the new phy_init() location
to be in ufs_qcom_init().

Even with phy_init() out of the way, ufs_qcom_power_up_sequence() ->
phy_power_off() is still needed, for a separate reason which will be
dealt with separately.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Cc: Nitin Rawat <quic_nitirawa@quicinc.com>
Cc: Manivannan Sadhasivam <mani@kernel.org>

v7->v8: patch is new
Commit was previously posted here but did not get any testing.
https://lore.kernel.org/linux-phy/20260327112858.r5lpqygtvsane2vf@skbuf/
---
drivers/ufs/host/ufs-qcom.c | 22 ++++++++--------------
1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index bc037db46624..9039b087bf21 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -513,13 +513,6 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba *hba)

/* phy initialization - calibrate the phy */
- ret = phy_init(phy);
- if (ret) {
- dev_err(hba->dev, "%s: phy init failed, ret = %d\n",
- __func__, ret);
- return ret;
- }
-
ret = phy_set_mode_ext(phy, mode, host->phy_gear);
if (ret)
goto out_disable_phy;
@@ -529,23 +522,18 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba *hba)
if (ret) {
dev_err(hba->dev, "%s: phy power on failed, ret = %d\n",
__func__, ret);
- goto out_disable_phy;
+ return ret;
}

ret = phy_calibrate(phy);
if (ret) {
dev_err(hba->dev, "Failed to calibrate PHY: %d\n", ret);
- goto out_disable_phy;
+ return ret;
}

ufs_qcom_select_unipro_mode(host);

return 0;
-
-out_disable_phy:
- phy_exit(phy);
-
- return ret;
}

/*
@@ -1625,6 +1613,12 @@ static int ufs_qcom_init(struct ufs_hba *hba)
if (err)
goto out_variant_clear;

+ err = phy_init(host->generic_phy);
+ if (err) {
+ dev_err(hba->dev, "phy_init failed: %pe\n", ERR_PTR(err));
+ goto out_variant_clear;
+ }
+
ufs_qcom_setup_clocks(hba, true, POST_CHANGE);

ufs_qcom_get_default_testbus_cfg(host);
--
2.34.1