[PATCH] ufs: core: add caps UFSHCD_CAP_MCQ_EN

Huan Tang posted 1 patch 9 months, 3 weeks ago
drivers/ufs/core/ufshcd.c | 3 ++-
include/ufs/ufshcd.h      | 6 ++++++
2 files changed, 8 insertions(+), 1 deletion(-)
[PATCH] ufs: core: add caps UFSHCD_CAP_MCQ_EN
Posted by Huan Tang 9 months, 3 weeks ago
Add caps UFSHCD_CAP_MCQ_EN(default enable), then host driver to
control the on/off of MCQ; Sometimes, we don't want to enable
MCQ and want to disable it through the host driver, for example,
ufs-qcom.c .

Signed-off-by: Huan Tang <tanghuan@vivo.com>
---
 drivers/ufs/core/ufshcd.c | 3 ++-
 include/ufs/ufshcd.h      | 6 ++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 44156041d88f..b8937f85d81a 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -112,7 +112,7 @@ static bool use_mcq_mode = true;
 
 static bool is_mcq_supported(struct ufs_hba *hba)
 {
-	return hba->mcq_sup && use_mcq_mode;
+	return hba->mcq_sup && use_mcq_mode && (hba->caps & UFSHCD_CAP_MCQ_EN);
 }
 
 module_param(use_mcq_mode, bool, 0644);
@@ -10389,6 +10389,7 @@ int ufshcd_alloc_host(struct device *dev, struct ufs_hba **hba_handle)
 	hba->dev = dev;
 	hba->dev_ref_clk_freq = REF_CLK_FREQ_INVAL;
 	hba->nop_out_timeout = NOP_OUT_TIMEOUT;
