[U-Boot] [PATCH 05/22] mmc: introduces mmc modes.

Simon Glass sjg at chromium.org
Mon May 15 03:28:03 UTC 2017


On 12 May 2017 at 12:16, Jean-Jacques Hiblot <jjhiblot at 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 at ti.com>
> ---
>  drivers/mmc/mmc.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++-------
>  include/mmc.h     | 34 ++++++++++++++++++++++++++++------
>  2 files changed, 74 insertions(+), 13 deletions(-)
>

Reviewed-by: Simon Glass <sjg at 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


More information about the U-Boot mailing list