scsi: ufs: Fix device management timeouts and runtime PM errors

This series fixes two problems in UFS core entry points that issue device management commands or touch host registers.

Patch 1/5 restores the millisecond to jiffies conversion of device management command timeouts. Since the timeout moved to rq->timeout, a 1500 ms query timeout waits 6 s with CONFIG_HZ=250.

Patches 2/5 to 5/5 handle runtime PM resume errors. Several UFS core, sysfs, BSG, hwmon, and debugfs entry points call ufshcd_rpm_get_sync() before accessing host registers or issuing device management and UIC commands, but ignore its return value.

ufshcd_rpm_get_sync() wraps pm_runtime_get_sync() on the UFS device W-LUN and returns a negative errno when runtime resume of the W-LUN or host controller fails, leaving the device or controller in RPM_SUSPENDED.

Continuing after such a failure can access host registers or issue commands while the device or controller is not known to be active, while checking pm_runtime_get_sync() directly would leave power.usage_count elevated unless balanced with pm_runtime_put_noidle().

Add ufshcd_rpm_resume_and_get() around pm_runtime_resume_and_get() and convert the 20 external and helper call sites to abort and propagate the error when resuming the W-LUN fails. The three lifecycle call sites in ufshcd_rpm_dev_flush_recheck_work(), ufshcd_wl_shutdown(), and ufshcd_remove(), plus ufshcd_err_handling_prepare(), are intentionally left unchanged.

Tested on QEMU ARM64 UFS (SDB and MCQ) with W-LUN runtime resume and suspend failures via sysfs offline (no behavior change on suspend failure) and GDB power.disable_depth=1 to make ufshcd_rpm_get_sync() fail, and on SDB with a timed out query request.

Changes since v1:
- Drop the incorrect claim that a failed W-LUN runtime suspend latches power.runtime_error from the cover letter and commit messages. The PM core latches a runtime suspend error only if it is neither -EBUSY nor -EAGAIN, and in-tree UFS drivers fail W-LUN runtime suspend only with -EBUSY or -EAGAIN (Bart).
- Say in the cover letter how the series was tested.
- Describe each failing sequence in patches 2-5 step by step in time order, and drop the runtime PM disabled (-EACCES) scenario.

Stanley Jhu (5):
scsi: ufs: core: Restore jiffies conversion of device management
timeouts
scsi: ufs: core: Introduce ufshcd_rpm_resume_and_get() helper
scsi: ufs: sysfs: Propagate runtime PM resume errors
scsi: ufs: bsg: Abort ufs_bsg_request() when runtime resume fails
scsi: ufs: hwmon, debugfs: Propagate runtime PM resume errors

 drivers/ufs/core/ufs-debugfs.c |  8 +++-
 drivers/ufs/core/ufs-hwmon.c   | 12 +++++-
 drivers/ufs/core/ufs-sysfs.c   | 67 +++++++++++++++++++++++++++-------
 drivers/ufs/core/ufs_bsg.c     |  4 +-
 drivers/ufs/core/ufshcd-priv.h |  5 +++
 drivers/ufs/core/ufshcd.c      |  9 +++--
 6 files changed, 84 insertions(+), 21 deletions(-)

base-commit: 503e6172fa74da6b4944ece458f5534681e51d21

2.56.0.307.g5fe7bd680c-goog

Before commit 08b12cda6c44 ("scsi: ufs: core: Switch to scsi_get_internal_cmd()"), ufshcd_wait_for_dev_cmd() converted the millisecond timeout with msecs_to_jiffies() before waiting for the command. That commit moved the timeout to the block layer and assigns the millisecond value to rq->timeout as is, but rq->timeout is in jiffies. A 1500 ms query timeout (QUERY_REQ_TIMEOUT_DEFAULT) becomes 6 s with CONFIG_HZ=250 and 15 s with CONFIG_HZ=100, and NOP_OUT_TIMEOUT and ADVANCED_RPMB_REQ_TIMEOUT grow the same way.

Restore the conversion with msecs_to_jiffies().

---
 drivers/ufs/core/ufshcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index fd52deb72d21..68a98cde5812 100644
