Skip to contentSkip to conversation

Three minor changes related with error handling

PATCH Applied Can Guo 3 participants 13 messages Feb 24, 2021 – Mar 4, 2021

Subsystems (2)
SCSI Arm Qualcomm mailing list
lore
Share & tools
lore

Can Guo

author
#0 lore raw

v2 cover Three minor changes related with error handling

The 1st change is a complementary change for error handling.
The 2nd change disables IRQ in host reset path to avoid possible NoC issues. The 3rd change is a minor cleanup to !hba checks in suspend/resume paths

Can Guo (2):
scsi: ufs: Minor adjustments to error handling
scsi: ufs: Remove redundant checks of !hba in suspend/resume callbacks

Nitin Rawat (1):
scsi: ufs-qcom: Disable interrupt in reset path

 drivers/scsi/ufs/ufs-qcom.c | 10 ++++++++++
 drivers/scsi/ufs/ufshcd.c   | 39 ++++++++++++---------------------------
 2 files changed, 22 insertions(+), 27 deletions(-)
Signature · 1 line
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
author
#1 lore raw

PATCH v2 1/3 scsi: ufs: Minor adjustments to error handling

In error handling prepare stage, after SCSI requests are blocked, do a down/up_write(clk_scaling_lock) to clean up the queuecommand() path. Meanwhile, stop eeh_work in case it disturbs error recovery. Moreover, reset ufshcd_state at the entrance of ufshcd_probe_hba(), since it may be called multiple times during error recovery.

Signed-off-by Can Guo <[email protected]>
---
 drivers/scsi/ufs/ufshcd.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)
drivers/scsi/ufs/ufshcd.c +12 -6
index 80620c8..013eb73 100644
@@ -4987,6 +4987,7 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
 			 * UFS device needs urgent BKOPs.
 			 */
 			if (!hba->pm_op_in_progress &&
+			    !ufshcd_eh_in_progress(hba) &&
 			    ufshcd_is_exception_event(lrbp->ucd_rsp_ptr) &&
 			    schedule_work(&hba->eeh_work)) {
 				/*

@@ -5784,13 +5785,20 @@ static void ufshcd_err_handling_prepare(struct ufs_hba *hba)
 			ufshcd_suspend_clkscaling(hba);
 		ufshcd_clk_scaling_allow(hba, false);
 	}
+	ufshcd_scsi_block_requests(hba);
+	/* Drain ufshcd_queuecommand() */
+	down_write(&hba->clk_scaling_lock);
+	up_write(&hba->clk_scaling_lock);
+	cancel_work_sync(&hba->eeh_work);
 }
 
 static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
 {
+	ufshcd_scsi_unblock_requests(hba);
 	ufshcd_release(hba);
 	if (ufshcd_is_clkscaling_supported(hba))
 		ufshcd_clk_scaling_suspend(hba, false);
+	ufshcd_clear_ua_wluns(hba);
 	pm_runtime_put(hba->dev);
 }
 

@@ -5882,8 +5890,8 @@ static void ufshcd_err_handler(struct work_struct *work)
 	spin_unlock_irqrestore(hba->host->host_lock, flags);
 	ufshcd_err_handling_prepare(hba);
 	spin_lock_irqsave(hba->host->host_lock, flags);
-	ufshcd_scsi_block_requests(hba);
-	hba->ufshcd_state = UFSHCD_STATE_RESET;
+	if (hba->ufshcd_state != UFSHCD_STATE_ERROR)
+		hba->ufshcd_state = UFSHCD_STATE_RESET;
 
 	/* Complete requests that have door-bell cleared by h/w */
 	ufshcd_complete_requests(hba);

@@ -6042,12 +6050,8 @@ static void ufshcd_err_handler(struct work_struct *work)
 	}
 	ufshcd_clear_eh_in_progress(hba);
 	spin_unlock_irqrestore(hba->host->host_lock, flags);
-	ufshcd_scsi_unblock_requests(hba);
 	ufshcd_err_handling_unprepare(hba);
 	up(&hba->host_sem);
