[RFC 0/4] mmc: sdhci-msm: Add CQE support for sdhci-msm

[RFC 0/4] mmc: sdhci-msm: Add CQE support for sdhci-msm

[Ritesh Harjani]

Hi All,

Please ignore the previous patch series from a wrong email
address. Stupid gitconfig issue. Apologies for the spam.

This is RFC patch series based on top of ulfh_mmc/cmdq branch
which is based upon Adrian's CMDQ patch series.

Below patch series enables CQE for sdhci-msm platform.
This has been tested on internal 8996 MTP which has CMDQ support.

Fixes w.r.t. CMDQ:-
There are some patches identified which were required atleast on
MSM platform. I am not sure if these are required for any other
CQE platform or not. Patchset 1, 3 & 4 commit text describes
the problems.

Performance related:- 
I gave one small shot for performance and the numbers were not looking good.
So, unless I have tested for performance completely, I should not discuss 
on performance numbers as of now with this patchset. 
I can try doing some more performance testing and post the results -
though this may take some while.

I used below test script for random read/write test.

*randwrite-test-script*
[global]
bs=32k
size=1g
rw=randwrite
direct=1
directory=/data/fiotest

[file1]
filename=singlefile1

*randread-test-script*
[global]
bs=32k
size=1g
rw=randread
directory=/data/fiotest

[file1]
filename=singlefile1

@Adrian,
Thanks a lot for pursuing and bringing CMDQ patch series to it's final stages :)


Ritesh Harjani (4):
  mmc: cqhci: Move CQHCI_ENABLE before setting TDLBA/TDLBAU
  mmc: sdhci-msm: Add CQHCI support for sdhci-msm
  mmc: sdhci-msm: Change the desc_sz on cqe_enable/disable.
  mmc: sdhci-msm: Handle unexpected interrupt case on enabling legacy
    IRQs on CQE halt

 .../devicetree/bindings/mmc/sdhci-msm.txt          |   1 +
 drivers/mmc/host/Kconfig                           |   1 +
 drivers/mmc/host/cqhci.c                           |   7 +-
 drivers/mmc/host/sdhci-msm.c                       | 121 ++++++++++++++++++++-
 4 files changed, 125 insertions(+), 5 deletions(-)

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

[RFC 1/4] mmc: cqhci: Move CQHCI_ENABLE before setting TDLBA/TDLBAU

[Ritesh Harjani]

Without this patch the CQHCI registers are getting reset
again.

Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
---
 drivers/mmc/host/cqhci.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/cqhci.c b/drivers/mmc/host/cqhci.c
index 8650a13..2a7351c 100644
--- a/drivers/mmc/host/cqhci.c
+++ b/drivers/mmc/host/cqhci.c
@@ -262,6 +262,9 @@ static void __cqhci_enable(struct cqhci_host *cq_host)
 
 	cqhci_writel(cq_host, cqcfg, CQHCI_CFG);
 
+	cqcfg |= CQHCI_ENABLE;
+	cqhci_writel(cq_host, cqcfg, CQHCI_CFG);
+
 	cqhci_writel(cq_host, lower_32_bits(cq_host->desc_dma_base),
 		     CQHCI_TDLBA);
 	cqhci_writel(cq_host, upper_32_bits(cq_host->desc_dma_base),
@@ -271,10 +274,6 @@ static void __cqhci_enable(struct cqhci_host *cq_host)
 
 	cqhci_set_irqs(cq_host, 0);
 
-	cqcfg |= CQHCI_ENABLE;
-
-	cqhci_writel(cq_host, cqcfg, CQHCI_CFG);
-
 	mmc->cqe_on = true;
 
 	if (cq_host->ops->enable)
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

[RFC 2/4] mmc: sdhci-msm: Add CQHCI support for sdhci-msm

[Ritesh Harjani]

This adds CQHCI support for sdhci-msm platforms.

Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
---
 .../devicetree/bindings/mmc/sdhci-msm.txt          |  1 +
 drivers/mmc/host/Kconfig                           |  1 +
 drivers/mmc/host/sdhci-msm.c                       | 90 +++++++++++++++++++++-
 3 files changed, 91 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
index 0576264..897294f 100644
--- a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
+++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
@@ -5,6 +5,7 @@ and the properties used by the sdhci-msm driver.
 
 Required properties:
 - compatible: Should contain "qcom,sdhci-msm-v4".
+- compatible: "qcom,sdhci-msm-cqe" - Optional in case the controller support CQE.
 - reg: Base address and length of the register in the following order:
 	- Host controller register map (required)
 	- SD Core register map (required)
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index b8c9ea5..a2524c7 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -430,6 +430,7 @@ config MMC_SDHCI_MSM
 	tristate "Qualcomm SDHCI Controller Support"
 	depends on ARCH_QCOM || (ARM && COMPILE_TEST)
 	depends on MMC_SDHCI_PLTFM
+	select MMC_CQHCI
 	help
 	  This selects the Secure Digital Host Controller Interface (SDHCI)
 	  support present in Qualcomm SOCs. The controller supports
diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index df66724..346cdfb 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -23,6 +23,7 @@
 #include <linux/iopoll.h>
 
 #include "sdhci-pltfm.h"
+#include "cqhci.h"
 
 #define CORE_MCI_VERSION		0x50
 #define CORE_VERSION_MAJOR_SHIFT	28
@@ -1092,8 +1093,87 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
 	__sdhci_msm_set_clock(host, clock);
 }
 
