[PATCH] scsi: ufs: core: Retry the full device init on fatal link loss

[PATCH] scsi: ufs: core: Retry the full device init on fatal link loss

[Shawn Lin]

On an RK3576 board the UFS device is rarely reported dead during the
initialization at boot: DEVICE_FATAL_ERROR | UIC_LINK_LOST is latched and
the following UIC query fails with -EIO, leaving the device unusable.

Recover by resetting the device and the controller and redoing the whole
initialization sequence, up to three times with a 100 ms settle delay in
between. The retry covers the link startup, the device verification, the
completion of the device initialization and the device parameters setup,
since any of them can fail once the link has been lost.

This is done in ufshcd_init() rather than in the platform driver's probe.
A host driver only reaches its private data through ufshcd_get_variant(),
which is set from the ->init callback that ufshcd_pltfrm_init() invokes
on the caller's behalf, so the resources needed to reset the device are
not available before or after that call. More importantly, retrying
ufshcd_pltfrm_init() would request the interrupt, clocks and regulators a
second time without any of the earlier registrations being rolled back:
ufshcd_pltfrm_init() returns the error as-is, it does not undo what the
previous attempt already set up. That leaves the controller with leaked
resources and makes the retry fail on the interrupt registration.

Doing the retry inside ufshcd_init() avoids both problems: the host data
is already reachable through hba->priv, and the interrupt, clocks and
regulators are acquired only once.

The failure looks like this on RK3576:

  ufshcd-rockchip 29e00000.ufs: ufshcd_check_errors: saved_err 0x880 saved_uic_err 0x0
  host_regs: 00000000: 0187011f 00000000 00000200 00000000
  host_regs: 00000010: 00000000 000005e6 00000000 00000000
  host_regs: 00000020: 00000000 00031ef5 00000000 00000000
  host_regs: 00000030: 00000008 00000001 00000000 00000000
  host_regs: 00000040: 00000000 00000000 00000000 00000000
  host_regs: 00000050: 02dd3000 00000001 00000000 00000000
  host_regs: 00000060: 00000000 00000000 00000000 00000000
  host_regs: 00000070: 02dd4000 00000001 00000000 00000000
  host_regs: 00000080: 00000000 00000000 00000000 00000000
  host_regs: 00000090: 00000016 00000000 00000000 00000000
  ufshcd-rockchip 29e00000.ufs: ufshcd_err_handler started; HBA state eh_fatal; powered 1; shutting down 0; saved_err = 2176; saved_uic_err = 0; force_reset = 0
  ufshcd-rockchip 29e00000.ufs: ufshcd_query_flag: Sending flag query for idn 1 failed, err = 5

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/ufs/core/ufshcd.c | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index ee643830a57a..e75370d1b950 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -11419,12 +11419,14 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 
 	/* Initialize hba, detect and initialize UFS device */
 	ktime_t probe_start = ktime_get();
+	int retries = 3;
 
+retry_init:
 	hba->ufshcd_state = UFSHCD_STATE_RESET;
 
 	err = ufshcd_link_startup(hba);
 	if (err)
-		goto out_release;
+		goto retry_device_init;
 
 	if (hba->mcq_enabled)
 		ufshcd_config_mcq(hba);
@@ -11441,16 +11443,16 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 	/* Verify device initialization by sending NOP OUT UPIU */
 	err = ufshcd_verify_dev_init(hba);
 	if (err)
-		goto out_release;
+		goto retry_device_init;
 
 	/* Initiate UFS initialization, and waiting until completion */
 	err = ufshcd_complete_dev_init(hba);
 	if (err)
-		goto out_release;
+		goto retry_device_init;
 
 	err = ufshcd_device_params_init(hba);
 	if (err)
-		goto out_release;
+		goto retry_device_init;
 
 	err = ufshcd_post_device_init(hba);
 
@@ -11468,6 +11470,25 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 	ufshcd_pm_qos_init(hba);
 	return 0;
 
+retry_device_init:
+	if (retries--) {
+		dev_warn(hba->dev, "UFS init failed %d, full retry (%d left)\n",
+			 err, retries);
+
+		ufshcd_device_reset(hba);
+		ufs_put_device_desc(hba);
+		ufshcd_hba_stop(hba);
+		if (ufshcd_hba_enable(hba)) {
+			dev_err(hba->dev, "Host controller enable failed\n");
+			ufshcd_print_evt_hist(hba);
+			ufshcd_print_host_state(hba);
+			goto out_release;
+		}
+
+		msleep(100);
+		goto retry_init;
+	}
+
 out_release:
 	ufshcd_release(hba);
 out_disable:
-- 
2.25.1

[PATCH] scsi: ufs: core: Retry the full device init on fatal link loss

[Shawn Lin]

On an RK3576 board the UFS device is rarely reported dead during the
initialization at boot: DEVICE_FATAL_ERROR | UIC_LINK_LOST is latched and
the following UIC query fails with -EIO, leaving the device unusable.

Recover by resetting the device and the controller and redoing the whole
initialization sequence, up to three times with a 100 ms settle delay in
between. The retry covers the link startup, the device verification, the
completion of the device initialization and the device parameters setup,
since any of them can fail once the link has been lost.

This is done in ufshcd_init() rather than in the platform driver's probe.
A host driver only reaches its private data through ufshcd_get_variant(),
which is set from the ->init callback that ufshcd_pltfrm_init() invokes
on the caller's behalf, so the resources needed to reset the device are
not available before or after that call. More importantly, retrying
ufshcd_pltfrm_init() would request the interrupt, clocks and regulators a
second time without any of the earlier registrations being rolled back:
ufshcd_pltfrm_init() returns the error as-is, it does not undo what the
previous attempt already set up. That leaves the controller with leaked
resources and makes the retry fail on the interrupt registration.

Doing the retry inside ufshcd_init() avoids both problems: the host data
is already reachable through hba->priv, and the interrupt, clocks and
regulators are acquired only once.

The failure looks like this on RK3576:

  ufshcd-rockchip 29e00000.ufs: ufshcd_check_errors: saved_err 0x880 saved_uic_err 0x0
  host_regs: 00000000: 0187011f 00000000 00000200 00000000
  host_regs: 00000010: 00000000 000005e6 00000000 00000000
  host_regs: 00000020: 00000000 00031ef5 00000000 00000000
  host_regs: 00000030: 00000008 00000001 00000000 00000000
  host_regs: 00000040: 00000000 00000000 00000000 00000000
  host_regs: 00000050: 02dd3000 00000001 00000000 00000000
  host_regs: 00000060: 00000000 00000000 00000000 00000000
  host_regs: 00000070: 02dd4000 00000001 00000000 00000000
  host_regs: 00000080: 00000000 00000000 00000000 00000000
  host_regs: 00000090: 00000016 00000000 00000000 00000000
  ufshcd-rockchip 29e00000.ufs: ufshcd_err_handler started; HBA state eh_fatal; powered 1; shutting down 0; saved_err = 2176; saved_uic_err = 0; force_reset = 0
  ufshcd-rockchip 29e00000.ufs: ufshcd_query_flag: Sending flag query for idn 1 failed, err = 5

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/ufs/core/ufshcd.c | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index ee643830a57a..e75370d1b950 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -11419,12 +11419,14 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 
 	/* Initialize hba, detect and initialize UFS device */
 	ktime_t probe_start = ktime_get();
+	int retries = 3;
 
+retry_init:
 	hba->ufshcd_state = UFSHCD_STATE_RESET;
 
 	err = ufshcd_link_startup(hba);
 	if (err)
-		goto out_release;
+		goto retry_device_init;
 
 	if (hba->mcq_enabled)
 		ufshcd_config_mcq(hba);
@@ -11441,16 +11443,16 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 	/* Verify device initialization by sending NOP OUT UPIU */
 	err = ufshcd_verify_dev_init(hba);
 	if (err)
-		goto out_release;
+		goto retry_device_init;
 
 	/* Initiate UFS initialization, and waiting until completion */
 	err = ufshcd_complete_dev_init(hba);
 	if (err)
-		goto out_release;
+		goto retry_device_init;
 
 	err = ufshcd_device_params_init(hba);
 	if (err)
-		goto out_release;
+		goto retry_device_init;
 
 	err = ufshcd_post_device_init(hba);
 
@@ -11468,6 +11470,25 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 	ufshcd_pm_qos_init(hba);
 	return 0;
 
+retry_device_init:
+	if (retries--) {
+		dev_warn(hba->dev, "UFS init failed %d, full retry (%d left)\n",
+			 err, retries);
+
+		ufshcd_device_reset(hba);
+		ufs_put_device_desc(hba);
+		ufshcd_hba_stop(hba);
+		if (ufshcd_hba_enable(hba)) {
+			dev_err(hba->dev, "Host controller enable failed\n");
+			ufshcd_print_evt_hist(hba);
+			ufshcd_print_host_state(hba);
+			goto out_release;
+		}
+
+		msleep(100);
+		goto retry_init;
+	}
+
 out_release:
 	ufshcd_release(hba);
 out_disable:
