[PATCH 1/1] scsi: ufs: core: Fix EH failure after wlun resume error

[PATCH 1/1] scsi: ufs: core: Fix EH failure after wlun resume error

[Po-Wen Kao]

From: Brian Kao <powenkao@google.com>

When a W-LUN resume fails, its parent devices in the SCSI hierarchy,
including the scsi_target, may be runtime suspended. Subsequently, the
error handler in ufshcd_recover_pm_error() fails to set the W-LUN
device back to active because the parent target is not active.
This results in the following errors:

  google-ufshcd 3c2d0000.ufs: ufshcd_err_handler started; HBA state eh_fatal; ...
  ufs_device_wlun 0:0:0:49488: START_STOP failed for power mode: 1, result 40000
  ufs_device_wlun 0:0:0:49488: ufshcd_wl_runtime_resume failed: -5
  ...
  ufs_device_wlun 0:0:0:49488: runtime PM trying to activate child device 0:0:0:49488 but parent (target0:0:0) is not active

This patch addresses this by:

1.  Ensuring the W-LUN's parent scsi_target is runtime resumed before
attempting to set the W-LUN to active within ufshcd_recover_pm_error().
2.  Explicitly checking for power.runtime_error on the HBA and W-LUN
devices before calling pm_runtime_set_active() to clear the error state.
3.  Adding pm_runtime_get_sync(hba->dev) in
ufshcd_err_handling_prepare() to ensure the HBA itself
is active during error recovery, even if a child device resume failed.

These changes ensure the device power states are managed correctly
during error recovery.

Signed-off-by: Brian Kao <powenkao@google.com>
Tested-by: Brian Kao <powenkao@google.com>
---
 drivers/ufs/core/ufshcd.c | 36 ++++++++++++++++++++++++++++--------
 1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index fc4d1b6576dc..9176d7ce25b4 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -6504,6 +6504,11 @@ static void ufshcd_clk_scaling_suspend(struct ufs_hba *hba, bool suspend)
 
 static void ufshcd_err_handling_prepare(struct ufs_hba *hba)
 {
+	/*
+	 * A WLUN resume failure could potentially lead to the HBA being
+	 * runtime suspended, so take an extra reference on hba->dev.
+	 */
+	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) {
@@ -6543,6 +6548,7 @@ static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
 	if (ufshcd_is_clkscaling_supported(hba))
 		ufshcd_clk_scaling_suspend(hba, false);
 	ufshcd_rpm_put(hba);
+	pm_runtime_put(hba->dev);
 }
 
 static inline bool ufshcd_err_handling_should_stop(struct ufs_hba *hba)
@@ -6557,28 +6563,42 @@ 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_Host *shost = hba->host;
 	struct scsi_device *sdev;
 	struct request_queue *q;
-	int ret;
+	bool resume_sdev_queues = false;
 
 	hba->is_sys_suspended = false;
+
 	/*
-	 * Set RPM status of wlun device to RPM_ACTIVE,
-	 * this also clears its runtime error.
+	 * Ensure the parent's error status is cleared before proceeding
+	 * to the child, as the parent must be active to activate the child.
 	 */
-	ret = pm_runtime_set_active(&hba->ufs_device_wlun->sdev_gendev);
+	if (hba->dev->power.runtime_error) {
+		/* hba->dev has no functional parent thus simplily set RPM_ACTIVE */
+		pm_runtime_set_active(hba->dev);
+		resume_sdev_queues = true;
+	}
+
+	if (hba->ufs_device_wlun->sdev_gendev.power.runtime_error) {
+		/*
+		 * starget, parent of wlun, might be suspended if wlun resume failed.
+		 * Make sure parent is resumed before set child (wlun) active.
+		 */
+		pm_runtime_get_sync(&starget->dev);
+		pm_runtime_set_active(&hba->ufs_device_wlun->sdev_gendev);
+		pm_runtime_put_sync(&starget->dev);
+		resume_sdev_queues = true;
+	}
 
-	/* hba device might have a runtime error otherwise */
-	if (ret)
-		ret = pm_runtime_set_active(hba->dev);
 	/*
 	 * If wlun device had runtime error, we also need to resume those
 	 * consumer scsi devices in case any of them has failed to be
 	 * resumed due to supplier runtime resume failure. This is to unblock
 	 * blk_queue_enter in case there are bios waiting inside it.
 	 */
-	if (!ret) {
+	if (resume_sdev_queues) {
 		shost_for_each_device(sdev, shost) {
 			q = sdev->request_queue;
 			if (q->dev && (q->rpm_status == RPM_SUSPENDED ||

base-commit: c53a741a7fd4b8e9d07acf1861b5e4a188c6585a
-- 
2.51.2.1041.gc1ab5b90ca-goog

Re: [PATCH 1/1] scsi: ufs: core: Fix EH failure after wlun resume error

[Peter Wang (王信友)]

On Wed, 2025-11-12 at 06:32 +0000, Po-Wen Kao wrote:
>   google-ufshcd 3c2d0000.ufs: ufshcd_err_handler started; HBA state
> eh_fatal; ...
>   ufs_device_wlun 0:0:0:49488: START_STOP failed for power mode: 1,
> result 40000
>   ufs_device_wlun 0:0:0:49488: ufshcd_wl_runtime_resume failed: -5
>   ...
>   ufs_device_wlun 0:0:0:49488: runtime PM trying to activate child
> device 0:0:0:49488 but parent (target0:0:0) is not active
> 

Hi Brian,

How is ufshcd_err_handler triggered before the parent device
resumes? I mean, what causes ufshcd_err_handler to be 
triggered, an error interrupt or something else?

Thanks
Peter

Re: [PATCH 1/1] scsi: ufs: core: Fix EH failure after wlun resume error

[Brian Kao]

[RESEND using plain text]

Hi Peter,

This log actually is triggered by fault injection when wlun is suspended
echo 0xf > /sys/kernel/debug/ufshcd/3c2d0000.ufs/saved_uic_err

In a real-world scenario, this situation can occur when an error
interrupt calls `ufshcd_check_errors()`, which schedules the error
handler before the system reaches `ufshcd_set_dev_pwr_mode()`.

Thanks,
Brian


On Wed, Nov 12, 2025 at 5:18 PM Brian Kao <powenkao@google.com> wrote:
>
> Hi Peter,
>
> This log actually is triggered by fault injection when wlun is suspended
> echo 0xf > /sys/kernel/debug/ufshcd/3c2d0000.ufs/saved_uic_err
>
> In a real-world scenario, this situation can occur when an error interrupt calls `ufshcd_check_errors()`, which schedules the error handler before the system reaches `ufshcd_set_dev_pwr_mode()`.
>
> Thanks,
> Brian
>
>
> On Wed, Nov 12, 2025 at 4:17 PM Peter Wang (王信友) <peter.wang@mediatek.com> wrote:
>>
>> On Wed, 2025-11-12 at 06:32 +0000, Po-Wen Kao wrote:
>> >   google-ufshcd 3c2d0000.ufs: ufshcd_err_handler started; HBA state
>> > eh_fatal; ...
>> >   ufs_device_wlun 0:0:0:49488: START_STOP failed for power mode: 1,
>> > result 40000
>> >   ufs_device_wlun 0:0:0:49488: ufshcd_wl_runtime_resume failed: -5
>> >   ...
>> >   ufs_device_wlun 0:0:0:49488: runtime PM trying to activate child
>> > device 0:0:0:49488 but parent (target0:0:0) is not active
>> >
>>
>> Hi Brian,
>>
>> How is ufshcd_err_handler triggered before the parent device
>> resumes? I mean, what causes ufshcd_err_handler to be
>> triggered, an error interrupt or something else?
>>
>> Thanks
>> Peter
>>
>>
>> ************* MEDIATEK Confidentiality Notice
>>  ********************
>> The information contained in this e-mail message (including any
>> attachments) may be confidential, proprietary, privileged, or otherwise
>> exempt from disclosure under applicable laws. It is intended to be
>> conveyed only to the designated recipient(s). Any use, dissemination,
>> distribution, printing, retaining or copying of this e-mail (including its
>> attachments) by unintended recipient(s) is strictly prohibited and may
>> be unlawful. If you are not an intended recipient of this e-mail, or believe
>>
>> that you have received this e-mail in error, please notify the sender
>> immediately (by replying to this e-mail), delete any and all copies of
>> this e-mail (including any attachments) from your system, and do not
>> disclose the content of this e-mail to any other person. Thank you!

Re: [PATCH 1/1] scsi: ufs: core: Fix EH failure after wlun resume error

[Peter Wang (王信友)]

On Wed, 2025-11-12 at 17:18 +0800, Brian Kao wrote:
> This log actually is triggered by fault injection when wlun is
> suspended
> echo 0xf > /sys/kernel/debug/ufshcd/3c2d0000.ufs/saved_uic_err
> 
> In a real-world scenario, this situation can occur when an error
> interrupt calls `ufshcd_check_errors()`, which schedules the error
> handler before the system reaches `ufshcd_set_dev_pwr_mode()`.

Hi Brian,

When ufshcd_suspend is called, it disables IRQs by calling
ufshcd_disable_irq. So, in a real-world scenario, it should
not be possible for the UFS to be suspended and the error
handler work to be triggered by an IRQ.

Fault injection using saved_uic_err could create this situation,
but if this cannot happen in reality, we should consider whether
this is a reasonable test case? If it is, then I think this patch
looks good to me. If not, maybe we should get the runtime PM state
in the fault injection and remove this redundant recovery code.
What do you think?

Thanks
Peter

Re: [PATCH 1/1] scsi: ufs: core: Fix EH failure after wlun resume error

[Brian Kao]

> Fault injection using saved_uic_err could create this situation,
> but if this cannot happen in reality, we should consider whether
> this is a reasonable test case? If it is, then I think this patch
> looks good to me. If not, maybe we should get the runtime PM state
> in the fault injection and remove this redundant recovery code.
> What do you think?

Hi Peter,

Sorry that I didn't mention that this is in the resume path.
The resume flow in bellow will re-enable IRQ and go through full
re-initialization which potentially trigger issue I described
previously
```
ufshcd_resume()
    ufshcd_enable_irq(hba);
ufshcd_wl_resume()
    ufshcd_reset_and_restore()                            <== depends
on PM level
        ufshcd_hba_enable()
        ufshcd_probe_hba()
    ufshcd_set_dev_pwr_mode(hba, UFS_ACTIVE_PWR_MODE);    <== EH is
scheduled before this point
    ....
```

Re: [PATCH 1/1] scsi: ufs: core: Fix EH failure after wlun resume error

[Peter Wang (王信友)]

On Tue, 2025-11-18 at 15:26 +0800, Brian Kao wrote
> 
> Hi Peter,
> 
> Sorry that I didn't mention that this is in the resume path.
> The resume flow in bellow will re-enable IRQ and go through full
> re-initialization which potentially trigger issue I described
> previously
> ```
> ufshcd_resume()
>     ufshcd_enable_irq(hba);
> ufshcd_wl_resume()
>     ufshcd_reset_and_restore()                            <== depends
> on PM level
>         ufshcd_hba_enable()
>         ufshcd_probe_hba()
>     ufshcd_set_dev_pwr_mode(hba, UFS_ACTIVE_PWR_MODE);    <== EH is
> scheduled before this point
>     ....
> ```

Hi Brian,

You mean that the suspend level sets the link to off, and then
ufshcd_reset_and_restore encounters an error during the resume 
flow, right? This does seem to be a potential issue.

Thanks
Peter

Re: [PATCH 1/1] scsi: ufs: core: Fix EH failure after wlun resume error

[Brian Kao]

HI Peter

Yes, that is correct. :)

Re: [PATCH 1/1] scsi: ufs: core: Fix EH failure after wlun resume error

[Peter Wang (王信友)]

On Thu, 2025-11-20 at 07:44 +0800, Brian Kao wrote:
> 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> HI Peter
> 
> Yes, that is correct. :)


Thank you for your explanation.

Reviewed-by: Peter Wang <peter.wang@mediatek.com>

Re: [PATCH 1/1] scsi: ufs: core: Fix EH failure after wlun resume error

[Bart Van Assche]

On 11/11/25 10:32 PM, Po-Wen Kao wrote:
> When a W-LUN resume fails, its parent devices in the SCSI hierarchy,
> including the scsi_target, may be runtime suspended. Subsequently, the
> error handler in ufshcd_recover_pm_error() fails to set the W-LUN
> device back to active because the parent target is not active.
> This results in the following errors:
> 
>    google-ufshcd 3c2d0000.ufs: ufshcd_err_handler started; HBA state eh_fatal; ...
>    ufs_device_wlun 0:0:0:49488: START_STOP failed for power mode: 1, result 40000
>    ufs_device_wlun 0:0:0:49488: ufshcd_wl_runtime_resume failed: -5
>    ...
>    ufs_device_wlun 0:0:0:49488: runtime PM trying to activate child device 0:0:0:49488 but parent (target0:0:0) is not active
> 
> This patch addresses this by:
> 
> 1.  Ensuring the W-LUN's parent scsi_target is runtime resumed before
> attempting to set the W-LUN to active within ufshcd_recover_pm_error().
> 2.  Explicitly checking for power.runtime_error on the HBA and W-LUN
> devices before calling pm_runtime_set_active() to clear the error state.
> 3.  Adding pm_runtime_get_sync(hba->dev) in
> ufshcd_err_handling_prepare() to ensure the HBA itself
> is active during error recovery, even if a child device resume failed.
> 
> These changes ensure the device power states are managed correctly
> during error recovery.

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

Re: [PATCH 1/1] scsi: ufs: core: Fix EH failure after wlun resume error

[Martin K. Petersen]

Po-Wen,

> When a W-LUN resume fails, its parent devices in the SCSI hierarchy,
> including the scsi_target, may be runtime suspended. Subsequently, the
> error handler in ufshcd_recover_pm_error() fails to set the W-LUN
> device back to active because the parent target is not active. This
> results in the following errors:

Applied to 6.19/scsi-staging, thanks!

-- 
Martin K. Petersen

Re: [PATCH 1/1] scsi: ufs: core: Fix EH failure after wlun resume error

[Martin K. Petersen]

On Wed, 12 Nov 2025 06:32:02 +0000, Po-Wen Kao wrote:

> When a W-LUN resume fails, its parent devices in the SCSI hierarchy,
> including the scsi_target, may be runtime suspended. Subsequently, the
> error handler in ufshcd_recover_pm_error() fails to set the W-LUN
> device back to active because the parent target is not active.
> This results in the following errors:
> 
>   google-ufshcd 3c2d0000.ufs: ufshcd_err_handler started; HBA state eh_fatal; ...
>   ufs_device_wlun 0:0:0:49488: START_STOP failed for power mode: 1, result 40000
>   ufs_device_wlun 0:0:0:49488: ufshcd_wl_runtime_resume failed: -5
>   ...
>   ufs_device_wlun 0:0:0:49488: runtime PM trying to activate child device 0:0:0:49488 but parent (target0:0:0) is not active
> 
> [...]

Applied to 6.19/scsi-queue, thanks!

[1/1] scsi: ufs: core: Fix EH failure after wlun resume error
      https://git.kernel.org/mkp/scsi/c/b4bb6daf4ac4

-- 
Martin K. Petersen