[U-Boot] [PATCH 1/2] mmc: add HS400 support

Jean-Jacques Hiblot jjhiblot at ti.com
Mon Mar 5 16:29:08 UTC 2018


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 at nxp.com>
> Cc: Jaehoon Chung <jh80.chung at samsung.com>
> Cc: Jean-Jacques Hiblot <jjhiblot at ti.com>
> Cc: Stefano Babic <sbabic at denx.de>
> Cc: Simon Glass <sjg at chromium.org>
> Cc: Kishon Vijay Abraham I <kishon at ti.com>
> Cc: Bin Meng <bmeng.cn at 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;



More information about the U-Boot mailing list