-- 
2.25.1


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH] scsi: ufs: core: Retry the full device init on fatal link loss

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] scsi: ufs: core: uncollected eh_work and sticky saved_err in ufshcd_init()
- [High] scsi: ufs: core: missing ufshcd_mcq_enable() on retry in ufshcd_init()
--

commit dd93c52494a126db3d5dec49c3f84c857b512392
Author: Shawn Lin <shawn.lin@rock-chips.com>

scsi: ufs: core: Retry the full device init on fatal link loss

This commit adds a retry loop in ufshcd_init() that resets the UFS device
and host controller up to three times with a 100 ms delay if link startup
or device initialization fails during boot.

> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index ee643830a57a2..e75370d1b9501 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -11419,12 +11419,14 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
>  
>  	/* Initialize hba, detect and initialize UFS device */
>  	ktime_t probe_start = ktime_get();
> +	int retries = 3;
>  
> +retry_init:
>  	hba->ufshcd_state = UFSHCD_STATE_RESET;
>  
>  	err = ufshcd_link_startup(hba);
>  	if (err)
> -		goto out_release;
> +		goto retry_device_init;
>  
>  	if (hba->mcq_enabled)
>  		ufshcd_config_mcq(hba);

[Severity: High]
On controllers with MCQ enabled, does retrying after ufshcd_hba_stop() and
ufshcd_hba_enable() leave the hardware in legacy single doorbell mode while
hba->mcq_enabled remains true?

Before the retry_init label in drivers/ufs/core/ufshcd.c:ufshcd_init(),
ufshcd_add_scsi_host() enables MCQ by calling ufshcd_mcq_enable():

drivers/ufs/core/ufs-mcq.c:ufshcd_mcq_enable() {
	ufshcd_rmwl(hba, MCQ_MODE_SELECT, MCQ_MODE_SELECT, REG_UFS_MEM_CFG);
	hba->mcq_enabled = true;
}

When initialization fails and enters retry_device_init, calling
ufshcd_hba_stop() and ufshcd_hba_enable() toggles REG_CONTROLLER_ENABLE,
which resets the host controller registers including MCQ_MODE_SELECT in
REG_UFS_MEM_CFG while hba->mcq_enabled stays true.

When jumping back to retry_init, ufshcd_init() calls ufshcd_config_mcq(hba)
without calling ufshcd_mcq_enable(hba), unlike ufshcd_device_init() on
subsequent resets:

drivers/ufs/core/ufshcd.c:ufshcd_device_init() {
	/* Reconfigure MCQ upon reset */
	if (hba->mcq_enabled && !init_dev_params) {
		ufshcd_config_mcq(hba);
		ufshcd_mcq_enable(hba);
	}
}

Without re-enabling MCQ_MODE_SELECT in REG_UFS_MEM_CFG, won't subsequent
device management commands (starting with NOP OUT in
ufshcd_verify_dev_init()) be posted to the MCQ submission queues while the
hardware ignores them and times out?

[ ... ]

> @@ -11468,6 +11470,25 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
>  	ufshcd_pm_qos_init(hba);
>  	return 0;
>  
> +retry_device_init:
> +	if (retries--) {
> +		dev_warn(hba->dev, "UFS init failed %d, full retry (%d left)\n",
> +			 err, retries);
> +
> +		ufshcd_device_reset(hba);
> +		ufs_put_device_desc(hba);
> +		ufshcd_hba_stop(hba);
> +		if (ufshcd_hba_enable(hba)) {
> +			dev_err(hba->dev, "Host controller enable failed\n");
> +			ufshcd_print_evt_hist(hba);
> +			ufshcd_print_host_state(hba);
> +			goto out_release;
> +		}
> +
> +		msleep(100);
> +		goto retry_init;
> +	}

[Severity: High]
What happens to the scheduled hba->eh_work and the latched error bits in
hba->saved_err and hba->saved_uic_err when retry_device_init loops back to
retry_init after a fatal error interrupt?

When DEVICE_FATAL_ERROR | UIC_LINK_LOST (0x880) fires during the failed
attempt, ufshcd_check_errors() latches the error into hba->saved_err and
hba->saved_uic_err, sets hba->ufshcd_state to
UFSHCD_STATE_EH_SCHEDULED_FATAL, and queues hba->eh_work:

drivers/ufs/core/ufshcd.c:ufshcd_check_errors() {
		hba->saved_err |= hba->errors;
		hba->saved_uic_err |= hba->uic_error;
		...
		ufshcd_schedule_eh_work(hba);
}

