* [PATCH v2 0/2] ufs: fix POST_CHANGE pairing and AHIT race condition
@ 2026-09-14 8:49 peter.wang
2026-09-14 8:49 ` [PATCH v2 1/2] ufs: always notify POST_CHANGE even on gear switch failure peter.wang
` (4 more replies)
0 siblings, 5 replies; 12+ messages in thread
From: peter.wang @ 2026-09-14 8:49 UTC (permalink / raw)
To: linux-scsi, martin.petersen, avri.altman, alim.akhtar, bvanassche,
santosh.sy, h.vinayak
Cc: wsd_upstream, linux-mediatek, peter.wang, chun-hung.wu,
alice.chao, cc.chou, chaotian.jing, tun-yu.yu, naomi.chu, ed.tsai
From: Peter Wang <peter.wang@mediatek.com>
Patch 1 fixes a bug where POST_CHANGE was skipped on a failed gear
switch, leaving the host in an inconsistent state if resources acquired
in PRE_CHANGE were never released.
Patch 2 fixes a race between the sysfs AHIT update path and the
MediaTek host driver's power mode change path. A mutex and depth
counter are introduced to serialize AHIT register access and ensure
AHIT is correctly restored after the last disabler completes.
The two patches are related: patch 1 guarantees POST_CHANGE is always
called, ensuring the depth counter introduced in patch 2 is always
properly decremented.
Peter Wang (2):
ufs: always notify POST_CHANGE even on gear switch failure
ufs: serialize AHIT register access between sysfs and host driver
drivers/ufs/core/ufshcd.c | 22 +++++++++++++++++++---
drivers/ufs/host/ufs-exynos.c | 4 +++-
drivers/ufs/host/ufs-mediatek.c | 16 ++++++++++++----
drivers/ufs/host/ufshcd-pci.c | 3 ++-
include/ufs/ufshcd.h | 8 ++++++++
5 files changed, 44 insertions(+), 9 deletions(-)
--
2.45.2
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 1/2] ufs: always notify POST_CHANGE even on gear switch failure
2026-09-14 8:49 [PATCH v2 0/2] ufs: fix POST_CHANGE pairing and AHIT race condition peter.wang
@ 2026-09-14 8:49 ` peter.wang
2026-09-14 9:02 ` sashiko-bot
2026-09-14 8:49 ` [PATCH v2 2/2] ufs: serialize AHIT register access between sysfs and host driver peter.wang
` (3 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: peter.wang @ 2026-09-14 8:49 UTC (permalink / raw)
To: linux-scsi, martin.petersen, avri.altman, alim.akhtar, bvanassche,
santosh.sy, h.vinayak
Cc: wsd_upstream, linux-mediatek, peter.wang, chun-hung.wu,
alice.chao, cc.chou, chaotian.jing, tun-yu.yu, naomi.chu, ed.tsai
From: Peter Wang <peter.wang@mediatek.com>
Previously, POST_CHANGE was only notified on a successful gear switch.
This could leave the host in an inconsistent state if resources were
acquired in PRE_CHANGE (e.g. disabling auto-hibern8) but never
released due to a missing POST_CHANGE.
Fix this by always calling POST_CHANGE regardless of the gear switch
result. On failure, pass dev_req_params = NULL to signal that there is
nothing to apply; hosts that receive NULL simply skip their post work.
Update the affected vendor drivers (ufs-exynos and ufshcd-pci) to
guard their POST_CHANGE logic with a NULL check so they correctly
handle the failure case.
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 | 8 ++++++--
drivers/ufs/host/ufs-exynos.c | 4 +++-
drivers/ufs/host/ufshcd-pci.c | 3 ++-
3 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 2ba244cf40ac..39a4bdafc1af 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -4979,8 +4979,12 @@ int ufshcd_change_power_mode(struct ufs_hba *hba,
ret = ufshcd_dme_change_power_mode(hba, pwr_mode, pmc_policy);
- if (!ret)
- ufshcd_vops_pwr_change_notify(hba, POST_CHANGE, pwr_mode);
+ /*
+ * Always notify POST_CHANGE to pair with PRE_CHANGE, even on failure.
+ * On failure, pass dev_req_params = NULL to indicate nothing to apply.
+ * POST_CHANGE return value is advisory and does not mask @ret.
+ */
+ ufshcd_vops_pwr_change_notify(hba, POST_CHANGE, ret ? NULL : pwr_mode);
return ret;
}
diff --git a/drivers/ufs/host/ufs-exynos.c b/drivers/ufs/host/ufs-exynos.c
index b2f65c465525..3bcbc9ebbb28 100644
--- a/drivers/ufs/host/ufs-exynos.c
+++ b/drivers/ufs/host/ufs-exynos.c
@@ -1782,7 +1782,9 @@ static int exynos_ufs_pwr_change_notify(struct ufs_hba *hba,
ret = exynos_ufs_pre_pwr_mode(hba, dev_req_params);
break;
case POST_CHANGE:
- ret = exynos_ufs_post_pwr_mode(hba, dev_req_params);
+ /* NULL when the gear switch failed: nothing to apply */
+ if (dev_req_params)
+ ret = exynos_ufs_post_pwr_mode(hba, dev_req_params);
break;
}
diff --git a/drivers/ufs/host/ufshcd-pci.c b/drivers/ufs/host/ufshcd-pci.c
index 21bb11c724be..668b57403e62 100644
--- a/drivers/ufs/host/ufshcd-pci.c
+++ b/drivers/ufs/host/ufshcd-pci.c
@@ -166,7 +166,8 @@ static int ufs_intel_lkf_pwr_change_notify(struct ufs_hba *hba,
ufs_intel_set_lanes(hba, 2);
break;
case POST_CHANGE:
- if (ufshcd_is_hs_mode(dev_req_params)) {
+ /* NULL when the gear switch failed: nothing to apply */
+ if (dev_req_params && ufshcd_is_hs_mode(dev_req_params)) {
u32 peer_granularity;
usleep_range(1000, 1250);
--
2.45.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 2/2] ufs: serialize AHIT register access between sysfs and host driver
2026-09-14 8:49 [PATCH v2 0/2] ufs: fix POST_CHANGE pairing and AHIT race condition peter.wang
2026-09-14 8:49 ` [PATCH v2 1/2] ufs: always notify POST_CHANGE even on gear switch failure peter.wang
@ 2026-09-14 8:49 ` peter.wang
2026-09-14 9:13 ` sashiko-bot
2026-09-22 10:06 ` [PATCH v2 0/2] ufs: fix POST_CHANGE pairing and AHIT race condition chun-hung.wu
` (2 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: peter.wang @ 2026-09-14 8:49 UTC (permalink / raw)
To: linux-scsi, martin.petersen, avri.altman, alim.akhtar, bvanassche,
santosh.sy, h.vinayak
Cc: wsd_upstream, linux-mediatek, peter.wang, chun-hung.wu,
alice.chao, cc.chou, chaotian.jing, tun-yu.yu, naomi.chu, ed.tsai
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 39a4bdafc1af..c54ff35b4196 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);
}
@@ -11272,6 +11282,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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] ufs: always notify POST_CHANGE even on gear switch failure
2026-09-14 8:49 ` [PATCH v2 1/2] ufs: always notify POST_CHANGE even on gear switch failure peter.wang
@ 2026-09-14 9:02 ` sashiko-bot
2026-09-14 12:42 ` Peter Wang (王信友)
0 siblings, 1 reply; 12+ messages in thread
From: sashiko-bot @ 2026-09-14 9:02 UTC (permalink / raw)
To: peter.wang; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Passing `NULL` for `dev_req_params` on failure causes spurious error logs in unmodified vendor drivers (`ufs-qcom`, `ufs-hisi`) and destroys the context needed for proper resource rollback.
--
commit e9e864935f979762e71567d19e93d8a30870f23b
Author: Peter Wang <peter.wang@mediatek.com>
ufs: always notify POST_CHANGE even on gear switch failure
This commit modifies ufshcd_change_power_mode() to always notify POST_CHANGE,
even if the gear switch fails. On failure, it passes NULL for dev_req_params
and updates ufs-exynos and ufshcd-pci to handle the NULL argument.
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 234e18b5078f2..2e7a592dbf2f1 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -5003,8 +5003,12 @@ int ufshcd_change_power_mode(struct ufs_hba *hba,
>
> ret = ufshcd_dme_change_power_mode(hba, pwr_mode, pmc_policy);
>
> - if (!ret)
> - ufshcd_vops_pwr_change_notify(hba, POST_CHANGE, pwr_mode);
> + /*
> + * Always notify POST_CHANGE to pair with PRE_CHANGE, even on failure.
> + * On failure, pass dev_req_params = NULL to indicate nothing to apply.
> + * POST_CHANGE return value is advisory and does not mask @ret.
> + */
> + ufshcd_vops_pwr_change_notify(hba, POST_CHANGE, ret ? NULL : pwr_mode);
[Severity: Medium]
Will passing NULL for dev_req_params on failure cause issues for unmodified
vendor drivers?
Looking at ufs_qcom_pwr_change_notify() in drivers/ufs/host/ufs-qcom.c, it
has an early check that will trap this NULL argument and log a spurious error
before any POST_CHANGE cleanup can execute:
drivers/ufs/host/ufs-qcom.c:ufs_qcom_pwr_change_notify() {
if (!dev_req_params) {
pr_err("%s: incoming dev_req_params is NULL\n", __func__);
return -EINVAL;
}
...
}
Similarly, ufs_hisi_pwr_change_notify() in drivers/ufs/host/ufs-hisi.c will
also log a spurious error on every power mode change failure:
drivers/ufs/host/ufs-hisi.c:ufs_hisi_pwr_change_notify() {
if (!dev_req_params) {
dev_err(hba->dev,
"%s: incoming dev_req_params is NULL\n", __func__);
ret = -EINVAL;
goto out;
}
...
}
Additionally, does passing NULL here destroy the context needed for proper
resource rollback?
For instance, in ufs_qcom_pwr_change_notify(), resources are acquired in
PRE_CHANGE based on the dev_req_params:
drivers/ufs/host/ufs-qcom.c:ufs_qcom_pwr_change_notify() {
...
/* enable the device ref clock before changing to HS mode */
if (!ufshcd_is_hs_mode(&hba->pwr_info) &&
ufshcd_is_hs_mode(dev_req_params))
ufs_qcom_dev_ref_clk_ctrl(host, true);
...
}
By omitting the original parameters on failure, the target drivers are prevented
from using the requested mode context in POST_CHANGE to safely revert their
state.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260914085148.136429-1-peter.wang@mediatek.com?part=1
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] ufs: serialize AHIT register access between sysfs and host driver
2026-09-14 8:49 ` [PATCH v2 2/2] ufs: serialize AHIT register access between sysfs and host driver peter.wang
@ 2026-09-14 9:13 ` sashiko-bot
2026-09-14 12:44 ` Peter Wang (王信友)
0 siblings, 1 reply; 12+ messages in thread
From: sashiko-bot @ 2026-09-14 9:13 UTC (permalink / raw)
To: peter.wang; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] The `ufshcd_auto_hibern8_update()` function ignores the return value of `ufshcd_rpm_get_sync()`. If the runtime PM resume fails, it proceeds to unsafely write to the `REG_AUTO_HIBERNATE_IDLE_TIMER` register on a potentially suspended device.
- [High] Potential NULL pointer dereference in `ufshcd_auto_hibern8_update` if called via sysfs before the device WLUN is fully initialized.
- [High] The UFS core driver registers a shared IRQ (`IRQF_SHARED`) but calls `disable_irq()` during runtime suspend instead of `synchronize_irq()`, and fails to verify device activity in its interrupt handler.
--
commit 10d014f6ed545565699c7988448c9337de308b17
Author: Peter Wang <peter.wang@mediatek.com>
ufs: serialize AHIT register access between sysfs and host driver
This commit introduces a mutex and a depth counter to serialize AHIT register
access between the host driver and sysfs writes. This prevents stale values
from being programmed if a sysfs write races with a gear change.
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 2e7a592dbf2f1..1d168eecfd013 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -4772,7 +4772,17 @@ void ufshcd_auto_hibern8_update(struct ufs_hba *hba, u32 ahit)
> if (!pm_runtime_suspended(&hba->ufs_device_wlun->sdev_gendev)) {
[Severity: High]
This isn't a bug introduced by this patch, but can this dereference a
NULL pointer?
Sysfs attributes are exposed in ufshcd_init() via ufs_sysfs_add_nodes()
before hba->ufs_device_wlun is allocated. The initialization is deferred
to the ufshcd_async_scan worker.
If a user writes to the sysfs node before ufshcd_add_lus() allocates
hba->ufs_device_wlun, can auto_hibern8_store() call
ufshcd_auto_hibern8_update() and locklessly dereference a NULL pointer here?
> ufshcd_rpm_get_sync(hba);
[Severity: High]
This is a pre-existing issue, but is it safe to ignore the return value
of ufshcd_rpm_get_sync() here?
If the runtime PM resume fails (e.g., due to a power domain failure or
device removal), pm_runtime_get_sync() returns a negative error code.
Does ignoring this error allow the function to proceed and write to
REG_AUTO_HIBERNATE_IDLE_TIMER on a potentially suspended device, causing a
synchronous external abort?
> 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);
> }
[ ... ]
[Severity: High]
This is a pre-existing issue, but does the UFS core driver handle shared
IRQs correctly?
The driver registers its IRQ with IRQF_SHARED. When the UFS controller enters
runtime suspend, ufshcd_suspend() disables the interrupt line entirely:
drivers/ufs/core/ufshcd.c:ufshcd_suspend() {
...
ufshcd_disable_irq(hba);
...
}
Does calling disable_irq() mask interrupts for any other devices sharing the
line? Should this use synchronize_irq() instead?
Additionally, ufshcd_intr() reads the hardware register unconditionally:
drivers/ufs/core/ufshcd.c:ufshcd_intr() {
...
last_intr_status = intr_status = ufshcd_readl(hba, REG_INTERRUPT_STATUS);
...
}
Can this access powered-off hardware if the interrupt belongs to another
device on the same shared line? Should this verify the device is actively
powered using pm_runtime_get_if_active() before accessing registers?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260914085148.136429-1-peter.wang@mediatek.com?part=2
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] ufs: always notify POST_CHANGE even on gear switch failure
2026-09-14 9:02 ` sashiko-bot
@ 2026-09-14 12:42 ` Peter Wang (王信友)
0 siblings, 0 replies; 12+ messages in thread
From: Peter Wang (王信友) @ 2026-09-14 12:42 UTC (permalink / raw)
To: sashiko-reviews@lists.linux.dev; +Cc: linux-scsi@vger.kernel.org
On Mon, 2026-09-14 at 09:02 +0000, sashiko-bot@kernel.org wrote
>
> [Severity: Medium]
> Will passing NULL for dev_req_params on failure cause issues for
> unmodified
> vendor drivers?
>
> Looking at ufs_qcom_pwr_change_notify() in drivers/ufs/host/ufs-
> qcom.c, it
> has an early check that will trap this NULL argument and log a
> spurious error
> before any POST_CHANGE cleanup can execute:
>
> drivers/ufs/host/ufs-qcom.c:ufs_qcom_pwr_change_notify() {
> if (!dev_req_params) {
> pr_err("%s: incoming dev_req_params is NULL\n",
> __func__);
> return -EINVAL;
> }
> ...
> }
>
> Similarly, ufs_hisi_pwr_change_notify() in drivers/ufs/host/ufs-
> hisi.c will
> also log a spurious error on every power mode change failure:
>
> drivers/ufs/host/ufs-hisi.c:ufs_hisi_pwr_change_notify() {
> if (!dev_req_params) {
> dev_err(hba->dev,
> "%s: incoming dev_req_params is NULL\n",
> __func__);
> ret = -EINVAL;
> goto out;
> }
> ...
> }
>
Yes, both the qcom and hisi host drivers ignore NULL and only print an
error log that indicates a power‑mode‑change failure.
> Additionally, does passing NULL here destroy the context needed for
> proper
> resource rollback?
>
> For instance, in ufs_qcom_pwr_change_notify(), resources are acquired
> in
> PRE_CHANGE based on the dev_req_params:
>
This patch doesn't change the behavior of the qcom driver.
Thanks
Peter
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] ufs: serialize AHIT register access between sysfs and host driver
2026-09-14 9:13 ` sashiko-bot
@ 2026-09-14 12:44 ` Peter Wang (王信友)
0 siblings, 0 replies; 12+ messages in thread
From: Peter Wang (王信友) @ 2026-09-14 12:44 UTC (permalink / raw)
To: sashiko-reviews@lists.linux.dev; +Cc: linux-scsi@vger.kernel.org
On Mon, 2026-09-14 at 09:13 +0000, sashiko-bot@kernel.org wrote:
> [Severity: High]
> This isn't a bug introduced by this patch, but can this dereference a
> NULL pointer?
>
> Sysfs attributes are exposed in ufshcd_init() via
> ufs_sysfs_add_nodes()
> before hba->ufs_device_wlun is allocated. The initialization is
> deferred
> to the ufshcd_async_scan worker.
>
> If a user writes to the sysfs node before ufshcd_add_lus() allocates
> hba->ufs_device_wlun, can auto_hibern8_store() call
> ufshcd_auto_hibern8_update() and locklessly dereference a NULL
> pointer here?
>
This patch focuses on the current issue.
The previous issue can be addressed with a separate patch,
but not with this one.
> > ufshcd_rpm_get_sync(hba);
>
> [Severity: High]
> This is a pre-existing issue, but is it safe to ignore the return
> value
> of ufshcd_rpm_get_sync() here?
>
> If the runtime PM resume fails (e.g., due to a power domain failure
> or
> device removal), pm_runtime_get_sync() returns a negative error code.
>
> Does ignoring this error allow the function to proceed and write to
> REG_AUTO_HIBERNATE_IDLE_TIMER on a potentially suspended device,
> causing a
> synchronous external abort?
>
Same, pre-existing issue should be addressed with a separate patch.
> > 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);
> > }
>
> [ ... ]
>
> [Severity: High]
> This is a pre-existing issue, but does the UFS core driver handle
> shared
> IRQs correctly?
>
Same, pre-existing issue should be addressed with a separate patch.
> The driver registers its IRQ with IRQF_SHARED. When the UFS
> controller enters
> runtime suspend, ufshcd_suspend() disables the interrupt line
> entirely:
>
> drivers/ufs/core/ufshcd.c:ufshcd_suspend() {
> ...
> ufshcd_disable_irq(hba);
> ...
> }
>
> Does calling disable_irq() mask interrupts for any other devices
> sharing the
> line? Should this use synchronize_irq() instead?
>
> Additionally, ufshcd_intr() reads the hardware register
> unconditionally:
>
> drivers/ufs/core/ufshcd.c:ufshcd_intr() {
> ...
> last_intr_status = intr_status = ufshcd_readl(hba,
> REG_INTERRUPT_STATUS);
> ...
> }
>
> Can this access powered-off hardware if the interrupt belongs to
> another
> device on the same shared line? Should this verify the device is
> actively
> powered using pm_runtime_get_if_active() before accessing registers?
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/2] ufs: fix POST_CHANGE pairing and AHIT race condition
2026-09-14 8:49 [PATCH v2 0/2] ufs: fix POST_CHANGE pairing and AHIT race condition peter.wang
2026-09-14 8:49 ` [PATCH v2 1/2] ufs: always notify POST_CHANGE even on gear switch failure peter.wang
2026-09-14 8:49 ` [PATCH v2 2/2] ufs: serialize AHIT register access between sysfs and host driver peter.wang
@ 2026-09-22 10:06 ` chun-hung.wu
2026-09-26 2:30 ` Martin K. Petersen (Oracle)
2026-10-06 2:44 ` Martin K. Petersen (Oracle)
4 siblings, 0 replies; 12+ messages in thread
From: chun-hung.wu @ 2026-09-22 10:06 UTC (permalink / raw)
To: peter.wang, linux-scsi, martin.petersen, avri.altman, alim.akhtar,
bvanassche, santosh.sy, h.vinayak
Cc: wsd_upstream, linux-mediatek, alice.chao, cc.chou, chaotian.jing,
tun-yu.yu, naomi.chu, ed.tsai
On Mon, 2026-09-14 at 16:49 +0800, peter.wang@mediatek.com wrote:
> From: Peter Wang <peter.wang@mediatek.com>
>
> Patch 1 fixes a bug where POST_CHANGE was skipped on a failed gear
> switch, leaving the host in an inconsistent state if resources
> acquired
> in PRE_CHANGE were never released.
>
> Patch 2 fixes a race between the sysfs AHIT update path and the
> MediaTek host driver's power mode change path. A mutex and depth
> counter are introduced to serialize AHIT register access and ensure
> AHIT is correctly restored after the last disabler completes.
>
> The two patches are related: patch 1 guarantees POST_CHANGE is always
> called, ensuring the depth counter introduced in patch 2 is always
> properly decremented.
>
> Peter Wang (2):
> ufs: always notify POST_CHANGE even on gear switch failure
> ufs: serialize AHIT register access between sysfs and host driver
>
> drivers/ufs/core/ufshcd.c | 22 +++++++++++++++++++---
> drivers/ufs/host/ufs-exynos.c | 4 +++-
> drivers/ufs/host/ufs-mediatek.c | 16 ++++++++++++----
> drivers/ufs/host/ufshcd-pci.c | 3 ++-
> include/ufs/ufshcd.h | 8 ++++++++
> 5 files changed, 44 insertions(+), 9 deletions(-)
>
Reviewed-by: Chun-Hung Wu <chun-hung.wu@mediatek.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/2] ufs: fix POST_CHANGE pairing and AHIT race condition
2026-09-14 8:49 [PATCH v2 0/2] ufs: fix POST_CHANGE pairing and AHIT race condition peter.wang
` (2 preceding siblings ...)
2026-09-22 10:06 ` [PATCH v2 0/2] ufs: fix POST_CHANGE pairing and AHIT race condition chun-hung.wu
@ 2026-09-26 2:30 ` Martin K. Petersen (Oracle)
2026-10-05 15:45 ` Sandeep Dhavale
2026-10-06 2:44 ` Martin K. Petersen (Oracle)
4 siblings, 1 reply; 12+ messages in thread
From: Martin K. Petersen (Oracle) @ 2026-09-26 2:30 UTC (permalink / raw)
To: peter.wang
Cc: linux-scsi, martin.petersen, avri.altman, alim.akhtar, bvanassche,
santosh.sy, h.vinayak, wsd_upstream, linux-mediatek, chun-hung.wu,
alice.chao, cc.chou, chaotian.jing, tun-yu.yu, naomi.chu, ed.tsai
> Patch 1 fixes a bug where POST_CHANGE was skipped on a failed gear
> switch, leaving the host in an inconsistent state if resources acquired
> in PRE_CHANGE were never released.
>
> Patch 2 fixes a race between the sysfs AHIT update path and the
> MediaTek host driver's power mode change path. A mutex and depth
> counter are introduced to serialize AHIT register access and ensure
> AHIT is correctly restored after the last disabler completes.
Applied to 7.4/scsi-staging, thanks!
--
Martin K. Petersen
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/2] ufs: fix POST_CHANGE pairing and AHIT race condition
2026-09-26 2:30 ` Martin K. Petersen (Oracle)
@ 2026-10-05 15:45 ` Sandeep Dhavale
2026-10-06 2:42 ` Martin K. Petersen
0 siblings, 1 reply; 12+ messages in thread
From: Sandeep Dhavale @ 2026-10-05 15:45 UTC (permalink / raw)
To: Martin K. Petersen (Oracle)
Cc: peter.wang, linux-scsi, martin.petersen, avri.altman, alim.akhtar,
bvanassche, santosh.sy, h.vinayak, wsd_upstream, linux-mediatek,
chun-hung.wu, alice.chao, cc.chou, chaotian.jing, tun-yu.yu,
naomi.chu, ed.tsai
>
> Applied to 7.4/scsi-staging, thanks!
>
> --
> Martin K. Petersen
Hi Martin,
I don't see this series in in 7.4/scsi-staging at [1], or perhaps just
push is missing?
Thanks,
Sandeep.
[1]: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git/log/?h=7.4/scsi-staging
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/2] ufs: fix POST_CHANGE pairing and AHIT race condition
2026-10-05 15:45 ` Sandeep Dhavale
@ 2026-10-06 2:42 ` Martin K. Petersen
0 siblings, 0 replies; 12+ messages in thread
From: Martin K. Petersen @ 2026-10-06 2:42 UTC (permalink / raw)
To: Sandeep Dhavale
Cc: Martin K. Petersen (Oracle), peter.wang, linux-scsi,
martin.petersen, avri.altman, alim.akhtar, bvanassche, santosh.sy,
h.vinayak, wsd_upstream, linux-mediatek, chun-hung.wu, alice.chao,
cc.chou, chaotian.jing, tun-yu.yu, naomi.chu, ed.tsai
Sandeep,
> I don't see this series in in 7.4/scsi-staging at [1], or perhaps just
> push is missing?
Just pushed.
--
Martin K. Petersen
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/2] ufs: fix POST_CHANGE pairing and AHIT race condition
2026-09-14 8:49 [PATCH v2 0/2] ufs: fix POST_CHANGE pairing and AHIT race condition peter.wang
` (3 preceding siblings ...)
2026-09-26 2:30 ` Martin K. Petersen (Oracle)
@ 2026-10-06 2:44 ` Martin K. Petersen (Oracle)
4 siblings, 0 replies; 12+ messages in thread
From: Martin K. Petersen (Oracle) @ 2026-10-06 2:44 UTC (permalink / raw)
To: linux-scsi, avri.altman, alim.akhtar, bvanassche, santosh.sy,
h.vinayak, Martin K. Petersen, peter.wang
Cc: wsd_upstream, linux-mediatek, chun-hung.wu, alice.chao, cc.chou,
chaotian.jing, tun-yu.yu, naomi.chu, ed.tsai
On Mon, 14 Sep 2026 16:49:23 +0800, peter.wang@mediatek.com wrote:
> Patch 1 fixes a bug where POST_CHANGE was skipped on a failed gear
> switch, leaving the host in an inconsistent state if resources acquired
> in PRE_CHANGE were never released.
>
> Patch 2 fixes a race between the sysfs AHIT update path and the
> MediaTek host driver's power mode change path. A mutex and depth
> counter are introduced to serialize AHIT register access and ensure
> AHIT is correctly restored after the last disabler completes.
>
> [...]
Applied to 7.4/scsi-queue, thanks!
[1/2] ufs: always notify POST_CHANGE even on gear switch failure
https://git.kernel.org/mkp/scsi/c/6efee14ca0ae
[2/2] ufs: serialize AHIT register access between sysfs and host driver
https://git.kernel.org/mkp/scsi/c/7dec5cedf7b2
--
Martin K. Petersen
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2026-10-06 2:44 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-14 8:49 [PATCH v2 0/2] ufs: fix POST_CHANGE pairing and AHIT race condition peter.wang
2026-09-14 8:49 ` [PATCH v2 1/2] ufs: always notify POST_CHANGE even on gear switch failure peter.wang
2026-09-14 9:02 ` sashiko-bot
2026-09-14 12:42 ` Peter Wang (王信友)
2026-09-14 8:49 ` [PATCH v2 2/2] ufs: serialize AHIT register access between sysfs and host driver peter.wang
2026-09-14 9:13 ` sashiko-bot
2026-09-14 12:44 ` Peter Wang (王信友)
2026-09-22 10:06 ` [PATCH v2 0/2] ufs: fix POST_CHANGE pairing and AHIT race condition chun-hung.wu
2026-09-26 2:30 ` Martin K. Petersen (Oracle)
2026-10-05 15:45 ` Sandeep Dhavale
2026-10-06 2:42 ` Martin K. Petersen
2026-10-06 2:44 ` Martin K. Petersen (Oracle)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox