[PATCH 00/10] mci: rockchip-dwcmshc: add HS400(ES) support

[PATCH 00/10] mci: rockchip-dwcmshc: add HS400(ES) support

[Sascha Hauer]

Based on the previous series to add HS200 support for the Rockchip SDHCI
controller this series now adds core support for HS400(ES) and also
Rockchip support for HS400(ES). As expected the speed gain isn't that
dramatic anymore, but it still goes up from 120MB/s to 170MB/s on reads.
Write is at about 22MB/s.

Tested on a Radxa Rock 5T.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
Sascha Hauer (10):
      mci: sdhci: define VDD_180 and shrink UHS_MASK to bits 0..2
      mci: mmc_send_tuning: actually point data.dest at the buffer
      mci: sdhci: add ADMA2 descriptor helpers
      mci: add HS400 mode selection
      mci: add HS400 Enhanced Strobe (HS400ES) selection
      mci: rockchip-dwcmshc-sdhci: use ADMA2
      mci: sdhci: rockchip: set TX-path source-select bit in DWCMSHC_EMMC_DLL_TXCLK
      mci: sdhci: rockchip: distinguish IP revision 0 (rk3568) from 1 (rk3576/rk3588)
      mci: sdhci: rockchip: support HS400
      mci: sdhci: rockchip: support HS400 Enhanced Strobe

 drivers/mci/imx-esdhc-common.c       |   2 -
 drivers/mci/mci-core.c               | 188 ++++++++++++++++++++++++++++++++++-
 drivers/mci/rockchip-dwcmshc-sdhci.c |  99 ++++++++++++++++--
 drivers/mci/sdhci.c                  | 154 +++++++++++++++++++++++++++-
 drivers/mci/sdhci.h                  |  62 +++++++++++-
 include/mci.h                        |   9 ++
 6 files changed, 499 insertions(+), 15 deletions(-)
---
base-commit: 4c7af34b16eb83fcab8aeb3444d6cbf184cadd39
change-id: 20260511-rockchip-emmc-hs400-e652b3e7a6b9

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>

[PATCH 01/10] mci: sdhci: define VDD_180 and shrink UHS_MASK to bits 0..2

[Sascha Hauer]

Per SDHCI 3.0+, HOST_CONTROL2's UHS Mode Select is bits 0..2 (3 bits)
and bit 3 is the 1.8 V Signaling Enable. The current SDHCI_CTRL_UHS_MASK
of GENMASK(3, 0) is one bit too wide and clears 1.8 V signaling every
time sdhci_set_uhs_signaling() runs.

Shrink the mask to GENMASK(2, 0) and add SDHCI_CTRL_VDD_180 = BIT(3) so
controllers that need to drive 1.8 V on I/O for HS200/HS400/UHS modes
can OR the bit in without it being clobbered on the next set_clock().

No functional change for controllers where VDD_180 was unused (most
in-tree drivers do not touch it; voltage switching there is either
fixed by board wiring or handled by an external regulator).

Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mci/imx-esdhc-common.c | 2 --
 drivers/mci/sdhci.h            | 3 ++-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/mci/imx-esdhc-common.c b/drivers/mci/imx-esdhc-common.c
index 050621d7fb..eb6a71915f 100644
--- a/drivers/mci/imx-esdhc-common.c
+++ b/drivers/mci/imx-esdhc-common.c
@@ -12,8 +12,6 @@
 
 #define	ESDHC_CTRL_D3CD			0x08
 
-#define  SDHCI_CTRL_VDD_180         0x0008
-
 #define ESDHC_MIX_CTRL			0x48
 #define  ESDHC_MIX_CTRL_DDREN		(1 << 3)
 #define  ESDHC_MIX_CTRL_AC23EN		(1 << 7)
diff --git a/drivers/mci/sdhci.h b/drivers/mci/sdhci.h
index 8d21febd7a..a04eb5b7ed 100644
--- a/drivers/mci/sdhci.h
+++ b/drivers/mci/sdhci.h
@@ -132,13 +132,14 @@
 #define SDHCI_SIGNAL_ENABLE					0x38
 #define SDHCI_ACMD12_ERR__HOST_CONTROL2				0x3C
 #define SDHCI_HOST_CONTROL2					0x3E
-#define  SDHCI_CTRL_UHS_MASK			GENMASK(3, 0)
+#define  SDHCI_CTRL_UHS_MASK			GENMASK(2, 0)
 #define   SDHCI_CTRL_UHS_SDR12			0x0
 #define   SDHCI_CTRL_UHS_SDR25			0x1
 #define   SDHCI_CTRL_UHS_SDR50			0x2
 #define   SDHCI_CTRL_UHS_SDR104			0x3
 #define   SDHCI_CTRL_UHS_DDR50			0x4
 #define   SDHCI_CTRL_HS400			0x5 /* Non-standard */
+#define  SDHCI_CTRL_VDD_180			BIT(3)
 #define  SDHCI_CTRL_DRV_TYPE_MASK		GENMASK(5, 4)
 #define   SDHCI_CTRL_DRV_TYPE_B			0x0000
 #define   SDHCI_CTRL_DRV_TYPE_A			0x0010

-- 
2.47.3

Re: [PATCH 01/10] mci: sdhci: define VDD_180 and shrink UHS_MASK to bits 0..2

[Ahmad Fatoum]

On 5/11/26 2:07 PM, Sascha Hauer wrote:
> Per SDHCI 3.0+, HOST_CONTROL2's UHS Mode Select is bits 0..2 (3 bits)
> and bit 3 is the 1.8 V Signaling Enable. The current SDHCI_CTRL_UHS_MASK
> of GENMASK(3, 0) is one bit too wide and clears 1.8 V signaling every
> time sdhci_set_uhs_signaling() runs.
> 
> Shrink the mask to GENMASK(2, 0) and add SDHCI_CTRL_VDD_180 = BIT(3) so
> controllers that need to drive 1.8 V on I/O for HS200/HS400/UHS modes
> can OR the bit in without it being clobbered on the next set_clock().
> 
> No functional change for controllers where VDD_180 was unused (most
> in-tree drivers do not touch it; voltage switching there is either
> fixed by board wiring or handled by an external regulator).
> 
> Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

> ---
>  drivers/mci/imx-esdhc-common.c | 2 --
>  drivers/mci/sdhci.h            | 3 ++-
>  2 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mci/imx-esdhc-common.c b/drivers/mci/imx-esdhc-common.c
> index 050621d7fb..eb6a71915f 100644
> --- a/drivers/mci/imx-esdhc-common.c
> +++ b/drivers/mci/imx-esdhc-common.c
> @@ -12,8 +12,6 @@
>  
>  #define	ESDHC_CTRL_D3CD			0x08
>  
> -#define  SDHCI_CTRL_VDD_180         0x0008
> -
>  #define ESDHC_MIX_CTRL			0x48
>  #define  ESDHC_MIX_CTRL_DDREN		(1 << 3)
>  #define  ESDHC_MIX_CTRL_AC23EN		(1 << 7)
> diff --git a/drivers/mci/sdhci.h b/drivers/mci/sdhci.h
> index 8d21febd7a..a04eb5b7ed 100644
> --- a/drivers/mci/sdhci.h
> +++ b/drivers/mci/sdhci.h
> @@ -132,13 +132,14 @@
>  #define SDHCI_SIGNAL_ENABLE					0x38
>  #define SDHCI_ACMD12_ERR__HOST_CONTROL2				0x3C
>  #define SDHCI_HOST_CONTROL2					0x3E
> -#define  SDHCI_CTRL_UHS_MASK			GENMASK(3, 0)
> +#define  SDHCI_CTRL_UHS_MASK			GENMASK(2, 0)
>  #define   SDHCI_CTRL_UHS_SDR12			0x0
>  #define   SDHCI_CTRL_UHS_SDR25			0x1
>  #define   SDHCI_CTRL_UHS_SDR50			0x2
>  #define   SDHCI_CTRL_UHS_SDR104			0x3
>  #define   SDHCI_CTRL_UHS_DDR50			0x4
>  #define   SDHCI_CTRL_HS400			0x5 /* Non-standard */
> +#define  SDHCI_CTRL_VDD_180			BIT(3)
>  #define  SDHCI_CTRL_DRV_TYPE_MASK		GENMASK(5, 4)
>  #define   SDHCI_CTRL_DRV_TYPE_B			0x0000
>  #define   SDHCI_CTRL_DRV_TYPE_A			0x0010
> 

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |

[PATCH 02/10] mci: mmc_send_tuning: actually point data.dest at the buffer

[Sascha Hauer]

mmc_send_tuning() allocates a tuning-block-sized buffer with calloc()
but never assigns it to data.dest, so the host transfers the tuning
block to wherever data.dest happened to be (uninitialised — typically
NULL). The subsequent memcmp against the expected pattern then always
mismatches, so any caller would observe -EIO regardless of the
controller actually receiving the block correctly.

Fix by setting data.dest to the buffer.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mci/mci-core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
index 33934a6a35..23b9117869 100644
--- a/drivers/mci/mci-core.c
+++ b/drivers/mci/mci-core.c
@@ -1684,6 +1684,7 @@ int mmc_send_tuning(struct mci *mci, u32 opcode)
 	mci_setup_cmd(&cmd, opcode, 0, MMC_RSP_R1 | MMC_CMD_ADTC);
 
 	cmd.data = xzalloc(sizeof(struct mci_data));
+	cmd.data->dest = data_buf;
 	cmd.data->blocksize = size;
 	cmd.data->blocks = 1;
 	cmd.data->flags = MMC_DATA_READ;

-- 
2.47.3

Re: [PATCH 02/10] mci: mmc_send_tuning: actually point data.dest at the buffer

[Ahmad Fatoum]

Hello Sascha,

On 5/11/26 2:07 PM, Sascha Hauer wrote:
> mmc_send_tuning() allocates a tuning-block-sized buffer with calloc()
> but never assigns it to data.dest, so the host transfers the tuning
> block to wherever data.dest happened to be (uninitialised — typically
> NULL). The subsequent memcmp against the expected pattern then always
> mismatches, so any caller would observe -EIO regardless of the
> controller actually receiving the block correctly.
> 
> Fix by setting data.dest to the buffer.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

But ouch. Apparently, I had added this, but it remained unused until
Steffen added cadence-sdhci as first user.

I recall there were issues with that driver, so Cc'ing Steffen and
Michael to make them aware of this change.

Cheers,
Ahmad

> ---
>  drivers/mci/mci-core.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
> index 33934a6a35..23b9117869 100644
> --- a/drivers/mci/mci-core.c
> +++ b/drivers/mci/mci-core.c
> @@ -1684,6 +1684,7 @@ int mmc_send_tuning(struct mci *mci, u32 opcode)
>  	mci_setup_cmd(&cmd, opcode, 0, MMC_RSP_R1 | MMC_CMD_ADTC);
>  
>  	cmd.data = xzalloc(sizeof(struct mci_data));
> +	cmd.data->dest = data_buf;
>  	cmd.data->blocksize = size;
>  	cmd.data->blocks = 1;
>  	cmd.data->flags = MMC_DATA_READ;
> 

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |

[PATCH 03/10] mci: sdhci: add ADMA2 descriptor helpers

[Sascha Hauer]

Add reusable ADMA2 (32-bit and 64-bit) support to the SDHCI core so
drivers can opt in to ADMA without each having to reimplement descriptor
table management.

A driver enables ADMA by calling sdhci_setup_adma() after
sdhci_setup_host(). The helper allocates a DMA-coherent descriptor
table sized for SDHCI_DEFAULT_ADMA_DESCS entries (drivers can override
adma_table_cnt before calling), picks the descriptor format based on
SDHCI_USE_64_BIT_DMA, sets SDHCI_USE_ADMA in host->flags and caps
mci->max_req_size so the MCI core splits requests to fit. From there,
sdhci_setup_data_dma() builds an ADMA2 descriptor chain for the
contiguous transfer buffer (one descriptor per up-to-64 KiB chunk plus
a terminating nop/end entry) and programs SDHCI_ADMA_ADDRESS instead
of the SDMA address. sdhci_config_dma() now selects ADMA32/ADMA64 in
HOST_CONTROL accordingly.

If sdhci_setup_adma() fails (no SDHCI_CAN_DO_ADMA2, no memory, or
unaligned table), the host transparently falls back to the existing
SDMA path.

Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mci/sdhci.c | 154 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 drivers/mci/sdhci.h |  59 ++++++++++++++++++++
 2 files changed, 212 insertions(+), 1 deletion(-)

diff --git a/drivers/mci/sdhci.c b/drivers/mci/sdhci.c
index f7172347e1..0c3ca69e9a 100644
--- a/drivers/mci/sdhci.c
+++ b/drivers/mci/sdhci.c
@@ -585,6 +585,13 @@ static void sdhci_config_dma(struct sdhci *host)
 	ctrl = sdhci_read8(host, SDHCI_HOST_CONTROL);
 	/* Note if DMA Select is zero then SDMA is selected */
 	ctrl &= ~SDHCI_CTRL_DMA_MASK;
+
+	if (host->flags & SDHCI_USE_ADMA) {
+		ctrl |= SDHCI_CTRL_ADMA32;
+		if (host->flags & SDHCI_USE_64_BIT_DMA && !host->v4_mode)
+			ctrl |= SDHCI_CTRL_ADMA64;
+	}
+
 	sdhci_write8(host, SDHCI_HOST_CONTROL, ctrl);
 
 	if (host->flags & SDHCI_USE_64_BIT_DMA) {
@@ -601,11 +608,67 @@ static void sdhci_config_dma(struct sdhci *host)
 	}
 }
 