Because hba->ufs_device_wlun is still NULL during ufshcd_init() (until
ufshcd_async_scan() runs later), ufshcd_err_handling_should_stop(hba)
returns true when ufshcd_err_handler() executes:

drivers/ufs/core/ufshcd.c:ufshcd_err_handler() {
	down(&hba->host_sem);
	spin_lock_irqsave(&hba->host->host_lock, flags);
	if (ufshcd_err_handling_should_stop(hba)) {
		if (hba->ufshcd_state != UFSHCD_STATE_ERROR)
			hba->ufshcd_state = UFSHCD_STATE_OPERATIONAL;
		spin_unlock_irqrestore(&hba->host->host_lock, flags);
		up(&hba->host_sem);
		return;
	}
}

Since retry_device_init does not flush hba->eh_work or clear hba->saved_err,
hba->saved_uic_err, and hba->force_reset under hba->host->host_lock (as
ufshcd_reset_and_restore() does), two issues appear possible:

1. If hba->eh_work runs concurrently with the retry, ufshcd_err_handler()
can overwrite hba->ufshcd_state with UFSHCD_STATE_OPERATIONAL after
retry_init sets it to UFSHCD_STATE_RESET.

2. Even after a retry succeeds, hba->saved_err retains the sticky
DEVICE_FATAL_ERROR | UIC_LINK_LOST bits. On any later non-fatal UIC event
(such as a line reset during power mode configuration in
ufshcd_post_device_init() or during normal runtime),
ufshcd_check_errors() checks ufshcd_is_saved_err_fatal(hba), misclassifies
the event as a fatal error, and either leaves hba->ufshcd_state in
UFSHCD_STATE_EH_SCHEDULED_FATAL across ufshcd_process_probe_result() or
triggers an unnecessary full reset once hba->ufs_device_wlun is added.

Should retry_device_init flush hba->eh_work and clear hba->saved_err,
hba->saved_uic_err, and hba->force_reset under host_lock before retrying?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261028073129.1661829-1-shawn.lin@rock-chips.com?part=1

Re: [PATCH] scsi: ufs: core: Retry the full device init on fatal link loss

[Bart Van Assche]

On 10/28/26 12:31 AM, Shawn Lin wrote:
> +retry_init:
>   	hba->ufshcd_state = UFSHCD_STATE_RESET;
>   
>   	err = ufshcd_link_startup(hba);
>   	if (err)
> -		goto out_release;
> +		goto retry_device_init;

Move the code that needs to be repeated into a new function in a first
patch and add a loop around the call site of that new function in a
second patch. Don't change ufshcd_init() into a goto-spaghetti.

Bart.

Re: [PATCH] scsi: ufs: core: Retry the full device init on fatal link loss

[Bart Van Assche]

On 10/28/26 12:31 AM, Shawn Lin wrote:
> +retry_init:
>   	hba->ufshcd_state = UFSHCD_STATE_RESET;
>   
>   	err = ufshcd_link_startup(hba);
>   	if (err)
> -		goto out_release;
> +		goto retry_device_init;

Move the code that needs to be repeated into a new function in a first
patch and add a loop around the call site of that new function in a
second patch. Don't change ufshcd_init() into a goto-spaghetti.

Bart.


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH] scsi: ufs: core: Retry the full device init on fatal link loss

[Shawn Lin]

Hi Bart

在 2026/10/29 星期四 4:16, Bart Van Assche 写道:
> On 10/28/26 12:31 AM, Shawn Lin wrote:
>> +retry_init:
>>       hba->ufshcd_state = UFSHCD_STATE_RESET;
>>       err = ufshcd_link_startup(hba);
>>       if (err)
>> -        goto out_release;
>> +        goto retry_device_init;
> 
> Move the code that needs to be repeated into a new function in a first
> patch and add a loop around the call site of that new function in a
> second patch. Don't change ufshcd_init() into a goto-spaghetti.
> 

Will do, thanks.

> Bart.
> 
> 

Re: [PATCH] scsi: ufs: core: Retry the full device init on fatal link loss

[Shawn Lin]

Hi Bart

在 2026/10/29 星期四 4:16, Bart Van Assche 写道:
> On 10/28/26 12:31 AM, Shawn Lin wrote:
>> +retry_init:
>>       hba->ufshcd_state = UFSHCD_STATE_RESET;
>>       err = ufshcd_link_startup(hba);
>>       if (err)
>> -        goto out_release;
>> +        goto retry_device_init;
> 
> Move the code that needs to be repeated into a new function in a first
> patch and add a loop around the call site of that new function in a
> second patch. Don't change ufshcd_init() into a goto-spaghetti.
> 

Will do, thanks.

> Bart.
> 
> 


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip