[RFC PATCH v1 0/4] ufs: core: mcq: Add ufshcd_abort() support in MCQ mode

[RFC PATCH v1 0/4] ufs: core: mcq: Add ufshcd_abort() support in MCQ mode

[Bao D. Nguyen]

This patch series enable support for ufshcd_abort() in MCQ mode.
The first patch is to prepare synchronization for ufshcd_abort() and interrupt contexts.
The second patch contains the supporting functions for ufshcd_abort().
The third and fourth patches add support for MCQ abort as discussed in the UFS
host controller spec.

Bao D. Nguyen (4):
  ufs: mcq: Use ufshcd_mcq_poll_cqe_lock() in mcq mode
  ufs: mcq: Add supporting functions for mcq abort
  ufs: mcq: Add support for clean up mcq resources
  ufs: mcq: Added ufshcd_mcq_abort()

 drivers/ufs/core/ufs-mcq.c     | 299 ++++++++++++++++++++++++++++++++++++++++-
 drivers/ufs/core/ufshcd-priv.h |  19 ++-
 drivers/ufs/core/ufshcd.c      |  57 ++++++--
 drivers/ufs/host/ufs-qcom.c    |   2 +-
 include/ufs/ufshcd.h           |   5 +-
 include/ufs/ufshci.h           |  16 +++
 6 files changed, 383 insertions(+), 15 deletions(-)

-- 
2.7.4

[RFC PATCH v1 1/4] ufs: mcq: Use ufshcd_mcq_poll_cqe_lock() in mcq mode

[Bao D. Nguyen]

In preparation for adding mcq error handler support, update the mcq
code to use the ufshcd_mcq_poll_cqe_lock() in interrupt context
instead of using ufshcd_mcq_poll_cqe_nolock(). This is to keep
synchronization between mcq interrupt and error handler contexts
because both need to access the mcq hardware in separate contexts.

Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
---
 drivers/ufs/core/ufs-mcq.c     | 6 +++---
 drivers/ufs/core/ufshcd-priv.h | 2 --
 drivers/ufs/core/ufshcd.c      | 2 +-
 drivers/ufs/host/ufs-qcom.c    | 2 +-
 include/ufs/ufshcd.h           | 2 +-
 5 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
index 31df052..63729dd 100644
--- a/drivers/ufs/core/ufs-mcq.c
+++ b/drivers/ufs/core/ufs-mcq.c
@@ -277,8 +277,8 @@ static void ufshcd_mcq_process_cqe(struct ufs_hba *hba,
 	ufshcd_compl_one_cqe(hba, tag, cqe);
 }
 
