[PATCH v2 0/7] Several UFS MCQ Code Changes

[PATCH v2 0/7] Several UFS MCQ Code Changes

[Po-Wen Kao]

v1 -> v2:
- Add 2 new patches
	- Update MTK driver for mcq
	- Export symbols for MTK driver
- Fix commit message in "scsi: ufs: core: Fix mcq nr_hw_queues"
- Split name change and fix into two patches

Po-Wen Kao (7):
  scsi: ufs: core: Fix mcq tag calcualtion
  scsi: ufs: core: Rename symbols
  scsi: ufs: core: Fix mcq nr_hw_queues
  scsi: ufs: core: Add hwq print for debug
  scsi: ufs: core: Remove redundant check
  scsi: ufs: core: Export symbols for MTK driver module
  scsi: ufs: mtk-host: Add MCQ support for MTK platform

 drivers/ufs/core/ufs-mcq.c      |  39 ++++++-
 drivers/ufs/core/ufshcd-priv.h  |  11 +-
 drivers/ufs/core/ufshcd.c       |  31 ++---
 drivers/ufs/host/ufs-mediatek.c | 193 +++++++++++++++++++++++++++++++-
 drivers/ufs/host/ufs-mediatek.h |  33 ++++++
 include/ufs/ufshcd.h            |   8 +-
 include/ufs/ufshci.h            |  12 ++
 7 files changed, 306 insertions(+), 21 deletions(-)

-- 
2.18.0

[PATCH v2 1/7] scsi: ufs: core: Fix mcq tag calcualtion

[Po-Wen Kao]

Transfer command descriptor is allocated in ufshcd_memory_alloc()
and referenced by transfer request descriptor with stride size
sizeof_utp_transfer_cmd_desc()
instead of
sizeof(struct utp_transfer_cmd_desc).

Consequently, computing tag by address offset should also refer to the
same stride.

Signed-off-by: Po-Wen Kao <powen.kao@mediatek.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
Reviewed-by: Stanley Chu <stanley.chu@mediatek.com>
---
 drivers/ufs/core/ufs-mcq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