-
-	if (!err && needs_reset)
-		ufshcd_clear_ua_wluns(hba);
 }
 
 /**

@@ -7858,6 +7862,8 @@ static int ufshcd_probe_hba(struct ufs_hba *hba, bool async)
 	unsigned long flags;
 	ktime_t start = ktime_get();
 
+	hba->ufshcd_state = UFSHCD_STATE_RESET;
+
 	ret = ufshcd_link_startup(hba);
 	if (ret)
 		goto out;
Signature · 1 line
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
#2 lore raw

RE: [PATCH v2 1/3] scsi: ufs: Minor adjustments to error handling

Quoted 9 lines

In error handling prepare stage, after SCSI requests are blocked, do a down/up_write(clk_scaling_lock) to clean up the queuecommand() path. Meanwhile, stop eeh_work in case it disturbs error recovery. Moreover, reset ufshcd_state at the entrance of ufshcd_probe_hba(), since it may be called multiple times during error recovery.

Signed-off-by Can Guo <[email protected]>

I noticed that you tagged Adrian's patch -
https://lore.kernel.org/lkml/[email protected]/
So this patch needs to be adjusted accordingly?

Thanks,
Avri

Quoted 78 lines
---
 drivers/scsi/ufs/ufshcd.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)
drivers/scsi/ufs/ufshcd.c +1 -0
index 80620c8..013eb73 100644
@@ -4987,6 +4987,7 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba,
 struct ufshcd_lrb *lrbp)
                         * UFS device needs urgent BKOPs.
                         */
                        if (!hba->pm_op_in_progress &&
+                           !ufshcd_eh_in_progress(hba) &&
                            ufshcd_is_exception_event(lrbp->ucd_rsp_ptr) &&
                            schedule_work(&hba->eeh_work)) {

                                /*
drivers/scsi/ufs/ufshcd.c +9 -2
@@ -5784,13 +5785,20 @@ static void ufshcd_err_handling_prepare(struct
 ufs_hba *hba)
                        ufshcd_suspend_clkscaling(hba);
                ufshcd_clk_scaling_allow(hba, false);
        }
+       ufshcd_scsi_block_requests(hba);
+       /* Drain ufshcd_queuecommand() */
+       down_write(&hba->clk_scaling_lock);
+       up_write(&hba->clk_scaling_lock);
+       cancel_work_sync(&hba->eeh_work);
 }
 
 static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
 {
+       ufshcd_scsi_unblock_requests(hba);
        ufshcd_release(hba);
        if (ufshcd_is_clkscaling_supported(hba))
                ufshcd_clk_scaling_suspend(hba, false);
+       ufshcd_clear_ua_wluns(hba);
        pm_runtime_put(hba->dev);
 }

@@ -5882,8 +5890,8 @@ static void ufshcd_err_handler(struct work_struct
 *work)
        spin_unlock_irqrestore(hba->host->host_lock, flags);
        ufshcd_err_handling_prepare(hba);
        spin_lock_irqsave(hba->host->host_lock, flags);
-       ufshcd_scsi_block_requests(hba);
-       hba->ufshcd_state = UFSHCD_STATE_RESET;
+       if (hba->ufshcd_state != UFSHCD_STATE_ERROR)
+               hba->ufshcd_state = UFSHCD_STATE_RESET;
 
        /* Complete requests that have door-bell cleared by h/w */

        ufshcd_complete_requests(hba);
drivers/scsi/ufs/ufshcd.c +0 -4
@@ -6042,12 +6050,8 @@ static void ufshcd_err_handler(struct work_struct
 *work)
        }
        ufshcd_clear_eh_in_progress(hba);
        spin_unlock_irqrestore(hba->host->host_lock, flags);
-       ufshcd_scsi_unblock_requests(hba);
        ufshcd_err_handling_unprepare(hba);
        up(&hba->host_sem);
-
-       if (!err && needs_reset)
-               ufshcd_clear_ua_wluns(hba);
 }

