[PATCH 1/2] mci: add eMMC DDR52 support

[PATCH 1/2] mci: add eMMC DDR52 support

[Ahmad Fatoum]

The maximum card frequency that can be configured by barebox currently
is 50MHz for SD and 52MHz for eMMC. Higher speed modes require runtime
voltage switching or tuning sequences, which are not yet implemented.

Only exception is eMMC's DDR52: This mode was first introduced with
MMC 4.4 and can be used even at 3.3V.

This commit adds DDR52 support to the core. This introduces no functional
change, because host controllers must opt-in by setting the appropriate
host capabilities.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/mci/mci-core.c | 54 +++++++++++++++++++++++++++++++++++-------
 include/mci.h          | 19 +++++++++++++++
 2 files changed, 64 insertions(+), 9 deletions(-)

diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
index f647cae8203b..86f468edfea6 100644
--- a/drivers/mci/mci-core.c
+++ b/drivers/mci/mci-core.c
@@ -135,6 +135,9 @@ static int mci_set_blocklen(struct mci *mci, unsigned len)
 {
 	struct mci_cmd cmd;
 
+	if (mci->host->timing == MMC_TIMING_MMC_DDR52)
+		return 0;
+
 	mci_setup_cmd(&cmd, MMC_CMD_SET_BLOCKLEN, len, MMC_RSP_R1);
 	return mci_send_cmd(mci, &cmd, NULL);
 }
@@ -649,11 +652,15 @@ static int mmc_change_freq(struct mci *mci)
 		return 0;
 	}
 
-	/* High Speed is set, there are two types: 52MHz and 26MHz */
-	if (cardtype & EXT_CSD_CARD_TYPE_52)
-		mci->card_caps |= MMC_CAP_MMC_HIGHSPEED_52MHZ | MMC_CAP_MMC_HIGHSPEED;
-	else
-		mci->card_caps |= MMC_CAP_MMC_HIGHSPEED;
+	mci->card_caps |= MMC_CAP_MMC_HIGHSPEED;
+
+	/* High Speed is set, there are three types: 26MHz, 52MHz, 52MHz DDR */
+	if (cardtype & EXT_CSD_CARD_TYPE_52) {
+		mci->card_caps |= MMC_CAP_MMC_HIGHSPEED_52MHZ;
+
+		if (cardtype & EXT_CSD_CARD_TYPE_DDR_1_8V)
+			mci->card_caps |= MMC_CAP_MMC_3_3V_DDR | MMC_CAP_MMC_1_8V_DDR;
+	}
 
 	if (IS_ENABLED(CONFIG_MCI_MMC_BOOT_PARTITIONS) &&
 			mci->ext_csd[EXT_CSD_REV] >= 3 && mci->ext_csd[EXT_CSD_BOOT_SIZE_MULT]) {
@@ -1170,15 +1177,20 @@ static int mci_startup_sd(struct mci *mci)
 static int mci_startup_mmc(struct mci *mci)
 {
 	struct mci_host *host = mci->host;
+	enum mci_timing timing_orig;
 	int err;
 	int idx = 0;
 	static unsigned ext_csd_bits[] = {
 		EXT_CSD_BUS_WIDTH_4,
 		EXT_CSD_BUS_WIDTH_8,
+		EXT_CSD_DDR_BUS_WIDTH_4,
+		EXT_CSD_DDR_BUS_WIDTH_8,
 	};
 	static unsigned bus_widths[] = {
 		MMC_BUS_WIDTH_4,
 		MMC_BUS_WIDTH_8,
+		MMC_BUS_WIDTH_4,
+		MMC_BUS_WIDTH_8,
 	};
 
 	/* if possible, speed up the transfer */
@@ -1191,6 +1203,8 @@ static int mci_startup_mmc(struct mci *mci)
 		host->timing = MMC_TIMING_MMC_HS;
 	}
 
+	timing_orig = host->timing;
+
 	mci_set_clock(mci, mci->tran_speed);
 
 	if (!(host->host_caps & (MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA)))
@@ -1205,6 +1219,9 @@ static int mci_startup_mmc(struct mci *mci)
 	if (host->host_caps & MMC_CAP_8_BIT_DATA)
 		idx = 1;
 
+	if (mci_caps(mci) & MMC_CAP_MMC_1_8V_DDR)
+		idx += 2;
+
 	for (; idx >= 0; idx--) {
 
 		/*
@@ -1221,11 +1238,25 @@ static int mci_startup_mmc(struct mci *mci)
 			continue;
 		}
 
+		if (ext_csd_bits[idx] & EXT_CSD_DDR_FLAG)
+			host->timing = MMC_TIMING_MMC_DDR52;
+		else
+			host->timing = timing_orig;
+
+		dev_dbg(&mci->dev, "attempting buswidth %u%s\n", bus_widths[idx],
+			mci_timing_is_ddr(host->timing) ? " (DDR)" : "");
+
 		mci_set_bus_width(mci, bus_widths[idx]);
 
 		err = mmc_compare_ext_csds(mci, bus_widths[idx]);
-		if (!err)
-			break;
+		if (!err) {
+			if (host->timing == MMC_TIMING_MMC_DDR52) {
+				mci->read_bl_len = SECTOR_SIZE;
+				mci->write_bl_len = SECTOR_SIZE;
+			}
+
+			return 0;
+		}
 	}
 
 	return err;
@@ -1654,6 +1685,8 @@ static const char *mci_timing_tostr(unsigned timing)
 		return "MMC HS";
 	case MMC_TIMING_SD_HS:
 		return "SD HS";
+	case MMC_TIMING_MMC_DDR52:
+		return "MMC DDR52";
 	default:
 		return "unknown"; /* shouldn't happen */
 	}
@@ -1661,12 +1694,15 @@ static const char *mci_timing_tostr(unsigned timing)
 
 static void mci_print_caps(unsigned caps)
 {
-	printf("  capabilities: %s%s%s%s%s\n",
+	printf("  capabilities: %s%s%s%s%s%s%s%s\n",
 		caps & MMC_CAP_4_BIT_DATA ? "4bit " : "",
 		caps & MMC_CAP_8_BIT_DATA ? "8bit " : "",
 		caps & MMC_CAP_SD_HIGHSPEED ? "sd-hs " : "",
 		caps & MMC_CAP_MMC_HIGHSPEED ? "mmc-hs " : "",
-		caps & MMC_CAP_MMC_HIGHSPEED_52MHZ ? "mmc-52MHz " : "");
+		caps & MMC_CAP_MMC_HIGHSPEED_52MHZ ? "mmc-52MHz " : "",
+		caps & MMC_CAP_MMC_3_3V_DDR ? "ddr-3.3v " : "",
+		caps & MMC_CAP_MMC_1_8V_DDR ? "ddr-1.8v " : "",
+		caps & MMC_CAP_MMC_1_2V_DDR ? "ddr-1.2v " : "");
 }
 
 /**
diff --git a/include/mci.h b/include/mci.h
index d356f071f7f2..88712c35492e 100644
--- a/include/mci.h
+++ b/include/mci.h
@@ -51,6 +51,11 @@
 #define MMC_CAP_SD_HIGHSPEED		(1 << 3)
 #define MMC_CAP_MMC_HIGHSPEED		(1 << 4)
 #define MMC_CAP_MMC_HIGHSPEED_52MHZ	(1 << 5)
+#define MMC_CAP_MMC_3_3V_DDR		(1 << 7)	/* Host supports eMMC DDR 3.3V */
+#define MMC_CAP_MMC_1_8V_DDR		(1 << 8)	/* Host supports eMMC DDR 1.8V */
+#define MMC_CAP_MMC_1_2V_DDR		(1 << 9)	/* Host supports eMMC DDR 1.2V */
+#define MMC_CAP_DDR			(MMC_CAP_3_3V_DDR | MMC_CAP_1_8V_DDR | \
+					 MMC_CAP_1_2V_DDR)
 /* Mask of all caps for bus width */
 #define MMC_CAP_BIT_DATA_MASK		(MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA)
 