@@ -3390,7 +3390,7 @@ static int ufshcd_issue_dev_cmd(struct ufs_hba *hba, struct scsi_cmnd *cmd,
 	struct request *rq = scsi_cmd_to_rq(cmd);
 	blk_status_t sts;
 
-	rq->timeout = timeout;
+	rq->timeout = msecs_to_jiffies(timeout);
 	sts = blk_execute_rq(rq, true);
 	if (sts != BLK_STS_OK)
 		return blk_status_to_errno(sts);

2.56.0.313.g2479322e68-goog

ufshcd_clkscale_enable_store() and ufshcd_auto_hibern8_update() resume the UFS device W-LUN with ufshcd_rpm_get_sync() and ignore its return value.

For clkscale_enable:

1. The W-LUN is runtime suspended and clock scaling is disabled.
2. User space writes 1 to clkscale_enable.
3. ufshcd_clkscale_enable_store() calls ufshcd_rpm_get_sync().
4. The W-LUN runtime resume fails and the W-LUN stays suspended.
5. ufshcd_rpm_get_sync() returns a negative errno, which is ignored.
6. ufshcd_clkscale_enable_store() enables clock scaling and resumes devfreq while the W-LUN is still suspended.

For auto_hibern8:

1. The W-LUN starts a runtime suspend.
2. User space writes a new value to auto_hibern8.
3. ufshcd_auto_hibern8_update() sees that the W-LUN is not yet suspended and calls ufshcd_rpm_get_sync().
4. ufshcd_rpm_get_sync() waits for the runtime suspend to finish and then runs the W-LUN runtime resume.
5. The W-LUN runtime resume fails and the W-LUN stays suspended.
6. ufshcd_rpm_get_sync() returns a negative errno, which is ignored.
7. ufshcd_auto_hibern8_update() writes REG_AUTO_HIBERNATE_IDLE_TIMER while the W-LUN is still suspended.

Add ufshcd_rpm_resume_and_get() wrapping pm_runtime_resume_and_get() so failed resumes drop the usage reference automatically, and return early in ufshcd_clkscale_enable_store() and ufshcd_auto_hibern8_update() when it fails.

---
 drivers/ufs/core/ufshcd-priv.h | 5 +++++
 drivers/ufs/core/ufshcd.c      | 7 +++++--
 2 files changed, 10 insertions(+), 2 deletions(-)

index e55c2a02c1f5..99f654b79035 100644
@@ -388,6 +388,11 @@ static inline int ufshcd_rpm_get_sync(struct ufs_hba *hba)
 	return pm_runtime_get_sync(&hba->ufs_device_wlun->sdev_gendev);
 }
 
+static inline int ufshcd_rpm_resume_and_get(struct ufs_hba *hba)
+{
+	return pm_runtime_resume_and_get(&hba->ufs_device_wlun->sdev_gendev);
+}
+
 static inline int ufshcd_rpm_get_if_active(struct ufs_hba *hba)
 {
 	return pm_runtime_get_if_active(&hba->ufs_device_wlun->sdev_gendev);

index 68a98cde5812..5972e85d279e 100644
@@ -1864,7 +1864,9 @@ static ssize_t ufshcd_clkscale_enable_store(struct device *dev,
 	if (value == hba->clk_scaling.is_enabled)
 		goto out;
 
-	ufshcd_rpm_get_sync(hba);
+	err = ufshcd_rpm_resume_and_get(hba);
+	if (err)
+		goto out;
 	ufshcd_hold(hba);
 
 	hba->clk_scaling.is_enabled = value;

@@ -4792,7 +4794,8 @@ void ufshcd_auto_hibern8_update(struct ufs_hba *hba, u32 ahit)
 
 	WRITE_ONCE(hba->ahit, ahit);
 	if (!pm_runtime_suspended(&hba->ufs_device_wlun->sdev_gendev)) {
-		ufshcd_rpm_get_sync(hba);
+		if (ufshcd_rpm_resume_and_get(hba))
+			return;
 		ufshcd_hold(hba);
 		/*
 		 * Serialize with the host driver's AHIT control. Skip the

2.56.0.313.g2479322e68-goog

All 14 ufshcd_rpm_get_sync() callers in ufs-sysfs.c resume the UFS device W-LUN and ignore the return value. For example, for a UFS descriptor read:

1. The W-LUN is runtime suspended.
2. User space reads a UFS descriptor attribute in sysfs.
3. ufs_sysfs_read_desc_param() calls ufshcd_rpm_get_sync().
4. The W-LUN runtime resume fails and the W-LUN stays suspended.
5. ufshcd_rpm_get_sync() returns a negative errno, which is ignored.
6. ufs_sysfs_read_desc_param() sends a query UPIU while the W-LUN is still suspended.

The other sysfs callers follow the same sequence. They send other query UPIUs, or read a host register in ufshcd_read_hci_reg().

Switch all 14 sysfs callers to ufshcd_rpm_resume_and_get() and return the error after releasing any held buffer and hba->host_sem.

---
 drivers/ufs/core/ufs-sysfs.c | 67 ++++++++++++++++++++++++++++--------
 1 file changed, 53 insertions(+), 14 deletions(-)

index cf49ad0258e1..19b62cf52410 100644
@@ -250,13 +250,19 @@ EXPORT_SYMBOL_GPL(ufshcd_us_to_ahit);
 
 static int ufshcd_read_hci_reg(struct ufs_hba *hba, u32 *val, unsigned int reg)
 {
+	int ret;
+
 	down(&hba->host_sem);
 	if (!ufshcd_is_user_access_allowed(hba)) {
 		up(&hba->host_sem);
 		return -EBUSY;
 	}
 
-	ufshcd_rpm_get_sync(hba);
+	ret = ufshcd_rpm_resume_and_get(hba);
+	if (ret) {
+		up(&hba->host_sem);
+		return ret;
+	}
 	ufshcd_hold(hba);
 	*val = ufshcd_readl(hba, reg);
 	ufshcd_release(hba);

@@ -350,7 +356,9 @@ static ssize_t wb_on_store(struct device *dev, struct device_attribute *attr,
 		goto out;
 	}
 
-	ufshcd_rpm_get_sync(hba);
+	res = ufshcd_rpm_resume_and_get(hba);
+	if (res)
+		goto out;
 	res = ufshcd_wb_toggle(hba, wb_enable);
 	ufshcd_rpm_put_sync(hba);
 out:

@@ -421,7 +429,9 @@ static ssize_t enable_wb_buf_flush_store(struct device *dev,
 		goto out;
 	}
 
-	ufshcd_rpm_get_sync(hba);
+	res = ufshcd_rpm_resume_and_get(hba);
+	if (res)
+		goto out;
 	res = ufshcd_wb_toggle_buf_flush(hba, enable_wb_buf_flush);
 	ufshcd_rpm_put_sync(hba);
 

@@ -490,7 +500,9 @@ static ssize_t wb_resize_enable_store(struct device *dev,
 		goto out;
 	}
 
-	ufshcd_rpm_get_sync(hba);
+	res = ufshcd_rpm_resume_and_get(hba);
+	if (res)
+		goto out;
 	res = ufshcd_wb_set_resize_en(hba, mode);
 	ufshcd_rpm_put_sync(hba);
 

@@ -598,7 +610,9 @@ static ssize_t device_lvl_exception_id_show(struct device *dev,
 	if (hba->dev_info.wspecversion < 0x410)
 		return -EOPNOTSUPP;
 
-	ufshcd_rpm_get_sync(hba);
+	err = ufshcd_rpm_resume_and_get(hba);
+	if (err)
+		return err;
 	err = ufshcd_query_attr_qword(hba, UPIU_QUERY_OPCODE_READ_ATTR,
 				      QUERY_ATTR_IDN_DEV_LVL_EXCEPTION_ID,
 				      0, 0, &exception_id);

@@ -1101,7 +1115,9 @@ static ssize_t ufs_sysfs_read_desc_param(struct ufs_hba *hba,
 		goto out;
 	}
 
-	ufshcd_rpm_get_sync(hba);
+	ret = ufshcd_rpm_resume_and_get(hba);
+	if (ret)
+		goto out;
 	ret = ufshcd_read_desc_param(hba, desc_id, desc_index,
 				param_offset, desc_buf, param_size);
 	ufshcd_rpm_put_sync(hba);

@@ -1482,7 +1498,12 @@ static ssize_t _name##_show(struct device *dev,				\
 		up(&hba->host_sem);					\
 		return -ENOMEM;						\
 	}								\
-	ufshcd_rpm_get_sync(hba);					\
+	ret = ufshcd_rpm_resume_and_get(hba);				\
+	if (ret) {							\
+		kfree(desc_buf);					\
+		up(&hba->host_sem);					\
+		return ret;						\
+	}								\
 	ret = ufshcd_query_descriptor_retry(hba,			\
 		UPIU_QUERY_OPCODE_READ_DESC, QUERY_DESC_IDN_DEVICE,	\
 		0, 0, desc_buf, &desc_len);				\

@@ -1548,7 +1569,9 @@ static ssize_t _name##_show(struct device *dev,				\
 	}								\
 	if (ufshcd_is_wb_flags(QUERY_FLAG_IDN##_uname))			\
 		index = ufshcd_wb_get_query_index(hba);			\
-	ufshcd_rpm_get_sync(hba);					\
+	ret = ufshcd_rpm_resume_and_get(hba);				\
+	if (ret)							\
+		goto out;						\
 	ret = ufshcd_query_flag(hba, UPIU_QUERY_OPCODE_READ_FLAG,	\
 		QUERY_FLAG_IDN##_uname, index, &flag);			\
 	ufshcd_rpm_put_sync(hba);					\

@@ -1608,7 +1631,9 @@ static ssize_t max_number_of_rtt_show(struct device *dev,
 		return -EBUSY;
 	}
 
-	ufshcd_rpm_get_sync(hba);
+	ret = ufshcd_rpm_resume_and_get(hba);
+	if (ret)
+		goto out;
 	ret = ufshcd_query_attr(hba, UPIU_QUERY_OPCODE_READ_ATTR,
 		QUERY_ATTR_IDN_MAX_NUM_OF_RTT, 0, 0, &rtt);
 	ufshcd_rpm_put_sync(hba);

@@ -1648,7 +1673,9 @@ static ssize_t max_number_of_rtt_store(struct device *dev,
 		goto out;
 	}
 
-	ufshcd_rpm_get_sync(hba);
+	ret = ufshcd_rpm_resume_and_get(hba);
+	if (ret)
+		goto out;
 
 	memflags = memalloc_noio_save();
 	shost_for_each_device(sdev, hba->host)

@@ -1700,7 +1727,11 @@ static int wb_read_resize_attrs(struct ufs_hba *hba,
 	}
 
 	index = ufshcd_wb_get_query_index(hba);
-	ufshcd_rpm_get_sync(hba);
+	ret = ufshcd_rpm_resume_and_get(hba);
+	if (ret) {
+		up(&hba->host_sem);
+		return ret;
+	}
 	ret = ufshcd_query_attr(hba, UPIU_QUERY_OPCODE_READ_ATTR,
 			idn, index, 0, attr_val);
 	ufshcd_rpm_put_sync(hba);

@@ -1760,7 +1791,9 @@ static ssize_t _name##_show(struct device *dev,				\
 	}								\
 	if (ufshcd_is_wb_attrs(QUERY_ATTR_IDN##_uname))			\
 		index = ufshcd_wb_get_query_index(hba);			\
-	ufshcd_rpm_get_sync(hba);					\
+	ret = ufshcd_rpm_resume_and_get(hba);				\
+	if (ret)							\
+		goto out;						\
 	if (ufshcd_is_qword_attr(QUERY_ATTR_IDN##_uname))		\
 		ret = ufshcd_query_attr_qword(hba,			\
 			UPIU_QUERY_OPCODE_READ_ATTR,			\

@@ -1848,7 +1881,11 @@ static int hid_query_attr(struct ufs_hba *hba, enum query_opcode opcode,
 		return -EBUSY;
 	}
 
-	ufshcd_rpm_get_sync(hba);
+	ret = ufshcd_rpm_resume_and_get(hba);
+	if (ret) {
+		up(&hba->host_sem);
+		return ret;
+	}
 	ret = ufshcd_query_attr(hba, opcode, idn, 0, 0, attr_val);
 	ufshcd_rpm_put_sync(hba);
 

@@ -2121,7 +2158,9 @@ static ssize_t dyn_cap_needed_attribute_show(struct device *dev,
 		goto out;
 	}
 
-	ufshcd_rpm_get_sync(hba);
+	ret = ufshcd_rpm_resume_and_get(hba);
+	if (ret)
+		goto out;
 	ret = ufshcd_query_attr(hba, UPIU_QUERY_OPCODE_READ_ATTR,
 		QUERY_ATTR_IDN_DYN_CAP_NEEDED, lun, 0, &value);
 	ufshcd_rpm_put_sync(hba);

2.56.0.313.g2479322e68-goog

ufs_bsg_request() resumes the UFS device W-LUN with ufshcd_rpm_get_sync() and ignores its return value:

1. The W-LUN is runtime suspended.
2. User space submits a BSG request.
3. ufs_bsg_request() calls ufshcd_rpm_get_sync().
4. The W-LUN runtime resume fails and the W-LUN stays suspended.
5. ufshcd_rpm_get_sync() returns a negative errno, which is ignored.
6. ufs_bsg_request() dispatches the BSG command while the W-LUN is still suspended.

Use ufshcd_rpm_resume_and_get() and jump to out_complete on error so the BSG job is completed via bsg_job_done() with the failure code without dropping an unacquired runtime PM reference.

---
 drivers/ufs/core/ufs_bsg.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

index baf6ae917d36..8df1a2c6abf6 100644
@@ -150,7 +150,9 @@ static int ufs_bsg_request(struct bsg_job *job)
 		goto out_complete;
 	}
 
-	ufshcd_rpm_get_sync(hba);
+	ret = ufshcd_rpm_resume_and_get(hba);
+	if (ret)
+		goto out_complete;
 
 	msgcode = bsg_request->msgcode;
 	switch (msgcode) {

2.56.0.313.g2479322e68-goog

ufs_hwmon_read(), ufs_hwmon_write(), and ufs_debugfs_get_user_access() resume the UFS device W-LUN with ufshcd_rpm_get_sync() and ignore its return value. For example, for a hwmon temperature read:

1. The W-LUN is runtime suspended.
2. User space reads temp1_input of the UFS hwmon device.
3. ufs_hwmon_read() calls ufshcd_rpm_get_sync().
4. The W-LUN runtime resume fails and the W-LUN stays suspended.
5. ufshcd_rpm_get_sync() returns a negative errno, which is ignored.
6. ufs_hwmon_read() sends an attribute query UPIU while the W-LUN is still suspended.

ufs_hwmon_write() and the debugfs writers that use ufs_debugfs_get_user_access() (exception_event_mask and tx_eq_ctrl) follow the same sequence.

Convert all three call sites to ufshcd_rpm_resume_and_get() and release hba->host_sem before returning the error, ahead of __acquire(ufs_debugfs) in ufs_debugfs_get_user_access().

---
 drivers/ufs/core/ufs-debugfs.c |  8 +++++++-
 drivers/ufs/core/ufs-hwmon.c   | 12 ++++++++++--
 2 files changed, 17 insertions(+), 3 deletions(-)

index 9f83628f3e2c..44a1a9692324 100644
@@ -70,12 +70,18 @@ token_context_lock(ufs_debugfs);
 static int ufs_debugfs_get_user_access(struct ufs_hba *hba)
 	__cond_acquires(0, ufs_debugfs)
 {
+	int err;
+
 	down(&hba->host_sem);
 	if (!ufshcd_is_user_access_allowed(hba)) {
 		up(&hba->host_sem);
 		return -EBUSY;
 	}
-	ufshcd_rpm_get_sync(hba);
+	err = ufshcd_rpm_resume_and_get(hba);
+	if (err) {
+		up(&hba->host_sem);
+		return err;
+	}
 	__acquire(ufs_debugfs);
 	return 0;
 }

index aa0b60a0ee8a..2b9673d92573 100644
@@ -61,7 +61,11 @@ static int ufs_hwmon_read(struct device *dev, enum hwmon_sensor_types type, u32
 		return -EBUSY;
 	}
 
-	ufshcd_rpm_get_sync(hba);
+	err = ufshcd_rpm_resume_and_get(hba);
+	if (err) {
+		up(&hba->host_sem);
+		return err;
+	}
 
 	switch (attr) {
 	case hwmon_temp_enable:

@@ -113,7 +117,11 @@ static int ufs_hwmon_write(struct device *dev, enum hwmon_sensor_types type, u32
 		return -EBUSY;
 	}
 
-	ufshcd_rpm_get_sync(hba);
+	err = ufshcd_rpm_resume_and_get(hba);
+	if (err) {
+		up(&hba->host_sem);
+		return err;
+	}
 
 	if (val == 1)
 		err = ufshcd_update_ee_usr_mask(hba, MASK_EE_URGENT_TEMP, 0);

2.56.0.313.g2479322e68-goog

For the series:

Stanley,

Applied to 7.4/scsi-fixes, thanks!

Martin K. Petersen

Applied to 7.4/scsi-fixes, thanks!

[1/5] scsi: ufs: core: Restore jiffies conversion of device management timeouts

      https://git.kernel.org/mkp/scsi/c/ebf5cab0dd22
[2/5] scsi: ufs: core: Introduce ufshcd_rpm_resume_and_get() helper
      https://git.kernel.org/mkp/scsi/c/cbab12e5b4a7
[3/5] scsi: ufs: sysfs: Propagate runtime PM resume errors
      https://git.kernel.org/mkp/scsi/c/93891d1c486e
[4/5] scsi: ufs: bsg: Abort ufs_bsg_request() when runtime resume fails
      https://git.kernel.org/mkp/scsi/c/2b8541edf7c4
[5/5] scsi: ufs: hwmon, debugfs: Propagate runtime PM resume errors
      https://git.kernel.org/mkp/scsi/c/9eb89611d959

Martin K. Petersen