/**

drivers/scsi/ufs/ufshcd.c +2 -0
@@ -7858,6 +7862,8 @@ static int ufshcd_probe_hba(struct ufs_hba *hba,
 bool async)
        unsigned long flags;
        ktime_t start = ktime_get();
 
+       hba->ufshcd_state = UFSHCD_STATE_RESET;
+
        ret = ufshcd_link_startup(hba);
        if (ret)

                goto out;

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

author
#3 lore raw

Re: [PATCH v2 1/3] scsi: ufs: Minor adjustments to error handling

Hi Avri,

Quoted 8 lines · Avri Altman
Quoted 8 lines

In error handling prepare stage, after SCSI requests are blocked, do a down/up_write(clk_scaling_lock) to clean up the queuecommand() path. Meanwhile, stop eeh_work in case it disturbs error recovery. Moreover, reset ufshcd_state at the entrance of ufshcd_probe_hba(), since it may be called multiple times during error recovery.

Quoted 4 lines
Quoted 1 line
Signed-off-by Can Guo <[email protected]>

I noticed that you tagged Adrian's patch -
https://lore.kernel.org/lkml/[email protected]/
So this patch needs to be adjusted accordingly?

Thanks for pointing me to that one, I will rebase mine.

Regards,
Can Guo.

Quoted 85 lines

Thanks,
Avri

Quoted 81 lines
---
 drivers/scsi/ufs/ufshcd.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)
drivers/scsi/ufs/ufshcd.c +1 -0
index 80620c8..013eb73 100644
@@ -4987,6 +4987,7 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba,
 struct ufshcd_lrb *lrbp)
                         * UFS device needs urgent BKOPs.
                         */
                        if (!hba->pm_op_in_progress &&
+                           !ufshcd_eh_in_progress(hba) &&

ufshcd_is_exception_event(lrbp->ucd_rsp_ptr) &&

                            schedule_work(&hba->eeh_work)) {
                                /*
drivers/scsi/ufs/ufshcd.c +7 -0
@@ -5784,13 +5785,20 @@ static void ufshcd_err_handling_prepare(struct
 ufs_hba *hba)
                        ufshcd_suspend_clkscaling(hba);
                ufshcd_clk_scaling_allow(hba, false);
        }
+       ufshcd_scsi_block_requests(hba);
+       /* Drain ufshcd_queuecommand() */
+       down_write(&hba->clk_scaling_lock);
+       up_write(&hba->clk_scaling_lock);
+       cancel_work_sync(&hba->eeh_work);
 }
 
 static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
 {
+       ufshcd_scsi_unblock_requests(hba);
        ufshcd_release(hba);
        if (ufshcd_is_clkscaling_supported(hba))
                ufshcd_clk_scaling_suspend(hba, false);
+       ufshcd_clear_ua_wluns(hba);
        pm_runtime_put(hba->dev);
 }

@@ -5882,8 +5890,8 @@ static void ufshcd_err_handler(struct

work_struct
*work)

        spin_unlock_irqrestore(hba->host->host_lock, flags);
        ufshcd_err_handling_prepare(hba);
        spin_lock_irqsave(hba->host->host_lock, flags);
drivers/scsi/ufs/ufshcd.c +2 -2
-       ufshcd_scsi_block_requests(hba);
-       hba->ufshcd_state = UFSHCD_STATE_RESET;
+       if (hba->ufshcd_state != UFSHCD_STATE_ERROR)
+               hba->ufshcd_state = UFSHCD_STATE_RESET;
 
        /* Complete requests that have door-bell cleared by h/w */
        ufshcd_complete_requests(hba);

@@ -6042,12 +6050,8 @@ static void ufshcd_err_handler(struct

work_struct
*work)

        }
        ufshcd_clear_eh_in_progress(hba);
        spin_unlock_irqrestore(hba->host->host_lock, flags);
drivers/scsi/ufs/ufshcd.c +2 -4
-       ufshcd_scsi_unblock_requests(hba);
        ufshcd_err_handling_unprepare(hba);
        up(&hba->host_sem);