+/*****************************************************************************\
+ *                                                                           *
+ * MSM Command Queue Engine (CQE)                                            *
+ *                                                                           *
+\*****************************************************************************/
+
+static u32 sdhci_msm_cqe_irq(struct sdhci_host *host, u32 intmask)
+{
+	int cmd_error = 0;
+	int data_error = 0;
+
+	if (!sdhci_cqe_irq(host, intmask, &cmd_error, &data_error))
+		return intmask;
+
+	cqhci_irq(host->mmc, intmask, cmd_error, data_error);
+	return 0;
+}
+
+static const struct cqhci_host_ops sdhci_msm_cqhci_ops = {
+	.enable		= sdhci_cqe_enable,
+	.disable	= sdhci_cqe_disable,
+};
+
+#ifdef CONFIG_MMC_CQHCI
+static int sdhci_msm_cqe_add_host(struct sdhci_host *host,
+				struct platform_device *pdev)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+	struct cqhci_host *cq_host;
+	bool dma64;
+	int ret;
+
+	ret = sdhci_setup_host(host);
+	if (ret)
+		return ret;
+
+	cq_host = cqhci_pltfm_init(pdev);
+	if (IS_ERR(cq_host)) {
+		ret = PTR_ERR(cq_host);
+		dev_err(&pdev->dev, "cqhci-pltfm init: failed: %d\n", ret);
+		goto cleanup;
+	}
+
+	msm_host->mmc->caps2 |= MMC_CAP2_CQE;
+	cq_host->ops = &sdhci_msm_cqhci_ops;
+
+	dma64 = host->flags & SDHCI_USE_64_BIT_DMA;
+	if (dma64)
+		cq_host->caps |= CQHCI_TASK_DESC_SZ_128;
+
+	ret = cqhci_init(cq_host, host->mmc, dma64);
+	if (ret) {
+		dev_err(&pdev->dev, "%s: CQE init: failed (%d)\n", mmc_hostname(host->mmc),
+				ret);
+		goto cleanup;
+	}
+
+	ret = __sdhci_add_host(host);
+	if (ret)
+		goto cleanup;
+
+	dev_info(&pdev->dev, "%s: CQE init: success\n", mmc_hostname(host->mmc));
+	return ret;
+
+cleanup:
+	sdhci_cleanup_host(host);
+	return ret;
+}
+#else
+static void sdhci_msm_cqe_add_host(struct sdhci_host *host,
+				struct platform_device *pdev)
+{
+	dev_warn(&pdev->dev, "CQE config not enabled, defaulting to sdhci\n");
+	return sdhci_add_host(host);
+}
+#endif /* CONFIG_MMC_CQHCI */
+
 static const struct of_device_id sdhci_msm_dt_match[] = {
 	{ .compatible = "qcom,sdhci-msm-v4" },
+	{ .compatible = "qcom,sdhci-msm-cqe" },
 	{},
 };
 
@@ -1107,6 +1187,7 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
 	.set_bus_width = sdhci_set_bus_width,
 	.set_uhs_signaling = sdhci_msm_set_uhs_signaling,
 	.voltage_switch = sdhci_msm_voltage_switch,
+	.irq			= sdhci_msm_cqe_irq,
 };
 
 static const struct sdhci_pltfm_data sdhci_msm_pdata = {
@@ -1125,6 +1206,7 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 	struct sdhci_pltfm_host *pltfm_host;
 	struct sdhci_msm_host *msm_host;
 	struct resource *core_memres;
+	struct device_node *node = pdev->dev.of_node;
 	int ret;
 	u16 host_version, core_minor;
 	u32 core_version, config;
@@ -1277,7 +1359,13 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 	pm_runtime_use_autosuspend(&pdev->dev);
 
 	host->mmc_host_ops.execute_tuning = sdhci_msm_execute_tuning;
-	ret = sdhci_add_host(host);
+
+	if (of_device_is_compatible(node, "qcom,sdhci-msm-cqe")) {
+		dev_dbg(&pdev->dev, "node with qcom,sdhci-msm-cqe\n");
+		ret = sdhci_msm_cqe_add_host(host, pdev);
+	} else {
+		ret = sdhci_add_host(host);
+	}
 	if (ret)
 		goto pm_runtime_disable;
 
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

[RFC 3/4] mmc: sdhci-msm: Change the desc_sz on cqe_enable/disable.

[Ritesh Harjani]

When CMDQ is halted the HW expects descriptor size to
be same which is using in CMDQ mode.
Thus adjust the desc_sz of sdhci accordingly.

Without this patch below command gives ADMA error
when CQE is enabled.
cat /sys/kernel/debug/mmc0/mmc0:0001/ext_csd

Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
---
 drivers/mmc/host/sdhci-msm.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 346cdfb..baa3409 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -1111,9 +1111,29 @@ static u32 sdhci_msm_cqe_irq(struct sdhci_host *host, u32 intmask)
 	return 0;
 }
 
+void sdhci_msm_cqe_enable(struct mmc_host *mmc)
+{
+	struct sdhci_host *host = mmc_priv(mmc);
+
+	if (host->flags & SDHCI_USE_64_BIT_DMA)
+		host->desc_sz = 12;
+
+	sdhci_cqe_enable(mmc);
+}
+
+void sdhci_msm_cqe_disable(struct mmc_host *mmc, bool recovery)
+{
+	struct sdhci_host *host = mmc_priv(mmc);
+
+	if (host->flags & SDHCI_USE_64_BIT_DMA)
+		host->desc_sz = 16;
+
+	sdhci_cqe_disable(mmc, recovery);
+}
+
 static const struct cqhci_host_ops sdhci_msm_cqhci_ops = {
-	.enable		= sdhci_cqe_enable,
-	.disable	= sdhci_cqe_disable,
+	.enable		= sdhci_msm_cqe_enable,
+	.disable	= sdhci_msm_cqe_disable,
 };
 
 #ifdef CONFIG_MMC_CQHCI
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

[RFC 4/4] mmc: sdhci-msm: Handle unexpected interrupt case on enabling legacy IRQs on CQE halt

[Ritesh Harjani]

There is a case when enabling the legacy IRQs and halting CQE is
resuling into a command response interrupt without any command in
progress. This patch handles such case here.

