On 12 May 2017 at 12:16, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
Subject: drop the period at the end
Also I think 'mmc: Introduce MMC modes' is better (imperative tense)
no functionnal changes. In order to add the support for the high speed SD and MMC modes, it is useful to track this information.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
drivers/mmc/mmc.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++------- include/mmc.h | 34 ++++++++++++++++++++++++++++------ 2 files changed, 74 insertions(+), 13 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Also see below
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index 344d760..2e1cb0d 100644 --- a/drivers/mmc/mmc.c +++ b/drivers/mmc/mmc.c @@ -149,6 +149,36 @@ void mmc_trace_state(struct mmc *mmc, struct mmc_cmd *cmd) } #endif
+const char *mmc_mode_name(enum bus_mode mode) +{
static const char *const names[] = {[MMC_LEGACY] = "MMC legacy",[SD_LEGACY] = "SD Legacy",[MMC_HS] = "MMC High Speed (26MHz)",[SD_HS] = "SD High Speed (50MHz)",[UHS_SDR12] = "UHS SDR12 (25MHz)",[UHS_SDR25] = "UHS SDR25 (50MHz)",[UHS_SDR50] = "UHS SDR50 (100MHz)",[UHS_SDR104] = "UHS SDR104 (208MHz)",[UHS_DDR50] = "UHS DDR50 (50MHz)",[MMC_HS_52] = "MMC High Speed (52MHz)",[MMC_DDR_52] = "MMC DDR52 (52MHz)",[MMC_HS_200] = "HS200 (200MHz)",
Can we hide this behind a Kconfig so boards can turn it off to reduce code size in SPL?
};if (mode >= MMC_MODES_END)return "Unknown mode";elsereturn names[mode];+} +static int mmc_select_mode(struct mmc *mmc, enum bus_mode mode) +{
mmc->selected_mode = mode;debug("selecting mode %s (freq : %d MHz)\n", mmc_mode_name(mode),mmc->tran_speed / 1000000);return 0;+}
#ifndef CONFIG_DM_MMC_OPS int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data) { @@ -1138,10 +1168,13 @@ static int sd_select_bus_freq_width(struct mmc *mmc) if (err) return err;
if (mmc->card_caps & MMC_MODE_HS)
if (mmc->card_caps & MMC_MODE_HS) {mmc_select_mode(mmc, SD_HS); mmc->tran_speed = 50000000;
else
} else {mmc_select_mode(mmc, SD_LEGACY); mmc->tran_speed = 25000000;} return 0;} @@ -1255,11 +1288,15 @@ static int mmc_select_bus_freq_width(struct mmc *mmc) if (err) return err;
if (mmc->card_caps & MMC_MODE_HS) {if (mmc->card_caps & MMC_MODE_HS_52MHz)mmc->tran_speed = 52000000;
if (mmc->card_caps & MMC_MODE_HS_52MHz) {if (mmc->ddr_mode)mmc_select_mode(mmc, MMC_DDR_52); else
mmc->tran_speed = 26000000;
mmc_select_mode(mmc, MMC_HS_52);mmc->tran_speed = 52000000;} else if (mmc->card_caps & MMC_MODE_HS) {mmc_select_mode(mmc, MMC_HS);mmc->tran_speed = 26000000; } return err;@@ -1529,7 +1566,9 @@ static int mmc_startup(struct mmc *mmc) freq = fbase[(cmd.response[0] & 0x7)]; mult = multipliers[((cmd.response[0] >> 3) & 0xf)];
mmc->tran_speed = freq * mult;
mmc->legacy_speed = freq * mult;mmc->tran_speed = mmc->legacy_speed;mmc_select_mode(mmc, MMC_LEGACY); mmc->dsr_imp = ((cmd.response[1] >> 12) & 0x1); mmc->read_bl_len = 1 << ((cmd.response[1] >> 16) & 0xf);diff --git a/include/mmc.h b/include/mmc.h index 9af6b52..60a43b0 100644 --- a/include/mmc.h +++ b/include/mmc.h @@ -52,12 +52,15 @@ #define MMC_VERSION_5_0 MAKE_MMC_VERSION(5, 0, 0) #define MMC_VERSION_5_1 MAKE_MMC_VERSION(5, 1, 0)
-#define MMC_MODE_HS (1 << 0) -#define MMC_MODE_HS_52MHz (1 << 1) -#define MMC_MODE_4BIT (1 << 2) -#define MMC_MODE_8BIT (1 << 3) -#define MMC_MODE_SPI (1 << 4) -#define MMC_MODE_DDR_52MHz (1 << 5) +#define MMC_CAP(mode) (1 << mode) +#define MMC_MODE_HS (MMC_CAP(MMC_HS) | MMC_CAP(SD_HS)) +#define MMC_MODE_HS_52MHz MMC_CAP(MMC_HS_52) +#define MMC_MODE_DDR_52MHz MMC_CAP(MMC_DDR_52)
+#define MMC_MODE_8BIT (1 << 30) +#define MMC_MODE_4BIT (1 << 29) +#define MMC_MODE_SPI (1 << 27)
#define SD_DATA_4BIT 0x00040000
@@ -402,6 +405,23 @@ struct sd_ssr { unsigned int erase_offset; /* In milliseconds */ };
+enum bus_mode {
MMC_LEGACY = 0,SD_LEGACY = 1,MMC_HS = 2,SD_HS = 3,UHS_SDR12 = 4,UHS_SDR25 = 5,UHS_SDR50 = 6,UHS_SDR104 = 7,UHS_DDR50 = 8,MMC_HS_52 = 9,MMC_DDR_52 = 10,MMC_HS_200 = 11,
Do you need to specify the values or would defaults be OK?
MMC_MODES_END+};
+const char *mmc_mode_name(enum bus_mode mode); /*
- With CONFIG_DM_MMC enabled, struct mmc can be accessed from the MMC device
- with mmc_get_mmc_dev().
@@ -432,6 +452,7 @@ struct mmc { u8 wr_rel_set; char part_config; uint tran_speed;
uint legacy_speed;
Please add comment. Should this be an enum?
uint read_bl_len; uint write_bl_len; uint erase_grp_size; /* in 512-byte sectors */@@ -455,6 +476,7 @@ struct mmc { struct udevice *dev; /* Device for this MMC controller */ #endif u8 *ext_csd;
enum bus_mode selected_mode;};
struct mmc_hwpart_conf {
1.9.1
Regards, Simon