index 31df052fbc41..3a27fa4b0024 100644
--- a/drivers/ufs/core/ufs-mcq.c
+++ b/drivers/ufs/core/ufs-mcq.c
@@ -265,7 +265,7 @@ static int ufshcd_mcq_get_tag(struct ufs_hba *hba,
 	addr = (le64_to_cpu(cqe->command_desc_base_addr) & CQE_UCD_BA) -
 		hba->ucdl_dma_addr;
 
-	return div_u64(addr, sizeof(struct utp_transfer_cmd_desc));
+	return div_u64(addr, sizeof_utp_transfer_cmd_desc(hba));
 }
 
 static void ufshcd_mcq_process_cqe(struct ufs_hba *hba,
-- 
2.18.0

[PATCH v2 2/7] scsi: ufs: core: Rename symbols

[Po-Wen Kao]

To avoid confusion, sizeof_utp_transfer_cmd_desc() is renamed to
ufshcd_get_ucd_size().

Signed-off-by: Po-Wen Kao <powen.kao@mediatek.com>
---
 drivers/ufs/core/ufs-mcq.c | 2 +-
 drivers/ufs/core/ufshcd.c  | 8 ++++----
 include/ufs/ufshcd.h       | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
index 3a27fa4b0024..a39746b2a8be 100644
--- a/drivers/ufs/core/ufs-mcq.c
+++ b/drivers/ufs/core/ufs-mcq.c
@@ -265,7 +265,7 @@ static int ufshcd_mcq_get_tag(struct ufs_hba *hba,
 	addr = (le64_to_cpu(cqe->command_desc_base_addr) & CQE_UCD_BA) -
 		hba->ucdl_dma_addr;
 
-	return div_u64(addr, sizeof_utp_transfer_cmd_desc(hba));
+	return div_u64(addr, ufshcd_get_ucd_size(hba));
 }
 
 static void ufshcd_mcq_process_cqe(struct ufs_hba *hba,
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 3b3cf78d3b10..81c9f07ebfc8 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -2823,10 +2823,10 @@ static void ufshcd_map_queues(struct Scsi_Host *shost)
 static void ufshcd_init_lrb(struct ufs_hba *hba, struct ufshcd_lrb *lrb, int i)
 {
 	struct utp_transfer_cmd_desc *cmd_descp = (void *)hba->ucdl_base_addr +
-		i * sizeof_utp_transfer_cmd_desc(hba);
+		i * ufshcd_get_ucd_size(hba);
 	struct utp_transfer_req_desc *utrdlp = hba->utrdl_base_addr;
 	dma_addr_t cmd_desc_element_addr = hba->ucdl_dma_addr +
-		i * sizeof_utp_transfer_cmd_desc(hba);
+		i * ufshcd_get_ucd_size(hba);
 	u16 response_offset = offsetof(struct utp_transfer_cmd_desc,
 				       response_upiu);
 	u16 prdt_offset = offsetof(struct utp_transfer_cmd_desc, prd_table);
@@ -3735,7 +3735,7 @@ static int ufshcd_memory_alloc(struct ufs_hba *hba)
 	size_t utmrdl_size, utrdl_size, ucdl_size;
 
 	/* Allocate memory for UTP command descriptors */
-	ucdl_size = sizeof_utp_transfer_cmd_desc(hba) * hba->nutrs;
+	ucdl_size = ufshcd_get_ucd_size(hba) * hba->nutrs;
 	hba->ucdl_base_addr = dmam_alloc_coherent(hba->dev,
 						  ucdl_size,
 						  &hba->ucdl_dma_addr,
@@ -3835,7 +3835,7 @@ static void ufshcd_host_memory_configure(struct ufs_hba *hba)
 	prdt_offset =
 		offsetof(struct utp_transfer_cmd_desc, prd_table);
 
-	cmd_desc_size = sizeof_utp_transfer_cmd_desc(hba);
+	cmd_desc_size = ufshcd_get_ucd_size(hba);
 	cmd_desc_dma_addr = hba->ucdl_dma_addr;
 
 	for (i = 0; i < hba->nutrs; i++) {
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index ed9e3d5addb3..8f79cca449e1 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -1136,7 +1136,7 @@ static inline size_t ufshcd_sg_entry_size(const struct ufs_hba *hba)
 	({ (void)(hba); BUILD_BUG_ON(sg_entry_size != sizeof(struct ufshcd_sg_entry)); })
 #endif
 
-static inline size_t sizeof_utp_transfer_cmd_desc(const struct ufs_hba *hba)
+static inline size_t ufshcd_get_ucd_size(const struct ufs_hba *hba)
 {
 	return sizeof(struct utp_transfer_cmd_desc) + SG_ALL * ufshcd_sg_entry_size(hba);
 }
-- 
2.18.0

[PATCH v2 3/7] scsi: ufs: core: Fix mcq nr_hw_queues

[Po-Wen Kao]

Need to add one to MAXQ to obtain number of hardware queue.

Signed-off-by: Po-Wen Kao <powen.kao@mediatek.com>
---
 drivers/ufs/core/ufs-mcq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
index a39746b2a8be..5d5bc0bc6e88 100644
--- a/drivers/ufs/core/ufs-mcq.c
+++ b/drivers/ufs/core/ufs-mcq.c
@@ -150,7 +150,7 @@ static int ufshcd_mcq_config_nr_queues(struct ufs_hba *hba)
 	u32 hba_maxq, rem, tot_queues;
 	struct Scsi_Host *host = hba->host;
 
-	hba_maxq = FIELD_GET(MAX_QUEUE_SUP, hba->mcq_capabilities);
+	hba_maxq = FIELD_GET(MAX_QUEUE_SUP, hba->mcq_capabilities) + 1 ;
 
 	tot_queues = UFS_MCQ_NUM_DEV_CMD_QUEUES + read_queues + poll_queues +
 			rw_queues;
-- 
2.18.0

[PATCH v2 4/7] scsi: ufs: core: Add hwq print for debug

[Po-Wen Kao]

Introduce hwq printing function for debug purpose.
- ufshcd_mcq_print_hwqs()

Signed-off-by: Po-Wen Kao <powen.kao@mediatek.com>
---
 drivers/ufs/core/ufs-mcq.c     | 32 +++++++++++++++++++++++++++++++-
 drivers/ufs/core/ufshcd-priv.h |  9 +++++++++
 drivers/ufs/core/ufshcd.c      | 18 +++++++++++-------
 include/ufs/ufshci.h           | 12 ++++++++++++
 4 files changed, 63 insertions(+), 8 deletions(-)

diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
index 5d5bc0bc6e88..d1ff3ccd2085 100644
--- a/drivers/ufs/core/ufs-mcq.c
+++ b/drivers/ufs/core/ufs-mcq.c
@@ -23,7 +23,6 @@
 
 #define MAX_DEV_CMD_ENTRIES	2
 #define MCQ_CFG_MAC_MASK	GENMASK(16, 8)
-#define MCQ_QCFG_SIZE		0x40
 #define MCQ_ENTRY_SIZE_IN_DWORD	8
 #define CQE_UCD_BA GENMASK_ULL(63, 7)
 
@@ -75,6 +74,13 @@ module_param_cb(poll_queues, &poll_queue_count_ops, &poll_queues, 0644);
 MODULE_PARM_DESC(poll_queues,
 		 "Number of poll queues used for r/w. Default value is 1");
 
+static const int mcq_opr_size[] = {
+	MCQ_SQD_SIZE,
+	MCQ_SQIS_SIZE,
+	MCQ_CQD_SIZE,
+	MCQ_CQIS_SIZE,
+};
+
 /**
  * ufshcd_mcq_config_mac - Set the #Max Activ Cmds.
  * @hba: per adapter instance
@@ -237,6 +243,30 @@ static void __iomem *mcq_opr_base(struct ufs_hba *hba,
 	return opr->base + opr->stride * i;
 }
 
+void ufshcd_mcq_print_hwqs(struct ufs_hba *hba, unsigned long bitmap)
+{
+	int id, i;
+	char prefix[15];
+
+	if (!is_mcq_enabled(hba))
+		return;
+
+	for_each_set_bit(id, &bitmap, hba->nr_hw_queues) {
+		snprintf(prefix, sizeof(prefix), "q%d SQCFG: ", id);
+		ufshcd_hex_dump(prefix,
+			hba->mcq_base + MCQ_QCFG_SIZE * id, MCQ_QCFG_SQ_SIZE);
+
+		snprintf(prefix, sizeof(prefix), "q%d CQCFG: ", id);
+		ufshcd_hex_dump(prefix,
+			hba->mcq_base + MCQ_QCFG_SIZE * id + MCQ_QCFG_SQ_SIZE, MCQ_QCFG_CQ_SIZE);
+
+		for (i = 0; i < OPR_MAX ; i++) {
+			snprintf(prefix, sizeof(prefix), "q%d OPR%d: ", id, i);
+			ufshcd_hex_dump(prefix, mcq_opr_base(hba, i, id), mcq_opr_size[i]);
+		}
+	}
+}
+
 u32 ufshcd_mcq_read_cqis(struct ufs_hba *hba, int i)
 {
 	return readl(mcq_opr_base(hba, OPR_CQIS, i) + REG_CQIS);
diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h
index 529f8507a5e4..13534a9a6d0a 100644
--- a/drivers/ufs/core/ufshcd-priv.h
+++ b/drivers/ufs/core/ufshcd-priv.h
@@ -6,6 +6,14 @@
 #include <linux/pm_runtime.h>
 #include <ufs/ufshcd.h>
 
+#define ufshcd_hex_dump(prefix_str, buf, len) do {                       \
+	size_t __len = (len);                                            \
+	print_hex_dump(KERN_ERR, prefix_str,                             \
+		       __len > 4 ? DUMP_PREFIX_OFFSET : DUMP_PREFIX_NONE,\
+		       16, 4, buf, __len, false);                        \
+} while (0)
+
+
 static inline bool ufshcd_is_user_access_allowed(struct ufs_hba *hba)
 {
 	return !hba->shutting_down;
@@ -65,6 +73,7 @@ void ufshcd_compl_one_cqe(struct ufs_hba *hba, int task_tag,
 			  struct cq_entry *cqe);
 int ufshcd_mcq_init(struct ufs_hba *hba);
 int ufshcd_mcq_decide_queue_depth(struct ufs_hba *hba);
+void ufshcd_mcq_print_hwqs(struct ufs_hba *hba, unsigned long bitmap);
 int ufshcd_mcq_memory_alloc(struct ufs_hba *hba);
 void ufshcd_mcq_make_queues_operational(struct ufs_hba *hba);
 void ufshcd_mcq_config_mac(struct ufs_hba *hba, u32 max_active_cmds);
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 81c9f07ebfc8..a15a5a78160c 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -135,13 +135,6 @@ MODULE_PARM_DESC(use_mcq_mode, "Control MCQ mode for controllers starting from U
 		_ret;                                                   \
 	})
 
-#define ufshcd_hex_dump(prefix_str, buf, len) do {                       \
-	size_t __len = (len);                                            \
-	print_hex_dump(KERN_ERR, prefix_str,                             \
-		       __len > 4 ? DUMP_PREFIX_OFFSET : DUMP_PREFIX_NONE,\
-		       16, 4, buf, __len, false);                        \
-} while (0)
-
 int ufshcd_dump_regs(struct ufs_hba *hba, size_t offset, size_t len,
 		     const char *prefix)
 {
@@ -536,6 +529,8 @@ static
 void ufshcd_print_trs(struct ufs_hba *hba, unsigned long bitmap, bool pr_prdt)
 {
 	const struct ufshcd_lrb *lrbp;
+	struct ufs_hw_queue *hwq;
+	struct scsi_cmnd *cmd;
 	int prdt_length;
 	int tag;
 
@@ -574,7 +569,16 @@ void ufshcd_print_trs(struct ufs_hba *hba, unsigned long bitmap, bool pr_prdt)
 		if (pr_prdt)
 			ufshcd_hex_dump("UPIU PRDT: ", lrbp->ucd_prdt_ptr,
 				ufshcd_sg_entry_size(hba) * prdt_length);
+
+		if (is_mcq_enabled(hba)) {
+			cmd = lrbp->cmd;
+			if (!cmd)
+				return;
+			hwq = ufshcd_mcq_req_to_hwq(hba, scsi_cmd_to_rq(cmd));
+			ufshcd_mcq_print_hwqs(hba, 1 << hwq->id);
+		}
 	}
+
 }
 
 static void ufshcd_print_tmrs(struct ufs_hba *hba, unsigned long bitmap)
diff --git a/include/ufs/ufshci.h b/include/ufs/ufshci.h
index 11424bb03814..027a2e884f89 100644
--- a/include/ufs/ufshci.h
+++ b/include/ufs/ufshci.h
@@ -185,6 +185,18 @@ static inline u32 ufshci_version(u32 major, u32 minor)
 				CRYPTO_ENGINE_FATAL_ERROR |\
 				UIC_LINK_LOST)
 
+/* MCQ size */
+#define MCQ_QCFG_SIZE			0x40
+#define MCQ_QCFG_SQ_SIZE		0x20
+#define MCQ_QCFG_CQ_SIZE		0x20
+
+enum {
+	MCQ_SQD_SIZE		= 0x14,
+	MCQ_SQIS_SIZE		= 0x08,
+	MCQ_CQD_SIZE		= 0x08,
+	MCQ_CQIS_SIZE		= 0x0C,
+};
+
 /* HCS - Host Controller Status 30h */
 #define DEVICE_PRESENT				0x1
 #define UTP_TRANSFER_REQ_LIST_READY		0x2
-- 
2.18.0

[PATCH v2 5/7] scsi: ufs: core: Remove redundant check

[Po-Wen Kao]

is_mcq_supported() already check on use_mcq_mode.

Signed-off-by: Po-Wen Kao <powen.kao@mediatek.com>
---
 drivers/ufs/core/ufshcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index a15a5a78160c..21e3bf5d8f08 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -8548,7 +8548,7 @@ static int ufshcd_device_init(struct ufs_hba *hba, bool init_dev_params)
 			hba->scsi_host_added = true;
 		}
 		/* MCQ may be disabled if ufshcd_alloc_mcq() fails */
-		if (is_mcq_supported(hba) && use_mcq_mode)
+		if (is_mcq_supported(hba))
 			ufshcd_config_mcq(hba);
 	}
 
-- 
2.18.0

[PATCH v2 6/7] scsi: ufs: core: Export symbols for MTK driver module

[Po-Wen Kao]

Export
- ufshcd_mcq_config_mac
- ufshcd_mcq_make_queues_operational
- ufshcd_mcq_read_cqis
- ufshcd_disable_intr

Signed-off-by: Po-Wen Kao <powen.kao@mediatek.com>
---
 drivers/ufs/core/ufs-mcq.c     | 3 +++
 drivers/ufs/core/ufshcd-priv.h | 2 --
 drivers/ufs/core/ufshcd.c      | 3 ++-
 include/ufs/ufshcd.h           | 6 ++++++
 4 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
index d1ff3ccd2085..ae67ab90bd29 100644
--- a/drivers/ufs/core/ufs-mcq.c
+++ b/drivers/ufs/core/ufs-mcq.c
@@ -98,6 +98,7 @@ void ufshcd_mcq_config_mac(struct ufs_hba *hba, u32 max_active_cmds)
 	val |= FIELD_PREP(MCQ_CFG_MAC_MASK, max_active_cmds);
 	ufshcd_writel(hba, val, REG_UFS_MCQ_CFG);
 }
+EXPORT_SYMBOL_GPL(ufshcd_mcq_config_mac);
 
 /**
  * ufshcd_mcq_req_to_hwq - find the hardware queue on which the
@@ -271,6 +272,7 @@ u32 ufshcd_mcq_read_cqis(struct ufs_hba *hba, int i)
 {
 	return readl(mcq_opr_base(hba, OPR_CQIS, i) + REG_CQIS);
 }
+EXPORT_SYMBOL_GPL(ufshcd_mcq_read_cqis);
 
 void ufshcd_mcq_write_cqis(struct ufs_hba *hba, u32 val, int i)
 {
@@ -401,6 +403,7 @@ void ufshcd_mcq_make_queues_operational(struct ufs_hba *hba)
 			      MCQ_CFG_n(REG_SQATTR, i));
 	}
 }
+EXPORT_SYMBOL_GPL(ufshcd_mcq_make_queues_operational);
 
 void ufshcd_mcq_enable_esi(struct ufs_hba *hba)
 {
diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h
index 13534a9a6d0a..1c83a6bc88b7 100644
--- a/drivers/ufs/core/ufshcd-priv.h
+++ b/drivers/ufs/core/ufshcd-priv.h
@@ -75,8 +75,6 @@ int ufshcd_mcq_init(struct ufs_hba *hba);
 int ufshcd_mcq_decide_queue_depth(struct ufs_hba *hba);
 void ufshcd_mcq_print_hwqs(struct ufs_hba *hba, unsigned long bitmap);
 int ufshcd_mcq_memory_alloc(struct ufs_hba *hba);
-void ufshcd_mcq_make_queues_operational(struct ufs_hba *hba);
-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);
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 21e3bf5d8f08..a0848a8e2e6f 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -2567,7 +2567,7 @@ static void ufshcd_enable_intr(struct ufs_hba *hba, u32 intrs)
  * @hba: per adapter instance
  * @intrs: interrupt bits
  */
-static void ufshcd_disable_intr(struct ufs_hba *hba, u32 intrs)
+void ufshcd_disable_intr(struct ufs_hba *hba, u32 intrs)
 {
 	u32 set = ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
 
@@ -2583,6 +2583,7 @@ static void ufshcd_disable_intr(struct ufs_hba *hba, u32 intrs)
 
 	ufshcd_writel(hba, set, REG_INTERRUPT_ENABLE);
 }
+EXPORT_SYMBOL_GPL(ufshcd_disable_intr);
 
 /**
  * ufshcd_prepare_req_desc_hdr - Fill UTP Transfer request descriptor header according to request
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index 8f79cca449e1..d4dc7bcec127 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -1230,6 +1230,7 @@ static inline void ufshcd_rmwl(struct ufs_hba *hba, u32 mask, u32 val, u32 reg)
 
 int ufshcd_alloc_host(struct device *, struct ufs_hba **);
 void ufshcd_dealloc_host(struct ufs_hba *);
+void ufshcd_disable_intr(struct ufs_hba *hba, u32 intrs);
 int ufshcd_hba_enable(struct ufs_hba *hba);
 int ufshcd_init(struct ufs_hba *, void __iomem *, unsigned int);
 int ufshcd_link_recovery(struct ufs_hba *hba);
@@ -1242,9 +1243,14 @@ void ufshcd_parse_dev_ref_clk_freq(struct ufs_hba *hba, struct clk *refclk);
 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_config_mac(struct ufs_hba *hba, u32 max_active_cmds);
+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);
+void ufshcd_mcq_make_queues_operational(struct ufs_hba *hba);
 void ufshcd_mcq_enable_esi(struct ufs_hba *hba);
 void ufshcd_mcq_config_esi(struct ufs_hba *hba, struct msi_msg *msg);
 
-- 
2.18.0

[PATCH v2 7/7] scsi: ufs: mtk-host: Add MCQ support for MTK platform

[Po-Wen Kao]

Changes
- Implement vops and setup irq
- Fix pm flow under mcq mode

Signed-off-by: Po-Wen Kao <powen.kao@mediatek.com>
---
 drivers/ufs/host/ufs-mediatek.c | 193 +++++++++++++++++++++++++++++++-
 drivers/ufs/host/ufs-mediatek.h |  33 ++++++
 2 files changed, 224 insertions(+), 2 deletions(-)

diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c
index 21d9b047539f..162bbde675aa 100644
--- a/drivers/ufs/host/ufs-mediatek.c
+++ b/drivers/ufs/host/ufs-mediatek.c
@@ -30,6 +30,14 @@
 #define CREATE_TRACE_POINTS
 #include "ufs-mediatek-trace.h"
 
+static int  ufs_mtk_config_mcq(struct ufs_hba *hba, bool irq);
+static void ufs_mtk_dbg_register_dump(struct ufs_hba *hba);
+
+#define MAX_SUPP_MAC 64
+#define MCQ_QUEUE_OFFSET(c) ((((c) >> 16) & 0xFF) * 0x200)
+
+static unsigned int mtk_mcq_irq[UFSHCD_MAX_Q_NR];
+
 static const struct ufs_dev_quirk ufs_mtk_dev_fixups[] = {
 	{ .wmanufacturerid = UFS_ANY_VENDOR,
 	  .model = UFS_ANY_MODEL,
@@ -843,6 +851,19 @@ static void ufs_mtk_vreg_fix_vccqx(struct ufs_hba *hba)
 	}
 }
 
+static void ufs_mtk_init_interrupt(struct ufs_hba *hba)
+{
+	struct ufs_mtk_host *host = ufshcd_get_variant(hba);
+	int i;
+
+	host->mcq_nr_intr = UFSHCD_MAX_Q_NR;
+
+	for (i = 0; i < host->mcq_nr_intr; i++) {
+		host->mcq_intr_info[i].hba = hba;
+		host->mcq_intr_info[i].intr = mtk_mcq_irq[i];
+	}
+}
+
 /**
  * ufs_mtk_init - find other essential mmio bases
  * @hba: host controller instance
@@ -879,6 +900,8 @@ static int ufs_mtk_init(struct ufs_hba *hba)
 	/* Initialize host capability */
 	ufs_mtk_init_host_caps(hba);
 
+	ufs_mtk_init_interrupt(hba);
+
 	err = ufs_mtk_bind_mphy(hba);
 	if (err)
 		goto out_variant_clear;
@@ -1174,7 +1197,16 @@ static int ufs_mtk_link_set_hpm(struct ufs_hba *hba)
 	else
 		return err;
 
-	err = ufshcd_make_hba_operational(hba);
+	if (!hba->mcq_enabled)
+		err = ufshcd_make_hba_operational(hba);
+	else {
+		ufs_mtk_config_mcq(hba, false);
+		ufshcd_mcq_make_queues_operational(hba);
+		ufshcd_mcq_config_mac(hba, hba->nutrs);
+		ufshcd_writel(hba, ufshcd_readl(hba, REG_UFS_MEM_CFG) | 0x1,
+			REG_UFS_MEM_CFG);
+	}
+
 	if (err)
 		return err;
 
@@ -1361,6 +1393,12 @@ static int ufs_mtk_apply_dev_quirks(struct ufs_hba *hba)
 		ufshcd_dme_set(hba, UIC_ARG_MIB(PA_HIBERN8TIME), 10);
 	}
 
+	/*
+	 * Disable MCQ_CQ_EVENT interrupt.
+	 * Use CQ Tail Entry Push Status instead.
+	 */
+	ufshcd_disable_intr(hba, MCQ_CQ_EVENT_STATUS);
+
 	/*
 	 * Decide waiting time before gating reference clock and
 	 * after ungating reference clock according to vendors'
@@ -1498,6 +1536,116 @@ static int ufs_mtk_clk_scale_notify(struct ufs_hba *hba, bool scale_up,
 	return 0;
 }
 
+static int ufs_mtk_get_hba_mac(struct ufs_hba *hba)
+{
+	return MAX_SUPP_MAC;
+}
+
+static int ufs_mtk_op_runtime_config(struct ufs_hba *hba)
+{
+	struct ufshcd_mcq_opr_info_t *opr;
+	int i;
+
+	for (i = 0; i < OPR_MAX; i++) {
+		opr = &hba->mcq_opr[i];
+		opr->stride = REG_UFS_MCQ_STRIDE;
+	}
+
+	hba->mcq_opr[OPR_SQD].offset = REG_UFS_MTK_SQD;
+	hba->mcq_opr[OPR_SQIS].offset = REG_UFS_MTK_SQIS;
+	hba->mcq_opr[OPR_CQD].offset = REG_UFS_MTK_CQD;
+	hba->mcq_opr[OPR_CQIS].offset = REG_UFS_MTK_CQIS;
+
+	hba->mcq_opr[OPR_SQD].base = hba->mmio_base + REG_UFS_MTK_SQD;
+	hba->mcq_opr[OPR_SQIS].base = hba->mmio_base + REG_UFS_MTK_SQIS;
+	hba->mcq_opr[OPR_CQD].base = hba->mmio_base + REG_UFS_MTK_CQD;
+	hba->mcq_opr[OPR_CQIS].base = hba->mmio_base + REG_UFS_MTK_CQIS;
+
+	return 0;
+}
+
+static int ufs_mtk_mcq_config_resource(struct ufs_hba *hba)
+{
+	hba->mcq_base = hba->mmio_base + MCQ_QUEUE_OFFSET(hba->mcq_capabilities);
+	return 0;
+}
+
+static irqreturn_t ufs_mtk_mcq_intr(int irq, void *__intr_info)
+{
+	struct ufs_mtk_mcq_intr_info *mcq_intr_info = __intr_info;
+	struct ufs_hba *hba = mcq_intr_info->hba;
+	struct ufs_hw_queue *hwq;
+	u32 events;
+	int i = mcq_intr_info->qid;
+
+	hwq = &hba->uhq[i];
+
+	events = ufshcd_mcq_read_cqis(hba, i);
+	if (events)
+		ufshcd_mcq_write_cqis(hba, events, i);
+
+	if (events & UFSHCD_MCQ_CQIS_TAIL_ENT_PUSH_STS)
+		ufshcd_mcq_poll_cqe_nolock(hba, hwq);
+
+	return IRQ_HANDLED;
+}
+
+static int ufs_mtk_config_mcq_irq(struct ufs_hba *hba)
+{
+	struct ufs_mtk_host *host = ufshcd_get_variant(hba);
+	u32 irq, i;
+	int ret;
+
+	for (i = 0; i < host->mcq_nr_intr; i++) {
+		irq = host->mcq_intr_info[i].intr;
+		if (irq == MTK_MCQ_INVALID_IRQ) {
+			dev_err(hba->dev, "invalid irq. %d\n", i);
+			return -ENOPARAM;
+		}
+
+		host->mcq_intr_info[i].qid = i;
+		ret = devm_request_irq(hba->dev, irq, ufs_mtk_mcq_intr, 0, UFSHCD,
+									&host->mcq_intr_info[i]);
+
+		dev_info(hba->dev, "request irq %d intr %s\n", irq, ret ? "failed": "");
+
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int ufs_mtk_config_mcq(struct ufs_hba *hba, bool irq)
+{
+	struct ufs_mtk_host *host = ufshcd_get_variant(hba);
+	int ret = 0;
+
+	if (!host->mcq_set_intr) {
+
+		/* Disable irq option register */
+		ufshcd_rmwl(hba, MCQ_INTR_EN_MSK, 0, REG_UFS_MMIO_OPT_CTRL_0);
+
+		if (irq)
+			ret = ufs_mtk_config_mcq_irq(hba);
+
+		if (ret)
+			return ret;
+
+		host->mcq_set_intr = true;
+	}
+
+	ufshcd_rmwl(hba, MCQ_AH8, MCQ_AH8, REG_UFS_MMIO_OPT_CTRL_0);
+	ufshcd_rmwl(hba, MCQ_INTR_EN_MSK, MCQ_MULTI_INTR_EN, REG_UFS_MMIO_OPT_CTRL_0);
+
+	return 0;
+}
+
+static int ufs_mtk_config_esi(struct ufs_hba *hba)
+{
+	return ufs_mtk_config_mcq(hba, true);
+}
+
 /*
  * struct ufs_hba_mtk_vops - UFS MTK specific variant operations
  *
@@ -1521,8 +1669,43 @@ static const struct ufs_hba_variant_ops ufs_hba_mtk_vops = {
 	.event_notify        = ufs_mtk_event_notify,
 	.config_scaling_param = ufs_mtk_config_scaling_param,
 	.clk_scale_notify    = ufs_mtk_clk_scale_notify,
+	/* mcq vops */
+	.get_hba_mac         = ufs_mtk_get_hba_mac,
+	.op_runtime_config   = ufs_mtk_op_runtime_config,
+	.mcq_config_resource = ufs_mtk_mcq_config_resource,
+	.config_esi          = ufs_mtk_config_esi,
 };
 