Error signature without this patch:-
mmc0: Got command interrupt 0x00000001 even though no command operation
was in progress.

Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
---
 drivers/mmc/host/sdhci-msm.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index baa3409..8fdc2c0 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -1124,10 +1124,21 @@ void sdhci_msm_cqe_enable(struct mmc_host *mmc)
 void sdhci_msm_cqe_disable(struct mmc_host *mmc, bool recovery)
 {
 	struct sdhci_host *host = mmc_priv(mmc);
+	unsigned long flags;
+	u32 ctrl;
 
 	if (host->flags & SDHCI_USE_64_BIT_DMA)
 		host->desc_sz = 16;
 
+	spin_lock_irqsave(&host->lock, flags);
+
+	ctrl = sdhci_readl(host, SDHCI_INT_ENABLE);
+	ctrl |= SDHCI_INT_RESPONSE;
+	sdhci_writel(host,  ctrl, SDHCI_INT_ENABLE);
+	sdhci_writel(host, SDHCI_INT_RESPONSE, SDHCI_INT_STATUS);
+
+	spin_unlock_irqrestore(&host->lock, flags);
+
 	sdhci_cqe_disable(mmc, recovery);
 }
 
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [RFC 1/4] mmc: cqhci: Move CQHCI_ENABLE before setting TDLBA/TDLBAU

[Adrian Hunter]

On 30/08/17 16:04, Ritesh Harjani wrote:
> Without this patch the CQHCI registers are getting reset
> again.
> 
> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
> ---
>  drivers/mmc/host/cqhci.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/host/cqhci.c b/drivers/mmc/host/cqhci.c
> index 8650a13..2a7351c 100644
> --- a/drivers/mmc/host/cqhci.c
> +++ b/drivers/mmc/host/cqhci.c
> @@ -262,6 +262,9 @@ static void __cqhci_enable(struct cqhci_host *cq_host)
>  
>  	cqhci_writel(cq_host, cqcfg, CQHCI_CFG);
>  
> +	cqcfg |= CQHCI_ENABLE;
> +	cqhci_writel(cq_host, cqcfg, CQHCI_CFG);
That doesn't follow the flow in the specification B.6.1. Command Queuing
Initialization Sequence.  Also in B.3.5 Task List, the spec. says "Changing
the value of TDLBA is not allowed when command queue mode is enabled."

So you will need to add a quirk for this.

> +
>  	cqhci_writel(cq_host, lower_32_bits(cq_host->desc_dma_base),
>  		     CQHCI_TDLBA);
>  	cqhci_writel(cq_host, upper_32_bits(cq_host->desc_dma_base),
> @@ -271,10 +274,6 @@ static void __cqhci_enable(struct cqhci_host *cq_host)
>  
>  	cqhci_set_irqs(cq_host, 0);
>  
> -	cqcfg |= CQHCI_ENABLE;
> -
> -	cqhci_writel(cq_host, cqcfg, CQHCI_CFG);
> -
>  	mmc->cqe_on = true;
>  
>  	if (cq_host->ops->enable)
> 

Re: [RFC 3/4] mmc: sdhci-msm: Change the desc_sz on cqe_enable/disable.

[Adrian Hunter]

On 30/08/17 16:04, Ritesh Harjani wrote:
> When CMDQ is halted the HW expects descriptor size to
> be same which is using in CMDQ mode.
> Thus adjust the desc_sz of sdhci accordingly.
> 
> Without this patch below command gives ADMA error
> when CQE is enabled.
> cat /sys/kernel/debug/mmc0/mmc0:0001/ext_csd
> 
> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
> ---
>  drivers/mmc/host/sdhci-msm.c | 24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 346cdfb..baa3409 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -1111,9 +1111,29 @@ static u32 sdhci_msm_cqe_irq(struct sdhci_host *host, u32 intmask)
>  	return 0;
>  }
>  
> +void sdhci_msm_cqe_enable(struct mmc_host *mmc)
> +{
> +	struct sdhci_host *host = mmc_priv(mmc);
> +
> +	if (host->flags & SDHCI_USE_64_BIT_DMA)
> +		host->desc_sz = 12;

This has no effect.

> +
> +	sdhci_cqe_enable(mmc);
> +}
> +
> +void sdhci_msm_cqe_disable(struct mmc_host *mmc, bool recovery)
> +{
> +	struct sdhci_host *host = mmc_priv(mmc);
> +
> +	if (host->flags & SDHCI_USE_64_BIT_DMA)
> +		host->desc_sz = 16;

You can't change the descriptor size this way.  If you need 128-bit ADMA
descriptors it must be done in sdhci_setup_host().

> +
> +	sdhci_cqe_disable(mmc, recovery);
> +}
> +
>  static const struct cqhci_host_ops sdhci_msm_cqhci_ops = {
> -	.enable		= sdhci_cqe_enable,
> -	.disable	= sdhci_cqe_disable,
> +	.enable		= sdhci_msm_cqe_enable,
> +	.disable	= sdhci_msm_cqe_disable,
>  };
>  
>  #ifdef CONFIG_MMC_CQHCI
> 

Re: [RFC 2/4] mmc: sdhci-msm: Add CQHCI support for sdhci-msm

[Adrian Hunter]