@@ -308,6 +313,7 @@
 #define EXT_CSD_BUS_WIDTH_8	2	/* Card is in 8 bit mode */
 #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 R1_ILLEGAL_COMMAND		(1 << 22)
 #define R1_STATUS(x)			(x & 0xFFF9A000)
@@ -410,6 +416,19 @@ enum mci_timing {
 	MMC_TIMING_MMC_HS400	= 8,
 };
 
+static inline bool mci_timing_is_ddr(enum mci_timing timing)
+{
+	switch (timing) {
+	case MMC_TIMING_UHS_DDR50:
+	case MMC_TIMING_MMC_HS200:
+	case MMC_TIMING_MMC_DDR52:
+	case MMC_TIMING_MMC_HS400:
+		return true;
+	default:
+		return false;
+	}
+}
+
 struct mci_ios {
 	unsigned int	clock;			/* clock rate */
 
-- 
2.39.2

[PATCH 2/2] mci: imx-esdhc: add uSDHC eMMC DDR52 supprot

[Ahmad Fatoum]

The uSDHC available with the i.MX6/7/8 SoCs has support for DDR
clock operation. This is enabled by flipping a bit in
IMX_SDHCI_MIXCTRL and adjusting the clock divider calculation to
account for the automatic internal halving of the clock.

Let's do that to speed up boot from eMMC. How much effect this has
in practice is not constant. Comparing two Kingston eMMCs: DDR on the
older v4.5 connected to an i.MX6 did not yield any difference. On the
newer v5.1 one connected to an i.MX8MM, I observe a 70% improvement in
throughput: from 40MiB/s to 69.5 MiB/s.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/mci/imx-esdhc-common.c |  1 +
 drivers/mci/imx-esdhc.c        | 45 +++++++++++++++++++++++++++++-----
 drivers/mci/imx-esdhc.h        | 14 +++++++++++
 3 files changed, 54 insertions(+), 6 deletions(-)

diff --git a/drivers/mci/imx-esdhc-common.c b/drivers/mci/imx-esdhc-common.c
index 27293e44b724..3c1ff9882432 100644
--- a/drivers/mci/imx-esdhc-common.c
+++ b/drivers/mci/imx-esdhc-common.c
@@ -160,6 +160,7 @@ int __esdhc_send_cmd(struct fsl_esdhc_host *host, struct mci_cmd *cmd,
 		mixctrl = xfertyp;
 		/* Keep the bits 22-25 of the register as is */
 		mixctrl |= (sdhci_read32(&host->sdhci, IMX_SDHCI_MIXCTRL) & (0xF << 22));
+		mixctrl |= mci_timing_is_ddr(host->sdhci.timing) ? MIX_CTRL_DDREN : 0;
 		sdhci_write32(&host->sdhci, IMX_SDHCI_MIXCTRL, mixctrl);
 	}
 
diff --git a/drivers/mci/imx-esdhc.c b/drivers/mci/imx-esdhc.c
index 2e2bc14ef95c..94f18fe2b3cf 100644
--- a/drivers/mci/imx-esdhc.c
+++ b/drivers/mci/imx-esdhc.c
@@ -43,9 +43,9 @@ esdhc_send_cmd(struct mci_host *mci, struct mci_cmd *cmd, struct mci_data *data)
 	return  __esdhc_send_cmd(host, cmd, data);
 }
 
-static void set_sysctl(struct mci_host *mci, u32 clock)
+static void set_sysctl(struct mci_host *mci, u32 clock, bool ddr)
 {
-	int div, pre_div;
+	int div, pre_div, ddr_pre_div = ddr ? 2 : 1;
 	struct fsl_esdhc_host *host = to_fsl_esdhc(mci);
 	int sdhc_clk = clk_get_rate(host->clk);
 	u32 clk;
@@ -65,11 +65,11 @@ static void set_sysctl(struct mci_host *mci, u32 clock)
 		pre_div = 1;
 	else if (sdhc_clk / 16 > clock)
 		for (; pre_div < 256; pre_div *= 2)
-			if ((sdhc_clk / pre_div) <= (clock * 16))
+			if ((sdhc_clk / (pre_div * ddr_pre_div)) <= (clock * 16))
 				break;
 
 	for (div = 1; div <= 16; div++)
-		if ((sdhc_clk / (div * pre_div)) <= clock)
+		if ((sdhc_clk / (div * pre_div * ddr_pre_div)) <= clock)
 			break;
 
 	cur_clock = sdhc_clk / pre_div / div;
@@ -103,12 +103,42 @@ static void set_sysctl(struct mci_host *mci, u32 clock)
 		   10 * MSECOND);
 }
 
+static void esdhc_set_timing(struct fsl_esdhc_host *host, enum mci_timing timing)
+{
+	u32 mixctrl;
+
+	mixctrl = sdhci_read32(&host->sdhci, IMX_SDHCI_MIXCTRL);
+	mixctrl &= ~MIX_CTRL_DDREN;
+
+	switch (timing) {
+	case MMC_TIMING_UHS_DDR50:
+	case MMC_TIMING_MMC_DDR52:
+		mixctrl |= MIX_CTRL_DDREN;
+		sdhci_write32(&host->sdhci, IMX_SDHCI_MIXCTRL, mixctrl);
+		break;
+	default:
+		sdhci_write32(&host->sdhci, IMX_SDHCI_MIXCTRL, mixctrl);
+	}
+
+	host->sdhci.timing = timing;
+}
+
 static void esdhc_set_ios(struct mci_host *mci, struct mci_ios *ios)
 {
 	struct fsl_esdhc_host *host = to_fsl_esdhc(mci);
 
+	/*
+	 * call esdhc_set_timing() before update the clock rate,
+	 * This is because current we support DDR and SDR timing,
+	 * Once the DDR_EN bit is set, the card clock will be
+	 * divide by 2 automatically. So need to do this before
+	 * setting clock rate.
+	 */
+	if (esdhc_is_usdhc(host) && host->sdhci.timing != ios->timing)
+		esdhc_set_timing(host, ios->timing);
+
 	/* Set the clock speed */
-	set_sysctl(mci, ios->clock);
+	set_sysctl(mci, ios->clock, mci_timing_is_ddr(ios->timing));
 
 	/* Set the bus width */
 	esdhc_clrbits32(host, SDHCI_HOST_CONTROL__POWER_CONTROL__BLOCK_GAP_CONTROL,
@@ -206,7 +236,7 @@ static int esdhc_init(struct mci_host *mci, struct device *dev)
 		esdhc_setbits32(host, ESDHC_DMA_SYSCTL, ESDHC_SYSCTL_DMA_SNOOP);
 
 	/* Set the initial clock speed */
-	set_sysctl(mci, 400000);
+	set_sysctl(mci, 400000, false);
 
 	sdhci_write32(&host->sdhci, SDHCI_INT_ENABLE, SDHCI_INT_CMD_COMPLETE |
 			SDHCI_INT_XFER_COMPLETE | SDHCI_INT_CARD_INT |
@@ -285,6 +315,9 @@ static int fsl_esdhc_probe(struct device *dev)
 	if (ret)
 		goto err_clk_disable;
 
+	if (esdhc_is_usdhc(host))
+		mci->host_caps |= MMC_CAP_MMC_3_3V_DDR | MMC_CAP_MMC_1_8V_DDR;
+
 	rate = clk_get_rate(host->clk);
 	host->mci.f_min = rate >> 12;
 	if (host->mci.f_min < 200000)
diff --git a/drivers/mci/imx-esdhc.h b/drivers/mci/imx-esdhc.h
index 0d8f157a7615..b14039757a7a 100644
--- a/drivers/mci/imx-esdhc.h
+++ b/drivers/mci/imx-esdhc.h
@@ -41,6 +41,20 @@
 
 #define IMX_SDHCI_WML		0x44
 #define IMX_SDHCI_MIXCTRL	0x48
+/* Imported from Linux Kernel drivers/mmc/host/sdhci-esdhc-imx.c */
+#define  MIX_CTRL_DDREN		BIT(3)
+#define  MIX_CTRL_DTDSEL_READ	BIT(4)
+#define  MIX_CTRL_AC23EN	BIT(7)
+#define  MIX_CTRL_EXE_TUNE	BIT(22)
+#define  MIX_CTRL_SMPCLK_SEL	BIT(23)
+#define  MIX_CTRL_AUTO_TUNE_EN	BIT(24)
+#define  MIX_CTRL_FBCLK_SEL	BIT(25)
+#define  MIX_CTRL_HS400_EN	BIT(26)
+#define  MIX_CTRL_HS400_ES	BIT(27)
+/* Bits 3 and 6 are not SDHCI standard definitions */
+#define  MIX_CTRL_SDHCI_MASK	0xb7
+/* Tuning bits */
+#define  MIX_CTRL_TUNING_MASK	0x03c00000
 #define IMX_SDHCI_DLL_CTRL	0x60
 #define IMX_SDHCI_MIX_CTRL_FBCLK_SEL	BIT(25)
 
-- 
2.39.2

Re: [PATCH 1/2] mci: add eMMC DDR52 support

[Marco Felsch]

Hi Ahmad,

On 23-04-17, Ahmad Fatoum wrote:
> The maximum card frequency that can be configured by barebox currently
> is 50MHz for SD and 52MHz for eMMC. Higher speed modes require runtime
> voltage switching or tuning sequences, which are not yet implemented.
> 
> Only exception is eMMC's DDR52: This mode was first introduced with
> MMC 4.4 and can be used even at 3.3V.

Nice :)

> This commit adds DDR52 support to the core. This introduces no functional
> change, because host controllers must opt-in by setting the appropriate
> host capabilities.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  drivers/mci/mci-core.c | 54 +++++++++++++++++++++++++++++++++++-------
>  include/mci.h          | 19 +++++++++++++++
>  2 files changed, 64 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
> index f647cae8203b..86f468edfea6 100644
> --- a/drivers/mci/mci-core.c
> +++ b/drivers/mci/mci-core.c
> @@ -135,6 +135,9 @@ static int mci_set_blocklen(struct mci *mci, unsigned len)
>  {
>  	struct mci_cmd cmd;
>  
> +	if (mci->host->timing == MMC_TIMING_MMC_DDR52)
> +		return 0;
> +
>  	mci_setup_cmd(&cmd, MMC_CMD_SET_BLOCKLEN, len, MMC_RSP_R1);
>  	return mci_send_cmd(mci, &cmd, NULL);
>  }
> @@ -649,11 +652,15 @@ static int mmc_change_freq(struct mci *mci)
>  		return 0;
>  	}
>  
> -	/* High Speed is set, there are two types: 52MHz and 26MHz */
> -	if (cardtype & EXT_CSD_CARD_TYPE_52)
> -		mci->card_caps |= MMC_CAP_MMC_HIGHSPEED_52MHZ | MMC_CAP_MMC_HIGHSPEED;
> -	else
> -		mci->card_caps |= MMC_CAP_MMC_HIGHSPEED;
> +	mci->card_caps |= MMC_CAP_MMC_HIGHSPEED;
> +
> +	/* High Speed is set, there are three types: 26MHz, 52MHz, 52MHz DDR */
> +	if (cardtype & EXT_CSD_CARD_TYPE_52) {
> +		mci->card_caps |= MMC_CAP_MMC_HIGHSPEED_52MHZ;
> +
> +		if (cardtype & EXT_CSD_CARD_TYPE_DDR_1_8V)
> +			mci->card_caps |= MMC_CAP_MMC_3_3V_DDR | MMC_CAP_MMC_1_8V_DDR;
> +	}
>  
>  	if (IS_ENABLED(CONFIG_MCI_MMC_BOOT_PARTITIONS) &&
>  			mci->ext_csd[EXT_CSD_REV] >= 3 && mci->ext_csd[EXT_CSD_BOOT_SIZE_MULT]) {
> @@ -1170,15 +1177,20 @@ static int mci_startup_sd(struct mci *mci)
>  static int mci_startup_mmc(struct mci *mci)
>  {
>  	struct mci_host *host = mci->host;
> +	enum mci_timing timing_orig;
>  	int err;
>  	int idx = 0;
>  	static unsigned ext_csd_bits[] = {
>  		EXT_CSD_BUS_WIDTH_4,
>  		EXT_CSD_BUS_WIDTH_8,
> +		EXT_CSD_DDR_BUS_WIDTH_4,
> +		EXT_CSD_DDR_BUS_WIDTH_8,
>  	};
>  	static unsigned bus_widths[] = {
>  		MMC_BUS_WIDTH_4,
>  		MMC_BUS_WIDTH_8,
> +		MMC_BUS_WIDTH_4,
> +		MMC_BUS_WIDTH_8,

This is duplicated or should it be MMC_DDR_BUS_WIDTH_4/8?

>  	};
>  
>  	/* if possible, speed up the transfer */
> @@ -1191,6 +1203,8 @@ static int mci_startup_mmc(struct mci *mci)
>  		host->timing = MMC_TIMING_MMC_HS;
>  	}
>  
> +	timing_orig = host->timing;
> +
>  	mci_set_clock(mci, mci->tran_speed);
>  
>  	if (!(host->host_caps & (MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA)))
> @@ -1205,6 +1219,9 @@ static int mci_startup_mmc(struct mci *mci)
>  	if (host->host_caps & MMC_CAP_8_BIT_DATA)
>  		idx = 1;
>  
> +	if (mci_caps(mci) & MMC_CAP_MMC_1_8V_DDR)
> +		idx += 2;
> +
>  	for (; idx >= 0; idx--) {
>  
>  		/*
> @@ -1221,11 +1238,25 @@ static int mci_startup_mmc(struct mci *mci)
>  			continue;
>  		}
>  
> +		if (ext_csd_bits[idx] & EXT_CSD_DDR_FLAG)
> +			host->timing = MMC_TIMING_MMC_DDR52;
> +		else
> +			host->timing = timing_orig;
> +
> +		dev_dbg(&mci->dev, "attempting buswidth %u%s\n", bus_widths[idx],
> +			mci_timing_is_ddr(host->timing) ? " (DDR)" : "");
> +
>  		mci_set_bus_width(mci, bus_widths[idx]);
>  
>  		err = mmc_compare_ext_csds(mci, bus_widths[idx]);
> -		if (!err)
> -			break;
> +		if (!err) {
> +			if (host->timing == MMC_TIMING_MMC_DDR52) {
> +				mci->read_bl_len = SECTOR_SIZE;
> +				mci->write_bl_len = SECTOR_SIZE;
> +			}
> +
> +			return 0;
> +		}
>  	}
>  
>  	return err;
> @@ -1654,6 +1685,8 @@ static const char *mci_timing_tostr(unsigned timing)
>  		return "MMC HS";
>  	case MMC_TIMING_SD_HS:
>  		return "SD HS";
> +	case MMC_TIMING_MMC_DDR52:
> +		return "MMC DDR52";
>  	default:
>  		return "unknown"; /* shouldn't happen */
>  	}
> @@ -1661,12 +1694,15 @@ static const char *mci_timing_tostr(unsigned timing)
>  
>  static void mci_print_caps(unsigned caps)
>  {
> -	printf("  capabilities: %s%s%s%s%s\n",
> +	printf("  capabilities: %s%s%s%s%s%s%s%s\n",
>  		caps & MMC_CAP_4_BIT_DATA ? "4bit " : "",
>  		caps & MMC_CAP_8_BIT_DATA ? "8bit " : "",
>  		caps & MMC_CAP_SD_HIGHSPEED ? "sd-hs " : "",
>  		caps & MMC_CAP_MMC_HIGHSPEED ? "mmc-hs " : "",
> -		caps & MMC_CAP_MMC_HIGHSPEED_52MHZ ? "mmc-52MHz " : "");
> +		caps & MMC_CAP_MMC_HIGHSPEED_52MHZ ? "mmc-52MHz " : "",
> +		caps & MMC_CAP_MMC_3_3V_DDR ? "ddr-3.3v " : "",
> +		caps & MMC_CAP_MMC_1_8V_DDR ? "ddr-1.8v " : "",
> +		caps & MMC_CAP_MMC_1_2V_DDR ? "ddr-1.2v " : "");

At the moment we only report what barebox does support, ddr-1.8v and
ddr-1.2v isn't supported. Do we really want to report this?

Regards,
  Marco

>  }
>  
>  /**
> diff --git a/include/mci.h b/include/mci.h
> index d356f071f7f2..88712c35492e 100644
> --- a/include/mci.h
> +++ b/include/mci.h
> @@ -51,6 +51,11 @@
>  #define MMC_CAP_SD_HIGHSPEED		(1 << 3)
>  #define MMC_CAP_MMC_HIGHSPEED		(1 << 4)
>  #define MMC_CAP_MMC_HIGHSPEED_52MHZ	(1 << 5)
> +#define MMC_CAP_MMC_3_3V_DDR		(1 << 7)	/* Host supports eMMC DDR 3.3V */
> +#define MMC_CAP_MMC_1_8V_DDR		(1 << 8)	/* Host supports eMMC DDR 1.8V */
> +#define MMC_CAP_MMC_1_2V_DDR		(1 << 9)	/* Host supports eMMC DDR 1.2V */
> +#define MMC_CAP_DDR			(MMC_CAP_3_3V_DDR | MMC_CAP_1_8V_DDR | \
> +					 MMC_CAP_1_2V_DDR)
>  /* Mask of all caps for bus width */
>  #define MMC_CAP_BIT_DATA_MASK		(MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA)
>  
> @@ -308,6 +313,7 @@
>  #define EXT_CSD_BUS_WIDTH_8	2	/* Card is in 8 bit mode */
>  #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 R1_ILLEGAL_COMMAND		(1 << 22)
>  #define R1_STATUS(x)			(x & 0xFFF9A000)
> @@ -410,6 +416,19 @@ enum mci_timing {
>  	MMC_TIMING_MMC_HS400	= 8,
>  };
>  
> +static inline bool mci_timing_is_ddr(enum mci_timing timing)
> +{
> +	switch (timing) {
> +	case MMC_TIMING_UHS_DDR50:
> +	case MMC_TIMING_MMC_HS200:
> +	case MMC_TIMING_MMC_DDR52:
> +	case MMC_TIMING_MMC_HS400:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
>  struct mci_ios {
>  	unsigned int	clock;			/* clock rate */
>  
> -- 
> 2.39.2
> 
> 
> 

Re: [PATCH 2/2] mci: imx-esdhc: add uSDHC eMMC DDR52 supprot

[Marco Felsch]

On 23-04-17, Ahmad Fatoum wrote:
> The uSDHC available with the i.MX6/7/8 SoCs has support for DDR
> clock operation. This is enabled by flipping a bit in
> IMX_SDHCI_MIXCTRL and adjusting the clock divider calculation to
> account for the automatic internal halving of the clock.
> 
> Let's do that to speed up boot from eMMC. How much effect this has
> in practice is not constant. Comparing two Kingston eMMCs: DDR on the
> older v4.5 connected to an i.MX6 did not yield any difference. On the
> newer v5.1 one connected to an i.MX8MM, I observe a 70% improvement in
> throughput: from 40MiB/s to 69.5 MiB/s.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>

> ---
>  drivers/mci/imx-esdhc-common.c |  1 +
>  drivers/mci/imx-esdhc.c        | 45 +++++++++++++++++++++++++++++-----
>  drivers/mci/imx-esdhc.h        | 14 +++++++++++
>  3 files changed, 54 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mci/imx-esdhc-common.c b/drivers/mci/imx-esdhc-common.c
> index 27293e44b724..3c1ff9882432 100644
> --- a/drivers/mci/imx-esdhc-common.c
> +++ b/drivers/mci/imx-esdhc-common.c
> @@ -160,6 +160,7 @@ int __esdhc_send_cmd(struct fsl_esdhc_host *host, struct mci_cmd *cmd,
>  		mixctrl = xfertyp;
>  		/* Keep the bits 22-25 of the register as is */
>  		mixctrl |= (sdhci_read32(&host->sdhci, IMX_SDHCI_MIXCTRL) & (0xF << 22));
> +		mixctrl |= mci_timing_is_ddr(host->sdhci.timing) ? MIX_CTRL_DDREN : 0;
>  		sdhci_write32(&host->sdhci, IMX_SDHCI_MIXCTRL, mixctrl);
>  	}
>  
> diff --git a/drivers/mci/imx-esdhc.c b/drivers/mci/imx-esdhc.c
> index 2e2bc14ef95c..94f18fe2b3cf 100644
> --- a/drivers/mci/imx-esdhc.c
> +++ b/drivers/mci/imx-esdhc.c
> @@ -43,9 +43,9 @@ esdhc_send_cmd(struct mci_host *mci, struct mci_cmd *cmd, struct mci_data *data)
>  	return  __esdhc_send_cmd(host, cmd, data);
>  }
>  
> -static void set_sysctl(struct mci_host *mci, u32 clock)
> +static void set_sysctl(struct mci_host *mci, u32 clock, bool ddr)
>  {
> -	int div, pre_div;
> +	int div, pre_div, ddr_pre_div = ddr ? 2 : 1;
>  	struct fsl_esdhc_host *host = to_fsl_esdhc(mci);
>  	int sdhc_clk = clk_get_rate(host->clk);
>  	u32 clk;
> @@ -65,11 +65,11 @@ static void set_sysctl(struct mci_host *mci, u32 clock)
>  		pre_div = 1;
>  	else if (sdhc_clk / 16 > clock)
>  		for (; pre_div < 256; pre_div *= 2)
> -			if ((sdhc_clk / pre_div) <= (clock * 16))
> +			if ((sdhc_clk / (pre_div * ddr_pre_div)) <= (clock * 16))
>  				break;
>  
>  	for (div = 1; div <= 16; div++)
> -		if ((sdhc_clk / (div * pre_div)) <= clock)
> +		if ((sdhc_clk / (div * pre_div * ddr_pre_div)) <= clock)
>  			break;
>  
>  	cur_clock = sdhc_clk / pre_div / div;
> @@ -103,12 +103,42 @@ static void set_sysctl(struct mci_host *mci, u32 clock)
>  		   10 * MSECOND);
>  }
>  
> +static void esdhc_set_timing(struct fsl_esdhc_host *host, enum mci_timing timing)
> +{
> +	u32 mixctrl;
> +
> +	mixctrl = sdhci_read32(&host->sdhci, IMX_SDHCI_MIXCTRL);
> +	mixctrl &= ~MIX_CTRL_DDREN;
> +
> +	switch (timing) {
> +	case MMC_TIMING_UHS_DDR50:
> +	case MMC_TIMING_MMC_DDR52:
> +		mixctrl |= MIX_CTRL_DDREN;
> +		sdhci_write32(&host->sdhci, IMX_SDHCI_MIXCTRL, mixctrl);
> +		break;
> +	default:
> +		sdhci_write32(&host->sdhci, IMX_SDHCI_MIXCTRL, mixctrl);
> +	}
> +
> +	host->sdhci.timing = timing;
> +}
> +
>  static void esdhc_set_ios(struct mci_host *mci, struct mci_ios *ios)
>  {
>  	struct fsl_esdhc_host *host = to_fsl_esdhc(mci);
>  
> +	/*
> +	 * call esdhc_set_timing() before update the clock rate,
> +	 * This is because current we support DDR and SDR timing,
> +	 * Once the DDR_EN bit is set, the card clock will be
> +	 * divide by 2 automatically. So need to do this before
> +	 * setting clock rate.
> +	 */
> +	if (esdhc_is_usdhc(host) && host->sdhci.timing != ios->timing)
> +		esdhc_set_timing(host, ios->timing);
> +
>  	/* Set the clock speed */
> -	set_sysctl(mci, ios->clock);
> +	set_sysctl(mci, ios->clock, mci_timing_is_ddr(ios->timing));
>  
>  	/* Set the bus width */
>  	esdhc_clrbits32(host, SDHCI_HOST_CONTROL__POWER_CONTROL__BLOCK_GAP_CONTROL,
> @@ -206,7 +236,7 @@ static int esdhc_init(struct mci_host *mci, struct device *dev)
>  		esdhc_setbits32(host, ESDHC_DMA_SYSCTL, ESDHC_SYSCTL_DMA_SNOOP);
>  
>  	/* Set the initial clock speed */
> -	set_sysctl(mci, 400000);
> +	set_sysctl(mci, 400000, false);
>  
>  	sdhci_write32(&host->sdhci, SDHCI_INT_ENABLE, SDHCI_INT_CMD_COMPLETE |
>  			SDHCI_INT_XFER_COMPLETE | SDHCI_INT_CARD_INT |
> @@ -285,6 +315,9 @@ static int fsl_esdhc_probe(struct device *dev)
>  	if (ret)
>  		goto err_clk_disable;
>  
> +	if (esdhc_is_usdhc(host))
> +		mci->host_caps |= MMC_CAP_MMC_3_3V_DDR | MMC_CAP_MMC_1_8V_DDR;
> +
>  	rate = clk_get_rate(host->clk);
>  	host->mci.f_min = rate >> 12;
>  	if (host->mci.f_min < 200000)
> diff --git a/drivers/mci/imx-esdhc.h b/drivers/mci/imx-esdhc.h
> index 0d8f157a7615..b14039757a7a 100644
> --- a/drivers/mci/imx-esdhc.h
> +++ b/drivers/mci/imx-esdhc.h
> @@ -41,6 +41,20 @@
>  
>  #define IMX_SDHCI_WML		0x44
>  #define IMX_SDHCI_MIXCTRL	0x48
> +/* Imported from Linux Kernel drivers/mmc/host/sdhci-esdhc-imx.c */
> +#define  MIX_CTRL_DDREN		BIT(3)
> +#define  MIX_CTRL_DTDSEL_READ	BIT(4)
> +#define  MIX_CTRL_AC23EN	BIT(7)
> +#define  MIX_CTRL_EXE_TUNE	BIT(22)
> +#define  MIX_CTRL_SMPCLK_SEL	BIT(23)
> +#define  MIX_CTRL_AUTO_TUNE_EN	BIT(24)
> +#define  MIX_CTRL_FBCLK_SEL	BIT(25)
> +#define  MIX_CTRL_HS400_EN	BIT(26)
> +#define  MIX_CTRL_HS400_ES	BIT(27)
> +/* Bits 3 and 6 are not SDHCI standard definitions */
> +#define  MIX_CTRL_SDHCI_MASK	0xb7
> +/* Tuning bits */
> +#define  MIX_CTRL_TUNING_MASK	0x03c00000
>  #define IMX_SDHCI_DLL_CTRL	0x60
>  #define IMX_SDHCI_MIX_CTRL_FBCLK_SEL	BIT(25)
>  
> -- 
> 2.39.2
> 
> 
> 

Re: [PATCH 1/2] mci: add eMMC DDR52 support

[Ahmad Fatoum]

Hello Marco,

On 18.04.23 09:27, Marco Felsch wrote:
> Hi Ahmad,
> 
> On 23-04-17, Ahmad Fatoum wrote:
>> The maximum card frequency that can be configured by barebox currently
>> is 50MHz for SD and 52MHz for eMMC. Higher speed modes require runtime
>> voltage switching or tuning sequences, which are not yet implemented.
>>
>> Only exception is eMMC's DDR52: This mode was first introduced with
>> MMC 4.4 and can be used even at 3.3V.
> 
> Nice :)
> 
>> This commit adds DDR52 support to the core. This introduces no functional
>> change, because host controllers must opt-in by setting the appropriate
>> host capabilities.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  drivers/mci/mci-core.c | 54 +++++++++++++++++++++++++++++++++++-------
>>  include/mci.h          | 19 +++++++++++++++
>>  2 files changed, 64 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
>> index f647cae8203b..86f468edfea6 100644
>> --- a/drivers/mci/mci-core.c
>> +++ b/drivers/mci/mci-core.c
>> @@ -135,6 +135,9 @@ static int mci_set_blocklen(struct mci *mci, unsigned len)
>>  {
>>  	struct mci_cmd cmd;
>>  
>> +	if (mci->host->timing == MMC_TIMING_MMC_DDR52)
>> +		return 0;
>> +
>>  	mci_setup_cmd(&cmd, MMC_CMD_SET_BLOCKLEN, len, MMC_RSP_R1);
>>  	return mci_send_cmd(mci, &cmd, NULL);
>>  }
>> @@ -649,11 +652,15 @@ static int mmc_change_freq(struct mci *mci)
>>  		return 0;
>>  	}
>>  
>> -	/* High Speed is set, there are two types: 52MHz and 26MHz */
>> -	if (cardtype & EXT_CSD_CARD_TYPE_52)
>> -		mci->card_caps |= MMC_CAP_MMC_HIGHSPEED_52MHZ | MMC_CAP_MMC_HIGHSPEED;
>> -	else
>> -		mci->card_caps |= MMC_CAP_MMC_HIGHSPEED;
>> +	mci->card_caps |= MMC_CAP_MMC_HIGHSPEED;
>> +
>> +	/* High Speed is set, there are three types: 26MHz, 52MHz, 52MHz DDR */
>> +	if (cardtype & EXT_CSD_CARD_TYPE_52) {
>> +		mci->card_caps |= MMC_CAP_MMC_HIGHSPEED_52MHZ;
>> +
>> +		if (cardtype & EXT_CSD_CARD_TYPE_DDR_1_8V)
>> +			mci->card_caps |= MMC_CAP_MMC_3_3V_DDR | MMC_CAP_MMC_1_8V_DDR;
>> +	}
>>  
>>  	if (IS_ENABLED(CONFIG_MCI_MMC_BOOT_PARTITIONS) &&
>>  			mci->ext_csd[EXT_CSD_REV] >= 3 && mci->ext_csd[EXT_CSD_BOOT_SIZE_MULT]) {
>> @@ -1170,15 +1177,20 @@ static int mci_startup_sd(struct mci *mci)
>>  static int mci_startup_mmc(struct mci *mci)
>>  {
>>  	struct mci_host *host = mci->host;
>> +	enum mci_timing timing_orig;
>>  	int err;
>>  	int idx = 0;
>>  	static unsigned ext_csd_bits[] = {
>>  		EXT_CSD_BUS_WIDTH_4,
>>  		EXT_CSD_BUS_WIDTH_8,
>> +		EXT_CSD_DDR_BUS_WIDTH_4,
>> +		EXT_CSD_DDR_BUS_WIDTH_8,
>>  	};
>>  	static unsigned bus_widths[] = {
>>  		MMC_BUS_WIDTH_4,
>>  		MMC_BUS_WIDTH_8,
>> +		MMC_BUS_WIDTH_4,
>> +		MMC_BUS_WIDTH_8,
> 
> This is duplicated or should it be MMC_DDR_BUS_WIDTH_4/8?

The trial sequence I had in mind originally was:

  DDR 8-bit -> DDR 4-bit -> SDR 8-bit -> SDR 4-bit

That's how U-Boot does it. I compared with Linux and there we have:

  SDR 8-bit -> SDR 4-bit and then depending on the result, either
  DDR 8-bit or DDR 4-bit

Given that DDR is not a 100% improvement in my testing, I should
rather do it like Linux does and prefer SDR 8-bit over DDR 4-bit.

I will do this for v2.

This also fixes a bug with the code: If host doesn't support 8-bit
MMC, we would still not test for 8-bit SDR, but would for 8-bit DDR.

>> @@ -1661,12 +1694,15 @@ static const char *mci_timing_tostr(unsigned timing)
>>  
>>  static void mci_print_caps(unsigned caps)
>>  {
>> -	printf("  capabilities: %s%s%s%s%s\n",
>> +	printf("  capabilities: %s%s%s%s%s%s%s%s\n",
>>  		caps & MMC_CAP_4_BIT_DATA ? "4bit " : "",
>>  		caps & MMC_CAP_8_BIT_DATA ? "8bit " : "",
>>  		caps & MMC_CAP_SD_HIGHSPEED ? "sd-hs " : "",
>>  		caps & MMC_CAP_MMC_HIGHSPEED ? "mmc-hs " : "",
>> -		caps & MMC_CAP_MMC_HIGHSPEED_52MHZ ? "mmc-52MHz " : "");
>> +		caps & MMC_CAP_MMC_HIGHSPEED_52MHZ ? "mmc-52MHz " : "",
>> +		caps & MMC_CAP_MMC_3_3V_DDR ? "ddr-3.3v " : "",
>> +		caps & MMC_CAP_MMC_1_8V_DDR ? "ddr-1.8v " : "",
>> +		caps & MMC_CAP_MMC_1_2V_DDR ? "ddr-1.2v " : "");
> 
> At the moment we only report what barebox does support, ddr-1.8v and
> ddr-1.2v isn't supported. Do we really want to report this?

DDR 1.8V on eMMC is not dynamically selected, but 1.2v is. So this
series works for both 3.3v and 1.8v. I don't see a problem with reporting
1.2v support for cards. No host will set this as long we don't have
voltage switching.

Cheers,
Ahmad

> 
> Regards,
>   Marco
> 
>>  }
>>  
>>  /**
>> diff --git a/include/mci.h b/include/mci.h
>> index d356f071f7f2..88712c35492e 100644
>> --- a/include/mci.h
>> +++ b/include/mci.h
>> @@ -51,6 +51,11 @@
>>  #define MMC_CAP_SD_HIGHSPEED		(1 << 3)
>>  #define MMC_CAP_MMC_HIGHSPEED		(1 << 4)
>>  #define MMC_CAP_MMC_HIGHSPEED_52MHZ	(1 << 5)
>> +#define MMC_CAP_MMC_3_3V_DDR		(1 << 7)	/* Host supports eMMC DDR 3.3V */
>> +#define MMC_CAP_MMC_1_8V_DDR		(1 << 8)	/* Host supports eMMC DDR 1.8V */
>> +#define MMC_CAP_MMC_1_2V_DDR		(1 << 9)	/* Host supports eMMC DDR 1.2V */
>> +#define MMC_CAP_DDR			(MMC_CAP_3_3V_DDR | MMC_CAP_1_8V_DDR | \
>> +					 MMC_CAP_1_2V_DDR)
>>  /* Mask of all caps for bus width */
>>  #define MMC_CAP_BIT_DATA_MASK		(MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA)
>>  
>> @@ -308,6 +313,7 @@
>>  #define EXT_CSD_BUS_WIDTH_8	2	/* Card is in 8 bit mode */
>>  #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 R1_ILLEGAL_COMMAND		(1 << 22)
>>  #define R1_STATUS(x)			(x & 0xFFF9A000)
>> @@ -410,6 +416,19 @@ enum mci_timing {
>>  	MMC_TIMING_MMC_HS400	= 8,
>>  };
>>  
>> +static inline bool mci_timing_is_ddr(enum mci_timing timing)
>> +{
>> +	switch (timing) {
>> +	case MMC_TIMING_UHS_DDR50:
>> +	case MMC_TIMING_MMC_HS200:
>> +	case MMC_TIMING_MMC_DDR52:
>> +	case MMC_TIMING_MMC_HS400:
>> +		return true;
>> +	default:
>> +		return false;
>> +	}
>> +}
>> +
>>  struct mci_ios {
>>  	unsigned int	clock;			/* clock rate */
>>  
>> -- 
>> 2.39.2
>>
>>
>>
> 

-- 
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 |