+	hba->caps |= UFSHCD_CAP_MCQ_EN;
 	ufshcd_set_sg_entry_size(hba, sizeof(struct ufshcd_sg_entry));
 	INIT_LIST_HEAD(&hba->clk_list_head);
 	spin_lock_init(&hba->outstanding_lock);
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index e928ed0265ff..d8547bc6bf79 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -771,6 +771,12 @@ enum ufshcd_caps {
 	 * WriteBooster when scaling the clock down.
 	 */
 	UFSHCD_CAP_WB_WITH_CLK_SCALING			= 1 << 12,
+
+	/*
+	 * This capability allows the host controller driver to use the
+	 * multi-circular queue, if it is present
+	 */
+	UFSHCD_CAP_MCQ_EN				= 1 << 13,
 };
 
 struct ufs_hba_variant_params {
-- 
2.39.0
Re: [PATCH] ufs: core: add caps UFSHCD_CAP_MCQ_EN
Posted by Peter Wang (王信友) 9 months, 3 weeks ago
On Mon, 2025-04-21 at 21:51 +0800, Huan Tang wrote:
> 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Add caps UFSHCD_CAP_MCQ_EN(default enable), then host driver to
> control the on/off of MCQ; Sometimes, we don't want to enable
> MCQ and want to disable it through the host driver, for example,
> ufs-qcom.c .
> 
> Signed-off-by: Huan Tang <tanghuan@vivo.com>
> ---


Hi Huan,

This patch of only add a flag and always enables this flag. 
In other words, it is a meaningless patch.
Suggest also uptream the flow of the disable flag patch 
as part of a patch series.

Thanks
Peter


Re: [PATCH] ufs: core: add caps UFSHCD_CAP_MCQ_EN
Posted by Huan Tang 9 months, 3 weeks ago
> This patch of only add a flag and always enables this flag. 
> In other words, it is a meaningless patch.
> Suggest also uptream the flow of the disable flag patch 
> as part of a patch series.

Hi Peter sir,

Thanks for you reply!
I found that using low-end UFS in SoCs that support MCQ may cause
device latency to reach extreme values.

My idea is that after having this patch, I can add hba->caps &=
~UFSHCD_CAP_MCQ_EN to disable mcq via ufs-qcom.c or ufs-mediatek.c. 

Do you mean to add a caps UFSHCD_CAP_MCQ_DISABLE? 
Or remove hba->caps |= UFSHCD_CAP_MCQ_EN in ufshcd_alloc_host, 
and then add hba->caps |= UFSHCD_CAP_MCQ_EN in ufs-xxx.c, such as ufs-mediatek.c?

Thanks
Huan
Re: [PATCH] ufs: core: add caps UFSHCD_CAP_MCQ_EN
Posted by Peter Wang (王信友) 9 months, 3 weeks ago
On Tue, 2025-04-22 at 14:25 +0800, Huan Tang wrote:
> 
> Hi Peter sir,
> 
> 
> 
> Thanks for you reply!
> 
> I found that using low-end UFS in SoCs that support MCQ may cause
> 
> device latency to reach extreme values.
> 
> 
> 
> My idea is that after having this patch, I can add hba->caps &=
> 
> ~UFSHCD_CAP_MCQ_EN to disable mcq via ufs-qcom.c or ufs-mediatek.c.
> 
> 
> 
> Do you mean to add a caps UFSHCD_CAP_MCQ_DISABLE?
> 
> Or remove hba->caps |= UFSHCD_CAP_MCQ_EN in ufshcd_alloc_host,
> 
> and then add hba->caps |= UFSHCD_CAP_MCQ_EN in ufs-xxx.c, such as
> ufs-mediatek.c?
> 
> 
> 
> Thanks
> 
> Huan

Hi Huan,

What I mean is that you need upstream code to disable this flag. 
hba->caps &= ~UFSHCD_CAP_MCQ_EN

But this seems a bit strange to me, as MCQ is a UFS host feature. 
It shouldn't be related to the old ufs2.x device. 
If it is ultimately confirmed to be an issue with the UFS device, 
you can add a device quirk to disable this flag.

Thanks.
Peter



Re: [PATCH] ufs: core: add caps UFSHCD_CAP_MCQ_EN
Posted by Bart Van Assche 9 months, 3 weeks ago
On 4/21/25 11:25 PM, Huan Tang wrote:
> I found that using low-end UFS in SoCs that support MCQ may cause
> device latency to reach extreme values.

Hi Huan,

What is the root-cause of the high latency? If it is the time spent in
UFS interrupt handlers, please try to switch to threaded interrupt
handlers instead of disabling MCQ. See also
https://lore.kernel.org/linux-scsi/20250407-topic-ufs-use-threaded-irq-v3-0-08bee980f71e@linaro.org/

Bart.
Re: [PATCH] ufs: core: add caps UFSHCD_CAP_MCQ_EN
Posted by Huan Tang 9 months, 3 weeks ago
> What is the root-cause of the high latency? If it is the time spent in
> UFS interrupt handlers, please try to switch to threaded interrupt
> handlers instead of disabling MCQ. See also
> https://lore.kernel.org/linux-scsi/20250407-topic-ufs-use-threaded-irq-v3-0-08bee980f71e@linaro.org/

Hi bart sir,

Thanks for your reply!

Thank you for your advice on the direction of interrupt handlers. 

The reason has not been found yet, but a comparative experiment was done, 
and the results are as follows:

The failure probability is 7/400 when MCQ is turned on, and 0/400
when MCQ is turned off; the definition of failure here is: the maximum
latency is greater than 5s.



Thanks
Huan
[PATCH] ufs: core: add caps UFSHCD_CAP_MCQ_EN
Posted by Huan Tang 9 months, 3 weeks ago
> Add caps UFSHCD_CAP_MCQ_EN(default enable), then host driver to
> control the on/off of MCQ; Sometimes, we don't want to enable
> MCQ and want to disable it through the host driver, for example,
> ufs-qcom.c .

Hi Bart sir,

Sorry to disturb you!
Some phones use the qcom SM7750 platform, but I don't want to use MCQ; currently, to turn it off, I have to break GKI, so I want to add a way to control MCQ on/off, so that I can turn it off in ufs-qcom.c

Thanks
Huan