-
-       if (!err && needs_reset)
-               ufshcd_clear_ua_wluns(hba);
 }
 
 /**

@@ -7858,6 +7862,8 @@ static int ufshcd_probe_hba(struct ufs_hba *hba,
 bool async)
        unsigned long flags;
        ktime_t start = ktime_get();
 
+       hba->ufshcd_state = UFSHCD_STATE_RESET;
+
        ret = ufshcd_link_startup(hba);
        if (ret)

                goto out;

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

author
#4 lore raw

Re: [PATCH v2 1/3] scsi: ufs: Minor adjustments to error handling

Quoted 12 lines · Can Guo

Hi Avri,

Quoted 9 lines · Avri Altman
Quoted 9 lines

In error handling prepare stage, after SCSI requests are blocked, do a down/up_write(clk_scaling_lock) to clean up the queuecommand() path. Meanwhile, stop eeh_work in case it disturbs error recovery. Moreover, reset ufshcd_state at the entrance of ufshcd_probe_hba(), since it may be called multiple times during error recovery.

Signed-off-by Can Guo <[email protected]>
Quoted 9 lines
Quoted 3 lines

I noticed that you tagged Adrian's patch -
https://lore.kernel.org/lkml/[email protected]/
So this patch needs to be adjusted accordingly?

Thanks for pointing me to that one, I will rebase mine.

Regards,
Can Guo.

Just noticed that Adrian's change comes later than mine, so I may not need to adjust mine.

Thanks,
Can Guo.

Quoted 87 lines
Quoted 87 lines

Thanks,
Avri

Quoted 83 lines
---
 drivers/scsi/ufs/ufshcd.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)
drivers/scsi/ufs/ufshcd.c +1 -0
index 80620c8..013eb73 100644
@@ -4987,6 +4987,7 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba,
 struct ufshcd_lrb *lrbp)
                         * UFS device needs urgent BKOPs.
                         */
                        if (!hba->pm_op_in_progress &&
+                           !ufshcd_eh_in_progress(hba) &&

ufshcd_is_exception_event(lrbp->ucd_rsp_ptr) &&

                            schedule_work(&hba->eeh_work)) {
                                /*
drivers/scsi/ufs/ufshcd.c +0 -0
@@ -5784,13 +5785,20 @@ static void

ufshcd_err_handling_prepare(struct
ufs_hba *hba)

                        ufshcd_suspend_clkscaling(hba);
                ufshcd_clk_scaling_allow(hba, false);
        }
drivers/scsi/ufs/ufshcd.c +7 -0
+       ufshcd_scsi_block_requests(hba);
+       /* Drain ufshcd_queuecommand() */
+       down_write(&hba->clk_scaling_lock);
+       up_write(&hba->clk_scaling_lock);
+       cancel_work_sync(&hba->eeh_work);
 }
 
 static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
 {
+       ufshcd_scsi_unblock_requests(hba);
        ufshcd_release(hba);
        if (ufshcd_is_clkscaling_supported(hba))
                ufshcd_clk_scaling_suspend(hba, false);
+       ufshcd_clear_ua_wluns(hba);
        pm_runtime_put(hba->dev);
 }

@@ -5882,8 +5890,8 @@ static void ufshcd_err_handler(struct

work_struct
*work)

        spin_unlock_irqrestore(hba->host->host_lock, flags);
        ufshcd_err_handling_prepare(hba);
        spin_lock_irqsave(hba->host->host_lock, flags);
drivers/scsi/ufs/ufshcd.c +2 -2
-       ufshcd_scsi_block_requests(hba);
-       hba->ufshcd_state = UFSHCD_STATE_RESET;
+       if (hba->ufshcd_state != UFSHCD_STATE_ERROR)
+               hba->ufshcd_state = UFSHCD_STATE_RESET;
 
        /* Complete requests that have door-bell cleared by h/w */
        ufshcd_complete_requests(hba);

@@ -6042,12 +6050,8 @@ static void ufshcd_err_handler(struct

work_struct
*work)

        }
        ufshcd_clear_eh_in_progress(hba);
        spin_unlock_irqrestore(hba->host->host_lock, flags);
drivers/scsi/ufs/ufshcd.c +2 -4
-       ufshcd_scsi_unblock_requests(hba);
        ufshcd_err_handling_unprepare(hba);
        up(&hba->host_sem);
