[PATCH v2] cmd: mmc: Add mmc reg read command for reading card registers
Marek Vasut
marex at denx.de
Tue Oct 31 08:52:54 CET 2023
On 10/31/23 07:29, Jaehoon Chung wrote:
> Hi,
>
>> -----Original Message-----
>> From: U-Boot <u-boot-bounces at lists.denx.de> On Behalf Of Jaehoon Chung
>> Sent: Tuesday, October 31, 2023 3:08 PM
>> To: Marek Vasut <marex at denx.de>; u-boot at lists.denx.de
>> Cc: Abdellatif El Khlifi <abdellatif.elkhlifi at arm.com>; Heinrich Schuchardt <xypron.glpk at gmx.de>;
>> Ilias Apalodimas <ilias.apalodimas at linaro.org>; Ramon Fried <rfried.dev at gmail.com>; Roger Knecht
>> <rknecht at pm.me>; Sean Edmond <seanedmond at microsoft.com>; Simon Glass <sjg at chromium.org>; Tobias
>> Waldekranz <tobias at waldekranz.com>
>> Subject: Re: [PATCH v2] cmd: mmc: Add mmc reg read command for reading card registers
>>
>> Hi Marek,
>>
>> On 10/10/23 21:47, Marek Vasut wrote:
>>> Add extension to the 'mmc' command to read out the card registers.
>>> Currently, only the eMMC OCR/CID/CSD/EXTCSD/RCA/DSR register are
>>> supported. A register value can either be displayed or read into
>>> an environment variable.
>>>
>>> Signed-off-by: Marek Vasut <marex at denx.de>
>>> ---
>>> Cc: Abdellatif El Khlifi <abdellatif.elkhlifi at arm.com>
>>> Cc: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>> Cc: Ilias Apalodimas <ilias.apalodimas at linaro.org>
>>> Cc: Jaehoon Chung <jh80.chung at samsung.com>
>>> Cc: Ramon Fried <rfried.dev at gmail.com>
>>> Cc: Roger Knecht <rknecht at pm.me>
>>> Cc: Sean Edmond <seanedmond at microsoft.com>
>>> Cc: Simon Glass <sjg at chromium.org>
>>> Cc: Tobias Waldekranz <tobias at waldekranz.com>
>>
>> Looks good to me. I have tested with your patch on my target.
>>
>> mmc reg read cid all
>> CID[0]: 0x15010042
>> => mmc reg read extcsd all
>> EXT_CSD:
>> 000: 00 00 00 00 00 00 00 00 00 00
>> 010: 00 00 00 00 00 00 39 00 00 00
>> ..[snip]..
>>
>> Tested-by: Jaehoon Chung <jh80.chung at samsung.com>
>> Reviewed-by: Jaehoon Chung <jh80.chung at samsung.com>
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>> ---
>>> V2: - Update documentation
>>> ---
>>> cmd/Kconfig | 8 ++++
>>> cmd/mmc.c | 96 +++++++++++++++++++++++++++++++++++++++++++
>>> doc/usage/cmd/mmc.rst | 26 ++++++++++++
>>> 3 files changed, 130 insertions(+)
>>>
>>> diff --git a/cmd/Kconfig b/cmd/Kconfig
>>> index 6470b138d2f..dcd99757a1e 100644
>>> --- a/cmd/Kconfig
>>> +++ b/cmd/Kconfig
>>> @@ -1307,6 +1307,14 @@ config CMD_BKOPS_ENABLE
>>> on a eMMC device. The feature is optionally available on eMMC devices
>>> conforming to standard >= 4.41.
>>>
>>> +config CMD_MMC_REG
>>> + bool "Enable support for reading card registers in the mmc command"
>>> + depends on CMD_MMC
>>> + default n
>>> + help
>>> + Enable the commands for reading card registers. This is useful
>>> + mostly for debugging or extracting details from the card.
>>> +
>>> config CMD_MMC_RPMB
>>> bool "Enable support for RPMB in the mmc command"
>>> depends on SUPPORT_EMMC_RPMB
>>> diff --git a/cmd/mmc.c b/cmd/mmc.c
>>> index c6bd81cebbc..c29f44b7a18 100644
>>> --- a/cmd/mmc.c
>>> +++ b/cmd/mmc.c
>>> @@ -1110,6 +1110,93 @@ static int do_mmc_boot_wp(struct cmd_tbl *cmdtp, int flag,
>>> return CMD_RET_SUCCESS;
>>> }
>>>
>>> +#if CONFIG_IS_ENABLED(CMD_MMC_REG)
>>> +static int do_mmc_reg(struct cmd_tbl *cmdtp, int flag,
>>> + int argc, char *const argv[])
>>> +{
>>> + ALLOC_CACHE_ALIGN_BUFFER(u8, ext_csd, MMC_MAX_BLOCK_LEN);
>>> + struct mmc *mmc;
>>> + int i, ret;
>>> + u32 off;
>>> +
>>> + if (argc < 3 || argc > 5)
>>> + return CMD_RET_USAGE;
>>> +
>>> + mmc = find_mmc_device(curr_device);
>>> + if (!mmc) {
>>> + printf("no mmc device at slot %x\n", curr_device);
>>> + return CMD_RET_FAILURE;
>>> + }
>>> +
>>> + if (IS_SD(mmc)) {
>>> + printf("SD registers are not supported\n");
>>> + return CMD_RET_FAILURE;
>>> + }
>>> +
>>> + off = simple_strtoul(argv[3], NULL, 10);
>>> + if (!strcmp(argv[2], "cid")) {
>>> + if (off > 3)
>>> + return CMD_RET_USAGE;
>>> + printf("CID[%i]: 0x%08x\n", off, mmc->cid[off]);
>>> + if (argv[4])
>>> + env_set_hex(argv[4], mmc->cid[off]);
>>> + return CMD_RET_SUCCESS;
>>> + }
>>> + if (!strcmp(argv[2], "csd")) {
>>> + if (off > 3)
>>> + return CMD_RET_USAGE;
>>> + printf("CSD[%i]: 0x%08x\n", off, mmc->csd[off]);
>>> + if (argv[4])
>>> + env_set_hex(argv[4], mmc->csd[off]);
>>> + return CMD_RET_SUCCESS;
>>> + }
>>> + if (!strcmp(argv[2], "dsr")) {
>>> + printf("DSR: 0x%08x\n", mmc->dsr);
>>> + if (argv[4])
>>> + env_set_hex(argv[4], mmc->dsr);
>>> + return CMD_RET_SUCCESS;
>>> + }
>>> + if (!strcmp(argv[2], "ocr")) {
>>> + printf("OCR: 0x%08x\n", mmc->ocr);
>>> + if (argv[4])
>>> + env_set_hex(argv[4], mmc->ocr);
>>> + return CMD_RET_SUCCESS;
>>> + }
>>> + if (!strcmp(argv[2], "rca")) {
>>> + printf("RCA: 0x%08x\n", mmc->rca);
>>> + if (argv[4])
>>> + env_set_hex(argv[4], mmc->rca);
>>> + return CMD_RET_SUCCESS;
>>> + }
>>> + if (!strcmp(argv[2], "extcsd") &&
>>> + mmc->version >= MMC_VERSION_4_41) {
>>> + ret = mmc_send_ext_csd(mmc, ext_csd);
>>> + if (ret)
>>> + return ret;
>>> + if (!strcmp(argv[3], "all")) {
>>> + /* Dump the entire register */
>>> + printf("EXT_CSD:");
>>> + for (i = 0; i < MMC_MAX_BLOCK_LEN; i++) {
>>> + if (!(i % 10))
>>> + printf("\n%03i: ", i);
>>> + printf(" %02x", ext_csd[i]);
>>> + }
>>> + printf("\n");
>>> + return CMD_RET_SUCCESS;
>>> + }
>>> + off = simple_strtoul(argv[3], NULL, 10);
>>> + if (off > 512)
>>> + return CMD_RET_USAGE;
>>> + printf("EXT_CSD[%i]: 0x%02x\n", off, ext_csd[off]);
>>> + if (argv[4])
>>> + env_set_hex(argv[4], ext_csd[off]);
>>> + return CMD_RET_SUCCESS;
>>> + }
>>> +
>>> + return CMD_RET_FAILURE;
>>> +}
>>> +#endif
>>> +
>>> static struct cmd_tbl cmd_mmc[] = {
>>> U_BOOT_CMD_MKENT(info, 1, 0, do_mmcinfo, "", ""),
>>> U_BOOT_CMD_MKENT(read, 4, 1, do_mmc_read, "", ""),
>>> @@ -1142,6 +1229,9 @@ static struct cmd_tbl cmd_mmc[] = {
>>> U_BOOT_CMD_MKENT(bkops-enable, 2, 0, do_mmc_bkops_enable, "", ""),
>>> U_BOOT_CMD_MKENT(bkops, 4, 0, do_mmc_bkops, "", ""),
>>> #endif
>>> +#if CONFIG_IS_ENABLED(CMD_MMC_REG)
>>> + U_BOOT_CMD_MKENT(reg, 5, 0, do_mmc_reg, "", ""),
>>> +#endif
>>> };
>>>
>>> static int do_mmcops(struct cmd_tbl *cmdtp, int flag, int argc,
>>> @@ -1229,6 +1319,12 @@ U_BOOT_CMD(
>>> " WARNING: This is a write-once setting.\n"
>>> "mmc bkops <dev> [auto|manual] [enable|disable]\n"
>>> " - configure background operations handshake on device\n"
>>> +#endif
>>> +#if CONFIG_IS_ENABLED(CMD_MMC_REG)
>>> + "mmc reg read <reg> <offset> [env] - read card register <reg> offset <offset>\n"
>>> + " (optionally into [env] variable)\n"
>>> + " - reg: cid/csd/dsr/ocr/rca/extcsd\n"
>>> + " - offset: for cid/csd [0..3], for extcsd [0..511,all]\n"
>
> Is there any reason to add "all" option for cid/csd?
CID/CSD are simple and the "all" loop can be easily implemented on
command line even without setexpr. With extcsd I often want to get the
whole dump for debug purposes and it is not so easy for the whole block,
hence the 'all' .
More information about the U-Boot
mailing list