Re: [PATCH v14 05/23] phy: mediatek: ufs: Add support for resets

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] phy: mediatek: ufs: ufs_mtk_phy_ctrl silently ignores unknown SMC error codes
--

commit f23118e44631c954bc88642a9abc5f62081c23bf
Author: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

phy: mediatek: ufs: Add support for resets

The MediaTek UFS PHY supports PHY resets. Until now, they've been
implemented in the UFS host driver. Since they were never documented in
the UFS HCI node's DT bindings, and no mainline DT uses it, it's fine if
it's moved to the correct location, which is the PHY driver.

> diff --git a/drivers/phy/mediatek/phy-mtk-ufs.c b/drivers/phy/mediatek/phy-mtk-ufs.c
> index fc19e0fa8ed55..4439a9eb6b7ad 100644
> --- a/drivers/phy/mediatek/phy-mtk-ufs.c
> +++ b/drivers/phy/mediatek/phy-mtk-ufs.c

[ ... ]

> @@ -140,9 +151,59 @@ static int ufs_mtk_phy_power_off(struct phy *generic_phy)
>  	return 0;
>  }
>  
> +static int ufs_mtk_phy_ctrl(struct ufs_mtk_phy *phy, enum ufs_mtk_mphy_op op)
> +{
> +	struct arm_smccc_res res;
> +
> +	arm_smccc_smc(MTK_SIP_UFS_CONTROL, UFS_MTK_SIP_MPHY_CTRL, op,
> +		      0, 0, 0, 0, 0, &res);
> +
> +	switch (res.a0) {
> +	case SMCCC_RET_NOT_SUPPORTED:
> +		return -EOPNOTSUPP;
> +	case SMCCC_RET_INVALID_PARAMETER:
> +		return -EINVAL;
> +	default:
> +		return 0;

[Severity: Medium]
In ufs_mtk_phy_ctrl(), will this default case silently swallow other error
codes from the SMC call and treat them as success?

For example, mtk_sip_svc.h defines SIP_SVC_E_INVALID_PARAMS as -2 and
SIP_SVC_E_PERMISSION_DENIED as -4. Standard SMCCC errors like
SMCCC_RET_NOT_REQUIRED (-2) could also be returned.

If the firmware returns one of these errors, the default case will return 0,
making the driver assume the MPHY backup or restore operation succeeded when
it actually failed. Should this explicitly check for success (res.a0 == 0)
and return an error for any other unhandled code?

> +	}
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016-mt8196-ufs-v14-0-e8d565e0cd45@collabora.com?part=5