Hi Aswath,

On 7/30/21 9:26 PM, 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.
> 
> Signed-off-by: Aswath Govindraju <a-govindr...@ti.com>
> ---
> 
> Changes since v1:
> - Removed #ifdef around the definition of user_speed_mode field
> - Replaced #if with if (IS_ENABLED())
> - Removed repeated MMC_CAP(MMC_LEGACY)
> - initialized user_speed_mode to MMC_MODES_SPEED in mmc_init_device
>   and mmc_dp_preinit
> - corrected curr_device to dev and included part check, the case when
>   mode is provided in the argument
> 
>  cmd/Kconfig              | 10 ++++++
>  cmd/mmc.c                | 75 +++++++++++++++++++++++++++++-----------
>  doc/usage/mmc.rst        | 49 ++++++++++++++++++++++++--
>  drivers/mmc/mmc-uclass.c |  5 ++-
>  drivers/mmc/mmc.c        | 22 +++++++++++-
>  include/mmc.h            |  2 ++
>  6 files changed, 138 insertions(+), 25 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 b942576b58ac..e256210a3705 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)

how about using wrapper function?

static struct mmc *__init_mmc_device(int dev, bool force_init,
                                   enum bus_mode spped_mode)
{
        ...
}

static struct mmc *init_mmc_device(int dev, bool force_init)
{
        return __init_mmc_device(dev, force_init, MMC_MODES_END);
}

Then it doesn't need to touch all init_mmc_device caller.
Just my opinion :)

Best Regards,
Jaehoon Chung

>  {
>       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;
>  
> @@ -159,7 +165,7 @@ static int do_mmcinfo(struct cmd_tbl *cmdtp, int flag, 
> int argc,
>               }
>       }
>  
> -     mmc = init_mmc_device(curr_device, false);
> +     mmc = init_mmc_device(curr_device, false, MMC_MODES_END);
>       if (!mmc)
>               return CMD_RET_FAILURE;
>  
> @@ -293,7 +299,7 @@ static int do_mmcrpmb(struct cmd_tbl *cmdtp, int flag,
>       if (flag == CMD_FLAG_REPEAT && !cmd_is_repeatable(cp))
>               return CMD_RET_SUCCESS;
>  
> -     mmc = init_mmc_device(curr_device, false);
> +     mmc = init_mmc_device(curr_device, false, MMC_MODES_END);
>       if (!mmc)
>               return CMD_RET_FAILURE;
>  
> @@ -338,7 +344,7 @@ static int do_mmc_read(struct cmd_tbl *cmdtp, int flag,
>       blk = simple_strtoul(argv[2], NULL, 16);
>       cnt = simple_strtoul(argv[3], NULL, 16);
>  
> -     mmc = init_mmc_device(curr_device, false);
> +     mmc = init_mmc_device(curr_device, false, MMC_MODES_END);
>       if (!mmc)
>               return CMD_RET_FAILURE;
>  
> @@ -387,7 +393,7 @@ static int do_mmc_sparse_write(struct cmd_tbl *cmdtp, int 
> flag,
>               return CMD_RET_FAILURE;
>       }
>  
> -     mmc = init_mmc_device(curr_device, false);
> +     mmc = init_mmc_device(curr_device, false, MMC_MODES_END);
>       if (!mmc)
>               return CMD_RET_FAILURE;
>  
> @@ -431,7 +437,7 @@ static int do_mmc_write(struct cmd_tbl *cmdtp, int flag,
>       blk = simple_strtoul(argv[2], NULL, 16);
>       cnt = simple_strtoul(argv[3], NULL, 16);
>  
> -     mmc = init_mmc_device(curr_device, false);
> +     mmc = init_mmc_device(curr_device, false, MMC_MODES_END);
>       if (!mmc)
>               return CMD_RET_FAILURE;
>  
> @@ -460,7 +466,7 @@ static int do_mmc_erase(struct cmd_tbl *cmdtp, int flag,
>       blk = simple_strtoul(argv[1], NULL, 16);
>       cnt = simple_strtoul(argv[2], NULL, 16);
>  
> -     mmc = init_mmc_device(curr_device, false);
> +     mmc = init_mmc_device(curr_device, false, MMC_MODES_END);
>       if (!mmc)
>               return CMD_RET_FAILURE;
>  
> @@ -482,8 +488,19 @@ 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, MMC_MODES_END);
> +     } else if (argc == 2) {
> +             /* parse the mode */
> +             speed_mode = simple_strtoul(argv[1], NULL, 10);
> +             /* Need to pass the the speed mode required too */
> +             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;
>  
> @@ -496,7 +513,7 @@ static int do_mmc_part(struct cmd_tbl *cmdtp, int flag,
>       struct blk_desc *mmc_dev;
>       struct mmc *mmc;
>  
> -     mmc = init_mmc_device(curr_device, false);
> +     mmc = init_mmc_device(curr_device, false, MMC_MODES_END);
>       if (!mmc)
>               return CMD_RET_FAILURE;
>  
> @@ -515,11 +532,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, MMC_MODES_END);
>       } else if (argc == 2) {
>               dev = simple_strtoul(argv[1], NULL, 10);
> +             mmc = init_mmc_device(dev, true, MMC_MODES_END);
>       } else if (argc == 3) {
>               dev = (int)simple_strtoul(argv[1], NULL, 10);
>               part = (int)simple_strtoul(argv[2], NULL, 10);
> @@ -528,11 +548,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, MMC_MODES_END);
> +     } else if (argc == 4) {
> +             dev = (int)simple_strtoul(argv[1], NULL, 10);
> +             part = (int)simple_strtoul(argv[2], NULL, 10);
> +             if (part > PART_ACCESS_MASK) {
> +                     printf("#part_num shouldn't be larger than %d\n",
> +                            PART_ACCESS_MASK);
> +                     return CMD_RET_FAILURE;
> +             }
> +             speed_mode = simple_strtoul(argv[3], NULL, 10);
> +             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;
>  
> @@ -636,7 +666,7 @@ static int do_mmc_hwpartition(struct cmd_tbl *cmdtp, int 
> flag,
>       enum mmc_hwpart_conf_mode mode = MMC_HWPART_CONF_CHECK;
>       int i, r, pidx;
>  
> -     mmc = init_mmc_device(curr_device, false);
> +     mmc = init_mmc_device(curr_device, false, MMC_MODES_END);
>       if (!mmc)
>               return CMD_RET_FAILURE;
>  
> @@ -726,7 +756,7 @@ static int do_mmc_bootbus(struct cmd_tbl *cmdtp, int flag,
>       reset = simple_strtoul(argv[3], NULL, 10);
>       mode = simple_strtoul(argv[4], NULL, 10);
>  
> -     mmc = init_mmc_device(dev, false);
> +     mmc = init_mmc_device(dev, false, MMC_MODES_END);
>       if (!mmc)
>               return CMD_RET_FAILURE;
>  
> @@ -789,7 +819,7 @@ static int do_mmc_boot_resize(struct cmd_tbl *cmdtp, int 
> flag,
>       bootsize = simple_strtoul(argv[2], NULL, 10);
>       rpmbsize = simple_strtoul(argv[3], NULL, 10);
>  
> -     mmc = init_mmc_device(dev, false);
> +     mmc = init_mmc_device(dev, false, MMC_MODES_END);
>       if (!mmc)
>               return CMD_RET_FAILURE;
>  
> @@ -844,7 +874,7 @@ static int do_mmc_partconf(struct cmd_tbl *cmdtp, int 
> flag,
>  
>       dev = simple_strtoul(argv[1], NULL, 10);
>  
> -     mmc = init_mmc_device(dev, false);
> +     mmc = init_mmc_device(dev, false, MMC_MODES_END);
>       if (!mmc)
>               return CMD_RET_FAILURE;
>  
> @@ -887,7 +917,7 @@ static int do_mmc_rst_func(struct cmd_tbl *cmdtp, int 
> flag,
>               return CMD_RET_USAGE;
>       }
>  
> -     mmc = init_mmc_device(dev, false);
> +     mmc = init_mmc_device(dev, false, MMC_MODES_END);
>       if (!mmc)
>               return CMD_RET_FAILURE;
>  
> @@ -939,7 +969,7 @@ static int do_mmc_bkops_enable(struct cmd_tbl *cmdtp, int 
> flag,
>  
>       dev = simple_strtoul(argv[1], NULL, 10);
>  
> -     mmc = init_mmc_device(dev, false);
> +     mmc = init_mmc_device(dev, false, MMC_MODES_END);
>       if (!mmc)
>               return CMD_RET_FAILURE;
>  
> @@ -958,7 +988,7 @@ static int do_mmc_boot_wp(struct cmd_tbl *cmdtp, int flag,
>       int err;
>       struct mmc *mmc;
>  
> -     mmc = init_mmc_device(curr_device, false);
> +     mmc = init_mmc_device(curr_device, false, MMC_MODES_END);
>       if (!mmc)
>               return CMD_RET_FAILURE;
>       if (IS_SD(mmc)) {
> @@ -983,9 +1013,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 +1072,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"
>       "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/doc/usage/mmc.rst b/doc/usage/mmc.rst
> index f20efe3d7063..d15b151884fd 100644
> --- a/doc/usage/mmc.rst
> +++ b/doc/usage/mmc.rst
> @@ -12,9 +12,9 @@ Synopsis
>      mmc read addr blk# cnt
>      mmc write addr blk# cnt
>      mmc erase blk# cnt
> -    mmc rescan
> +    mmc rescan [mode]
>      mmc part
> -    mmc dev [dev] [part]
> +    mmc dev [dev] [part] [mode]
>      mmc list
>      mmc wp
>      mmc bootbus <dev> <boot_bus_width> <reset_boot_bus_width> <boot_mode>
> @@ -49,6 +49,27 @@ The 'mmc erase' command erases *cnt* blocks on the MMC 
> device starting at block
>  
>  The 'mmc rescan' command scans the available MMC device.
>  
> +   mode
> +       speed mode to set.
> +       CONFIG_MMC_SPEED_MODE_SET should be enabled. The required speed mode 
> is
> +       passed as the index from the following list.
> +
> +       0   - MMC_LEGACY
> +       1   - MMC_HS
> +       2   - SD_HS
> +       3   - MMC_HS_52
> +       4   - MMC_DDR_52
> +       5   - UHS_SDR12
> +       6   - UHS_SDR25
> +       7   - UHS_SDR50
> +       8   - UHS_DDR50
> +       9   - UHS_SDR104
> +       10  - MMC_HS_200
> +       11  - MMC_HS_400
> +       12  - MMC_HS_400_ES
> +
> +       A speed mode can be set only if it has already been enabled in the 
> device tree
> +
>  The 'mmc part' command displays the list available partition on current mmc 
> device.
>  
>  The 'mmc dev' command shows or set current mmc device.
> @@ -58,6 +79,27 @@ The 'mmc dev' command shows or set current mmc device.
>      part
>          partition number to change
>  
> +   mode
> +       speed mode to set.
> +       CONFIG_MMC_SPEED_MODE_SET should be enabled. The required speed mode 
> is
> +       passed as the index from the following list.
> +
> +       0   - MMC_LEGACY
> +       1   - MMC_HS
> +       2   - SD_HS
> +       3   - MMC_HS_52
> +       4   - MMC_DDR_52
> +       5   - UHS_SDR12
> +       6   - UHS_SDR25
> +       7   - UHS_SDR50
> +       8   - UHS_DDR50
> +       9   - UHS_SDR104
> +       10  - MMC_HS_200
> +       11  - MMC_HS_400
> +       12  - MMC_HS_400_ES
> +
> +       A speed mode can be set only if it has already been enabled in the 
> device tree
> +
>  The 'mmc list' command displays the list available devices.
>  
>  The 'mmc wp' command enables "power on write protect" function for boot 
> partitions.
> @@ -194,6 +236,9 @@ The current device can be shown or set via 'mmc dev' 
> command:
>      => mmc dev 2 0
>      switch to partitions #0, OK
>      mmc2 is current device
> +    => mmc dev 0 1 4
> +    switch to partitions #1, OK
> +    mmc0(part 1) is current device
>  
>  The list of available devices can be shown via 'mmc list' command:
>  ::
> 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..d24cebbe4781 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; /* intput speed mode from user */
>  };
>  
>  #if CONFIG_IS_ENABLED(DM_MMC)
> 

Reply via email to