LinuxLists.cc - [PATCH v2 0/7] Several UFS MCQ Code Changes

[permalink] [raw]

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

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 <[email protected]>
Reviewed-by: Bart Van Assche <[email protected]>
Reviewed-by: Manivannan Sadhasivam <[email protected]>
Reviewed-by: Stanley Chu <[email protected]>
---
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

2023-02-22 03:05:00

[permalink] [raw]

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

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

Signed-off-by: Po-Wen Kao <[email protected]>
---
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

2023-02-22 03:05:09

[permalink] [raw]

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

is_mcq_supported() already check on use_mcq_mode.

Signed-off-by: Po-Wen Kao <[email protected]>
---
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

2023-02-22 03:05:09

[permalink] [raw]

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

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

Signed-off-by: Po-Wen Kao <[email protected]>
---
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

2023-02-22 03:05:10

[permalink] [raw]

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

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

Signed-off-by: Po-Wen Kao <[email protected]>
---
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

2023-02-22 03:05:17

[permalink] [raw]

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

Export
- ufshcd_mcq_config_mac
- ufshcd_mcq_make_queues_operational
- ufshcd_mcq_read_cqis
- ufshcd_disable_intr

Signed-off-by: Po-Wen Kao <[email protected]>
---
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

2023-02-22 03:05:22

[permalink] [raw]

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

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

Signed-off-by: Po-Wen Kao <[email protected]>
---
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

2023-02-22 03:12:31

[permalink] [raw]

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

On Wed, Feb 22, 2023 at 11:05 AM Po-Wen Kao <[email protected]> wrote:
>
> To avoid confusion, sizeof_utp_transfer_cmd_desc() is renamed to
> ufshcd_get_ucd_size().
>
> Signed-off-by: Po-Wen Kao <[email protected]>

Reviewed-by: Stanley Chu <[email protected]>

2023-02-22 03:17:13

[permalink] [raw]

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

On Wed, Feb 22, 2023 at 11:05 AM Po-Wen Kao <[email protected]> wrote:
>
> Need to add one to MAXQ to obtain number of hardware queue.
>
> Signed-off-by: Po-Wen Kao <[email protected]>

Reviewed-by: Stanley Chu <[email protected]>

2023-02-22 03:34:12

[permalink] [raw]

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

On Wed, Feb 22, 2023 at 11:05 AM Po-Wen Kao <[email protected]> wrote:
>
> is_mcq_supported() already check on use_mcq_mode.
>
> Signed-off-by: Po-Wen Kao <[email protected]>

Reviewed-by: Stanley Chu <[email protected]>

2023-02-22 05:19:38

[permalink] [raw]

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

On Wed, Feb 22, 2023 at 11:05 AM Po-Wen Kao <[email protected]> wrote:
>
> Export
> - ufshcd_mcq_config_mac
> - ufshcd_mcq_make_queues_operational
> - ufshcd_mcq_read_cqis
> - ufshcd_disable_intr
>
> Signed-off-by: Po-Wen Kao <[email protected]>
> ---
> 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 <[email protected]>

2023-02-23 10:15:23

[permalink] [raw]

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

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

2023-02-23 10:23:41

[permalink] [raw]

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

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 <[email protected]>
> Reviewed-by: Bart Van Assche <[email protected]>
> Reviewed-by: Manivannan Sadhasivam <[email protected]>
> Reviewed-by: Stanley Chu <[email protected]>
reviewed-by: Ziqi Chen <[email protected]>
> ---
> 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,

2023-02-23 10:27:55

[permalink] [raw]

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

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 <[email protected]>
reviewed-by: Ziqi Chen <[email protected]>
> ---
> 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);
> }

2023-02-23 10:33:29

[permalink] [raw]

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

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 <[email protected]>
> ---
> 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

2023-02-23 10:50:27

[permalink] [raw]

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

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
>

2023-02-23 10:53:47

[permalink] [raw]

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

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 <[email protected]>
reviewed-by: Ziqi Chen <[email protected]>
> ---
> 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);
> }
>

2023-02-23 14:14:00

[permalink] [raw]

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

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
>

2023-02-23 14:43:43

[permalink] [raw]

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

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 <[email protected]>
> > ---
> > 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
>

2023-02-27 03:15:16

[permalink] [raw]

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

Hi Powen,

The Bao. D . Nguyen ([email protected]) 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
>>

2023-02-27 22:43:33

[permalink] [raw]

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

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 <[email protected]>

2023-02-27 22:44:01

[permalink] [raw]

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

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 <[email protected]>

2023-02-27 22:48:34

[permalink] [raw]

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

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

2023-02-28 02:57:55

by Bao D. Nguyen

[permalink] [raw]

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

On 2/26/2023 7:14 PM, Ziqi Chen wrote:
> Hi Powen,
>
> The Bao. D . Nguyen ([email protected]) 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

2023-03-01 02:17:58

[permalink] [raw]

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

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 ([email protected]) 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
>

2023-03-01 18:51:05