[RFC PATCH] scsi: ufs: core: Allow error recovery before the device W-LUN exists

[RFC PATCH] scsi: ufs: core: Allow error recovery before the device W-LUN exists

[Asdlerto Korry]

ufshcd_err_handling_should_stop() returns true while hba->ufs_device_wlun
is NULL, i.e. until the first probe has added the device W-LUN. If the
link fails during that first probe (for example NOP OUT failing after
link startup), the error handler therefore never resets the host or the
link: it only marks the state operational and returns, and the probe
keeps failing until it gives up.

On a Samsung Galaxy M52 5G (SM7325) the first link startup regularly
ends in PA errors (LINERESET). With the W-LUN check removed, the error
handler performs a real host reset and restore and the device comes up;
without it, it never does.

Removing the check exposes three places that dereference
hba->ufs_device_wlun unconditionally on this path:
ufshcd_rpm_get_sync()/ufshcd_rpm_put(), ufshcd_err_handling_prepare()
and ufshcd_recover_pm_error(). Skip the W-LUN runtime PM handling there
when the W-LUN does not exist yet; there is nothing to resume and no
regular SCSI I/O before it is added.

Note: the error handler then calls ufshcd_verify_dev_init() through
ufshcd_reset_and_restore() while the tag set is quiesced. This needs the
pseudo SCSI device queue to be unquiesced in ufshcd_err_handling_prepare()
(as done upstream), otherwise blk_execute_rq() waits forever.

Signed-off-by: Asdlerto Korry <korryasdlerto@gmail.com>
---
 drivers/ufs/core/ufshcd-priv.h |  4 ++++
 drivers/ufs/core/ufshcd.c      | 13 +++++++++----
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h
index e55c2a02c1f5..2cb0d542d114 100644
--- a/drivers/ufs/core/ufshcd-priv.h
+++ b/drivers/ufs/core/ufshcd-priv.h
@@ -385,6 +385,8 @@ static inline int ufshcd_update_ee_usr_mask(struct ufs_hba *hba,
 
 static inline int ufshcd_rpm_get_sync(struct ufs_hba *hba)
 {
+	if (!hba->ufs_device_wlun)
+		return 0;
 	return pm_runtime_get_sync(&hba->ufs_device_wlun->sdev_gendev);
 }
 
@@ -410,6 +412,8 @@ static inline int ufshcd_rpm_resume(struct ufs_hba *hba)
 
 static inline void ufshcd_rpm_put(struct ufs_hba *hba)
 {
+	if (!hba->ufs_device_wlun)
+		return;
 	pm_runtime_put(&hba->ufs_device_wlun->sdev_gendev);
 }
 
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index fd52deb72d21..9767fbc7b870 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -6876,8 +6876,9 @@ static void ufshcd_err_handling_prepare(struct ufs_hba *hba)
 	 */
 	pm_runtime_get_sync(hba->dev);
 	ufshcd_rpm_get_sync(hba);
-	if (pm_runtime_status_suspended(&hba->ufs_device_wlun->sdev_gendev) ||
-	    hba->is_sys_suspended) {
+	if (hba->ufs_device_wlun &&
+	    (pm_runtime_status_suspended(&hba->ufs_device_wlun->sdev_gendev) ||
+	     hba->is_sys_suspended)) {
 		enum ufs_pm_op pm_op;
 
 		/*
@@ -6926,7 +6927,6 @@ static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
 static inline bool ufshcd_err_handling_should_stop(struct ufs_hba *hba)
 {
 	return (!hba->is_powered || hba->shutting_down ||
-		!hba->ufs_device_wlun ||
 		hba->ufshcd_state == UFSHCD_STATE_ERROR ||
 		(!(hba->saved_err || hba->saved_uic_err || hba->force_reset ||
 		   ufshcd_is_link_broken(hba))));
@@ -6935,12 +6935,17 @@ static inline bool ufshcd_err_handling_should_stop(struct ufs_hba *hba)
 #ifdef CONFIG_PM
 static void ufshcd_recover_pm_error(struct ufs_hba *hba)
 {
-	struct scsi_target *starget = hba->ufs_device_wlun->sdev_target;
+	struct scsi_target *starget;
 	struct Scsi_Host *shost = hba->host;
 	struct scsi_device *sdev;
 	struct request_queue *q;
 	bool resume_sdev_queues = false;
 
+	/* Nothing to recover before the device W-LUN has been added */
+	if (!hba->ufs_device_wlun)
+		return;
+	starget = hba->ufs_device_wlun->sdev_target;
+
 	hba->is_sys_suspended = false;
 
 	/*
-- 
2.25.1

Re: [RFC PATCH] scsi: ufs: core: Allow error recovery before the device W-LUN exists

[Peter Wang]

On Mon, 2026-11-02 at 02:36 +0200, Asdlerto Korry wrote:
> ufshcd_err_handling_should_stop() returns true while hba-
> >ufs_device_wlun
> is NULL, i.e. until the first probe has added the device W-LUN. If
> the
> link fails during that first probe (for example NOP OUT failing after
> link startup), the error handler therefore never resets the host or
> the
> link: it only marks the state operational and returns, and the probe
> keeps failing until it gives up.
> 
> On a Samsung Galaxy M52 5G (SM7325) the first link startup regularly
> ends in PA errors (LINERESET). With the W-LUN check removed, the
> error
> handler performs a real host reset and restore and the device comes
> up;
> without it, it never does.

Hi Asdlerto,

I’m a bit curious why the first link startup fails, while 
the link startup after a reset and restore doesn’t. 
Shouldn’t these two flows be identical? 
I think the driver is designed on the assumption that the 
basic init must succeed, and that only a re-init can make 
recovery possible.

Thanks.
Peter

Re: [RFC PATCH] scsi: ufs: core: Allow error recovery before the device W-LUN exists

[Asdlerto Korry]

> I'm a bit curious why the first link startup fails, while
> the link startup after a reset and restore doesn't.
> Shouldn't these two flows be identical?
> I think the driver is designed on the assumption that the
> basic init must succeed, and that only a re-init can make
> recovery possible.

Hi Peter,

Thanks for the question, you are right. I went back to my notes:
the first init failure was caused by the board, not by the core.

The vendor device tree of this phone has the UFS VCC and VCCQ
supplies (PM7325 LDO7 and LDO9) without regulator-allow-set-load,
so regulator_set_load() from the UFS driver is a no-op and both LDOs
stay in low-power mode. Under the load of the first transfers the
device does not answer the NOP OUT, the host sees
TCx_REPLAY_TIMER_EXPIRED, and the PA error / LINERESET storm I
described only starts after that. With regulator-allow-set-load and
an HPM initial mode on those regulators, the first init succeeds
and the error handler is never needed.

Also, my claim that the device always comes up after the reset was
too strong: in my tests the second link startup succeeded, but
ufshcd_device_init() did not always complete.

So please drop this RFC. The races Sashiko pointed out are another
reason not to remove that check. Sorry for the noise, and thanks
for the review.

Asdlerto