On 8/30/21 1:18 PM, Aswath Govindraju wrote: > Hi Jaehoon, > > On 28/08/21 10:09 am, Jaehoon Chung wrote: >> 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" > [1] > >>> + " [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? >> > > Yes, that is correct. The index of the speed mode has be provided[1].
Thanks! It's my mis-reading. :) It's working fine. Reviewed-by: Jaehoon Chung <jh80.ch...@samsung.com> Tested-by: Jaehoon Chung <jh80.ch...@samsung.com> Best Regards, Jaehoon Chung > > Thanks, > Aswath > >> 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) >>> >> > >