[PATCH v4 0/2] CQE support for cadence eMMC host controller

[PATCH v4 0/2] CQE support for cadence eMMC host controller

[Rohan Joshi via B4 Relay]

Hi,

This series adds CQE (Command Queue Engine) support for cadence
eMMC host controller. The driver enables CQE when both the
conditions mentioned below are met
        1. Hardware reports the presence of CQE
        2. DT provides a "cqhci" register space

The sdhci-cadence driver currently maps the host register
space (index 0). On the amd,pensando-elba platform, an
additional register space (byte-lane control) is present
at index 1, which the driver already handles separately.
The original author for Elba, Brad Larson(blarson@amd.com)
has confirmed that Elba does support CQE!
In this case, total 3 register spaces are mapped in the
order [hc, bctl, cqhci].

The CQHCI core expects the CQE region to be provided as
a named resource ("cqhci") via DT rather than by index.
The driver has never wired up CQE initialization for any
platform and the feature remains unused.

This series adds the support to look up the "cqhci" named
resource from the DT and when present, initializes the
CQHCI engine. This allows platforms to enable Command
Queuing simply by adding the named resource to the DT node
without requiring any platform-specific driver changes.

Signed-off-by: Rohan Joshi <rohan1sj@cadence.com>
---
Changes in v4:
- On cqhci_resume() failure, emit a dev_warn and continue rather than
  disabling the clock and halting the operation
- Link to v3: https://lore.kernel.org/r/20260722-cdns_sdhci_cqe-support-v3-0-4412856a4488@cadence.com

Changes in v3:
- Device Tree updates to account for multiple platforms supporting CQE
- Link to v2: https://patch.msgid.link/20260506-cdns_sdhci_cqe-support-v2-0-754fe4de8b65@cadence.com

Changes in v2:
- Use dev_err_probe() for error messages when in probe path
- Updated commit messages as per subsystem naming conventions
- Select MMC_CQHCI in Kconfig for MMC_SDHCI_CADENCE
- Drop new dt-binding example and corrected existing one
- Link to v1: https://patch.msgid.link/20260407-cdns_sdhci_cqe-support-v1-0-13efc0810631@cadence.com

To: Ulf Hansson <ulfh@kernel.org>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Masahiro Yamada <yamada.masahiro@socionext.com>
To: Adrian Hunter <adrian.hunter@intel.com>
Cc: linux-mmc@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: mparab@cadence.com
Cc: pawell@cadence.com
Cc: sjakhade@cadence.com
Cc: mpillai@cadence.com
Cc: rohan1sj@cadence.com
Cc: razb@mobileye.com
Cc: blarson@amd.com

---
Rohan Joshi (2):
      dt-bindings: mmc: cdns,sdhci: Add CQE support
      mmc: sdhci-cadence: Add CQE support

 .../devicetree/bindings/mmc/cdns,sdhci.yaml        |  28 ++++-
 drivers/mmc/host/Kconfig                           |   1 +
 drivers/mmc/host/sdhci-cadence.c                   | 125 ++++++++++++++++++++-
 3 files changed, 147 insertions(+), 7 deletions(-)
---
base-commit: 66498c75b4f8017f62d720d9b59675bdf3abce91
change-id: 20260721-cdns_sdhci_cqe-support-bf4e4dfcfe67

Best regards,
--  
Rohan Joshi <rohan1sj@cadence.com>

[PATCH v4 1/2] dt-bindings: mmc: cdns,sdhci: Add CQE support

[Rohan Joshi via B4 Relay]

From: Rohan Joshi <rohan1sj@cadence.com>

Add optional named resource "cqhci" to map the CQE
register space. When present, it is used for CQHCI
initialization and enable Command Queue Engine for eMMC.

The "cqhci" space is optional to maintain compatibility
for platforms that don't have CQE block. When absent, the
controller can operate without the CQE block as routined.

Update amd,pensando-elba platform to include this resource
since Elba does have hardware block required for CQE! The
original author for Elba, Brad Larson (blarson@amd.com)
was able to confirm the CQE support.

Signed-off-by: Rohan Joshi <rohan1sj@cadence.com>
---
 .../devicetree/bindings/mmc/cdns,sdhci.yaml        | 28 +++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
index 6c7317d13aa6..10bc9ac2073a 100644
--- a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
+++ b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
@@ -22,7 +22,14 @@ properties:
 
   reg:
     minItems: 1
-    maxItems: 2
+    maxItems: 3
+
+  reg-names:
+    minItems: 1
+    items:
+      - const: hc
+      - enum: [bctl, cqhci]
+      - const: cqhci
 
   interrupts:
     maxItems: 1
@@ -131,13 +138,27 @@ allOf:
     then:
       properties:
         reg:
+          minItems: 2
           items:
             - description: Host controller registers
             - description: Elba byte-lane enable register for writes
+            - description: CQE (Command Queuing Engine) registers (optional)
+        reg-names:
+          minItems: 2
+          items:
+            - const: hc
+            - const: bctl
+            - const: cqhci
     else:
       properties:
         reg:
-          maxItems: 1
+          items:
+            - description: Host Controller registers
+            - description: CQE (Command Queuing Engine) registers (optional)
+        reg-names:
+          items:
+            - const: hc
+            - const: cqhci
 
 unevaluatedProperties: false
 
@@ -145,7 +166,8 @@ examples:
   - |
     emmc: mmc@5a000000 {
         compatible = "socionext,uniphier-sd4hc", "cdns,sd4hc";
-        reg = <0x5a000000 0x400>;
+        reg = <0x5a000000 0x400>, <0x5a000400 0x060>;
+        reg-names = "hc", "cqhci";
         interrupts = <0 78 4>;
         clocks = <&clk 4>;
         bus-width = <8>;

-- 
2.43.0

Re: [PATCH v4 1/2] dt-bindings: mmc: cdns,sdhci: Add CQE support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The `else` block for non-Elba platforms lacks explicit `maxItems: 2` constraints for `reg` and `reg-names`, creating a validation loophole that allows non-Elba platforms to define an invalid third register.
--

--- Patch [1]: [PATCH v4 1/2] dt-bindings: mmc: cdns,sdhci: Add CQE support ---
commit 48a4d35cce2e12157d9c52de4ba805fd4e404597
Author: Rohan Joshi <rohan1sj@cadence.com>

dt-bindings: mmc: cdns,sdhci: Add CQE support

This commit adds an optional cqhci named resource to map the CQE register
space. It updates the amd,pensando-elba platform to include this resource
while maintaining compatibility for platforms without the CQE block.

> diff --git a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> --- a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> +++ b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml

[ ... ]

> @@ -131,13 +138,27 @@ allOf:
>      then:
>        properties:
>          reg:
> +          minItems: 2
>            items:
>              - description: Host controller registers
>              - description: Elba byte-lane enable register for writes
> +            - description: CQE (Command Queuing Engine) registers (optional)
> +        reg-names:
> +          minItems: 2
> +          items:
> +            - const: hc
> +            - const: bctl
> +            - const: cqhci
>      else:
>        properties:
>          reg:
> -          maxItems: 1
> +          items:
> +            - description: Host Controller registers
> +            - description: CQE (Command Queuing Engine) registers (optional)
> +        reg-names:
> +          items:
> +            - const: hc
> +            - const: cqhci

[Severity: Low]
Since the top-level schema was updated to permit up to 3 items for reg, does
omitting an explicit maxItems limit in this else block allow non-Elba
platforms to define an invalid third register and silently pass validation?

In dt-schema, an items list inside an if/then/else block does not restrict
the overall length unless maxItems is explicitly defined.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260825-cdns_sdhci_cqe-support-v4-0-e9f72e338373@cadence.com?part=1

Re: [PATCH v4 1/2] dt-bindings: mmc: cdns,sdhci: Add CQE support

[Krzysztof Kozlowski]

On 25/08/2026 13:10, Rohan Joshi via B4 Relay wrote:
> From: Rohan Joshi <rohan1sj@cadence.com>
> 
> Add optional named resource "cqhci" to map the CQE
> register space. When present, it is used for CQHCI
> initialization and enable Command Queue Engine for eMMC.
> 
> The "cqhci" space is optional to maintain compatibility
> for platforms that don't have CQE block. When absent, the
> controller can operate without the CQE block as routined.

routined? routed? What did you want to say?

Which platforms do not have CQE? Usually devices NOT having something
should have this resource disallowed/restricted. But your binding says
all devices have cqhci

> 
> Update amd,pensando-elba platform to include this resource
> since Elba does have hardware block required for CQE! The
> original author for Elba, Brad Larson (blarson@amd.com)
> was able to confirm the CQE support.


Please wrap commit message according to Linux coding style / submission
process (neither too early nor over the limit):
https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597

> 
> Signed-off-by: Rohan Joshi <rohan1sj@cadence.com>
> ---
>  .../devicetree/bindings/mmc/cdns,sdhci.yaml        | 28 +++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> index 6c7317d13aa6..10bc9ac2073a 100644
> --- a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> +++ b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> @@ -22,7 +22,14 @@ properties:
>  
>    reg:
>      minItems: 1
> -    maxItems: 2
> +    maxItems: 3
> +
> +  reg-names:
> +    minItems: 1
> +    items:
> +      - const: hc
> +      - enum: [bctl, cqhci]
> +      - const: cqhci
>  
>    interrupts:
>      maxItems: 1
> @@ -131,13 +138,27 @@ allOf:
>      then:
>        properties:
>          reg:
> +          minItems: 2
>            items:
>              - description: Host controller registers
>              - description: Elba byte-lane enable register for writes
> +            - description: CQE (Command Queuing Engine) registers (optional)

Schema says what is optional, do not repeat it in free form text.

> +        reg-names:
> +          minItems: 2
> +          items:
> +            - const: hc
> +            - const: bctl
> +            - const: cqhci

So are the names required now? If not then how does your driver work?

>      else:
>        properties:
>          reg:
> -          maxItems: 1
> +          items:
> +            - description: Host Controller registers
> +            - description: CQE (Command Queuing Engine) registers (optional)
> +        reg-names:
> +          items:
> +            - const: hc
> +            - const: cqhci

So now all devices have exactly two entries?

>  
>  unevaluatedProperties: false
>  
> @@ -145,7 +166,8 @@ examples:
>    - |
>      emmc: mmc@5a000000 {
>          compatible = "socionext,uniphier-sd4hc", "cdns,sd4hc";
> -        reg = <0x5a000000 0x400>;
> +        reg = <0x5a000000 0x400>, <0x5a000400 0x060>;
> +        reg-names = "hc", "cqhci";
>          interrupts = <0 78 4>;
>          clocks = <&clk 4>;
>          bus-width = <8>;
> 


Best regards,
Krzysztof

[PATCH v4 2/2] mmc: sdhci-cadence: Add CQE support

[Rohan Joshi via B4 Relay]

From: Rohan Joshi <rohan1sj@cadence.com>

Cadence host controller has optional CQE (Command Queue Engine).
The HW presence of CQE is determined by an additional host capability
register. Look up for "cqhci" register space from the DT and when
present, initialize CQHCI engine.

Signed-off-by: Rohan Joshi <rohan1sj@cadence.com>
---
 drivers/mmc/host/Kconfig         |   1 +
 drivers/mmc/host/sdhci-cadence.c | 125 +++++++++++++++++++++++++++++++++++++--
 2 files changed, 122 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 4f060d3e5636..2cc8133e8ef3 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -293,6 +293,7 @@ config MMC_SDHCI_CADENCE
 	depends on MMC_SDHCI_PLTFM
 	depends on OF
 	select MMC_SDHCI_IO_ACCESSORS
+	select MMC_CQHCI
 	help
 	  This selects the Cadence SD/SDIO/eMMC driver.
 
diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c
index 435603c8c00b..aa4431ef2b39 100644
--- a/drivers/mmc/host/sdhci-cadence.c
+++ b/drivers/mmc/host/sdhci-cadence.c
@@ -15,6 +15,8 @@
 #include <linux/reset.h>
 
 #include "sdhci-pltfm.h"
+#include "sdhci-cqhci.h"
+#include "cqhci.h"
 
 /* HRS - Host Register Set (specific to Cadence) */
 #define SDHCI_CDNS_HRS04		0x10		/* PHY access port */
@@ -36,6 +38,10 @@
 #define   SDHCI_CDNS_HRS06_MODE_MMC_HS400	0x5
 #define   SDHCI_CDNS_HRS06_MODE_MMC_HS400ES	0x6
 
+/* Host capabilities not covered by the standard capability registers (SRS16-SRS18) */
+#define SDHCI_CDNS_HRS30		0x78	/* Host capabilities */
+#define SDHCI_CDNS_HRS30_CQE_SUPPORTED	BIT(0)
+
 /* Read block gap */
 #define SDHCI_CDNS_HRS37		0x94	/* interface mode select */
 #define   SDHCI_CDNS_HRS37_MODE_DS		0x0
@@ -88,6 +94,7 @@ struct sdhci_cdns_priv {
 	void __iomem *ctl_addr;	/* write control */
 	spinlock_t wrlock;	/* write lock */
 	bool enhanced_strobe;
+	bool cqe_support;	/* Command Queuing Engine support */
 	void (*priv_writel)(struct sdhci_cdns_priv *priv, u32 val, void __iomem *reg);
 	struct reset_control *rst_hw;
 	unsigned int nr_phy_params;
@@ -385,6 +392,68 @@ static void sdhci_cdns_set_uhs_signaling(struct sdhci_host *host,
 		sdhci_set_uhs_signaling(host, timing);
 }
 
+static u32 sdhci_cdns_cqhci_irq(struct sdhci_host *host, u32 intmask)
+{
+	int cmd_err = 0;
+	int data_err = 0;
+
+	/* return original intmask to be handled by other handlers if it's not a CQE interrupt */
+	if (!sdhci_cqe_irq(host, intmask, &cmd_err, &data_err))
+		return intmask;
+
+	cqhci_irq(host->mmc, cmd_err, data_err);
+
+	return 0;
+}
+
+static const struct cqhci_host_ops sdhci_cdns_cqhci_ops = {
+	.enable		= sdhci_cqe_enable,
+	.disable	= sdhci_cqe_disable,
+};
+
+static int sdhci_cdns_cqe_add_host(struct sdhci_host *host, struct platform_device *pdev)
+{
+	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_probe(&pdev->dev, ret, "cqhci platform init failed\n");
+		goto cleanup;
+	}
+
+	dma64 = host->flags & SDHCI_USE_64_BIT_DMA;
+	if (dma64)
+		cq_host->caps |= CQHCI_TASK_DESC_SZ_128;
+
+	cq_host->ops = &sdhci_cdns_cqhci_ops;
+
+	host->mmc->caps2 |= MMC_CAP2_CQE | MMC_CAP2_CQE_DCMD;
+
+	ret = cqhci_init(cq_host, host->mmc, dma64);
+	if (ret) {
+		dev_err_probe(&pdev->dev, ret, "cqhci init failed\n");
+		goto cleanup;
+	}
+
+	/* add host to MMC subsystem */
+	ret = __sdhci_add_host(host);
+	if (ret)
+		goto cleanup;
+
+	return 0;
+
+cleanup:
+	sdhci_cleanup_host(host);
+	return ret;
+}
+
 /* Elba control register bits [6:3] are byte-lane enables */
 #define ELBA_BYTE_ENABLE_MASK(x)	((x) << 3)
 
@@ -445,8 +514,9 @@ static const struct sdhci_ops sdhci_elba_ops = {
 	.set_clock = sdhci_set_clock,
 	.get_timeout_clock = sdhci_cdns_get_timeout_clock,
 	.set_bus_width = sdhci_set_bus_width,
-	.reset = sdhci_reset,
+	.reset = sdhci_and_cqhci_reset,
 	.set_uhs_signaling = sdhci_cdns_set_uhs_signaling,
+	.irq = sdhci_cdns_cqhci_irq,
 };
 
 static int elba_drv_init(struct platform_device *pdev)
@@ -474,9 +544,10 @@ static const struct sdhci_ops sdhci_cdns_ops = {
 	.set_clock = sdhci_set_clock,
 	.get_timeout_clock = sdhci_cdns_get_timeout_clock,
 	.set_bus_width = sdhci_set_bus_width,
-	.reset = sdhci_reset,
+	.reset = sdhci_and_cqhci_reset,
 	.platform_execute_tuning = sdhci_cdns_execute_tuning,
 	.set_uhs_signaling = sdhci_cdns_set_uhs_signaling,
+	.irq = sdhci_cdns_cqhci_irq,
 };
 
 static const struct sdhci_cdns_drv_data sdhci_cdns_uniphier_drv_data = {
@@ -553,6 +624,8 @@ static int sdhci_cdns_probe(struct platform_device *pdev)
 	int ret;
 	struct device *dev = &pdev->dev;
 	static const u16 version = SDHCI_SPEC_400 << SDHCI_SPEC_VER_SHIFT;
+	bool cqe_enabled;
+	u32 host_caps;
 
 	clk = devm_clk_get_enabled(dev, NULL);
 	if (IS_ERR(clk))
@@ -608,7 +681,43 @@ static int sdhci_cdns_probe(struct platform_device *pdev)
 			host->mmc_host_ops.card_hw_reset = sdhci_cdns_mmc_hw_reset;
 	}
 
-	return sdhci_add_host(host);
+	host_caps = readl(priv->hrs_addr + SDHCI_CDNS_HRS30);
+
+	/*
+	 * CQE is enabled only when both conditions are met:
+	 *   1. Hardware reports CQE support via HRS30[0].
+	 *   2. DT provides a named "cqhci" register space.
+	 */
+	cqe_enabled = (host_caps & SDHCI_CDNS_HRS30_CQE_SUPPORTED) &&
+		      !!platform_get_resource_byname(pdev, IORESOURCE_MEM, "cqhci");
+
+	if ((host_caps & SDHCI_CDNS_HRS30_CQE_SUPPORTED) && !cqe_enabled)
+		dev_dbg(dev, "CQE supported by hardware but no 'cqhci' register space in DT, disabling\n");
+
+	if (cqe_enabled) {
+		priv->cqe_support = true;
+		ret = sdhci_cdns_cqe_add_host(host, pdev);
+	} else {
+		ret = sdhci_add_host(host);
+	}
+
+	return ret;
+}
+
+static int sdhci_cdns_suspend(struct device *dev)
+{
+	struct sdhci_host *host = dev_get_drvdata(dev);
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_cdns_priv *priv = sdhci_pltfm_priv(pltfm_host);
+	int ret;
+
+	if (priv->cqe_support) {
+		ret = cqhci_suspend(host->mmc);
+		if (ret)
+			return ret;
+	}
+
+	return sdhci_pltfm_suspend(dev);
 }
 
 static int sdhci_cdns_resume(struct device *dev)
@@ -630,6 +739,14 @@ static int sdhci_cdns_resume(struct device *dev)
 	if (ret)
 		goto disable_clk;
 
+	/* Resume CQE if enabled */
+	if (priv->cqe_support) {
+		ret = cqhci_resume(host->mmc);
+		if (ret)
+			dev_warn(dev, "CQE resume failed (%pe), continuing without CQE\n",
+				 ERR_PTR(ret));
+	}
+
 	return 0;
 
 disable_clk:
@@ -638,7 +755,7 @@ static int sdhci_cdns_resume(struct device *dev)
 	return ret;
 }
 