+static int ufs_mtk_mcq_get_irq(struct platform_device *pdev)
+{
+	int i, irq, cnt;
+
+	for (i = 0; i < UFSHCD_MAX_Q_NR; i++)
+		mtk_mcq_irq[i] = MTK_MCQ_INVALID_IRQ;
+
+	cnt = platform_irq_count(pdev);
+
+	if (cnt < 0)
+		return cnt;
+
+	/* no irq for mcq */
+	if (cnt == 1)
+		return 0;
+
+	for (i = 0; i < UFSHCD_MAX_Q_NR; i++) {
+		/* irq index 0 is ufshcd system irq, sq, cq irq start from index 1 */
+		irq = platform_get_irq(pdev, i + 1);
+		if (irq < 0) {
+			dev_err(&pdev->dev, "get platform mcq irq fail: %d\n", i);
+			return irq;
+		}
+		mtk_mcq_irq[i] = irq;
+		dev_info(&pdev->dev, "get platform mcq irq: %d, %d\n", i, irq);
+	}
+
+	return 0;
+}
+
 /**
  * ufs_mtk_probe - probe routine of the driver
  * @pdev: pointer to Platform device handle
@@ -1562,12 +1745,18 @@ static int ufs_mtk_probe(struct platform_device *pdev)
 	}
 
 skip_reset:
+	err = ufs_mtk_mcq_get_irq(pdev);
+	if (err) {
+		dev_err(dev, "get irq failed %d\n", err);
+		goto out;
+	}
+
 	/* perform generic probe */
 	err = ufshcd_pltfrm_init(pdev, &ufs_hba_mtk_vops);
 
 out:
 	if (err)
