Re: [PATCH v5 phy-next 10/27] scsi: ufs: qcom: keep parallel track of PHY power state

[Vladimir Oltean <vladimir.oltean@nxp.com>]

On Tue, Mar 24, 2026 at 11:00:10AM +0530, Manivannan Sadhasivam wrote:
> On Fri, Mar 20, 2026 at 12:32:24AM +0200, Vladimir Oltean wrote:
> > As explained in the similar ufs-exynos.c change, PHY consumer drivers
> > should not look at the phy->power_count, because in the general case
> > there might also be other consumers who have called phy_power_on() too,
> > so the fact that the power_count is non-zero does not mean that we did.
> > 
> > Moreover, struct phy will become opaque soon, so the qcom UFS driver
> > will not be able to apply this pattern. Keep parallel track of the PHY
> > power state, instead of looking at a field which will become unavailable
> > (phy->power_count).
> > 
> > About treating the phy_power_off() return code: from an API perspective,
> > this should have probably returned void, otherwise consumers would be
> > stuck in a state they can't escape. The provider, phy-qcom-qmp-ufs.c,
> > does return 0 in its power_off() implementation. I consider it safe to
> > discard potential errors from phy_power_off() instead of complicating
> > the phy_powered_on logic.
> > 
> 
> You could even simplify the code by getting rid of the 'phy_powered_on' check
> altogether. There is no real need to track the PHY power state in this driver.
> It is safe to call phy_power_off() without any checks.
> 
> - Mani

Ok.. as the author of commit 7bac65687510 ("scsi: ufs: qcom: Power off
the PHY if it was already powered on in ufs_qcom_power_up_sequence()"),
I assume you have hardware to test. Would you mind writing a patch that
I could pick up to replace this one with?

I suppose that the power_count test is somehow no longer necessary after
commit 77d2fa54a945 ("scsi: ufs: qcom : Refactor phy_power_on/off
calls"), but frankly I don't see it - the ufshcd state machine is a bit
too complicated for me to just statically analyze.