Messages in this thread | | | Date | Tue, 28 Mar 2023 20:07:27 -0700 | Subject | Re: [PATCH 1/2] scsi: ufs: core: Add host quirk UFSHCD_QUIRK_MCQ_BROKEN_INTR | From | Bart Van Assche <> |
| |
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.
| |