-		dev_info(dev, "probe failed %d\n", err);
+		dev_err(dev, "probe failed %d\n", err);
 
 	of_node_put(reset_node);
 	return err;
diff --git a/drivers/ufs/host/ufs-mediatek.h b/drivers/ufs/host/ufs-mediatek.h
index 2fc6d7b87694..542b4c3a763c 100644
--- a/drivers/ufs/host/ufs-mediatek.h
+++ b/drivers/ufs/host/ufs-mediatek.h
@@ -10,11 +10,27 @@
 #include <linux/pm_qos.h>
 #include <linux/soc/mediatek/mtk_sip_svc.h>
 
+/*
+ * MCQ define and struct
+ */
+#define UFSHCD_MAX_Q_NR 8
+#define MTK_MCQ_INVALID_IRQ	0xFFFF
+
+/* REG_UFS_MMIO_OPT_CTRL_0 160h */
+#define EHS_EN                  0x1
+#define PFM_IMPV                0x2
+#define MCQ_MULTI_INTR_EN       0x4
+#define MCQ_CMB_INTR_EN         0x8
+#define MCQ_AH8                 0x10
+
+#define MCQ_INTR_EN_MSK         (MCQ_MULTI_INTR_EN | MCQ_CMB_INTR_EN)
+
 /*
  * Vendor specific UFSHCI Registers
  */
 #define REG_UFS_XOUFS_CTRL          0x140
 #define REG_UFS_REFCLK_CTRL         0x144
+#define REG_UFS_MMIO_OPT_CTRL_0     0x160
 #define REG_UFS_EXTREG              0x2100
 #define REG_UFS_MPHYCTRL            0x2200
 #define REG_UFS_MTK_IP_VER          0x2240
@@ -26,6 +42,13 @@
 #define REG_UFS_DEBUG_SEL_B2        0x22D8
 #define REG_UFS_DEBUG_SEL_B3        0x22DC
 
