[PATCH v2] mmc: Add support for enumerating MMC card in a given mode using mmc command

Aswath Govindraju a-govindraju at ti.com
Wed Aug 4 15:18:28 CEST 2021


Hi Jaehoon,

On 02/08/21 3:26 am, Jaehoon Chung wrote:
> 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-govindraju at 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 :)
> 

Yeah, I think that is good idea. Will post a respin after making this
change.

Also, may I know if you were able test this patch ?

Thanks,
Aswath

> 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)
>>
> 



More information about the U-Boot mailing list