-static DEFINE_SIMPLE_DEV_PM_OPS(sdhci_cdns_pm_ops, sdhci_pltfm_suspend, sdhci_cdns_resume);
+static DEFINE_SIMPLE_DEV_PM_OPS(sdhci_cdns_pm_ops, sdhci_cdns_suspend, sdhci_cdns_resume);
 
 static const struct of_device_id sdhci_cdns_match[] = {
 	{

-- 
2.43.0

Re: [PATCH v4 2/2] mmc: sdhci-cadence: Add CQE support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Hardcoded cqhci_host_ops breaks SoC-specific write wrappers (e.g., Elba SoC byte-lane enables), causing potential hardware state corruption.
--

--- Patch [2]: [PATCH v4 2/2] mmc: sdhci-cadence: Add CQE support ---
commit dbab4b1c448ae45285ec3f2e6b5c3e939ca07cc3
Author: Rohan Joshi <rohan1sj@cadence.com>

mmc: sdhci-cadence: Add CQE support
    
Cadence host controller has optional CQE (Command Queue Engine).
The HW presence of CQE is determined by an additional host capability
register. Look up for "cqhci" register space from the DT and when
present, initialize CQHCI engine.

> diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c
> index 435603c8c00b2..aa4431ef2b394 100644
> --- a/drivers/mmc/host/sdhci-cadence.c
> +++ b/drivers/mmc/host/sdhci-cadence.c
[ ... ]
> @@ -385,6 +392,68 @@ static void sdhci_cdns_set_uhs_signaling(struct sdhci_host *host,
>  		sdhci_set_uhs_signaling(host, timing);
>  }
>  
> +static u32 sdhci_cdns_cqhci_irq(struct sdhci_host *host, u32 intmask)
> +{
> +	int cmd_err = 0;
> +	int data_err = 0;
> +
> +	/* return original intmask to be handled by other handlers if it's not a CQE interrupt */
> +	if (!sdhci_cqe_irq(host, intmask, &cmd_err, &data_err))
> +		return intmask;
> +
> +	cqhci_irq(host->mmc, cmd_err, data_err);
> +
> +	return 0;
> +}
> +
> +static const struct cqhci_host_ops sdhci_cdns_cqhci_ops = {
> +	.enable		= sdhci_cqe_enable,
> +	.disable	= sdhci_cqe_disable,
> +};
> +
> +static int sdhci_cdns_cqe_add_host(struct sdhci_host *host, struct platform_device *pdev)
> +{
> +	struct cqhci_host *cq_host;
> +	bool dma64;
> +	int ret;
[ ... ]
> +	dma64 = host->flags & SDHCI_USE_64_BIT_DMA;
> +	if (dma64)
> +		cq_host->caps |= CQHCI_TASK_DESC_SZ_128;
> +
> +	cq_host->ops = &sdhci_cdns_cqhci_ops;

[Severity: Medium]
By hardcoding cq_host->ops to sdhci_cdns_cqhci_ops here, does this bypass
the SoC-specific write wrappers needed by platforms like the Pensando Elba
SoC?

The sdhci_cdns_cqhci_ops struct lacks a custom .write_l callback. Elba
requires a custom byte-lane enable configuration protected by wrlock
(elba_priv_writel). If a raw writel() from the CQE core interleaves with
an elba_write_b(), the 32-bit CQHCI write could be truncated by the hardware
to 1 byte, potentially corrupting the CQE state.

While Elba's current device tree might not define the cqhci resource, it
appears CQE support is architecturally intended for it since CQE hooks are
added to sdhci_elba_ops further below in this patch. Is there a way to allow
sdhci_elba_drv_data to supply its own cqhci_host_ops, or dynamically inherit
the write_l wrapper from the host operations?

> +
> +	host->mmc->caps2 |= MMC_CAP2_CQE | MMC_CAP2_CQE_DCMD;
> +
> +	ret = cqhci_init(cq_host, host->mmc, dma64);
> +	if (ret) {
> +		dev_err_probe(&pdev->dev, ret, "cqhci init failed\n");
> +		goto cleanup;
> +	}
> +
> +	/* add host to MMC subsystem */
> +	ret = __sdhci_add_host(host);
> +	if (ret)
> +		goto cleanup;
> +
> +	return 0;
> +
> +cleanup:
> +	sdhci_cleanup_host(host);
> +	return ret;
> +}
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260825-cdns_sdhci_cqe-support-v4-0-e9f72e338373@cadence.com?part=2

Re: [PATCH v4 2/2] mmc: sdhci-cadence: Add CQE support

[Krzysztof Kozlowski]

On 25/08/2026 13:10, Rohan Joshi via B4 Relay wrote:
> From: Rohan Joshi <rohan1sj@cadence.com>
> 
> Cadence host controller has optional CQE (Command Queue Engine).
> The HW presence of CQE is determined by an additional host capability
> register. Look up for "cqhci" register space from the DT and when
> present, initialize CQHCI engine.
> 
> Signed-off-by: Rohan Joshi <rohan1sj@cadence.com>
> ---
>  drivers/mmc/host/Kconfig         |   1 +
>  drivers/mmc/host/sdhci-cadence.c | 125 +++++++++++++++++++++++++++++++++++++--
>  2 files changed, 122 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 4f060d3e5636..2cc8133e8ef3 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -293,6 +293,7 @@ config MMC_SDHCI_CADENCE
>  	depends on MMC_SDHCI_PLTFM
>  	depends on OF
>  	select MMC_SDHCI_IO_ACCESSORS
> +	select MMC_CQHCI
>  	help
>  	  This selects the Cadence SD/SDIO/eMMC driver.
>  
> diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c
> index 435603c8c00b..aa4431ef2b39 100644
> --- a/drivers/mmc/host/sdhci-cadence.c
> +++ b/drivers/mmc/host/sdhci-cadence.c
> @@ -15,6 +15,8 @@
>  #include <linux/reset.h>
>  
>  #include "sdhci-pltfm.h"
> +#include "sdhci-cqhci.h"
> +#include "cqhci.h"
>  
>  /* HRS - Host Register Set (specific to Cadence) */
>  #define SDHCI_CDNS_HRS04		0x10		/* PHY access port */
> @@ -36,6 +38,10 @@
>  #define   SDHCI_CDNS_HRS06_MODE_MMC_HS400	0x5
>  #define   SDHCI_CDNS_HRS06_MODE_MMC_HS400ES	0x6
>  
> +/* Host capabilities not covered by the standard capability registers (SRS16-SRS18) */
> +#define SDHCI_CDNS_HRS30		0x78	/* Host capabilities */
> +#define SDHCI_CDNS_HRS30_CQE_SUPPORTED	BIT(0)
> +
>  /* Read block gap */
>  #define SDHCI_CDNS_HRS37		0x94	/* interface mode select */
>  #define   SDHCI_CDNS_HRS37_MODE_DS		0x0
> @@ -88,6 +94,7 @@ struct sdhci_cdns_priv {
>  	void __iomem *ctl_addr;	/* write control */
>  	spinlock_t wrlock;	/* write lock */
>  	bool enhanced_strobe;
> +	bool cqe_support;	/* Command Queuing Engine support */
>  	void (*priv_writel)(struct sdhci_cdns_priv *priv, u32 val, void __iomem *reg);
>  	struct reset_control *rst_hw;
>  	unsigned int nr_phy_params;
> @@ -385,6 +392,68 @@ static void sdhci_cdns_set_uhs_signaling(struct sdhci_host *host,
>  		sdhci_set_uhs_signaling(host, timing);
>  }
>  
> +static u32 sdhci_cdns_cqhci_irq(struct sdhci_host *host, u32 intmask)
> +{
> +	int cmd_err = 0;
> +	int data_err = 0;
> +
> +	/* return original intmask to be handled by other handlers if it's not a CQE interrupt */
> +	if (!sdhci_cqe_irq(host, intmask, &cmd_err, &data_err))
> +		return intmask;
> +
> +	cqhci_irq(host->mmc, cmd_err, data_err);
> +
> +	return 0;
> +}
> +
> +static const struct cqhci_host_ops sdhci_cdns_cqhci_ops = {
> +	.enable		= sdhci_cqe_enable,
> +	.disable	= sdhci_cqe_disable,
> +};
> +
> +static int sdhci_cdns_cqe_add_host(struct sdhci_host *host, struct platform_device *pdev)
> +{
> +	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_probe(&pdev->dev, ret, "cqhci platform init failed\n");

If this is probe path, why aren't you using standard syntax:
ret = dev_err_probe?

> +		goto cleanup;
> +	}
> +
> +	dma64 = host->flags & SDHCI_USE_64_BIT_DMA;
> +	if (dma64)
> +		cq_host->caps |= CQHCI_TASK_DESC_SZ_128;
> +
> +	cq_host->ops = &sdhci_cdns_cqhci_ops;
> +
> +	host->mmc->caps2 |= MMC_CAP2_CQE | MMC_CAP2_CQE_DCMD;
> +
> +	ret = cqhci_init(cq_host, host->mmc, dma64);
> +	if (ret) {
> +		dev_err_probe(&pdev->dev, ret, "cqhci init failed\n");
> +		goto cleanup;
> +	}
> +
> +	/* add host to MMC subsystem */
> +	ret = __sdhci_add_host(host);
> +	if (ret)
> +		goto cleanup;
> +
> +	return 0;


Best regards,
Krzysztof

Re: [PATCH v4 2/2] mmc: sdhci-cadence: Add CQE support

[Adrian Hunter]

On 25/08/2026 14:10, Rohan Joshi via B4 Relay wrote:
> From: Rohan Joshi <rohan1sj@cadence.com>
> 
> Cadence host controller has optional CQE (Command Queue Engine).
> The HW presence of CQE is determined by an additional host capability
> register. Look up for "cqhci" register space from the DT and when
> present, initialize CQHCI engine.

Still says nothing about Elba.  Only the cover letter says.
Needs to be here too.

However, Sashiko raises a valid question of whether Elba
would need special accessor functions for cqhci registers
like it does for sdhci registers:

	https://sashiko.dev/#/patchset/20260825-cdns_sdhci_cqe-support-v4-0-e9f72e338373%40cadence.com?part=2

Probably you should just say (in the commit message) that
Elba is not supported, and then make sure in the code that
it will be unaffected.

> 
> Signed-off-by: Rohan Joshi <rohan1sj@cadence.com>
> ---
>  drivers/mmc/host/Kconfig         |   1 +
>  drivers/mmc/host/sdhci-cadence.c | 125 +++++++++++++++++++++++++++++++++++++--
>  2 files changed, 122 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 4f060d3e5636..2cc8133e8ef3 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -293,6 +293,7 @@ config MMC_SDHCI_CADENCE
>  	depends on MMC_SDHCI_PLTFM
>  	depends on OF
>  	select MMC_SDHCI_IO_ACCESSORS
> +	select MMC_CQHCI
>  	help
>  	  This selects the Cadence SD/SDIO/eMMC driver.
>  
> diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c
> index 435603c8c00b..aa4431ef2b39 100644
> --- a/drivers/mmc/host/sdhci-cadence.c
> +++ b/drivers/mmc/host/sdhci-cadence.c
> @@ -15,6 +15,8 @@
>  #include <linux/reset.h>
>  
>  #include "sdhci-pltfm.h"
> +#include "sdhci-cqhci.h"
> +#include "cqhci.h"
>  
>  /* HRS - Host Register Set (specific to Cadence) */
>  #define SDHCI_CDNS_HRS04		0x10		/* PHY access port */
> @@ -36,6 +38,10 @@
>  #define   SDHCI_CDNS_HRS06_MODE_MMC_HS400	0x5
>  #define   SDHCI_CDNS_HRS06_MODE_MMC_HS400ES	0x6
>  
> +/* Host capabilities not covered by the standard capability registers (SRS16-SRS18) */
> +#define SDHCI_CDNS_HRS30		0x78	/* Host capabilities */
> +#define SDHCI_CDNS_HRS30_CQE_SUPPORTED	BIT(0)
> +
>  /* Read block gap */
>  #define SDHCI_CDNS_HRS37		0x94	/* interface mode select */
>  #define   SDHCI_CDNS_HRS37_MODE_DS		0x0
> @@ -88,6 +94,7 @@ struct sdhci_cdns_priv {
>  	void __iomem *ctl_addr;	/* write control */
>  	spinlock_t wrlock;	/* write lock */
>  	bool enhanced_strobe;
> +	bool cqe_support;	/* Command Queuing Engine support */
>  	void (*priv_writel)(struct sdhci_cdns_priv *priv, u32 val, void __iomem *reg);
>  	struct reset_control *rst_hw;
>  	unsigned int nr_phy_params;
> @@ -385,6 +392,68 @@ static void sdhci_cdns_set_uhs_signaling(struct sdhci_host *host,
>  		sdhci_set_uhs_signaling(host, timing);
>  }
>  
> +static u32 sdhci_cdns_cqhci_irq(struct sdhci_host *host, u32 intmask)
> +{
> +	int cmd_err = 0;
> +	int data_err = 0;
> +
> +	/* return original intmask to be handled by other handlers if it's not a CQE interrupt */
> +	if (!sdhci_cqe_irq(host, intmask, &cmd_err, &data_err))
> +		return intmask;
> +
> +	cqhci_irq(host->mmc, cmd_err, data_err);
> +
> +	return 0;
> +}
> +
> +static const struct cqhci_host_ops sdhci_cdns_cqhci_ops = {
> +	.enable		= sdhci_cqe_enable,
> +	.disable	= sdhci_cqe_disable,
> +};
> +
> +static int sdhci_cdns_cqe_add_host(struct sdhci_host *host, struct platform_device *pdev)
> +{
> +	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_probe(&pdev->dev, ret, "cqhci platform init failed\n");
> +		goto cleanup;
> +	}
> +
> +	dma64 = host->flags & SDHCI_USE_64_BIT_DMA;
> +	if (dma64)
> +		cq_host->caps |= CQHCI_TASK_DESC_SZ_128;
> +
> +	cq_host->ops = &sdhci_cdns_cqhci_ops;
> +
> +	host->mmc->caps2 |= MMC_CAP2_CQE | MMC_CAP2_CQE_DCMD;
> +
> +	ret = cqhci_init(cq_host, host->mmc, dma64);
> +	if (ret) {
> +		dev_err_probe(&pdev->dev, ret, "cqhci init failed\n");
> +		goto cleanup;
> +	}
> +
> +	/* add host to MMC subsystem */
> +	ret = __sdhci_add_host(host);
> +	if (ret)
> +		goto cleanup;
> +
> +	return 0;
> +
> +cleanup:
> +	sdhci_cleanup_host(host);
> +	return ret;
> +}
> +
>  /* Elba control register bits [6:3] are byte-lane enables */
>  #define ELBA_BYTE_ENABLE_MASK(x)	((x) << 3)
>  
> @@ -445,8 +514,9 @@ static const struct sdhci_ops sdhci_elba_ops = {
>  	.set_clock = sdhci_set_clock,
>  	.get_timeout_clock = sdhci_cdns_get_timeout_clock,
>  	.set_bus_width = sdhci_set_bus_width,
> -	.reset = sdhci_reset,
> +	.reset = sdhci_and_cqhci_reset,
>  	.set_uhs_signaling = sdhci_cdns_set_uhs_signaling,
> +	.irq = sdhci_cdns_cqhci_irq,
>  };
>  
>  static int elba_drv_init(struct platform_device *pdev)
> @@ -474,9 +544,10 @@ static const struct sdhci_ops sdhci_cdns_ops = {
>  	.set_clock = sdhci_set_clock,
>  	.get_timeout_clock = sdhci_cdns_get_timeout_clock,
>  	.set_bus_width = sdhci_set_bus_width,
> -	.reset = sdhci_reset,
> +	.reset = sdhci_and_cqhci_reset,
>  	.platform_execute_tuning = sdhci_cdns_execute_tuning,
>  	.set_uhs_signaling = sdhci_cdns_set_uhs_signaling,
> +	.irq = sdhci_cdns_cqhci_irq,
>  };
>  
>  static const struct sdhci_cdns_drv_data sdhci_cdns_uniphier_drv_data = {
> @@ -553,6 +624,8 @@ static int sdhci_cdns_probe(struct platform_device *pdev)
>  	int ret;
>  	struct device *dev = &pdev->dev;
>  	static const u16 version = SDHCI_SPEC_400 << SDHCI_SPEC_VER_SHIFT;
> +	bool cqe_enabled;
> +	u32 host_caps;
>  
>  	clk = devm_clk_get_enabled(dev, NULL);
>  	if (IS_ERR(clk))
> @@ -608,7 +681,43 @@ static int sdhci_cdns_probe(struct platform_device *pdev)
>  			host->mmc_host_ops.card_hw_reset = sdhci_cdns_mmc_hw_reset;
>  	}
>  
> -	return sdhci_add_host(host);
> +	host_caps = readl(priv->hrs_addr + SDHCI_CDNS_HRS30);
> +
> +	/*
> +	 * CQE is enabled only when both conditions are met:
> +	 *   1. Hardware reports CQE support via HRS30[0].
> +	 *   2. DT provides a named "cqhci" register space.
> +	 */
> +	cqe_enabled = (host_caps & SDHCI_CDNS_HRS30_CQE_SUPPORTED) &&
> +		      !!platform_get_resource_byname(pdev, IORESOURCE_MEM, "cqhci");
> +
> +	if ((host_caps & SDHCI_CDNS_HRS30_CQE_SUPPORTED) && !cqe_enabled)
> +		dev_dbg(dev, "CQE supported by hardware but no 'cqhci' register space in DT, disabling\n");
> +
> +	if (cqe_enabled) {
> +		priv->cqe_support = true;
> +		ret = sdhci_cdns_cqe_add_host(host, pdev);
> +	} else {
> +		ret = sdhci_add_host(host);
> +	}
> +
> +	return ret;
> +}
> +
> +static int sdhci_cdns_suspend(struct device *dev)
> +{
> +	struct sdhci_host *host = dev_get_drvdata(dev);
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_cdns_priv *priv = sdhci_pltfm_priv(pltfm_host);
> +	int ret;
> +
> +	if (priv->cqe_support) {
> +		ret = cqhci_suspend(host->mmc);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return sdhci_pltfm_suspend(dev);
>  }
>  
>  static int sdhci_cdns_resume(struct device *dev)
> @@ -630,6 +739,14 @@ static int sdhci_cdns_resume(struct device *dev)
>  	if (ret)
>  		goto disable_clk;
>  
> +	/* Resume CQE if enabled */
> +	if (priv->cqe_support) {
> +		ret = cqhci_resume(host->mmc);
> +		if (ret)
> +			dev_warn(dev, "CQE resume failed (%pe), continuing without CQE\n",
> +				 ERR_PTR(ret));
> +	}
> +
>  	return 0;
>  
>  disable_clk:
> @@ -638,7 +755,7 @@ static int sdhci_cdns_resume(struct device *dev)
>  	return ret;
>  }
>  
> -static DEFINE_SIMPLE_DEV_PM_OPS(sdhci_cdns_pm_ops, sdhci_pltfm_suspend, sdhci_cdns_resume);
> +static DEFINE_SIMPLE_DEV_PM_OPS(sdhci_cdns_pm_ops, sdhci_cdns_suspend, sdhci_cdns_resume);
>  
>  static const struct of_device_id sdhci_cdns_match[] = {
>  	{
>