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

[Vladimir Oltean]

 On Fri, Mar 27, 2026 at 12:22:46PM +0530, Manivannan Sadhasivam wrote:
> 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

Understood. Thanks for testing.

I'm still not satisfied with this level of complexity. If I get you
right, ufs_qcom_phy_power_off() is still needed because phy_calibrate()
expects a "fresh after power on" state, otherwise it fails? That would
be the second reason, apart from the first one I already identified
(undo a phy_power_on() done prior to phy_init()).

If so, could you please test the 3 patches attached (no relationship
with anything else we've exchanged thus far)?