On Thu, Mar 26, 2026 at 10:04:44AM +0200, Vladimir Oltean wrote:

On Wed, Mar 25, 2026 at 01:57:31PM +0200, Vladimir Oltean wrote:

quoted

On Wed, Mar 25, 2026 at 05:21:14PM +0530, Manivannan Sadhasivam wrote:

quoted

I believe I added the power_count check for phy_exit(). But since that got
moved, the check becomes no longer necessary.

FYI, the power_count keeps track of the balance of phy_power_on() and
phy_power_off() calls, whereas it is the init_count keeps track of
phy_init() and phy_exit() calls. They are only related to the extent
that you must respect the phy_init() -> phy_power_on() -> phy_power_off()
-> phy_exit() sequence. But in any case, both should be considered
PHY-internal fields. The "Order of API calls" section from
Documentation/driver-api/phy/phy.rst mentions the order that I just
described above, and consumers should just ensure they follow that.

Ok, so we can close this topic of "checking the power_count not needed"
by linking to the conversation which spun off here:
https://lore.kernel.org/lkml/20260325120122.265973-1-manivannan.sadhasivam@oss.qualcomm.com/ (local)

Sure.

Mani, I spent some more time to figure out what's really going on with
this unexpected phy_power_off() call. Do you think you could
regression-test the patch attached?

I tested the patch. But it fails ufs_qcom_power_up_sequence() if PHY was already
powered on:

[   31.513321] qcom-qmp-ufs-phy 1d87000.phy: phy initialization timed-out
[   31.513335] ufshcd-qcom 1d84000.ufshc: Failed to calibrate PHY: -110
[   31.565273] ufshcd-qcom 1d84000.ufshc: Enabling the controller failed

Funny thing is, it didn't affect the functionality since the UFS core retries
ufshcd_hba_enable() and in the error path of ufs_qcom_power_up_sequence(),
phy_power_off() gets called and that causes the next try to succeed. So it is
evident that, if PHY was already powered ON, it should be powered off before
ufs_qcom_phy_power_on(). And due to the UFS driver design,
ufs_qcom_power_up_sequence() can get called multiple times. So we cannot just
remove phy_power_off().

Below diff on top of your patch fixes the issue:

diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index ed067247d72a..2c9fe03f349e 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -567,6 +567,8 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba *hba)
        if (ret)
                return ret;
 
+       ufs_qcom_phy_power_off(host);
+
        ret = ufs_qcom_phy_set_gear(host, mode);
        if (ret) {
                dev_err(hba->dev, "%s: phy_set_mode_ext() failed, ret = %d\n",

- Mani

-- 
மணிவண்ணன் சதாசிவம்