On 30/08/17 16:04, Ritesh Harjani wrote:
> This adds CQHCI support for sdhci-msm platforms.
> 
> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
> ---
>  .../devicetree/bindings/mmc/sdhci-msm.txt          |  1 +
>  drivers/mmc/host/Kconfig                           |  1 +
>  drivers/mmc/host/sdhci-msm.c                       | 90 +++++++++++++++++++++-
>  3 files changed, 91 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> index 0576264..897294f 100644
> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> @@ -5,6 +5,7 @@ and the properties used by the sdhci-msm driver.
>  
>  Required properties:
>  - compatible: Should contain "qcom,sdhci-msm-v4".
> +- compatible: "qcom,sdhci-msm-cqe" - Optional in case the controller support CQE.
>  - reg: Base address and length of the register in the following order:
>  	- Host controller register map (required)
>  	- SD Core register map (required)
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index b8c9ea5..a2524c7 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -430,6 +430,7 @@ config MMC_SDHCI_MSM
>  	tristate "Qualcomm SDHCI Controller Support"
>  	depends on ARCH_QCOM || (ARM && COMPILE_TEST)
>  	depends on MMC_SDHCI_PLTFM
> +	select MMC_CQHCI
>  	help
>  	  This selects the Secure Digital Host Controller Interface (SDHCI)
>  	  support present in Qualcomm SOCs. The controller supports
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index df66724..346cdfb 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -23,6 +23,7 @@
>  #include <linux/iopoll.h>
>  
>  #include "sdhci-pltfm.h"
> +#include "cqhci.h"
>  
>  #define CORE_MCI_VERSION		0x50
>  #define CORE_VERSION_MAJOR_SHIFT	28
> @@ -1092,8 +1093,87 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>  	__sdhci_msm_set_clock(host, clock);
>  }
>  
> +/*****************************************************************************\
> + *                                                                           *
> + * MSM Command Queue Engine (CQE)                                            *
> + *                                                                           *
> +\*****************************************************************************/
> +
> +static u32 sdhci_msm_cqe_irq(struct sdhci_host *host, u32 intmask)
> +{
> +	int cmd_error = 0;
> +	int data_error = 0;
> +
> +	if (!sdhci_cqe_irq(host, intmask, &cmd_error, &data_error))
> +		return intmask;
> +
> +	cqhci_irq(host->mmc, intmask, cmd_error, data_error);
> +	return 0;
> +}
> +
> +static const struct cqhci_host_ops sdhci_msm_cqhci_ops = {
> +	.enable		= sdhci_cqe_enable,
> +	.disable	= sdhci_cqe_disable,
> +};
> +
> +#ifdef CONFIG_MMC_CQHCI

If you select MMC_CQHCI then this #ifdef is not needed

> +static int sdhci_msm_cqe_add_host(struct sdhci_host *host,
> +				struct platform_device *pdev)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +	struct cqhci_host *cq_host;
> +	bool dma64;
> +	int ret;
> +
> +	ret = sdhci_setup_host(host);
> +	if (ret)
> +		return ret;
> +
> +	cq_host = cqhci_pltfm_init(pdev);
> +	if (IS_ERR(cq_host)) {
> +		ret = PTR_ERR(cq_host);
> +		dev_err(&pdev->dev, "cqhci-pltfm init: failed: %d\n", ret);
> +		goto cleanup;
> +	}
> +
> +	msm_host->mmc->caps2 |= MMC_CAP2_CQE;

If DCMD works you need MMC_CAP2_CQE_DCMD also

