>From 8d156781d38597865da37a86417f553143d74eaa Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Fri, 27 Mar 2026 13:14:39 +0200 Subject: [PATCH 2/3] scsi: ufs: qcom: call phy_init() before phy_power_on() The Qualcomm UFS host controller driver 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. The expectation is that 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) -> phy_power_on(phy) -> 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" 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 from drivers. This is a PHY-internal field, and using it from drivers is highly likely to be incorrect, as this case showcases rather well. As commit 77d2fa54a945 ("scsi: ufs: qcom : Refactor phy_power_on/off calls") shows, this driver tries to couple the PHY power state with the HBA clocks, for power saving reasons. I won't try to change that, I will just move the phy_init() call earlier, to ufs_qcom_init(). After the phy_init() movement, ufs_qcom_power_up_sequence() should no longer need to do either phy_init() nor the conditional phy_power_off(). However, 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: Manivannan Sadhasivam Cc: "Martin K. Petersen" Cc: Nitin Rawat v5->v6: rewrite after actually understanding the core issue v4->v5: patch is new --- drivers/ufs/host/ufs-qcom.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c index 375fd24ba458..ffa70c6c7143 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; @@ -1441,6 +1434,13 @@ 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, "%s: phy_init failed, ret = %d\n", + __func__, err); + goto out_variant_clear; + } + ufs_qcom_setup_clocks(hba, true, POST_CHANGE); ufs_qcom_get_default_testbus_cfg(host); -- 2.34.1