Re: [PATCH v1 01/10] ufs: host: mediatek: Fix runtime suspend error deadlock

[Bart Van Assche <bvanassche@acm.org>]

On 9/18/25 3:36 AM, peter.wang@mediatek.com wrote:
> Fix the deadlock issue during runtime suspend by checking
> the error handler's progress. If the error handler is active,
> break the runtime suspend process by returning -EAGAIN.
> This approach prevents potential deadlocks when acquiring
> runtime PM and enhances system stability.

"enhances system stability" sounds like marketing language to me. Please
provide a root-cause analysis and also explain why this change is only
required for the MediaTek driver and not for any other UFS host drivers.

> diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c
> index 758a393a9de1..b1797386668c 100644
> --- a/drivers/ufs/host/ufs-mediatek.c
> +++ b/drivers/ufs/host/ufs-mediatek.c
> @@ -1746,9 +1746,15 @@ static int ufs_mtk_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op,
>   	struct arm_smccc_res res;
>   
>   	if (status == PRE_CHANGE) {
> -		if (ufshcd_is_auto_hibern8_supported(hba))
> -			return ufs_mtk_auto_hibern8_disable(hba);
> -		return 0;
> +		if (!ufshcd_is_auto_hibern8_supported(hba))
> +			return 0;
> +		err = ufs_mtk_auto_hibern8_disable(hba);
> +
> +		/* May trigger EH work without exiting hibern8 error */
> +		if (ufshcd_eh_in_progress(hba))
> +			return -EAGAIN;
> +		else
> +			return err;
>   	}
>   
>   	if (ufshcd_is_link_hibern8(hba)) {

How can ufs_mtk_suspend() be called while the error handler is in
progress? ufshcd_err_handler() disables RPM before it sets the
UFSHCD_EH_IN_PROGRESS flag.

> diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
> index ea0021f067c9..45e2ca65de90 100644
> --- a/include/ufs/ufshcd.h
> +++ b/include/ufs/ufshcd.h
> @@ -97,6 +97,11 @@ enum uic_link_state {
>   	UIC_LINK_BROKEN_STATE	= 3, /* Link is in broken state */
>   };
>   
> +/* UFSHCD error handling flags */
> +enum {
> +	UFSHCD_EH_IN_PROGRESS = (1 << 0),
> +};
> +
>   #define ufshcd_is_link_off(hba) ((hba)->uic_link_state == UIC_LINK_OFF_STATE)
>   #define ufshcd_is_link_active(hba) ((hba)->uic_link_state == \
>   				    UIC_LINK_ACTIVE_STATE)
> @@ -129,6 +134,13 @@ enum uic_link_state {
>   #define ufshcd_is_ufs_dev_deepsleep(h) \
>   	((h)->curr_dev_pwr_mode == UFS_DEEPSLEEP_PWR_MODE)
>   
> +#define ufshcd_set_eh_in_progress(h) \
> +	((h)->eh_flags |= UFSHCD_EH_IN_PROGRESS)
> +#define ufshcd_eh_in_progress(h) \
> +	((h)->eh_flags & UFSHCD_EH_IN_PROGRESS)
> +#define ufshcd_clear_eh_in_progress(h) \
> +	((h)->eh_flags &= ~UFSHCD_EH_IN_PROGRESS)

The UFSHCD_EH_IN_PROGRESS definition and also the
ufshcd_set_eh_in_progress() and ufshcd_clear_eh_in_progress()
definitions must remain in the UFS core private code. Please do not move
these definitions into the include/ufs/ufshcd.h header file.

Thanks,

Bart.