-
-       if (!err && needs_reset)
-               ufshcd_clear_ua_wluns(hba);
 }
 
 /**

@@ -7858,6 +7862,8 @@ static int ufshcd_probe_hba(struct ufs_hba 
 *hba,
 bool async)
        unsigned long flags;
        ktime_t start = ktime_get();
 
+       hba->ufshcd_state = UFSHCD_STATE_RESET;
+
        ret = ufshcd_link_startup(hba);

        if (ret)
                goto out;

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

Reviewed-by on v2 1/3
#5 lore raw

RE: [PATCH v2 1/3] scsi: ufs: Minor adjustments to error handling

In reply to Can Guo
Quoted 8 lines

In error handling prepare stage, after SCSI requests are blocked, do a down/up_write(clk_scaling_lock) to clean up the queuecommand() path. Meanwhile, stop eeh_work in case it disturbs error recovery. Moreover, reset ufshcd_state at the entrance of ufshcd_probe_hba(), since it may be called multiple times during error recovery.

Signed-off-by Can Guo <[email protected]>
Reviewed-by Avri Altman <[email protected]>
Quoted 78 lines
---
 drivers/scsi/ufs/ufshcd.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)
drivers/scsi/ufs/ufshcd.c +1 -0
index 80620c8..013eb73 100644
@@ -4987,6 +4987,7 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba,
 struct ufshcd_lrb *lrbp)
                         * UFS device needs urgent BKOPs.
                         */
                        if (!hba->pm_op_in_progress &&
+                           !ufshcd_eh_in_progress(hba) &&
                            ufshcd_is_exception_event(lrbp->ucd_rsp_ptr) &&
                            schedule_work(&hba->eeh_work)) {

                                /*
drivers/scsi/ufs/ufshcd.c +9 -2
@@ -5784,13 +5785,20 @@ static void ufshcd_err_handling_prepare(struct
 ufs_hba *hba)
                        ufshcd_suspend_clkscaling(hba);
                ufshcd_clk_scaling_allow(hba, false);
        }
+       ufshcd_scsi_block_requests(hba);
+       /* Drain ufshcd_queuecommand() */
+       down_write(&hba->clk_scaling_lock);
+       up_write(&hba->clk_scaling_lock);
+       cancel_work_sync(&hba->eeh_work);
 }
 
 static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
 {
+       ufshcd_scsi_unblock_requests(hba);
        ufshcd_release(hba);
        if (ufshcd_is_clkscaling_supported(hba))
                ufshcd_clk_scaling_suspend(hba, false);
+       ufshcd_clear_ua_wluns(hba);
        pm_runtime_put(hba->dev);
 }

@@ -5882,8 +5890,8 @@ static void ufshcd_err_handler(struct work_struct
 *work)
        spin_unlock_irqrestore(hba->host->host_lock, flags);
        ufshcd_err_handling_prepare(hba);
        spin_lock_irqsave(hba->host->host_lock, flags);
-       ufshcd_scsi_block_requests(hba);
-       hba->ufshcd_state = UFSHCD_STATE_RESET;
+       if (hba->ufshcd_state != UFSHCD_STATE_ERROR)
+               hba->ufshcd_state = UFSHCD_STATE_RESET;
 
        /* Complete requests that have door-bell cleared by h/w */

        ufshcd_complete_requests(hba);
drivers/scsi/ufs/ufshcd.c +0 -4
@@ -6042,12 +6050,8 @@ static void ufshcd_err_handler(struct work_struct
 *work)
        }
        ufshcd_clear_eh_in_progress(hba);
        spin_unlock_irqrestore(hba->host->host_lock, flags);
-       ufshcd_scsi_unblock_requests(hba);
        ufshcd_err_handling_unprepare(hba);
        up(&hba->host_sem);
-
-       if (!err && needs_reset)
-               ufshcd_clear_ua_wluns(hba);
 }

