[PATCH] scsi: ufs: mediatek: Handle mPHY power-on failures

[PATCH] scsi: ufs: mediatek: Handle mPHY power-on failures

[Pengpeng Hou]

ufs_mtk_mphy_power_on() enables the optional VA09 supply before powering
on the mPHY.  The PHY result is ignored, so a failure leaves VA09
enabled and records mphy_powered_on as true, suppressing later retries.
The initialization path also ignores the helper result and continues
setting up the host.

Check phy_power_on(), turn VA09 back off on failure, and leave the state
flag unchanged.  Propagate the helper error from initialization as well
as the existing resume caller.  Keep regulator and mPHY diagnostics on
separate paths so regulator errors are not mislabeled.

The issue was identified via static analysis and manually reviewed.

Fixes: 561e3a8726b2 ("scsi: ufs-mediatek: Fix unbalanced clock on/off")
Fixes: cf137b3ea49a ("scsi: ufs-mediatek: Support VA09 regulator operations")

Assisted-by: LLM
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 drivers/ufs/host/ufs-mediatek.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c
index 814c1b7343b9..9d481147ea38 100644
--- a/drivers/ufs/host/ufs-mediatek.c
+++ b/drivers/ufs/host/ufs-mediatek.c
@@ -509,6 +509,7 @@ static int ufs_mtk_mphy_power_on(struct ufs_hba *hba, bool on)
 	struct ufs_mtk_host *host = ufshcd_get_variant(hba);
 	struct phy *mphy = host->mphy;
 	struct arm_smccc_res res;
+	int rollback_ret;
 	int ret = 0;
 
 	if (!mphy || !(on ^ host->mphy_powered_on))
@@ -523,7 +524,18 @@ static int ufs_mtk_mphy_power_on(struct ufs_hba *hba, bool on)
 			usleep_range(200, 210);
 			ufs_mtk_va09_pwr_ctrl(res, 1);
 		}
