Re: [PATCH v1] ufs: core: serialize AHIT register access between sysfs and host driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The `ahit_disable_depth` counter leaks when a DME power mode change fails, permanently blocking sysfs AHIT updates.
--

commit 1d1062f790f3ef8d13bc9cf9bc515eb73918a76f
Author: Peter Wang <peter.wang@mediatek.com>

ufs: core: serialize AHIT register access between sysfs and host driver

This commit introduces a mutex and depth counter to serialize Auto-Hibernate
Idle Timer (AHIT) register access between sysfs updates and power mode changes.
The counter suppresses sysfs writes while a host driver has forced AHIT off.

> diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c
> index 541bbaf92c21c..b0ce134ec9cf1 100644
> --- a/drivers/ufs/host/ufs-mediatek.c
> +++ b/drivers/ufs/host/ufs-mediatek.c
> @@ -1508,19 +1508,27 @@ static int ufs_mtk_pwr_change_notify(struct ufs_hba *hba,
>  				struct ufs_pa_layer_attr *dev_req_params)
>  {
>  	int ret = 0;
>  
>  	switch (stage) {
>  	case PRE_CHANGE:
>  		if (ufshcd_is_auto_hibern8_supported(hba)) {
> -			reg = ufshcd_readl(hba, REG_AUTO_HIBERNATE_IDLE_TIMER);
> +			/* Block sysfs AHIT writes while we force AHIT off */
> +			mutex_lock(&hba->ahit_mutex);
> +			hba->ahit_disable_depth++;

[Severity: High]
Does this code leak the ahit_disable_depth counter if a power mode change
fails?

If ufshcd_dme_change_power_mode() fails after ufshcd_vops_pwr_change_notify()
has run for PRE_CHANGE, the POST_CHANGE notification is skipped:

drivers/ufs/core/ufshcd.c:ufshcd_change_power_mode() {
    ...
	ufshcd_vops_pwr_change_notify(hba, PRE_CHANGE, pwr_mode);

	ret = ufshcd_dme_change_power_mode(hba, pwr_mode, pmc_policy);

	if (!ret)
		ufshcd_vops_pwr_change_notify(hba, POST_CHANGE, pwr_mode);
    ...
}

Because POST_CHANGE is never called on failure, the ahit_disable_depth
counter is never decremented. This permanently blocks sysfs AHIT updates in
ufshcd_auto_hibern8_update() until the device is reset or the system is
rebooted.

> +			mutex_unlock(&hba->ahit_mutex);
>  			ufs_mtk_auto_hibern8_disable(hba);
>  		}
>  		ret = ufs_mtk_pre_pwr_change(hba, dev_req_params);
>  		break;
>  	case POST_CHANGE:
> -		if (ufshcd_is_auto_hibern8_supported(hba))
> -			ufshcd_writel(hba, reg, REG_AUTO_HIBERNATE_IDLE_TIMER);
> +		if (ufshcd_is_auto_hibern8_supported(hba)) {
> +			mutex_lock(&hba->ahit_mutex);
> +			/* re-enable only when the last disabler drops off */
> +			if (!--hba->ahit_disable_depth)
> +				ufshcd_writel(hba, hba->ahit,
> +					      REG_AUTO_HIBERNATE_IDLE_TIMER);
> +			mutex_unlock(&hba->ahit_mutex);
> +		}
>  		break;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909091120.1134414-1-peter.wang@mediatek.com?part=1