phy_power_off() can safely be called even when PHY is not powered on. So
drop the PHY power_count check.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/ufs/host/ufs-qcom.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index 375fd24ba458..4a410a0137bb 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -508,9 +508,7 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba *hba)
 	if (ret)
 		return ret;
 
-	if (phy->power_count)
-		phy_power_off(phy);
-
+	phy_power_off(phy);
 
 	/* phy initialization - calibrate the phy */
 	ret = phy_init(phy);
-- 
2.51.0

On Wed, Mar 25, 2026 at 05:31:22PM +0530, Manivannan Sadhasivam wrote:
> phy_power_off() can safely be called even when PHY is not powered on. So
> drop the PHY power_count check.

Sorry, do you mind writing a more elaborate explanation _why_ you think
that phy_power_off() can safely be called even when the PHY is not
powered on? If 0, the power_count would become negative after
phy_power_off(), and a subsequent phy_power_on() would merely bump it
back to 0, thereby not calling into the driver's qmp_ufs_power_on().
If there is a regulator assigned to phy->pwr, this would see a call to
regulator_disable() with no prior call having been made to regulator_enable().
At the very least you'd want to explain in the commit message why that
is something to be desired, so as to give reviewers confidence that
conscientious thought has been given to the change.

On Wed, Mar 25, 2026 at 02:29:22PM +0200, Vladimir Oltean wrote:
> On Wed, Mar 25, 2026 at 05:31:22PM +0530, Manivannan Sadhasivam wrote:
> > phy_power_off() can safely be called even when PHY is not powered on. So
> > drop the PHY power_count check.
> 
> Sorry, do you mind writing a more elaborate explanation _why_ you think
> that phy_power_off() can safely be called even when the PHY is not
> powered on? If 0, the power_count would become negative after
> phy_power_off(), and a subsequent phy_power_on() would merely bump it
> back to 0, thereby not calling into the driver's qmp_ufs_power_on().
> If there is a regulator assigned to phy->pwr, this would see a call to
> regulator_disable() with no prior call having been made to regulator_enable().
> At the very least you'd want to explain in the commit message why that
> is something to be desired, so as to give reviewers confidence that
> conscientious thought has been given to the change.

Sorry, I just rushed through it. So far in my tests, phy_power_on() gets called
first, before phy_power_off() in ufs_qcom_power_up_sequence(), so I mistakenly
thought that the clk, regulator disable APIs will silently bail out if not
enabled earlier. But I was wrong as they will just scream out loudly with
WARN() and that's too bad.

Though I couldn't trigger that in my tests, I'm still not 100% sure about that.
So let's drop this patch and proceed with your state tracking patch instead.

Apologies for the noise!

- Mani

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