> +	cq_host->ops = &sdhci_msm_cqhci_ops;
> +
> +	dma64 = host->flags & SDHCI_USE_64_BIT_DMA;
> +	if (dma64)
> +		cq_host->caps |= CQHCI_TASK_DESC_SZ_128;
> +
> +	ret = cqhci_init(cq_host, host->mmc, dma64);
> +	if (ret) {
> +		dev_err(&pdev->dev, "%s: CQE init: failed (%d)\n", mmc_hostname(host->mmc),
> +				ret);
> +		goto cleanup;
> +	}
> +
> +	ret = __sdhci_add_host(host);
> +	if (ret)
> +		goto cleanup;
> +
> +	dev_info(&pdev->dev, "%s: CQE init: success\n", mmc_hostname(host->mmc));
> +	return ret;
> +
> +cleanup:
> +	sdhci_cleanup_host(host);
> +	return ret;
> +}
> +#else
> +static void sdhci_msm_cqe_add_host(struct sdhci_host *host,
> +				struct platform_device *pdev)
> +{
> +	dev_warn(&pdev->dev, "CQE config not enabled, defaulting to sdhci\n");
> +	return sdhci_add_host(host);
> +}
> +#endif /* CONFIG_MMC_CQHCI */
> +
>  static const struct of_device_id sdhci_msm_dt_match[] = {
>  	{ .compatible = "qcom,sdhci-msm-v4" },
> +	{ .compatible = "qcom,sdhci-msm-cqe" },
>  	{},
>  };
>  
> @@ -1107,6 +1187,7 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>  	.set_bus_width = sdhci_set_bus_width,
>  	.set_uhs_signaling = sdhci_msm_set_uhs_signaling,
>  	.voltage_switch = sdhci_msm_voltage_switch,
> +	.irq			= sdhci_msm_cqe_irq,
>  };
>  
>  static const struct sdhci_pltfm_data sdhci_msm_pdata = {
> @@ -1125,6 +1206,7 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>  	struct sdhci_pltfm_host *pltfm_host;
>  	struct sdhci_msm_host *msm_host;
>  	struct resource *core_memres;
> +	struct device_node *node = pdev->dev.of_node;
>  	int ret;
>  	u16 host_version, core_minor;
>  	u32 core_version, config;
> @@ -1277,7 +1359,13 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>  	pm_runtime_use_autosuspend(&pdev->dev);
>  
>  	host->mmc_host_ops.execute_tuning = sdhci_msm_execute_tuning;
> -	ret = sdhci_add_host(host);
> +
> +	if (of_device_is_compatible(node, "qcom,sdhci-msm-cqe")) {
> +		dev_dbg(&pdev->dev, "node with qcom,sdhci-msm-cqe\n");
> +		ret = sdhci_msm_cqe_add_host(host, pdev);
> +	} else {
> +		ret = sdhci_add_host(host);
> +	}
>  	if (ret)
>  		goto pm_runtime_disable;
>  
> 

Re: [RFC 4/4] mmc: sdhci-msm: Handle unexpected interrupt case on enabling legacy IRQs on CQE halt

[Adrian Hunter]

On 30/08/17 16:04, Ritesh Harjani wrote:
> There is a case when enabling the legacy IRQs and halting CQE is
> resuling into a command response interrupt without any command in
> progress. This patch handles such case here.
> 
> Error signature without this patch:-
> mmc0: Got command interrupt 0x00000001 even though no command operation
> was in progress.
> 
> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>

Seems fine, but this is a necessary part of enabling i.e. put all the
sdhci-msm changes into one patch.

> ---
>  drivers/mmc/host/sdhci-msm.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index baa3409..8fdc2c0 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -1124,10 +1124,21 @@ void sdhci_msm_cqe_enable(struct mmc_host *mmc)
>  void sdhci_msm_cqe_disable(struct mmc_host *mmc, bool recovery)
>  {
>  	struct sdhci_host *host = mmc_priv(mmc);
> +	unsigned long flags;
> +	u32 ctrl;
>  
>  	if (host->flags & SDHCI_USE_64_BIT_DMA)
>  		host->desc_sz = 16;
>  
> +	spin_lock_irqsave(&host->lock, flags);
> +

Could use a comment here.

> +	ctrl = sdhci_readl(host, SDHCI_INT_ENABLE);
> +	ctrl |= SDHCI_INT_RESPONSE;
> +	sdhci_writel(host,  ctrl, SDHCI_INT_ENABLE);
> +	sdhci_writel(host, SDHCI_INT_RESPONSE, SDHCI_INT_STATUS);
> +
> +	spin_unlock_irqrestore(&host->lock, flags);
> +
>  	sdhci_cqe_disable(mmc, recovery);
>  }
>  
> 

Re: [RFC 0/4] mmc: sdhci-msm: Add CQE support for sdhci-msm

[Adrian Hunter]

On 30/08/17 16:04, Ritesh Harjani wrote:
> Hi All,
> 
> Please ignore the previous patch series from a wrong email
> address. Stupid gitconfig issue. Apologies for the spam.
> 
> This is RFC patch series based on top of ulfh_mmc/cmdq branch
> which is based upon Adrian's CMDQ patch series.
> 
> Below patch series enables CQE for sdhci-msm platform.
> This has been tested on internal 8996 MTP which has CMDQ support.
> 
> Fixes w.r.t. CMDQ:-
> There are some patches identified which were required atleast on
> MSM platform. I am not sure if these are required for any other
> CQE platform or not. Patchset 1, 3 & 4 commit text describes
> the problems.
> 
> Performance related:- 
> I gave one small shot for performance and the numbers were not looking good.
> So, unless I have tested for performance completely, I should not discuss 
> on performance numbers as of now with this patchset. 
> I can try doing some more performance testing and post the results -
> though this may take some while.

You might also need custom Send Status Configuration.

> 
> I used below test script for random read/write test.
> 
> *randwrite-test-script*
> [global]
> bs=32k
> size=1g
> rw=randwrite
> direct=1
> directory=/data/fiotest

Random write results can vary a lot.  It is important to know if the eMMC
has lots of un-mapped blocks or not. e.g. for ext4 is the "-o discard"
option being used.  I find I get more consistent results if I always have
discards enabled.

> 
> [file1]
> filename=singlefile1
> 
> *randread-test-script*
> [global]
> bs=32k
> size=1g
> rw=randread
> directory=/data/fiotest

If you don't set numjobs > 1 then there is little benefit of the queue.
Also still need direct=1

> 
> [file1]
> filename=singlefile1
> 
> @Adrian,
> Thanks a lot for pursuing and bringing CMDQ patch series to it's final stages :)
> 
> 
> Ritesh Harjani (4):
>   mmc: cqhci: Move CQHCI_ENABLE before setting TDLBA/TDLBAU
>   mmc: sdhci-msm: Add CQHCI support for sdhci-msm
>   mmc: sdhci-msm: Change the desc_sz on cqe_enable/disable.
>   mmc: sdhci-msm: Handle unexpected interrupt case on enabling legacy
>     IRQs on CQE halt
> 
>  .../devicetree/bindings/mmc/sdhci-msm.txt          |   1 +
>  drivers/mmc/host/Kconfig                           |   1 +
>  drivers/mmc/host/cqhci.c                           |   7 +-
>  drivers/mmc/host/sdhci-msm.c                       | 121 ++++++++++++++++++++-
>  4 files changed, 125 insertions(+), 5 deletions(-)
> 

Re: [RFC 1/4] mmc: cqhci: Move CQHCI_ENABLE before setting TDLBA/TDLBAU

[Ritesh Harjani]

On 8/31/2017 11:31 AM, Adrian Hunter wrote:
> On 30/08/17 16:04, Ritesh Harjani wrote:
>> Without this patch the CQHCI registers are getting reset
>> again.
>>
>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>> ---
>>   drivers/mmc/host/cqhci.c | 7 +++----
>>   1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mmc/host/cqhci.c b/drivers/mmc/host/cqhci.c
>> index 8650a13..2a7351c 100644
>> --- a/drivers/mmc/host/cqhci.c
>> +++ b/drivers/mmc/host/cqhci.c
>> @@ -262,6 +262,9 @@ static void __cqhci_enable(struct cqhci_host *cq_host)
>>   
>>   	cqhci_writel(cq_host, cqcfg, CQHCI_CFG);
>>   
>> +	cqcfg |= CQHCI_ENABLE;
>> +	cqhci_writel(cq_host, cqcfg, CQHCI_CFG);
> That doesn't follow the flow in the specification B.6.1. Command Queuing
> Initialization Sequence.  Also in B.3.5 Task List, the spec. says "Changing
> the value of TDLBA is not allowed when command queue mode is enabled."
> 
> So you will need to add a quirk for this.

Sure. thanks.

> 
>> +
>>   	cqhci_writel(cq_host, lower_32_bits(cq_host->desc_dma_base),
>>   		     CQHCI_TDLBA);
>>   	cqhci_writel(cq_host, upper_32_bits(cq_host->desc_dma_base),
>> @@ -271,10 +274,6 @@ static void __cqhci_enable(struct cqhci_host *cq_host)
>>   
>>   	cqhci_set_irqs(cq_host, 0);
>>   
>> -	cqcfg |= CQHCI_ENABLE;
>> -
>> -	cqhci_writel(cq_host, cqcfg, CQHCI_CFG);
>> -
>>   	mmc->cqe_on = true;
>>   
>>   	if (cq_host->ops->enable)
>>
> 

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, 
Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a 
Linux Foundation Collaborative Project.

Re: [RFC 2/4] mmc: sdhci-msm: Add CQHCI support for sdhci-msm

[Ritesh Harjani]

On 8/31/2017 12:55 PM, Adrian Hunter wrote:
> On 30/08/17 16:04, Ritesh Harjani wrote:
>> This adds CQHCI support for sdhci-msm platforms.
>>
>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>> ---
>>   .../devicetree/bindings/mmc/sdhci-msm.txt          |  1 +
>>   drivers/mmc/host/Kconfig                           |  1 +
>>   drivers/mmc/host/sdhci-msm.c                       | 90 +++++++++++++++++++++-
>>   3 files changed, 91 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>> index 0576264..897294f 100644
>> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>> @@ -5,6 +5,7 @@ and the properties used by the sdhci-msm driver.
>>   
>>   Required properties:
>>   - compatible: Should contain "qcom,sdhci-msm-v4".
>> +- compatible: "qcom,sdhci-msm-cqe" - Optional in case the controller support CQE.
>>   - reg: Base address and length of the register in the following order:
>>   	- Host controller register map (required)
>>   	- SD Core register map (required)
>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>> index b8c9ea5..a2524c7 100644
>> --- a/drivers/mmc/host/Kconfig
>> +++ b/drivers/mmc/host/Kconfig
>> @@ -430,6 +430,7 @@ config MMC_SDHCI_MSM
>>   	tristate "Qualcomm SDHCI Controller Support"
>>   	depends on ARCH_QCOM || (ARM && COMPILE_TEST)
>>   	depends on MMC_SDHCI_PLTFM
>> +	select MMC_CQHCI
>>   	help
>>   	  This selects the Secure Digital Host Controller Interface (SDHCI)
>>   	  support present in Qualcomm SOCs. The controller supports
>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>> index df66724..346cdfb 100644
>> --- a/drivers/mmc/host/sdhci-msm.c
>> +++ b/drivers/mmc/host/sdhci-msm.c
>> @@ -23,6 +23,7 @@
>>   #include <linux/iopoll.h>
>>   
>>   #include "sdhci-pltfm.h"
>> +#include "cqhci.h"
>>   
>>   #define CORE_MCI_VERSION		0x50
>>   #define CORE_VERSION_MAJOR_SHIFT	28
>> @@ -1092,8 +1093,87 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>>   	__sdhci_msm_set_clock(host, clock);
>>   }
>>   
>> +/*****************************************************************************\
>> + *                                                                           *
>> + * MSM Command Queue Engine (CQE)                                            *
>> + *                                                                           *
>> +\*****************************************************************************/
>> +
>> +static u32 sdhci_msm_cqe_irq(struct sdhci_host *host, u32 intmask)
>> +{
>> +	int cmd_error = 0;
>> +	int data_error = 0;
>> +
>> +	if (!sdhci_cqe_irq(host, intmask, &cmd_error, &data_error))
>> +		return intmask;
>> +
>> +	cqhci_irq(host->mmc, intmask, cmd_error, data_error);
>> +	return 0;
>> +}
>> +
>> +static const struct cqhci_host_ops sdhci_msm_cqhci_ops = {
>> +	.enable		= sdhci_cqe_enable,
>> +	.disable	= sdhci_cqe_disable,
>> +};
>> +
>> +#ifdef CONFIG_MMC_CQHCI
> 
> If you select MMC_CQHCI then this #ifdef is not needed

Yes. Thanks.


> 
>> +static int sdhci_msm_cqe_add_host(struct sdhci_host *host,
>> +				struct platform_device *pdev)
>> +{
>> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>> +	struct cqhci_host *cq_host;
>> +	bool dma64;
>> +	int ret;
>> +
>> +	ret = sdhci_setup_host(host);
>> +	if (ret)
>> +		return ret;
>> +
>> +	cq_host = cqhci_pltfm_init(pdev);
>> +	if (IS_ERR(cq_host)) {
>> +		ret = PTR_ERR(cq_host);
>> +		dev_err(&pdev->dev, "cqhci-pltfm init: failed: %d\n", ret);
>> +		goto cleanup;
>> +	}
>> +
>> +	msm_host->mmc->caps2 |= MMC_CAP2_CQE;
> 
> If DCMD works you need MMC_CAP2_CQE_DCMD also

Yes, I could not test DCMD. Sure I will check it once.

> 
>> +	cq_host->ops = &sdhci_msm_cqhci_ops;
>> +
>> +	dma64 = host->flags & SDHCI_USE_64_BIT_DMA;
>> +	if (dma64)
>> +		cq_host->caps |= CQHCI_TASK_DESC_SZ_128;
>> +
>> +	ret = cqhci_init(cq_host, host->mmc, dma64);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "%s: CQE init: failed (%d)\n", mmc_hostname(host->mmc),
>> +				ret);
>> +		goto cleanup;
>> +	}
>> +
>> +	ret = __sdhci_add_host(host);
>> +	if (ret)
>> +		goto cleanup;
>> +
>> +	dev_info(&pdev->dev, "%s: CQE init: success\n", mmc_hostname(host->mmc));
>> +	return ret;
>> +
>> +cleanup:
>> +	sdhci_cleanup_host(host);
>> +	return ret;
>> +}
>> +#else
>> +static void sdhci_msm_cqe_add_host(struct sdhci_host *host,
>> +				struct platform_device *pdev)
>> +{
>> +	dev_warn(&pdev->dev, "CQE config not enabled, defaulting to sdhci\n");
>> +	return sdhci_add_host(host);
>> +}
>> +#endif /* CONFIG_MMC_CQHCI */
>> +
>>   static const struct of_device_id sdhci_msm_dt_match[] = {
>>   	{ .compatible = "qcom,sdhci-msm-v4" },
>> +	{ .compatible = "qcom,sdhci-msm-cqe" },
>>   	{},
>>   };
>>   
>> @@ -1107,6 +1187,7 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>>   	.set_bus_width = sdhci_set_bus_width,
>>   	.set_uhs_signaling = sdhci_msm_set_uhs_signaling,
>>   	.voltage_switch = sdhci_msm_voltage_switch,
>> +	.irq			= sdhci_msm_cqe_irq,
>>   };
>>   
>>   static const struct sdhci_pltfm_data sdhci_msm_pdata = {
>> @@ -1125,6 +1206,7 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>>   	struct sdhci_pltfm_host *pltfm_host;
>>   	struct sdhci_msm_host *msm_host;
>>   	struct resource *core_memres;
>> +	struct device_node *node = pdev->dev.of_node;
>>   	int ret;
>>   	u16 host_version, core_minor;
>>   	u32 core_version, config;
>> @@ -1277,7 +1359,13 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>>   	pm_runtime_use_autosuspend(&pdev->dev);
>>   
>>   	host->mmc_host_ops.execute_tuning = sdhci_msm_execute_tuning;
>> -	ret = sdhci_add_host(host);
>> +
>> +	if (of_device_is_compatible(node, "qcom,sdhci-msm-cqe")) {
>> +		dev_dbg(&pdev->dev, "node with qcom,sdhci-msm-cqe\n");
>> +		ret = sdhci_msm_cqe_add_host(host, pdev);
>> +	} else {
>> +		ret = sdhci_add_host(host);
>> +	}
>>   	if (ret)
>>   		goto pm_runtime_disable;
>>   
>>
> 

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, 
Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a 
Linux Foundation Collaborative Project.

Re: [RFC 3/4] mmc: sdhci-msm: Change the desc_sz on cqe_enable/disable.

[Ritesh Harjani]

On 8/31/2017 12:12 PM, Adrian Hunter wrote:
> On 30/08/17 16:04, Ritesh Harjani wrote:
>> When CMDQ is halted the HW expects descriptor size to
>> be same which is using in CMDQ mode.
>> Thus adjust the desc_sz of sdhci accordingly.
>>
>> Without this patch below command gives ADMA error
>> when CQE is enabled.
>> cat /sys/kernel/debug/mmc0/mmc0:0001/ext_csd
>>
>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>> ---
>>   drivers/mmc/host/sdhci-msm.c | 24 ++++++++++++++++++++++--
>>   1 file changed, 22 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>> index 346cdfb..baa3409 100644
>> --- a/drivers/mmc/host/sdhci-msm.c
>> +++ b/drivers/mmc/host/sdhci-msm.c
>> @@ -1111,9 +1111,29 @@ static u32 sdhci_msm_cqe_irq(struct sdhci_host *host, u32 intmask)
>>   	return 0;
>>   }
>>   
>> +void sdhci_msm_cqe_enable(struct mmc_host *mmc)
>> +{
>> +	struct sdhci_host *host = mmc_priv(mmc);
>> +
>> +	if (host->flags & SDHCI_USE_64_BIT_DMA)
>> +		host->desc_sz = 12;
> 
> This has no effect.

Yes, I added it for the sake of completion.
Sure I will check on this.

> 
>> +
>> +	sdhci_cqe_enable(mmc);
>> +}
>> +
>> +void sdhci_msm_cqe_disable(struct mmc_host *mmc, bool recovery)
>> +{
>> +	struct sdhci_host *host = mmc_priv(mmc);
>> +
>> +	if (host->flags & SDHCI_USE_64_BIT_DMA)
>> +		host->desc_sz = 16;
> 
> You can't change the descriptor size this way.  If you need 128-bit ADMA
> descriptors it must be done in sdhci_setup_host().

Let me again check on this once. I will get back then.
Thanks.

> 
>> +
>> +	sdhci_cqe_disable(mmc, recovery);
>> +}
>> +
>>   static const struct cqhci_host_ops sdhci_msm_cqhci_ops = {
>> -	.enable		= sdhci_cqe_enable,
>> -	.disable	= sdhci_cqe_disable,
>> +	.enable		= sdhci_msm_cqe_enable,
>> +	.disable	= sdhci_msm_cqe_disable,
>>   };
>>   
>>   #ifdef CONFIG_MMC_CQHCI
>>
> 

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, 
Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a 
Linux Foundation Collaborative Project.