+#define REG_UFS_MTK_SQD             0x2800
+#define REG_UFS_MTK_SQIS            0x2814
+#define REG_UFS_MTK_CQD             0x281C
+#define REG_UFS_MTK_CQIS            0x2824
+
+#define REG_UFS_MCQ_STRIDE          0x30
+
 /*
  * Ref-clk control
  *
@@ -136,6 +159,12 @@ struct ufs_mtk_hw_ver {
 	u8 major;
 };
 
+struct ufs_mtk_mcq_intr_info {
+	struct ufs_hba *hba;
+	u32 intr;
+	u8 qid;
+};
+
 struct ufs_mtk_host {
 	struct phy *mphy;
 	struct pm_qos_request pm_qos_req;
@@ -155,6 +184,10 @@ struct ufs_mtk_host {
 	u16 ref_clk_ungating_wait_us;
 	u16 ref_clk_gating_wait_us;
 	u32 ip_ver;
+
+	bool mcq_set_intr;
+	int mcq_nr_intr;
+	struct ufs_mtk_mcq_intr_info mcq_intr_info[UFSHCD_MAX_Q_NR];
 };
 
 /*
-- 
2.18.0

Re: [PATCH v2 2/7] scsi: ufs: core: Rename symbols

[Stanley Chu]

On Wed, Feb 22, 2023 at 11:05 AM Po-Wen Kao <powen.kao@mediatek.com> wrote:
>
> To avoid confusion, sizeof_utp_transfer_cmd_desc() is renamed to
> ufshcd_get_ucd_size().
>
> Signed-off-by: Po-Wen Kao <powen.kao@mediatek.com>

Reviewed-by: Stanley Chu <stanley.chu@mediatek.com>

Re: [PATCH v2 3/7] scsi: ufs: core: Fix mcq nr_hw_queues

[Stanley Chu]

On Wed, Feb 22, 2023 at 11:05 AM Po-Wen Kao <powen.kao@mediatek.com> wrote:
>
> Need to add one to MAXQ to obtain number of hardware queue.
>
> Signed-off-by: Po-Wen Kao <powen.kao@mediatek.com>

Reviewed-by: Stanley Chu <stanley.chu@mediatek.com>

Re: [PATCH v2 5/7] scsi: ufs: core: Remove redundant check

[Stanley Chu]

On Wed, Feb 22, 2023 at 11:05 AM Po-Wen Kao <powen.kao@mediatek.com> wrote:
>
> is_mcq_supported() already check on use_mcq_mode.
>
> Signed-off-by: Po-Wen Kao <powen.kao@mediatek.com>

Reviewed-by: Stanley Chu <stanley.chu@mediatek.com>

Re: [PATCH v2 6/7] scsi: ufs: core: Export symbols for MTK driver module

[Stanley Chu]

On Wed, Feb 22, 2023 at 11:05 AM Po-Wen Kao <powen.kao@mediatek.com> wrote:
>
> Export
> - ufshcd_mcq_config_mac
> - ufshcd_mcq_make_queues_operational
> - ufshcd_mcq_read_cqis
> - ufshcd_disable_intr
>
> Signed-off-by: Po-Wen Kao <powen.kao@mediatek.com>
> ---
>  drivers/ufs/core/ufs-mcq.c     | 3 +++
>  drivers/ufs/core/ufshcd-priv.h | 2 --
>  drivers/ufs/core/ufshcd.c      | 3 ++-
>  include/ufs/ufshcd.h           | 6 ++++++
>  4 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
> index d1ff3ccd2085..ae67ab90bd29 100644
> --- a/drivers/ufs/core/ufs-mcq.c
> +++ b/drivers/ufs/core/ufs-mcq.c
> @@ -98,6 +98,7 @@ void ufshcd_mcq_config_mac(struct ufs_hba *hba, u32 max_active_cmds)
>         val |= FIELD_PREP(MCQ_CFG_MAC_MASK, max_active_cmds);
>         ufshcd_writel(hba, val, REG_UFS_MCQ_CFG);
>  }
> +EXPORT_SYMBOL_GPL(ufshcd_mcq_config_mac);
>
>  /**
>   * ufshcd_mcq_req_to_hwq - find the hardware queue on which the
> @@ -271,6 +272,7 @@ u32 ufshcd_mcq_read_cqis(struct ufs_hba *hba, int i)
>  {
>         return readl(mcq_opr_base(hba, OPR_CQIS, i) + REG_CQIS);
>  }
> +EXPORT_SYMBOL_GPL(ufshcd_mcq_read_cqis);
>
>  void ufshcd_mcq_write_cqis(struct ufs_hba *hba, u32 val, int i)
>  {
> @@ -401,6 +403,7 @@ void ufshcd_mcq_make_queues_operational(struct ufs_hba *hba)
>                               MCQ_CFG_n(REG_SQATTR, i));
>         }
>  }
> +EXPORT_SYMBOL_GPL(ufshcd_mcq_make_queues_operational);
>
>  void ufshcd_mcq_enable_esi(struct ufs_hba *hba)
>  {
> diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h
> index 13534a9a6d0a..1c83a6bc88b7 100644
> --- a/drivers/ufs/core/ufshcd-priv.h
> +++ b/drivers/ufs/core/ufshcd-priv.h
> @@ -75,8 +75,6 @@ int ufshcd_mcq_init(struct ufs_hba *hba);
>  int ufshcd_mcq_decide_queue_depth(struct ufs_hba *hba);
>  void ufshcd_mcq_print_hwqs(struct ufs_hba *hba, unsigned long bitmap);
>  int ufshcd_mcq_memory_alloc(struct ufs_hba *hba);
> -void ufshcd_mcq_make_queues_operational(struct ufs_hba *hba);
> -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);
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 21e3bf5d8f08..a0848a8e2e6f 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -2567,7 +2567,7 @@ static void ufshcd_enable_intr(struct ufs_hba *hba, u32 intrs)
>   * @hba: per adapter instance
>   * @intrs: interrupt bits
>   */
> -static void ufshcd_disable_intr(struct ufs_hba *hba, u32 intrs)
> +void ufshcd_disable_intr(struct ufs_hba *hba, u32 intrs)
>  {
>         u32 set = ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
>
> @@ -2583,6 +2583,7 @@ static void ufshcd_disable_intr(struct ufs_hba *hba, u32 intrs)
>
>         ufshcd_writel(hba, set, REG_INTERRUPT_ENABLE);
>  }
> +EXPORT_SYMBOL_GPL(ufshcd_disable_intr);
>
>  /**
>   * ufshcd_prepare_req_desc_hdr - Fill UTP Transfer request descriptor header according to request
> diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
> index 8f79cca449e1..d4dc7bcec127 100644
> --- a/include/ufs/ufshcd.h
> +++ b/include/ufs/ufshcd.h
> @@ -1230,6 +1230,7 @@ static inline void ufshcd_rmwl(struct ufs_hba *hba, u32 mask, u32 val, u32 reg)
>
>  int ufshcd_alloc_host(struct device *, struct ufs_hba **);
>  void ufshcd_dealloc_host(struct ufs_hba *);
> +void ufshcd_disable_intr(struct ufs_hba *hba, u32 intrs);
>  int ufshcd_hba_enable(struct ufs_hba *hba);
>  int ufshcd_init(struct ufs_hba *, void __iomem *, unsigned int);
>  int ufshcd_link_recovery(struct ufs_hba *hba);
> @@ -1242,9 +1243,14 @@ void ufshcd_parse_dev_ref_clk_freq(struct ufs_hba *hba, struct clk *refclk);
>  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_config_mac(struct ufs_hba *hba, u32 max_active_cmds);
> +u32 ufshcd_mcq_read_cqis(struct ufs_hba *hba, int i);
>  void ufshcd_mcq_write_cqis(struct ufs_hba *hba, u32 val, int i);
> +

Dummy empty line?

>  unsigned long ufshcd_mcq_poll_cqe_nolock(struct ufs_hba *hba,
>                                          struct ufs_hw_queue *hwq);
> +void ufshcd_mcq_make_queues_operational(struct ufs_hba *hba);
>  void ufshcd_mcq_enable_esi(struct ufs_hba *hba);
>  void ufshcd_mcq_config_esi(struct ufs_hba *hba, struct msi_msg *msg);
>
> --
> 2.18.0
>

Except for the above nickpicking, others look good to me.

Reviewed-by: Stanley Chu <stanley.chu@mediatek.com>

Re: [PATCH v2 4/7] scsi: ufs: core: Add hwq print for debug

[Ziqi Chen]

Hi Po-Wen,

On 2/22/2023 11:04 AM, Po-Wen Kao wrote:
> +void ufshcd_mcq_print_hwqs(struct ufs_hba *hba, unsigned long bitmap)
> +{
> +	int id, i;
> +	char prefix[15];
> +
> +	if (!is_mcq_enabled(hba))
> +		return;
> +
> +	for_each_set_bit(id, &bitmap, hba->nr_hw_queues) {
> +		snprintf(prefix, sizeof(prefix), "q%d SQCFG: ", id);
> +		ufshcd_hex_dump(prefix,
> +			hba->mcq_base + MCQ_QCFG_SIZE * id, MCQ_QCFG_SQ_SIZE);

Is your purpose dump per hardware queue registers here?  If yes, why 
don't use ufsmcq_readl() to save to a buffer and then use ufshcd_hex_dump()

to dump ? Are you sure ufshcd_hex_dump() can dump register directly?

> +
> +		snprintf(prefix, sizeof(prefix), "q%d CQCFG: ", id);
> +		ufshcd_hex_dump(prefix,
> +			hba->mcq_base + MCQ_QCFG_SIZE * id + MCQ_QCFG_SQ_SIZE, MCQ_QCFG_CQ_SIZE);
Same to above comment.
> +
> +		for (i = 0; i < OPR_MAX ; i++) {
> +			snprintf(prefix, sizeof(prefix), "q%d OPR%d: ", id, i);
> +			ufshcd_hex_dump(prefix, mcq_opr_base(hba, i, id), mcq_opr_size[i]);
Same.
> +		}
> +	}
> +}
> +
>
>   
> @@ -574,7 +569,16 @@ void ufshcd_print_trs(struct ufs_hba *hba, unsigned long bitmap, bool pr_prdt)
>   		if (pr_prdt)
>   			ufshcd_hex_dump("UPIU PRDT: ", lrbp->ucd_prdt_ptr,
>   				ufshcd_sg_entry_size(hba) * prdt_length);
> +
> +		if (is_mcq_enabled(hba)) {
> +			cmd = lrbp->cmd;
> +			if (!cmd)
> +				return;
> +			hwq = ufshcd_mcq_req_to_hwq(hba, scsi_cmd_to_rq(cmd));
> +			ufshcd_mcq_print_hwqs(hba, 1 << hwq->id);

Calling registers dump function in ufshcd_print_trs() is not reasonable, 
eg.. for each aborted request, it would print out all hwq registers, 
it's not make sense.

I think we should move it out of ufshcd_print_trs().

> +		}
>   	}
> +
>   }


Best Regards,

Ziqi

Re: [PATCH v2 1/7] scsi: ufs: core: Fix mcq tag calcualtion

[Ziqi Chen]

On 2/22/2023 11:04 AM, Po-Wen Kao wrote:
> Transfer command descriptor is allocated in ufshcd_memory_alloc()
> and referenced by transfer request descriptor with stride size
> sizeof_utp_transfer_cmd_desc()
> instead of
> sizeof(struct utp_transfer_cmd_desc).
>
> Consequently, computing tag by address offset should also refer to the
> same stride.
>
> Signed-off-by: Po-Wen Kao <powen.kao@mediatek.com>
> Reviewed-by: Bart Van Assche <bvanassche@acm.org>
> Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
> Reviewed-by: Stanley Chu <stanley.chu@mediatek.com>
reviewed-by:  Ziqi Chen <quic_ziqichen@quicinc.com>
> ---
>   drivers/ufs/core/ufs-mcq.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
> index 31df052fbc41..3a27fa4b0024 100644
> --- a/drivers/ufs/core/ufs-mcq.c
> +++ b/drivers/ufs/core/ufs-mcq.c
> @@ -265,7 +265,7 @@ static int ufshcd_mcq_get_tag(struct ufs_hba *hba,
>   	addr = (le64_to_cpu(cqe->command_desc_base_addr) & CQE_UCD_BA) -
>   		hba->ucdl_dma_addr;
>   
> -	return div_u64(addr, sizeof(struct utp_transfer_cmd_desc));
> +	return div_u64(addr, sizeof_utp_transfer_cmd_desc(hba));
>   }
>   
>   static void ufshcd_mcq_process_cqe(struct ufs_hba *hba,

Re: [PATCH v2 2/7] scsi: ufs: core: Rename symbols

[Ziqi Chen]

On 2/22/2023 11:04 AM, Po-Wen Kao wrote:
> To avoid confusion, sizeof_utp_transfer_cmd_desc() is renamed to
> ufshcd_get_ucd_size().
>
> Signed-off-by: Po-Wen Kao <powen.kao@mediatek.com>
reviewed-by:  Ziqi Chen <quic_ziqichen@quicinc.com>
> ---
>   drivers/ufs/core/ufs-mcq.c | 2 +-
>   drivers/ufs/core/ufshcd.c  | 8 ++++----
>   include/ufs/ufshcd.h       | 2 +-
>   3 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
> index 3a27fa4b0024..a39746b2a8be 100644
> --- a/drivers/ufs/core/ufs-mcq.c
> +++ b/drivers/ufs/core/ufs-mcq.c
> @@ -265,7 +265,7 @@ static int ufshcd_mcq_get_tag(struct ufs_hba *hba,
>   	addr = (le64_to_cpu(cqe->command_desc_base_addr) & CQE_UCD_BA) -
>   		hba->ucdl_dma_addr;
>   
> -	return div_u64(addr, sizeof_utp_transfer_cmd_desc(hba));
> +	return div_u64(addr, ufshcd_get_ucd_size(hba));
>   }
>   
>   static void ufshcd_mcq_process_cqe(struct ufs_hba *hba,
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 3b3cf78d3b10..81c9f07ebfc8 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -2823,10 +2823,10 @@ static void ufshcd_map_queues(struct Scsi_Host *shost)
>   static void ufshcd_init_lrb(struct ufs_hba *hba, struct ufshcd_lrb *lrb, int i)
>   {
>   	struct utp_transfer_cmd_desc *cmd_descp = (void *)hba->ucdl_base_addr +
> -		i * sizeof_utp_transfer_cmd_desc(hba);
> +		i * ufshcd_get_ucd_size(hba);
>   	struct utp_transfer_req_desc *utrdlp = hba->utrdl_base_addr;
>   	dma_addr_t cmd_desc_element_addr = hba->ucdl_dma_addr +
> -		i * sizeof_utp_transfer_cmd_desc(hba);
> +		i * ufshcd_get_ucd_size(hba);
>   	u16 response_offset = offsetof(struct utp_transfer_cmd_desc,
>   				       response_upiu);
>   	u16 prdt_offset = offsetof(struct utp_transfer_cmd_desc, prd_table);
> @@ -3735,7 +3735,7 @@ static int ufshcd_memory_alloc(struct ufs_hba *hba)
>   	size_t utmrdl_size, utrdl_size, ucdl_size;
>   
>   	/* Allocate memory for UTP command descriptors */
> -	ucdl_size = sizeof_utp_transfer_cmd_desc(hba) * hba->nutrs;
> +	ucdl_size = ufshcd_get_ucd_size(hba) * hba->nutrs;
>   	hba->ucdl_base_addr = dmam_alloc_coherent(hba->dev,
>   						  ucdl_size,
>   						  &hba->ucdl_dma_addr,
> @@ -3835,7 +3835,7 @@ static void ufshcd_host_memory_configure(struct ufs_hba *hba)
>   	prdt_offset =
>   		offsetof(struct utp_transfer_cmd_desc, prd_table);
>   
> -	cmd_desc_size = sizeof_utp_transfer_cmd_desc(hba);
> +	cmd_desc_size = ufshcd_get_ucd_size(hba);
>   	cmd_desc_dma_addr = hba->ucdl_dma_addr;
>   
>   	for (i = 0; i < hba->nutrs; i++) {
> diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
> index ed9e3d5addb3..8f79cca449e1 100644
> --- a/include/ufs/ufshcd.h
> +++ b/include/ufs/ufshcd.h
> @@ -1136,7 +1136,7 @@ static inline size_t ufshcd_sg_entry_size(const struct ufs_hba *hba)
>   	({ (void)(hba); BUILD_BUG_ON(sg_entry_size != sizeof(struct ufshcd_sg_entry)); })
>   #endif
>   
> -static inline size_t sizeof_utp_transfer_cmd_desc(const struct ufs_hba *hba)
> +static inline size_t ufshcd_get_ucd_size(const struct ufs_hba *hba)
>   {
>   	return sizeof(struct utp_transfer_cmd_desc) + SG_ALL * ufshcd_sg_entry_size(hba);
>   }

Re: [PATCH v2 3/7] scsi: ufs: core: Fix mcq nr_hw_queues

[Ziqi Chen]

Hi Po-Wen,

On 2/22/2023 11:04 AM, Po-Wen Kao wrote:
> Need to add one to MAXQ to obtain number of hardware queue.
>
> Signed-off-by: Po-Wen Kao <powen.kao@mediatek.com>
> ---
>   drivers/ufs/core/ufs-mcq.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
> index a39746b2a8be..5d5bc0bc6e88 100644
> --- a/drivers/ufs/core/ufs-mcq.c
> +++ b/drivers/ufs/core/ufs-mcq.c
> @@ -150,7 +150,7 @@ static int ufshcd_mcq_config_nr_queues(struct ufs_hba *hba)
>   	u32 hba_maxq, rem, tot_queues;
>   	struct Scsi_Host *host = hba->host;
>   
> -	hba_maxq = FIELD_GET(MAX_QUEUE_SUP, hba->mcq_capabilities);
> +	hba_maxq = FIELD_GET(MAX_QUEUE_SUP, hba->mcq_capabilities) + 1 ;
Can we add one line comment why need to  add one to hba_maxq  here or in 
commit message?
>   
>   	tot_queues = UFS_MCQ_NUM_DEV_CMD_QUEUES + read_queues + poll_queues +
>   			rw_queues;

Best Regards.

Ziqi

Re: [PATCH v2 4/7] scsi: ufs: core: Add hwq print for debug

[Ziqi Chen]

On 2/23/2023 6:14 PM, Ziqi Chen wrote:

> Hi Po-Wen,
>
> On 2/22/2023 11:04 AM, Po-Wen Kao wrote:
>> +void ufshcd_mcq_print_hwqs(struct ufs_hba *hba, unsigned long bitmap)
>> +{
>> +    int id, i;
>> +    char prefix[15];
>> +
>> +    if (!is_mcq_enabled(hba))
>> +        return;
>> +
>> +    for_each_set_bit(id, &bitmap, hba->nr_hw_queues) {
>> +        snprintf(prefix, sizeof(prefix), "q%d SQCFG: ", id);
>> +        ufshcd_hex_dump(prefix,
>> +            hba->mcq_base + MCQ_QCFG_SIZE * id, MCQ_QCFG_SQ_SIZE);
>
> Is your purpose dump per hardware queue registers here?  If yes, why 
> don't use ufsmcq_readl() to save to a buffer and then use 
> ufshcd_hex_dump()
>
> to dump ? Are you sure ufshcd_hex_dump() can dump register directly?
>
>> +
>> +        snprintf(prefix, sizeof(prefix), "q%d CQCFG: ", id);
>> +        ufshcd_hex_dump(prefix,
>> +            hba->mcq_base + MCQ_QCFG_SIZE * id + MCQ_QCFG_SQ_SIZE, 
>> MCQ_QCFG_CQ_SIZE);
> Same to above comment.
>> +
>> +        for (i = 0; i < OPR_MAX ; i++) {
>> +            snprintf(prefix, sizeof(prefix), "q%d OPR%d: ", id, i);
>> +            ufshcd_hex_dump(prefix, mcq_opr_base(hba, i, id), 
>> mcq_opr_size[i]);
> Same.
>> +        }
>> +    }
>> +}
>> +
>>
>>   @@ -574,7 +569,16 @@ void ufshcd_print_trs(struct ufs_hba *hba, 
>> unsigned long bitmap, bool pr_prdt)
>>           if (pr_prdt)
>>               ufshcd_hex_dump("UPIU PRDT: ", lrbp->ucd_prdt_ptr,
>>                   ufshcd_sg_entry_size(hba) * prdt_length);
>> +
>> +        if (is_mcq_enabled(hba)) {
>> +            cmd = lrbp->cmd;
>> +            if (!cmd)
>> +                return;
>> +            hwq = ufshcd_mcq_req_to_hwq(hba, scsi_cmd_to_rq(cmd));
>> +            ufshcd_mcq_print_hwqs(hba, 1 << hwq->id);
>
> Calling registers dump function in ufshcd_print_trs() is not 
> reasonable, eg.. for each aborted request, it would print out all hwq 
> registers, it's not make sense.
>
> I think we should move it out of ufshcd_print_trs().

One more thing,  ufshcd_err_handler() pass hba-> outstanding_reqs to 
ufshcd_print_trs() as the 2nd parameter, but the hba-> outstanding_reqs 
is not used in MCQ mode.

I am making a change to print trs for MCQ mode by trying to get bitmap 
of started Reqs from block layer .

My opinion is keeping ufshcd_print_trs just print UPIU details , don't 
invoke register dump.

>
>> +        }
>>       }
>> +
>>   }
>
>
> Best Regards,
>
> Ziqi
>

