On 12 May 2017 at 12:16, Jean-Jacques Hiblot <jjhib...@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 <jjhib...@ti.com>
> ---
> drivers/mmc/mmc.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++-------
> include/mmc.h | 34 ++++++++++++++++++++++++++++------
> 2 files changed, 74 insertions(+), 13 deletions(-)
>
Reviewed-by: Simon Glass <s...@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";
> + else
> + return 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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
