Skip to contentSkip to conversation

mmc: sdhci-msm: Add CQE support for sdhci-msm

RFC In review Ritesh Harjani 2 participants 15 messages Aug 30, 2017 – Sep 2, 2017

Subsystems (5)
Device tree bindings Arm Qualcomm mailing list MMC, SD and SDIO Secure digital host controller interface (SDHCI) driver +1 more subsystems
lore
Share & tools
lore

Ritesh Harjani

author
#0 lore raw

cover mmc: sdhci-msm: Add CQE support for sdhci-msm

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(-)
Signature · 2 lines
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.
author
#1 lore raw

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

Without this patch the CQHCI registers are getting reset again.

Signed-off-by Ritesh Harjani <[email protected]>
---
 drivers/mmc/host/cqhci.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
drivers/mmc/host/cqhci.c +3 -4
index 8650a13..2a7351c 100644
@@ -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)
Signature · 2 lines
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.
maintainer
#2 lore raw

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

Quoted 13 lines · Ritesh Harjani

Without this patch the CQHCI registers are getting reset again.

Signed-off-by Ritesh Harjani <[email protected]>
---
 drivers/mmc/host/cqhci.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
drivers/mmc/host/cqhci.c +0 -0
index 8650a13..2a7351c 100644

drivers/mmc/host/cqhci.c static void __cqhci_enable(struct cqhci_host *cq_host) · line 266

Quoted 5 lines
drivers/mmc/host/cqhci.c +2 -0
@@ -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.

Quoted 16 lines
drivers/mmc/host/cqhci.c +1 -4
+
 	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)
author
#3 lore raw

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

Quoted 17 lines · Adrian Hunter
Quoted 16 lines · Ritesh Harjani

Without this patch the CQHCI registers are getting reset again.

Signed-off-by Ritesh Harjani <[email protected]>
---
  drivers/mmc/host/cqhci.c | 7 +++----
  1 file changed, 3 insertions(+), 4 deletions(-)
drivers/mmc/host/cqhci.c +0 -0
index 8650a13..2a7351c 100644
@@ -262,6 +262,9 @@ static void __cqhci_enable(struct cqhci_host *cq_host)
 
  	cqhci_writel(cq_host, cqcfg, CQHCI_CFG);
Quoted 7 lines

drivers/mmc/host/cqhci.c static void __cqhci_enable(struct cqhci_host *cq_host) · line 266

Quoted 2 lines
drivers/mmc/host/cqhci.c +2 -0
+	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.

Quoted 18 lines
Quoted 17 lines
drivers/mmc/host/cqhci.c +1 -4
+
  	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)
Signature · 2 lines
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.
author
#4 lore raw

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

In reply to Ritesh Harjani

This adds CQHCI support for sdhci-msm platforms.

Signed-off-by Ritesh Harjani <[email protected]>
---
 .../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(-)
Documentation/devicetree/bindings/mmc/sdhci-msm.txt +1 -0
index 0576264..897294f 100644
@@ -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)
drivers/mmc/host/Kconfig +1 -0
index b8c9ea5..a2524c7 100644
@@ -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
drivers/mmc/host/sdhci-msm.c +89 -1

122 lines not on this page · raw message

Signature · 2 lines
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.
maintainer
#5 lore raw

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

Quoted 63 lines · Ritesh Harjani

This adds CQHCI support for sdhci-msm platforms.

Signed-off-by Ritesh Harjani <[email protected]>
---
 .../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(-)
Documentation/devicetree/bindings/mmc/sdhci-msm.txt +1 -0
index 0576264..897294f 100644
@@ -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)
drivers/mmc/host/Kconfig +1 -0
index b8c9ea5..a2524c7 100644
@@ -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
drivers/mmc/host/sdhci-msm.c +15 -0
index df66724..346cdfb 100644
@@ -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;
+

drivers/mmc/host/sdhci-msm.c static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock) · line 1119

Quoted 10 lines
drivers/mmc/host/sdhci-msm.c +10 -0
+	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

Quoted 11 lines
drivers/mmc/host/sdhci-msm.c +11 -0
+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)

drivers/mmc/host/sdhci-msm.c static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock) · line 1140

Quoted 10 lines
drivers/mmc/host/sdhci-msm.c +10 -0
+		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

Quoted 71 lines
drivers/mmc/host/sdhci-msm.c +43 -1
+	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;
author
#6 lore raw

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

Quoted 68 lines · Adrian Hunter
Quoted 67 lines · Ritesh Harjani

This adds CQHCI support for sdhci-msm platforms.

Signed-off-by Ritesh Harjani <[email protected]>
---
  .../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(-)
Documentation/devicetree/bindings/mmc/sdhci-msm.txt +1 -0
index 0576264..897294f 100644
@@ -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)
drivers/mmc/host/Kconfig +1 -0
index b8c9ea5..a2524c7 100644
@@ -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
drivers/mmc/host/sdhci-msm.c +19 -0
index df66724..346cdfb 100644
@@ -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;
+}
+
Quoted 8 lines

drivers/mmc/host/sdhci-msm.c static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock) · line 1119

Quoted 6 lines
drivers/mmc/host/sdhci-msm.c +6 -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.

Quoted 16 lines
Quoted 16 lines
drivers/mmc/host/sdhci-msm.c +15 -0
+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)) {
Quoted 8 lines

drivers/mmc/host/sdhci-msm.c static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock) · line 1140