+static void sdhci_adma_write_desc(struct sdhci *host, void **desc,
+				  dma_addr_t addr, int len, unsigned int cmd)
+{
+	struct sdhci_adma2_64_desc *dma_desc = *desc;
+
+	/* 32-bit and 64-bit descriptors share these fields. */
+	dma_desc->cmd = cpu_to_le16(cmd);
+	dma_desc->len = cpu_to_le16(len);
+	dma_desc->addr_lo = cpu_to_le32(lower_32_bits(addr));
+
+	if (host->flags & SDHCI_USE_64_BIT_DMA)
+		dma_desc->addr_hi = cpu_to_le32(upper_32_bits(addr));
+
+	*desc += host->desc_sz;
+}
+EXPORT_SYMBOL_GPL(sdhci_adma_write_desc);
+
+/*
+ * Build the ADMA2 descriptor table for a single contiguous DMA buffer.
+ * Each entry of the table covers up to SDHCI_ADMA2_MAX_LEN bytes; longer
+ * transfers are split across multiple descriptors.
+ */
+static int sdhci_adma_build_table(struct sdhci *host, dma_addr_t addr,
+				  unsigned int len)
+{
+	void *desc = host->adma_table;
+	void *desc_end = host->adma_table + host->adma_table_sz;
+
+	while (len) {
+		unsigned int chunk = min_t(unsigned int, len,
+					   SDHCI_ADMA2_MAX_LEN);
+
+		if (desc + host->desc_sz > desc_end)
+			return -ENOSPC;
+
+		/*
+		 * The length field is 16-bit; a length of 0 encodes
+		 * SDHCI_ADMA2_MAX_LEN bytes per the SD Host Controller
+		 * specification.
+		 */
+		sdhci_adma_write_desc(host, &desc, addr, chunk & 0xffff,
+				      ADMA2_TRAN_VALID);
+		addr += chunk;
+		len -= chunk;
+	}
+
+	if (desc + host->desc_sz > desc_end)
+		return -ENOSPC;
+
+	/* Append a terminating descriptor (nop, end, valid). */
+	sdhci_adma_write_desc(host, &desc, 0, 0, ADMA2_NOP_END_VALID);
+
+	return 0;
+}
+
 void sdhci_setup_data_dma(struct sdhci *sdhci, struct mci_data *data,
 			  dma_addr_t *dma)
 {
 	struct device *dev = sdhci_dev(sdhci);
 	int nbytes;
+	int ret;
 
 	if (!data) {
 		if (dma)
@@ -633,7 +696,22 @@ void sdhci_setup_data_dma(struct sdhci *sdhci, struct mci_data *data,
 	}
 
 	sdhci_config_dma(sdhci);
-	sdhci_set_sdma_addr(sdhci, *dma);
+
+	if (sdhci->flags & SDHCI_USE_ADMA) {
+		ret = sdhci_adma_build_table(sdhci, *dma, nbytes);
+		if (ret) {
+			dev_err(dev, "ADMA table build failed: %pe\n",
+				ERR_PTR(ret));
+			dma_unmap_single(dev, *dma, nbytes,
+					 (data->flags & MMC_DATA_READ) ?
+					 DMA_FROM_DEVICE : DMA_TO_DEVICE);
+			*dma = SDHCI_NO_DMA;
+			return;
+		}
+		sdhci_set_adma_addr(sdhci, sdhci->adma_addr);
+	} else {
+		sdhci_set_sdma_addr(sdhci, *dma);
+	}
 }
 
 void sdhci_teardown_data(struct sdhci *sdhci,
@@ -1213,3 +1291,77 @@ int sdhci_setup_host(struct sdhci *host)
 
 	return 0;
 }
+
+/**
+ * sdhci_setup_adma() - allocate ADMA descriptor table and enable ADMA
+ * @host: sdhci host
+ *
+ * Allocate a DMA-coherent ADMA2 descriptor table and mark the host as
+ * ADMA-capable so subsequent calls to sdhci_setup_data_dma() use ADMA
+ * instead of SDMA. Drivers must call this after sdhci_setup_host() since
+ * it relies on the SDHCI_USE_64_BIT_DMA flag established there.
+ *
+ * The descriptor count defaults to SDHCI_DEFAULT_ADMA_DESCS, which caps
+ * the largest single transfer at SDHCI_DEFAULT_ADMA_DESCS *
+ * SDHCI_ADMA2_MAX_LEN bytes. Drivers can override host->adma_table_cnt
+ * before calling to allocate a different size.
+ *
+ * Returns 0 on success or a negative error code on failure. On failure
+ * the host falls back to SDMA.
+ */
+int sdhci_setup_adma(struct sdhci *host)
+{
+	struct device *dev = sdhci_dev(host);
+	struct mci_host *mci = host->mci;
+	dma_addr_t dma;
+	void *buf;
+
+	BUG_ON(!mci);
+
+	/*
+	 * Without a controller capability bit ADMA2 cannot be used. Don't
+	 * fail loudly: the driver may have called us speculatively, just
+	 * leave SDMA as the fallback.
+	 */
+	if (!(host->caps & SDHCI_CAN_DO_ADMA2))
+		return -ENOTSUPP;
+
+	if (host->flags & SDHCI_USE_64_BIT_DMA)
+		host->desc_sz = SDHCI_ADMA2_64_DESC_SZ(host);
+	else
+		host->desc_sz = SDHCI_ADMA2_32_DESC_SZ;
+
+	if (!host->adma_table_cnt)
+		host->adma_table_cnt = SDHCI_DEFAULT_ADMA_DESCS;
+
+	host->adma_table_sz = host->adma_table_cnt * host->desc_sz;
+
+	buf = dma_alloc_coherent(dev, host->adma_table_sz, &dma);
+	if (!buf)
+		return -ENOMEM;
+
+	host->adma_table = buf;
+	host->adma_addr = dma;
+	host->flags |= SDHCI_USE_ADMA;
+
+	/*
+	 * One descriptor handles up to SDHCI_ADMA2_MAX_LEN bytes; the last
+	 * one is reserved for the terminating entry.
+	 */
+	mci->max_req_size = (host->adma_table_cnt - 1) * SDHCI_ADMA2_MAX_LEN;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(sdhci_setup_adma);
+
+void sdhci_release_adma(struct sdhci *host)
+{
+	if (!(host->flags & SDHCI_USE_ADMA))
+		return;
+
+	dma_free_coherent(sdhci_dev(host), host->adma_table, host->adma_addr,
+			  host->adma_table_sz);
+	host->adma_table = NULL;
+	host->flags &= ~SDHCI_USE_ADMA;
+}
+EXPORT_SYMBOL_GPL(sdhci_release_adma);
diff --git a/drivers/mci/sdhci.h b/drivers/mci/sdhci.h
index a04eb5b7ed..ab1cbbe3fd 100644
--- a/drivers/mci/sdhci.h
+++ b/drivers/mci/sdhci.h
@@ -222,6 +222,56 @@
 #define SDHCI_ADMA_ADDRESS					0x58
 #define SDHCI_ADMA_ADDRESS_HI					0x5c
 
+/* ADMA2 32-bit DMA descriptor size */
+#define SDHCI_ADMA2_32_DESC_SZ	8
+
+/* ADMA2 32-bit descriptor */
+struct sdhci_adma2_32_desc {
+	__le16	cmd;
+	__le16	len;
+	__le32	addr;
+} __packed __aligned(4);
+
+/*
+ * ADMA2 64-bit DMA descriptor size
+ * According to SD Host Controller spec v4.10, there are two kinds of
+ * descriptors for 64-bit addressing mode: 96-bit Descriptor and 128-bit
+ * Descriptor, if Host Version 4 Enable is set in the Host Control 2
+ * register, 128-bit Descriptor will be selected.
+ */
+#define SDHCI_ADMA2_64_DESC_SZ(host)	((host)->v4_mode ? 16 : 12)
+
+/*
+ * ADMA2 64-bit descriptor. Note 12-byte descriptor can't always be 8-byte
+ * aligned.
+ */
+struct sdhci_adma2_64_desc {
+	__le16	cmd;
+	__le16	len;
+	__le32	addr_lo;
+	__le32	addr_hi;
+} __packed __aligned(4);
+
+#define ADMA2_TRAN_VALID	0x21
+#define ADMA2_NOP_END_VALID	0x3
+#define ADMA2_END		0x2
+
+/*
+ * ADMA descriptor alignment.  Some controllers (e.g. Intel) require 8 byte
+ * alignment for the descriptor table even in 32-bit DMA mode.
+ */
+#define SDHCI_ADMA2_DESC_ALIGN	8
+
+/*
+ * Maximum length per ADMA2 descriptor. The length field in the descriptor
+ * is 16-bit wide; a value of 0 encodes 65536 bytes per the SD spec, so the
+ * effective per-descriptor maximum is 64 KiB.
+ */
+#define SDHCI_ADMA2_MAX_LEN	SZ_64K
+
+/* Default number of ADMA descriptors allocated by sdhci_setup_adma() */
+#define SDHCI_DEFAULT_ADMA_DESCS	128
+
 #define SDHCI_MMC_BOOT						0xC4
 
 #define SDHCI_SLOT_INT_STATUS	0xFC
@@ -279,6 +329,13 @@ struct sdhci {
 	bool read_caps;	/* Capability flags have been read */
 	u32 sdma_boundary;
 
+	/* ADMA descriptor table (allocated via sdhci_setup_adma()) */
+	void		*adma_table;
+	dma_addr_t	adma_addr;
+	unsigned int	adma_table_sz;	/* size of the descriptor table in bytes */
+	unsigned int	desc_sz;	/* per-descriptor size in bytes */
+	unsigned int	adma_table_cnt;	/* number of descriptor entries */
+
 	unsigned int		tuning_count;	/* Timer count for re-tuning */
 	unsigned int		tuning_mode;	/* Re-tuning mode supported by host */
 	unsigned int		tuning_err;	/* Error code for re-tuning */
@@ -364,6 +421,8 @@ int sdhci_transfer_data_pio(struct sdhci *sdhci, struct mci_cmd *cmd,
 int sdhci_transfer_data_dma(struct sdhci *sdhci, struct mci_cmd *cmd,
 			    struct mci_data *data, dma_addr_t dma);
 int sdhci_reset(struct sdhci *sdhci, u8 mask);
+int sdhci_setup_adma(struct sdhci *host);
+void sdhci_release_adma(struct sdhci *host);
 u16 sdhci_calc_clk(struct sdhci *host, unsigned int clock,
 		   unsigned int *actual_clock, unsigned int input_clock);
 void sdhci_set_clock(struct sdhci *host, unsigned int clock, unsigned int input_clock);

-- 
2.47.3

Re: [PATCH 03/10] mci: sdhci: add ADMA2 descriptor helpers

[Ahmad Fatoum]

Hi,

On 5/11/26 2:07 PM, Sascha Hauer wrote:
> Add reusable ADMA2 (32-bit and 64-bit) support to the SDHCI core so
> drivers can opt in to ADMA without each having to reimplement descriptor
> table management.
> 
> A driver enables ADMA by calling sdhci_setup_adma() after
> sdhci_setup_host(). The helper allocates a DMA-coherent descriptor
> table sized for SDHCI_DEFAULT_ADMA_DESCS entries (drivers can override
> adma_table_cnt before calling), picks the descriptor format based on
> SDHCI_USE_64_BIT_DMA, sets SDHCI_USE_ADMA in host->flags and caps
> mci->max_req_size so the MCI core splits requests to fit. From there,
> sdhci_setup_data_dma() builds an ADMA2 descriptor chain for the
> contiguous transfer buffer (one descriptor per up-to-64 KiB chunk plus
> a terminating nop/end entry) and programs SDHCI_ADMA_ADDRESS instead
> of the SDMA address. sdhci_config_dma() now selects ADMA32/ADMA64 in
> HOST_CONTROL accordingly.
> 
> If sdhci_setup_adma() fails (no SDHCI_CAN_DO_ADMA2, no memory, or
> unaligned table), the host transparently falls back to the existing
> SDMA path.

See below for feedback.

> 
> Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/mci/sdhci.c | 154 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>  drivers/mci/sdhci.h |  59 ++++++++++++++++++++
>  2 files changed, 212 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mci/sdhci.c b/drivers/mci/sdhci.c
> index f7172347e1..0c3ca69e9a 100644
> --- a/drivers/mci/sdhci.c
> +++ b/drivers/mci/sdhci.c
> @@ -585,6 +585,13 @@ static void sdhci_config_dma(struct sdhci *host)
>  	ctrl = sdhci_read8(host, SDHCI_HOST_CONTROL);
>  	/* Note if DMA Select is zero then SDMA is selected */
>  	ctrl &= ~SDHCI_CTRL_DMA_MASK;
> +
> +	if (host->flags & SDHCI_USE_ADMA) {
> +		ctrl |= SDHCI_CTRL_ADMA32;
> +		if (host->flags & SDHCI_USE_64_BIT_DMA && !host->v4_mode)
> +			ctrl |= SDHCI_CTRL_ADMA64;
> +	}
> +
>  	sdhci_write8(host, SDHCI_HOST_CONTROL, ctrl);
>  
>  	if (host->flags & SDHCI_USE_64_BIT_DMA) {
> @@ -601,11 +608,67 @@ static void sdhci_config_dma(struct sdhci *host)
>  	}
>  }
>  
> +static void sdhci_adma_write_desc(struct sdhci *host, void **desc,
> +				  dma_addr_t addr, int len, unsigned int cmd)
> +{
> +	struct sdhci_adma2_64_desc *dma_desc = *desc;

This should be a union between sdhci_adma2_32_desc and sdhci_adma2_64_desc.

> +
> +	/* 32-bit and 64-bit descriptors share these fields. */
> +	dma_desc->cmd = cpu_to_le16(cmd);
> +	dma_desc->len = cpu_to_le16(len);
> +	dma_desc->addr_lo = cpu_to_le32(lower_32_bits(addr));
> +
> +	if (host->flags & SDHCI_USE_64_BIT_DMA)
> +		dma_desc->addr_hi = cpu_to_le32(upper_32_bits(addr));

On a 64-bit system without SDHCI_USE_64_BIT_DMA, but with a 64-bit addr,
we will end up with memory corruption here.

Please add at least a BUG, so it fails reliably or propagate an error.

> +
> +	*desc += host->desc_sz;
> +}
> +EXPORT_SYMBOL_GPL(sdhci_adma_write_desc);

Why export a static symbol?

> +
> +/*
> + * Build the ADMA2 descriptor table for a single contiguous DMA buffer.
> + * Each entry of the table covers up to SDHCI_ADMA2_MAX_LEN bytes; longer
> + * transfers are split across multiple descriptors.
> + */
> +static int sdhci_adma_build_table(struct sdhci *host, dma_addr_t addr,
> +				  unsigned int len)
> +{
> +	void *desc = host->adma_table;
> +	void *desc_end = host->adma_table + host->adma_table_sz;
> +
> +	while (len) {
> +		unsigned int chunk = min_t(unsigned int, len,
> +					   SDHCI_ADMA2_MAX_LEN);

I think the min_t is unneeded and if it is, just give len the same type
as SDHCI_ADMA2_MAX_LEN?

> +
> +		if (desc + host->desc_sz > desc_end)
> +			return -ENOSPC;
> +
> +		/*
> +		 * The length field is 16-bit; a length of 0 encodes
> +		 * SDHCI_ADMA2_MAX_LEN bytes per the SD Host Controller
> +		 * specification.
> +		 */
> +		sdhci_adma_write_desc(host, &desc, addr, chunk & 0xffff,
> +				      ADMA2_TRAN_VALID);
> +		addr += chunk;
> +		len -= chunk;
> +	}
> +
> +	if (desc + host->desc_sz > desc_end)
> +		return -ENOSPC;

Nitpick: I think it would be cleaner to move this check into
sdhci_adma_write_desc(), so it returns an error on out-of-bound write
and just propagate the error instead of redoing the same if clause here
and in the loop.

> +
> +	/* Append a terminating descriptor (nop, end, valid). */
> +	sdhci_adma_write_desc(host, &desc, 0, 0, ADMA2_NOP_END_VALID);
> +
> +	return 0;
> +}
> +
>  void sdhci_setup_data_dma(struct sdhci *sdhci, struct mci_data *data,
>  			  dma_addr_t *dma)
>  {
>  	struct device *dev = sdhci_dev(sdhci);
>  	int nbytes;
> +	int ret;
>  
>  	if (!data) {
>  		if (dma)
> @@ -633,7 +696,22 @@ void sdhci_setup_data_dma(struct sdhci *sdhci, struct mci_data *data,
>  	}
>  
>  	sdhci_config_dma(sdhci);
> -	sdhci_set_sdma_addr(sdhci, *dma);
> +
> +	if (sdhci->flags & SDHCI_USE_ADMA) {
> +		ret = sdhci_adma_build_table(sdhci, *dma, nbytes);
> +		if (ret) {
> +			dev_err(dev, "ADMA table build failed: %pe\n",
> +				ERR_PTR(ret));
> +			dma_unmap_single(dev, *dma, nbytes,
> +					 (data->flags & MMC_DATA_READ) ?
> +					 DMA_FROM_DEVICE : DMA_TO_DEVICE);

Why is the data umapped here?!

> +			*dma = SDHCI_NO_DMA;
> +			return;
> +		}
> +		sdhci_set_adma_addr(sdhci, sdhci->adma_addr);
> +	} else {
> +		sdhci_set_sdma_addr(sdhci, *dma);
> +	}
>  }
>  
>  void sdhci_teardown_data(struct sdhci *sdhci,
> @@ -1213,3 +1291,77 @@ int sdhci_setup_host(struct sdhci *host)
>  
>  	return 0;
>  }
> +
> +/**
> + * sdhci_setup_adma() - allocate ADMA descriptor table and enable ADMA
> + * @host: sdhci host
> + *
> + * Allocate a DMA-coherent ADMA2 descriptor table and mark the host as
> + * ADMA-capable so subsequent calls to sdhci_setup_data_dma() use ADMA
> + * instead of SDMA. Drivers must call this after sdhci_setup_host() since
> + * it relies on the SDHCI_USE_64_BIT_DMA flag established there.
> + *
> + * The descriptor count defaults to SDHCI_DEFAULT_ADMA_DESCS, which caps
> + * the largest single transfer at SDHCI_DEFAULT_ADMA_DESCS *
> + * SDHCI_ADMA2_MAX_LEN bytes. Drivers can override host->adma_table_cnt
> + * before calling to allocate a different size.
> + *
> + * Returns 0 on success or a negative error code on failure. On failure
> + * the host falls back to SDMA.
> + */
> +int sdhci_setup_adma(struct sdhci *host)
> +{
> +	struct device *dev = sdhci_dev(host);
> +	struct mci_host *mci = host->mci;
> +	dma_addr_t dma;
> +	void *buf;
> +
> +	BUG_ON(!mci);
> +
> +	/*
> +	 * Without a controller capability bit ADMA2 cannot be used. Don't
> +	 * fail loudly: the driver may have called us speculatively, just
> +	 * leave SDMA as the fallback.
> +	 */
> +	if (!(host->caps & SDHCI_CAN_DO_ADMA2))
> +		return -ENOTSUPP;
> +
> +	if (host->flags & SDHCI_USE_64_BIT_DMA)
> +		host->desc_sz = SDHCI_ADMA2_64_DESC_SZ(host);
> +	else
> +		host->desc_sz = SDHCI_ADMA2_32_DESC_SZ;
> +
> +	if (!host->adma_table_cnt)
> +		host->adma_table_cnt = SDHCI_DEFAULT_ADMA_DESCS;
> +
> +	host->adma_table_sz = host->adma_table_cnt * host->desc_sz;
> +
> +	buf = dma_alloc_coherent(dev, host->adma_table_sz, &dma);
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	host->adma_table = buf;
> +	host->adma_addr = dma;
> +	host->flags |= SDHCI_USE_ADMA;
> +
> +	/*
> +	 * One descriptor handles up to SDHCI_ADMA2_MAX_LEN bytes; the last
> +	 * one is reserved for the terminating entry.
> +	 */
> +	mci->max_req_size = (host->adma_table_cnt - 1) * SDHCI_ADMA2_MAX_LEN;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(sdhci_setup_adma);
> +
> +void sdhci_release_adma(struct sdhci *host)
> +{
> +	if (!(host->flags & SDHCI_USE_ADMA))
> +		return;
> +
> +	dma_free_coherent(sdhci_dev(host), host->adma_table, host->adma_addr,
> +			  host->adma_table_sz);
> +	host->adma_table = NULL;

Why zero host->adma_table, but not host->adma_addr?

> +	host->flags &= ~SDHCI_USE_ADMA;
> +}
> +EXPORT_SYMBOL_GPL(sdhci_release_adma);


Cheers,
Ahmad

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |

Re: [PATCH 03/10] mci: sdhci: add ADMA2 descriptor helpers

[Sascha Hauer]

On 2026-05-18 11:18, Ahmad Fatoum wrote:
> Hi,
> 
> On 5/11/26 2:07 PM, Sascha Hauer wrote:
> > Add reusable ADMA2 (32-bit and 64-bit) support to the SDHCI core so
> > drivers can opt in to ADMA without each having to reimplement descriptor
> > table management.
> > 
> > A driver enables ADMA by calling sdhci_setup_adma() after
> > sdhci_setup_host(). The helper allocates a DMA-coherent descriptor
> > table sized for SDHCI_DEFAULT_ADMA_DESCS entries (drivers can override
> > adma_table_cnt before calling), picks the descriptor format based on
> > SDHCI_USE_64_BIT_DMA, sets SDHCI_USE_ADMA in host->flags and caps
> > mci->max_req_size so the MCI core splits requests to fit. From there,
> > sdhci_setup_data_dma() builds an ADMA2 descriptor chain for the
> > contiguous transfer buffer (one descriptor per up-to-64 KiB chunk plus
> > a terminating nop/end entry) and programs SDHCI_ADMA_ADDRESS instead
> > of the SDMA address. sdhci_config_dma() now selects ADMA32/ADMA64 in
> > HOST_CONTROL accordingly.
> > 
> > If sdhci_setup_adma() fails (no SDHCI_CAN_DO_ADMA2, no memory, or
> > unaligned table), the host transparently falls back to the existing
> > SDMA path.
> 
> See below for feedback.
> 
> > 
> > Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  drivers/mci/sdhci.c | 154 +++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  drivers/mci/sdhci.h |  59 ++++++++++++++++++++
> >  2 files changed, 212 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/mci/sdhci.c b/drivers/mci/sdhci.c
> > index f7172347e1..0c3ca69e9a 100644
> > --- a/drivers/mci/sdhci.c
> > +++ b/drivers/mci/sdhci.c
> > @@ -585,6 +585,13 @@ static void sdhci_config_dma(struct sdhci *host)
> >  	ctrl = sdhci_read8(host, SDHCI_HOST_CONTROL);
> >  	/* Note if DMA Select is zero then SDMA is selected */
> >  	ctrl &= ~SDHCI_CTRL_DMA_MASK;
> > +
> > +	if (host->flags & SDHCI_USE_ADMA) {
> > +		ctrl |= SDHCI_CTRL_ADMA32;
> > +		if (host->flags & SDHCI_USE_64_BIT_DMA && !host->v4_mode)
> > +			ctrl |= SDHCI_CTRL_ADMA64;
> > +	}
> > +
> >  	sdhci_write8(host, SDHCI_HOST_CONTROL, ctrl);
> >  
> >  	if (host->flags & SDHCI_USE_64_BIT_DMA) {
> > @@ -601,11 +608,67 @@ static void sdhci_config_dma(struct sdhci *host)
> >  	}
> >  }
> >  
> > +static void sdhci_adma_write_desc(struct sdhci *host, void **desc,
> > +				  dma_addr_t addr, int len, unsigned int cmd)
> > +{
> > +	struct sdhci_adma2_64_desc *dma_desc = *desc;
> 
> This should be a union between sdhci_adma2_32_desc and sdhci_adma2_64_desc.

This exactly matches the Kernel. Might be cleaner to make this a union,
but having the same code as the kernel here is also nice.

> 
> > +
> > +	/* 32-bit and 64-bit descriptors share these fields. */
> > +	dma_desc->cmd = cpu_to_le16(cmd);
> > +	dma_desc->len = cpu_to_le16(len);
> > +	dma_desc->addr_lo = cpu_to_le32(lower_32_bits(addr));
> > +
> > +	if (host->flags & SDHCI_USE_64_BIT_DMA)
> > +		dma_desc->addr_hi = cpu_to_le32(upper_32_bits(addr));
> 
> On a 64-bit system without SDHCI_USE_64_BIT_DMA, but with a 64-bit addr,
> we will end up with memory corruption here.
> 
> Please add at least a BUG, so it fails reliably or propagate an error.

This problem is pre-existing in the tree. Yes, we should catch this, but
should be another patch.

>
> > +
> > +	*desc += host->desc_sz;
> > +}
> > +EXPORT_SYMBOL_GPL(sdhci_adma_write_desc);
> 
> Why export a static symbol?

Was exported previously, forgot to remove when making it static.

> 
> > +
> > +/*
> > + * Build the ADMA2 descriptor table for a single contiguous DMA buffer.
> > + * Each entry of the table covers up to SDHCI_ADMA2_MAX_LEN bytes; longer
> > + * transfers are split across multiple descriptors.
> > + */
> > +static int sdhci_adma_build_table(struct sdhci *host, dma_addr_t addr,
> > +				  unsigned int len)
> > +{
> > +	void *desc = host->adma_table;
> > +	void *desc_end = host->adma_table + host->adma_table_sz;
> > +
> > +	while (len) {
> > +		unsigned int chunk = min_t(unsigned int, len,
> > +					   SDHCI_ADMA2_MAX_LEN);
> 
> I think the min_t is unneeded and if it is, just give len the same type
> as SDHCI_ADMA2_MAX_LEN?

That would be int. But should we really allow negative lengths as input
to this function?

> 
> > +
> > +		if (desc + host->desc_sz > desc_end)
> > +			return -ENOSPC;
> > +
> > +		/*
> > +		 * The length field is 16-bit; a length of 0 encodes
> > +		 * SDHCI_ADMA2_MAX_LEN bytes per the SD Host Controller
> > +		 * specification.
> > +		 */
> > +		sdhci_adma_write_desc(host, &desc, addr, chunk & 0xffff,
> > +				      ADMA2_TRAN_VALID);
> > +		addr += chunk;
> > +		len -= chunk;
> > +	}
> > +
> > +	if (desc + host->desc_sz > desc_end)
> > +		return -ENOSPC;
> 
> Nitpick: I think it would be cleaner to move this check into
> sdhci_adma_write_desc(), so it returns an error on out-of-bound write
> and just propagate the error instead of redoing the same if clause here
> and in the loop.

Ok.

> 
> > +
> > +	/* Append a terminating descriptor (nop, end, valid). */
> > +	sdhci_adma_write_desc(host, &desc, 0, 0, ADMA2_NOP_END_VALID);
> > +
> > +	return 0;
> > +}
> > +
> >  void sdhci_setup_data_dma(struct sdhci *sdhci, struct mci_data *data,
> >  			  dma_addr_t *dma)
> >  {
> >  	struct device *dev = sdhci_dev(sdhci);
> >  	int nbytes;
> > +	int ret;
> >  
> >  	if (!data) {
> >  		if (dma)
> > @@ -633,7 +696,22 @@ void sdhci_setup_data_dma(struct sdhci *sdhci, struct mci_data *data,
> >  	}
> >  
> >  	sdhci_config_dma(sdhci);
> > -	sdhci_set_sdma_addr(sdhci, *dma);
> > +
> > +	if (sdhci->flags & SDHCI_USE_ADMA) {
> > +		ret = sdhci_adma_build_table(sdhci, *dma, nbytes);
> > +		if (ret) {
> > +			dev_err(dev, "ADMA table build failed: %pe\n",
> > +				ERR_PTR(ret));
> > +			dma_unmap_single(dev, *dma, nbytes,
> > +					 (data->flags & MMC_DATA_READ) ?
> > +					 DMA_FROM_DEVICE : DMA_TO_DEVICE);
> 
> Why is the data umapped here?!

We have to. We are returning SDHCI_NO_DMA as dma address, so the caller
can't unmap it.

Sascha


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 03/10] mci: sdhci: add ADMA2 descriptor helpers

[Ahmad Fatoum]

Hello,

On 5/18/26 2:16 PM, Sascha Hauer wrote:
> On 2026-05-18 11:18, Ahmad Fatoum wrote:
>> Hi,
>>
>> On 5/11/26 2:07 PM, Sascha Hauer wrote:
>>> Add reusable ADMA2 (32-bit and 64-bit) support to the SDHCI core so
>>> drivers can opt in to ADMA without each having to reimplement descriptor
>>> table management.
>>>
>>> A driver enables ADMA by calling sdhci_setup_adma() after
>>> sdhci_setup_host(). The helper allocates a DMA-coherent descriptor
>>> table sized for SDHCI_DEFAULT_ADMA_DESCS entries (drivers can override
>>> adma_table_cnt before calling), picks the descriptor format based on
>>> SDHCI_USE_64_BIT_DMA, sets SDHCI_USE_ADMA in host->flags and caps
>>> mci->max_req_size so the MCI core splits requests to fit. From there,
>>> sdhci_setup_data_dma() builds an ADMA2 descriptor chain for the
>>> contiguous transfer buffer (one descriptor per up-to-64 KiB chunk plus
>>> a terminating nop/end entry) and programs SDHCI_ADMA_ADDRESS instead
>>> of the SDMA address. sdhci_config_dma() now selects ADMA32/ADMA64 in
>>> HOST_CONTROL accordingly.
>>>
>>> If sdhci_setup_adma() fails (no SDHCI_CAN_DO_ADMA2, no memory, or
>>> unaligned table), the host transparently falls back to the existing
>>> SDMA path.
>>
>> See below for feedback.
>>
>>>
>>> Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
>>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>>> ---
>>>  drivers/mci/sdhci.c | 154 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>>>  drivers/mci/sdhci.h |  59 ++++++++++++++++++++
>>>  2 files changed, 212 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mci/sdhci.c b/drivers/mci/sdhci.c
>>> index f7172347e1..0c3ca69e9a 100644
>>> --- a/drivers/mci/sdhci.c
>>> +++ b/drivers/mci/sdhci.c
>>> @@ -585,6 +585,13 @@ static void sdhci_config_dma(struct sdhci *host)
>>>  	ctrl = sdhci_read8(host, SDHCI_HOST_CONTROL);
>>>  	/* Note if DMA Select is zero then SDMA is selected */
>>>  	ctrl &= ~SDHCI_CTRL_DMA_MASK;
>>> +
>>> +	if (host->flags & SDHCI_USE_ADMA) {
>>> +		ctrl |= SDHCI_CTRL_ADMA32;
>>> +		if (host->flags & SDHCI_USE_64_BIT_DMA && !host->v4_mode)
>>> +			ctrl |= SDHCI_CTRL_ADMA64;
>>> +	}
>>> +
>>>  	sdhci_write8(host, SDHCI_HOST_CONTROL, ctrl);
>>>  
>>>  	if (host->flags & SDHCI_USE_64_BIT_DMA) {
>>> @@ -601,11 +608,67 @@ static void sdhci_config_dma(struct sdhci *host)
>>>  	}
>>>  }
>>>  
>>> +static void sdhci_adma_write_desc(struct sdhci *host, void **desc,
>>> +				  dma_addr_t addr, int len, unsigned int cmd)
>>> +{
>>> +	struct sdhci_adma2_64_desc *dma_desc = *desc;
>>
>> This should be a union between sdhci_adma2_32_desc and sdhci_adma2_64_desc.
> 
> This exactly matches the Kernel. Might be cleaner to make this a union,
> but having the same code as the kernel here is also nice.

I don't like that, it's not standards-compliant and will just makes it
harder for static analysis.. :/

>> Please add at least a BUG, so it fails reliably or propagate an error.
> 
> This problem is pre-existing in the tree. Yes, we should catch this, but
> should be another patch.

Okay.

>> I think the min_t is unneeded and if it is, just give len the same type
>> as SDHCI_ADMA2_MAX_LEN?
> 
> That would be int. But should we really allow negative lengths as input
> to this function?

Fair enough.

>>>  	sdhci_config_dma(sdhci);
>>> -	sdhci_set_sdma_addr(sdhci, *dma);
>>> +
>>> +	if (sdhci->flags & SDHCI_USE_ADMA) {
>>> +		ret = sdhci_adma_build_table(sdhci, *dma, nbytes);
>>> +		if (ret) {
>>> +			dev_err(dev, "ADMA table build failed: %pe\n",
>>> +				ERR_PTR(ret));
>>> +			dma_unmap_single(dev, *dma, nbytes,
>>> +					 (data->flags & MMC_DATA_READ) ?
>>> +					 DMA_FROM_DEVICE : DMA_TO_DEVICE);
>>
>> Why is the data umapped here?!
> 
> We have to. We are returning SDHCI_NO_DMA as dma address, so the caller
> can't unmap it.

I somehow missed that this is error handling.
That's fine then.

Thanks,
Ahmad

> 
> Sascha
> 
> 

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |

[PATCH 04/10] mci: add HS400 mode selection

[Sascha Hauer]

eMMC HS400 transitions through HS and 8-bit DDR per JEDEC, on top of
a card already tuned in HS200. Wire that up in the core:

- mmc_select_max_dtr() now records EXT_CSD_CARD_TYPE_HS400_{1_8V,1_2V}
  in mmc_avail_type when both the host (mmc-hs400-1_8v / mmc-hs400-1_2v
  in DT) and the card advertise it. HS400 reuses hs200_max_dtr (200 MHz)
  since it's the same SDCLK rate, just DDR-sampled.

- mmc_set_bus_speed() treats HS200 and HS400 as the same rate tier.

- mmc_select_hs400() implements the JEDEC sequence:
    1. CMD6 EXT_CSD_HS_TIMING = HS
    2. host: MMC_TIMING_MMC_HS, clock = hs_max_dtr
    3. CMD6 EXT_CSD_BUS_WIDTH = EXT_CSD_DDR_BUS_WIDTH_8
    4. CMD6 EXT_CSD_HS_TIMING = HS400
    5. host: MMC_TIMING_MMC_HS400, clock back to 200 MHz

  Bails early if the card lacks HS400 capability or the bus isn't 8-bit.

- mci_startup_mmc() runs mmc_select_hs400() right after a successful
  HS200 tuning when EXT_CSD_CARD_TYPE_HS400 is in mmc_avail_type.

- mmc_card_hs400() helper added in mci.h, mirroring mmc_card_hs200().

Host drivers must additionally handle MMC_TIMING_MMC_HS400 in their
set_ios / set_clock paths (e.g. setting the controller-specific HS400
bit in HOST_CONTROL2, configuring data-strobe sampling). Without that
the core will run the transition but the host won't sample correctly.

Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mci/mci-core.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++--
 include/mci.h          |  5 ++++
 2 files changed, 82 insertions(+), 2 deletions(-)

diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
index 23b9117869..58b28ec653 100644
--- a/drivers/mci/mci-core.c
+++ b/drivers/mci/mci-core.c
@@ -126,7 +126,8 @@ static int mci_set_blocklen(struct mci *mci, unsigned len)
 {
 	struct mci_cmd cmd = {0};
 
-	if (mci->host->ios.timing == MMC_TIMING_MMC_DDR52)
+	if (mci->host->ios.timing == MMC_TIMING_MMC_DDR52 ||
+	    mmc_card_hs400(mci))
 		return 0;
 
 	mci_setup_cmd(&cmd, MMC_CMD_SET_BLOCKLEN, len, MMC_RSP_R1);
@@ -1051,6 +1052,8 @@ static const char *mci_timing_tostr(unsigned timing)
 		return "MMC DDR52";
 	case MMC_TIMING_MMC_HS200:
 		return "HS200";
+	case MMC_TIMING_MMC_HS400:
+		return "HS400";
 	default:
 		return "unknown"; /* shouldn't happen */
 	}
@@ -1783,6 +1786,18 @@ static void mmc_select_max_dtr(struct mci *mci)
 		avail_type |= EXT_CSD_CARD_TYPE_HS200_1_2V;
 	}
 
+	if ((caps2 & MMC_CAP2_HS400_1_8V) &&
+	    (card_type & EXT_CSD_CARD_TYPE_HS400_1_8V)) {
+		hs200_max_dtr = MMC_HS200_MAX_DTR;
+		avail_type |= EXT_CSD_CARD_TYPE_HS400_1_8V;
+	}
+
+	if ((caps2 & MMC_CAP2_HS400_1_2V) &&
+	    (card_type & EXT_CSD_CARD_TYPE_HS400_1_2V)) {
+		hs200_max_dtr = MMC_HS200_MAX_DTR;
+		avail_type |= EXT_CSD_CARD_TYPE_HS400_1_2V;
+	}
+
 	mci->host->hs200_max_dtr = hs200_max_dtr;
 	mci->host->hs_max_dtr = hs_max_dtr;
 	mci->host->mmc_avail_type = avail_type;
@@ -1875,7 +1890,7 @@ static void mmc_set_bus_speed(struct mci *mci)
 {
 	unsigned int max_dtr = (unsigned int)-1;
 
-	if (mmc_card_hs200(mci) &&
+	if ((mmc_card_hs200(mci) || mmc_card_hs400(mci)) &&
 		max_dtr > mci->host->hs200_max_dtr)
 		max_dtr = mci->host->hs200_max_dtr;
 	else if (mmc_card_hs(mci) && max_dtr > mci->host->hs_max_dtr)
@@ -1921,6 +1936,62 @@ int mmc_hs200_tuning(struct mci *mci)
 	return mci_execute_tuning(mci);
 }
 
+/*
+ * Switch from HS200 to HS400 per JEDEC. The card must already be in HS200
+ * (with successful tuning) and 8-bit bus width. The transition sequence is:
+ *
+ *   1. Switch the card back to HS timing.
+ *   2. Drop the host clock to HS rate (<= 52 MHz).
+ *   3. Switch the card to 8-bit DDR bus width.
+ *   4. Switch the card to HS400 timing.
+ *   5. Switch the host to HS400 timing and ramp the clock back to HS200 rate
+ *      (200 MHz, used as DDR so effective 400 MHz / 8-bit data lane).
+ */
+static int mmc_select_hs400(struct mci *mci)
+{
+	struct mci_host *host = mci->host;
+	int err;
+	u8 val;
+
+	if (!(host->mmc_avail_type & EXT_CSD_CARD_TYPE_HS400) ||
+	    host->ios.bus_width != MMC_BUS_WIDTH_8)
+		return 0;
+
+	/* Step 1: switch card back to HS timing */
+	err = mci_switch(mci, EXT_CSD_HS_TIMING, EXT_CSD_TIMING_HS);
+	if (err) {
+		dev_err(&mci->dev, "switch to HS from HS200 failed: %d\n", err);
+		return err;
+	}
+
+	/* Step 2: drop host clock to HS rate */
+	mci_set_timing(mci, MMC_TIMING_MMC_HS);
+	mci_set_clock(mci, host->hs_max_dtr);
+
+	/* Step 3: switch card to 8-bit DDR */
+	err = mci_switch(mci, EXT_CSD_BUS_WIDTH, EXT_CSD_DDR_BUS_WIDTH_8);
+	if (err) {
+		dev_err(&mci->dev, "switch to DDR for HS400 failed: %d\n", err);
+		return err;
+	}
+
+	/* Step 4: switch card to HS400 timing */
+	val = EXT_CSD_TIMING_HS400 | (host->drive_strength << EXT_CSD_DRV_STR_SHIFT);
+	err = mci_switch(mci, EXT_CSD_HS_TIMING, val);
+	if (err) {
+		dev_err(&mci->dev, "switch to HS400 failed: %d\n", err);
+		return err;
+	}
+
+	/* Step 5: switch host to HS400 timing and ramp clock */
+	mci_set_timing(mci, MMC_TIMING_MMC_HS400);
+	mmc_set_bus_speed(mci);
+
+	dev_dbg(&mci->dev, "HS400 selected\n");
+
+	return 0;
+}
+
 static int mci_startup_mmc(struct mci *mci)
 {
 	struct mci_host *host = mci->host;
@@ -1947,6 +2018,10 @@ static int mci_startup_mmc(struct mci *mci)
 			ret = mmc_hs200_tuning(mci);
 			if (!ret) {
 				dev_dbg(&mci->dev, "HS200 tuning succeeded\n");
+
+				if (host->mmc_avail_type & EXT_CSD_CARD_TYPE_HS400)
+					mmc_select_hs400(mci);
+
 				return 0;
 			}
 
diff --git a/include/mci.h b/include/mci.h
index 8348f97432..a7cd0c5ae2 100644
--- a/include/mci.h
+++ b/include/mci.h
@@ -802,4 +802,9 @@ static inline bool mmc_card_hs200(struct mci *mci)
 	return mci->host->ios.timing == MMC_TIMING_MMC_HS200;
 }
 
+static inline bool mmc_card_hs400(struct mci *mci)
+{
+	return mci->host->ios.timing == MMC_TIMING_MMC_HS400;
+}
+
 #endif /* _MCI_H_ */

-- 
2.47.3

Re: [PATCH 04/10] mci: add HS400 mode selection

[Ahmad Fatoum]

Hello Sascha,

On 5/11/26 2:07 PM, Sascha Hauer wrote:
> eMMC HS400 transitions through HS and 8-bit DDR per JEDEC, on top of
> a card already tuned in HS200. Wire that up in the core:
> 
> - mmc_select_max_dtr() now records EXT_CSD_CARD_TYPE_HS400_{1_8V,1_2V}
>   in mmc_avail_type when both the host (mmc-hs400-1_8v / mmc-hs400-1_2v
>   in DT) and the card advertise it. HS400 reuses hs200_max_dtr (200 MHz)
>   since it's the same SDCLK rate, just DDR-sampled.
> 
> - mmc_set_bus_speed() treats HS200 and HS400 as the same rate tier.
> 
> - mmc_select_hs400() implements the JEDEC sequence:
>     1. CMD6 EXT_CSD_HS_TIMING = HS
>     2. host: MMC_TIMING_MMC_HS, clock = hs_max_dtr
>     3. CMD6 EXT_CSD_BUS_WIDTH = EXT_CSD_DDR_BUS_WIDTH_8
>     4. CMD6 EXT_CSD_HS_TIMING = HS400
>     5. host: MMC_TIMING_MMC_HS400, clock back to 200 MHz
> 
>   Bails early if the card lacks HS400 capability or the bus isn't 8-bit.
> 
> - mci_startup_mmc() runs mmc_select_hs400() right after a successful
>   HS200 tuning when EXT_CSD_CARD_TYPE_HS400 is in mmc_avail_type.
> 
> - mmc_card_hs400() helper added in mci.h, mirroring mmc_card_hs200().
> 
> Host drivers must additionally handle MMC_TIMING_MMC_HS400 in their
> set_ios / set_clock paths (e.g. setting the controller-specific HS400
> bit in HOST_CONTROL2, configuring data-strobe sampling). Without that
> the core will run the transition but the host won't sample correctly.

This patch is missing the downgrade code. If HS400 doesn't work out, we
should fallback to HS200.

Cheers,
Ahmad

> 
> Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/mci/mci-core.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++--
>  include/mci.h          |  5 ++++
>  2 files changed, 82 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
> index 23b9117869..58b28ec653 100644
> --- a/drivers/mci/mci-core.c
> +++ b/drivers/mci/mci-core.c
> @@ -126,7 +126,8 @@ static int mci_set_blocklen(struct mci *mci, unsigned len)
>  {
>  	struct mci_cmd cmd = {0};
>  
> -	if (mci->host->ios.timing == MMC_TIMING_MMC_DDR52)
> +	if (mci->host->ios.timing == MMC_TIMING_MMC_DDR52 ||
> +	    mmc_card_hs400(mci))
>  		return 0;
>  
>  	mci_setup_cmd(&cmd, MMC_CMD_SET_BLOCKLEN, len, MMC_RSP_R1);
> @@ -1051,6 +1052,8 @@ static const char *mci_timing_tostr(unsigned timing)
>  		return "MMC DDR52";
>  	case MMC_TIMING_MMC_HS200:
>  		return "HS200";
> +	case MMC_TIMING_MMC_HS400:
> +		return "HS400";
>  	default:
>  		return "unknown"; /* shouldn't happen */
>  	}
> @@ -1783,6 +1786,18 @@ static void mmc_select_max_dtr(struct mci *mci)
>  		avail_type |= EXT_CSD_CARD_TYPE_HS200_1_2V;
>  	}
>  
> +	if ((caps2 & MMC_CAP2_HS400_1_8V) &&

This bit is not decoded in mci_print_caps (same or _1_2V)


> +	    (card_type & EXT_CSD_CARD_TYPE_HS400_1_8V)) {
> +		hs200_max_dtr = MMC_HS200_MAX_DTR;
> +		avail_type |= EXT_CSD_CARD_TYPE_HS400_1_8V;
> +	}
> +
> +	if ((caps2 & MMC_CAP2_HS400_1_2V) &&
> +	    (card_type & EXT_CSD_CARD_TYPE_HS400_1_2V)) {
> +		hs200_max_dtr = MMC_HS200_MAX_DTR;
> +		avail_type |= EXT_CSD_CARD_TYPE_HS400_1_2V;
> +	}
> +
>  	mci->host->hs200_max_dtr = hs200_max_dtr;
>  	mci->host->hs_max_dtr = hs_max_dtr;
>  	mci->host->mmc_avail_type = avail_type;
> @@ -1875,7 +1890,7 @@ static void mmc_set_bus_speed(struct mci *mci)
>  {
>  	unsigned int max_dtr = (unsigned int)-1;
>  
> -	if (mmc_card_hs200(mci) &&
> +	if ((mmc_card_hs200(mci) || mmc_card_hs400(mci)) &&
>  		max_dtr > mci->host->hs200_max_dtr)
>  		max_dtr = mci->host->hs200_max_dtr;
>  	else if (mmc_card_hs(mci) && max_dtr > mci->host->hs_max_dtr)
> @@ -1921,6 +1936,62 @@ int mmc_hs200_tuning(struct mci *mci)
>  	return mci_execute_tuning(mci);
>  }
>  
> +/*
> + * Switch from HS200 to HS400 per JEDEC. The card must already be in HS200
> + * (with successful tuning) and 8-bit bus width. The transition sequence is:
> + *
> + *   1. Switch the card back to HS timing.
> + *   2. Drop the host clock to HS rate (<= 52 MHz).
> + *   3. Switch the card to 8-bit DDR bus width.
> + *   4. Switch the card to HS400 timing.
> + *   5. Switch the host to HS400 timing and ramp the clock back to HS200 rate
> + *      (200 MHz, used as DDR so effective 400 MHz / 8-bit data lane).
> + */
> +static int mmc_select_hs400(struct mci *mci)
> +{
> +	struct mci_host *host = mci->host;
> +	int err;
> +	u8 val;
> +
> +	if (!(host->mmc_avail_type & EXT_CSD_CARD_TYPE_HS400) ||
> +	    host->ios.bus_width != MMC_BUS_WIDTH_8)
> +		return 0;
> +
> +	/* Step 1: switch card back to HS timing */
> +	err = mci_switch(mci, EXT_CSD_HS_TIMING, EXT_CSD_TIMING_HS);
> +	if (err) {
> +		dev_err(&mci->dev, "switch to HS from HS200 failed: %d\n", err);
> +		return err;
> +	}
> +
> +	/* Step 2: drop host clock to HS rate */
> +	mci_set_timing(mci, MMC_TIMING_MMC_HS);
> +	mci_set_clock(mci, host->hs_max_dtr);

i.MX eSDHC implements ops.hs400_prepare_ddr here, but I guess it's ok to
skip until we add HS400 support there.

> +
> +	/* Step 3: switch card to 8-bit DDR */
> +	err = mci_switch(mci, EXT_CSD_BUS_WIDTH, EXT_CSD_DDR_BUS_WIDTH_8);
> +	if (err) {
> +		dev_err(&mci->dev, "switch to DDR for HS400 failed: %d\n", err);
> +		return err;
> +	}
> +> +	/* Step 4: switch card to HS400 timing */
> +	val = EXT_CSD_TIMING_HS400 | (host->drive_strength << EXT_CSD_DRV_STR_SHIFT);
> +	err = mci_switch(mci, EXT_CSD_HS_TIMING, val);
> +	if (err) {
> +		dev_err(&mci->dev, "switch to HS400 failed: %d\n", err);
> +		return err;
> +	}
> +
> +	/* Step 5: switch host to HS400 timing and ramp clock */
> +	mci_set_timing(mci, MMC_TIMING_MMC_HS400);
> +	mmc_set_bus_speed(mci);
> +
> +	dev_dbg(&mci->dev, "HS400 selected\n");
> +
> +	return 0;
> +}
> +
>  static int mci_startup_mmc(struct mci *mci)
>  {
>  	struct mci_host *host = mci->host;
> @@ -1947,6 +2018,10 @@ static int mci_startup_mmc(struct mci *mci)
>  			ret = mmc_hs200_tuning(mci);
>  			if (!ret) {
>  				dev_dbg(&mci->dev, "HS200 tuning succeeded\n");
> +
> +				if (host->mmc_avail_type & EXT_CSD_CARD_TYPE_HS400)
> +					mmc_select_hs400(mci);
> +
>  				return 0;
>  			}
>  
> diff --git a/include/mci.h b/include/mci.h
> index 8348f97432..a7cd0c5ae2 100644
> --- a/include/mci.h
> +++ b/include/mci.h
> @@ -802,4 +802,9 @@ static inline bool mmc_card_hs200(struct mci *mci)
>  	return mci->host->ios.timing == MMC_TIMING_MMC_HS200;
>  }
>  
> +static inline bool mmc_card_hs400(struct mci *mci)
> +{
> +	return mci->host->ios.timing == MMC_TIMING_MMC_HS400;
> +}
> +
>  #endif /* _MCI_H_ */
> 

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |

Re: [PATCH 04/10] mci: add HS400 mode selection

[Sascha Hauer]

On 2026-05-18 11:36, Ahmad Fatoum wrote:
> Hello Sascha,
> 
> On 5/11/26 2:07 PM, Sascha Hauer wrote:
> > eMMC HS400 transitions through HS and 8-bit DDR per JEDEC, on top of
> > a card already tuned in HS200. Wire that up in the core:
> > 
> > - mmc_select_max_dtr() now records EXT_CSD_CARD_TYPE_HS400_{1_8V,1_2V}
> >   in mmc_avail_type when both the host (mmc-hs400-1_8v / mmc-hs400-1_2v
> >   in DT) and the card advertise it. HS400 reuses hs200_max_dtr (200 MHz)
> >   since it's the same SDCLK rate, just DDR-sampled.
> > 
> > - mmc_set_bus_speed() treats HS200 and HS400 as the same rate tier.
> > 
> > - mmc_select_hs400() implements the JEDEC sequence:
> >     1. CMD6 EXT_CSD_HS_TIMING = HS
> >     2. host: MMC_TIMING_MMC_HS, clock = hs_max_dtr
> >     3. CMD6 EXT_CSD_BUS_WIDTH = EXT_CSD_DDR_BUS_WIDTH_8
> >     4. CMD6 EXT_CSD_HS_TIMING = HS400
> >     5. host: MMC_TIMING_MMC_HS400, clock back to 200 MHz
> > 
> >   Bails early if the card lacks HS400 capability or the bus isn't 8-bit.
> > 
> > - mci_startup_mmc() runs mmc_select_hs400() right after a successful
> >   HS200 tuning when EXT_CSD_CARD_TYPE_HS400 is in mmc_avail_type.
> > 
> > - mmc_card_hs400() helper added in mci.h, mirroring mmc_card_hs200().
> > 
> > Host drivers must additionally handle MMC_TIMING_MMC_HS400 in their
> > set_ios / set_clock paths (e.g. setting the controller-specific HS400
> > bit in HOST_CONTROL2, configuring data-strobe sampling). Without that
> > the core will run the transition but the host won't sample correctly.
> 
> This patch is missing the downgrade code. If HS400 doesn't work out, we
> should fallback to HS200.

If a switch to HS400 fails we do not know in which mode the card
actually is, so we do not have a reliable way to talk to the card. Is it
DDR or SDR? So our only chance would be to re-do the whole card
initialization.

> > @@ -1783,6 +1786,18 @@ static void mmc_select_max_dtr(struct mci *mci)
> >  		avail_type |= EXT_CSD_CARD_TYPE_HS200_1_2V;
> >  	}
> >  
> > +	if ((caps2 & MMC_CAP2_HS400_1_8V) &&
> 
> This bit is not decoded in mci_print_caps (same or _1_2V)

Ok, will add.

Sascha

--
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 05/10] mci: add HS400 Enhanced Strobe (HS400ES) selection

[Sascha Hauer]

HS400ES bypasses HS200 tuning entirely: the eMMC drives a data strobe
line which the controller samples on, so the read path doesn't need
software-tuned phase. It still ends up in MMC_TIMING_MMC_HS400.

Detection: a card advertises HS400ES via EXT_CSD_STROBE_SUPPORT[184],
which sits outside DEVICE_TYPE; record it as a synthetic
EXT_CSD_CARD_TYPE_HS400ES bit in mmc_avail_type when the host
declares MMC_CAP2_HS400_ES (mmc-hs400-enhanced-strobe in DT) and the
card has both STROBE_SUPPORT set and one of the HS400 voltage variants.

Transition (mmc_select_hs400es):
  1. Negotiate 8-bit bus in HS mode (HS400 needs 8-bit).
  2. CMD6 EXT_CSD_HS_TIMING = HS, host to HS rate.
  3. CMD6 EXT_CSD_BUS_WIDTH = 8-bit DDR | STROBE bit, so the card
     starts driving the strobe line.
  4. CMD6 EXT_CSD_HS_TIMING = HS400, host to HS400 timing at
     hs200_max_dtr (200 MHz, DDR-sampled => 400 MT/s).
  5. Call host->ops.hs400_enhanced_strobe so the controller enables
     enhanced strobe sampling.

mmc_select_timing prefers HS400ES over HS200 + HS400; on any failure
it clears the flag and falls back to HS200. mci_startup_mmc returns
early when mmc_select_timing already landed us in HS400 (HS400ES has
no follow-up tuning step).

mci_ops gains an optional hs400_enhanced_strobe hook for the
controller's enhanced-strobe enable.

Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mci/mci-core.c | 108 ++++++++++++++++++++++++++++++++++++++++++++++++-
 include/mci.h          |   4 ++
 2 files changed, 111 insertions(+), 1 deletion(-)

diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
index 58b28ec653..0cfddb8d43 100644
--- a/drivers/mci/mci-core.c
+++ b/drivers/mci/mci-core.c
@@ -1798,6 +1798,16 @@ static void mmc_select_max_dtr(struct mci *mci)
 		avail_type |= EXT_CSD_CARD_TYPE_HS400_1_2V;
 	}
 
+	/*
+	 * HS400ES capability is reported in EXT_CSD_STROBE_SUPPORT (byte 184),
+	 * not in DEVICE_TYPE. Combine with one of the HS400 voltage variants;
+	 * which voltage is used follows the same negotiation as HS400.
+	 */
+	if ((caps2 & MMC_CAP2_HS400_ES) &&
+	    mci->ext_csd[EXT_CSD_STROBE_SUPPORT] &&
+	    (avail_type & EXT_CSD_CARD_TYPE_HS400))
+		avail_type |= EXT_CSD_CARD_TYPE_HS400ES;
+
 	mci->host->hs200_max_dtr = hs200_max_dtr;
 	mci->host->hs_max_dtr = hs_max_dtr;
 	mci->host->mmc_avail_type = avail_type;
@@ -1901,6 +1911,83 @@ static void mmc_set_bus_speed(struct mci *mci)
 	mci_set_clock(mci, max_dtr);
 }
 
+/*
+ * Switch directly to HS400 with Enhanced Strobe per JEDEC. Unlike plain
+ * HS400 this does not require HS200 tuning first - the controller samples
+ * data using the eMMC-driven strobe line. The transition is:
+ *
+ *   1. Negotiate 8-bit bus width in HS mode.
+ *   2. Switch the card to HS timing.
+ *   3. Set host to MMC_TIMING_MMC_HS at hs_max_dtr.
+ *   4. Switch card BUS_WIDTH to 8-bit DDR with the STROBE bit set, so
+ *      the card drives the strobe line.
+ *   5. Switch the card to HS400 timing.
+ *   6. Switch the host to MMC_TIMING_MMC_HS400 and ramp the clock to
+ *      hs200_max_dtr (200 MHz, DDR-sampled => 400 MT/s).
+ *   7. Tell the controller to enable enhanced strobe sampling.
+ */
+static int mmc_select_hs400es(struct mci *mci)
+{
+	struct mci_host *host = mci->host;
+	int err;
+	u8 val;
+
+	/* Step 1: 8-bit bus is mandatory for HS400ES */
+	err = mci_mmc_try_bus_width(mci, MMC_BUS_WIDTH_8, MMC_TIMING_MMC_HS);
+	if (err < 0) {
+		dev_err(&mci->dev, "HS400ES requires 8-bit bus width\n");
+		return err;
+	}
+
+	/* Step 2: switch card to HS timing */
+	err = mci_switch(mci, EXT_CSD_HS_TIMING, EXT_CSD_TIMING_HS);
+	if (err) {
+		dev_err(&mci->dev, "switch to HS for HS400ES failed: %d\n", err);
+		return err;
+	}
+
+	/* Step 3: host follows to HS rate */
+	mci_set_timing(mci, MMC_TIMING_MMC_HS);
+	mci_set_clock(mci, host->hs_max_dtr);
+
+	err = mci_switch_status(mci, true);
+	if (err)
+		return err;
+
+	/* Step 4: 8-bit DDR with strobe enabled */
+	val = EXT_CSD_DDR_BUS_WIDTH_8 | EXT_CSD_BUS_WIDTH_STROBE;
+	err = mci_switch(mci, EXT_CSD_BUS_WIDTH, val);
+	if (err) {
+		dev_err(&mci->dev, "switch to DDR8 with strobe failed: %d\n", err);
+		return err;
+	}
+
+	/* Step 5: switch card to HS400 */
+	val = EXT_CSD_TIMING_HS400 | (host->drive_strength << EXT_CSD_DRV_STR_SHIFT);
+	err = mci_switch(mci, EXT_CSD_HS_TIMING, val);
+	if (err) {
+		dev_err(&mci->dev, "switch to HS400 for HS400ES failed: %d\n", err);
+		return err;
+	}
+
+	/* Step 6: host to HS400 timing and final clock */
+	mci_set_timing(mci, MMC_TIMING_MMC_HS400);
+	mmc_set_bus_speed(mci);
+
+	err = mci_switch_status(mci, true);
+	if (err)
+		return err;
+
+	/* Step 7: enable enhanced strobe in the controller */
+	host->ios.enhanced_strobe = true;
+	if (host->ops.hs400_enhanced_strobe)
+		host->ops.hs400_enhanced_strobe(host, &host->ios);
+
+	dev_dbg(&mci->dev, "HS400ES selected\n");
+
+	return 0;
+}
+
 /*
  * Activate HS200 or HS400ES mode if supported.
  */
@@ -1910,6 +1997,20 @@ int mmc_select_timing(struct mci *mci)
 
 	mmc_select_max_dtr(mci);
 
+	/*
+	 * HS400ES is the preferred path when both card and host support it:
+	 * it ends up in HS400 without needing HS200 tuning. If the transition
+	 * fails for any reason, drop the flag and fall through to HS200/HS400.
+	 */
+	if (mci->host->mmc_avail_type & EXT_CSD_CARD_TYPE_HS400ES) {
+		err = mmc_select_hs400es(mci);
+		if (!err)
+			goto out;
+		dev_dbg(&mci->dev, "HS400ES failed (%d), trying HS200\n", err);
+		mci->host->mmc_avail_type &= ~EXT_CSD_CARD_TYPE_HS400ES;
+		err = 0;
+	}
+
 	if (mci->host->mmc_avail_type & EXT_CSD_CARD_TYPE_HS200) {
 		err = mmc_select_hs200(mci);
 		if (err == -EBADMSG)
@@ -2014,6 +2115,10 @@ static int mci_startup_mmc(struct mci *mci)
 		if (ret)
 			return ret;
 
+		/* HS400ES took us straight to HS400 without tuning. Done. */
+		if (mmc_card_hs400(mci))
+			return 0;
+
 		if (mmc_card_hs200(mci)) {
 			ret = mmc_hs200_tuning(mci);
 			if (!ret) {
@@ -2698,7 +2803,8 @@ static void mci_info(struct device *dev)
 		bw = 1;
 
 	printf("  current buswidth: %d\n", bw);
-	printf("  current timing: %s\n", mci_timing_tostr(host->ios.timing));
+	printf("  current timing: %s%s\n", mci_timing_tostr(host->ios.timing),
+	       host->ios.enhanced_strobe ? " (Enhanced Strobe)" : "");
 	mci_print_caps(host->host_caps, host->caps2);
 
 	printf("Card information:\n");
diff --git a/include/mci.h b/include/mci.h
index a7cd0c5ae2..7a86a80b03 100644
--- a/include/mci.h
+++ b/include/mci.h
@@ -406,6 +406,7 @@
 #define EXT_CSD_DDR_BUS_WIDTH_4	5	/* Card is in 4 bit DDR mode */
 #define EXT_CSD_DDR_BUS_WIDTH_8	6	/* Card is in 8 bit DDR mode */
 #define EXT_CSD_DDR_FLAG	BIT(2)	/* Flag for DDR mode */
+#define EXT_CSD_BUS_WIDTH_STROBE BIT(7) /* Enhanced strobe mode (HS400ES) */
 
 #define EXT_CSD_TIMING_BC	0	/* Backwards compatility */
 #define EXT_CSD_TIMING_HS	1	/* High speed */
@@ -567,6 +568,7 @@ struct mci_ios {
 	enum mci_bus_width	bus_width;		/* data bus width */
 	enum mci_timing		timing;			/* timing specification used */
 	unsigned char		drv_type;		/* driver type (A, B, C, D) */
+	bool			enhanced_strobe;	/* HS400ES selected */
 };
 
 struct mci;
@@ -586,6 +588,8 @@ struct mci_ops {
 	/* The tuning command opcode value is different for SD and eMMC cards */
 	int (*execute_tuning)(struct mci_host *, u32);
 	void (*set_uhs_signaling)(struct mci_host *host, unsigned int timing);
+	/* Enable enhanced strobe in the controller (HS400ES) */
+	void (*hs400_enhanced_strobe)(struct mci_host *, struct mci_ios *);
 };
 
 /** host information */

-- 
2.47.3

Re: [PATCH 05/10] mci: add HS400 Enhanced Strobe (HS400ES) selection

[Ahmad Fatoum]

Hi,

On 5/11/26 2:08 PM, Sascha Hauer wrote:
> HS400ES bypasses HS200 tuning entirely: the eMMC drives a data strobe
> line which the controller samples on, so the read path doesn't need
> software-tuned phase. It still ends up in MMC_TIMING_MMC_HS400.
> 
> Detection: a card advertises HS400ES via EXT_CSD_STROBE_SUPPORT[184],
> which sits outside DEVICE_TYPE; record it as a synthetic
> EXT_CSD_CARD_TYPE_HS400ES bit in mmc_avail_type when the host
> declares MMC_CAP2_HS400_ES (mmc-hs400-enhanced-strobe in DT) and the
> card has both STROBE_SUPPORT set and one of the HS400 voltage variants.
> 
> Transition (mmc_select_hs400es):
>   1. Negotiate 8-bit bus in HS mode (HS400 needs 8-bit).
>   2. CMD6 EXT_CSD_HS_TIMING = HS, host to HS rate.
>   3. CMD6 EXT_CSD_BUS_WIDTH = 8-bit DDR | STROBE bit, so the card
>      starts driving the strobe line.
>   4. CMD6 EXT_CSD_HS_TIMING = HS400, host to HS400 timing at
>      hs200_max_dtr (200 MHz, DDR-sampled => 400 MT/s).
>   5. Call host->ops.hs400_enhanced_strobe so the controller enables
>      enhanced strobe sampling.
> 
> mmc_select_timing prefers HS400ES over HS200 + HS400; on any failure
> it clears the flag and falls back to HS200. mci_startup_mmc returns
> early when mmc_select_timing already landed us in HS400 (HS400ES has
> no follow-up tuning step).
> 
> mci_ops gains an optional hs400_enhanced_strobe hook for the
> controller's enhanced-strobe enable.
> 
> Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/mci/mci-core.c | 108 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  include/mci.h          |   4 ++
>  2 files changed, 111 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
> index 58b28ec653..0cfddb8d43 100644
> --- a/drivers/mci/mci-core.c
> +++ b/drivers/mci/mci-core.c
> @@ -1798,6 +1798,16 @@ static void mmc_select_max_dtr(struct mci *mci)
>  		avail_type |= EXT_CSD_CARD_TYPE_HS400_1_2V;
>  	}
>  
> +	/*
> +	 * HS400ES capability is reported in EXT_CSD_STROBE_SUPPORT (byte 184),
> +	 * not in DEVICE_TYPE. Combine with one of the HS400 voltage variants;
> +	 * which voltage is used follows the same negotiation as HS400.
> +	 */
> +	if ((caps2 & MMC_CAP2_HS400_ES) &&
> +	    mci->ext_csd[EXT_CSD_STROBE_SUPPORT] &&

Please add to mci_print_caps()

Also, please extend mci_timing_tostr() to return "HS400ES" for enhanced
strobe.

> +	    (avail_type & EXT_CSD_CARD_TYPE_HS400))
> +		avail_type |= EXT_CSD_CARD_TYPE_HS400ES;
> +
>  	mci->host->hs200_max_dtr = hs200_max_dtr;
>  	mci->host->hs_max_dtr = hs_max_dtr;
>  	mci->host->mmc_avail_type = avail_type;
> @@ -1901,6 +1911,83 @@ static void mmc_set_bus_speed(struct mci *mci)
>  	mci_set_clock(mci, max_dtr);
>  }
>  
> +/*
> + * Switch directly to HS400 with Enhanced Strobe per JEDEC. Unlike plain
> + * HS400 this does not require HS200 tuning first - the controller samples
> + * data using the eMMC-driven strobe line. The transition is:
> + *
> + *   1. Negotiate 8-bit bus width in HS mode.
> + *   2. Switch the card to HS timing.
> + *   3. Set host to MMC_TIMING_MMC_HS at hs_max_dtr.
> + *   4. Switch card BUS_WIDTH to 8-bit DDR with the STROBE bit set, so
> + *      the card drives the strobe line.
> + *   5. Switch the card to HS400 timing.
> + *   6. Switch the host to MMC_TIMING_MMC_HS400 and ramp the clock to
> + *      hs200_max_dtr (200 MHz, DDR-sampled => 400 MT/s).
> + *   7. Tell the controller to enable enhanced strobe sampling.
> + */
> +static int mmc_select_hs400es(struct mci *mci)
> +{
> +	struct mci_host *host = mci->host;
> +	int err;
> +	u8 val;
> +
> +	/* Step 1: 8-bit bus is mandatory for HS400ES */
> +	err = mci_mmc_try_bus_width(mci, MMC_BUS_WIDTH_8, MMC_TIMING_MMC_HS);
> +	if (err < 0) {
> +		dev_err(&mci->dev, "HS400ES requires 8-bit bus width\n");
> +		return err;
> +	}
> +
> +	/* Step 2: switch card to HS timing */
> +	err = mci_switch(mci, EXT_CSD_HS_TIMING, EXT_CSD_TIMING_HS);
> +	if (err) {
> +		dev_err(&mci->dev, "switch to HS for HS400ES failed: %d\n", err);
> +		return err;
> +	}
> +
> +	/* Step 3: host follows to HS rate */
> +	mci_set_timing(mci, MMC_TIMING_MMC_HS);
> +	mci_set_clock(mci, host->hs_max_dtr);
> +
> +	err = mci_switch_status(mci, true);
> +	if (err)
> +		return err;
> +
> +	/* Step 4: 8-bit DDR with strobe enabled */
> +	val = EXT_CSD_DDR_BUS_WIDTH_8 | EXT_CSD_BUS_WIDTH_STROBE;
> +	err = mci_switch(mci, EXT_CSD_BUS_WIDTH, val);
> +	if (err) {
> +		dev_err(&mci->dev, "switch to DDR8 with strobe failed: %d\n", err);
> +		return err;
> +	}
> +> +	/* Step 5: switch card to HS400 */
> +	val = EXT_CSD_TIMING_HS400 | (host->drive_strength << EXT_CSD_DRV_STR_SHIFT);

The drive_strength is initialized by mmc_select_driver_type() and it
seems no one will have called it for HS400ES?

> +	err = mci_switch(mci, EXT_CSD_HS_TIMING, val);
> +	if (err) {
> +		dev_err(&mci->dev, "switch to HS400 for HS400ES failed: %d\n", err);
> +		return err;
> +	}
> +
> +	/* Step 6: host to HS400 timing and final clock */
> +	mci_set_timing(mci, MMC_TIMING_MMC_HS400);
> +	mmc_set_bus_speed(mci);
> +
> +	err = mci_switch_status(mci, true);
> +	if (err)
> +		return err;

Should this not be _after_ the enhanced strobe setting, so we are able
to propagate an error on issues?

> +
> +	/* Step 7: enable enhanced strobe in the controller */
> +	host->ios.enhanced_strobe = true;
> +	if (host->ops.hs400_enhanced_strobe)
> +		host->ops.hs400_enhanced_strobe(host, &host->ios);
> +
> +	dev_dbg(&mci->dev, "HS400ES selected\n");
> +
> +	return 0;
> +}
> +
>  /*
>   * Activate HS200 or HS400ES mode if supported.
>   */
> @@ -1910,6 +1997,20 @@ int mmc_select_timing(struct mci *mci)
>  
>  	mmc_select_max_dtr(mci);
>  
> +	/*
> +	 * HS400ES is the preferred path when both card and host support it:
> +	 * it ends up in HS400 without needing HS200 tuning. If the transition
> +	 * fails for any reason, drop the flag and fall through to HS200/HS400.
> +	 */
> +	if (mci->host->mmc_avail_type & EXT_CSD_CARD_TYPE_HS400ES) {
> +		err = mmc_select_hs400es(mci);
> +		if (!err)
> +			goto out;
> +		dev_dbg(&mci->dev, "HS400ES failed (%d), trying HS200\n", err);
> +		mci->host->mmc_avail_type &= ~EXT_CSD_CARD_TYPE_HS400ES;
> +		err = 0;
> +	}
> +
>  	if (mci->host->mmc_avail_type & EXT_CSD_CARD_TYPE_HS200) {
>  		err = mmc_select_hs200(mci);
>  		if (err == -EBADMSG)
> @@ -2014,6 +2115,10 @@ static int mci_startup_mmc(struct mci *mci)
>  		if (ret)
>  			return ret;
>  
> +		/* HS400ES took us straight to HS400 without tuning. Done. */
> +		if (mmc_card_hs400(mci))
> +			return 0;
> +
>  		if (mmc_card_hs200(mci)) {
>  			ret = mmc_hs200_tuning(mci);
>  			if (!ret) {
> @@ -2698,7 +2803,8 @@ static void mci_info(struct device *dev)
>  		bw = 1;
>  
>  	printf("  current buswidth: %d\n", bw);
> -	printf("  current timing: %s\n", mci_timing_tostr(host->ios.timing));
> +	printf("  current timing: %s%s\n", mci_timing_tostr(host->ios.timing),
> +	       host->ios.enhanced_strobe ? " (Enhanced Strobe)" : "");

I would just append an ES suffix.

>  	mci_print_caps(host->host_caps, host->caps2);
>  
>  	printf("Card information:\n");
> diff --git a/include/mci.h b/include/mci.h
> index a7cd0c5ae2..7a86a80b03 100644
> --- a/include/mci.h
> +++ b/include/mci.h
> @@ -406,6 +406,7 @@
>  #define EXT_CSD_DDR_BUS_WIDTH_4	5	/* Card is in 4 bit DDR mode */
>  #define EXT_CSD_DDR_BUS_WIDTH_8	6	/* Card is in 8 bit DDR mode */
>  #define EXT_CSD_DDR_FLAG	BIT(2)	/* Flag for DDR mode */
> +#define EXT_CSD_BUS_WIDTH_STROBE BIT(7) /* Enhanced strobe mode (HS400ES) */
>  
>  #define EXT_CSD_TIMING_BC	0	/* Backwards compatility */
>  #define EXT_CSD_TIMING_HS	1	/* High speed */
> @@ -567,6 +568,7 @@ struct mci_ios {
>  	enum mci_bus_width	bus_width;		/* data bus width */
>  	enum mci_timing		timing;			/* timing specification used */
>  	unsigned char		drv_type;		/* driver type (A, B, C, D) */
> +	bool			enhanced_strobe;	/* HS400ES selected */
>  };
>  
>  struct mci;
> @@ -586,6 +588,8 @@ struct mci_ops {
>  	/* The tuning command opcode value is different for SD and eMMC cards */
>  	int (*execute_tuning)(struct mci_host *, u32);
>  	void (*set_uhs_signaling)(struct mci_host *host, unsigned int timing);
> +	/* Enable enhanced strobe in the controller (HS400ES) */
> +	void (*hs400_enhanced_strobe)(struct mci_host *, struct mci_ios *);
>  };
>  
>  /** host information */

Cheers,
Ahmad

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |

Re: [PATCH 05/10] mci: add HS400 Enhanced Strobe (HS400ES) selection

[Sascha Hauer]

On 2026-05-18 11:54, Ahmad Fatoum wrote:
> Hi,
> 
> On 5/11/26 2:08 PM, Sascha Hauer wrote:
> > HS400ES bypasses HS200 tuning entirely: the eMMC drives a data strobe
> > line which the controller samples on, so the read path doesn't need
> > software-tuned phase. It still ends up in MMC_TIMING_MMC_HS400.
> > 
> > Detection: a card advertises HS400ES via EXT_CSD_STROBE_SUPPORT[184],
> > which sits outside DEVICE_TYPE; record it as a synthetic
> > EXT_CSD_CARD_TYPE_HS400ES bit in mmc_avail_type when the host
> > declares MMC_CAP2_HS400_ES (mmc-hs400-enhanced-strobe in DT) and the
> > card has both STROBE_SUPPORT set and one of the HS400 voltage variants.
> > 
> > Transition (mmc_select_hs400es):
> >   1. Negotiate 8-bit bus in HS mode (HS400 needs 8-bit).
> >   2. CMD6 EXT_CSD_HS_TIMING = HS, host to HS rate.
> >   3. CMD6 EXT_CSD_BUS_WIDTH = 8-bit DDR | STROBE bit, so the card
> >      starts driving the strobe line.
> >   4. CMD6 EXT_CSD_HS_TIMING = HS400, host to HS400 timing at
> >      hs200_max_dtr (200 MHz, DDR-sampled => 400 MT/s).
> >   5. Call host->ops.hs400_enhanced_strobe so the controller enables
> >      enhanced strobe sampling.
> > 
> > mmc_select_timing prefers HS400ES over HS200 + HS400; on any failure
> > it clears the flag and falls back to HS200. mci_startup_mmc returns
> > early when mmc_select_timing already landed us in HS400 (HS400ES has
> > no follow-up tuning step).
> > 
> > mci_ops gains an optional hs400_enhanced_strobe hook for the
> > controller's enhanced-strobe enable.
> > 
> > Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  drivers/mci/mci-core.c | 108 ++++++++++++++++++++++++++++++++++++++++++++++++-
> >  include/mci.h          |   4 ++
> >  2 files changed, 111 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
> > index 58b28ec653..0cfddb8d43 100644
> > --- a/drivers/mci/mci-core.c
> > +++ b/drivers/mci/mci-core.c
> > @@ -1798,6 +1798,16 @@ static void mmc_select_max_dtr(struct mci *mci)
> >  		avail_type |= EXT_CSD_CARD_TYPE_HS400_1_2V;
> >  	}
> >  
> > +	/*
> > +	 * HS400ES capability is reported in EXT_CSD_STROBE_SUPPORT (byte 184),
> > +	 * not in DEVICE_TYPE. Combine with one of the HS400 voltage variants;
> > +	 * which voltage is used follows the same negotiation as HS400.
> > +	 */
> > +	if ((caps2 & MMC_CAP2_HS400_ES) &&
> > +	    mci->ext_csd[EXT_CSD_STROBE_SUPPORT] &&
> 
> Please add to mci_print_caps()

ok.

> 
> Also, please extend mci_timing_tostr() to return "HS400ES" for enhanced
> strobe.

There is no separate mode for HS400ES, it is still MMC_TIMING_MMC_HS400,
that's why it's decoded separately by the caller.

> > +
> > +	/* Step 4: 8-bit DDR with strobe enabled */
> > +	val = EXT_CSD_DDR_BUS_WIDTH_8 | EXT_CSD_BUS_WIDTH_STROBE;
> > +	err = mci_switch(mci, EXT_CSD_BUS_WIDTH, val);
> > +	if (err) {
> > +		dev_err(&mci->dev, "switch to DDR8 with strobe failed: %d\n", err);
> > +		return err;
> > +	}
> > +> +	/* Step 5: switch card to HS400 */
> > +	val = EXT_CSD_TIMING_HS400 | (host->drive_strength << EXT_CSD_DRV_STR_SHIFT);
> 
> The drive_strength is initialized by mmc_select_driver_type() and it
> seems no one will have called it for HS400ES?

Ok, will add it at the same place the kernel has it.

> 
> > +	err = mci_switch(mci, EXT_CSD_HS_TIMING, val);
> > +	if (err) {
> > +		dev_err(&mci->dev, "switch to HS400 for HS400ES failed: %d\n", err);
> > +		return err;
> > +	}
> > +
> > +	/* Step 6: host to HS400 timing and final clock */
> > +	mci_set_timing(mci, MMC_TIMING_MMC_HS400);
> > +	mmc_set_bus_speed(mci);
> > +
> > +	err = mci_switch_status(mci, true);
> > +	if (err)
> > +		return err;
> 
> Should this not be _after_ the enhanced strobe setting, so we are able
> to propagate an error on issues?

Ok.

> > @@ -2698,7 +2803,8 @@ static void mci_info(struct device *dev)
> >  		bw = 1;
> >
> >  	printf("  current buswidth: %d\n", bw);
> > -	printf("  current timing: %s\n", mci_timing_tostr(host->ios.timing));
> > +	printf("  current timing: %s%s\n", mci_timing_tostr(host->ios.timing),
> > +	       host->ios.enhanced_strobe ? " (Enhanced Strobe)" : "");
> 
> I would just append an ES suffix.

Ok.

--
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 06/10] mci: rockchip-dwcmshc-sdhci: use ADMA2

[Sascha Hauer]

The SDMA engine doesn't seem to be fast enough to keep up with HS400
support. In preparation to add HS400 support to the driver switch to
ADMA when available.

Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mci/rockchip-dwcmshc-sdhci.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/mci/rockchip-dwcmshc-sdhci.c b/drivers/mci/rockchip-dwcmshc-sdhci.c
index 04ee528f07..23c887e906 100644
--- a/drivers/mci/rockchip-dwcmshc-sdhci.c
+++ b/drivers/mci/rockchip-dwcmshc-sdhci.c
@@ -361,6 +361,11 @@ static int rk_sdhci_probe(struct device *dev)
 
 	sdhci_setup_host(&host->sdhci);
 
+	ret = sdhci_setup_adma(&host->sdhci);
+	if (ret && ret != -ENOTSUPP)
+		dev_warn(dev, "ADMA setup failed (%pe), falling back to SDMA\n",
+			 ERR_PTR(ret));
+
 	dev->priv = host;
 
 	return mci_register(&host->mci);

-- 
2.47.3

Re: [PATCH 06/10] mci: rockchip-dwcmshc-sdhci: use ADMA2

[Ahmad Fatoum]

Hello Sascha,

On 5/11/26 2:08 PM, Sascha Hauer wrote:
> The SDMA engine doesn't seem to be fast enough to keep up with HS400
> support. In preparation to add HS400 support to the driver switch to
> ADMA when available.
> 
> Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/mci/rockchip-dwcmshc-sdhci.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/mci/rockchip-dwcmshc-sdhci.c b/drivers/mci/rockchip-dwcmshc-sdhci.c
> index 04ee528f07..23c887e906 100644
> --- a/drivers/mci/rockchip-dwcmshc-sdhci.c
> +++ b/drivers/mci/rockchip-dwcmshc-sdhci.c
> @@ -361,6 +361,11 @@ static int rk_sdhci_probe(struct device *dev)
>  
>  	sdhci_setup_host(&host->sdhci);
>  
> +	ret = sdhci_setup_adma(&host->sdhci);
> +	if (ret && ret != -ENOTSUPP)
> +		dev_warn(dev, "ADMA setup failed (%pe), falling back to SDMA\n",
> +			 ERR_PTR(ret));

As transparent fallback is implemented, could this be folded into
sdhci_setup_host, so it's opt-out instead of opt-in?

Opt-out could be setting SDHCI_QUIRK_BROKEN_ADMA.

Cheers,
Ahmad

> +
>  	dev->priv = host;
>  
>  	return mci_register(&host->mci);
> 

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |

Re: [PATCH 06/10] mci: rockchip-dwcmshc-sdhci: use ADMA2

[Sascha Hauer]

On 2026-05-11 14:55, Ahmad Fatoum wrote:
> Hello Sascha,
>
> On 5/11/26 2:08 PM, Sascha Hauer wrote:
> > The SDMA engine doesn't seem to be fast enough to keep up with HS400
> > support. In preparation to add HS400 support to the driver switch to
> > ADMA when available.
> > 
> > Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  drivers/mci/rockchip-dwcmshc-sdhci.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/mci/rockchip-dwcmshc-sdhci.c b/drivers/mci/rockchip-dwcmshc-sdhci.c
> > index 04ee528f07..23c887e906 100644
> > --- a/drivers/mci/rockchip-dwcmshc-sdhci.c
> > +++ b/drivers/mci/rockchip-dwcmshc-sdhci.c
> > @@ -361,6 +361,11 @@ static int rk_sdhci_probe(struct device *dev)
> >  
> >  	sdhci_setup_host(&host->sdhci);
> >  
> > +	ret = sdhci_setup_adma(&host->sdhci);
> > +	if (ret && ret != -ENOTSUPP)
> > +		dev_warn(dev, "ADMA setup failed (%pe), falling back to SDMA\n",
> > +			 ERR_PTR(ret));
> 
> As transparent fallback is implemented, could this be folded into
> sdhci_setup_host, so it's opt-out instead of opt-in?

I don't expect much advantage from ADMA for drivers that do not need it.
The advantage is that ADMA can do scatter gather DMA, but we don't make
use of it in barebox.

I'd rather leave it opt-in for now at least for a few rounds.

Sascha

--
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 06/10] mci: rockchip-dwcmshc-sdhci: use ADMA2

[Ahmad Fatoum]

On 5/11/26 4:01 PM, Sascha Hauer wrote:
> On 2026-05-11 14:55, Ahmad Fatoum wrote:
>> Hello Sascha,
>>
>> On 5/11/26 2:08 PM, Sascha Hauer wrote:
>>> The SDMA engine doesn't seem to be fast enough to keep up with HS400
>>> support. In preparation to add HS400 support to the driver switch to
>>> ADMA when available.
>>>
>>> Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
>>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>>> ---
>>>  drivers/mci/rockchip-dwcmshc-sdhci.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/mci/rockchip-dwcmshc-sdhci.c b/drivers/mci/rockchip-dwcmshc-sdhci.c
>>> index 04ee528f07..23c887e906 100644
>>> --- a/drivers/mci/rockchip-dwcmshc-sdhci.c
>>> +++ b/drivers/mci/rockchip-dwcmshc-sdhci.c
>>> @@ -361,6 +361,11 @@ static int rk_sdhci_probe(struct device *dev)
>>>  
>>>  	sdhci_setup_host(&host->sdhci);
>>>  
>>> +	ret = sdhci_setup_adma(&host->sdhci);
>>> +	if (ret && ret != -ENOTSUPP)
>>> +		dev_warn(dev, "ADMA setup failed (%pe), falling back to SDMA\n",
>>> +			 ERR_PTR(ret));
>>
>> As transparent fallback is implemented, could this be folded into
>> sdhci_setup_host, so it's opt-out instead of opt-in?
> 
> I don't expect much advantage from ADMA for drivers that do not need it.
> The advantage is that ADMA can do scatter gather DMA, but we don't make
> use of it in barebox.
> 
> I'd rather leave it opt-in for now at least for a few rounds.

Fair enough.

> 
> Sascha
> 
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |

[PATCH 07/10] mci: sdhci: rockchip: set TX-path source-select bit in DWCMSHC_EMMC_DLL_TXCLK

[Sascha Hauer]

Linux's sdhci-of-dwcmshc has, since rk3588 support was added in 2022,
unconditionally OR'd DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL
(bit 29) into the DWCMSHC_EMMC_DLL_TXCLK write that follows DLL lock
in the high-speed path. Despite its name, position 29 in this
register controls a TX-path source-select; the constant just happens
to share the SRCSEL shift with the RX path register.

Barebox was never writing this bit. The omission was harmless on
controllers where the RX path's matching write (a few lines earlier
in this function) already sets the same bit position 29 - because on
those parts both clock paths end up with a working source-select.
The bit is needed for the TX path to have a working source-select in
its own right, however; the next commit's revision split, which
correctly stops setting the RX-path bit on rk3588 silicon, would
otherwise leave the TX path with no source-select at all and break
eMMC writes.

Match Linux's behaviour: set the bit unconditionally in the TXCLK
write.

Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mci/rockchip-dwcmshc-sdhci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mci/rockchip-dwcmshc-sdhci.c b/drivers/mci/rockchip-dwcmshc-sdhci.c
index 23c887e906..fa2d9964a5 100644
--- a/drivers/mci/rockchip-dwcmshc-sdhci.c
+++ b/drivers/mci/rockchip-dwcmshc-sdhci.c
@@ -212,6 +212,7 @@ static void rk_sdhci_set_clock(struct rk_sdhci_host *host, unsigned int clock)
 
 	extra = DWCMSHC_EMMC_DLL_DLYENA |
 		DLL_TXCLK_TAPNUM_FROM_SW |
+		DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL |
 		txclk_tapnum;
 	sdhci_write32(&host->sdhci, DWCMSHC_EMMC_DLL_TXCLK, extra);
 

-- 
2.47.3

Re: [PATCH 07/10] mci: sdhci: rockchip: set TX-path source-select bit in DWCMSHC_EMMC_DLL_TXCLK

[Ahmad Fatoum]

On 5/11/26 2:08 PM, Sascha Hauer wrote:
> Linux's sdhci-of-dwcmshc has, since rk3588 support was added in 2022,
> unconditionally OR'd DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL
> (bit 29) into the DWCMSHC_EMMC_DLL_TXCLK write that follows DLL lock
> in the high-speed path. Despite its name, position 29 in this
> register controls a TX-path source-select; the constant just happens
> to share the SRCSEL shift with the RX path register.
> 
> Barebox was never writing this bit. The omission was harmless on
> controllers where the RX path's matching write (a few lines earlier
> in this function) already sets the same bit position 29 - because on
> those parts both clock paths end up with a working source-select.
> The bit is needed for the TX path to have a working source-select in
> its own right, however; the next commit's revision split, which
> correctly stops setting the RX-path bit on rk3588 silicon, would
> otherwise leave the TX path with no source-select at all and break
> eMMC writes.
> 
> Match Linux's behaviour: set the bit unconditionally in the TXCLK
> write.
> 
> Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Acked-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

> ---
>  drivers/mci/rockchip-dwcmshc-sdhci.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mci/rockchip-dwcmshc-sdhci.c b/drivers/mci/rockchip-dwcmshc-sdhci.c
> index 23c887e906..fa2d9964a5 100644
> --- a/drivers/mci/rockchip-dwcmshc-sdhci.c
> +++ b/drivers/mci/rockchip-dwcmshc-sdhci.c
> @@ -212,6 +212,7 @@ static void rk_sdhci_set_clock(struct rk_sdhci_host *host, unsigned int clock)
>  
>  	extra = DWCMSHC_EMMC_DLL_DLYENA |
>  		DLL_TXCLK_TAPNUM_FROM_SW |
> +		DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL |
>  		txclk_tapnum;
>  	sdhci_write32(&host->sdhci, DWCMSHC_EMMC_DLL_TXCLK, extra);
>  
> 

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |

[PATCH 08/10] mci: sdhci: rockchip: distinguish IP revision 0 (rk3568) from 1 (rk3576/rk3588)

[Sascha Hauer]

The dwcmshc IP comes in two revisions with subtly different DLL needs.
Upstream Linux (sdhci-of-dwcmshc.c) tracks this via a per-SoC
revision field; mirror that here so HS400 works robustly on rk3588
and we can add rk3576 with the right behaviour.

Revision 0 (rk3566, rk3568):
  - DLL_RXCLK source-select needs DLL_RXCLK_NO_INVERTER.
  - HS400 uses the default 0x10 TX clock tap and leaves
    DECMSHC_EMMC_DLL_CMDOUT untouched.

Revision 1 (rk3576, rk3588, ...):
  - DLL_RXCLK source-select must be 0 (inverted), not NO_INVERTER.
  - HS400 needs a 90-degree TX clock tap together with a matching
    CMDOUT tap programmed into DECMSHC_EMMC_DLL_CMDOUT (with
    DLL_CMDOUT_SRC_CLK_NEG | DLL_CMDOUT_EN_SRC_CLK_NEG |
    DWCMSHC_EMMC_DLL_DLYENA | DLL_CMDOUT_TAPNUM_90_DEGREES |
    DLL_CMDOUT_TAPNUM_FROM_SW).

Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mci/rockchip-dwcmshc-sdhci.c | 60 +++++++++++++++++++++++++++++++-----
 1 file changed, 52 insertions(+), 8 deletions(-)

diff --git a/drivers/mci/rockchip-dwcmshc-sdhci.c b/drivers/mci/rockchip-dwcmshc-sdhci.c
index fa2d9964a5..4b2ee03e9a 100644
--- a/drivers/mci/rockchip-dwcmshc-sdhci.c
+++ b/drivers/mci/rockchip-dwcmshc-sdhci.c
@@ -78,10 +78,23 @@ enum {
 	CLK_MAX,
 };
 
+struct rk_sdhci_soc_data {
+	u8 revision;
+};
+
+static const struct rk_sdhci_soc_data rk_sdhci_rk3568_data = {
+	.revision = 0,
+};
+
+static const struct rk_sdhci_soc_data rk_sdhci_rk35xx_data = {
+	.revision = 1,
+};
+
 struct rk_sdhci_host {
 	struct mci_host		mci;
 	struct sdhci		sdhci;
 	struct clk_bulk_data	clks[CLK_MAX];
+	const struct rk_sdhci_soc_data *soc;
 };
 
 
@@ -132,9 +145,14 @@ static void rk_sdhci_set_clock(struct rk_sdhci_host *host, unsigned int clock)
 
 	host->mci.ios.clock = 0;
 
-	/* DO NOT TOUCH THIS SETTING */
-	extra = DWCMSHC_EMMC_DLL_DLYENA |
-		DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL;
+	/*
+	 * Revision 0 IPs (rk3568) need DLL_RXCLK_NO_INVERTER; revision 1
+	 * (rk3576, rk3588 and later) must leave the source-select field at
+	 * 0 (inverted).
+	 */
+	extra = DWCMSHC_EMMC_DLL_DLYENA;
+	if (host->soc->revision == 0)
+		extra |= DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL;
 	sdhci_write32(&host->sdhci, DWCMSHC_EMMC_DLL_RXCLK, extra);
 
 	if (clock == 0)
@@ -210,6 +228,23 @@ static void rk_sdhci_set_clock(struct rk_sdhci_host *host, unsigned int clock)
 		0x3 << 19;  /* post-change delay */
 	sdhci_write32(&host->sdhci, DWCMSHC_EMMC_ATCTRL, extra);
 
+	/*
+	 * On revision 1 IPs, HS400 needs a 90-degree TX clock tap together
+	 * with a matching CMDOUT-tap programmed via DECMSHC_EMMC_DLL_CMDOUT.
+	 * Revision 0 keeps the default 0x10 TX tap and leaves CMDOUT alone.
+	 */
+	if (host->soc->revision == 1 &&
+	    host->mci.ios.timing == MMC_TIMING_MMC_HS400) {
+		txclk_tapnum = DLL_TXCLK_TAPNUM_90_DEGREES;
+
+		extra = DLL_CMDOUT_SRC_CLK_NEG |
+			DLL_CMDOUT_EN_SRC_CLK_NEG |
+			DWCMSHC_EMMC_DLL_DLYENA |
+			DLL_CMDOUT_TAPNUM_90_DEGREES |
+			DLL_CMDOUT_TAPNUM_FROM_SW;
+		sdhci_write32(&host->sdhci, DECMSHC_EMMC_DLL_CMDOUT, extra);
+	}
+
 	extra = DWCMSHC_EMMC_DLL_DLYENA |
 		DLL_TXCLK_TAPNUM_FROM_SW |
 		DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL |
@@ -327,6 +362,10 @@ static int rk_sdhci_probe(struct device *dev)
 
 	mci = &host->mci;
 
+	host->soc = device_get_match_data(dev);
+	if (!host->soc)
+		return -ENODEV;
+
 	iores = dev_request_mem_resource(dev, 0);
 	if (IS_ERR(iores))
 		return PTR_ERR(iores);
@@ -374,12 +413,17 @@ static int rk_sdhci_probe(struct device *dev)
 
 static __maybe_unused struct of_device_id rk_sdhci_compatible[] = {
 	{
-		.compatible = "rockchip,rk3562-dwcmshc"
-	},
-	{
-		.compatible = "rockchip,rk3568-dwcmshc"
+		.compatible = "rockchip,rk3562-dwcmshc",
+		.data = &rk_sdhci_rk3568_data,
+	}, {
+		.compatible = "rockchip,rk3568-dwcmshc",
+		.data = &rk_sdhci_rk3568_data,
+	}, {
+		.compatible = "rockchip,rk3576-dwcmshc",
+		.data = &rk_sdhci_rk35xx_data,
 	}, {
-		.compatible = "rockchip,rk3588-dwcmshc"
+		.compatible = "rockchip,rk3588-dwcmshc",
+		.data = &rk_sdhci_rk35xx_data,
 	}, {
 		/* sentinel */
 	}

-- 
2.47.3

Re: [PATCH 08/10] mci: sdhci: rockchip: distinguish IP revision 0 (rk3568) from 1 (rk3576/rk3588)

[Ahmad Fatoum]

On 5/11/26 2:08 PM, Sascha Hauer wrote:
> The dwcmshc IP comes in two revisions with subtly different DLL needs.
> Upstream Linux (sdhci-of-dwcmshc.c) tracks this via a per-SoC
> revision field; mirror that here so HS400 works robustly on rk3588
> and we can add rk3576 with the right behaviour.
> 
> Revision 0 (rk3566, rk3568):
>   - DLL_RXCLK source-select needs DLL_RXCLK_NO_INVERTER.
>   - HS400 uses the default 0x10 TX clock tap and leaves
>     DECMSHC_EMMC_DLL_CMDOUT untouched.
> 
> Revision 1 (rk3576, rk3588, ...):
>   - DLL_RXCLK source-select must be 0 (inverted), not NO_INVERTER.
>   - HS400 needs a 90-degree TX clock tap together with a matching
>     CMDOUT tap programmed into DECMSHC_EMMC_DLL_CMDOUT (with
>     DLL_CMDOUT_SRC_CLK_NEG | DLL_CMDOUT_EN_SRC_CLK_NEG |
>     DWCMSHC_EMMC_DLL_DLYENA | DLL_CMDOUT_TAPNUM_90_DEGREES |
>     DLL_CMDOUT_TAPNUM_FROM_SW).
> 
> Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

> ---
>  drivers/mci/rockchip-dwcmshc-sdhci.c | 60 +++++++++++++++++++++++++++++++-----
>  1 file changed, 52 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mci/rockchip-dwcmshc-sdhci.c b/drivers/mci/rockchip-dwcmshc-sdhci.c
> index fa2d9964a5..4b2ee03e9a 100644
> --- a/drivers/mci/rockchip-dwcmshc-sdhci.c
> +++ b/drivers/mci/rockchip-dwcmshc-sdhci.c
> @@ -78,10 +78,23 @@ enum {
>  	CLK_MAX,
>  };
>  
> +struct rk_sdhci_soc_data {
> +	u8 revision;
> +};
> +
> +static const struct rk_sdhci_soc_data rk_sdhci_rk3568_data = {
> +	.revision = 0,
> +};
> +
> +static const struct rk_sdhci_soc_data rk_sdhci_rk35xx_data = {
> +	.revision = 1,
> +};
> +
>  struct rk_sdhci_host {
>  	struct mci_host		mci;
>  	struct sdhci		sdhci;
>  	struct clk_bulk_data	clks[CLK_MAX];
> +	const struct rk_sdhci_soc_data *soc;
>  };
>  
>  
> @@ -132,9 +145,14 @@ static void rk_sdhci_set_clock(struct rk_sdhci_host *host, unsigned int clock)
>  
>  	host->mci.ios.clock = 0;
>  
> -	/* DO NOT TOUCH THIS SETTING */
> -	extra = DWCMSHC_EMMC_DLL_DLYENA |
> -		DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL;
> +	/*
> +	 * Revision 0 IPs (rk3568) need DLL_RXCLK_NO_INVERTER; revision 1
> +	 * (rk3576, rk3588 and later) must leave the source-select field at
> +	 * 0 (inverted).
> +	 */
> +	extra = DWCMSHC_EMMC_DLL_DLYENA;
> +	if (host->soc->revision == 0)
> +		extra |= DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL;
>  	sdhci_write32(&host->sdhci, DWCMSHC_EMMC_DLL_RXCLK, extra);
>  
>  	if (clock == 0)
> @@ -210,6 +228,23 @@ static void rk_sdhci_set_clock(struct rk_sdhci_host *host, unsigned int clock)
>  		0x3 << 19;  /* post-change delay */
>  	sdhci_write32(&host->sdhci, DWCMSHC_EMMC_ATCTRL, extra);
>  
> +	/*
> +	 * On revision 1 IPs, HS400 needs a 90-degree TX clock tap together
> +	 * with a matching CMDOUT-tap programmed via DECMSHC_EMMC_DLL_CMDOUT.
> +	 * Revision 0 keeps the default 0x10 TX tap and leaves CMDOUT alone.
> +	 */
> +	if (host->soc->revision == 1 &&
> +	    host->mci.ios.timing == MMC_TIMING_MMC_HS400) {
> +		txclk_tapnum = DLL_TXCLK_TAPNUM_90_DEGREES;
> +
> +		extra = DLL_CMDOUT_SRC_CLK_NEG |
> +			DLL_CMDOUT_EN_SRC_CLK_NEG |
> +			DWCMSHC_EMMC_DLL_DLYENA |
> +			DLL_CMDOUT_TAPNUM_90_DEGREES |
> +			DLL_CMDOUT_TAPNUM_FROM_SW;
> +		sdhci_write32(&host->sdhci, DECMSHC_EMMC_DLL_CMDOUT, extra);
> +	}
> +
>  	extra = DWCMSHC_EMMC_DLL_DLYENA |
>  		DLL_TXCLK_TAPNUM_FROM_SW |
>  		DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL |
> @@ -327,6 +362,10 @@ static int rk_sdhci_probe(struct device *dev)
>  
>  	mci = &host->mci;
>  
> +	host->soc = device_get_match_data(dev);
> +	if (!host->soc)
> +		return -ENODEV;
> +
>  	iores = dev_request_mem_resource(dev, 0);
>  	if (IS_ERR(iores))
>  		return PTR_ERR(iores);
> @@ -374,12 +413,17 @@ static int rk_sdhci_probe(struct device *dev)
>  
>  static __maybe_unused struct of_device_id rk_sdhci_compatible[] = {
>  	{
> -		.compatible = "rockchip,rk3562-dwcmshc"
> -	},
> -	{
> -		.compatible = "rockchip,rk3568-dwcmshc"
> +		.compatible = "rockchip,rk3562-dwcmshc",
> +		.data = &rk_sdhci_rk3568_data,
> +	}, {
> +		.compatible = "rockchip,rk3568-dwcmshc",
> +		.data = &rk_sdhci_rk3568_data,
> +	}, {
> +		.compatible = "rockchip,rk3576-dwcmshc",
> +		.data = &rk_sdhci_rk35xx_data,
>  	}, {
> -		.compatible = "rockchip,rk3588-dwcmshc"
> +		.compatible = "rockchip,rk3588-dwcmshc",
> +		.data = &rk_sdhci_rk35xx_data,
>  	}, {
>  		/* sentinel */
>  	}
> 

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |

[PATCH 09/10] mci: sdhci: rockchip: support HS400

[Sascha Hauer]

After sdhci_set_clock() runs sdhci_set_uhs_signaling() (which writes
the standard SDHCI_CTRL_HS400 = 0x5 to HOST_CONTROL2's UHS field) and
the SDCLK is gated for the high-speed DLL config, fix up two things
that the dwcmshc controller needs in HS400:

- HOST_CONTROL2's UHS field needs DWCMSHC_CTRL_HS400 (0x7) instead of
  the standard SDHCI_CTRL_HS400 (0x5).
- EMMC_CONTROL.CARD_IS_EMMC (BIT 0 at offset 0x52c) must be set to
  enable the data-strobe sampling path that HS400 uses.

The high-speed DLL branch (>52 MHz) already programs DLL_TXCLK,
DLL_STRBIN and locks the DLL the same way HS200 does, which is what
HS400 needs as well. Together with the core HS200 -> HS -> HS400
transition, this lets HS400 run on rk3568 / rk3588.

Tested on a K3588 Radxa Rock5T board.

Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mci/rockchip-dwcmshc-sdhci.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/mci/rockchip-dwcmshc-sdhci.c b/drivers/mci/rockchip-dwcmshc-sdhci.c
index 4b2ee03e9a..3aa3f12930 100644
--- a/drivers/mci/rockchip-dwcmshc-sdhci.c
+++ b/drivers/mci/rockchip-dwcmshc-sdhci.c
@@ -18,6 +18,7 @@
 #define DWCMSHC_VER_TYPE		0x504
 #define DWCMSHC_HOST_CTRL3		0x508
 #define DWCMSHC_EMMC_CONTROL		0x52c
+#define  DWCMSHC_CARD_IS_EMMC		BIT(0)
 #define DWCMSHC_EMMC_ATCTRL		0x540
 
 /* Rockchip specific Registers */
@@ -183,6 +184,25 @@ static void rk_sdhci_set_clock(struct rk_sdhci_host *host, unsigned int clock)
 	/* Disable clock while config DLL */
 	sdhci_write16(&host->sdhci, SDHCI_CLOCK_CONTROL, 0);
 
+	/*
+	 * HS400 needs the dwcmshc-specific value (0x7) in HOST_CONTROL2's UHS
+	 * field rather than the standard SDHCI_CTRL_HS400 (0x5) the generic
+	 * sdhci_set_uhs_signaling() wrote. It also requires CARD_IS_EMMC in
+	 * EMMC_CONTROL to enable the data strobe sampling path.
+	 */
+	if (host->mci.ios.timing == MMC_TIMING_MMC_HS400) {
+		u16 ctrl_2;
+
+		ctrl_2 = sdhci_read16(&host->sdhci, SDHCI_HOST_CONTROL2);
+		ctrl_2 &= ~SDHCI_CTRL_UHS_MASK;
+		ctrl_2 |= DWCMSHC_CTRL_HS400;
+		sdhci_write16(&host->sdhci, SDHCI_HOST_CONTROL2, ctrl_2);
+
+		extra = sdhci_read32(&host->sdhci, DWCMSHC_EMMC_CONTROL);
+		extra |= DWCMSHC_CARD_IS_EMMC;
+		sdhci_write32(&host->sdhci, DWCMSHC_EMMC_CONTROL, extra);
+	}
+
 	if (clock <= 52000000) {
 		/*
 		 * Disable DLL and reset both of sample and drive clock.

-- 
2.47.3

Re: [PATCH 09/10] mci: sdhci: rockchip: support HS400

[Ahmad Fatoum]

Hello Sascha,

On 5/11/26 2:08 PM, Sascha Hauer wrote:
> After sdhci_set_clock() runs sdhci_set_uhs_signaling() (which writes
> the standard SDHCI_CTRL_HS400 = 0x5 to HOST_CONTROL2's UHS field) and
> the SDCLK is gated for the high-speed DLL config, fix up two things
> that the dwcmshc controller needs in HS400:
> 
> - HOST_CONTROL2's UHS field needs DWCMSHC_CTRL_HS400 (0x7) instead of
>   the standard SDHCI_CTRL_HS400 (0x5).
> - EMMC_CONTROL.CARD_IS_EMMC (BIT 0 at offset 0x52c) must be set to
>   enable the data-strobe sampling path that HS400 uses.
> 
> The high-speed DLL branch (>52 MHz) already programs DLL_TXCLK,
> DLL_STRBIN and locks the DLL the same way HS200 does, which is what
> HS400 needs as well. Together with the core HS200 -> HS -> HS400
> transition, this lets HS400 run on rk3568 / rk3588.
> 
> Tested on a K3588 Radxa Rock5T board.
> 
> Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

But see comment below

> ---
>  drivers/mci/rockchip-dwcmshc-sdhci.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/mci/rockchip-dwcmshc-sdhci.c b/drivers/mci/rockchip-dwcmshc-sdhci.c
> index 4b2ee03e9a..3aa3f12930 100644
> --- a/drivers/mci/rockchip-dwcmshc-sdhci.c
> +++ b/drivers/mci/rockchip-dwcmshc-sdhci.c
> @@ -18,6 +18,7 @@
>  #define DWCMSHC_VER_TYPE		0x504
>  #define DWCMSHC_HOST_CTRL3		0x508
>  #define DWCMSHC_EMMC_CONTROL		0x52c
> +#define  DWCMSHC_CARD_IS_EMMC		BIT(0)
>  #define DWCMSHC_EMMC_ATCTRL		0x540
>  
>  /* Rockchip specific Registers */
> @@ -183,6 +184,25 @@ static void rk_sdhci_set_clock(struct rk_sdhci_host *host, unsigned int clock)
>  	/* Disable clock while config DLL */
>  	sdhci_write16(&host->sdhci, SDHCI_CLOCK_CONTROL, 0);
>  
> +	/*
> +	 * HS400 needs the dwcmshc-specific value (0x7) in HOST_CONTROL2's UHS
> +	 * field rather than the standard SDHCI_CTRL_HS400 (0x5) the generic
> +	 * sdhci_set_uhs_signaling() wrote. It also requires CARD_IS_EMMC in
> +	 * EMMC_CONTROL to enable the data strobe sampling path.
> +	 */
> +	if (host->mci.ios.timing == MMC_TIMING_MMC_HS400) {
> +		u16 ctrl_2;
> +
> +		ctrl_2 = sdhci_read16(&host->sdhci, SDHCI_HOST_CONTROL2);
> +		ctrl_2 &= ~SDHCI_CTRL_UHS_MASK;
> +		ctrl_2 |= DWCMSHC_CTRL_HS400;
> +		sdhci_write16(&host->sdhci, SDHCI_HOST_CONTROL2, ctrl_2);
> +
> +		extra = sdhci_read32(&host->sdhci, DWCMSHC_EMMC_CONTROL);
> +		extra |= DWCMSHC_CARD_IS_EMMC;
> +		sdhci_write32(&host->sdhci, DWCMSHC_EMMC_CONTROL, extra);

Linux does 16-bit write here (register is 4-byte aligned and bit 0 is
written). Should we do the same in barebox?

Cheers,
Ahmad

> +	}
> +
>  	if (clock <= 52000000) {
>  		/*
>  		 * Disable DLL and reset both of sample and drive clock.
> 

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |

[PATCH 10/10] mci: sdhci: rockchip: support HS400 Enhanced Strobe

[Sascha Hauer]

Implement the hs400_enhanced_strobe op so the dwcmshc controller
samples on the eMMC strobe instead of the source clock when in
HS400ES. The bit lives at EMMC_CONTROL[8] (DWCMSHC_ENHANCED_STROBE)
in the same vendor area register that already holds CARD_IS_EMMC.

Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mci/rockchip-dwcmshc-sdhci.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/mci/rockchip-dwcmshc-sdhci.c b/drivers/mci/rockchip-dwcmshc-sdhci.c
index 3aa3f12930..89bc4781b4 100644
--- a/drivers/mci/rockchip-dwcmshc-sdhci.c
+++ b/drivers/mci/rockchip-dwcmshc-sdhci.c
@@ -19,6 +19,7 @@
 #define DWCMSHC_HOST_CTRL3		0x508
 #define DWCMSHC_EMMC_CONTROL		0x52c
 #define  DWCMSHC_CARD_IS_EMMC		BIT(0)
+#define  DWCMSHC_ENHANCED_STROBE	BIT(8)
 #define DWCMSHC_EMMC_ATCTRL		0x540
 
 /* Rockchip specific Registers */
@@ -363,12 +364,24 @@ static int rk_sdhci_execute_tuning(struct mci_host *mci, u32 opcode)
 	return sdhci_execute_tuning(&host->sdhci, opcode);
 }
 
+static void rk_sdhci_hs400_enhanced_strobe(struct mci_host *mci,
+					   struct mci_ios *ios)
+{
+	struct rk_sdhci_host *host = to_rk_sdhci_host(mci);
+	u32 val;
+
+	val = sdhci_read32(&host->sdhci, DWCMSHC_EMMC_CONTROL);
+	val |= DWCMSHC_ENHANCED_STROBE;
+	sdhci_write32(&host->sdhci, DWCMSHC_EMMC_CONTROL, val);
+}
+
 static const struct mci_ops rk_sdhci_ops = {
 	.send_cmd = rk_sdhci_send_cmd,
 	.set_ios = rk_sdhci_set_ios,
 	.init = rk_sdhci_init,
 	.card_present = rk_sdhci_card_present,
 	.execute_tuning = rk_sdhci_execute_tuning,
+	.hs400_enhanced_strobe = rk_sdhci_hs400_enhanced_strobe,
 };
 
 static int rk_sdhci_probe(struct device *dev)

-- 
2.47.3

Re: [PATCH 10/10] mci: sdhci: rockchip: support HS400 Enhanced Strobe

[Ahmad Fatoum]

Hello Sascha,

On 5/11/26 2:08 PM, Sascha Hauer wrote:
> Implement the hs400_enhanced_strobe op so the dwcmshc controller
> samples on the eMMC strobe instead of the source clock when in
> HS400ES. The bit lives at EMMC_CONTROL[8] (DWCMSHC_ENHANCED_STROBE)
> in the same vendor area register that already holds CARD_IS_EMMC.
> 
> Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/mci/rockchip-dwcmshc-sdhci.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/mci/rockchip-dwcmshc-sdhci.c b/drivers/mci/rockchip-dwcmshc-sdhci.c
> index 3aa3f12930..89bc4781b4 100644
> --- a/drivers/mci/rockchip-dwcmshc-sdhci.c
> +++ b/drivers/mci/rockchip-dwcmshc-sdhci.c
> @@ -19,6 +19,7 @@
>  #define DWCMSHC_HOST_CTRL3		0x508
>  #define DWCMSHC_EMMC_CONTROL		0x52c
>  #define  DWCMSHC_CARD_IS_EMMC		BIT(0)
> +#define  DWCMSHC_ENHANCED_STROBE	BIT(8)
>  #define DWCMSHC_EMMC_ATCTRL		0x540
>  
>  /* Rockchip specific Registers */
> @@ -363,12 +364,24 @@ static int rk_sdhci_execute_tuning(struct mci_host *mci, u32 opcode)
>  	return sdhci_execute_tuning(&host->sdhci, opcode);
>  }
>  
> +static void rk_sdhci_hs400_enhanced_strobe(struct mci_host *mci,
> +					   struct mci_ios *ios)
> +{
> +	struct rk_sdhci_host *host = to_rk_sdhci_host(mci);
> +	u32 val;
> +
> +	val = sdhci_read32(&host->sdhci, DWCMSHC_EMMC_CONTROL);
> +	val |= DWCMSHC_ENHANCED_STROBE;
> +	sdhci_write32(&host->sdhci, DWCMSHC_EMMC_CONTROL, val);

Linux checks ios->enhanced strobe and disables/enables the strobe based
on that. I think that's useful for barebox as well, so the core has a
chance to implement fallback.

Cheers,
Ahmad

> +}
> +
>  static const struct mci_ops rk_sdhci_ops = {
>  	.send_cmd = rk_sdhci_send_cmd,
>  	.set_ios = rk_sdhci_set_ios,
>  	.init = rk_sdhci_init,
>  	.card_present = rk_sdhci_card_present,
>  	.execute_tuning = rk_sdhci_execute_tuning,
> +	.hs400_enhanced_strobe = rk_sdhci_hs400_enhanced_strobe,
>  };
>  
>  static int rk_sdhci_probe(struct device *dev)
> 

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |