[PATCH] ufs: sysfs: fix current_power_mode read without SSU

[PATCH] ufs: sysfs: fix current_power_mode read without SSU

[Yiwei Lin]

current_power_mode was generated by UFS_ATTRIBUTE(), which calls
ufshcd_rpm_get_sync() before issuing the query.

That resumes the UFS device WLUN, and ufshcd_wl_runtime_resume() sends
START STOP UNIT to bring the device back to Active. As a result
bCurrentPowerMode always read back as 0x11 (Active) no matter what
state the device was actually in. Sleep could never be observed through
sysfs.

Per the UFS spec, bCurrentPowerMode is the one attribute the device
must answer in any power mode, so there is no need to wake it.

Tested on hardware: with the device runtime-suspended, reading
current_power_mode used to return 0x11; after this change it returns
0x22 (Sleep), matching the device's real state.

Assisted-by: LLM
Signed-off-by: Yiwei Lin <s921975628@gmail.com>
---
 drivers/ufs/core/ufs-sysfs.c | 59 +++++++++++++++++++++++++++++++++++-
 1 file changed, 58 insertions(+), 1 deletion(-)

diff --git a/drivers/ufs/core/ufs-sysfs.c b/drivers/ufs/core/ufs-sysfs.c
index 63e670d1a9d9e..36cffd535ab7d 100644
--- a/drivers/ufs/core/ufs-sysfs.c
+++ b/drivers/ufs/core/ufs-sysfs.c
@@ -1786,7 +1786,64 @@ out:									\
 static DEVICE_ATTR_RO(_name)
 
 UFS_ATTRIBUTE(boot_lun_enabled, _BOOT_LU_EN);
-UFS_ATTRIBUTE(current_power_mode, _POWER_MODE);
+
+/*
+ * Per UFS spec, bCurrentPowerMode is the only attribute the device must
+ * respond to in any power mode.  Read it without waking the device to
+ * Active: restore HBA clocks via pm_runtime_get_sync(hba->dev) only
+ * (avoids triggering ufshcd_wl_runtime_resume and its START STOP UNIT),
+ * exit Hibern8 if needed so the link can carry the UPIU, then query.
+ */
+static ssize_t current_power_mode_show(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	struct ufs_hba *hba = dev_get_drvdata(dev);
+	u32 value;
+	int ret;
+
+	down(&hba->host_sem);
+	if (!ufshcd_is_user_access_allowed(hba)) {
+		up(&hba->host_sem);
+		return -EBUSY;
+	}
+
+	ret = pm_runtime_get_sync(hba->dev);
+	if (ret < 0) {
+		pr_warn("%s: pm_runtime_get_sync failed %d\n", __func__, ret);
+		pm_runtime_put_noidle(hba->dev);
+		up(&hba->host_sem);
+		return ret;
+	}
+	ret = 0;
+
+	ufshcd_hold(hba);
+
+	if (ufshcd_is_link_hibern8(hba)) {
+		ret = ufshcd_uic_hibern8_exit(hba);
+		if (!ret)
+			ufshcd_set_link_active(hba);
+		else
+			dev_err(hba->dev, "%s: hibern8 exit failed %d\n",
+				__func__, ret);
+	}
+
+	if (!ret)
+		ret = ufshcd_query_attr(hba, UPIU_QUERY_OPCODE_READ_ATTR,
+			QUERY_ATTR_IDN_POWER_MODE, 0, 0, &value);
+
+	ufshcd_release(hba);
+	pm_runtime_put(hba->dev);
+
+	if (ret) {
+		up(&hba->host_sem);
+		return -EINVAL;
+	}
+	ret = sysfs_emit(buf, "0x%08X\n", value);
+	up(&hba->host_sem);
+	return ret;
+}
+static DEVICE_ATTR_RO(current_power_mode);
+
 UFS_ATTRIBUTE(active_icc_level, _ACTIVE_ICC_LVL);
 UFS_ATTRIBUTE(ooo_data_enabled, _OOO_DATA_EN);
 UFS_ATTRIBUTE(bkops_status, _BKOPS_STATUS);
-- 
2.34.1

Re: [PATCH] ufs: sysfs: fix current_power_mode read without SSU

[Yiwei Lin]

On 2026/09/15 01:00, Yiwei Lin wrote:

> current_power_mode was generated by UFS_ATTRIBUTE(), which calls
> ufshcd_rpm_get_sync() before issuing the query.
>
> That resumes the UFS device WLUN, and ufshcd_wl_runtime_resume() sends
> START STOP UNIT to bring the device back to Active. As a result
> bCurrentPowerMode always read back as 0x11 (Active) no matter what
> state the device was actually in. Sleep could never be observed through
> sysfs.
>
> Per the UFS spec, bCurrentPowerMode is the one attribute the device
> must answer in any power mode, so there is no need to wake it.
>
> Tested on hardware: with the device runtime-suspended, reading
> current_power_mode used to return 0x11; after this change it returns
> 0x22 (Sleep), matching the device's real state.
>
> Assisted-by: LLM
> Signed-off-by: Yiwei Lin <s921975628@gmail.com>
> ---
>  drivers/ufs/core/ufs-sysfs.c | 59 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 58 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/ufs/core/ufs-sysfs.c b/drivers/ufs/core/ufs-sysfs.c
> index 63e670d1a9d9e..36cffd535ab7d 100644
> --- a/drivers/ufs/core/ufs-sysfs.c
> +++ b/drivers/ufs/core/ufs-sysfs.c
> @@ -1786,7 +1786,64 @@ out:									\
>  static DEVICE_ATTR_RO(_name)
>  
>  UFS_ATTRIBUTE(boot_lun_enabled, _BOOT_LU_EN);
> -UFS_ATTRIBUTE(current_power_mode, _POWER_MODE);
> +
> +/*
> + * Per UFS spec, bCurrentPowerMode is the only attribute the device must
> + * respond to in any power mode.  Read it without waking the device to
> + * Active: restore HBA clocks via pm_runtime_get_sync(hba->dev) only
> + * (avoids triggering ufshcd_wl_runtime_resume and its START STOP UNIT),
> + * exit Hibern8 if needed so the link can carry the UPIU, then query.
> + */
> +static ssize_t current_power_mode_show(struct device *dev,
> +	struct device_attribute *attr, char *buf)
> +{
> +	struct ufs_hba *hba = dev_get_drvdata(dev);
> +	u32 value;
> +	int ret;
> +
> +	down(&hba->host_sem);
> +	if (!ufshcd_is_user_access_allowed(hba)) {
> +		up(&hba->host_sem);
> +		return -EBUSY;
> +	}
> +
> +	ret = pm_runtime_get_sync(hba->dev);
> +	if (ret < 0) {
> +		pr_warn("%s: pm_runtime_get_sync failed %d\n", __func__, ret);
> +		pm_runtime_put_noidle(hba->dev);
> +		up(&hba->host_sem);
> +		return ret;
> +	}
> +	ret = 0;
> +
> +	ufshcd_hold(hba);
> +
> +	if (ufshcd_is_link_hibern8(hba)) {
> +		ret = ufshcd_uic_hibern8_exit(hba);
> +		if (!ret)
> +			ufshcd_set_link_active(hba);
> +		else
> +			dev_err(hba->dev, "%s: hibern8 exit failed %d\n",
> +				__func__, ret);
> +	}
> +
> +	if (!ret)
> +		ret = ufshcd_query_attr(hba, UPIU_QUERY_OPCODE_READ_ATTR,
> +			QUERY_ATTR_IDN_POWER_MODE, 0, 0, &value);
> +
> +	ufshcd_release(hba);
> +	pm_runtime_put(hba->dev);
> +
> +	if (ret) {
> +		up(&hba->host_sem);
> +		return -EINVAL;
> +	}
> +	ret = sysfs_emit(buf, "0x%08X\n", value);
> +	up(&hba->host_sem);
> +	return ret;
> +}
> +static DEVICE_ATTR_RO(current_power_mode);
> +
>  UFS_ATTRIBUTE(active_icc_level, _ACTIVE_ICC_LVL);
>  UFS_ATTRIBUTE(ooo_data_enabled, _OOO_DATA_EN);
>  UFS_ATTRIBUTE(bkops_status, _BKOPS_STATUS);

According to [Sashiki's review](https://sashiko.dev/#/patchset/20260914170032.5551-1-s9      21975628@gmail.com?part=1), I think I overlooked the condition that the link is not forcefully active, which may lead to a race condition or unexpected behavior. 

I'll provide v2 with careful review and validation.

> -- 
> 2.34.1
>