Quoted 6 lines
drivers/mmc/host/sdhci-msm.c +6 -0
+		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.

Quoted 73 lines
Quoted 72 lines
drivers/mmc/host/sdhci-msm.c +43 -1
+	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;
Signature · 2 lines
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.
author
#7 lore raw

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

In reply to 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 <[email protected]>
---
 drivers/mmc/host/sdhci-msm.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)
drivers/mmc/host/sdhci-msm.c +22 -2
index 346cdfb..baa3409 100644
@@ -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
Signature · 2 lines
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.
maintainer
#8 lore raw

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

Quoted 18 lines · 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 <[email protected]>
---
 drivers/mmc/host/sdhci-msm.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)
drivers/mmc/host/sdhci-msm.c +0 -0
index 346cdfb..baa3409 100644

drivers/mmc/host/sdhci-msm.c static u32 sdhci_msm_cqe_irq(struct sdhci_host *host, u32 intmask) · line 1119

Quoted 9 lines
drivers/mmc/host/sdhci-msm.c +6 -0
@@ -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.

drivers/mmc/host/sdhci-msm.c static u32 sdhci_msm_cqe_irq(struct sdhci_host *host, u32 intmask) · line 1129

Quoted 10 lines
drivers/mmc/host/sdhci-msm.c +10 -0
+
+	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().

Quoted 13 lines
drivers/mmc/host/sdhci-msm.c +6 -2
+
+	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
author
#9 lore raw

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

Quoted 22 lines · Adrian Hunter
Quoted 21 lines · 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 <[email protected]>
---
  drivers/mmc/host/sdhci-msm.c | 24 ++++++++++++++++++++++--
  1 file changed, 22 insertions(+), 2 deletions(-)
drivers/mmc/host/sdhci-msm.c +0 -0
index 346cdfb..baa3409 100644
@@ -1111,9 +1111,29 @@ static u32 sdhci_msm_cqe_irq(struct sdhci_host *host, u32 intmask)
  	return 0;
  }
Quoted 8 lines

drivers/mmc/host/sdhci-msm.c static u32 sdhci_msm_cqe_irq(struct sdhci_host *host, u32 intmask) · line 1119

Quoted 6 lines
drivers/mmc/host/sdhci-msm.c +6 -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.

Quoted 14 lines

drivers/mmc/host/sdhci-msm.c static u32 sdhci_msm_cqe_irq(struct sdhci_host *host, u32 intmask) · line 1129

Quoted 11 lines
drivers/mmc/host/sdhci-msm.c +10 -0
+
+	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.

Quoted 15 lines
Quoted 14 lines
drivers/mmc/host/sdhci-msm.c +6 -2
+
+	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
Signature · 2 lines
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.
author
#10 lore raw

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

In reply to 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 <[email protected]>
---
 drivers/mmc/host/sdhci-msm.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
drivers/mmc/host/sdhci-msm.c +11 -0
index baa3409..8fdc2c0 100644
@@ -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);
 }
Signature · 2 lines
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.
maintainer
#11 lore raw

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

Quoted 9 lines · 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 <[email protected]>

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

Quoted 10 lines
---
 drivers/mmc/host/sdhci-msm.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
drivers/mmc/host/sdhci-msm.c +0 -0
index baa3409..8fdc2c0 100644
@@ -1124,10 +1124,21 @@ void sdhci_msm_cqe_enable(struct mmc_host *mmc)
 void sdhci_msm_cqe_disable(struct mmc_host *mmc, bool recovery)

drivers/mmc/host/sdhci-msm.c void sdhci_msm_cqe_enable(struct mmc_host *mmc) · line 1134

Quoted 10 lines
drivers/mmc/host/sdhci-msm.c +4 -0
 {
 	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.

Quoted 11 lines
drivers/mmc/host/sdhci-msm.c +7 -0
+	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);
 }
author
#12 lore raw

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

Quoted 5 lines · Adrian Hunter
Quoted 4 lines · 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.

Quoted 8 lines
Quoted 5 lines

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

Signed-off-by Ritesh Harjani <[email protected]>

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

Yes, sure.

Quoted 16 lines
Quoted 16 lines
---
  drivers/mmc/host/sdhci-msm.c | 11 +++++++++++
  1 file changed, 11 insertions(+)
drivers/mmc/host/sdhci-msm.c +2 -0
index baa3409..8fdc2c0 100644
@@ -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;
Quoted 7 lines

drivers/mmc/host/sdhci-msm.c void sdhci_msm_cqe_enable(struct mmc_host *mmc) · line 1134

Quoted 5 lines
drivers/mmc/host/sdhci-msm.c +2 -0
  	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.

Quoted 13 lines
Quoted 12 lines
drivers/mmc/host/sdhci-msm.c +7 -0
+	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);
  }
Signature · 2 lines
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.
maintainer
#13 lore raw
In reply to Ritesh Harjani
Quoted 17 lines · 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.

Quoted 6 lines

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.

Quoted 10 lines

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.

Quoted 10 lines

[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

Quoted 21 lines

[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(-)
author
#14 lore raw
Quoted 18 lines · Adrian Hunter
Quoted 17 lines · 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.

Quoted 8 lines
Quoted 6 lines

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.

Quoted 28 lines
Quoted 11 lines

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.

Quoted 10 lines

[file1]
filename=singlefile1

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

Quoted 2 lines

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.

Quoted 23 lines
Quoted 22 lines

[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(-)
Signature · 2 lines
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.