Re: [RFC 4/4] mmc: sdhci-msm: Handle unexpected interrupt case on enabling legacy IRQs on CQE halt

[Ritesh Harjani]

On 8/31/2017 1:05 PM, Adrian Hunter wrote:
> On 30/08/17 16:04, Ritesh Harjani wrote:
>> There is a case when enabling the legacy IRQs and halting CQE is
>> resuling into a command response interrupt without any command in
>> progress. This patch handles such case here.
>>
>> Error signature without this patch:-
>> mmc0: Got command interrupt 0x00000001 even though no command operation
>> was in progress.
>>
>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
> 
> Seems fine, but this is a necessary part of enabling i.e. put all the
> sdhci-msm changes into one patch.

Yes, sure.

> 
>> ---
>>   drivers/mmc/host/sdhci-msm.c | 11 +++++++++++
>>   1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>> index baa3409..8fdc2c0 100644
>> --- a/drivers/mmc/host/sdhci-msm.c
>> +++ b/drivers/mmc/host/sdhci-msm.c
>> @@ -1124,10 +1124,21 @@ void sdhci_msm_cqe_enable(struct mmc_host *mmc)
>>   void sdhci_msm_cqe_disable(struct mmc_host *mmc, bool recovery)
>>   {
>>   	struct sdhci_host *host = mmc_priv(mmc);
>> +	unsigned long flags;
>> +	u32 ctrl;
>>   
>>   	if (host->flags & SDHCI_USE_64_BIT_DMA)
>>   		host->desc_sz = 16;
>>   
>> +	spin_lock_irqsave(&host->lock, flags);
>> +
> 
> Could use a comment here.

Ok. Thanks.

> 
>> +	ctrl = sdhci_readl(host, SDHCI_INT_ENABLE);
>> +	ctrl |= SDHCI_INT_RESPONSE;
>> +	sdhci_writel(host,  ctrl, SDHCI_INT_ENABLE);
>> +	sdhci_writel(host, SDHCI_INT_RESPONSE, SDHCI_INT_STATUS);
>> +
>> +	spin_unlock_irqrestore(&host->lock, flags);
>> +
>>   	sdhci_cqe_disable(mmc, recovery);
>>   }
>>   
>>
> 

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, 
Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a 
Linux Foundation Collaborative Project.