/**

drivers/scsi/ufs/ufshcd.c +2 -0
@@ -7858,6 +7862,8 @@ static int ufshcd_probe_hba(struct ufs_hba *hba,
 bool async)
        unsigned long flags;
        ktime_t start = ktime_get();
 
+       hba->ufshcd_state = UFSHCD_STATE_RESET;
+
        ret = ufshcd_link_startup(hba);
        if (ret)

                goto out;

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

author
#6 lore raw

PATCH v2 2/3 scsi: ufs-qcom: Disable interrupt in reset path

In reply to Can Guo

From: Nitin Rawat <[email protected]>

Disable interrupt in reset path to flush pending IRQ handler in order to avoid possible NoC issues.

Signed-off-by Nitin Rawat <[email protected]>
Signed-off-by Can Guo <[email protected]>
---
 drivers/scsi/ufs/ufs-qcom.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
drivers/scsi/ufs/ufs-qcom.c +10 -0
index f97d7b0..a9dc8d7 100644
@@ -253,12 +253,17 @@ static int ufs_qcom_host_reset(struct ufs_hba *hba)
 {
 	int ret = 0;
 	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
+	bool reenable_intr = false;
 
 	if (!host->core_reset) {
 		dev_warn(hba->dev, "%s: reset control not set\n", __func__);
 		goto out;
 	}
 
+	reenable_intr = hba->is_irq_enabled;
+	disable_irq(hba->irq);
+	hba->is_irq_enabled = false;
+
 	ret = reset_control_assert(host->core_reset);
 	if (ret) {
 		dev_err(hba->dev, "%s: core_reset assert failed, err = %d\n",

@@ -280,6 +285,11 @@ static int ufs_qcom_host_reset(struct ufs_hba *hba)
 
 	usleep_range(1000, 1100);
 
+	if (reenable_intr) {
+		enable_irq(hba->irq);
+		hba->is_irq_enabled = true;
+	}
+
 out:
 	return ret;
 }
Signature · 1 line
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
#7 lore raw

RE: [PATCH v2 2/3] scsi: ufs-qcom: Disable interrupt in reset path

Quoted 35 lines

From: Nitin Rawat <[email protected]>

Disable interrupt in reset path to flush pending IRQ handler in order to avoid possible NoC issues.

Signed-off-by Nitin Rawat <[email protected]>
Signed-off-by Can Guo <[email protected]>
---
 drivers/scsi/ufs/ufs-qcom.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
drivers/scsi/ufs/ufs-qcom.c +5 -0
index f97d7b0..a9dc8d7 100644
@@ -253,12 +253,17 @@ static int ufs_qcom_host_reset(struct ufs_hba *hba)
 {
        int ret = 0;
        struct ufs_qcom_host *host = ufshcd_get_variant(hba);
+       bool reenable_intr = false;
 
        if (!host->core_reset) {
                dev_warn(hba->dev, "%s: reset control not set\n", __func__);
                goto out;
        }
 
+       reenable_intr = hba->is_irq_enabled;
+       disable_irq(hba->irq);
+       hba->is_irq_enabled = false;
+
        ret = reset_control_assert(host->core_reset);
        if (ret) {
                dev_err(hba->dev, "%s: core_reset assert failed, err = %d\n",

drivers/scsi/ufs/ufs-qcom.c static int ufs_qcom_host_reset(struct ufs_hba *hba) · line 292

Quoted 8 lines
drivers/scsi/ufs/ufs-qcom.c +5 -0
@@ -280,6 +285,11 @@ static int ufs_qcom_host_reset(struct ufs_hba *hba)
 
        usleep_range(1000, 1100);
 
+       if (reenable_intr) {
+               enable_irq(hba->irq);
+               hba->is_irq_enabled = true;
+       }
+

If in the future, you will enable UFSHCI_QUIRK_BROKEN_HCE on your platform (currently only for Exynos), Will this code still work?

author
#8 lore raw

Re: [PATCH v2 2/3] scsi: ufs-qcom: Disable interrupt in reset path

Quoted 42 lines · Avri Altman
Quoted 42 lines

From: Nitin Rawat <[email protected]>

Disable interrupt in reset path to flush pending IRQ handler in order to avoid possible NoC issues.

Signed-off-by Nitin Rawat <[email protected]>
Signed-off-by Can Guo <[email protected]>
---
 drivers/scsi/ufs/ufs-qcom.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
drivers/scsi/ufs/ufs-qcom.c +5 -0
index f97d7b0..a9dc8d7 100644
@@ -253,12 +253,17 @@ static int ufs_qcom_host_reset(struct ufs_hba 
 *hba)
 {
        int ret = 0;
        struct ufs_qcom_host *host = ufshcd_get_variant(hba);
+       bool reenable_intr = false;
 
        if (!host->core_reset) {
                dev_warn(hba->dev, "%s: reset control not set\n", 
 __func__);
                goto out;
        }
 
+       reenable_intr = hba->is_irq_enabled;
+       disable_irq(hba->irq);
+       hba->is_irq_enabled = false;
+
        ret = reset_control_assert(host->core_reset);

        if (ret) {
                dev_err(hba->dev, "%s: core_reset assert failed, err =

%d\n",

drivers/scsi/ufs/ufs-qcom.c +0 -0
@@ -280,6 +285,11 @@ static int ufs_qcom_host_reset(struct ufs_hba 
 *hba)
 
        usleep_range(1000, 1100);
Quoted 8 lines

drivers/scsi/ufs/ufs-qcom.c static int ufs_qcom_host_reset(struct ufs_hba *hba) · line 292

Quoted 5 lines
drivers/scsi/ufs/ufs-qcom.c +5 -0
+       if (reenable_intr) {
+               enable_irq(hba->irq);
+               hba->is_irq_enabled = true;
+       }
+

If in the future, you will enable UFSHCI_QUIRK_BROKEN_HCE on your platform (currently only for Exynos),
Will this code still work?

Yes, it still works.

Thanks,
Can Guo.

Reviewed-by
#9 lore raw

RE: [PATCH v2 2/3] scsi: ufs-qcom: Disable interrupt in reset path

Quoted 11 lines · Avri Altman
Quoted 9 lines
Quoted 9 lines

From: Nitin Rawat <[email protected]>

Disable interrupt in reset path to flush pending IRQ handler in order to
avoid possible NoC issues.

Signed-off-by Nitin Rawat <[email protected]>
Signed-off-by Can Guo <[email protected]>
Reviewed-by Avri Altman <[email protected]>
Quoted 47 lines
Quoted 34 lines
Quoted 34 lines
---
 drivers/scsi/ufs/ufs-qcom.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
drivers/scsi/ufs/ufs-qcom.c +5 -0
index f97d7b0..a9dc8d7 100644
@@ -253,12 +253,17 @@ static int ufs_qcom_host_reset(struct ufs_hba
 *hba)
 {
        int ret = 0;
        struct ufs_qcom_host *host = ufshcd_get_variant(hba);
+       bool reenable_intr = false;
 
        if (!host->core_reset) {
                dev_warn(hba->dev, "%s: reset control not set\n",
 __func__);
                goto out;
        }
 
+       reenable_intr = hba->is_irq_enabled;
+       disable_irq(hba->irq);
+       hba->is_irq_enabled = false;
+
        ret = reset_control_assert(host->core_reset);

        if (ret) {
                dev_err(hba->dev, "%s: core_reset assert failed, err =

%d\n",

drivers/scsi/ufs/ufs-qcom.c +0 -0
@@ -280,6 +285,11 @@ static int ufs_qcom_host_reset(struct ufs_hba
 *hba)
 
        usleep_range(1000, 1100);
Quoted 8 lines

drivers/scsi/ufs/ufs-qcom.c static int ufs_qcom_host_reset(struct ufs_hba *hba) · line 292

Quoted 5 lines
drivers/scsi/ufs/ufs-qcom.c +5 -0
+       if (reenable_intr) {
+               enable_irq(hba->irq);
+               hba->is_irq_enabled = true;
+       }
+

If in the future, you will enable UFSHCI_QUIRK_BROKEN_HCE on your platform (currently only for Exynos),
Will this code still work?

Yes, it still works.

Thanks,
Can Guo.

author
#10 lore raw

PATCH v2 3/3 scsi: ufs: Remove redundant checks of !hba in suspend/resume callbacks

In reply to Can Guo

Runtime and system suspend/resume can only come after hba probe invokes platform_set_drvdata(pdev, hba), meaning hba cannot be NULL in these PM callbacks, so remove the checks of !hba.

Signed-off-by Can Guo <[email protected]>
---
 drivers/scsi/ufs/ufshcd.c | 21 ---------------------
 1 file changed, 21 deletions(-)
drivers/scsi/ufs/ufshcd.c +0 -21
index 013eb73..2517ef1 100644
@@ -95,8 +95,6 @@
 		       16, 4, buf, __len, false);                        \
 } while (0)
 
-static bool early_suspend;
-
 int ufshcd_dump_regs(struct ufs_hba *hba, size_t offset, size_t len,
 		     const char *prefix)
 {

@@ -8978,11 +8976,6 @@ int ufshcd_system_suspend(struct ufs_hba *hba)
 	int ret = 0;
 	ktime_t start = ktime_get();
 
-	if (!hba) {
-		early_suspend = true;
-		return 0;
-	}
-
 	down(&hba->host_sem);
 
 	if (!hba->is_powered)

@@ -9034,14 +9027,6 @@ int ufshcd_system_resume(struct ufs_hba *hba)
 	int ret = 0;
 	ktime_t start = ktime_get();
 
-	if (!hba)
-		return -EINVAL;
-
-	if (unlikely(early_suspend)) {
-		early_suspend = false;
-		down(&hba->host_sem);
-	}
-
 	if (!hba->is_powered || pm_runtime_suspended(hba->dev))
 		/*
 		 * Let the runtime resume take care of resuming

@@ -9074,9 +9059,6 @@ int ufshcd_runtime_suspend(struct ufs_hba *hba)
 	int ret = 0;
 	ktime_t start = ktime_get();
 
-	if (!hba)
-		return -EINVAL;
-
 	if (!hba->is_powered)
 		goto out;
 	else

@@ -9115,9 +9097,6 @@ int ufshcd_runtime_resume(struct ufs_hba *hba)
 	int ret = 0;
 	ktime_t start = ktime_get();
 
-	if (!hba)
-		return -EINVAL;
-
 	if (!hba->is_powered)
 		goto out;
 	else
Signature · 1 line
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
Reviewed-by on v2 3/3
#11 lore raw

RE: [PATCH v2 3/3] scsi: ufs: Remove redundant checks of !hba in suspend/resume callbacks

Quoted 6 lines

Runtime and system suspend/resume can only come after hba probe invokes platform_set_drvdata(pdev, hba), meaning hba cannot be NULL in these PM callbacks, so remove the checks of !hba.

Signed-off-by Can Guo <[email protected]>
Reviewed-by Avri Altman <[email protected]>
Quoted 67 lines
---
 drivers/scsi/ufs/ufshcd.c | 21 ---------------------
 1 file changed, 21 deletions(-)
drivers/scsi/ufs/ufshcd.c +0 -22
index 013eb73..2517ef1 100644
@@ -95,8 +95,6 @@
                       16, 4, buf, __len, false);                        \
 } while (0)
 
-static bool early_suspend;
-
 int ufshcd_dump_regs(struct ufs_hba *hba, size_t offset, size_t len,
                     const char *prefix)
 {

@@ -8978,11 +8976,6 @@ int ufshcd_system_suspend(struct ufs_hba *hba)
        int ret = 0;
        ktime_t start = ktime_get();
 
-       if (!hba) {
-               early_suspend = true;
-               return 0;
-       }
-
        down(&hba->host_sem);
 
        if (!hba->is_powered)

@@ -9034,14 +9027,6 @@ int ufshcd_system_resume(struct ufs_hba *hba)
        int ret = 0;
        ktime_t start = ktime_get();
 
-       if (!hba)
-               return -EINVAL;
-
-       if (unlikely(early_suspend)) {
-               early_suspend = false;
-               down(&hba->host_sem);
-       }
-
        if (!hba->is_powered || pm_runtime_suspended(hba->dev))
                /*
                 * Let the runtime resume take care of resuming

@@ -9074,9 +9059,6 @@ int ufshcd_runtime_suspend(struct ufs_hba *hba)
        int ret = 0;
        ktime_t start = ktime_get();
 
-       if (!hba)
-               return -EINVAL;
-
        if (!hba->is_powered)
                goto out;
        else

@@ -9115,9 +9097,6 @@ int ufshcd_runtime_resume(struct ufs_hba *hba)
        int ret = 0;
        ktime_t start = ktime_get();
 
-       if (!hba)
-               return -EINVAL;
-
        if (!hba->is_powered)
                goto out;
        else
--

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

Applied
#12 lore raw
In reply to Can Guo
Quoted 9 lines · Can Guo

The 1st change is a complementary change for error handling.
The 2nd change disables IRQ in host reset path to avoid possible NoC issues. The 3rd change is a minor cleanup to !hba checks in suspend/resume paths

Can Guo (2):
scsi: ufs: Minor adjustments to error handling
scsi: ufs: Remove redundant checks of !hba in suspend/resume callbacks

[...]

Applied to 5.12/scsi-fixes, thanks!

[1/3] scsi: ufs: Minor adjustments to error handling

      https://git.kernel.org/mkp/scsi/c/5e7363b98e21
[2/3] scsi: ufs-qcom: Disable interrupt in reset path
      https://git.kernel.org/mkp/scsi/c/51d31ee8de31
[3/3] scsi: ufs: Remove redundant checks of !hba in suspend/resume callbacks
      https://git.kernel.org/mkp/scsi/c/8514907e4f64
Signature · 1 line
Martin K. Petersen	Oracle Linux Engineering