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 --- Cc: "James E.J. Bottomley" Cc: "Martin K. Petersen" Cc: Dmitry Baryshkov Cc: Nitin Rawat Cc: Manivannan Sadhasivam 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