Re: [PATCH v2 5/7] scsi: ufs: core: Remove redundant check

[Ziqi Chen]

On 2/22/2023 11:04 AM, Po-Wen Kao wrote:
> is_mcq_supported() already check on use_mcq_mode.
>
> Signed-off-by: Po-Wen Kao <powen.kao@mediatek.com>
reviewed-by:  Ziqi Chen <quic_ziqichen@quicinc.com>
> ---
>   drivers/ufs/core/ufshcd.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index a15a5a78160c..21e3bf5d8f08 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -8548,7 +8548,7 @@ static int ufshcd_device_init(struct ufs_hba *hba, bool init_dev_params)
>   			hba->scsi_host_added = true;
>   		}
>   		/* MCQ may be disabled if ufshcd_alloc_mcq() fails */
> -		if (is_mcq_supported(hba) && use_mcq_mode)
> +		if (is_mcq_supported(hba))
>   			ufshcd_config_mcq(hba);
>   	}
>   

Re: [PATCH v2 4/7] scsi: ufs: core: Add hwq print for debug

[Powen Kao (高伯文)]

Hi Ziqi,

Thanks for ur comments.

This piece of code successfully dump relevent registers on our
platform. As you know, mcq error handling flow is not ready yet so the
insertion point might not seems to be reasonable. 

Maybe drop this patch for now, I will send it later with error handling
patches.


On Thu, 2023-02-23 at 18:14 +0800, Ziqi Chen wrote:
> Hi Po-Wen,
> 
> On 2/22/2023 11:04 AM, Po-Wen Kao wrote:
> > +void ufshcd_mcq_print_hwqs(struct ufs_hba *hba, unsigned long
> > bitmap)
> > +{
> > +	int id, i;
> > +	char prefix[15];
> > +
> > +	if (!is_mcq_enabled(hba))
> > +		return;
> > +
> > +	for_each_set_bit(id, &bitmap, hba->nr_hw_queues) {
> > +		snprintf(prefix, sizeof(prefix), "q%d SQCFG: ", id);
> > +		ufshcd_hex_dump(prefix,
> > +			hba->mcq_base + MCQ_QCFG_SIZE * id,
> > MCQ_QCFG_SQ_SIZE);
> 
> Is your purpose dump per hardware queue registers here?  If yes, why 
> don't use ufsmcq_readl() to save to a buffer and then use
> ufshcd_hex_dump()
> 
> to dump ? Are you sure ufshcd_hex_dump() can dump register directly?
> 
> > +
> > +		snprintf(prefix, sizeof(prefix), "q%d CQCFG: ", id);
> > +		ufshcd_hex_dump(prefix,
> > +			hba->mcq_base + MCQ_QCFG_SIZE * id +
> > MCQ_QCFG_SQ_SIZE, MCQ_QCFG_CQ_SIZE);
> 
> Same to above comment.
> > +
> > +		for (i = 0; i < OPR_MAX ; i++) {
> > +			snprintf(prefix, sizeof(prefix), "q%d OPR%d: ",
> > id, i);
> > +			ufshcd_hex_dump(prefix, mcq_opr_base(hba, i,
> > id), mcq_opr_size[i]);
> 
> Same.
> > +		}
> > +	}
> > +}
> > +
> > 
> >   
> > @@ -574,7 +569,16 @@ void ufshcd_print_trs(struct ufs_hba *hba,
> > unsigned long bitmap, bool pr_prdt)
> >   		if (pr_prdt)
> >   			ufshcd_hex_dump("UPIU PRDT: ", lrbp-
> > >ucd_prdt_ptr,
> >   				ufshcd_sg_entry_size(hba) *
> > prdt_length);
> > +
> > +		if (is_mcq_enabled(hba)) {
> > +			cmd = lrbp->cmd;
> > +			if (!cmd)
> > +				return;
> > +			hwq = ufshcd_mcq_req_to_hwq(hba,
> > scsi_cmd_to_rq(cmd));
> > +			ufshcd_mcq_print_hwqs(hba, 1 << hwq->id);
> 
> Calling registers dump function in ufshcd_print_trs() is not
> reasonable, 
> eg.. for each aborted request, it would print out all hwq registers, 
> it's not make sense.
> 
> I think we should move it out of ufshcd_print_trs().
> 
> > +		}
> >   	}
> > +
> >   }
> 
> 
> Best Regards,
> 
> Ziqi
> 

Re: [PATCH v2 3/7] scsi: ufs: core: Fix mcq nr_hw_queues

[Powen Kao (高伯文)]

Okay, I will add a comment here in next update. :)

On Thu, 2023-02-23 at 18:32 +0800, Ziqi Chen wrote:
> Hi Po-Wen,
> 
> On 2/22/2023 11:04 AM, Po-Wen Kao wrote:
> > Need to add one to MAXQ to obtain number of hardware queue.
> > 
> > Signed-off-by: Po-Wen Kao <powen.kao@mediatek.com>
> > ---
> >   drivers/ufs/core/ufs-mcq.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-
> > mcq.c
> > index a39746b2a8be..5d5bc0bc6e88 100644
> > --- a/drivers/ufs/core/ufs-mcq.c
> > +++ b/drivers/ufs/core/ufs-mcq.c
> > @@ -150,7 +150,7 @@ static int ufshcd_mcq_config_nr_queues(struct
> > ufs_hba *hba)
> >   	u32 hba_maxq, rem, tot_queues;
> >   	struct Scsi_Host *host = hba->host;
> >   
> > -	hba_maxq = FIELD_GET(MAX_QUEUE_SUP, hba->mcq_capabilities);
> > +	hba_maxq = FIELD_GET(MAX_QUEUE_SUP, hba->mcq_capabilities) + 1
> > ;
> 
> Can we add one line comment why need to  add one to hba_maxq  here or
> in 
> commit message?
> >   
> >   	tot_queues = UFS_MCQ_NUM_DEV_CMD_QUEUES + read_queues +
> > poll_queues +
> >   			rw_queues;
> 
> Best Regards.
> 
> Ziqi
> 

Re: [PATCH v2 4/7] scsi: ufs: core: Add hwq print for debug

[Ziqi Chen]

Hi  Powen,

The Bao. D . Nguyen (quic_nguyenb@quicinc.com) from QCOM already made 
patch to support MCQ abort.

++ Bao here to be aware of it in case your error handing patch conflict 
with his abort handling patch.


Best Regards,

Ziqi


On 2/23/2023 10:13 PM, Powen Kao (高伯文) wrote:
> Hi Ziqi,
>
> Thanks for ur comments.
>
> This piece of code successfully dump relevent registers on our
> platform. As you know, mcq error handling flow is not ready yet so the
> insertion point might not seems to be reasonable.
>
> Maybe drop this patch for now, I will send it later with error handling
> patches.
>
>
> On Thu, 2023-02-23 at 18:14 +0800, Ziqi Chen wrote:
>> Hi Po-Wen,
>>
>> On 2/22/2023 11:04 AM, Po-Wen Kao wrote:
>>> +void ufshcd_mcq_print_hwqs(struct ufs_hba *hba, unsigned long
>>> bitmap)
>>> +{
>>> +	int id, i;
>>> +	char prefix[15];
>>> +
>>> +	if (!is_mcq_enabled(hba))
>>> +		return;
>>> +
>>> +	for_each_set_bit(id, &bitmap, hba->nr_hw_queues) {
>>> +		snprintf(prefix, sizeof(prefix), "q%d SQCFG: ", id);
>>> +		ufshcd_hex_dump(prefix,
>>> +			hba->mcq_base + MCQ_QCFG_SIZE * id,
>>> MCQ_QCFG_SQ_SIZE);
>> Is your purpose dump per hardware queue registers here?  If yes, why
>> don't use ufsmcq_readl() to save to a buffer and then use
>> ufshcd_hex_dump()
>>
>> to dump ? Are you sure ufshcd_hex_dump() can dump register directly?
>>
>>> +
>>> +		snprintf(prefix, sizeof(prefix), "q%d CQCFG: ", id);
>>> +		ufshcd_hex_dump(prefix,
>>> +			hba->mcq_base + MCQ_QCFG_SIZE * id +
>>> MCQ_QCFG_SQ_SIZE, MCQ_QCFG_CQ_SIZE);
>> Same to above comment.
>>> +
>>> +		for (i = 0; i < OPR_MAX ; i++) {
>>> +			snprintf(prefix, sizeof(prefix), "q%d OPR%d: ",
>>> id, i);
>>> +			ufshcd_hex_dump(prefix, mcq_opr_base(hba, i,
>>> id), mcq_opr_size[i]);
>> Same.
>>> +		}
>>> +	}
>>> +}
>>> +
>>>
>>>    
>>> @@ -574,7 +569,16 @@ void ufshcd_print_trs(struct ufs_hba *hba,
>>> unsigned long bitmap, bool pr_prdt)
>>>    		if (pr_prdt)
>>>    			ufshcd_hex_dump("UPIU PRDT: ", lrbp-
>>>> ucd_prdt_ptr,
>>>    				ufshcd_sg_entry_size(hba) *
>>> prdt_length);
>>> +
>>> +		if (is_mcq_enabled(hba)) {
>>> +			cmd = lrbp->cmd;
>>> +			if (!cmd)
>>> +				return;
>>> +			hwq = ufshcd_mcq_req_to_hwq(hba,
>>> scsi_cmd_to_rq(cmd));
>>> +			ufshcd_mcq_print_hwqs(hba, 1 << hwq->id);
>> Calling registers dump function in ufshcd_print_trs() is not
>> reasonable,
>> eg.. for each aborted request, it would print out all hwq registers,
>> it's not make sense.
>>
>> I think we should move it out of ufshcd_print_trs().
>>
>>> +		}
>>>    	}
>>> +
>>>    }
>>
>> Best Regards,
>>
>> Ziqi
>>

Re: [PATCH v2 2/7] scsi: ufs: core: Rename symbols

[Bart Van Assche]

On 2/21/23 19:04, Po-Wen Kao wrote:
> To avoid confusion, sizeof_utp_transfer_cmd_desc() is renamed to
> ufshcd_get_ucd_size().

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

Re: [PATCH v2 3/7] scsi: ufs: core: Fix mcq nr_hw_queues

[Bart Van Assche]

On 2/21/23 19:04, Po-Wen Kao wrote:
> Need to add one to MAXQ to obtain number of hardware queue.

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

Re: [PATCH v2 4/7] scsi: ufs: core: Add hwq print for debug

[Bart Van Assche]

On 2/23/23 02:14, Ziqi Chen wrote:
> Calling registers dump function in ufshcd_print_trs() is not reasonable, 
> eg.. for each aborted request, it would print out all hwq registers, 
> it's not make sense.
> 
> I think we should move it out of ufshcd_print_trs().

+1

Re: [PATCH v2 4/7] scsi: ufs: core: Add hwq print for debug

[Bao D. Nguyen]

On 2/26/2023 7:14 PM, Ziqi Chen wrote:
> Hi Powen,
>
> The Bao. D . Nguyen (quic_nguyenb@quicinc.com) from QCOM already made 
> patch to support MCQ abort.
>
> ++ Bao here to be aware of it in case your error handing patch 
> conflict with his abort handling patch.
>
>
> Best Regards,
>
> Ziqi
>
>
> On 2/23/2023 10:13 PM, Powen Kao (高伯文) wrote:
>> Hi Ziqi,
>>
>> Thanks for ur comments.
>>
>> This piece of code successfully dump relevent registers on our
>> platform. As you know, mcq error handling flow is not ready yet so the
>> insertion point might not seems to be reasonable.
>>
>> Maybe drop this patch for now, I will send it later with error handling
>> patches.
>>
>>
>> On Thu, 2023-02-23 at 18:14 +0800, Ziqi Chen wrote:
>>> Hi Po-Wen,
>>>
>>> On 2/22/2023 11:04 AM, Po-Wen Kao wrote:
>>>> +void ufshcd_mcq_print_hwqs(struct ufs_hba *hba, unsigned long
>>>> bitmap)
>>>> +{
>>>> +    int id, i;
>>>> +    char prefix[15];
>>>> +
>>>> +    if (!is_mcq_enabled(hba))
>>>> +        return;
>>>> +
>>>> +    for_each_set_bit(id, &bitmap, hba->nr_hw_queues) {
>>>> +        snprintf(prefix, sizeof(prefix), "q%d SQCFG: ", id);
>>>> +        ufshcd_hex_dump(prefix,
>>>> +            hba->mcq_base + MCQ_QCFG_SIZE * id,
>>>> MCQ_QCFG_SQ_SIZE);
>>> Is your purpose dump per hardware queue registers here?  If yes, why
>>> don't use ufsmcq_readl() to save to a buffer and then use
>>> ufshcd_hex_dump()
>>>
>>> to dump ? Are you sure ufshcd_hex_dump() can dump register directly?
>>>
>>>> +
>>>> +        snprintf(prefix, sizeof(prefix), "q%d CQCFG: ", id);
>>>> +        ufshcd_hex_dump(prefix,
>>>> +            hba->mcq_base + MCQ_QCFG_SIZE * id +
>>>> MCQ_QCFG_SQ_SIZE, MCQ_QCFG_CQ_SIZE);
>>> Same to above comment.
>>>> +
>>>> +        for (i = 0; i < OPR_MAX ; i++) {
>>>> +            snprintf(prefix, sizeof(prefix), "q%d OPR%d: ",
>>>> id, i);
>>>> +            ufshcd_hex_dump(prefix, mcq_opr_base(hba, i,
>>>> id), mcq_opr_size[i]);
>>> Same.
>>>> +        }
>>>> +    }
>>>> +}
>>>> +
>>>>
>>>>    @@ -574,7 +569,16 @@ void ufshcd_print_trs(struct ufs_hba *hba,
>>>> unsigned long bitmap, bool pr_prdt)
>>>>            if (pr_prdt)
>>>>                ufshcd_hex_dump("UPIU PRDT: ", lrbp-
>>>>> ucd_prdt_ptr,
>>>>                    ufshcd_sg_entry_size(hba) *
>>>> prdt_length);
>>>> +
>>>> +        if (is_mcq_enabled(hba)) {
>>>> +            cmd = lrbp->cmd;
>>>> +            if (!cmd)
>>>> +                return;
>>>> +            hwq = ufshcd_mcq_req_to_hwq(hba,
>>>> scsi_cmd_to_rq(cmd));
>>>> +            ufshcd_mcq_print_hwqs(hba, 1 << hwq->id);
>>> Calling registers dump function in ufshcd_print_trs() is not
>>> reasonable,
>>> eg.. for each aborted request, it would print out all hwq registers,
>>> it's not make sense.
>>>
>>> I think we should move it out of ufshcd_print_trs().
>>>
>>>> +        }
>>>>        }
>>>> +
>>>>    }
>>>
>>> Best Regards,
>>>
>>> Ziqi
>>>
Hi Powen,

I am going to push the mcq abort handling and mcq error handling code 
upstream for review in a couple days. Would that work for you?

Regards,
Bao

Re: [PATCH v2 4/7] scsi: ufs: core: Add hwq print for debug

[Powen Kao (高伯文)]

Hi Bao,

Sure, we can first integrate ur patch and see if anything is missing
that need further upstream. Due to comapct schedule, I would kindly ask
if it will be ready by the end of this week? :) Thanks