Re: [RFC 0/4] mmc: sdhci-msm: Add CQE support for sdhci-msm

[Ritesh Harjani]

On 8/31/2017 2:09 PM, Adrian Hunter wrote:
> On 30/08/17 16:04, Ritesh Harjani wrote:
>> Hi All,
>>
>> Please ignore the previous patch series from a wrong email
>> address. Stupid gitconfig issue. Apologies for the spam.
>>
>> This is RFC patch series based on top of ulfh_mmc/cmdq branch
>> which is based upon Adrian's CMDQ patch series.
>>
>> Below patch series enables CQE for sdhci-msm platform.
>> This has been tested on internal 8996 MTP which has CMDQ support.
>>
>> Fixes w.r.t. CMDQ:-
>> There are some patches identified which were required atleast on
>> MSM platform. I am not sure if these are required for any other
>> CQE platform or not. Patchset 1, 3 & 4 commit text describes
>> the problems.
>>
>> Performance related:-
>> I gave one small shot for performance and the numbers were not looking good.
>> So, unless I have tested for performance completely, I should not discuss
>> on performance numbers as of now with this patchset.
>> I can try doing some more performance testing and post the results -
>> though this may take some while.
> 
> You might also need custom Send Status Configuration.

Yes, I will check that once.
I think for the single job I/O this may not would have matter much.
But sure, I will check on this, if re-configuring is needed.

> 
>>
>> I used below test script for random read/write test.
>>
>> *randwrite-test-script*
>> [global]
>> bs=32k
>> size=1g
>> rw=randwrite
>> direct=1
>> directory=/data/fiotest
> 
> Random write results can vary a lot.  It is important to know if the eMMC
> has lots of un-mapped blocks or not. e.g. for ext4 is the "-o discard"
> option being used.  I find I get more consistent results if I always have
> discards enabled.
> 
>>
>> [file1]
>> filename=singlefile1
>>
>> *randread-test-script*
>> [global]
>> bs=32k
>> size=1g
>> rw=randread
>> directory=/data/fiotest
> 
> If you don't set numjobs > 1 then there is little benefit of the queue.
> Also still need direct=1

Yes, silly me. But still I got lower results with single job then.
But anyways let me again check this out. Thanks.

> 
>>
>> [file1]
>> filename=singlefile1
>>
>> @Adrian,
>> Thanks a lot for pursuing and bringing CMDQ patch series to it's final stages :)
>>
>>
>> Ritesh Harjani (4):
>>    mmc: cqhci: Move CQHCI_ENABLE before setting TDLBA/TDLBAU
>>    mmc: sdhci-msm: Add CQHCI support for sdhci-msm
>>    mmc: sdhci-msm: Change the desc_sz on cqe_enable/disable.
>>    mmc: sdhci-msm: Handle unexpected interrupt case on enabling legacy
>>      IRQs on CQE halt
>>
>>   .../devicetree/bindings/mmc/sdhci-msm.txt          |   1 +
>>   drivers/mmc/host/Kconfig                           |   1 +
>>   drivers/mmc/host/cqhci.c                           |   7 +-
>>   drivers/mmc/host/sdhci-msm.c                       | 121 ++++++++++++++++++++-
>>   4 files changed, 125 insertions(+), 5 deletions(-)
>>
> 

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, 
Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a 
Linux Foundation Collaborative Project.

[RFC 0/4] mmc: sdhci-msm: Add CQE support for sdhci-msm

[Ritesh Harjani]

From: Ritesh Harjani <riteshh@codeaurora.org>

Hi All,

This is RFC patch series based on top of ulfh_mmc/cmdq branch
which is based upon Adrian's CMDQ patch series.

Below patch series enables CQE for sdhci-msm platform.
This has been tested on internal 8996 MTP which has CMDQ support.

Fixes w.r.t. CMDQ:-
There are some patches identified which were required atleast on
MSM platform. I am not sure if these are required for any other
CQE platform or not. Patchset 1, 3 & 4 commit text describes
the problems.

Performance related:- 
I gave one small shot for performance and the numbers were not looking good.
So, unless I have tested for performance completely, I should not discuss 
on performance numbers as of now with this patchset. 
I can try doing some more performance testing and post the results -
though this may take some while.

I used below test script for random read/write test.

*randwrite-test-script*
[global]
bs=32k
size=1g
rw=randwrite
direct=1
directory=/data/fiotest

[file1]
filename=singlefile1

*randread-test-script*
[global]
bs=32k
size=1g
rw=randread
directory=/data/fiotest

[file1]
filename=singlefile1

@Adrian,
Thanks a lot for pursuing and bringing CMDQ patch series to it's final stages :)


Ritesh Harjani (4):
  mmc: cqhci: Move CQHCI_ENABLE before setting TDLBA/TDLBAU
  mmc: sdhci-msm: Add CQHCI support for sdhci-msm
  mmc: sdhci-msm: Change the desc_sz on cqe_enable/disable.
  mmc: sdhci-msm: Handle unexpected interrupt case on enabling legacy
    IRQs on CQE halt

 .../devicetree/bindings/mmc/sdhci-msm.txt          |   1 +
 drivers/mmc/host/Kconfig                           |   1 +
 drivers/mmc/host/cqhci.c                           |   7 +-
 drivers/mmc/host/sdhci-msm.c                       | 121 ++++++++++++++++++++-
 4 files changed, 125 insertions(+), 5 deletions(-)

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.