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

[Yiwei Lin <s921975628@gmail.com>]

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
>