On Mon, 2023-02-27 at 18:57 -0800, Bao D. Nguyen wrote:
> On 2/26/2023 7:14 Sure PM, Ziqi Chen wrote:
> > Hi Powen,
> > 
> > The Bao. D . Nguyen (quic_nguyenb@quicinc.com) from QCOM already
> > made 
> > patch to support MCQ abort.
> > 
> > ++ Bao here to be aware of it in case your error handing patch 
> > conflict with his abort handling patch.
> > 
> > 
> > Best Regards,
> > 
> > Ziqi
> > 
> > 
> > On 2/23/2023 10:13 PM, Powen Kao (高伯文) wrote:
> > > Hi Ziqi,
> > > 
> > > Thanks for ur comments.
> > > 
> > > This piece of code successfully dump relevent registers on our
> > > platform. As you know, mcq error handling flow is not ready yet
> > > so the
> > > insertion point might not seems to be reasonable.
> > > 
> > > Maybe drop this patch for now, I will send it later with error
> > > handling
> > > patches.
> > > 
> > > 
> > > On Thu, 2023-02-23 at 18:14 +0800, Ziqi Chen wrote:
> > > > Hi Po-Wen,
> > > > 
> > > > On 2/22/2023 11:04 AM, Po-Wen Kao wrote:
> > > > > +void ufshcd_mcq_print_hwqs(struct ufs_hba *hba, unsigned
> > > > > long
> > > > > bitmap)
> > > > > +{
> > > > > +    int id, i;
> > > > > +    char prefix[15];
> > > > > +
> > > > > +    if (!is_mcq_enabled(hba))
> > > > > +        return;
> > > > > +
> > > > > +    for_each_set_bit(id, &bitmap, hba->nr_hw_queues) {
> > > > > +        snprintf(prefix, sizeof(prefix), "q%d SQCFG: ", id);
> > > > > +        ufshcd_hex_dump(prefix,
> > > > > +            hba->mcq_base + MCQ_QCFG_SIZE * id,
> > > > > MCQ_QCFG_SQ_SIZE);
> > > > 
> > > > Is your purpose dump per hardware queue registers here?  If
> > > > yes, why
> > > > don't use ufsmcq_readl() to save to a buffer and then use
> > > > ufshcd_hex_dump()
> > > > 
> > > > to dump ? Are you sure ufshcd_hex_dump() can dump register
> > > > directly?
> > > > 
> > > > > +
> > > > > +        snprintf(prefix, sizeof(prefix), "q%d CQCFG: ", id);
> > > > > +        ufshcd_hex_dump(prefix,
> > > > > +            hba->mcq_base + MCQ_QCFG_SIZE * id +
> > > > > MCQ_QCFG_SQ_SIZE, MCQ_QCFG_CQ_SIZE);
> > > > 
> > > > Same to above comment.
> > > > > +
> > > > > +        for (i = 0; i < OPR_MAX ; i++) {
> > > > > +            snprintf(prefix, sizeof(prefix), "q%d OPR%d: ",
> > > > > id, i);
> > > > > +            ufshcd_hex_dump(prefix, mcq_opr_base(hba, i,
> > > > > id), mcq_opr_size[i]);
> > > > 
> > > > Same.
> > > > > +        }
> > > > > +    }
> > > > > +}
> > > > > +
> > > > > 
> > > > >    @@ -574,7 +569,16 @@ void ufshcd_print_trs(struct ufs_hba
> > > > > *hba,
> > > > > unsigned long bitmap, bool pr_prdt)
> > > > >            if (pr_prdt)
> > > > >                ufshcd_hex_dump("UPIU PRDT: ", lrbp-
> > > > > > ucd_prdt_ptr,
> > > > > 
> > > > >                    ufshcd_sg_entry_size(hba) *
> > > > > prdt_length);
> > > > > +
> > > > > +        if (is_mcq_enabled(hba)) {
> > > > > +            cmd = lrbp->cmd;
> > > > > +            if (!cmd)
> > > > > +                return;
> > > > > +            hwq = ufshcd_mcq_req_to_hwq(hba,
> > > > > scsi_cmd_to_rq(cmd));
> > > > > +            ufshcd_mcq_print_hwqs(hba, 1 << hwq->id);
> > > > 
> > > > Calling registers dump function in ufshcd_print_trs() is not
> > > > reasonable,
> > > > eg.. for each aborted request, it would print out all hwq
> > > > registers,
> > > > it's not make sense.
> > > > 
> > > > I think we should move it out of ufshcd_print_trs().
> > > > 
> > > > > +        }
> > > > >        }
> > > > > +
> > > > >    }
> > > > 
> > > > Best Regards,
> > > > 
> > > > Ziqi
> > > > 
> 
> Hi Powen,
> 
> I am going to push the mcq abort handling and mcq error handling
> code 
> upstream for review in a couple days. Would that work for you?
> 
> Regards,
> Bao
> 

Re: [PATCH v2 4/7] scsi: ufs: core: Add hwq print for debug

[Bart Van Assche]

On 2/28/23 18:17, Powen Kao (高伯文) wrote:
> Sure, we can first integrate ur patch and see if anything is missing
> that need further upstream. Due to comapct schedule, I would kindly ask
> if it will be ready by the end of this week? :) Thanks

Please trim e-mails before replying and please reply below the original 
text instead of above. From https://en.wikipedia.org/wiki/Posting_style:

Because it messes up the order in which people normally read text.
Why is top-posting such a bad thing?
Top-posting.
What is the most annoying thing in e-mail?

Thanks,

Bart.

Re: [PATCH v2 4/7] scsi: ufs: core: Add hwq print for debug

[Slade's Kernel Patch Bot]

On 2/28/23 21:17, Powen Kao (高伯文) wrote:
> Hi Bao,
> 
> Sure, we can first integrate ur patch and see if anything is missing
> that need further upstream. Due to comapct schedule, I would kindly ask
> if it will be ready by the end of this week? :) Thanks
> 

This is Slade's kernel patch bot. When scanning his mailbox, I came across
this message, which appears to be a top-post. Please do not top-post on Linux
mailing lists.

A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

Please bottom-post to Linux mailing lists in the future. See also:
https://daringfireball.net/2007/07/on_top

If you believe this is an error, please address a message to Slade Watkins
<srw@sladewatkins.net>.

Thank you,
-- Slade's kernel patch bot

> 
> On Mon, 2023-02-27 at 18:57 -0800, Bao D. Nguyen wrote:
>> On 2/26/2023 7:14 Sure PM, Ziqi Chen wrote:
>>> Hi Powen,
>>>
>>> The Bao. D . Nguyen (quic_nguyenb@quicinc.com) from QCOM already
>>> made 
>>> patch to support MCQ abort.
>>>
>>> ++ Bao here to be aware of it in case your error handing patch 
>>> conflict with his abort handling patch.
>>>
>>>
>>> Best Regards,
>>>
>>> Ziqi
>>>
>>>
>>> On 2/23/2023 10:13 PM, Powen Kao (高伯文) wrote:
>>>> Hi Ziqi,
>>>>
>>>> Thanks for ur comments.
>>>>
>>>> This piece of code successfully dump relevent registers on our
>>>> platform. As you know, mcq error handling flow is not ready yet
>>>> so the
>>>> insertion point might not seems to be reasonable.
>>>>
>>>> Maybe drop this patch for now, I will send it later with error
>>>> handling
>>>> patches.
>>>>
>>>>
>>>> On Thu, 2023-02-23 at 18:14 +0800, Ziqi Chen wrote:
>>>>> Hi Po-Wen,
>>>>>
>>>>> On 2/22/2023 11:04 AM, Po-Wen Kao wrote:
>>>>>> +void ufshcd_mcq_print_hwqs(struct ufs_hba *hba, unsigned
>>>>>> long
>>>>>> bitmap)
>>>>>> +{
>>>>>> +    int id, i;
>>>>>> +    char prefix[15];
>>>>>> +
>>>>>> +    if (!is_mcq_enabled(hba))
>>>>>> +        return;
>>>>>> +
>>>>>> +    for_each_set_bit(id, &bitmap, hba->nr_hw_queues) {
>>>>>> +        snprintf(prefix, sizeof(prefix), "q%d SQCFG: ", id);
>>>>>> +        ufshcd_hex_dump(prefix,
>>>>>> +            hba->mcq_base + MCQ_QCFG_SIZE * id,
>>>>>> MCQ_QCFG_SQ_SIZE);
>>>>>
>>>>> Is your purpose dump per hardware queue registers here?  If
>>>>> yes, why
>>>>> don't use ufsmcq_readl() to save to a buffer and then use
>>>>> ufshcd_hex_dump()
>>>>>
>>>>> to dump ? Are you sure ufshcd_hex_dump() can dump register
>>>>> directly?
>>>>>
>>>>>> +
>>>>>> +        snprintf(prefix, sizeof(prefix), "q%d CQCFG: ", id);
>>>>>> +        ufshcd_hex_dump(prefix,
>>>>>> +            hba->mcq_base + MCQ_QCFG_SIZE * id +
>>>>>> MCQ_QCFG_SQ_SIZE, MCQ_QCFG_CQ_SIZE);
>>>>>
>>>>> Same to above comment.
>>>>>> +
>>>>>> +        for (i = 0; i < OPR_MAX ; i++) {
>>>>>> +            snprintf(prefix, sizeof(prefix), "q%d OPR%d: ",
>>>>>> id, i);
>>>>>> +            ufshcd_hex_dump(prefix, mcq_opr_base(hba, i,
>>>>>> id), mcq_opr_size[i]);
>>>>>
>>>>> Same.
>>>>>> +        }
>>>>>> +    }
>>>>>> +}
>>>>>> +
>>>>>>
>>>>>>    @@ -574,7 +569,16 @@ void ufshcd_print_trs(struct ufs_hba
>>>>>> *hba,
>>>>>> unsigned long bitmap, bool pr_prdt)
>>>>>>            if (pr_prdt)
>>>>>>                ufshcd_hex_dump("UPIU PRDT: ", lrbp-
>>>>>>> ucd_prdt_ptr,
>>>>>>
>>>>>>                    ufshcd_sg_entry_size(hba) *
>>>>>> prdt_length);
>>>>>> +
>>>>>> +        if (is_mcq_enabled(hba)) {
>>>>>> +            cmd = lrbp->cmd;
>>>>>> +            if (!cmd)
>>>>>> +                return;
>>>>>> +            hwq = ufshcd_mcq_req_to_hwq(hba,
>>>>>> scsi_cmd_to_rq(cmd));
>>>>>> +            ufshcd_mcq_print_hwqs(hba, 1 << hwq->id);
>>>>>
>>>>> Calling registers dump function in ufshcd_print_trs() is not
>>>>> reasonable,
>>>>> eg.. for each aborted request, it would print out all hwq
>>>>> registers,
>>>>> it's not make sense.
>>>>>
>>>>> I think we should move it out of ufshcd_print_trs().
>>>>>
>>>>>> +        }
>>>>>>        }
>>>>>> +
>>>>>>    }
>>>>>
>>>>> Best Regards,
>>>>>
>>>>> Ziqi
>>>>>
>>
>> Hi Powen,
>>
>> I am going to push the mcq abort handling and mcq error handling
>> code 
>> upstream for review in a couple days. Would that work for you?
>>
>> Regards,
>> Bao
>>