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....@nxp.com> >>Cc: Jaehoon Chung <jh80.ch...@samsung.com> >>Cc: Jean-Jacques Hiblot <jjhib...@ti.com> >>Cc: Stefano Babic <sba...@denx.de> >>Cc: Simon Glass <s...@chromium.org> >>Cc: Kishon Vijay Abraham I <kis...@ti.com> >>Cc: Bin Meng <bmeng...@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@lists.denx.de >https://lists.denx.de/listinfo/u-boot -- _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot