All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 1/2] mmc: add HS400 support
@ 2018-03-05  9:11 Peng Fan
  2018-03-05  9:11 ` [U-Boot] [PATCH 2/2] mmc: fsl_esdhc: enable HS400 feature Peng Fan
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Peng Fan @ 2018-03-05  9:11 UTC (permalink / raw)
  To: u-boot

Add HS400 support.
Selecting HS400 needs first select HS199 according to spec, so use
a dedicated function for HS400.
Add HS400 related macros.
Remove the restriction of only using the low 6 bits of
EXT_CSD_CARD_TYPE, using all the 8 bits.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
Cc: Jaehoon Chung <jh80.chung@samsung.com>
Cc: Jean-Jacques Hiblot <jjhiblot@ti.com>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Simon Glass <sjg@chromium.org>
Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Bin Meng <bmeng.cn@gmail.com>
---
 drivers/mmc/Kconfig |   7 +++
 drivers/mmc/mmc.c   | 133 ++++++++++++++++++++++++++++++++++++++++++----------
 include/mmc.h       |  12 +++++
 3 files changed, 127 insertions(+), 25 deletions(-)

diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
index 5f67e336db..e9be18b333 100644
--- a/drivers/mmc/Kconfig
+++ b/drivers/mmc/Kconfig
@@ -104,6 +104,13 @@ config SPL_MMC_UHS_SUPPORT
 	  cards. The IO voltage must be switchable from 3.3v to 1.8v. The bus
 	  frequency can go up to 208MHz (SDR104)
 
+config MMC_HS400_SUPPORT
+	bool "enable HS400 support"
+	select MMC_HS200_SUPPORT
+	help
+	  The HS400 mode is support by some eMMC. The bus frequency is up to
+	  200MHz. This mode requires tuning the IO.
+
 config MMC_HS200_SUPPORT
 	bool "enable HS200 support"
 	help
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 92ea78b8af..eef229c8b4 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -169,6 +169,7 @@ const char *mmc_mode_name(enum bus_mode mode)
 	      [MMC_HS_52]	= "MMC High Speed (52MHz)",
 	      [MMC_DDR_52]	= "MMC DDR52 (52MHz)",
 	      [MMC_HS_200]	= "HS200 (200MHz)",
+	      [MMC_HS_400]	= "HS400 (200MHz)",
 	};
 
 	if (mode >= MMC_MODES_END)
@@ -193,6 +194,7 @@ static uint mmc_mode2freq(struct mmc *mmc, enum bus_mode mode)
 	      [UHS_DDR50]	= 50000000,
 	      [UHS_SDR104]	= 208000000,
 	      [MMC_HS_200]	= 200000000,
+	      [MMC_HS_400]	= 200000000,
 	};
 
 	if (mode == MMC_LEGACY)
@@ -790,6 +792,11 @@ static int mmc_set_card_speed(struct mmc *mmc, enum bus_mode mode)
 	case MMC_HS_200:
 		speed_bits = EXT_CSD_TIMING_HS200;
 		break;
+#endif
+#if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
+	case MMC_HS_400:
+		speed_bits = EXT_CSD_TIMING_HS400;
+		break;
 #endif
 	case MMC_LEGACY:
 		speed_bits = EXT_CSD_TIMING_LEGACY;
@@ -837,7 +844,7 @@ static int mmc_get_capabilities(struct mmc *mmc)
 
 	mmc->card_caps |= MMC_MODE_4BIT | MMC_MODE_8BIT;
 
-	cardtype = ext_csd[EXT_CSD_CARD_TYPE] & 0x3f;
+	cardtype = ext_csd[EXT_CSD_CARD_TYPE];
 	mmc->cardtype = cardtype;
 
 #if CONFIG_IS_ENABLED(MMC_HS200_SUPPORT)
@@ -845,6 +852,12 @@ static int mmc_get_capabilities(struct mmc *mmc)
 			EXT_CSD_CARD_TYPE_HS200_1_8V)) {
 		mmc->card_caps |= MMC_MODE_HS200;
 	}
+#endif
+#if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
+	if (cardtype & (EXT_CSD_CARD_TYPE_HS400_1_2V |
+			EXT_CSD_CARD_TYPE_HS400_1_8V)) {
+		mmc->card_caps |= MMC_MODE_HS400;
+	}
 #endif
 	if (cardtype & EXT_CSD_CARD_TYPE_52) {
 		if (cardtype & EXT_CSD_CARD_TYPE_DDR_52)
@@ -1748,6 +1761,12 @@ static int mmc_set_lowest_voltage(struct mmc *mmc, enum bus_mode mode,
 	u32 card_mask = 0;
 
 	switch (mode) {
+	case MMC_HS_400:
+		if (mmc->cardtype & EXT_CSD_CARD_TYPE_HS400_1_8V)
+			card_mask |= MMC_SIGNAL_VOLTAGE_180;
+		if (mmc->cardtype & EXT_CSD_CARD_TYPE_HS400_1_2V)
+			card_mask |= MMC_SIGNAL_VOLTAGE_120;
+		break;
 	case MMC_HS_200:
 		if (mmc->cardtype & EXT_CSD_CARD_TYPE_HS200_1_8V)
 			card_mask |= MMC_SIGNAL_VOLTAGE_180;
@@ -1787,6 +1806,13 @@ static inline int mmc_set_lowest_voltage(struct mmc *mmc, enum bus_mode mode,
 #endif
 
 static const struct mode_width_tuning mmc_modes_by_pref[] = {
+#if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
+	{
+		.mode = MMC_HS_400,
+		.widths = MMC_MODE_8BIT | MMC_MODE_4BIT,
+		.tuning = MMC_CMD_SEND_TUNING_BLOCK_HS200
+	},
+#endif
 #if CONFIG_IS_ENABLED(MMC_HS200_SUPPORT)
 	{
 		.mode = MMC_HS_200,
@@ -1830,6 +1856,54 @@ static const struct ext_csd_bus_width {
 	{MMC_MODE_1BIT, false, EXT_CSD_BUS_WIDTH_1},
 };
 
+#if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
+static int mmc_select_hs400(struct mmc *mmc)
+{
+	int err;
+
+	/* Set timing to HS200 for tuning */
+	err = mmc_set_card_speed(mmc, MMC_HS_200);
+	if (err)
+		return err;
+
+	/* configure the bus mode (host) */
+	mmc_select_mode(mmc, MMC_HS_200);
+	mmc_set_clock(mmc, mmc->tran_speed, false);
+
+	/* execute tuning if needed */
+	err = mmc_execute_tuning(mmc, MMC_CMD_SEND_TUNING_BLOCK_HS200);
+	if (err) {
+		debug("tuning failed\n");
+		return err;
+	}
+
+	/* Set back to HS */
+	mmc_set_card_speed(mmc, MMC_HS);
+	mmc_set_clock(mmc, mmc_mode2freq(mmc, MMC_HS), false);
+
+	err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_BUS_WIDTH,
+			 EXT_CSD_BUS_WIDTH_8 | EXT_CSD_DDR_FLAG);
+	if (err)
+		return err;
+
+	err = mmc_set_card_speed(mmc, MMC_HS_400);
+	if (err)
+		return err;
+
+	mmc_select_mode(mmc, MMC_HS_400);
+	err = mmc_set_clock(mmc, mmc->tran_speed, false);
+	if (err)
+		return err;
+
+	return 0;
+}
+#else
+static int mmc_select_hs400(struct mmc *mmc)
+{
+	return -ENOTSUPP;
+}
+#endif
+
 #define for_each_supported_width(caps, ddr, ecbv) \
 	for (ecbv = ext_csd_bus_width;\
 	    ecbv < ext_csd_bus_width + ARRAY_SIZE(ext_csd_bus_width);\
@@ -1883,37 +1957,46 @@ static int mmc_select_mode_and_width(struct mmc *mmc, uint card_caps)
 				goto error;
 			mmc_set_bus_width(mmc, bus_width(ecbw->cap));
 
-			/* configure the bus speed (card) */
-			err = mmc_set_card_speed(mmc, mwt->mode);
-			if (err)
-				goto error;
-
-			/*
-			 * configure the bus width AND the ddr mode (card)
-			 * The host side will be taken care of in the next step
-			 */
-			if (ecbw->ext_csd_bits & EXT_CSD_DDR_FLAG) {
-				err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL,
-						 EXT_CSD_BUS_WIDTH,
-						 ecbw->ext_csd_bits);
+			if (mwt->mode == MMC_HS_400) {
+				err = mmc_select_hs400(mmc);
+				if (err)
+					goto error;
+			} else {
+				/* configure the bus speed (card) */
+				err = mmc_set_card_speed(mmc, mwt->mode);
 				if (err)
 					goto error;
-			}
 
-			/* configure the bus mode (host) */
-			mmc_select_mode(mmc, mwt->mode);
-			mmc_set_clock(mmc, mmc->tran_speed, false);
+				/*
+				 * configure the bus width AND the ddr mode
+				 * (card). The host side will be taken care
+				 * of in the next step
+				 */
+				if (ecbw->ext_csd_bits & EXT_CSD_DDR_FLAG) {
+					err = mmc_switch(mmc,
+							 EXT_CSD_CMD_SET_NORMAL,
+							 EXT_CSD_BUS_WIDTH,
+							 ecbw->ext_csd_bits);
+					if (err)
+						goto error;
+				}
+
+				/* configure the bus mode (host) */
+				mmc_select_mode(mmc, mwt->mode);
+				mmc_set_clock(mmc, mmc->tran_speed, false);
 #ifdef MMC_SUPPORTS_TUNING
 
-			/* execute tuning if needed */
-			if (mwt->tuning) {
-				err = mmc_execute_tuning(mmc, mwt->tuning);
-				if (err) {
-					pr_debug("tuning failed\n");
-					goto error;
+				/* execute tuning if needed */
+				if (mwt->tuning) {
+					err = mmc_execute_tuning(mmc,
+								 mwt->tuning);
+					if (err) {
+						pr_debug("tuning failed\n");
+						goto error;
+					}
 				}
-			}
 #endif
+			}
 
 			/* do a transfer to check the configuration */
 			err = mmc_read_and_compare_ext_csd(mmc);
diff --git a/include/mmc.h b/include/mmc.h
index 86f885b504..8c01c6a530 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -65,6 +65,7 @@
 #define MMC_MODE_HS_52MHz	MMC_CAP(MMC_HS_52)
 #define MMC_MODE_DDR_52MHz	MMC_CAP(MMC_DDR_52)
 #define MMC_MODE_HS200		MMC_CAP(MMC_HS_200)
+#define MMC_MODE_HS400		MMC_CAP(MMC_HS_400)
 
 #define MMC_MODE_8BIT		BIT(30)
 #define MMC_MODE_4BIT		BIT(29)
@@ -250,6 +251,11 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx)
 #define EXT_CSD_CARD_TYPE_HS200		(EXT_CSD_CARD_TYPE_HS200_1_8V | \
 					 EXT_CSD_CARD_TYPE_HS200_1_2V)
 
+#define EXT_CSD_CARD_TYPE_HS400_1_8V	BIT(6)
+#define EXT_CSD_CARD_TYPE_HS400_1_2V	BIT(7)
+#define EXT_CSD_CARD_TYPE_HS400		(EXT_CSD_CARD_TYPE_HS400_1_8V | \
+					 EXT_CSD_CARD_TYPE_HS400_1_2V)
+
 #define EXT_CSD_BUS_WIDTH_1	0	/* Card is in 1 bit mode */
 #define EXT_CSD_BUS_WIDTH_4	1	/* Card is in 4 bit mode */
 #define EXT_CSD_BUS_WIDTH_8	2	/* Card is in 8 bit mode */
@@ -260,6 +266,7 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx)
 #define EXT_CSD_TIMING_LEGACY	0	/* no high speed */
 #define EXT_CSD_TIMING_HS	1	/* HS */
 #define EXT_CSD_TIMING_HS200	2	/* HS200 */
+#define EXT_CSD_TIMING_HS400	3	/* HS400 */
 
 #define EXT_CSD_BOOT_ACK_ENABLE			(1 << 6)
 #define EXT_CSD_BOOT_PARTITION_ENABLE		(1 << 3)
@@ -520,6 +527,7 @@ enum bus_mode {
 	UHS_DDR50,
 	UHS_SDR104,
 	MMC_HS_200,
+	MMC_HS_400,
 	MMC_MODES_END
 };
 
@@ -533,6 +541,10 @@ static inline bool mmc_is_mode_ddr(enum bus_mode mode)
 #if CONFIG_IS_ENABLED(MMC_UHS_SUPPORT)
 	else if (mode == UHS_DDR50)
 		return true;
+#endif
+#if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
+	else if (mode == MMC_HS_400)
+		return true;
 #endif
 	else
 		return false;
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH 2/2] mmc: fsl_esdhc: enable HS400 feature
  2018-03-05  9:11 [U-Boot] [PATCH 1/2] mmc: add HS400 support Peng Fan
@ 2018-03-05  9:11 ` Peng Fan
       [not found]   ` <CGME20180426103032epcas1p47ee75c2117d1a7d4f2d4fdde5824074e@epcas1p4.samsung.com>
  2018-03-05 16:29 ` [U-Boot] [PATCH 1/2] mmc: add HS400 support Jean-Jacques Hiblot
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Peng Fan @ 2018-03-05  9:11 UTC (permalink / raw)
  To: u-boot

The strobe dll code is ported from Linux Kernel:
drivers/mmc/host/sdhci-esdhc-imx.c
The comments are from the above file,
"For HS400 eMMC, there is a data_strobe line. This signal is generated
by the device and used for data output and CRC status response output
in HS400 mode. The frequency of this signal follows the frequency of
CLK generated by host. The host receives the data which is aligned to the
edge of data_strobe line. Due to the time delay between CLK line and
data_strobe line, if the delay time is larger than one clock cycle,
then CLK and data_strobe line will be misaligned, read error shows up.
So when the CLK is higher than 100MHz, each clock cycle is short enough,
host should configure the delay target. "

Signed-off-by: Peng Fan <peng.fan@nxp.com>
Cc: Jaehoon Chung <jh80.chung@samsung.com>
Cc: Stefano Babic <sbabic@denx.de>
---
 drivers/mmc/fsl_esdhc.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
index 6018f84307..e42bdb6941 100644
--- a/drivers/mmc/fsl_esdhc.c
+++ b/drivers/mmc/fsl_esdhc.c
@@ -663,6 +663,7 @@ static int esdhc_change_pinstate(struct udevice *dev)
 		break;
 	case UHS_SDR104:
 	case MMC_HS_200:
+	case MMC_HS_400:
 		ret = pinctrl_select_state(dev, "state_200mhz");
 		break;
 	default:
@@ -690,6 +691,33 @@ static void esdhc_reset_tuning(struct mmc *mmc)
 	}
 }
 
+static void esdhc_set_strobe_dll(struct mmc *mmc)
+{
+	struct fsl_esdhc_priv *priv = dev_get_priv(mmc->dev);
+	struct fsl_esdhc *regs = priv->esdhc_regs;
+	u32 v;
+
+	if (priv->clock > ESDHC_STROBE_DLL_CLK_FREQ) {
+		writel(ESDHC_STROBE_DLL_CTRL_RESET, &regs->strobe_dllctrl);
+
+		/*
+		 * enable strobe dll ctrl and adjust the delay target
+		 * for the uSDHC loopback read clock
+		 */
+		v = ESDHC_STROBE_DLL_CTRL_ENABLE |
+			(priv->strobe_dll_delay_target <<
+			 ESDHC_STROBE_DLL_CTRL_SLV_DLY_TARGET_SHIFT);
+		writel(v, &regs->strobe_dllctrl);
+		/* wait 1us to make sure strobe dll status register stable */
+		mdelay(1);
+		v = readl(&regs->strobe_dllstat);
+		if (!(v & ESDHC_STROBE_DLL_STS_REF_LOCK))
+			pr_warn("HS400 strobe DLL status REF not lock!\n");
+		if (!(v & ESDHC_STROBE_DLL_STS_SLV_LOCK))
+			pr_warn("HS400 strobe DLL status SLV not lock!\n");
+	}
+}
+
 static int esdhc_set_timing(struct mmc *mmc)
 {
 	struct fsl_esdhc_priv *priv = dev_get_priv(mmc->dev);
@@ -704,6 +732,11 @@ static int esdhc_set_timing(struct mmc *mmc)
 	case SD_LEGACY:
 		esdhc_reset_tuning(mmc);
 		break;
+	case MMC_HS_400:
+		mixctrl |= MIX_CTRL_DDREN | MIX_CTRL_HS400_EN;
+		writel(mixctrl, &regs->mixctrl);
+		esdhc_set_strobe_dll(mmc);
+		break;
 	case MMC_HS:
 	case MMC_HS_52:
 	case MMC_HS_200:
@@ -1439,7 +1472,7 @@ static int fsl_esdhc_probe(struct udevice *dev)
 #endif
 
 	if (fdt_get_property(fdt, node, "no-1-8-v", NULL))
-		priv->caps &= ~(UHS_CAPS | MMC_MODE_HS200);
+		priv->caps &= ~(UHS_CAPS | MMC_MODE_HS200 | MMC_MODE_HS400);
 
 	/*
 	 * TODO:
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH 1/2] mmc: add HS400 support
  2018-03-05  9:11 [U-Boot] [PATCH 1/2] mmc: add HS400 support Peng Fan
  2018-03-05  9:11 ` [U-Boot] [PATCH 2/2] mmc: fsl_esdhc: enable HS400 feature Peng Fan
@ 2018-03-05 16:29 ` Jean-Jacques Hiblot
  2018-03-06  1:46   ` Peng Fan
  2018-03-06 17:41 ` Simon Glass
       [not found] ` <CGME20180426103029epcas1p4f0292a99571b6970f61d67ff42728fb0@epcas1p4.samsung.com>
  3 siblings, 1 reply; 10+ messages in thread
From: Jean-Jacques Hiblot @ 2018-03-05 16:29 UTC (permalink / raw)
  To: u-boot

Hi Peng,

I'm glad you are adding HS400 support. Thanks.


On 05/03/2018 10:11, Peng Fan wrote:
> Add HS400 support.
> Selecting HS400 needs first select HS199 according to spec, so use
> a dedicated function for HS400.
> Add HS400 related macros.
> Remove the restriction of only using the low 6 bits of
> EXT_CSD_CARD_TYPE, using all the 8 bits.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> Cc: Jaehoon Chung <jh80.chung@samsung.com>
> Cc: Jean-Jacques Hiblot <jjhiblot@ti.com>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Kishon Vijay Abraham I <kishon@ti.com>
> Cc: Bin Meng <bmeng.cn@gmail.com>
> ---
>   drivers/mmc/Kconfig |   7 +++
>   drivers/mmc/mmc.c   | 133 ++++++++++++++++++++++++++++++++++++++++++----------
>   include/mmc.h       |  12 +++++
>   3 files changed, 127 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
> index 5f67e336db..e9be18b333 100644
> --- a/drivers/mmc/Kconfig
> +++ b/drivers/mmc/Kconfig
> @@ -104,6 +104,13 @@ config SPL_MMC_UHS_SUPPORT
>   	  cards. The IO voltage must be switchable from 3.3v to 1.8v. The bus
>   	  frequency can go up to 208MHz (SDR104)
>   
> +config MMC_HS400_SUPPORT
> +	bool "enable HS400 support"
> +	select MMC_HS200_SUPPORT
I'd use "depends on" instead of select or maybe use the same option for 
both HS200 and HS400
> +	help
> +	  The HS400 mode is support by some eMMC. The bus frequency is up to
> +	  200MHz. This mode requires tuning the IO.
> +
>   config MMC_HS200_SUPPORT
>   	bool "enable HS200 support"
>   	help
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index 92ea78b8af..eef229c8b4 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -169,6 +169,7 @@ const char *mmc_mode_name(enum bus_mode mode)
>   	      [MMC_HS_52]	= "MMC High Speed (52MHz)",
>   	      [MMC_DDR_52]	= "MMC DDR52 (52MHz)",
>   	      [MMC_HS_200]	= "HS200 (200MHz)",
> +	      [MMC_HS_400]	= "HS400 (200MHz)",
>   	};
>   
>   	if (mode >= MMC_MODES_END)
> @@ -193,6 +194,7 @@ static uint mmc_mode2freq(struct mmc *mmc, enum bus_mode mode)
>   	      [UHS_DDR50]	= 50000000,
>   	      [UHS_SDR104]	= 208000000,
>   	      [MMC_HS_200]	= 200000000,
> +	      [MMC_HS_400]	= 200000000,
>   	};
>   
>   	if (mode == MMC_LEGACY)
> @@ -790,6 +792,11 @@ static int mmc_set_card_speed(struct mmc *mmc, enum bus_mode mode)
>   	case MMC_HS_200:
>   		speed_bits = EXT_CSD_TIMING_HS200;
>   		break;
> +#endif
> +#if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
> +	case MMC_HS_400:
> +		speed_bits = EXT_CSD_TIMING_HS400;
> +		break;
>   #endif
>   	case MMC_LEGACY:
>   		speed_bits = EXT_CSD_TIMING_LEGACY;
> @@ -837,7 +844,7 @@ static int mmc_get_capabilities(struct mmc *mmc)
>   
>   	mmc->card_caps |= MMC_MODE_4BIT | MMC_MODE_8BIT;
>   
> -	cardtype = ext_csd[EXT_CSD_CARD_TYPE] & 0x3f;
> +	cardtype = ext_csd[EXT_CSD_CARD_TYPE];
>   	mmc->cardtype = cardtype;
>   
>   #if CONFIG_IS_ENABLED(MMC_HS200_SUPPORT)
> @@ -845,6 +852,12 @@ static int mmc_get_capabilities(struct mmc *mmc)
>   			EXT_CSD_CARD_TYPE_HS200_1_8V)) {
>   		mmc->card_caps |= MMC_MODE_HS200;
>   	}
> +#endif
> +#if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
> +	if (cardtype & (EXT_CSD_CARD_TYPE_HS400_1_2V |
> +			EXT_CSD_CARD_TYPE_HS400_1_8V)) {
> +		mmc->card_caps |= MMC_MODE_HS400;
> +	}
>   #endif
>   	if (cardtype & EXT_CSD_CARD_TYPE_52) {
>   		if (cardtype & EXT_CSD_CARD_TYPE_DDR_52)
> @@ -1748,6 +1761,12 @@ static int mmc_set_lowest_voltage(struct mmc *mmc, enum bus_mode mode,
>   	u32 card_mask = 0;
>   
>   	switch (mode) {
> +	case MMC_HS_400:
> +		if (mmc->cardtype & EXT_CSD_CARD_TYPE_HS400_1_8V)
> +			card_mask |= MMC_SIGNAL_VOLTAGE_180;
> +		if (mmc->cardtype & EXT_CSD_CARD_TYPE_HS400_1_2V)
> +			card_mask |= MMC_SIGNAL_VOLTAGE_120;
> +		break;
>   	case MMC_HS_200:
>   		if (mmc->cardtype & EXT_CSD_CARD_TYPE_HS200_1_8V)
>   			card_mask |= MMC_SIGNAL_VOLTAGE_180;
> @@ -1787,6 +1806,13 @@ static inline int mmc_set_lowest_voltage(struct mmc *mmc, enum bus_mode mode,
>   #endif
>   
>   static const struct mode_width_tuning mmc_modes_by_pref[] = {
> +#if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
> +	{
> +		.mode = MMC_HS_400,
> +		.widths = MMC_MODE_8BIT | MMC_MODE_4BIT,
> +		.tuning = MMC_CMD_SEND_TUNING_BLOCK_HS200
> +	},
> +#endif
>   #if CONFIG_IS_ENABLED(MMC_HS200_SUPPORT)
>   	{
>   		.mode = MMC_HS_200,
> @@ -1830,6 +1856,54 @@ static const struct ext_csd_bus_width {
>   	{MMC_MODE_1BIT, false, EXT_CSD_BUS_WIDTH_1},
>   };
>   
> +#if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
> +static int mmc_select_hs400(struct mmc *mmc)
> +{
> +	int err;
> +
> +	/* Set timing to HS200 for tuning */
> +	err = mmc_set_card_speed(mmc, MMC_HS_200);
> +	if (err)
> +		return err;
> +
> +	/* configure the bus mode (host) */
> +	mmc_select_mode(mmc, MMC_HS_200);
> +	mmc_set_clock(mmc, mmc->tran_speed, false);
> +
> +	/* execute tuning if needed */
> +	err = mmc_execute_tuning(mmc, MMC_CMD_SEND_TUNING_BLOCK_HS200);
> +	if (err) {
> +		debug("tuning failed\n");
> +		return err;
> +	}
> +
> +	/* Set back to HS */
> +	mmc_set_card_speed(mmc, MMC_HS);
> +	mmc_set_clock(mmc, mmc_mode2freq(mmc, MMC_HS), false);
> +
> +	err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_BUS_WIDTH,
> +			 EXT_CSD_BUS_WIDTH_8 | EXT_CSD_DDR_FLAG);
What happens if only 4 wires are used. It is a legit mode, isn't it ?
> +	if (err)
> +		return err;
> +
> +	err = mmc_set_card_speed(mmc, MMC_HS_400);
> +	if (err)
> +		return err;
> +
> +	mmc_select_mode(mmc, MMC_HS_400);
> +	err = mmc_set_clock(mmc, mmc->tran_speed, false);
> +	if (err)
> +		return err;
> +
> +	return 0;
> +}
> +#else
> +static int mmc_select_hs400(struct mmc *mmc)
> +{
> +	return -ENOTSUPP;
> +}
> +#endif
> +
>   #define for_each_supported_width(caps, ddr, ecbv) \
>   	for (ecbv = ext_csd_bus_width;\
>   	    ecbv < ext_csd_bus_width + ARRAY_SIZE(ext_csd_bus_width);\
> @@ -1883,37 +1957,46 @@ static int mmc_select_mode_and_width(struct mmc *mmc, uint card_caps)
>   				goto error;
>   			mmc_set_bus_width(mmc, bus_width(ecbw->cap));
>   
> -			/* configure the bus speed (card) */
> -			err = mmc_set_card_speed(mmc, mwt->mode);
> -			if (err)
> -				goto error;
> -
> -			/*
> -			 * configure the bus width AND the ddr mode (card)
> -			 * The host side will be taken care of in the next step
> -			 */
> -			if (ecbw->ext_csd_bits & EXT_CSD_DDR_FLAG) {
> -				err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL,
> -						 EXT_CSD_BUS_WIDTH,
> -						 ecbw->ext_csd_bits);
> +			if (mwt->mode == MMC_HS_400) {
> +				err = mmc_select_hs400(mmc);
> +				if (err)
> +					goto error;
> +			} else {
> +				/* configure the bus speed (card) */
> +				err = mmc_set_card_speed(mmc, mwt->mode);
Instead of having  a separate mmc_select_hs400() is not possible to 
leverage the existing code ?

JJ

>   				if (err)
>   					goto error;
> -			}
>   
> -			/* configure the bus mode (host) */
> -			mmc_select_mode(mmc, mwt->mode);
> -			mmc_set_clock(mmc, mmc->tran_speed, false);
> +				/*
> +				 * configure the bus width AND the ddr mode
> +				 * (card). The host side will be taken care
> +				 * of in the next step
> +				 */
> +				if (ecbw->ext_csd_bits & EXT_CSD_DDR_FLAG) {
> +					err = mmc_switch(mmc,
> +							 EXT_CSD_CMD_SET_NORMAL,
> +							 EXT_CSD_BUS_WIDTH,
> +							 ecbw->ext_csd_bits);
> +					if (err)
> +						goto error;
> +				}
> +
> +				/* configure the bus mode (host) */
> +				mmc_select_mode(mmc, mwt->mode);
> +				mmc_set_clock(mmc, mmc->tran_speed, false);
>   #ifdef MMC_SUPPORTS_TUNING
>   
> -			/* execute tuning if needed */
> -			if (mwt->tuning) {
> -				err = mmc_execute_tuning(mmc, mwt->tuning);
> -				if (err) {
> -					pr_debug("tuning failed\n");
> -					goto error;
> +				/* execute tuning if needed */
> +				if (mwt->tuning) {
> +					err = mmc_execute_tuning(mmc,
> +								 mwt->tuning);
> +					if (err) {
> +						pr_debug("tuning failed\n");
> +						goto error;
> +					}
>   				}
> -			}
>   #endif
> +			}
>   
>   			/* do a transfer to check the configuration */
>   			err = mmc_read_and_compare_ext_csd(mmc);
> diff --git a/include/mmc.h b/include/mmc.h
> index 86f885b504..8c01c6a530 100644
> --- a/include/mmc.h
> +++ b/include/mmc.h
> @@ -65,6 +65,7 @@
>   #define MMC_MODE_HS_52MHz	MMC_CAP(MMC_HS_52)
>   #define MMC_MODE_DDR_52MHz	MMC_CAP(MMC_DDR_52)
>   #define MMC_MODE_HS200		MMC_CAP(MMC_HS_200)
> +#define MMC_MODE_HS400		MMC_CAP(MMC_HS_400)
>   
>   #define MMC_MODE_8BIT		BIT(30)
>   #define MMC_MODE_4BIT		BIT(29)
> @@ -250,6 +251,11 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx)
>   #define EXT_CSD_CARD_TYPE_HS200		(EXT_CSD_CARD_TYPE_HS200_1_8V | \
>   					 EXT_CSD_CARD_TYPE_HS200_1_2V)
>   
> +#define EXT_CSD_CARD_TYPE_HS400_1_8V	BIT(6)
> +#define EXT_CSD_CARD_TYPE_HS400_1_2V	BIT(7)
> +#define EXT_CSD_CARD_TYPE_HS400		(EXT_CSD_CARD_TYPE_HS400_1_8V | \
> +					 EXT_CSD_CARD_TYPE_HS400_1_2V)
> +
>   #define EXT_CSD_BUS_WIDTH_1	0	/* Card is in 1 bit mode */
>   #define EXT_CSD_BUS_WIDTH_4	1	/* Card is in 4 bit mode */
>   #define EXT_CSD_BUS_WIDTH_8	2	/* Card is in 8 bit mode */
> @@ -260,6 +266,7 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx)
>   #define EXT_CSD_TIMING_LEGACY	0	/* no high speed */
>   #define EXT_CSD_TIMING_HS	1	/* HS */
>   #define EXT_CSD_TIMING_HS200	2	/* HS200 */
> +#define EXT_CSD_TIMING_HS400	3	/* HS400 */
>   
>   #define EXT_CSD_BOOT_ACK_ENABLE			(1 << 6)
>   #define EXT_CSD_BOOT_PARTITION_ENABLE		(1 << 3)
> @@ -520,6 +527,7 @@ enum bus_mode {
>   	UHS_DDR50,
>   	UHS_SDR104,
>   	MMC_HS_200,
> +	MMC_HS_400,
>   	MMC_MODES_END
>   };
>   
> @@ -533,6 +541,10 @@ static inline bool mmc_is_mode_ddr(enum bus_mode mode)
>   #if CONFIG_IS_ENABLED(MMC_UHS_SUPPORT)
>   	else if (mode == UHS_DDR50)
>   		return true;
> +#endif
> +#if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
> +	else if (mode == MMC_HS_400)
> +		return true;
>   #endif
>   	else
>   		return false;

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH 1/2] mmc: add HS400 support
  2018-03-05 16:29 ` [U-Boot] [PATCH 1/2] mmc: add HS400 support Jean-Jacques Hiblot
@ 2018-03-06  1:46   ` Peng Fan
  2018-03-06 14:35     ` Jean-Jacques Hiblot
  0 siblings, 1 reply; 10+ messages in thread
From: Peng Fan @ 2018-03-06  1:46 UTC (permalink / raw)
  To: u-boot

Hi, 
On Mon, Mar 05, 2018 at 05:29:08PM +0100, Jean-Jacques Hiblot wrote:
>Hi Peng,
>
>I'm glad you are adding HS400 support. Thanks.
>
>
>On 05/03/2018 10:11, Peng Fan wrote:
>>Add HS400 support.
>>Selecting HS400 needs first select HS199 according to spec, so use
>>a dedicated function for HS400.
>>Add HS400 related macros.
>>Remove the restriction of only using the low 6 bits of
>>EXT_CSD_CARD_TYPE, using all the 8 bits.
>>
>>Signed-off-by: Peng Fan <peng.fan@nxp.com>
>>Cc: Jaehoon Chung <jh80.chung@samsung.com>
>>Cc: Jean-Jacques Hiblot <jjhiblot@ti.com>
>>Cc: Stefano Babic <sbabic@denx.de>
>>Cc: Simon Glass <sjg@chromium.org>
>>Cc: Kishon Vijay Abraham I <kishon@ti.com>
>>Cc: Bin Meng <bmeng.cn@gmail.com>
>>---
>>  drivers/mmc/Kconfig |   7 +++
>>  drivers/mmc/mmc.c   | 133 ++++++++++++++++++++++++++++++++++++++++++----------
>>  include/mmc.h       |  12 +++++
>>  3 files changed, 127 insertions(+), 25 deletions(-)
>>
>>diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
>>index 5f67e336db..e9be18b333 100644
>>--- a/drivers/mmc/Kconfig
>>+++ b/drivers/mmc/Kconfig
>>@@ -104,6 +104,13 @@ config SPL_MMC_UHS_SUPPORT
>>  	  cards. The IO voltage must be switchable from 3.3v to 1.8v. The bus
>>  	  frequency can go up to 208MHz (SDR104)
>>+config MMC_HS400_SUPPORT
>>+	bool "enable HS400 support"
>>+	select MMC_HS200_SUPPORT
>I'd use "depends on" instead of select or maybe use the same option
>for both HS200 and HS400
>>+	help
>>+	  The HS400 mode is support by some eMMC. The bus frequency is up to
>>+	  200MHz. This mode requires tuning the IO.
>>+
>>  config MMC_HS200_SUPPORT
>>  	bool "enable HS200 support"
>>  	help
>>diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
>>index 92ea78b8af..eef229c8b4 100644
>>--- a/drivers/mmc/mmc.c
>>+++ b/drivers/mmc/mmc.c
>>@@ -169,6 +169,7 @@ const char *mmc_mode_name(enum bus_mode mode)
>>  	      [MMC_HS_52]	= "MMC High Speed (52MHz)",
>>  	      [MMC_DDR_52]	= "MMC DDR52 (52MHz)",
>>  	      [MMC_HS_200]	= "HS200 (200MHz)",
>>+	      [MMC_HS_400]	= "HS400 (200MHz)",
>>  	};
>>  	if (mode >= MMC_MODES_END)
>>@@ -193,6 +194,7 @@ static uint mmc_mode2freq(struct mmc *mmc, enum bus_mode mode)
>>  	      [UHS_DDR50]	= 50000000,
>>  	      [UHS_SDR104]	= 208000000,
>>  	      [MMC_HS_200]	= 200000000,
>>+	      [MMC_HS_400]	= 200000000,
>>  	};
>>  	if (mode == MMC_LEGACY)
>>@@ -790,6 +792,11 @@ static int mmc_set_card_speed(struct mmc *mmc, enum bus_mode mode)
>>  	case MMC_HS_200:
>>  		speed_bits = EXT_CSD_TIMING_HS200;
>>  		break;
>>+#endif
>>+#if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
>>+	case MMC_HS_400:
>>+		speed_bits = EXT_CSD_TIMING_HS400;
>>+		break;
>>  #endif
>>  	case MMC_LEGACY:
>>  		speed_bits = EXT_CSD_TIMING_LEGACY;
>>@@ -837,7 +844,7 @@ static int mmc_get_capabilities(struct mmc *mmc)
>>  	mmc->card_caps |= MMC_MODE_4BIT | MMC_MODE_8BIT;
>>-	cardtype = ext_csd[EXT_CSD_CARD_TYPE] & 0x3f;
>>+	cardtype = ext_csd[EXT_CSD_CARD_TYPE];
>>  	mmc->cardtype = cardtype;
>>  #if CONFIG_IS_ENABLED(MMC_HS200_SUPPORT)
>>@@ -845,6 +852,12 @@ static int mmc_get_capabilities(struct mmc *mmc)
>>  			EXT_CSD_CARD_TYPE_HS200_1_8V)) {
>>  		mmc->card_caps |= MMC_MODE_HS200;
>>  	}
>>+#endif
>>+#if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
>>+	if (cardtype & (EXT_CSD_CARD_TYPE_HS400_1_2V |
>>+			EXT_CSD_CARD_TYPE_HS400_1_8V)) {
>>+		mmc->card_caps |= MMC_MODE_HS400;
>>+	}
>>  #endif
>>  	if (cardtype & EXT_CSD_CARD_TYPE_52) {
>>  		if (cardtype & EXT_CSD_CARD_TYPE_DDR_52)
>>@@ -1748,6 +1761,12 @@ static int mmc_set_lowest_voltage(struct mmc *mmc, enum bus_mode mode,
>>  	u32 card_mask = 0;
>>  	switch (mode) {
>>+	case MMC_HS_400:
>>+		if (mmc->cardtype & EXT_CSD_CARD_TYPE_HS400_1_8V)
>>+			card_mask |= MMC_SIGNAL_VOLTAGE_180;
>>+		if (mmc->cardtype & EXT_CSD_CARD_TYPE_HS400_1_2V)
>>+			card_mask |= MMC_SIGNAL_VOLTAGE_120;
>>+		break;
>>  	case MMC_HS_200:
>>  		if (mmc->cardtype & EXT_CSD_CARD_TYPE_HS200_1_8V)
>>  			card_mask |= MMC_SIGNAL_VOLTAGE_180;
>>@@ -1787,6 +1806,13 @@ static inline int mmc_set_lowest_voltage(struct mmc *mmc, enum bus_mode mode,
>>  #endif
>>  static const struct mode_width_tuning mmc_modes_by_pref[] = {
>>+#if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
>>+	{
>>+		.mode = MMC_HS_400,
>>+		.widths = MMC_MODE_8BIT | MMC_MODE_4BIT,
>>+		.tuning = MMC_CMD_SEND_TUNING_BLOCK_HS200
>>+	},
>>+#endif
>>  #if CONFIG_IS_ENABLED(MMC_HS200_SUPPORT)
>>  	{
>>  		.mode = MMC_HS_200,
>>@@ -1830,6 +1856,54 @@ static const struct ext_csd_bus_width {
>>  	{MMC_MODE_1BIT, false, EXT_CSD_BUS_WIDTH_1},
>>  };
>>+#if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
>>+static int mmc_select_hs400(struct mmc *mmc)
>>+{
>>+	int err;
>>+
>>+	/* Set timing to HS200 for tuning */
>>+	err = mmc_set_card_speed(mmc, MMC_HS_200);
>>+	if (err)
>>+		return err;
>>+
>>+	/* configure the bus mode (host) */
>>+	mmc_select_mode(mmc, MMC_HS_200);
>>+	mmc_set_clock(mmc, mmc->tran_speed, false);
>>+
>>+	/* execute tuning if needed */
>>+	err = mmc_execute_tuning(mmc, MMC_CMD_SEND_TUNING_BLOCK_HS200);
>>+	if (err) {
>>+		debug("tuning failed\n");
>>+		return err;
>>+	}
>>+
>>+	/* Set back to HS */
>>+	mmc_set_card_speed(mmc, MMC_HS);
>>+	mmc_set_clock(mmc, mmc_mode2freq(mmc, MMC_HS), false);
>>+
>>+	err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_BUS_WIDTH,
>>+			 EXT_CSD_BUS_WIDTH_8 | EXT_CSD_DDR_FLAG);
>What happens if only 4 wires are used. It is a legit mode, isn't it ?

According to spec, HS400 only supports 8bits mode, see "5.3.6 HS400 Bus Speed Mode".

>>+	if (err)
>>+		return err;
>>+
>>+	err = mmc_set_card_speed(mmc, MMC_HS_400);
>>+	if (err)
>>+		return err;
>>+
>>+	mmc_select_mode(mmc, MMC_HS_400);
>>+	err = mmc_set_clock(mmc, mmc->tran_speed, false);
>>+	if (err)
>>+		return err;
>>+
>>+	return 0;
>>+}
>>+#else
>>+static int mmc_select_hs400(struct mmc *mmc)
>>+{
>>+	return -ENOTSUPP;
>>+}
>>+#endif
>>+
>>  #define for_each_supported_width(caps, ddr, ecbv) \
>>  	for (ecbv = ext_csd_bus_width;\
>>  	    ecbv < ext_csd_bus_width + ARRAY_SIZE(ext_csd_bus_width);\
>>@@ -1883,37 +1957,46 @@ static int mmc_select_mode_and_width(struct mmc *mmc, uint card_caps)
>>  				goto error;
>>  			mmc_set_bus_width(mmc, bus_width(ecbw->cap));
>>-			/* configure the bus speed (card) */
>>-			err = mmc_set_card_speed(mmc, mwt->mode);
>>-			if (err)
>>-				goto error;
>>-
>>-			/*
>>-			 * configure the bus width AND the ddr mode (card)
>>-			 * The host side will be taken care of in the next step
>>-			 */
>>-			if (ecbw->ext_csd_bits & EXT_CSD_DDR_FLAG) {
>>-				err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL,
>>-						 EXT_CSD_BUS_WIDTH,
>>-						 ecbw->ext_csd_bits);
>>+			if (mwt->mode == MMC_HS_400) {
>>+				err = mmc_select_hs400(mmc);
>>+				if (err)
>>+					goto error;
>>+			} else {
>>+				/* configure the bus speed (card) */
>>+				err = mmc_set_card_speed(mmc, mwt->mode);
>Instead of having?? a separate mmc_select_hs400() is not possible to
>leverage the existing code ?

hs400 requires first runs into HS200 mode, after tuning finished,
need switch back to HS mode to configure bus. It's not that straightforward
to use the existing code. Use a dedicated mmc_select_hs400 will make it
clearer compared with adding more logic in the existing flow.

Thanks,
Peng.

>
>JJ
>
>>  				if (err)
>>  					goto error;
>>-			}
>>-			/* configure the bus mode (host) */
>>-			mmc_select_mode(mmc, mwt->mode);
>>-			mmc_set_clock(mmc, mmc->tran_speed, false);
>>+				/*
>>+				 * configure the bus width AND the ddr mode
>>+				 * (card). The host side will be taken care
>>+				 * of in the next step
>>+				 */
>>+				if (ecbw->ext_csd_bits & EXT_CSD_DDR_FLAG) {
>>+					err = mmc_switch(mmc,
>>+							 EXT_CSD_CMD_SET_NORMAL,
>>+							 EXT_CSD_BUS_WIDTH,
>>+							 ecbw->ext_csd_bits);
>>+					if (err)
>>+						goto error;
>>+				}
>>+
>>+				/* configure the bus mode (host) */
>>+				mmc_select_mode(mmc, mwt->mode);
>>+				mmc_set_clock(mmc, mmc->tran_speed, false);
>>  #ifdef MMC_SUPPORTS_TUNING
>>-			/* execute tuning if needed */
>>-			if (mwt->tuning) {
>>-				err = mmc_execute_tuning(mmc, mwt->tuning);
>>-				if (err) {
>>-					pr_debug("tuning failed\n");
>>-					goto error;
>>+				/* execute tuning if needed */
>>+				if (mwt->tuning) {
>>+					err = mmc_execute_tuning(mmc,
>>+								 mwt->tuning);
>>+					if (err) {
>>+						pr_debug("tuning failed\n");
>>+						goto error;
>>+					}
>>  				}
>>-			}
>>  #endif
>>+			}
>>  			/* do a transfer to check the configuration */
>>  			err = mmc_read_and_compare_ext_csd(mmc);
>>diff --git a/include/mmc.h b/include/mmc.h
>>index 86f885b504..8c01c6a530 100644
>>--- a/include/mmc.h
>>+++ b/include/mmc.h
>>@@ -65,6 +65,7 @@
>>  #define MMC_MODE_HS_52MHz	MMC_CAP(MMC_HS_52)
>>  #define MMC_MODE_DDR_52MHz	MMC_CAP(MMC_DDR_52)
>>  #define MMC_MODE_HS200		MMC_CAP(MMC_HS_200)
>>+#define MMC_MODE_HS400		MMC_CAP(MMC_HS_400)
>>  #define MMC_MODE_8BIT		BIT(30)
>>  #define MMC_MODE_4BIT		BIT(29)
>>@@ -250,6 +251,11 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx)
>>  #define EXT_CSD_CARD_TYPE_HS200		(EXT_CSD_CARD_TYPE_HS200_1_8V | \
>>  					 EXT_CSD_CARD_TYPE_HS200_1_2V)
>>+#define EXT_CSD_CARD_TYPE_HS400_1_8V	BIT(6)
>>+#define EXT_CSD_CARD_TYPE_HS400_1_2V	BIT(7)
>>+#define EXT_CSD_CARD_TYPE_HS400		(EXT_CSD_CARD_TYPE_HS400_1_8V | \
>>+					 EXT_CSD_CARD_TYPE_HS400_1_2V)
>>+
>>  #define EXT_CSD_BUS_WIDTH_1	0	/* Card is in 1 bit mode */
>>  #define EXT_CSD_BUS_WIDTH_4	1	/* Card is in 4 bit mode */
>>  #define EXT_CSD_BUS_WIDTH_8	2	/* Card is in 8 bit mode */
>>@@ -260,6 +266,7 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx)
>>  #define EXT_CSD_TIMING_LEGACY	0	/* no high speed */
>>  #define EXT_CSD_TIMING_HS	1	/* HS */
>>  #define EXT_CSD_TIMING_HS200	2	/* HS200 */
>>+#define EXT_CSD_TIMING_HS400	3	/* HS400 */
>>  #define EXT_CSD_BOOT_ACK_ENABLE			(1 << 6)
>>  #define EXT_CSD_BOOT_PARTITION_ENABLE		(1 << 3)
>>@@ -520,6 +527,7 @@ enum bus_mode {
>>  	UHS_DDR50,
>>  	UHS_SDR104,
>>  	MMC_HS_200,
>>+	MMC_HS_400,
>>  	MMC_MODES_END
>>  };
>>@@ -533,6 +541,10 @@ static inline bool mmc_is_mode_ddr(enum bus_mode mode)
>>  #if CONFIG_IS_ENABLED(MMC_UHS_SUPPORT)
>>  	else if (mode == UHS_DDR50)
>>  		return true;
>>+#endif
>>+#if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
>>+	else if (mode == MMC_HS_400)
>>+		return true;
>>  #endif
>>  	else
>>  		return false;
>
>_______________________________________________
>U-Boot mailing list
>U-Boot at lists.denx.de
>https://lists.denx.de/listinfo/u-boot

-- 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH 1/2] mmc: add HS400 support
  2018-03-06  1:46   ` Peng Fan
