Re: [PATCH v5 7/7] ufs: core: Add error handling for MCQ mode

From: Bao D. Nguyen
Date: Tue May 23 2023 - 02:59:41 EST


On 5/19/2023 4:03 PM, Bart Van Assche wrote:
On 5/11/23 23:28, Bao D. Nguyen wrote:
@@ -6378,18 +6407,36 @@ static bool ufshcd_abort_all(struct ufs_hba *hba)
      bool needs_reset = false;
      int tag, ret;
-    /* Clear pending transfer requests */
-    for_each_set_bit(tag, &hba->outstanding_reqs, hba->nutrs) {
-        ret = ufshcd_try_to_abort_task(hba, tag);
-        dev_err(hba->dev, "Aborting tag %d / CDB %#02x %s\n", tag,
-            hba->lrb[tag].cmd ? hba->lrb[tag].cmd->cmnd[0] : -1,
-            ret ? "failed" : "succeeded");
-        if (ret) {
-            needs_reset = true;
-            goto out;
+    if (is_mcq_enabled(hba)) {
+        struct ufshcd_lrb *lrbp;
+        int tag;
+
+        for (tag = 0; tag < hba->nutrs; tag++) {
+            lrbp = &hba->lrb[tag];
+            if (!ufshcd_cmd_inflight(lrbp->cmd))
+                continue;
+            ret = ufshcd_try_to_abort_task(hba, tag);
+            dev_err(hba->dev, "Aborting tag %d / CDB %#02x %s\n", tag,
+                hba->lrb[tag].cmd ? hba->lrb[tag].cmd->cmnd[0] : -1,
+                ret ? "failed" : "succeeded");
+            if (ret) {
+                needs_reset = true;
+                goto out;
+            }
+        }
+    } else {
+        /* Clear pending transfer requests */
+        for_each_set_bit(tag, &hba->outstanding_reqs, hba->nutrs) {
+            ret = ufshcd_try_to_abort_task(hba, tag);
+            dev_err(hba->dev, "Aborting tag %d / CDB %#02x %s\n", tag,
+                hba->lrb[tag].cmd ? hba->lrb[tag].cmd->cmnd[0] : -1,
+                ret ? "failed" : "succeeded");
+            if (ret) {
+                needs_reset = true;
+                goto out;
+            }
          }
      }

Please move the ufshcd_cmd_inflight() check into ufshcd_try_to_abort_task()
such that the same code path can be used for MCQ and legacy mode.
Hi Bart,
Because the ufshcd_try_to_abort_task() is shared by sdb and mcq modes, I feel a bit uncomfortable using the new function ufshcd_cmd_inflight() in ufshcd_try_to_abort_task(). In this patch series, I am trying to avoid changing the sdb error handling logic as much as possible; only add error handling support for mcq mode. If you feel there is a very good benefit in making the change, I would give it a try. Otherwise, I would prefer not touching sdb error handling code that has been working well. Please let me know.

Thanks,
Bao


Thanks,

Bart.