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

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

[peter.wang]

From: Peter Wang <peter.wang@mediatek.com>

A race exists between ufshcd_auto_hibern8_update() and
ufs_mtk_pwr_change_notify() when both paths concurrently access
REG_AUTO_HIBERNATE_IDLE_TIMER. Additionally, the static local
variable used to save/restore AHIT in the power change path is
not re-entrant and may hold a stale value if a sysfs write races
with a gear change.

Introduce ahit_mutex and ahit_disable_depth in struct ufs_hba to
serialize AHIT register access. The depth counter tracks how many
host-driver paths have forced AHIT off; sysfs writes are suppressed
while it is non-zero and the register is restored from hba->ahit
only when the last disabler drops it to zero.

Fixes: f5ca8d0c7a63 ("scsi: ufs: host: mediatek: Disable auto-hibern8 during power mode changes")
Signed-off-by: Peter Wang <peter.wang@mediatek.com>
---
 drivers/ufs/core/ufshcd.c       | 14 +++++++++++++-
 drivers/ufs/host/ufs-mediatek.c | 16 ++++++++++++----
 include/ufs/ufshcd.h            |  8 ++++++++
 3 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 2ba244cf40ac..e88b905eb7bc 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -4748,7 +4748,17 @@ void ufshcd_auto_hibern8_update(struct ufs_hba *hba, u32 ahit)
 	if (!pm_runtime_suspended(&hba->ufs_device_wlun->sdev_gendev)) {
 		ufshcd_rpm_get_sync(hba);
 		ufshcd_hold(hba);
-		ufshcd_configure_auto_hibern8(hba);
+		/*
+		 * Serialize with the host driver's AHIT control. Skip the
+		 * register write while the driver has AHIT forced off; the
+		 * driver programs hba->ahit back when it re-enables it.
+		 * Mutex is taken here (not across the rpm calls above) so it
+		 * never blocks a resume that the driver's path may need.
+		 */
+		mutex_lock(&hba->ahit_mutex);
+		if (!hba->ahit_disable_depth)
+			ufshcd_configure_auto_hibern8(hba);
+		mutex_unlock(&hba->ahit_mutex);
 		ufshcd_release(hba);
 		ufshcd_rpm_put_sync(hba);
 	}
@@ -11268,6 +11278,8 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 
 	mutex_init(&hba->wb_mutex);
 
+	mutex_init(&hba->ahit_mutex);
+
 	init_rwsem(&hba->clk_scaling_lock);
 
 	ufshcd_init_clk_gating(hba);
diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c
index 814c1b7343b9..b6279edde176 100644
--- a/drivers/ufs/host/ufs-mediatek.c
+++ b/drivers/ufs/host/ufs-mediatek.c
@@ -1507,19 +1507,27 @@ static int ufs_mtk_pwr_change_notify(struct ufs_hba *hba,
 				struct ufs_pa_layer_attr *dev_req_params)
 {
 	int ret = 0;
-	static u32 reg;
 
 	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++;
+			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;
 	default:
 		ret = -EINVAL;
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index dfd302f2dc7c..be62165b785e 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -1132,6 +1132,14 @@ struct ufs_hba {
 
 	/* Auto-Hibernate Idle Timer register value */
 	u32 ahit;
+	/* Serializes AHIT register access between sysfs and the host driver */
+	struct mutex ahit_mutex;
+	/*
+	 * Nesting count of host-driver paths that have AHIT forced off.
+	 * Non-zero blocks sysfs AHIT writes; AHIT is reprogrammed only when
+	 * the last disabler drops it back to zero. Guarded by ahit_mutex.
+	 */
+	int ahit_disable_depth;
 
 	unsigned long outstanding_tasks;
 	spinlock_t outstanding_lock;
-- 
2.45.2

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

[sashiko-bot]

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

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

[Peter Wang (王信友)]

On Wed, 2026-09-09 at 09:23 +0000, sashiko-bot@kernel.org wrote:
> [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.

This is indeed an issue, I will fix it in the next version.

Thanks
Peter