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

[Vladimir Oltean]

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 at nxp.com>
---
Cc: "James E.J. Bottomley" <James.Bottomley at HansenPartnership.com>
Cc: "Martin K. Petersen" <martin.petersen at oracle.com>
Cc: Dmitry Baryshkov <dmitry.baryshkov at oss.qualcomm.com>
Cc: Nitin Rawat <quic_nitirawa at quicinc.com>
Cc: Manivannan Sadhasivam <mani at 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