On 3/28/23 03:37, Po-Wen Kao wrote:
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index acae4e194ec4..1e1271aca1f2 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -8493,11 +8493,15 @@ static int ufshcd_alloc_mcq(struct ufs_hba *hba)
static void ufshcd_config_mcq(struct ufs_hba *hba)
{
int ret;
-
+ u32 intrs;
ret = ufshcd_mcq_vops_config_esi(hba);
+
dev_info(hba->dev, "ESI %sconfigured\n", ret ? "is not " : "");
The use of blank lines in the above code is weird. Please make sure
there is no blank line inside the declaration block and also that there
is a blank line between declarations and statements as required by the
kernel coding style.
- ufshcd_enable_intr(hba, UFSHCD_ENABLE_MCQ_INTRS);
+ intrs = (hba->quirks & UFSHCD_QUIRK_MCQ_BROKEN_INTR) ?
+ (UFSHCD_ENABLE_MCQ_INTRS & ~MCQ_CQ_EVENT_STATUS) : UFSHCD_ENABLE_MCQ_INTRS;
All parentheses in the above expression are superfluous. Please leave
these out. Or even better, rewrite the above code as follows:
intrs = UFSHCD_ENABLE_MCQ_INTRS;
if (hba->quirks & UFSHCD_QUIRK_MCQ_BROKEN_INTR)
intrs &= ~MCQ_CQ_EVENT_STATUS;
+
+ /*
+ * Some platform raises interrupt (per queue) in addition to
+ * CQES (traditional) when ESI is disabled.
+ * Enable this quirk will disable CQES and use per queue interrupt.
+ */
+ UFSHCD_QUIRK_MCQ_BROKEN_INTR = 1 << 20,
Isn't this an UFS controller behavior instead of a platform behavior?
Please consider changing "platform raises" into "controllers raise".
Thanks,
Bart.