-		phy_power_on(mphy);
+		ret = phy_power_on(mphy);
+		if (ret) {
+			if (ufs_mtk_is_va09_supported(hba)) {
+				ufs_mtk_va09_pwr_ctrl(res, 0);
+				rollback_ret = regulator_disable(host->reg_va09);
+				if (rollback_ret)
+					dev_warn(hba->dev, "failed to disable va09 after mphy failure: %d\n",
+						 rollback_ret);
+			}
+			dev_info(hba->dev, "failed to enable mphy: %d\n", ret);
+			return ret;
+		}
 	} else {
 		phy_power_off(mphy);
 		if (ufs_mtk_is_va09_supported(hba)) {
@@ -1295,7 +1307,9 @@ static int ufs_mtk_init(struct ufs_hba *hba)
 	 *
 	 * Enable phy clocks specifically here.
 	 */
-	ufs_mtk_mphy_power_on(hba, true);
+	err = ufs_mtk_mphy_power_on(hba, true);
+	if (err)
+		goto out_variant_clear;
 
 	if (ufs_mtk_is_rtff_mtcmos(hba)) {
 		/* First Restore here, to avoid backup unexpected value */
-- 
2.43.0

Re: [PATCH] scsi: ufs: mediatek: Handle mPHY power-on failures

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Failure in `regulator_disable()` during power-off leaves the software state flagged as 'powered on', despite the physical PHY being powered down, which breaks subsequent power-on attempts.
- [High] Unbalanced PHY power management leading to clock setup with an unpowered PHY and VA09 regulator during resume.
--

commit abe799aa942a1ffc27379395adfb1d36f8c84af2
Author: Pengpeng Hou <pengpeng@iscas.ac.cn>

scsi: ufs: mediatek: Handle mPHY power-on failures

This patch updates ufs_mtk_mphy_power_on() to handle phy_power_on() failures.
If the PHY fails to power on, the VA09 regulator is turned back off to prevent
it from being left enabled, and the initialization path now checks and
propagates this error.

> diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c
> index 865be3ddf448e..92dbd240c7ea5 100644
> --- a/drivers/ufs/host/ufs-mediatek.c
> +++ b/drivers/ufs/host/ufs-mediatek.c

[ ... ]

> @@ -524,7 +525,18 @@ static int ufs_mtk_mphy_power_on(struct ufs_hba *hba, bool on)
>  			usleep_range(200, 210);
>  			ufs_mtk_va09_pwr_ctrl(res, 1);
>  		}
> -		phy_power_on(mphy);
> +		ret = phy_power_on(mphy);
> +		if (ret) {
> +			if (ufs_mtk_is_va09_supported(hba)) {
> +				ufs_mtk_va09_pwr_ctrl(res, 0);
> +				rollback_ret = regulator_disable(host->reg_va09);
> +				if (rollback_ret)
> +					dev_warn(hba->dev, "failed to disable va09 after mphy failure: %d\n",
> +						 rollback_ret);
> +			}
> +			dev_info(hba->dev, "failed to enable mphy: %d\n", ret);
> +			return ret;
> +		}
>  	} else {
>  		phy_power_off(mphy);
>  		if (ufs_mtk_is_va09_supported(hba)) {

[Severity: High]
This isn't a bug introduced by this patch, but if regulator_disable() fails
here during power-off, does it leave the software state permanently out of sync?

In ufs_mtk_mphy_power_on(), we have:

	} else {
		phy_power_off(mphy);
		if (ufs_mtk_is_va09_supported(hba)) {
			ufs_mtk_va09_pwr_ctrl(res, 0);
			ret = regulator_disable(host->reg_va09);
		}
	}
out:
	if (ret) {
        ...
	} else {
		host->mphy_powered_on = on;
	}

If regulator_disable() returns an error, host->mphy_powered_on won't be
updated to false, even though phy_power_off() was already executed. Can this
cause future calls to ufs_mtk_mphy_power_on(hba, true) to early-exit, leaving
the PHY physically unpowered?


[Severity: High]
This is also a pre-existing issue, but does the current power management flow
cause unbalanced PHY power off calls?

During suspend, ufs_mtk_suspend() calls ufs_mtk_mphy_power_on(false), which
executes phy_power_off(mphy) and disables the regulator.

Later in the suspend path, ufshcd_suspend() calls ufs_mtk_setup_clocks(false),
which triggers ufs_mtk_pwr_ctrl(false). This redundantly calls
phy_power_off(mphy) again, dropping its internal power_count to -1.

During resume, ufs_mtk_pwr_ctrl(true) calls phy_power_on(mphy). Because
the power_count was -1, it only increments to 0 without actually powering on
the hardware. The VA09 regulator is also left disabled.

ufs_mtk_pwr_ctrl() then calls ufs_mtk_setup_ref_clk(true), which attempts to
poll reference clock registers on physically unpowered hardware.

Does this unconditionally lead to a timeout and broken I/O during resume on
systems binding a PHY?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260828092311.10774-1-pengpeng@iscas.ac.cn?part=1

Re: [PATCH] scsi: ufs: mediatek: Handle mPHY power-on failures

[Stanley Jhu]

On Fri, 28 Aug 2026 17:23:11 +0800, Pengpeng Hou wrote:
> @@ -509,6 +509,7 @@ static int ufs_mtk_mphy_power_on(struct ufs_hba *hba, bool on)
>       struct ufs_mtk_host *host = ufshcd_get_variant(hba);
>       struct phy *mphy = host->mphy;
>       struct arm_smccc_res res;
> +     int rollback_ret;
>       int ret = 0;

`rollback_ret` feels overly verbose; a shorter name like `err` would be
cleaner.

> +             ret = phy_power_on(mphy);
> +             if (ret) {
> +                     if (ufs_mtk_is_va09_supported(hba)) {
> +                             ufs_mtk_va09_pwr_ctrl(res, 0);
> +                             rollback_ret = regulator_disable(host->reg_va09);
> +                             if (rollback_ret)
> +                                     dev_warn(hba->dev, "failed to disable va09 after mphy failure: %d\n",
> +                                              rollback_ret);
> +                     }
> +                     dev_info(hba->dev, "failed to enable mphy: %d\n", ret);
> +                     return ret;
> +             }

`dev_info()` is inappropriate for an operation failure; please use `dev_err()`
instead.

Thanks,
Stanley Jhu