-unsigned long ufshcd_mcq_poll_cqe_nolock(struct ufs_hba *hba,
-					 struct ufs_hw_queue *hwq)
+static unsigned long ufshcd_mcq_poll_cqe_nolock(struct ufs_hba *hba,
+						struct ufs_hw_queue *hwq)
 {
 	unsigned long completed_reqs = 0;
 
@@ -294,7 +294,6 @@ unsigned long ufshcd_mcq_poll_cqe_nolock(struct ufs_hba *hba,
 
 	return completed_reqs;
 }
-EXPORT_SYMBOL_GPL(ufshcd_mcq_poll_cqe_nolock);
 
 unsigned long ufshcd_mcq_poll_cqe_lock(struct ufs_hba *hba,
 				       struct ufs_hw_queue *hwq)
@@ -307,6 +306,7 @@ unsigned long ufshcd_mcq_poll_cqe_lock(struct ufs_hba *hba,
 
 	return completed_reqs;
 }
+EXPORT_SYMBOL_GPL(ufshcd_mcq_poll_cqe_lock);
 
 void ufshcd_mcq_make_queues_operational(struct ufs_hba *hba)
 {
diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h
index 529f850..9597e955 100644
--- a/drivers/ufs/core/ufshcd-priv.h
+++ b/drivers/ufs/core/ufshcd-priv.h
@@ -71,8 +71,6 @@ void ufshcd_mcq_config_mac(struct ufs_hba *hba, u32 max_active_cmds);
 void ufshcd_mcq_select_mcq_mode(struct ufs_hba *hba);
 u32 ufshcd_mcq_read_cqis(struct ufs_hba *hba, int i);
 void ufshcd_mcq_write_cqis(struct ufs_hba *hba, u32 val, int i);
-unsigned long ufshcd_mcq_poll_cqe_nolock(struct ufs_hba *hba,
-					 struct ufs_hw_queue *hwq);
 struct ufs_hw_queue *ufshcd_mcq_req_to_hwq(struct ufs_hba *hba,
 					   struct request *req);
 unsigned long ufshcd_mcq_poll_cqe_lock(struct ufs_hba *hba,
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 629442c..e4d1b4e 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -6739,7 +6739,7 @@ static irqreturn_t ufshcd_handle_mcq_cq_events(struct ufs_hba *hba)
 			ufshcd_mcq_write_cqis(hba, events, i);
 
 		if (events & UFSHCD_MCQ_CQIS_TAIL_ENT_PUSH_STS)
-			ufshcd_mcq_poll_cqe_nolock(hba, hwq);
+			ufshcd_mcq_poll_cqe_lock(hba, hwq);
 	}
 
 	return IRQ_HANDLED;
diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index 34fc453..c686fb8 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -1558,7 +1558,7 @@ static irqreturn_t ufs_qcom_mcq_esi_handler(int irq, void *__hba)
 	struct ufs_hw_queue *hwq = &hba->uhq[id];
 
 	ufshcd_mcq_write_cqis(hba, 0x1, id);
-	ufshcd_mcq_poll_cqe_nolock(hba, hwq);
+	ufshcd_mcq_poll_cqe_lock(hba, hwq);
 
 	return IRQ_HANDLED;
 }
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index 05e4164..5d7f9b2 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -1244,7 +1244,7 @@ void ufshcd_update_evt_hist(struct ufs_hba *hba, u32 id, u32 val);
 void ufshcd_hba_stop(struct ufs_hba *hba);
 void ufshcd_schedule_eh_work(struct ufs_hba *hba);
 void ufshcd_mcq_write_cqis(struct ufs_hba *hba, u32 val, int i);
-unsigned long ufshcd_mcq_poll_cqe_nolock(struct ufs_hba *hba,
+unsigned long ufshcd_mcq_poll_cqe_lock(struct ufs_hba *hba,
 					 struct ufs_hw_queue *hwq);
 void ufshcd_mcq_enable_esi(struct ufs_hba *hba);
 void ufshcd_mcq_config_esi(struct ufs_hba *hba, struct msi_msg *msg);
-- 
2.7.4

[RFC PATCH v1 2/4] ufs: mcq: Add supporting functions for mcq abort

[Bao D. Nguyen]

Add supporting functions to handle ufs abort in mcq mode.

Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
---
 drivers/ufs/core/ufs-mcq.c     | 217 +++++++++++++++++++++++++++++++++++++++++
 drivers/ufs/core/ufshcd-priv.h |  14 +++
 drivers/ufs/core/ufshcd.c      |   4 +-
 include/ufs/ufshcd.h           |   3 +
 include/ufs/ufshci.h           |  16 +++
 5 files changed, 253 insertions(+), 1 deletion(-)

diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
index 63729dd..e27d8eb 100644
--- a/drivers/ufs/core/ufs-mcq.c
+++ b/drivers/ufs/core/ufs-mcq.c
@@ -12,6 +12,9 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include "ufshcd-priv.h"
+#include <linux/delay.h>
+#include <scsi/scsi_cmnd.h>
+#include <linux/bitfield.h>
 
 #define MAX_QUEUE_SUP GENMASK(7, 0)
 #define UFS_MCQ_MIN_RW_QUEUES 2
@@ -429,3 +432,217 @@ int ufshcd_mcq_init(struct ufs_hba *hba)
 	host->host_tagset = 1;
 	return 0;
 }
+
+static int ufshcd_mcq_poll_register(void __iomem *reg, u32 mask,
+			u32 val, unsigned long timeout_ms)
+{
+	unsigned long timeout = jiffies + msecs_to_jiffies(timeout_ms);
+	int err = 0;
+
+	/* ignore bits that we don't intend to wait on */
+	val = val & mask;
+
+	while ((readl(reg) & mask) != val) {
+		usleep_range(10, 50);
+		if (time_after(jiffies, timeout)) {
+			err = -ETIMEDOUT;
+			break;
+		}
+	}
+
+	return err;
+}
+
+static int ufshcd_mcq_sq_stop(struct ufs_hba *hba, struct ufs_hw_queue *hwq)
+{
+	void __iomem *reg;
+	u32 i = hwq->id;
+	int err;
+
+	writel(SQ_STOP, mcq_opr_base(hba, OPR_SQD, i) + REG_SQRTC);
+	reg = mcq_opr_base(hba, OPR_SQD, i) + REG_SQRTS;
+	err = ufshcd_mcq_poll_register(reg, SQ_STS, SQ_STS, hba->mcq_poll_ms);
+	if (err)
+		dev_err(hba->dev, "%s: failed. hwq-id=%d, err=%d\n",
+			__func__, i, err);
+	return err;
+}
+
+static int ufshcd_mcq_sq_start(struct ufs_hba *hba, struct ufs_hw_queue *hwq)
+{
+	void __iomem *reg;
+	u32 i = hwq->id;
+	int err;
+
+	writel(SQ_START, mcq_opr_base(hba, OPR_SQD, i) + REG_SQRTC);
+	reg = mcq_opr_base(hba, OPR_SQD, i) + REG_SQRTS;
+	err = ufshcd_mcq_poll_register(reg, SQ_STS, 0, hba->mcq_poll_ms);
+	if (err)
+		dev_err(hba->dev, "%s: failed. hwq-id=%d, err=%d\n",
+			__func__, i, err);
+	return err;
+}
+
+/**
+ * ufshcd_mcq_sq_cleanup - Clean up Submission Queue resources
+ * associated with the pending command.
+ * @hba - per adapter instance.
+ * @task_tag - The command's task tag.
+ * @result - Result of the Clean up operation.
+ *
+ * Returns 0 and result on completion. Returns error code if
+ * the operation fails.
+ */
+int ufshcd_mcq_sq_cleanup(struct ufs_hba *hba, int task_tag, int *result)
+{
+	struct ufshcd_lrb *lrbp = &hba->lrb[task_tag];
+	struct scsi_cmnd *cmd = lrbp->cmd;
+	struct ufs_hw_queue *hwq;
+	void __iomem *reg, *opr_sqd_base;
+	u32 nexus, i;
+	int err;
+
+	hwq = ufshcd_mcq_req_to_hwq(hba, scsi_cmd_to_rq(cmd));
+	i = hwq->id;
+
+	spin_lock(&hwq->sq_lock);
+
+	/* stop the SQ fetching before working on it */
+	err = ufshcd_mcq_sq_stop(hba, hwq);
+	if (err)
+		goto unlock;
+
+	/* SQCTI = EXT_IID, IID, LUN, Task Tag */
+	nexus = lrbp->lun << 8 | task_tag;
+	opr_sqd_base = mcq_opr_base(hba, OPR_SQD, i);
+	writel(nexus, opr_sqd_base + REG_SQCTI);
+
+	/* SQRTCy.ICU = 1 */
+	writel(SQ_ICU, opr_sqd_base + REG_SQRTC);
+
+	/* Poll SQRTSy.CUS = 1. Return result from SQRTSy.RTC */
+	reg = opr_sqd_base + REG_SQRTS;
+	err = ufshcd_mcq_poll_register(reg, SQ_CUS, SQ_CUS, hba->mcq_poll_ms);
+	if (err) {
+		dev_err(hba->dev, "%s: failed. hwq=%d, lun=0x%x, tag=%d\n",
+			__func__, i, lrbp->lun, task_tag);
+		*result = FIELD_GET(SQ_ICU_ERR_CODE_MASK, readl(reg));
+	}
+
+	err = ufshcd_mcq_sq_start(hba, hwq);
+
+unlock:
+	spin_unlock(&hwq->sq_lock);
+	return err;
+}
+
+static u64 ufshcd_mcq_get_cmd_desc_addr(struct ufs_hba *hba,
+					int task_tag)
+{
+	struct ufshcd_lrb *lrbp = &hba->lrb[task_tag];
+	__le32 hi = lrbp->utr_descriptor_ptr->command_desc_base_addr_hi;
+	__le32 lo = lrbp->utr_descriptor_ptr->command_desc_base_addr_lo;
+
+	return le64_to_cpu((__le64)hi << 32 | lo);
+}
+
+/**
+ * ufshcd_mcq_nullify_cmd - Nullify utrd. Host controller does not fetch
+ * transfer with Command Type = 0xF. post the Completion Queue with OCS=ABORTED.
+ * @hba - per adapter instance.
+ * @hwq - Hardware Queue of the nullified utrd.
+ */
+static void ufshcd_mcq_nullify_cmd(struct ufs_hba *hba, struct ufs_hw_queue *hwq)
+{
+	struct utp_transfer_req_desc *utrd;
+	u32 dword_0;
+
+	utrd = (struct utp_transfer_req_desc *)(hwq->sqe_base_addr +
+			hwq->id * sizeof(struct utp_transfer_req_desc));
+	dword_0 = le32_to_cpu(utrd->header.dword_0);
+	dword_0 &= ~UPIU_COMMAND_TYPE_MASK;
+	dword_0 |= FIELD_PREP(UPIU_COMMAND_TYPE_MASK, 0xF);
+	utrd->header.dword_0 = cpu_to_le32(dword_0);
+}
+
+/**
+ * ufshcd_mcq_sqe_search - Search for the cmd in the Submission Queue (SQ)
+ * @hba - per adapter instance.
+ * @hwq - Hardware Queue to be searched.
+ * @task_tag - The command's task tag.
+ *
+ * Returns true if the SQE containing the command is present in the SQ
+ * (not fetched by the controller); returns false if the SQE is not in the SQ.
+ */
+static bool ufshcd_mcq_sqe_search(struct ufs_hba *hba,
+		struct ufs_hw_queue *hwq, int task_tag)
+{
+	struct utp_transfer_req_desc *utrd;
+	u32 mask = hwq->max_entries - 1;
+	bool ret = false;
+	u64 addr, match;
+	u32 i;
+
+	spin_lock(&hwq->sq_lock);
+
+	ufshcd_mcq_update_sq_head_slot(hwq);
+	if (ufshcd_mcq_is_sq_empty(hwq))
+		goto out;
+
+	addr = ufshcd_mcq_get_cmd_desc_addr(hba, task_tag);
+	i = hwq->sq_head_slot;
+	while (i != hwq->sq_tail_slot) {
+		utrd = (struct utp_transfer_req_desc *)(hwq->sqe_base_addr +
+				i * sizeof(struct utp_transfer_req_desc));
+		match = le64_to_cpu((__le64)utrd->command_desc_base_addr_hi << 32 |
+				utrd->command_desc_base_addr_lo) & CQE_UCD_BA;
+		if (addr == match) {
+			ret = true;
+			goto out;
+		}
+		i = (i + 1) & mask;
+	}
+
+out:
+	spin_unlock(&hwq->sq_lock);
+	return ret;
+}
+
+/**
+ * ufshcd_mcq_cqe_search - Search for the cmd in the Completion Queue.
+ * @hba - per adapter instance.
+ * @hwq - the Hardware Queue to be searched.
+ * @task_tag - The command's tag.
+ *
+ * Returns true if the CQE containing the command is present in the CQ
+ * (not processed by host SW yet); returns false if the CQE is not in the CQ.
+ */
+static bool ufshcd_mcq_cqe_search(struct ufs_hba *hba,
+		struct ufs_hw_queue *hwq, int task_tag)
+{
+	struct cq_entry *cqe = hwq->cqe_base_addr;
+	u32 mask = hwq->max_entries - 1, i;
+	u64 addr, match;
+	bool ret = false;
+
+	spin_lock(&hwq->cq_lock);
+
+	if (ufshcd_mcq_is_cq_empty(hwq))
+		goto out;
+
+	addr = ufshcd_mcq_get_cmd_desc_addr(hba, task_tag);
+	i = hwq->cq_head_slot;
+	while (i != hwq->cq_tail_slot) {
+		cqe += i;
+		match = (le64_to_cpu(cqe->command_desc_base_addr) & CQE_UCD_BA);
+		if (addr == match) {
+			ret = true;
+			goto out;
+		}
+		i = (i + 1) & mask;
+	}
+
+out:
+	spin_unlock(&hwq->cq_lock);
+	return ret;
+}
diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h
index 9597e955..0527926 100644
--- a/drivers/ufs/core/ufshcd-priv.h
+++ b/drivers/ufs/core/ufshcd-priv.h
@@ -76,6 +76,8 @@ struct ufs_hw_queue *ufshcd_mcq_req_to_hwq(struct ufs_hba *hba,
 unsigned long ufshcd_mcq_poll_cqe_lock(struct ufs_hba *hba,
 				       struct ufs_hw_queue *hwq);
 
+int ufshcd_mcq_sq_cleanup(struct ufs_hba *hba, int task_tag, int *result);
+
 #define UFSHCD_MCQ_IO_QUEUE_OFFSET	1
 #define SD_ASCII_STD true
 #define SD_RAW false
@@ -401,4 +403,16 @@ static inline struct cq_entry *ufshcd_mcq_cur_cqe(struct ufs_hw_queue *q)
 
 	return cqe + q->cq_head_slot;
 }
+
+static inline void ufshcd_mcq_update_sq_head_slot(struct ufs_hw_queue *q)
+{
+	u32 val = readl(q->mcq_sq_head);
+
+	q->sq_head_slot = val / sizeof(struct utp_transfer_req_desc);
+}
+
+static inline bool ufshcd_mcq_is_sq_empty(struct ufs_hw_queue *q)
+{
+	return q->sq_head_slot == q->sq_tail_slot;
+}
 #endif /* _UFSHCD_PRIV_H_ */
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index e4d1b4e..7e52af6 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -56,7 +56,8 @@
 #define NOP_OUT_RETRIES    10
 /* Timeout after 50 msecs if NOP OUT hangs without response */
 #define NOP_OUT_TIMEOUT    50 /* msecs */
-
+/* Maximum MCQ registers polling time */
+#define MCQ_POLL_TIMEOUT   500
 /* Query request retries */
 #define QUERY_REQ_RETRIES 3
 /* Query request timeout */
@@ -10065,6 +10066,7 @@ int ufshcd_alloc_host(struct device *dev, struct ufs_hba **hba_handle)
 	hba->dev = dev;
 	hba->dev_ref_clk_freq = REF_CLK_FREQ_INVAL;
 	hba->nop_out_timeout = NOP_OUT_TIMEOUT;
+	hba->mcq_poll_ms = MCQ_POLL_TIMEOUT;
 	ufshcd_set_sg_entry_size(hba, sizeof(struct ufshcd_sg_entry));
 	INIT_LIST_HEAD(&hba->clk_list_head);
 	spin_lock_init(&hba->outstanding_lock);
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index 5d7f9b2..9b5bdf3 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -909,6 +909,7 @@ enum ufshcd_mcq_opr {
  * @mcq_base: Multi circular queue registers base address
  * @uhq: array of supported hardware queues
  * @dev_cmd_queue: Queue for issuing device management commands
+ * @mcq_poll_timeout_ms: Max polling time for MCQ registers in ms
  */
 struct ufs_hba {
 	void __iomem *mmio_base;
@@ -1072,6 +1073,7 @@ struct ufs_hba {
 	struct ufs_hw_queue *uhq;
 	struct ufs_hw_queue *dev_cmd_queue;
 	struct ufshcd_mcq_opr_info_t mcq_opr[OPR_MAX];
+	u32 mcq_poll_ms;
 };
 
 /**
@@ -1105,6 +1107,7 @@ struct ufs_hw_queue {
 	u32 max_entries;
 	u32 id;
 	u32 sq_tail_slot;
+	u32 sq_head_slot;
 	spinlock_t sq_lock;
 	u32 cq_tail_slot;
 	u32 cq_head_slot;
diff --git a/include/ufs/ufshci.h b/include/ufs/ufshci.h
index 11424bb..252e731 100644
--- a/include/ufs/ufshci.h
+++ b/include/ufs/ufshci.h
@@ -99,6 +99,9 @@ enum {
 enum {
 	REG_SQHP		= 0x0,
 	REG_SQTP		= 0x4,
+	REG_SQRTC		= 0x8,
+	REG_SQCTI		= 0xC,
+	REG_SQRTS		= 0x10,
 };
 
 enum {
@@ -111,6 +114,19 @@ enum {
 	REG_CQIE		= 0x4,
 };
 
+enum {
+	SQ_START		= 0x0,
+	SQ_STOP			= 0x1,
+	SQ_ICU			= 0x2,
+};
+
+enum {
+	SQ_STS			= 0x1,
+	SQ_CUS			= 0x2,
+};
+
+#define SQ_ICU_ERR_CODE_MASK		GENMASK(7, 4)
+#define UPIU_COMMAND_TYPE_MASK		GENMASK(31, 28)
 #define UFS_MASK(mask, offset)		((mask) << (offset))
 
 /* UFS Version 08h */
-- 
2.7.4

Re: [RFC PATCH v1 2/4] ufs: mcq: Add supporting functions for mcq abort

[Bart Van Assche]

On 3/7/23 20:01, Bao D. Nguyen wrote:
> +/* Maximum MCQ registers polling time */

registers -> register

> +#define MCQ_POLL_TIMEOUT   500

>   	struct ufshcd_mcq_opr_info_t mcq_opr[OPR_MAX];
> +	u32 mcq_poll_ms;
>   };

Why has the new member variable 'mcq_poll_ms' been introduced since its 
value is never changed?

Thanks,

Bart.

Re: [RFC PATCH v1 2/4] ufs: mcq: Add supporting functions for mcq abort

[Bao D. Nguyen]

Hi Bart, thank you very much for the reviews.

On 3/8/2023 10:48 AM, Bart Van Assche wrote:
> On 3/7/23 20:01, Bao D. Nguyen wrote:
>> +/* Maximum MCQ registers polling time */
>
> registers -> register
I will make the change in the next rev.
>
>> +#define MCQ_POLL_TIMEOUT   500
>
>>       struct ufshcd_mcq_opr_info_t mcq_opr[OPR_MAX];
>> +    u32 mcq_poll_ms;
>>   };
>
> Why has the new member variable 'mcq_poll_ms' been introduced since 
> its value is never changed?
>
This is to give us the flexibility to override this parameter in the 
downstream driver if needed.
> Thanks,
>
> Bart.

Re: [RFC PATCH v1 2/4] ufs: mcq: Add supporting functions for mcq abort

[Bart Van Assche]

On 3/8/23 14:27, Bao D. Nguyen wrote:
> This is to give us the flexibility to override this parameter in the 
> downstream driver if needed.

Please do not introduce functionality before it is needed. See also 
https://en.wikipedia.org/wiki/You_aren%27t_gonna_need_it.

Thanks,

Bart.

Re: [RFC PATCH v1 2/4] ufs: mcq: Add supporting functions for mcq abort

[Bao D. Nguyen]

On 3/8/2023 3:23 PM, Bart Van Assche wrote:
> On 3/8/23 14:27, Bao D. Nguyen wrote:
>> This is to give us the flexibility to override this parameter in the 
>> downstream driver if needed.
>
> Please do not introduce functionality before it is needed. See also 
> https://en.wikipedia.org/wiki/You_aren%27t_gonna_need_it.
I will remove it. Thanks.
>
> Thanks,
>
> Bart.
>

[RFC PATCH v1 3/4] ufs: mcq: Add support for clean up mcq resources

[Bao D. Nguyen]

Update ufshcd_clear_cmds() to clean up the mcq resources similar
to the function ufshcd_utrl_clear() does for sdb mode.

Update ufshcd_try_to_abort_task() to support mcq mode so that
this function can be invoked in either mcq or sdb mode.

Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
---
 drivers/ufs/core/ufshcd.c | 40 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 37 insertions(+), 3 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 7e52af6..408c16c 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -2988,7 +2988,26 @@ static int ufshcd_compose_dev_cmd(struct ufs_hba *hba,
 static int ufshcd_clear_cmds(struct ufs_hba *hba, u32 mask)
 {
 	unsigned long flags;
+	int err, result, tag;
 
+	if (is_mcq_enabled(hba)) {
+		/*
+		 * MCQ mode. Clean up the MCQ resources similar to
+		 * what the ufshcd_utrl_clear() does for SDB mode.
+		 */
+		flags = (unsigned long)mask;
+		for_each_set_bit(tag, &flags, hba->nutrs) {
+			err = ufshcd_mcq_sq_cleanup(hba, tag, &result);
+			if (err || result) {
+				dev_err(hba->dev, "%s: failed tag=%d. err=%d, result=%d\n",
+					__func__, tag, err, result);
+				return FAILED;
+			}
+		}
+		return 0;
+	}
+
+	/* Single Doorbell Mode */
 	/* clear outstanding transaction before retry */
 	spin_lock_irqsave(hba->host->host_lock, flags);
 	ufshcd_utrl_clear(hba, mask);
@@ -7344,6 +7363,20 @@ static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
 			 */
 			dev_err(hba->dev, "%s: cmd at tag %d not pending in the device.\n",
 				__func__, tag);
+			if (is_mcq_enabled(hba)) {
+				/* mcq mode */
+				if (!lrbp->cmd) {
+					/* command completed already */
+					dev_err(hba->dev, "%s: cmd at tag=%d is cleared.\n",
+						__func__, tag);
+					goto out;
+				}
+				/* sleep for max. 200us to stabilize */
+				usleep_range(100, 200);
+				continue;
+			}
+
+			/* single door bell mode */
 			reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
 			if (reg & (1 << tag)) {
 				/* sleep for max. 200us to stabilize */
@@ -7410,8 +7443,8 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
 
 	ufshcd_hold(hba, false);
 	reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
-	/* If command is already aborted/completed, return FAILED. */
-	if (!(test_bit(tag, &hba->outstanding_reqs))) {
+	if (!is_mcq_enabled(hba) && !(test_bit(tag, &hba->outstanding_reqs))) {
+		/* If command is already aborted/completed, return FAILED. */
 		dev_err(hba->dev,
 			"%s: cmd at tag %d already completed, outstanding=0x%lx, doorbell=0x%x\n",
 			__func__, tag, hba->outstanding_reqs, reg);
@@ -7440,7 +7473,8 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
 	}
 	hba->req_abort_count++;
 
-	if (!(reg & (1 << tag))) {
+	if (!is_mcq_enabled(hba) && !(reg & (1 << tag))) {
+		/* only execute this code in single doorbell mode */
 		dev_err(hba->dev,
 		"%s: cmd was completed, but without a notifying intr, tag = %d",
 		__func__, tag);
-- 
2.7.4

Re: [RFC PATCH v1 3/4] ufs: mcq: Add support for clean up mcq resources

[Bart Van Assche]

On 3/7/23 20:01, Bao D. Nguyen wrote:
>   static int ufshcd_clear_cmds(struct ufs_hba *hba, u32 mask)
>   {
>   	unsigned long flags;
> +	int err, result, tag;
>   
> +	if (is_mcq_enabled(hba)) {
> +		/*
> +		 * MCQ mode. Clean up the MCQ resources similar to
> +		 * what the ufshcd_utrl_clear() does for SDB mode.
> +		 */
> +		flags = (unsigned long)mask;
> +		for_each_set_bit(tag, &flags, hba->nutrs) {
> +			err = ufshcd_mcq_sq_cleanup(hba, tag, &result);
> +			if (err || result) {
> +				dev_err(hba->dev, "%s: failed tag=%d. err=%d, result=%d\n",
> +					__func__, tag, err, result);
> +				return FAILED;
> +			}
> +		}
> +		return 0;
> +	}

Please change the type of the 'mask' argument from u32 into unsigned 
long such that the assignment of 'mask' to 'flags' can be removed.

Thanks,

Bart.

Re: [RFC PATCH v1 3/4] ufs: mcq: Add support for clean up mcq resources

[Bao D. Nguyen]

On 3/8/2023 10:53 AM, Bart Van Assche wrote:
> On 3/7/23 20:01, Bao D. Nguyen wrote:
>>   static int ufshcd_clear_cmds(struct ufs_hba *hba, u32 mask)
>>   {
>>       unsigned long flags;
>> +    int err, result, tag;
>>   +    if (is_mcq_enabled(hba)) {
>> +        /*
>> +         * MCQ mode. Clean up the MCQ resources similar to
>> +         * what the ufshcd_utrl_clear() does for SDB mode.
>> +         */
>> +        flags = (unsigned long)mask;
>> +        for_each_set_bit(tag, &flags, hba->nutrs) {
>> +            err = ufshcd_mcq_sq_cleanup(hba, tag, &result);
>> +            if (err || result) {
>> +                dev_err(hba->dev, "%s: failed tag=%d. err=%d, 
>> result=%d\n",
>> +                    __func__, tag, err, result);
>> +                return FAILED;
>> +            }
>> +        }
>> +        return 0;
>> +    }
>
> Please change the type of the 'mask' argument from u32 into unsigned 
> long such that the assignment of 'mask' to 'flags' can be removed.
>
I will make the change in the next rev. Thank you.

> Thanks,
>
> Bart.

[RFC PATCH v1 4/4] ufs: mcq: Added ufshcd_mcq_abort()

[Bao D. Nguyen]

Add ufshcd_mcq_abort() to support ufs abort in mcq mode.

Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
---
 drivers/ufs/core/ufs-mcq.c     | 76 ++++++++++++++++++++++++++++++++++++++++++
 drivers/ufs/core/ufshcd-priv.h |  5 ++-
 drivers/ufs/core/ufshcd.c      | 11 ++++--
 3 files changed, 88 insertions(+), 4 deletions(-)

diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
index e27d8eb..6c65cd5 100644
--- a/drivers/ufs/core/ufs-mcq.c
+++ b/drivers/ufs/core/ufs-mcq.c
@@ -646,3 +646,79 @@ static bool ufshcd_mcq_cqe_search(struct ufs_hba *hba,
 	spin_unlock(&hwq->cq_lock);
 	return ret;
 }
+
+/**
+ * ufshcd_mcq_abort - Abort the command in MCQ.
+ * @cmd - The command to be aborted.
+ *
+ * Returns SUCCESS or FAILED error codes
+ */
+int ufshcd_mcq_abort(struct scsi_cmnd *cmd)
+{
+	struct Scsi_Host *host = cmd->device->host;
+	struct ufs_hba *hba = shost_priv(host);
+	int tag = scsi_cmd_to_rq(cmd)->tag;
+	struct ufshcd_lrb *lrbp = &hba->lrb[tag];
+	struct ufs_hw_queue *hwq;
+	int err = FAILED;
+
+	if (!lrbp->cmd) {
+		dev_err(hba->dev,
+			"%s: skip abort. cmd at tag %d already completed.\n",
+			__func__, tag);
+		goto out;
+	}
+
+	/* Skip task abort in case previous aborts failed and report failure */
+	if (lrbp->req_abort_skip) {
+		dev_err(hba->dev, "%s: skip abort. tag %d failed earlier\n",
+			__func__, tag);
+		goto out;
+	}
+
+	hwq = ufshcd_mcq_req_to_hwq(hba, scsi_cmd_to_rq(cmd));
+
+	if (ufshcd_mcq_sqe_search(hba, hwq, tag)) {
+		/*
+		 * Failure. The command should not be "stuck" in SQ for
+		 * a long time which resulted in command being aborted.
+		 */
+		dev_err(hba->dev, "%s: cmd found in sq. hwq=%d, tag=%d\n",
+				__func__, hwq->id, tag);
+		/* Set the Command Type to 0xF per the spec */
+		ufshcd_mcq_nullify_cmd(hba, hwq);
+		goto out;
+	}
+
+	if (ufshcd_mcq_cqe_search(hba, hwq, tag)) {
+		dev_err(hba->dev, "%s: cmd found in cq. hwq=%d, tag=%d\n",
+				__func__, hwq->id, tag);
+		/*
+		 * The command should not be 'stuck' in the CQ for such a long time.
+		 * Is interrupt missing? Process the CQEs here. If the interrupt is
+		 * invoked at a later time, the CQ will be empty because the CQEs
+		 * are already processed here.
+		 */
+		ufshcd_mcq_poll_cqe_lock(hba, hwq);
+		err = SUCCESS;
+		goto out;
+	}
+
+	/*
+	 * The command is not in the Submission Queue, and it is not
+	 * in the Completion Queue either. Query the device to see if
+	 * the command is being processed in the device.
+	 */
+	if (ufshcd_try_to_abort_task(hba, tag)) {
+		dev_err(hba->dev, "%s: device abort failed %d\n", __func__, err);
+		lrbp->req_abort_skip = true;
+		goto out;
+	}
+
+	err = SUCCESS;
+	if (lrbp->cmd)
+		ufshcd_release_scsi_cmd(hba, lrbp);
+
+out:
+	return err;
+}
diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h
index 0527926..d883c03 100644
--- a/drivers/ufs/core/ufshcd-priv.h
+++ b/drivers/ufs/core/ufshcd-priv.h
@@ -77,7 +77,10 @@ unsigned long ufshcd_mcq_poll_cqe_lock(struct ufs_hba *hba,
 				       struct ufs_hw_queue *hwq);
 
 int ufshcd_mcq_sq_cleanup(struct ufs_hba *hba, int task_tag, int *result);
-
+int ufshcd_mcq_abort(struct scsi_cmnd *cmd);
+int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag);
+void ufshcd_release_scsi_cmd(struct ufs_hba *hba,
+				struct ufshcd_lrb *lrbp);
 #define UFSHCD_MCQ_IO_QUEUE_OFFSET	1
 #define SD_ASCII_STD true
 #define SD_RAW false
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 408c16c..e06399e 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -302,7 +302,6 @@ static int ufshcd_setup_hba_vreg(struct ufs_hba *hba, bool on);
 static int ufshcd_setup_vreg(struct ufs_hba *hba, bool on);
 static inline int ufshcd_config_vreg_hpm(struct ufs_hba *hba,
 					 struct ufs_vreg *vreg);
-static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag);
 static void ufshcd_wb_toggle_buf_flush_during_h8(struct ufs_hba *hba,
 						 bool enable);
 static void ufshcd_hba_vreg_set_lpm(struct ufs_hba *hba);
@@ -5414,7 +5413,7 @@ static irqreturn_t ufshcd_uic_cmd_compl(struct ufs_hba *hba, u32 intr_status)
 }
 
 /* Release the resources allocated for processing a SCSI command. */
-static void ufshcd_release_scsi_cmd(struct ufs_hba *hba,
+void ufshcd_release_scsi_cmd(struct ufs_hba *hba,
 				    struct ufshcd_lrb *lrbp)
 {
 	struct scsi_cmnd *cmd = lrbp->cmd;
@@ -7340,7 +7339,7 @@ static void ufshcd_set_req_abort_skip(struct ufs_hba *hba, unsigned long bitmap)
  *
  * Returns zero on success, non-zero on failure
  */
-static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
+int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
 {
 	struct ufshcd_lrb *lrbp = &hba->lrb[tag];
 	int err = 0;
@@ -7500,6 +7499,12 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
 		goto release;
 	}
 
+	if (is_mcq_enabled(hba)) {
+		/* MCQ mode. Branch off to handle abort for mcq mode */
+		err = ufshcd_mcq_abort(cmd);
+		goto release;
+	}
+
 	/* Skip task abort in case previous aborts failed and report failure */
 	if (lrbp->req_abort_skip) {
 		dev_err(hba->dev, "%s: skipping abort\n", __func__);
-- 
2.7.4

Re: [RFC PATCH v1 4/4] ufs: mcq: Added ufshcd_mcq_abort()

[Bart Van Assche]

On 3/7/23 20:01, Bao D. Nguyen wrote:
> +	if (ufshcd_mcq_cqe_search(hba, hwq, tag)) {
> +		dev_err(hba->dev, "%s: cmd found in cq. hwq=%d, tag=%d\n",
> +				__func__, hwq->id, tag);
> +		/*
> +		 * The command should not be 'stuck' in the CQ for such a long time.
> +		 * Is interrupt missing? Process the CQEs here. If the interrupt is
> +		 * invoked at a later time, the CQ will be empty because the CQEs
> +		 * are already processed here.
> +		 */
> +		ufshcd_mcq_poll_cqe_lock(hba, hwq);
> +		err = SUCCESS;
> +		goto out;
> +	}

Please remove the above code and also the definition of the 
ufshcd_mcq_cqe_search() function. The SCSI error handler submits an 
abort to deal with command processing timeouts. ufshcd_mcq_cqe_search() 
can only return true in case of a software bug at the host side. 
Addressing such bugs is out of scope for the SCSI error handler.

Thanks,

Bart.

Re: [RFC PATCH v1 4/4] ufs: mcq: Added ufshcd_mcq_abort()

[Bao D. Nguyen]

On 3/8/2023 11:02 AM, Bart Van Assche wrote:
> On 3/7/23 20:01, Bao D. Nguyen wrote:
>> +    if (ufshcd_mcq_cqe_search(hba, hwq, tag)) {
>> +        dev_err(hba->dev, "%s: cmd found in cq. hwq=%d, tag=%d\n",
>> +                __func__, hwq->id, tag);
>> +        /*
>> +         * The command should not be 'stuck' in the CQ for such a 
>> long time.
>> +         * Is interrupt missing? Process the CQEs here. If the 
>> interrupt is
>> +         * invoked at a later time, the CQ will be empty because the 
>> CQEs
>> +         * are already processed here.
>> +         */
>> +        ufshcd_mcq_poll_cqe_lock(hba, hwq);
>> +        err = SUCCESS;
>> +        goto out;
>> +    }
>
> Please remove the above code and also the definition of the 
> ufshcd_mcq_cqe_search() function. The SCSI error handler submits an 
> abort to deal with command processing timeouts. 
> ufshcd_mcq_cqe_search() can only return true in case of a software bug 
> at the host side. Addressing such bugs is out of scope for the SCSI 
> error handler.

This is an attempt to handle the error case similar to SDB mode where it 
prints "%s: cmd was completed, but without a notifying intr, tag = %d" 
in the ufshcd_abort() function.

In this case the command has been completed by the hardware, but some 
reasons the software has not processed it. We have seen this print 
happened during debug sessions, so the error case does happen in SBL mode.

Are you suggesting we should return error in this case without calling 
ufshcd_mcq_poll_cqe_lock()?

Thanks.

>
>
> Thanks,
>
> Bart.

Re: [RFC PATCH v1 4/4] ufs: mcq: Added ufshcd_mcq_abort()

[Bart Van Assche]

On 3/8/23 14:37, Bao D. Nguyen wrote:
> On 3/8/2023 11:02 AM, Bart Van Assche wrote:
>> On 3/7/23 20:01, Bao D. Nguyen wrote:
>>> +    if (ufshcd_mcq_cqe_search(hba, hwq, tag)) {
>>> +        dev_err(hba->dev, "%s: cmd found in cq. hwq=%d, tag=%d\n",
>>> +                __func__, hwq->id, tag);
>>> +        /*
>>> +         * The command should not be 'stuck' in the CQ for such a 
>>> long time.
>>> +         * Is interrupt missing? Process the CQEs here. If the 
>>> interrupt is
>>> +         * invoked at a later time, the CQ will be empty because the 
>>> CQEs
>>> +         * are already processed here.
>>> +         */
>>> +        ufshcd_mcq_poll_cqe_lock(hba, hwq);
>>> +        err = SUCCESS;
>>> +        goto out;
>>> +    }
>>
>> Please remove the above code and also the definition of the 
>> ufshcd_mcq_cqe_search() function. The SCSI error handler submits an 
>> abort to deal with command processing timeouts. 
>> ufshcd_mcq_cqe_search() can only return true in case of a software bug 
>> at the host side. Addressing such bugs is out of scope for the SCSI 
>> error handler.
> 
> This is an attempt to handle the error case similar to SDB mode where it 
> prints "%s: cmd was completed, but without a notifying intr, tag = %d" 
> in the ufshcd_abort() function.
> 
> In this case the command has been completed by the hardware, but some 
> reasons the software has not processed it. We have seen this print 
> happened during debug sessions, so the error case does happen in SBL mode.
> 
> Are you suggesting we should return error in this case without calling 
> ufshcd_mcq_poll_cqe_lock()?

What I am asking is to remove ufshcd_mcq_poll_cqe_lock() and all code 
that depends on that function returning true. Although such code might 
be useful for SoC debugging, helping with SoC debugging is out of scope 
for Linux kernel drivers.

Thanks,

Bart.

Re: [RFC PATCH v1 4/4] ufs: mcq: Added ufshcd_mcq_abort()

[Bao D. Nguyen]

On 3/8/2023 3:25 PM, Bart Van Assche wrote:
> On 3/8/23 14:37, Bao D. Nguyen wrote:
>> On 3/8/2023 11:02 AM, Bart Van Assche wrote:
>>> On 3/7/23 20:01, Bao D. Nguyen wrote:
>>>> +    if (ufshcd_mcq_cqe_search(hba, hwq, tag)) {
>>>> +        dev_err(hba->dev, "%s: cmd found in cq. hwq=%d, tag=%d\n",
>>>> +                __func__, hwq->id, tag);
>>>> +        /*
>>>> +         * The command should not be 'stuck' in the CQ for such a 
>>>> long time.
>>>> +         * Is interrupt missing? Process the CQEs here. If the 
>>>> interrupt is
>>>> +         * invoked at a later time, the CQ will be empty because 
>>>> the CQEs
>>>> +         * are already processed here.
>>>> +         */
>>>> +        ufshcd_mcq_poll_cqe_lock(hba, hwq);
>>>> +        err = SUCCESS;
>>>> +        goto out;
>>>> +    }
>>>
>>> Please remove the above code and also the definition of the 
>>> ufshcd_mcq_cqe_search() function. The SCSI error handler submits an 
>>> abort to deal with command processing timeouts. 
>>> ufshcd_mcq_cqe_search() can only return true in case of a software 
>>> bug at the host side. Addressing such bugs is out of scope for the 
>>> SCSI error handler.
>>
>> This is an attempt to handle the error case similar to SDB mode where 
>> it prints "%s: cmd was completed, but without a notifying intr, tag = 
>> %d" in the ufshcd_abort() function.
>>
>> In this case the command has been completed by the hardware, but some 
>> reasons the software has not processed it. We have seen this print 
>> happened during debug sessions, so the error case does happen in SBL 
>> mode.
>>
>> Are you suggesting we should return error in this case without 
>> calling ufshcd_mcq_poll_cqe_lock()?
>
> What I am asking is to remove ufshcd_mcq_poll_cqe_lock() and all code 
> that depends on that function returning true. Although such code might 
> be useful for SoC debugging, helping with SoC debugging is out of 
> scope for Linux kernel drivers.
I will remove it. In that case, we don't need the first patch of this 
series, so I will remove the first patch as well. Thanks.
>
> Thanks,
>
> Bart.
>

Re: [RFC PATCH v1 4/4] ufs: mcq: Added ufshcd_mcq_abort()

[Stanley Chu]

Hi Bao,

On Wed, Mar 8, 2023 at 12:10 PM Bao D. Nguyen <quic_nguyenb@quicinc.com> wrote:
>
> Add ufshcd_mcq_abort() to support ufs abort in mcq mode.
>
> Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
> ---
>  drivers/ufs/core/ufs-mcq.c     | 76 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/ufs/core/ufshcd-priv.h |  5 ++-
>  drivers/ufs/core/ufshcd.c      | 11 ++++--
>  3 files changed, 88 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
> index e27d8eb..6c65cd5 100644
> --- a/drivers/ufs/core/ufs-mcq.c
> +++ b/drivers/ufs/core/ufs-mcq.c
> @@ -646,3 +646,79 @@ static bool ufshcd_mcq_cqe_search(struct ufs_hba *hba,
>         spin_unlock(&hwq->cq_lock);
>         return ret;
>  }
> +
> +/**
> + * ufshcd_mcq_abort - Abort the command in MCQ.
> + * @cmd - The command to be aborted.
> + *
> + * Returns SUCCESS or FAILED error codes
> + */
> +int ufshcd_mcq_abort(struct scsi_cmnd *cmd)
> +{
> +       struct Scsi_Host *host = cmd->device->host;
> +       struct ufs_hba *hba = shost_priv(host);
> +       int tag = scsi_cmd_to_rq(cmd)->tag;
> +       struct ufshcd_lrb *lrbp = &hba->lrb[tag];
> +       struct ufs_hw_queue *hwq;
> +       int err = FAILED;
> +
> +       if (!lrbp->cmd) {
> +               dev_err(hba->dev,
> +                       "%s: skip abort. cmd at tag %d already completed.\n",
> +                       __func__, tag);
> +               goto out;
> +       }
> +
> +       /* Skip task abort in case previous aborts failed and report failure */
> +       if (lrbp->req_abort_skip) {
> +               dev_err(hba->dev, "%s: skip abort. tag %d failed earlier\n",
> +                       __func__, tag);
> +               goto out;
> +       }
> +
> +       hwq = ufshcd_mcq_req_to_hwq(hba, scsi_cmd_to_rq(cmd));
> +
> +       if (ufshcd_mcq_sqe_search(hba, hwq, tag)) {
> +               /*
> +                * Failure. The command should not be "stuck" in SQ for
> +                * a long time which resulted in command being aborted.
> +                */
> +               dev_err(hba->dev, "%s: cmd found in sq. hwq=%d, tag=%d\n",
> +                               __func__, hwq->id, tag);
> +               /* Set the Command Type to 0xF per the spec */
> +               ufshcd_mcq_nullify_cmd(hba, hwq);
> +               goto out;
> +       }
> +
> +       if (ufshcd_mcq_cqe_search(hba, hwq, tag)) {
> +               dev_err(hba->dev, "%s: cmd found in cq. hwq=%d, tag=%d\n",
> +                               __func__, hwq->id, tag);
> +               /*
> +                * The command should not be 'stuck' in the CQ for such a long time.
> +                * Is interrupt missing? Process the CQEs here. If the interrupt is
> +                * invoked at a later time, the CQ will be empty because the CQEs
> +                * are already processed here.
> +                */
> +               ufshcd_mcq_poll_cqe_lock(hba, hwq);
> +               err = SUCCESS;
> +               goto out;
> +       }
> +
> +       /*
> +        * The command is not in the Submission Queue, and it is not
> +        * in the Completion Queue either. Query the device to see if
> +        * the command is being processed in the device.
> +        */
> +       if (ufshcd_try_to_abort_task(hba, tag)) {
> +               dev_err(hba->dev, "%s: device abort failed %d\n", __func__, err);
> +               lrbp->req_abort_skip = true;
> +               goto out;
> +       }
> +
> +       err = SUCCESS;
> +       if (lrbp->cmd)
> +               ufshcd_release_scsi_cmd(hba, lrbp);
> +
> +out:
> +       return err;
> +}
> diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h
> index 0527926..d883c03 100644
> --- a/drivers/ufs/core/ufshcd-priv.h
> +++ b/drivers/ufs/core/ufshcd-priv.h
> @@ -77,7 +77,10 @@ unsigned long ufshcd_mcq_poll_cqe_lock(struct ufs_hba *hba,
>                                        struct ufs_hw_queue *hwq);
>
>  int ufshcd_mcq_sq_cleanup(struct ufs_hba *hba, int task_tag, int *result);
> -
> +int ufshcd_mcq_abort(struct scsi_cmnd *cmd);
> +int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag);
> +void ufshcd_release_scsi_cmd(struct ufs_hba *hba,
> +                               struct ufshcd_lrb *lrbp);
>  #define UFSHCD_MCQ_IO_QUEUE_OFFSET     1
>  #define SD_ASCII_STD true
>  #define SD_RAW false
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 408c16c..e06399e 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -302,7 +302,6 @@ static int ufshcd_setup_hba_vreg(struct ufs_hba *hba, bool on);
>  static int ufshcd_setup_vreg(struct ufs_hba *hba, bool on);
>  static inline int ufshcd_config_vreg_hpm(struct ufs_hba *hba,
>                                          struct ufs_vreg *vreg);
> -static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag);
>  static void ufshcd_wb_toggle_buf_flush_during_h8(struct ufs_hba *hba,
>                                                  bool enable);
>  static void ufshcd_hba_vreg_set_lpm(struct ufs_hba *hba);
> @@ -5414,7 +5413,7 @@ static irqreturn_t ufshcd_uic_cmd_compl(struct ufs_hba *hba, u32 intr_status)
>  }
>
>  /* Release the resources allocated for processing a SCSI command. */
> -static void ufshcd_release_scsi_cmd(struct ufs_hba *hba,
> +void ufshcd_release_scsi_cmd(struct ufs_hba *hba,
>                                     struct ufshcd_lrb *lrbp)
>  {
>         struct scsi_cmnd *cmd = lrbp->cmd;
> @@ -7340,7 +7339,7 @@ static void ufshcd_set_req_abort_skip(struct ufs_hba *hba, unsigned long bitmap)
>   *
>   * Returns zero on success, non-zero on failure
>   */
> -static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
> +int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
>  {
>         struct ufshcd_lrb *lrbp = &hba->lrb[tag];
>         int err = 0;
> @@ -7500,6 +7499,12 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
>                 goto release;
>         }
>
> +       if (is_mcq_enabled(hba)) {
> +               /* MCQ mode. Branch off to handle abort for mcq mode */
> +               err = ufshcd_mcq_abort(cmd);
> +               goto release;
> +       }
> +
>         /* Skip task abort in case previous aborts failed and report failure */
>         if (lrbp->req_abort_skip) {
>                 dev_err(hba->dev, "%s: skipping abort\n", __func__);
> --
> 2.7.4
>

It seems that ufshcd_abort_all() also needs an error handling flow for MCQ.

Thanks,
Stanley

Re: [RFC PATCH v1 4/4] ufs: mcq: Added ufshcd_mcq_abort()

[Bao D. Nguyen]

On 3/8/2023 7:10 PM, Stanley Chu wrote:
> Hi Bao,
>
> On Wed, Mar 8, 2023 at 12:10 PM Bao D. Nguyen <quic_nguyenb@quicinc.com> wrote:
>> Add ufshcd_mcq_abort() to support ufs abort in mcq mode.
>>
>> Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
>> ---
>>   drivers/ufs/core/ufs-mcq.c     | 76 ++++++++++++++++++++++++++++++++++++++++++
>>   drivers/ufs/core/ufshcd-priv.h |  5 ++-
>>   drivers/ufs/core/ufshcd.c      | 11 ++++--
>>   3 files changed, 88 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
>> index e27d8eb..6c65cd5 100644
>> --- a/drivers/ufs/core/ufs-mcq.c
>> +++ b/drivers/ufs/core/ufs-mcq.c
>> @@ -646,3 +646,79 @@ static bool ufshcd_mcq_cqe_search(struct ufs_hba *hba,
>>          spin_unlock(&hwq->cq_lock);
>>          return ret;
>>   }
>> +
>> +/**
>> + * ufshcd_mcq_abort - Abort the command in MCQ.
>> + * @cmd - The command to be aborted.
>> + *
>> + * Returns SUCCESS or FAILED error codes
>> + */
>> +int ufshcd_mcq_abort(struct scsi_cmnd *cmd)
>> +{
>> +       struct Scsi_Host *host = cmd->device->host;
>> +       struct ufs_hba *hba = shost_priv(host);
>> +       int tag = scsi_cmd_to_rq(cmd)->tag;
>> +       struct ufshcd_lrb *lrbp = &hba->lrb[tag];
>> +       struct ufs_hw_queue *hwq;
>> +       int err = FAILED;
>> +
>> +       if (!lrbp->cmd) {
>> +               dev_err(hba->dev,
>> +                       "%s: skip abort. cmd at tag %d already completed.\n",
>> +                       __func__, tag);
>> +               goto out;
>> +       }
>> +
>> +       /* Skip task abort in case previous aborts failed and report failure */
>> +       if (lrbp->req_abort_skip) {
>> +               dev_err(hba->dev, "%s: skip abort. tag %d failed earlier\n",
>> +                       __func__, tag);
>> +               goto out;
>> +       }
>> +
>> +       hwq = ufshcd_mcq_req_to_hwq(hba, scsi_cmd_to_rq(cmd));
>> +
>> +       if (ufshcd_mcq_sqe_search(hba, hwq, tag)) {
>> +               /*
>> +                * Failure. The command should not be "stuck" in SQ for
>> +                * a long time which resulted in command being aborted.
>> +                */
>> +               dev_err(hba->dev, "%s: cmd found in sq. hwq=%d, tag=%d\n",
>> +                               __func__, hwq->id, tag);
>> +               /* Set the Command Type to 0xF per the spec */
>> +               ufshcd_mcq_nullify_cmd(hba, hwq);
>> +               goto out;
>> +       }
>> +
>> +       if (ufshcd_mcq_cqe_search(hba, hwq, tag)) {
>> +               dev_err(hba->dev, "%s: cmd found in cq. hwq=%d, tag=%d\n",
>> +                               __func__, hwq->id, tag);
>> +               /*
>> +                * The command should not be 'stuck' in the CQ for such a long time.
>> +                * Is interrupt missing? Process the CQEs here. If the interrupt is
>> +                * invoked at a later time, the CQ will be empty because the CQEs
>> +                * are already processed here.
>> +                */
>> +               ufshcd_mcq_poll_cqe_lock(hba, hwq);
>> +               err = SUCCESS;
>> +               goto out;
>> +       }
>> +
>> +       /*
>> +        * The command is not in the Submission Queue, and it is not
>> +        * in the Completion Queue either. Query the device to see if
>> +        * the command is being processed in the device.
>> +        */
>> +       if (ufshcd_try_to_abort_task(hba, tag)) {
>> +               dev_err(hba->dev, "%s: device abort failed %d\n", __func__, err);
>> +               lrbp->req_abort_skip = true;
>> +               goto out;
>> +       }
>> +
>> +       err = SUCCESS;
>> +       if (lrbp->cmd)
>> +               ufshcd_release_scsi_cmd(hba, lrbp);
>> +
>> +out:
>> +       return err;
>> +}
>> diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h
>> index 0527926..d883c03 100644
>> --- a/drivers/ufs/core/ufshcd-priv.h
>> +++ b/drivers/ufs/core/ufshcd-priv.h
>> @@ -77,7 +77,10 @@ unsigned long ufshcd_mcq_poll_cqe_lock(struct ufs_hba *hba,
>>                                         struct ufs_hw_queue *hwq);
>>
>>   int ufshcd_mcq_sq_cleanup(struct ufs_hba *hba, int task_tag, int *result);
>> -
>> +int ufshcd_mcq_abort(struct scsi_cmnd *cmd);
>> +int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag);
>> +void ufshcd_release_scsi_cmd(struct ufs_hba *hba,
>> +                               struct ufshcd_lrb *lrbp);
>>   #define UFSHCD_MCQ_IO_QUEUE_OFFSET     1
>>   #define SD_ASCII_STD true
>>   #define SD_RAW false
>> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
>> index 408c16c..e06399e 100644
>> --- a/drivers/ufs/core/ufshcd.c
>> +++ b/drivers/ufs/core/ufshcd.c
>> @@ -302,7 +302,6 @@ static int ufshcd_setup_hba_vreg(struct ufs_hba *hba, bool on);
>>   static int ufshcd_setup_vreg(struct ufs_hba *hba, bool on);
>>   static inline int ufshcd_config_vreg_hpm(struct ufs_hba *hba,
>>                                           struct ufs_vreg *vreg);
>> -static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag);
>>   static void ufshcd_wb_toggle_buf_flush_during_h8(struct ufs_hba *hba,
>>                                                   bool enable);
>>   static void ufshcd_hba_vreg_set_lpm(struct ufs_hba *hba);
>> @@ -5414,7 +5413,7 @@ static irqreturn_t ufshcd_uic_cmd_compl(struct ufs_hba *hba, u32 intr_status)
>>   }
>>
>>   /* Release the resources allocated for processing a SCSI command. */
>> -static void ufshcd_release_scsi_cmd(struct ufs_hba *hba,
>> +void ufshcd_release_scsi_cmd(struct ufs_hba *hba,
>>                                      struct ufshcd_lrb *lrbp)
>>   {
>>          struct scsi_cmnd *cmd = lrbp->cmd;
>> @@ -7340,7 +7339,7 @@ static void ufshcd_set_req_abort_skip(struct ufs_hba *hba, unsigned long bitmap)
>>    *
>>    * Returns zero on success, non-zero on failure
>>    */
>> -static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
>> +int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
>>   {
>>          struct ufshcd_lrb *lrbp = &hba->lrb[tag];
>>          int err = 0;
>> @@ -7500,6 +7499,12 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
>>                  goto release;
>>          }
>>
>> +       if (is_mcq_enabled(hba)) {
>> +               /* MCQ mode. Branch off to handle abort for mcq mode */
>> +               err = ufshcd_mcq_abort(cmd);
>> +               goto release;
>> +       }
>> +
>>          /* Skip task abort in case previous aborts failed and report failure */
>>          if (lrbp->req_abort_skip) {
>>                  dev_err(hba->dev, "%s: skipping abort\n", __func__);
>> --
>> 2.7.4
>>
> It seems that ufshcd_abort_all() also needs an error handling flow for MCQ.

Hi Stanley, thank you for the reviews.

I am not able to find the function ufshcd_abort_all() in the drivers/ufs 
directory. Can you please clarify where this function is located? Thanks.

>
> Thanks,
> Stanley

Re: [RFC PATCH v1 4/4] ufs: mcq: Added ufshcd_mcq_abort()

[Stanley Chu]

Hi Bao,

On Thu, Mar 9, 2023 at 1:31 PM Bao D. Nguyen <quic_nguyenb@quicinc.com> wrote:
>
> On 3/8/2023 7:10 PM, Stanley Chu wrote:
> > Hi Bao,
> >
> > On Wed, Mar 8, 2023 at 12:10 PM Bao D. Nguyen <quic_nguyenb@quicinc.com> wrote:
> >> Add ufshcd_mcq_abort() to support ufs abort in mcq mode.
> >>
> >> Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
> >> ---
> >>   drivers/ufs/core/ufs-mcq.c     | 76 ++++++++++++++++++++++++++++++++++++++++++
> >>   drivers/ufs/core/ufshcd-priv.h |  5 ++-
> >>   drivers/ufs/core/ufshcd.c      | 11 ++++--
> >>   3 files changed, 88 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
> >> index e27d8eb..6c65cd5 100644
> >> --- a/drivers/ufs/core/ufs-mcq.c
> >> +++ b/drivers/ufs/core/ufs-mcq.c
> >> @@ -646,3 +646,79 @@ static bool ufshcd_mcq_cqe_search(struct ufs_hba *hba,
> >>          spin_unlock(&hwq->cq_lock);
> >>          return ret;
> >>   }
> >> +
> >> +/**
> >> + * ufshcd_mcq_abort - Abort the command in MCQ.
> >> + * @cmd - The command to be aborted.
> >> + *
> >> + * Returns SUCCESS or FAILED error codes
> >> + */
> >> +int ufshcd_mcq_abort(struct scsi_cmnd *cmd)
> >> +{
> >> +       struct Scsi_Host *host = cmd->device->host;
> >> +       struct ufs_hba *hba = shost_priv(host);
> >> +       int tag = scsi_cmd_to_rq(cmd)->tag;
> >> +       struct ufshcd_lrb *lrbp = &hba->lrb[tag];
> >> +       struct ufs_hw_queue *hwq;
> >> +       int err = FAILED;
> >> +
> >> +       if (!lrbp->cmd) {
> >> +               dev_err(hba->dev,
> >> +                       "%s: skip abort. cmd at tag %d already completed.\n",
> >> +                       __func__, tag);
> >> +               goto out;
> >> +       }
> >> +
> >> +       /* Skip task abort in case previous aborts failed and report failure */
> >> +       if (lrbp->req_abort_skip) {
> >> +               dev_err(hba->dev, "%s: skip abort. tag %d failed earlier\n",
> >> +                       __func__, tag);
> >> +               goto out;
> >> +       }
> >> +
> >> +       hwq = ufshcd_mcq_req_to_hwq(hba, scsi_cmd_to_rq(cmd));
> >> +
> >> +       if (ufshcd_mcq_sqe_search(hba, hwq, tag)) {
> >> +               /*
> >> +                * Failure. The command should not be "stuck" in SQ for
> >> +                * a long time which resulted in command being aborted.
> >> +                */
> >> +               dev_err(hba->dev, "%s: cmd found in sq. hwq=%d, tag=%d\n",
> >> +                               __func__, hwq->id, tag);
> >> +               /* Set the Command Type to 0xF per the spec */
> >> +               ufshcd_mcq_nullify_cmd(hba, hwq);
> >> +               goto out;
> >> +       }
> >> +
> >> +       if (ufshcd_mcq_cqe_search(hba, hwq, tag)) {
> >> +               dev_err(hba->dev, "%s: cmd found in cq. hwq=%d, tag=%d\n",
> >> +                               __func__, hwq->id, tag);
> >> +               /*
> >> +                * The command should not be 'stuck' in the CQ for such a long time.
> >> +                * Is interrupt missing? Process the CQEs here. If the interrupt is
> >> +                * invoked at a later time, the CQ will be empty because the CQEs
> >> +                * are already processed here.
> >> +                */
> >> +               ufshcd_mcq_poll_cqe_lock(hba, hwq);
> >> +               err = SUCCESS;
> >> +               goto out;
> >> +       }
> >> +
> >> +       /*
> >> +        * The command is not in the Submission Queue, and it is not
> >> +        * in the Completion Queue either. Query the device to see if
> >> +        * the command is being processed in the device.
> >> +        */
> >> +       if (ufshcd_try_to_abort_task(hba, tag)) {
> >> +               dev_err(hba->dev, "%s: device abort failed %d\n", __func__, err);
> >> +               lrbp->req_abort_skip = true;
> >> +               goto out;
> >> +       }
> >> +
> >> +       err = SUCCESS;
> >> +       if (lrbp->cmd)
> >> +               ufshcd_release_scsi_cmd(hba, lrbp);
> >> +
> >> +out:
> >> +       return err;
> >> +}
> >> diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h
> >> index 0527926..d883c03 100644
> >> --- a/drivers/ufs/core/ufshcd-priv.h
> >> +++ b/drivers/ufs/core/ufshcd-priv.h
> >> @@ -77,7 +77,10 @@ unsigned long ufshcd_mcq_poll_cqe_lock(struct ufs_hba *hba,
> >>                                         struct ufs_hw_queue *hwq);
> >>
> >>   int ufshcd_mcq_sq_cleanup(struct ufs_hba *hba, int task_tag, int *result);
> >> -
> >> +int ufshcd_mcq_abort(struct scsi_cmnd *cmd);
> >> +int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag);
> >> +void ufshcd_release_scsi_cmd(struct ufs_hba *hba,
> >> +                               struct ufshcd_lrb *lrbp);
> >>   #define UFSHCD_MCQ_IO_QUEUE_OFFSET     1
> >>   #define SD_ASCII_STD true
> >>   #define SD_RAW false
> >> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> >> index 408c16c..e06399e 100644
> >> --- a/drivers/ufs/core/ufshcd.c
> >> +++ b/drivers/ufs/core/ufshcd.c
> >> @@ -302,7 +302,6 @@ static int ufshcd_setup_hba_vreg(struct ufs_hba *hba, bool on);
> >>   static int ufshcd_setup_vreg(struct ufs_hba *hba, bool on);
> >>   static inline int ufshcd_config_vreg_hpm(struct ufs_hba *hba,
> >>                                           struct ufs_vreg *vreg);
> >> -static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag);
> >>   static void ufshcd_wb_toggle_buf_flush_during_h8(struct ufs_hba *hba,
> >>                                                   bool enable);
> >>   static void ufshcd_hba_vreg_set_lpm(struct ufs_hba *hba);
> >> @@ -5414,7 +5413,7 @@ static irqreturn_t ufshcd_uic_cmd_compl(struct ufs_hba *hba, u32 intr_status)
> >>   }
> >>
> >>   /* Release the resources allocated for processing a SCSI command. */
> >> -static void ufshcd_release_scsi_cmd(struct ufs_hba *hba,
> >> +void ufshcd_release_scsi_cmd(struct ufs_hba *hba,
> >>                                      struct ufshcd_lrb *lrbp)
> >>   {
> >>          struct scsi_cmnd *cmd = lrbp->cmd;
> >> @@ -7340,7 +7339,7 @@ static void ufshcd_set_req_abort_skip(struct ufs_hba *hba, unsigned long bitmap)
> >>    *
> >>    * Returns zero on success, non-zero on failure
> >>    */
> >> -static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
> >> +int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
> >>   {
> >>          struct ufshcd_lrb *lrbp = &hba->lrb[tag];
> >>          int err = 0;
> >> @@ -7500,6 +7499,12 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
> >>                  goto release;
> >>          }
> >>
> >> +       if (is_mcq_enabled(hba)) {
> >> +               /* MCQ mode. Branch off to handle abort for mcq mode */
> >> +               err = ufshcd_mcq_abort(cmd);
> >> +               goto release;
> >> +       }
> >> +
> >>          /* Skip task abort in case previous aborts failed and report failure */
> >>          if (lrbp->req_abort_skip) {
> >>                  dev_err(hba->dev, "%s: skipping abort\n", __func__);
> >> --
> >> 2.7.4
> >>
> > It seems that ufshcd_abort_all() also needs an error handling flow for MCQ.
>
> Hi Stanley, thank you for the reviews.
>
> I am not able to find the function ufshcd_abort_all() in the drivers/ufs
> directory. Can you please clarify where this function is located? Thanks.

The function ufshcd_abort_all() was introduced in the following link:
https://android.googlesource.com/kernel/common/+/b817e6ffbad7a1a0a5ca5bb7d4020823c3f4d9d0

Can you confirm if these patches are based on the latest kernel
6.3-rc1 or the latest linux-next tree?

Thanks,
Stanley

>
> >
> > Thanks,
> > Stanley
>
>

Re: [RFC PATCH v1 4/4] ufs: mcq: Added ufshcd_mcq_abort()

[Bao D. Nguyen]

On 3/8/2023 10:21 PM, Stanley Chu wrote:
> Hi Bao,
>
> On Thu, Mar 9, 2023 at 1:31 PM Bao D. Nguyen <quic_nguyenb@quicinc.com> wrote:
>> On 3/8/2023 7:10 PM, Stanley Chu wrote:
>>> Hi Bao,
>>>
>>> On Wed, Mar 8, 2023 at 12:10 PM Bao D. Nguyen <quic_nguyenb@quicinc.com> wrote:
>>>> Add ufshcd_mcq_abort() to support ufs abort in mcq mode.
>>>>
>>>> Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
>>>> ---
>>>>    drivers/ufs/core/ufs-mcq.c     | 76 ++++++++++++++++++++++++++++++++++++++++++
>>>>    drivers/ufs/core/ufshcd-priv.h |  5 ++-
>>>>    drivers/ufs/core/ufshcd.c      | 11 ++++--
>>>>    3 files changed, 88 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
>>>> index e27d8eb..6c65cd5 100644
>>>> --- a/drivers/ufs/core/ufs-mcq.c
>>>> +++ b/drivers/ufs/core/ufs-mcq.c
>>>> @@ -646,3 +646,79 @@ static bool ufshcd_mcq_cqe_search(struct ufs_hba *hba,
>>>>           spin_unlock(&hwq->cq_lock);
>>>>           return ret;
>>>>    }
>>>> +
>>>> +/**
>>>> + * ufshcd_mcq_abort - Abort the command in MCQ.
>>>> + * @cmd - The command to be aborted.
>>>> + *
>>>> + * Returns SUCCESS or FAILED error codes
>>>> + */
>>>> +int ufshcd_mcq_abort(struct scsi_cmnd *cmd)
>>>> +{
>>>> +       struct Scsi_Host *host = cmd->device->host;
>>>> +       struct ufs_hba *hba = shost_priv(host);
>>>> +       int tag = scsi_cmd_to_rq(cmd)->tag;
>>>> +       struct ufshcd_lrb *lrbp = &hba->lrb[tag];
>>>> +       struct ufs_hw_queue *hwq;
>>>> +       int err = FAILED;
>>>> +
>>>> +       if (!lrbp->cmd) {
>>>> +               dev_err(hba->dev,
>>>> +                       "%s: skip abort. cmd at tag %d already completed.\n",
>>>> +                       __func__, tag);
>>>> +               goto out;
>>>> +       }
>>>> +
>>>> +       /* Skip task abort in case previous aborts failed and report failure */
>>>> +       if (lrbp->req_abort_skip) {
>>>> +               dev_err(hba->dev, "%s: skip abort. tag %d failed earlier\n",
>>>> +                       __func__, tag);
>>>> +               goto out;
>>>> +       }
>>>> +
>>>> +       hwq = ufshcd_mcq_req_to_hwq(hba, scsi_cmd_to_rq(cmd));
>>>> +
>>>> +       if (ufshcd_mcq_sqe_search(hba, hwq, tag)) {
>>>> +               /*
>>>> +                * Failure. The command should not be "stuck" in SQ for
>>>> +                * a long time which resulted in command being aborted.
>>>> +                */
>>>> +               dev_err(hba->dev, "%s: cmd found in sq. hwq=%d, tag=%d\n",
>>>> +                               __func__, hwq->id, tag);
>>>> +               /* Set the Command Type to 0xF per the spec */
>>>> +               ufshcd_mcq_nullify_cmd(hba, hwq);
>>>> +               goto out;
>>>> +       }
>>>> +
>>>> +       if (ufshcd_mcq_cqe_search(hba, hwq, tag)) {
>>>> +               dev_err(hba->dev, "%s: cmd found in cq. hwq=%d, tag=%d\n",
>>>> +                               __func__, hwq->id, tag);
>>>> +               /*
>>>> +                * The command should not be 'stuck' in the CQ for such a long time.
>>>> +                * Is interrupt missing? Process the CQEs here. If the interrupt is
>>>> +                * invoked at a later time, the CQ will be empty because the CQEs
>>>> +                * are already processed here.
>>>> +                */
>>>> +               ufshcd_mcq_poll_cqe_lock(hba, hwq);
>>>> +               err = SUCCESS;
>>>> +               goto out;
>>>> +       }
>>>> +
>>>> +       /*
>>>> +        * The command is not in the Submission Queue, and it is not
>>>> +        * in the Completion Queue either. Query the device to see if
>>>> +        * the command is being processed in the device.
>>>> +        */
>>>> +       if (ufshcd_try_to_abort_task(hba, tag)) {
>>>> +               dev_err(hba->dev, "%s: device abort failed %d\n", __func__, err);
>>>> +               lrbp->req_abort_skip = true;
>>>> +               goto out;
>>>> +       }
>>>> +
>>>> +       err = SUCCESS;
>>>> +       if (lrbp->cmd)
>>>> +               ufshcd_release_scsi_cmd(hba, lrbp);
>>>> +
>>>> +out:
>>>> +       return err;
>>>> +}
>>>> diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h
>>>> index 0527926..d883c03 100644
>>>> --- a/drivers/ufs/core/ufshcd-priv.h
>>>> +++ b/drivers/ufs/core/ufshcd-priv.h
>>>> @@ -77,7 +77,10 @@ unsigned long ufshcd_mcq_poll_cqe_lock(struct ufs_hba *hba,
>>>>                                          struct ufs_hw_queue *hwq);
>>>>
>>>>    int ufshcd_mcq_sq_cleanup(struct ufs_hba *hba, int task_tag, int *result);
>>>> -
>>>> +int ufshcd_mcq_abort(struct scsi_cmnd *cmd);
>>>> +int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag);
>>>> +void ufshcd_release_scsi_cmd(struct ufs_hba *hba,
>>>> +                               struct ufshcd_lrb *lrbp);
>>>>    #define UFSHCD_MCQ_IO_QUEUE_OFFSET     1
>>>>    #define SD_ASCII_STD true
>>>>    #define SD_RAW false
>>>> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
>>>> index 408c16c..e06399e 100644
>>>> --- a/drivers/ufs/core/ufshcd.c
>>>> +++ b/drivers/ufs/core/ufshcd.c
>>>> @@ -302,7 +302,6 @@ static int ufshcd_setup_hba_vreg(struct ufs_hba *hba, bool on);
>>>>    static int ufshcd_setup_vreg(struct ufs_hba *hba, bool on);
>>>>    static inline int ufshcd_config_vreg_hpm(struct ufs_hba *hba,
>>>>                                            struct ufs_vreg *vreg);
>>>> -static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag);
>>>>    static void ufshcd_wb_toggle_buf_flush_during_h8(struct ufs_hba *hba,
>>>>                                                    bool enable);
>>>>    static void ufshcd_hba_vreg_set_lpm(struct ufs_hba *hba);
>>>> @@ -5414,7 +5413,7 @@ static irqreturn_t ufshcd_uic_cmd_compl(struct ufs_hba *hba, u32 intr_status)
>>>>    }
>>>>
>>>>    /* Release the resources allocated for processing a SCSI command. */
>>>> -static void ufshcd_release_scsi_cmd(struct ufs_hba *hba,
>>>> +void ufshcd_release_scsi_cmd(struct ufs_hba *hba,
>>>>                                       struct ufshcd_lrb *lrbp)
>>>>    {
>>>>           struct scsi_cmnd *cmd = lrbp->cmd;
>>>> @@ -7340,7 +7339,7 @@ static void ufshcd_set_req_abort_skip(struct ufs_hba *hba, unsigned long bitmap)
>>>>     *
>>>>     * Returns zero on success, non-zero on failure
>>>>     */
>>>> -static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
>>>> +int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
>>>>    {
>>>>           struct ufshcd_lrb *lrbp = &hba->lrb[tag];
>>>>           int err = 0;
>>>> @@ -7500,6 +7499,12 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
>>>>                   goto release;
>>>>           }
>>>>
>>>> +       if (is_mcq_enabled(hba)) {
>>>> +               /* MCQ mode. Branch off to handle abort for mcq mode */
>>>> +               err = ufshcd_mcq_abort(cmd);
>>>> +               goto release;
>>>> +       }
>>>> +
>>>>           /* Skip task abort in case previous aborts failed and report failure */
>>>>           if (lrbp->req_abort_skip) {
>>>>                   dev_err(hba->dev, "%s: skipping abort\n", __func__);
>>>> --
>>>> 2.7.4
>>>>
>>> It seems that ufshcd_abort_all() also needs an error handling flow for MCQ.
>> Hi Stanley, thank you for the reviews.
>>
>> I am not able to find the function ufshcd_abort_all() in the drivers/ufs
>> directory. Can you please clarify where this function is located? Thanks.
> The function ufshcd_abort_all() was introduced in the following link:
> https://android.googlesource.com/kernel/common/+/b817e6ffbad7a1a0a5ca5bb7d4020823c3f4d9d0

Ah ok. Thank you for this info. The ufshcd_abort_all() is in 
ufshcd_err_handler(), it is unrelated to the ufshcd_abort().

Adding MCQ support to ufshcd_err_handler() will be in a separate patch.

> Can you confirm if these patches are based on the latest kernel
> 6.3-rc1 or the latest linux-next tree?
>
> Thanks,
> Stanley
>
>>> Thanks,
>>> Stanley
>>