Hi Aswath,
On 8/14/21 2:34 AM, Aswath Govindraju wrote:
> Add support for enumerating MMC card in a given mode using mmc rescan and
> mmc dev commands. The speed mode is provided as the last argument in these
> commands and is indicated using the index from enum bus_mode in
> include/mmc.h. A speed mode can be set only if it has already been enabled
> in the device tree.
Sorry for replying too late. I have tested with your patch.
It's confused the usage.
>
> Signed-off-by: Aswath Govindraju <a-govindr...@ti.com>
> ---
> cmd/Kconfig | 10 ++++++++
> cmd/mmc.c | 52 +++++++++++++++++++++++++++++++++-------
> drivers/mmc/mmc-uclass.c | 5 +++-
> drivers/mmc/mmc.c | 22 ++++++++++++++++-
> include/mmc.h | 2 ++
> 5 files changed, 81 insertions(+), 10 deletions(-)
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index ffef3cc76ca4..3a857b3f6e2e 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -2389,4 +2389,14 @@ config CMD_UBIFS
> help
> UBIFS is a file system for flash devices which works on top of UBI.
>
> +config MMC_SPEED_MODE_SET
> + bool "set speed mode using mmc command"
> + depends on CMD_MMC
> + default n
> + help
> + Enable setting speed mode using mmc rescan and mmc dev commands.
> + The speed mode is provided as the last argument in these commands
> + and is indicated using the index from enum bus_mode in
> + include/mmc.h. A speed mode can be set only if it has already
> + been enabled in the device tree.
> endmenu
> diff --git a/cmd/mmc.c b/cmd/mmc.c
> index c67ad7624227..f1e30d0cf64b 100644
> --- a/cmd/mmc.c
> +++ b/cmd/mmc.c
> @@ -120,7 +120,9 @@ static void print_mmcinfo(struct mmc *mmc)
> }
> }
> }
> -static struct mmc *init_mmc_device(int dev, bool force_init)
> +
> +static struct mmc *__init_mmc_device(int dev, bool force_init,
> + enum bus_mode speed_mode)
> {
> struct mmc *mmc;
> mmc = find_mmc_device(dev);
> @@ -134,6 +136,10 @@ static struct mmc *init_mmc_device(int dev, bool
> force_init)
>
> if (force_init)
> mmc->has_init = 0;
> +
> + if (IS_ENABLED(CONFIG_MMC_SPEED_MODE_SET))
> + mmc->user_speed_mode = speed_mode;
> +
> if (mmc_init(mmc))
> return NULL;
>
> @@ -145,6 +151,11 @@ static struct mmc *init_mmc_device(int dev, bool
> force_init)
> return mmc;
> }
>
> +static struct mmc *init_mmc_device(int dev, bool force_init)
> +{
> + return __init_mmc_device(dev, force_init, MMC_MODES_END);
> +}
> +
> static int do_mmcinfo(struct cmd_tbl *cmdtp, int flag, int argc,
> char *const argv[])
> {
> @@ -482,8 +493,17 @@ static int do_mmc_rescan(struct cmd_tbl *cmdtp, int flag,
> int argc, char *const argv[])
> {
> struct mmc *mmc;
> + enum bus_mode speed_mode = MMC_MODES_END;
> +
> + if (argc == 1) {
> + mmc = init_mmc_device(curr_device, true);
> + } else if (argc == 2) {
> + speed_mode = (int)dectoul(argv[1], NULL);
> + mmc = __init_mmc_device(curr_device, true, speed_mode);
> + } else {
> + return CMD_RET_USAGE;
> + }
>
> - mmc = init_mmc_device(curr_device, true);
> if (!mmc)
> return CMD_RET_FAILURE;
>
> @@ -515,11 +535,14 @@ static int do_mmc_dev(struct cmd_tbl *cmdtp, int flag,
> {
> int dev, part = 0, ret;
> struct mmc *mmc;
> + enum bus_mode speed_mode = MMC_MODES_END;
>
> if (argc == 1) {
> dev = curr_device;
> + mmc = init_mmc_device(dev, true);
> } else if (argc == 2) {
> - dev = dectoul(argv[1], NULL);
> + dev = (int)dectoul(argv[1], NULL);
> + mmc = init_mmc_device(dev, true);
> } else if (argc == 3) {
> dev = (int)dectoul(argv[1], NULL);
> part = (int)dectoul(argv[2], NULL);
> @@ -528,11 +551,21 @@ static int do_mmc_dev(struct cmd_tbl *cmdtp, int flag,
> PART_ACCESS_MASK);
> return CMD_RET_FAILURE;
> }
> + mmc = init_mmc_device(dev, true);
> + } else if (argc == 4) {
> + dev = (int)dectoul(argv[1], NULL);
> + part = (int)dectoul(argv[2], NULL);
> + if (part > PART_ACCESS_MASK) {
> + printf("#part_num shouldn't be larger than %d\n",
> + PART_ACCESS_MASK);
> + return CMD_RET_FAILURE;
> + }
> + speed_mode = (int)dectoul(argv[3], NULL);
> + mmc = __init_mmc_device(dev, true, speed_mode);
> } else {
> return CMD_RET_USAGE;
> }
>
> - mmc = init_mmc_device(dev, true);
> if (!mmc)
> return CMD_RET_FAILURE;
>
> @@ -983,9 +1016,9 @@ static struct cmd_tbl cmd_mmc[] = {
> #if CONFIG_IS_ENABLED(CMD_MMC_SWRITE)
> U_BOOT_CMD_MKENT(swrite, 3, 0, do_mmc_sparse_write, "", ""),
> #endif
> - U_BOOT_CMD_MKENT(rescan, 1, 1, do_mmc_rescan, "", ""),
> + U_BOOT_CMD_MKENT(rescan, 2, 1, do_mmc_rescan, "", ""),
> U_BOOT_CMD_MKENT(part, 1, 1, do_mmc_part, "", ""),
> - U_BOOT_CMD_MKENT(dev, 3, 0, do_mmc_dev, "", ""),
> + U_BOOT_CMD_MKENT(dev, 4, 0, do_mmc_dev, "", ""),
> U_BOOT_CMD_MKENT(list, 1, 1, do_mmc_list, "", ""),
> #if CONFIG_IS_ENABLED(MMC_HW_PARTITIONING)
> U_BOOT_CMD_MKENT(hwpartition, 28, 0, do_mmc_hwpartition, "", ""),
> @@ -1042,9 +1075,12 @@ U_BOOT_CMD(
> "mmc swrite addr blk#\n"
> #endif
> "mmc erase blk# cnt\n"
> - "mmc rescan\n"
> + "mmc rescan [mode]\n"
> "mmc part - lists available partition on current mmc device\n"
> - "mmc dev [dev] [part] - show or set current mmc device [partition]\n"
> + "mmc dev [dev] [part] [mode] - show or set current mmc device
> [partition] and set mode\n"
> + " - the required speed mode is passed as the index from the following
> list\n"
> + " [MMC_LEGACY, MMC_HS, SD_HS, MMC_HS_52, MMC_DDR_52, UHS_SDR12,
> UHS_SDR25,\n"
> + " UHS_SDR50, UHS_DDR50, UHS_SDR104, MMC_HS_200, MMC_HS_400,
> MMC_HS_400_ES]\n"
If developer doesn't read a doc file, it's difficult to know the number of mode.
Because this usage seems that [mode] can be passed by string.
When i have tested with your patch, I have run the below command.
# mmc rescan MMC_HS_52
- but it's always set to legacy mode. So I thought this was not working fine.
But I have changed to input 3 instead of MMC_HS_52. It's working
# mmc rescan 3
Is my test right?
Best Regards,
Jaehoon Chung
> "mmc list - lists available devices\n"
> "mmc wp - power on write protect boot partitions\n"
> #if CONFIG_IS_ENABLED(MMC_HW_PARTITIONING)
> diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c
> index 0e13238c7e7a..3ee92d03ca23 100644
> --- a/drivers/mmc/mmc-uclass.c
> +++ b/drivers/mmc/mmc-uclass.c
> @@ -342,6 +342,9 @@ void mmc_do_preinit(void)
>
> if (!m)
> continue;
> +
> + m->user_speed_mode = MMC_MODES_END; /* Initialising user set
> speed mode */
> +
> if (m->preinit)
> mmc_start_init(m);
> }
> @@ -414,7 +417,7 @@ int mmc_bind(struct udevice *dev, struct mmc *mmc, const
> struct mmc_config *cfg)
> /* setup initial part type */
> bdesc->part_type = cfg->part_type;
> mmc->dev = dev;
> -
> + mmc->user_speed_mode = MMC_MODES_END;
> return 0;
> }
>
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index 8078a89f18cb..bd348974ae7d 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -2862,7 +2862,25 @@ int mmc_start_init(struct mmc *mmc)
> * timings.
> */
> mmc->host_caps = mmc->cfg->host_caps | MMC_CAP(MMC_LEGACY) |
> - MMC_CAP(MMC_LEGACY) | MMC_MODE_1BIT;
> + MMC_MODE_1BIT;
> +
> + if (IS_ENABLED(CONFIG_MMC_SPEED_MODE_SET)) {
> + if (mmc->user_speed_mode != MMC_MODES_END) {
> + int i;
> + /* set host caps */
> + if (mmc->host_caps & MMC_CAP(mmc->user_speed_mode)) {
> + /* Remove all existing speed capabilities */
> + for (i = MMC_LEGACY; i < MMC_MODES_END; i++)
> + mmc->host_caps &= ~MMC_CAP(i);
> + mmc->host_caps |= (MMC_CAP(mmc->user_speed_mode)
> + | MMC_CAP(MMC_LEGACY) |
> + MMC_MODE_1BIT);
> + } else {
> + pr_err("bus_mode requested is not supported\n");
> + return -EINVAL;
> + }
> + }
> + }
> #if CONFIG_IS_ENABLED(DM_MMC)
> mmc_deferred_probe(mmc);
> #endif
> @@ -3060,6 +3078,8 @@ int mmc_init_device(int num)
> }
>
> m = mmc_get_mmc_dev(dev);
> + m->user_speed_mode = MMC_MODES_END; /* Initialising user set speed mode
> */
> +
> if (!m)
> return 0;
> if (m->preinit)
> diff --git a/include/mmc.h b/include/mmc.h
> index 0bf19de20e52..b92e25534021 100644
> --- a/include/mmc.h
> +++ b/include/mmc.h
> @@ -726,6 +726,8 @@ struct mmc {
> */
> u32 quirks;
> u8 hs400_tuning;
> +
> + enum bus_mode user_speed_mode; /* input speed mode from user */
> };
>
> #if CONFIG_IS_ENABLED(DM_MMC)
>