@ 2018-03-06 14:35     ` Jean-Jacques Hiblot
  2018-03-07  2:07       ` Peng Fan
  0 siblings, 1 reply; 10+ messages in thread
From: Jean-Jacques Hiblot @ 2018-03-06 14:35 UTC (permalink / raw)
  To: u-boot



On 06/03/2018 02:46, Peng Fan wrote:
> Hi,
> On Mon, Mar 05, 2018 at 05:29:08PM +0100, Jean-Jacques Hiblot wrote:
>> Hi Peng,
>>
>> I'm glad you are adding HS400 support. Thanks.
>>
>>
>> On 05/03/2018 10:11, Peng Fan wrote:
>>> Add HS400 support.
>>> Selecting HS400 needs first select HS199 according to spec, so use
>>> a dedicated function for HS400.
>>> Add HS400 related macros.
>>> Remove the restriction of only using the low 6 bits of
>>> EXT_CSD_CARD_TYPE, using all the 8 bits.
>>>
>>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>>> Cc: Jaehoon Chung <jh80.chung@samsung.com>
>>> Cc: Jean-Jacques Hiblot <jjhiblot@ti.com>
>>> Cc: Stefano Babic <sbabic@denx.de>
>>> Cc: Simon Glass <sjg@chromium.org>
>>> Cc: Kishon Vijay Abraham I <kishon@ti.com>
>>> Cc: Bin Meng <bmeng.cn@gmail.com>
>>> ---
>>>   drivers/mmc/Kconfig |   7 +++
>>>   drivers/mmc/mmc.c   | 133 ++++++++++++++++++++++++++++++++++++++++++----------
>>>   include/mmc.h       |  12 +++++
>>>   3 files changed, 127 insertions(+), 25 deletions(-)
>>>
>>> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
>>> index 5f67e336db..e9be18b333 100644
>>> --- a/drivers/mmc/Kconfig
>>> +++ b/drivers/mmc/Kconfig
>>> @@ -104,6 +104,13 @@ config SPL_MMC_UHS_SUPPORT
>>>   	  cards. The IO voltage must be switchable from 3.3v to 1.8v. The bus
>>>   	  frequency can go up to 208MHz (SDR104)
>>> +config MMC_HS400_SUPPORT
>>> +	bool "enable HS400 support"
>>> +	select MMC_HS200_SUPPORT
>> I'd use "depends on" instead of select or maybe use the same option
>> for both HS200 and HS400
>>> +	help
>>> +	  The HS400 mode is support by some eMMC. The bus frequency is up to
>>> +	  200MHz. This mode requires tuning the IO.
>>> +
>>>   config MMC_HS200_SUPPORT
>>>   	bool "enable HS200 support"
>>>   	help
>>> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
>>> index 92ea78b8af..eef229c8b4 100644
>>> --- a/drivers/mmc/mmc.c
>>> +++ b/drivers/mmc/mmc.c
>>> @@ -169,6 +169,7 @@ const char *mmc_mode_name(enum bus_mode mode)
>>>   	      [MMC_HS_52]	= "MMC High Speed (52MHz)",
>>>   	      [MMC_DDR_52]	= "MMC DDR52 (52MHz)",
>>>   	      [MMC_HS_200]	= "HS200 (200MHz)",
>>> +	      [MMC_HS_400]	= "HS400 (200MHz)",
>>>   	};
>>>   	if (mode >= MMC_MODES_END)
>>> @@ -193,6 +194,7 @@ static uint mmc_mode2freq(struct mmc *mmc, enum bus_mode mode)
>>>   	      [UHS_DDR50]	= 50000000,
>>>   	      [UHS_SDR104]	= 208000000,
>>>   	      [MMC_HS_200]	= 200000000,
>>> +	      [MMC_HS_400]	= 200000000,
>>>   	};
>>>   	if (mode == MMC_LEGACY)
>>> @@ -790,6 +792,11 @@ static int mmc_set_card_speed(struct mmc *mmc, enum bus_mode mode)
>>>   	case MMC_HS_200:
>>>   		speed_bits = EXT_CSD_TIMING_HS200;
>>>   		break;
>>> +#endif
>>> +#if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
>>> +	case MMC_HS_400:
>>> +		speed_bits = EXT_CSD_TIMING_HS400;
>>> +		break;
>>>   #endif
>>>   	case MMC_LEGACY:
>>>   		speed_bits = EXT_CSD_TIMING_LEGACY;
>>> @@ -837,7 +844,7 @@ static int mmc_get_capabilities(struct mmc *mmc)
>>>   	mmc->card_caps |= MMC_MODE_4BIT | MMC_MODE_8BIT;
>>> -	cardtype = ext_csd[EXT_CSD_CARD_TYPE] & 0x3f;
>>> +	cardtype = ext_csd[EXT_CSD_CARD_TYPE];
>>>   	mmc->cardtype = cardtype;
>>>   #if CONFIG_IS_ENABLED(MMC_HS200_SUPPORT)
>>> @@ -845,6 +852,12 @@ static int mmc_get_capabilities(struct mmc *mmc)
>>>   			EXT_CSD_CARD_TYPE_HS200_1_8V)) {
>>>   		mmc->card_caps |= MMC_MODE_HS200;
>>>   	}
>>> +#endif
>>> +#if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
>>> +	if (cardtype & (EXT_CSD_CARD_TYPE_HS400_1_2V |
>>> +			EXT_CSD_CARD_TYPE_HS400_1_8V)) {
>>> +		mmc->card_caps |= MMC_MODE_HS400;
>>> +	}
>>>   #endif
>>>   	if (cardtype & EXT_CSD_CARD_TYPE_52) {
>>>   		if (cardtype & EXT_CSD_CARD_TYPE_DDR_52)
>>> @@ -1748,6 +1761,12 @@ static int mmc_set_lowest_voltage(struct mmc *mmc, enum bus_mode mode,
>>>   	u32 card_mask = 0;
>>>   	switch (mode) {
>>> +	case MMC_HS_400:
>>> +		if (mmc->cardtype & EXT_CSD_CARD_TYPE_HS400_1_8V)
>>> +			card_mask |= MMC_SIGNAL_VOLTAGE_180;
>>> +		if (mmc->cardtype & EXT_CSD_CARD_TYPE_HS400_1_2V)
>>> +			card_mask |= MMC_SIGNAL_VOLTAGE_120;
>>> +		break;
>>>   	case MMC_HS_200:
>>>   		if (mmc->cardtype & EXT_CSD_CARD_TYPE_HS200_1_8V)
>>>   			card_mask |= MMC_SIGNAL_VOLTAGE_180;
>>> @@ -1787,6 +1806,13 @@ static inline int mmc_set_lowest_voltage(struct mmc *mmc, enum bus_mode mode,
>>>   #endif
>>>   static const struct mode_width_tuning mmc_modes_by_pref[] = {
>>> +#if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
>>> +	{
>>> +		.mode = MMC_HS_400,
>>> +		.widths = MMC_MODE_8BIT | MMC_MODE_4BIT,
>>> +		.tuning = MMC_CMD_SEND_TUNING_BLOCK_HS200
>>> +	},
>>> +#endif
>>>   #if CONFIG_IS_ENABLED(MMC_HS200_SUPPORT)
>>>   	{
>>>   		.mode = MMC_HS_200,
>>> @@ -1830,6 +1856,54 @@ static const struct ext_csd_bus_width {
>>>   	{MMC_MODE_1BIT, false, EXT_CSD_BUS_WIDTH_1},
>>>   };
>>> +#if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
>>> +static int mmc_select_hs400(struct mmc *mmc)
>>> +{
>>> +	int err;
>>> +
>>> +	/* Set timing to HS200 for tuning */
>>> +	err = mmc_set_card_speed(mmc, MMC_HS_200);
>>> +	if (err)
>>> +		return err;
>>> +
>>> +	/* configure the bus mode (host) */
>>> +	mmc_select_mode(mmc, MMC_HS_200);
>>> +	mmc_set_clock(mmc, mmc->tran_speed, false);
>>> +
>>> +	/* execute tuning if needed */
>>> +	err = mmc_execute_tuning(mmc, MMC_CMD_SEND_TUNING_BLOCK_HS200);
>>> +	if (err) {
>>> +		debug("tuning failed\n");
>>> +		return err;
>>> +	}
>>> +
>>> +	/* Set back to HS */
>>> +	mmc_set_card_speed(mmc, MMC_HS);
>>> +	mmc_set_clock(mmc, mmc_mode2freq(mmc, MMC_HS), false);
>>> +
>>> +	err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_BUS_WIDTH,
>>> +			 EXT_CSD_BUS_WIDTH_8 | EXT_CSD_DDR_FLAG);
>> What happens if only 4 wires are used. It is a legit mode, isn't it ?
> According to spec, HS400 only supports 8bits mode, see "5.3.6 HS400 Bus Speed Mode".
Good. That makes it easier.
Can you add something to remove HS400 from the host capabilities if the 
8-wires configuration is not available ?

>
>>> +	if (err)
>>> +		return err;
>>> +
>>> +	err = mmc_set_card_speed(mmc, MMC_HS_400);
>>> +	if (err)
>>> +		return err;
>>> +
>>> +	mmc_select_mode(mmc, MMC_HS_400);
>>> +	err = mmc_set_clock(mmc, mmc->tran_speed, false);
>>> +	if (err)
>>> +		return err;
>>> +
>>> +	return 0;
>>> +}
>>> +#else
>>> +static int mmc_select_hs400(struct mmc *mmc)
>>> +{
>>> +	return -ENOTSUPP;
>>> +}
>>> +#endif
>>> +
>>>   #define for_each_supported_width(caps, ddr, ecbv) \
>>>   	for (ecbv = ext_csd_bus_width;\
>>>   	    ecbv < ext_csd_bus_width + ARRAY_SIZE(ext_csd_bus_width);\
>>> @@ -1883,37 +1957,46 @@ static int mmc_select_mode_and_width(struct mmc *mmc, uint card_caps)
>>>   				goto error;
>>>   			mmc_set_bus_width(mmc, bus_width(ecbw->cap));
>>> -			/* configure the bus speed (card) */
>>> -			err = mmc_set_card_speed(mmc, mwt->mode);
>>> -			if (err)
>>> -				goto error;
>>> -
>>> -			/*
>>> -			 * configure the bus width AND the ddr mode (card)
>>> -			 * The host side will be taken care of in the next step
>>> -			 */
>>> -			if (ecbw->ext_csd_bits & EXT_CSD_DDR_FLAG) {
>>> -				err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL,
>>> -						 EXT_CSD_BUS_WIDTH,
>>> -						 ecbw->ext_csd_bits);
>>> +			if (mwt->mode == MMC_HS_400) {
>>> +				err = mmc_select_hs400(mmc);
>>> +				if (err)
>>> +					goto error;
>>> +			} else {
>>> +				/* configure the bus speed (card) */
>>> +				err = mmc_set_card_speed(mmc, mwt->mode);
>> Instead of having?? a separate mmc_select_hs400() is not possible to
>> leverage the existing code ?
> hs400 requires first runs into HS200 mode, after tuning finished,
> need switch back to HS mode to configure bus. It's not that straightforward
> to use the existing code. Use a dedicated mmc_select_hs400 will make it
> clearer compared with adding more logic in the existing flow.
OK
>
> Thanks,
> Peng.
>
>> JJ
>>
>>>   				if (err)
>>>   					goto error;
>>> -			}
>>> -			/* configure the bus mode (host) */
>>> -			mmc_select_mode(mmc, mwt->mode);
>>> -			mmc_set_clock(mmc, mmc->tran_speed, false);
>>> +				/*
>>> +				 * configure the bus width AND the ddr mode
>>> +				 * (card). The host side will be taken care
>>> +				 * of in the next step
>>> +				 */
>>> +				if (ecbw->ext_csd_bits & EXT_CSD_DDR_FLAG) {
>>> +					err = mmc_switch(mmc,
>>> +							 EXT_CSD_CMD_SET_NORMAL,
>>> +							 EXT_CSD_BUS_WIDTH,
>>> +							 ecbw->ext_csd_bits);
>>> +					if (err)
>>> +						goto error;
>>> +				}
>>> +
>>> +				/* configure the bus mode (host) */
>>> +				mmc_select_mode(mmc, mwt->mode);
>>> +				mmc_set_clock(mmc, mmc->tran_speed, false);
>>>   #ifdef MMC_SUPPORTS_TUNING
>>> -			/* execute tuning if needed */
>>> -			if (mwt->tuning) {
>>> -				err = mmc_execute_tuning(mmc, mwt->tuning);
>>> -				if (err) {
>>> -					pr_debug("tuning failed\n");
>>> -					goto error;
>>> +				/* execute tuning if needed */
>>> +				if (mwt->tuning) {
>>> +					err = mmc_execute_tuning(mmc,
>>> +								 mwt->tuning);
>>> +					if (err) {
>>> +						pr_debug("tuning failed\n");
>>> +						goto error;
>>> +					}
>>>   				}
>>> -			}
>>>   #endif
>>> +			}
>>>   			/* do a transfer to check the configuration */
>>>   			err = mmc_read_and_compare_ext_csd(mmc);
>>> diff --git a/include/mmc.h b/include/mmc.h
>>> index 86f885b504..8c01c6a530 100644
>>> --- a/include/mmc.h
>>> +++ b/include/mmc.h
>>> @@ -65,6 +65,7 @@
>>>   #define MMC_MODE_HS_52MHz	MMC_CAP(MMC_HS_52)
>>>   #define MMC_MODE_DDR_52MHz	MMC_CAP(MMC_DDR_52)
>>>   #define MMC_MODE_HS200		MMC_CAP(MMC_HS_200)
>>> +#define MMC_MODE_HS400		MMC_CAP(MMC_HS_400)
>>>   #define MMC_MODE_8BIT		BIT(30)
>>>   #define MMC_MODE_4BIT		BIT(29)
>>> @@ -250,6 +251,11 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx)
>>>   #define EXT_CSD_CARD_TYPE_HS200		(EXT_CSD_CARD_TYPE_HS200_1_8V | \
>>>   					 EXT_CSD_CARD_TYPE_HS200_1_2V)
>>> +#define EXT_CSD_CARD_TYPE_HS400_1_8V	BIT(6)
>>> +#define EXT_CSD_CARD_TYPE_HS400_1_2V	BIT(7)
>>> +#define EXT_CSD_CARD_TYPE_HS400		(EXT_CSD_CARD_TYPE_HS400_1_8V | \
>>> +					 EXT_CSD_CARD_TYPE_HS400_1_2V)
>>> +
>>>   #define EXT_CSD_BUS_WIDTH_1	0	/* Card is in 1 bit mode */
>>>   #define EXT_CSD_BUS_WIDTH_4	1	/* Card is in 4 bit mode */
>>>   #define EXT_CSD_BUS_WIDTH_8	2	/* Card is in 8 bit mode */
>>> @@ -260,6 +266,7 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx)
>>>   #define EXT_CSD_TIMING_LEGACY	0	/* no high speed */
>>>   #define EXT_CSD_TIMING_HS	1	/* HS */
>>>   #define EXT_CSD_TIMING_HS200	2	/* HS200 */
>>> +#define EXT_CSD_TIMING_HS400	3	/* HS400 */
>>>   #define EXT_CSD_BOOT_ACK_ENABLE			(1 << 6)
>>>   #define EXT_CSD_BOOT_PARTITION_ENABLE		(1 << 3)
>>> @@ -520,6 +527,7 @@ enum bus_mode {
>>>   	UHS_DDR50,
>>>   	UHS_SDR104,
>>>   	MMC_HS_200,
>>> +	MMC_HS_400,
>>>   	MMC_MODES_END
>>>   };
>>> @@ -533,6 +541,10 @@ static inline bool mmc_is_mode_ddr(enum bus_mode mode)
>>>   #if CONFIG_IS_ENABLED(MMC_UHS_SUPPORT)
>>>   	else if (mode == UHS_DDR50)
>>>   		return true;
>>> +#endif
>>> +#if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
>>> +	else if (mode == MMC_HS_400)
>>> +		return true;
>>>   #endif
>>>   	else
>>>   		return false;
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> https://lists.denx.de/listinfo/u-boot

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH 1/2] mmc: add HS400 support
  2018-03-05  9:11 [U-Boot] [PATCH 1/2] mmc: add HS400 support Peng Fan
  2018-03-05  9:11 ` [U-Boot] [PATCH 2/2] mmc: fsl_esdhc: enable HS400 feature Peng Fan
  2018-03-05 16:29 ` [U-Boot] [PATCH 1/2] mmc: add HS400 support Jean-Jacques Hiblot
@ 2018-03-06 17:41 ` Simon Glass
  2018-03-07  2:12   ` Peng Fan
       [not found] ` <CGME20180426103029epcas1p4f0292a99571b6970f61d67ff42728fb0@epcas1p4.samsung.com>
  3 siblings, 1 reply; 10+ messages in thread
From: Simon Glass @ 2018-03-06 17:41 UTC (permalink / raw)
  To: u-boot

Hi Peng,

On 5 March 2018 at 02:11, Peng Fan <peng.fan@nxp.com> wrote:
> Add HS400 support.
> Selecting HS400 needs first select HS199 according to spec, so use
> a dedicated function for HS400.
> Add HS400 related macros.
> Remove the restriction of only using the low 6 bits of
> EXT_CSD_CARD_TYPE, using all the 8 bits.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> Cc: Jaehoon Chung <jh80.chung@samsung.com>
> Cc: Jean-Jacques Hiblot <jjhiblot@ti.com>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Kishon Vijay Abraham I <kishon@ti.com>
> Cc: Bin Meng <bmeng.cn@gmail.com>
> ---
>  drivers/mmc/Kconfig |   7 +++
>  drivers/mmc/mmc.c   | 133 ++++++++++++++++++++++++++++++++++++++++++----------
>  include/mmc.h       |  12 +++++
>  3 files changed, 127 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
> index 5f67e336db..e9be18b333 100644
> --- a/drivers/mmc/Kconfig
> +++ b/drivers/mmc/Kconfig
> @@ -104,6 +104,13 @@ config SPL_MMC_UHS_SUPPORT
>           cards. The IO voltage must be switchable from 3.3v to 1.8v. The bus
>           frequency can go up to 208MHz (SDR104)
>
> +config MMC_HS400_SUPPORT
> +       bool "enable HS400 support"
> +       select MMC_HS200_SUPPORT
> +       help
> +         The HS400 mode is support by some eMMC. The bus frequency is up to
> +         200MHz. This mode requires tuning the IO.
> +
>  config MMC_HS200_SUPPORT
>         bool "enable HS200 support"
>         help
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index 92ea78b8af..eef229c8b4 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -169,6 +169,7 @@ const char *mmc_mode_name(enum bus_mode mode)
>               [MMC_HS_52]       = "MMC High Speed (52MHz)",
>               [MMC_DDR_52]      = "MMC DDR52 (52MHz)",
>               [MMC_HS_200]      = "HS200 (200MHz)",
> +             [MMC_HS_400]      = "HS400 (200MHz)",
>         };
>
>         if (mode >= MMC_MODES_END)
> @@ -193,6 +194,7 @@ static uint mmc_mode2freq(struct mmc *mmc, enum bus_mode mode)
>               [UHS_DDR50]       = 50000000,
>               [UHS_SDR104]      = 208000000,
>               [MMC_HS_200]      = 200000000,
> +             [MMC_HS_400]      = 200000000,
>         };
>
>         if (mode == MMC_LEGACY)
> @@ -790,6 +792,11 @@ static int mmc_set_card_speed(struct mmc *mmc, enum bus_mode mode)
>         case MMC_HS_200:
>                 speed_bits = EXT_CSD_TIMING_HS200;
>                 break;
> +#endif
> +#if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
> +       case MMC_HS_400:
> +               speed_bits = EXT_CSD_TIMING_HS400;
> +               break;
>  #endif
>         case MMC_LEGACY:
>                 speed_bits = EXT_CSD_TIMING_LEGACY;
> @@ -837,7 +844,7 @@ static int mmc_get_capabilities(struct mmc *mmc)
>
>         mmc->card_caps |= MMC_MODE_4BIT | MMC_MODE_8BIT;
>
> -       cardtype = ext_csd[EXT_CSD_CARD_TYPE] & 0x3f;
> +       cardtype = ext_csd[EXT_CSD_CARD_TYPE];
>         mmc->cardtype = cardtype;
>
>  #if CONFIG_IS_ENABLED(MMC_HS200_SUPPORT)
> @@ -845,6 +852,12 @@ static int mmc_get_capabilities(struct mmc *mmc)
>                         EXT_CSD_CARD_TYPE_HS200_1_8V)) {
>                 mmc->card_caps |= MMC_MODE_HS200;
>         }
> +#endif
> +#if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)

Is it possible to use if () instead of #if

Regards,
Simon

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH 1/2] mmc: add HS400 support
  2018-03-06 14:35     ` Jean-Jacques Hiblot
@ 2018-03-07  2:07       ` Peng Fan
  0 siblings, 0 replies; 10+ messages in thread
From: Peng Fan @ 2018-03-07  2:07 UTC (permalink / raw)
  To: u-boot



> -----Original Message-----
> From: Jean-Jacques Hiblot [mailto:jjhiblot at ti.com]
> Sent: 2018年3月6日 22:35
> To: Peng Fan <van.freenix@gmail.com>
> Cc: Peng Fan <peng.fan@nxp.com>; jh80.chung at samsung.com;
> sbabic at denx.de; u-boot at lists.denx.de; Kishon Vijay Abraham I <kishon@ti.com>
> Subject: Re: [U-Boot] [PATCH 1/2] mmc: add HS400 support
> 
> 
> 
> On 06/03/2018 02:46, Peng Fan wrote:
> > Hi,
> > On Mon, Mar 05, 2018 at 05:29:08PM +0100, Jean-Jacques Hiblot wrote:
> >> Hi Peng,
> >>
> >> I'm glad you are adding HS400 support. Thanks.
> >>
> >>
> >> On 05/03/2018 10:11, Peng Fan wrote:
> >>> Add HS400 support.
> >>> Selecting HS400 needs first select HS199 according to spec, so use a
> >>> dedicated function for HS400.
> >>> Add HS400 related macros.
> >>> Remove the restriction of only using the low 6 bits of
> >>> EXT_CSD_CARD_TYPE, using all the 8 bits.
> >>>
> >>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> >>> Cc: Jaehoon Chung <jh80.chung@samsung.com>
> >>> Cc: Jean-Jacques Hiblot <jjhiblot@ti.com>
> >>> Cc: Stefano Babic <sbabic@denx.de>
> >>> Cc: Simon Glass <sjg@chromium.org>
> >>> Cc: Kishon Vijay Abraham I <kishon@ti.com>
> >>> Cc: Bin Meng <bmeng.cn@gmail.com>
> >>> ---
> >>>   drivers/mmc/Kconfig |   7 +++
> >>>   drivers/mmc/mmc.c   | 133
> ++++++++++++++++++++++++++++++++++++++++++----------
> >>>   include/mmc.h       |  12 +++++
> >>>   3 files changed, 127 insertions(+), 25 deletions(-)
> >>>
> >>> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig index
> >>> 5f67e336db..e9be18b333 100644
> >>> --- a/drivers/mmc/Kconfig
> >>> +++ b/drivers/mmc/Kconfig
> >>> @@ -104,6 +104,13 @@ config SPL_MMC_UHS_SUPPORT
> >>>   	  cards. The IO voltage must be switchable from 3.3v to 1.8v. The bus
> >>>   	  frequency can go up to 208MHz (SDR104)
> >>> +config MMC_HS400_SUPPORT
> >>> +	bool "enable HS400 support"
> >>> +	select MMC_HS200_SUPPORT
> >> I'd use "depends on" instead of select or maybe use the same option
> >> for both HS200 and HS400
> >>> +	help
> >>> +	  The HS400 mode is support by some eMMC. The bus frequency is up
> to
> >>> +	  200MHz. This mode requires tuning the IO.
> >>> +
> >>>   config MMC_HS200_SUPPORT
> >>>   	bool "enable HS200 support"
> >>>   	help
> >>> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index
> >>> 92ea78b8af..eef229c8b4 100644
> >>> --- a/drivers/mmc/mmc.c
> >>> +++ b/drivers/mmc/mmc.c
> >>> @@ -169,6 +169,7 @@ const char *mmc_mode_name(enum bus_mode
> mode)
> >>>   	      [MMC_HS_52]	= "MMC High Speed (52MHz)",
> >>>   	      [MMC_DDR_52]	= "MMC DDR52 (52MHz)",
> >>>   	      [MMC_HS_200]	= "HS200 (200MHz)",
> >>> +	      [MMC_HS_400]	= "HS400 (200MHz)",
> >>>   	};
> >>>   	if (mode >= MMC_MODES_END)
> >>> @@ -193,6 +194,7 @@ static uint mmc_mode2freq(struct mmc *mmc,
> enum bus_mode mode)
> >>>   	      [UHS_DDR50]	= 50000000,
> >>>   	      [UHS_SDR104]	= 208000000,
> >>>   	      [MMC_HS_200]	= 200000000,
> >>> +	      [MMC_HS_400]	= 200000000,
> >>>   	};
> >>>   	if (mode == MMC_LEGACY)
> >>> @@ -790,6 +792,11 @@ static int mmc_set_card_speed(struct mmc
> *mmc, enum bus_mode mode)
> >>>   	case MMC_HS_200:
> >>>   		speed_bits = EXT_CSD_TIMING_HS200;
> >>>   		break;
> >>> +#endif
> >>> +#if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
> >>> +	case MMC_HS_400:
> >>> +		speed_bits = EXT_CSD_TIMING_HS400;
> >>> +		break;
> >>>   #endif
> >>>   	case MMC_LEGACY:
> >>>   		speed_bits = EXT_CSD_TIMING_LEGACY; @@ -837,7 +844,7
> @@ static
> >>> int mmc_get_capabilities(struct mmc *mmc)
> >>>   	mmc->card_caps |= MMC_MODE_4BIT | MMC_MODE_8BIT;
> >>> -	cardtype = ext_csd[EXT_CSD_CARD_TYPE] & 0x3f;
> >>> +	cardtype = ext_csd[EXT_CSD_CARD_TYPE];
> >>>   	mmc->cardtype = cardtype;
> >>>   #if CONFIG_IS_ENABLED(MMC_HS200_SUPPORT)
> >>> @@ -845,6 +852,12 @@ static int mmc_get_capabilities(struct mmc
> *mmc)
> >>>   			EXT_CSD_CARD_TYPE_HS200_1_8V)) {
> >>>   		mmc->card_caps |= MMC_MODE_HS200;
> >>>   	}
> >>> +#endif
> >>> +#if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
> >>> +	if (cardtype & (EXT_CSD_CARD_TYPE_HS400_1_2V |
> >>> +			EXT_CSD_CARD_TYPE_HS400_1_8V)) {
> >>> +		mmc->card_caps |= MMC_MODE_HS400;
> >>> +	}
> >>>   #endif
> >>>   	if (cardtype & EXT_CSD_CARD_TYPE_52) {
> >>>   		if (cardtype & EXT_CSD_CARD_TYPE_DDR_52) @@ -1748,6
> +1761,12 @@
> >>> static int mmc_set_lowest_voltage(struct mmc *mmc, enum bus_mode
> mode,
> >>>   	u32 card_mask = 0;
> >>>   	switch (mode) {
> >>> +	case MMC_HS_400:
> >>> +		if (mmc->cardtype & EXT_CSD_CARD_TYPE_HS400_1_8V)
> >>> +			card_mask |= MMC_SIGNAL_VOLTAGE_180;
> >>> +		if (mmc->cardtype & EXT_CSD_CARD_TYPE_HS400_1_2V)
> >>> +			card_mask |= MMC_SIGNAL_VOLTAGE_120;
> >>> +		break;
> >>>   	case MMC_HS_200:
> >>>   		if (mmc->cardtype & EXT_CSD_CARD_TYPE_HS200_1_8V)
> >>>   			card_mask |= MMC_SIGNAL_VOLTAGE_180; @@ -1787,6
> +1806,13 @@
> >>> static inline int mmc_set_lowest_voltage(struct mmc *mmc, enum
> bus_mode mode,
> >>>   #endif
> >>>   static const struct mode_width_tuning mmc_modes_by_pref[] = {
> >>> +#if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
> >>> +	{
> >>> +		.mode = MMC_HS_400,
> >>> +		.widths = MMC_MODE_8BIT | MMC_MODE_4BIT,
> >>> +		.tuning = MMC_CMD_SEND_TUNING_BLOCK_HS200
> >>> +	},
> >>> +#endif
> >>>   #if CONFIG_IS_ENABLED(MMC_HS200_SUPPORT)
> >>>   	{
> >>>   		.mode = MMC_HS_200,
> >>> @@ -1830,6 +1856,54 @@ static const struct ext_csd_bus_width {
> >>>   	{MMC_MODE_1BIT, false, EXT_CSD_BUS_WIDTH_1},
> >>>   };
> >>> +#if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
> >>> +static int mmc_select_hs400(struct mmc *mmc) {
> >>> +	int err;
> >>> +
> >>> +	/* Set timing to HS200 for tuning */
> >>> +	err = mmc_set_card_speed(mmc, MMC_HS_200);
> >>> +	if (err)
> >>> +		return err;
> >>> +
> >>> +	/* configure the bus mode (host) */
> >>> +	mmc_select_mode(mmc, MMC_HS_200);
> >>> +	mmc_set_clock(mmc, mmc->tran_speed, false);
> >>> +
> >>> +	/* execute tuning if needed */
> >>> +	err = mmc_execute_tuning(mmc,
> MMC_CMD_SEND_TUNING_BLOCK_HS200);
> >>> +	if (err) {
> >>> +		debug("tuning failed\n");
> >>> +		return err;
> >>> +	}
> >>> +
> >>> +	/* Set back to HS */
> >>> +	mmc_set_card_speed(mmc, MMC_HS);
> >>> +	mmc_set_clock(mmc, mmc_mode2freq(mmc, MMC_HS), false);
> >>> +
> >>> +	err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL,
> EXT_CSD_BUS_WIDTH,
> >>> +			 EXT_CSD_BUS_WIDTH_8 | EXT_CSD_DDR_FLAG);
> >> What happens if only 4 wires are used. It is a legit mode, isn't it ?
> > According to spec, HS400 only supports 8bits mode, see "5.3.6 HS400 Bus
> Speed Mode".
> Good. That makes it easier.
> Can you add something to remove HS400 from the host capabilities if the
> 8-wires configuration is not available ?

In mmc_select_mode_and_width:
"card_caps &= mmc->host_caps;" I think this line has already done that.
When host_caps does not support 8-wires, the card_caps will be set not
Supporting 8-wires and HS400 mode will be selected.

I think I need remove the MMC_MODE_4BIT from HS400 mode in the
mmc_modes_by_pref array.

Thanks,
Peng.

> 
> >
> >>> +	if (err)
> >>> +		return err;
> >>> +
> >>> +	err = mmc_set_card_speed(mmc, MMC_HS_400);
> >>> +	if (err)
> >>> +		return err;
> >>> +
> >>> +	mmc_select_mode(mmc, MMC_HS_400);
> >>> +	err = mmc_set_clock(mmc, mmc->tran_speed, false);
> >>> +	if (err)
> >>> +		return err;
> >>> +
> >>> +	return 0;
> >>> +}
> >>> +#else
> >>> +static int mmc_select_hs400(struct mmc *mmc) {
> >>> +	return -ENOTSUPP;
> >>> +}
> >>> +#endif
> >>> +
> >>>   #define for_each_supported_width(caps, ddr, ecbv) \
> >>>   	for (ecbv = ext_csd_bus_width;\
> >>>   	    ecbv < ext_csd_bus_width + ARRAY_SIZE(ext_csd_bus_width);\
> @@
> >>> -1883,37 +1957,46 @@ static int mmc_select_mode_and_width(struct
> mmc *mmc, uint card_caps)
> >>>   				goto error;
> >>>   			mmc_set_bus_width(mmc, bus_width(ecbw->cap));
> >>> -			/* configure the bus speed (card) */
> >>> -			err = mmc_set_card_speed(mmc, mwt->mode);
> >>> -			if (err)
> >>> -				goto error;
> >>> -
> >>> -			/*
> >>> -			 * configure the bus width AND the ddr mode (card)
> >>> -			 * The host side will be taken care of in the next step
> >>> -			 */
> >>> -			if (ecbw->ext_csd_bits & EXT_CSD_DDR_FLAG) {
> >>> -				err = mmc_switch(mmc,
> EXT_CSD_CMD_SET_NORMAL,
> >>> -						 EXT_CSD_BUS_WIDTH,
> >>> -						 ecbw->ext_csd_bits);
> >>> +			if (mwt->mode == MMC_HS_400) {
> >>> +				err = mmc_select_hs400(mmc);
> >>> +				if (err)
> >>> +					goto error;
> >>> +			} else {
> >>> +				/* configure the bus speed (card) */
> >>> +				err = mmc_set_card_speed(mmc, mwt->mode);
> >> Instead of having?? a separate mmc_select_hs400() is not possible to
> >> leverage the existing code ?
> > hs400 requires first runs into HS200 mode, after tuning finished, need
> > switch back to HS mode to configure bus. It's not that straightforward
> > to use the existing code. Use a dedicated mmc_select_hs400 will make
> > it clearer compared with adding more logic in the existing flow.
> OK
> >
> > Thanks,
> > Peng.
> >
> >> JJ
> >>
> >>>   				if (err)
> >>>   					goto error;
> >>> -			}
> >>> -			/* configure the bus mode (host) */
> >>> -			mmc_select_mode(mmc, mwt->mode);
> >>> -			mmc_set_clock(mmc, mmc->tran_speed, false);
> >>> +				/*
> >>> +				 * configure the bus width AND the ddr mode
> >>> +				 * (card). The host side will be taken care
> >>> +				 * of in the next step
> >>> +				 */
> >>> +				if (ecbw->ext_csd_bits & EXT_CSD_DDR_FLAG) {
> >>> +					err = mmc_switch(mmc,
> >>> +							 EXT_CSD_CMD_SET_NORMAL,
> >>> +							 EXT_CSD_BUS_WIDTH,
> >>> +							 ecbw->ext_csd_bits);
> >>> +					if (err)
> >>> +						goto error;
> >>> +				}
> >>> +
> >>> +				/* configure the bus mode (host) */
> >>> +				mmc_select_mode(mmc, mwt->mode);
> >>> +				mmc_set_clock(mmc, mmc->tran_speed, false);
> >>>   #ifdef MMC_SUPPORTS_TUNING
> >>> -			/* execute tuning if needed */
> >>> -			if (mwt->tuning) {
> >>> -				err = mmc_execute_tuning(mmc, mwt->tuning);
> >>> -				if (err) {
> >>> -					pr_debug("tuning failed\n");
> >>> -					goto error;
> >>> +				/* execute tuning if needed */
> >>> +				if (mwt->tuning) {
> >>> +					err = mmc_execute_tuning(mmc,
> >>> +								 mwt->tuning);
> >>> +					if (err) {
> >>> +						pr_debug("tuning failed\n");
> >>> +						goto error;
> >>> +					}
> >>>   				}
> >>> -			}
> >>>   #endif
> >>> +			}
> >>>   			/* do a transfer to check the configuration */
> >>>   			err = mmc_read_and_compare_ext_csd(mmc);
> >>> diff --git a/include/mmc.h b/include/mmc.h index
> >>> 86f885b504..8c01c6a530 100644
> >>> --- a/include/mmc.h
> >>> +++ b/include/mmc.h
> >>> @@ -65,6 +65,7 @@
> >>>   #define MMC_MODE_HS_52MHz	MMC_CAP(MMC_HS_52)
> >>>   #define MMC_MODE_DDR_52MHz	MMC_CAP(MMC_DDR_52)
> >>>   #define MMC_MODE_HS200		MMC_CAP(MMC_HS_200)
> >>> +#define MMC_MODE_HS400		MMC_CAP(MMC_HS_400)
> >>>   #define MMC_MODE_8BIT		BIT(30)
> >>>   #define MMC_MODE_4BIT		BIT(29)
> >>> @@ -250,6 +251,11 @@ static inline bool mmc_is_tuning_cmd(uint
> cmdidx)
> >>>   #define EXT_CSD_CARD_TYPE_HS200
> 	(EXT_CSD_CARD_TYPE_HS200_1_8V | \
> >>>   					 EXT_CSD_CARD_TYPE_HS200_1_2V)
> >>> +#define EXT_CSD_CARD_TYPE_HS400_1_8V	BIT(6)
> >>> +#define EXT_CSD_CARD_TYPE_HS400_1_2V	BIT(7)
> >>> +#define EXT_CSD_CARD_TYPE_HS400
> 	(EXT_CSD_CARD_TYPE_HS400_1_8V | \
> >>> +					 EXT_CSD_CARD_TYPE_HS400_1_2V)
> >>> +
> >>>   #define EXT_CSD_BUS_WIDTH_1	0	/* Card is in 1 bit mode */
> >>>   #define EXT_CSD_BUS_WIDTH_4	1	/* Card is in 4 bit mode */
> >>>   #define EXT_CSD_BUS_WIDTH_8	2	/* Card is in 8 bit mode */
> >>> @@ -260,6 +266,7 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx)
> >>>   #define EXT_CSD_TIMING_LEGACY	0	/* no high speed */
> >>>   #define EXT_CSD_TIMING_HS	1	/* HS */
> >>>   #define EXT_CSD_TIMING_HS200	2	/* HS200 */
> >>> +#define EXT_CSD_TIMING_HS400	3	/* HS400 */
> >>>   #define EXT_CSD_BOOT_ACK_ENABLE			(1 << 6)
> >>>   #define EXT_CSD_BOOT_PARTITION_ENABLE		(1 << 3)
> >>> @@ -520,6 +527,7 @@ enum bus_mode {
> >>>   	UHS_DDR50,
> >>>   	UHS_SDR104,
> >>>   	MMC_HS_200,
> >>> +	MMC_HS_400,
> >>>   	MMC_MODES_END
> >>>   };
> >>> @@ -533,6 +541,10 @@ static inline bool mmc_is_mode_ddr(enum
> bus_mode mode)
> >>>   #if CONFIG_IS_ENABLED(MMC_UHS_SUPPORT)
> >>>   	else if (mode == UHS_DDR50)
> >>>   		return true;
> >>> +#endif
> >>> +#if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
> >>> +	else if (mode == MMC_HS_400)
> >>> +		return true;
> >>>   #endif
> >>>   	else
> >>>   		return false;
> >> _______________________________________________
> >> U-Boot mailing list
> >> U-Boot at lists.denx.de
> >> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fli
> >>
> sts.denx.de%2Flistinfo%2Fu-boot&data=02%7C01%7Cpeng.fan%40nxp.com%7C
> c
> >>
> 1f326580e3a487c2b4f08d5836f7587%7C686ea1d3bc2b4c6fa92cd99c5c301635
> %7C
> >>
> 0%7C0%7C636559437115011742&sdata=l%2BuDMMe8ZWIi2uM7hIIVgAIezjm
> stIuZ66
> >> 6dewGYSjc%3D&reserved=0

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH 1/2] mmc: add HS400 support
  2018-03-06 17:41 ` Simon Glass
@ 2018-03-07  2:12   ` Peng Fan
  0 siblings, 0 replies; 10+ messages in thread
From: Peng Fan @ 2018-03-07  2:12 UTC (permalink / raw)
  To: u-boot


Hi Simon,

> -----Original Message-----
> From: sjg at google.com [mailto:sjg at google.com] On Behalf Of Simon Glass
> Sent: 2018年3月7日 1:42
> To: Peng Fan <peng.fan@nxp.com>
> Cc: Jaehoon Chung <jh80.chung@samsung.com>; Stefano Babic
> <sbabic@denx.de>; U-Boot Mailing List <u-boot@lists.denx.de>; Jean-Jacques
> Hiblot <jjhiblot@ti.com>; Kishon Vijay Abraham I <kishon@ti.com>; Bin Meng
> <bmeng.cn@gmail.com>
> Subject: Re: [PATCH 1/2] mmc: add HS400 support
> 
> Hi Peng,
> 
> On 5 March 2018 at 02:11, Peng Fan <peng.fan@nxp.com> wrote:
> > Add HS400 support.
> > Selecting HS400 needs first select HS199 according to spec, so use a
> > dedicated function for HS400.
> > Add HS400 related macros.
> > Remove the restriction of only using the low 6 bits of
> > EXT_CSD_CARD_TYPE, using all the 8 bits.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > Cc: Jaehoon Chung <jh80.chung@samsung.com>
> > Cc: Jean-Jacques Hiblot <jjhiblot@ti.com>
> > Cc: Stefano Babic <sbabic@denx.de>
> > Cc: Simon Glass <sjg@chromium.org>
> > Cc: Kishon Vijay Abraham I <kishon@ti.com>
> > Cc: Bin Meng <bmeng.cn@gmail.com>
> > ---
> >  drivers/mmc/Kconfig |   7 +++
> >  drivers/mmc/mmc.c   | 133
> ++++++++++++++++++++++++++++++++++++++++++----------
> >  include/mmc.h       |  12 +++++
> >  3 files changed, 127 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig index
> > 5f67e336db..e9be18b333 100644
> > --- a/drivers/mmc/Kconfig
> > +++ b/drivers/mmc/Kconfig
> > @@ -104,6 +104,13 @@ config SPL_MMC_UHS_SUPPORT
> >           cards. The IO voltage must be switchable from 3.3v to 1.8v. The
> bus
> >           frequency can go up to 208MHz (SDR104)
> >
> > +config MMC_HS400_SUPPORT
> > +       bool "enable HS400 support"
> > +       select MMC_HS200_SUPPORT
> > +       help
> > +         The HS400 mode is support by some eMMC. The bus frequency is
> up to
> > +         200MHz. This mode requires tuning the IO.
> > +
> >  config MMC_HS200_SUPPORT
> >         bool "enable HS200 support"
> >         help
> > diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index
> > 92ea78b8af..eef229c8b4 100644
> > --- a/drivers/mmc/mmc.c
> > +++ b/drivers/mmc/mmc.c
> > @@ -169,6 +169,7 @@ const char *mmc_mode_name(enum bus_mode
> mode)
> >               [MMC_HS_52]       = "MMC High Speed (52MHz)",
> >               [MMC_DDR_52]      = "MMC DDR52 (52MHz)",
> >               [MMC_HS_200]      = "HS200 (200MHz)",
> > +             [MMC_HS_400]      = "HS400 (200MHz)",
> >         };
> >
> >         if (mode >= MMC_MODES_END)
> > @@ -193,6 +194,7 @@ static uint mmc_mode2freq(struct mmc *mmc, enum
> bus_mode mode)
> >               [UHS_DDR50]       = 50000000,
> >               [UHS_SDR104]      = 208000000,
> >               [MMC_HS_200]      = 200000000,
> > +             [MMC_HS_400]      = 200000000,
> >         };
> >
> >         if (mode == MMC_LEGACY)
> > @@ -790,6 +792,11 @@ static int mmc_set_card_speed(struct mmc *mmc,
> enum bus_mode mode)
> >         case MMC_HS_200:
> >                 speed_bits = EXT_CSD_TIMING_HS200;
> >                 break;
> > +#endif
> > +#if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
> > +       case MMC_HS_400:
> > +               speed_bits = EXT_CSD_TIMING_HS400;
> > +               break;
> >  #endif
> >         case MMC_LEGACY:
> >                 speed_bits = EXT_CSD_TIMING_LEGACY; @@ -837,7
> +844,7
> > @@ static int mmc_get_capabilities(struct mmc *mmc)
> >
> >         mmc->card_caps |= MMC_MODE_4BIT | MMC_MODE_8BIT;
> >
> > -       cardtype = ext_csd[EXT_CSD_CARD_TYPE] & 0x3f;
> > +       cardtype = ext_csd[EXT_CSD_CARD_TYPE];
> >         mmc->cardtype = cardtype;
> >
> >  #if CONFIG_IS_ENABLED(MMC_HS200_SUPPORT)
> > @@ -845,6 +852,12 @@ static int mmc_get_capabilities(struct mmc *mmc)
> >                         EXT_CSD_CARD_TYPE_HS200_1_8V)) {
> >                 mmc->card_caps |= MMC_MODE_HS200;
> >         }
> > +#endif
> > +#if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
> 
> Is it possible to use if () instead of #if

Just want to keep the same code style in the file. How about use a dedicated patch to
cleanup the file after this patch?
Thanks,
Peng.

> 
> Regards,
> Simon

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [U-Boot,1/2] mmc: add HS400 support
       [not found] ` <CGME20180426103029epcas1p4f0292a99571b6970f61d67ff42728fb0@epcas1p4.samsung.com>
@ 2018-04-26 10:30   ` Jaehoon Chung
  0 siblings, 0 replies; 10+ messages in thread
From: Jaehoon Chung @ 2018-04-26 10:30 UTC (permalink / raw)
  To: u-boot

Hi,

On 03/05/2018 06:11 PM, Peng Fan wrote:
> Add HS400 support.
> Selecting HS400 needs first select HS199 according to spec, so use
> a dedicated function for HS400.
> Add HS400 related macros.
> Remove the restriction of only using the low 6 bits of
> EXT_CSD_CARD_TYPE, using all the 8 bits.

Sorry for late.

> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> Cc: Jaehoon Chung <jh80.chung@samsung.com>
> Cc: Jean-Jacques Hiblot <jjhiblot@ti.com>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Kishon Vijay Abraham I <kishon@ti.com>
> Cc: Bin Meng <bmeng.cn@gmail.com>
> ---
>  drivers/mmc/Kconfig |   7 +++
>  drivers/mmc/mmc.c   | 133 ++++++++++++++++++++++++++++++++++++++++++----------
>  include/mmc.h       |  12 +++++
>  3 files changed, 127 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
> index 5f67e336db..e9be18b333 100644
> --- a/drivers/mmc/Kconfig
> +++ b/drivers/mmc/Kconfig
> @@ -104,6 +104,13 @@ config SPL_MMC_UHS_SUPPORT
>  	  cards. The IO voltage must be switchable from 3.3v to 1.8v. The bus
>  	  frequency can go up to 208MHz (SDR104)
>  
> +config MMC_HS400_SUPPORT
> +	bool "enable HS400 support"
> +	select MMC_HS200_SUPPORT
> +	help
> +	  The HS400 mode is support by some eMMC. The bus frequency is up to
> +	  200MHz. This mode requires tuning the IO.
> +
>  config MMC_HS200_SUPPORT
>  	bool "enable HS200 support"
>  	help
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index 92ea78b8af..eef229c8b4 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -169,6 +169,7 @@ const char *mmc_mode_name(enum bus_mode mode)
>  	      [MMC_HS_52]	= "MMC High Speed (52MHz)",
>  	      [MMC_DDR_52]	= "MMC DDR52 (52MHz)",
>  	      [MMC_HS_200]	= "HS200 (200MHz)",
> +	      [MMC_HS_400]	= "HS400 (200MHz)",
>  	};
>  
>  	if (mode >= MMC_MODES_END)
> @@ -193,6 +194,7 @@ static uint mmc_mode2freq(struct mmc *mmc, enum bus_mode mode)
>  	      [UHS_DDR50]	= 50000000,
>  	      [UHS_SDR104]	= 208000000,
>  	      [MMC_HS_200]	= 200000000,
> +	      [MMC_HS_400]	= 200000000,
>  	};
>  
>  	if (mode == MMC_LEGACY)
> @@ -790,6 +792,11 @@ static int mmc_set_card_speed(struct mmc *mmc, enum bus_mode mode)
>  	case MMC_HS_200:
>  		speed_bits = EXT_CSD_TIMING_HS200;
>  		break;
> +#endif
> +#if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
> +	case MMC_HS_400:
> +		speed_bits = EXT_CSD_TIMING_HS400;
> +		break;>  #endif
>  	case MMC_LEGACY:
>  		speed_bits = EXT_CSD_TIMING_LEGACY;
> @@ -837,7 +844,7 @@ static int mmc_get_capabilities(struct mmc *mmc)
>  
>  	mmc->card_caps |= MMC_MODE_4BIT | MMC_MODE_8BIT;
>  
> -	cardtype = ext_csd[EXT_CSD_CARD_TYPE] & 0x3f;
> +	cardtype = ext_csd[EXT_CSD_CARD_TYPE];
>  	mmc->cardtype = cardtype;
>  
>  #if CONFIG_IS_ENABLED(MMC_HS200_SUPPORT)
> @@ -845,6 +852,12 @@ static int mmc_get_capabilities(struct mmc *mmc)
>  			EXT_CSD_CARD_TYPE_HS200_1_8V)) {
>  		mmc->card_caps |= MMC_MODE_HS200;
>  	}
> +#endif
> +#if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
> +	if (cardtype & (EXT_CSD_CARD_TYPE_HS400_1_2V |
> +			EXT_CSD_CARD_TYPE_HS400_1_8V)) {
> +		mmc->card_caps |= MMC_MODE_HS400;
> +	}
>  #endif
>  	if (cardtype & EXT_CSD_CARD_TYPE_52) {
>  		if (cardtype & EXT_CSD_CARD_TYPE_DDR_52)
> @@ -1748,6 +1761,12 @@ static int mmc_set_lowest_voltage(struct mmc *mmc, enum bus_mode mode,
>  	u32 card_mask = 0;
>  
>  	switch (mode) {
> +	case MMC_HS_400:
> +		if (mmc->cardtype & EXT_CSD_CARD_TYPE_HS400_1_8V)
> +			card_mask |= MMC_SIGNAL_VOLTAGE_180;
> +		if (mmc->cardtype & EXT_CSD_CARD_TYPE_HS400_1_2V)
> +			card_mask |= MMC_SIGNAL_VOLTAGE_120;
> +		break;

I think that it's possible to use the below.

case MMS_HS_400:
case MMC_HS_200:
	if (mmc->cardtype & (EXT_CSD_CARD_TYPE_HS200_1_8V | EXT_CSD_CARD_TYPE_HS400_1_8V)
		....

Isn't it?

>  	case MMC_HS_200:
>  		if (mmc->cardtype & EXT_CSD_CARD_TYPE_HS200_1_8V)
>  			card_mask |= MMC_SIGNAL_VOLTAGE_180;
> @@ -1787,6 +1806,13 @@ static inline int mmc_set_lowest_voltage(struct mmc *mmc, enum bus_mode mode,
>  #endif
>  
>  static const struct mode_width_tuning mmc_modes_by_pref[] = {
> +#if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
> +	{
> +		.mode = MMC_HS_400,
> +		.widths = MMC_MODE_8BIT | MMC_MODE_4BIT,

4BIT? As i know, HS400 mode doesn't support the 4BIT mode.

> +		.tuning = MMC_CMD_SEND_TUNING_BLOCK_HS200
> +	},
> +#endif
>  #if CONFIG_IS_ENABLED(MMC_HS200_SUPPORT)
>  	{
>  		.mode = MMC_HS_200,
> @@ -1830,6 +1856,54 @@ static const struct ext_csd_bus_width {
>  	{MMC_MODE_1BIT, false, EXT_CSD_BUS_WIDTH_1},
>  };
>  
> +#if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
> +static int mmc_select_hs400(struct mmc *mmc)
> +{
> +	int err;
> +
> +	/* Set timing to HS200 for tuning */
> +	err = mmc_set_card_speed(mmc, MMC_HS_200);
> +	if (err)
> +		return err;
> +
> +	/* configure the bus mode (host) */
> +	mmc_select_mode(mmc, MMC_HS_200);
> +	mmc_set_clock(mmc, mmc->tran_speed, false);
> +
> +	/* execute tuning if needed */
> +	err = mmc_execute_tuning(mmc, MMC_CMD_SEND_TUNING_BLOCK_HS200);
> +	if (err) {
> +		debug("tuning failed\n");
> +		return err;
> +	}
> +
> +	/* Set back to HS */
> +	mmc_set_card_speed(mmc, MMC_HS);
> +	mmc_set_clock(mmc, mmc_mode2freq(mmc, MMC_HS), false);
> +
> +	err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_BUS_WIDTH,
> +			 EXT_CSD_BUS_WIDTH_8 | EXT_CSD_DDR_FLAG);
> +	if (err)
> +		return err;
> +
> +	err = mmc_set_card_speed(mmc, MMC_HS_400);
> +	if (err)
> +		return err;
> +
> +	mmc_select_mode(mmc, MMC_HS_400);
> +	err = mmc_set_clock(mmc, mmc->tran_speed, false);
> +	if (err)
> +		return err;

Hmm. I didn't test this patch.. but i guess if it's failed, isn't it possible to use the other bus mode?

> +
> +	return 0;
> +}
> +#else
> +static int mmc_select_hs400(struct mmc *mmc)
> +{
> +	return -ENOTSUPP;
> +}
> +#endif
> +
>  #define for_each_supported_width(caps, ddr, ecbv) \
>  	for (ecbv = ext_csd_bus_width;\
>  	    ecbv < ext_csd_bus_width + ARRAY_SIZE(ext_csd_bus_width);\
> @@ -1883,37 +1957,46 @@ static int mmc_select_mode_and_width(struct mmc *mmc, uint card_caps)
>  				goto error;
>  			mmc_set_bus_width(mmc, bus_width(ecbw->cap));
>  
> -			/* configure the bus speed (card) */
> -			err = mmc_set_card_speed(mmc, mwt->mode);
> -			if (err)
> -				goto error;
> -
> -			/*
> -			 * configure the bus width AND the ddr mode (card)
> -			 * The host side will be taken care of in the next step
> -			 */
> -			if (ecbw->ext_csd_bits & EXT_CSD_DDR_FLAG) {
> -				err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL,
> -						 EXT_CSD_BUS_WIDTH,
> -						 ecbw->ext_csd_bits);
> +			if (mwt->mode == MMC_HS_400) {
> +				err = mmc_select_hs400(mmc);

doesn't need to add some error message? It's useful to notice to user.

> +				if (err)
> +					goto error;
> +			} else {
> +				/* configure the bus speed (card) */
> +				err = mmc_set_card_speed(mmc, mwt->mode);
>  				if (err)
>  					goto error;
> -			}
>  
> -			/* configure the bus mode (host) */
> -			mmc_select_mode(mmc, mwt->mode);
> -			mmc_set_clock(mmc, mmc->tran_speed, false);
> +				/*
> +				 * configure the bus width AND the ddr mode
> +				 * (card). The host side will be taken care
> +				 * of in the next step
> +				 */
> +				if (ecbw->ext_csd_bits & EXT_CSD_DDR_FLAG) {
> +					err = mmc_switch(mmc,
> +							 EXT_CSD_CMD_SET_NORMAL,
> +							 EXT_CSD_BUS_WIDTH,
> +							 ecbw->ext_csd_bits);
> +					if (err)
> +						goto error;
> +				}
> +
> +				/* configure the bus mode (host) */
> +				mmc_select_mode(mmc, mwt->mode);
> +				mmc_set_clock(mmc, mmc->tran_speed, false);
>  #ifdef MMC_SUPPORTS_TUNING
>  
> -			/* execute tuning if needed */
> -			if (mwt->tuning) {
> -				err = mmc_execute_tuning(mmc, mwt->tuning);
> -				if (err) {
> -					pr_debug("tuning failed\n");
> -					goto error;
> +				/* execute tuning if needed */
> +				if (mwt->tuning) {
> +					err = mmc_execute_tuning(mmc,
> +								 mwt->tuning);
> +					if (err) {
> +						pr_debug("tuning failed\n");
> +						goto error;
> +					}
>  				}
> -			}
>  #endif
> +			}
>  
>  			/* do a transfer to check the configuration */
>  			err = mmc_read_and_compare_ext_csd(mmc);
> diff --git a/include/mmc.h b/include/mmc.h
> index 86f885b504..8c01c6a530 100644
> --- a/include/mmc.h
> +++ b/include/mmc.h
> @@ -65,6 +65,7 @@
>  #define MMC_MODE_HS_52MHz	MMC_CAP(MMC_HS_52)
>  #define MMC_MODE_DDR_52MHz	MMC_CAP(MMC_DDR_52)
>  #define MMC_MODE_HS200		MMC_CAP(MMC_HS_200)
> +#define MMC_MODE_HS400		MMC_CAP(MMC_HS_400)
>  
>  #define MMC_MODE_8BIT		BIT(30)
>  #define MMC_MODE_4BIT		BIT(29)
> @@ -250,6 +251,11 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx)
>  #define EXT_CSD_CARD_TYPE_HS200		(EXT_CSD_CARD_TYPE_HS200_1_8V | \
>  					 EXT_CSD_CARD_TYPE_HS200_1_2V)
>  
> +#define EXT_CSD_CARD_TYPE_HS400_1_8V	BIT(6)
> +#define EXT_CSD_CARD_TYPE_HS400_1_2V	BIT(7)
> +#define EXT_CSD_CARD_TYPE_HS400		(EXT_CSD_CARD_TYPE_HS400_1_8V | \
> +					 EXT_CSD_CARD_TYPE_HS400_1_2V)
> +
>  #define EXT_CSD_BUS_WIDTH_1	0	/* Card is in 1 bit mode */
>  #define EXT_CSD_BUS_WIDTH_4	1	/* Card is in 4 bit mode */
>  #define EXT_CSD_BUS_WIDTH_8	2	/* Card is in 8 bit mode */
> @@ -260,6 +266,7 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx)
>  #define EXT_CSD_TIMING_LEGACY	0	/* no high speed */
>  #define EXT_CSD_TIMING_HS	1	/* HS */
>  #define EXT_CSD_TIMING_HS200	2	/* HS200 */
> +#define EXT_CSD_TIMING_HS400	3	/* HS400 */
>  
>  #define EXT_CSD_BOOT_ACK_ENABLE			(1 << 6)
>  #define EXT_CSD_BOOT_PARTITION_ENABLE		(1 << 3)
> @@ -520,6 +527,7 @@ enum bus_mode {
>  	UHS_DDR50,
>  	UHS_SDR104,
>  	MMC_HS_200,
> +	MMC_HS_400,
>  	MMC_MODES_END
>  };
>  
> @@ -533,6 +541,10 @@ static inline bool mmc_is_mode_ddr(enum bus_mode mode)
>  #if CONFIG_IS_ENABLED(MMC_UHS_SUPPORT)
>  	else if (mode == UHS_DDR50)
>  		return true;
> +#endif
> +#if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
> +	else if (mode == MMC_HS_400)
> +		return true;
>  #endif
>  	else
>  		return false;
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [U-Boot,2/2] mmc: fsl_esdhc: enable HS400 feature
       [not found]   ` <CGME20180426103032epcas1p47ee75c2117d1a7d4f2d4fdde5824074e@epcas1p4.samsung.com>
@ 2018-04-26 10:30     ` Jaehoon Chung
  0 siblings, 0 replies; 10+ messages in thread
From: Jaehoon Chung @ 2018-04-26 10:30 UTC (permalink / raw)
  To: u-boot

Hi,

On 03/05/2018 06:11 PM, Peng Fan wrote:
> The strobe dll code is ported from Linux Kernel:
> drivers/mmc/host/sdhci-esdhc-imx.c
> The comments are from the above file,
> "For HS400 eMMC, there is a data_strobe line. This signal is generated
> by the device and used for data output and CRC status response output
> in HS400 mode. The frequency of this signal follows the frequency of
> CLK generated by host. The host receives the data which is aligned to the
> edge of data_strobe line. Due to the time delay between CLK line and
> data_strobe line, if the delay time is larger than one clock cycle,
> then CLK and data_strobe line will be misaligned, read error shows up.
> So when the CLK is higher than 100MHz, each clock cycle is short enough,
> host should configure the delay target. "

Sorry for late.

> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> Cc: Jaehoon Chung <jh80.chung@samsung.com>
> Cc: Stefano Babic <sbabic@denx.de>
> ---
>  drivers/mmc/fsl_esdhc.c | 35 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
> index 6018f84307..e42bdb6941 100644
> --- a/drivers/mmc/fsl_esdhc.c
> +++ b/drivers/mmc/fsl_esdhc.c
> @@ -663,6 +663,7 @@ static int esdhc_change_pinstate(struct udevice *dev)
>  		break;
>  	case UHS_SDR104:
>  	case MMC_HS_200:
> +	case MMC_HS_400:
>  		ret = pinctrl_select_state(dev, "state_200mhz");
>  		break;
>  	default:
> @@ -690,6 +691,33 @@ static void esdhc_reset_tuning(struct mmc *mmc)
>  	}
>  }
>  
> +static void esdhc_set_strobe_dll(struct mmc *mmc)
> +{
> +	struct fsl_esdhc_priv *priv = dev_get_priv(mmc->dev);
> +	struct fsl_esdhc *regs = priv->esdhc_regs;
> +	u32 v;

v? I want to use the meaningful variable name. :)


> +
> +	if (priv->clock > ESDHC_STROBE_DLL_CLK_FREQ) {
> +		writel(ESDHC_STROBE_DLL_CTRL_RESET, &regs->strobe_dllctrl);
> +
> +		/*
> +		 * enable strobe dll ctrl and adjust the delay target
> +		 * for the uSDHC loopback read clock
> +		 */
> +		v = ESDHC_STROBE_DLL_CTRL_ENABLE |
> +			(priv->strobe_dll_delay_target <<
> +			 ESDHC_STROBE_DLL_CTRL_SLV_DLY_TARGET_SHIFT);
> +		writel(v, &regs->strobe_dllctrl);
> +		/* wait 1us to make sure strobe dll status register stable */
> +		mdelay(1);
> +		v = readl(&regs->strobe_dllstat);
> +		if (!(v & ESDHC_STROBE_DLL_STS_REF_LOCK))
> +			pr_warn("HS400 strobe DLL status REF not lock!\n");
> +		if (!(v & ESDHC_STROBE_DLL_STS_SLV_LOCK))
> +			pr_warn("HS400 strobe DLL status SLV not lock!\n");
> +	}
> +}
> +
>  static int esdhc_set_timing(struct mmc *mmc)
>  {
>  	struct fsl_esdhc_priv *priv = dev_get_priv(mmc->dev);
> @@ -704,6 +732,11 @@ static int esdhc_set_timing(struct mmc *mmc)
>  	case SD_LEGACY:
>  		esdhc_reset_tuning(mmc);
>  		break;
> +	case MMC_HS_400:
> +		mixctrl |= MIX_CTRL_DDREN | MIX_CTRL_HS400_EN;
> +		writel(mixctrl, &regs->mixctrl);
> +		esdhc_set_strobe_dll(mmc);
> +		break;
>  	case MMC_HS:
>  	case MMC_HS_52:
>  	case MMC_HS_200:
> @@ -1439,7 +1472,7 @@ static int fsl_esdhc_probe(struct udevice *dev)
>  #endif
>  
>  	if (fdt_get_property(fdt, node, "no-1-8-v", NULL))
> -		priv->caps &= ~(UHS_CAPS | MMC_MODE_HS200);
> +		priv->caps &= ~(UHS_CAPS | MMC_MODE_HS200 | MMC_MODE_HS400);
>  
>  	/*
>  	 * TODO:
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-04-26 10:30 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-05  9:11 [U-Boot] [PATCH 1/2] mmc: add HS400 support Peng Fan
2018-03-05  9:11 ` [U-Boot] [PATCH 2/2] mmc: fsl_esdhc: enable HS400 feature Peng Fan
     [not found]   ` <CGME20180426103032epcas1p47ee75c2117d1a7d4f2d4fdde5824074e@epcas1p4.samsung.com>
2018-04-26 10:30     ` [U-Boot] [U-Boot,2/2] " Jaehoon Chung
2018-03-05 16:29 ` [U-Boot] [PATCH 1/2] mmc: add HS400 support Jean-Jacques Hiblot
2018-03-06  1:46   ` Peng Fan
2018-03-06 14:35     ` Jean-Jacques Hiblot
2018-03-07  2:07       ` Peng Fan
2018-03-06 17:41 ` Simon Glass
2018-03-07  2:12   ` Peng Fan
     [not found] ` <CGME20180426103029epcas1p4f0292a99571b6970f61d67ff42728fb0@epcas1p4.samsung.com>
2018-04-26 10:30   ` [U-Boot] [U-Boot,1/2] " Jaehoon Chung

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.