diff mbox series

[v2] cmd: mmc: Add mmc reg read command for reading card registers

Message ID 20231010124728.7930-1-marex@denx.de
State Superseded
Delegated to: Jaehoon Chung
Headers show
Series [v2] cmd: mmc: Add mmc reg read command for reading card registers | expand

Commit Message

Marek Vasut Oct. 10, 2023, 12:47 p.m. UTC
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@denx.de>
---
Cc: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Cc: Jaehoon Chung <jh80.chung@samsung.com>
Cc: Ramon Fried <rfried.dev@gmail.com>
Cc: Roger Knecht <rknecht@pm.me>
Cc: Sean Edmond <seanedmond@microsoft.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Tobias Waldekranz <tobias@waldekranz.com>
---
V2: - Update documentation
---
 cmd/Kconfig           |  8 ++++
 cmd/mmc.c             | 96 +++++++++++++++++++++++++++++++++++++++++++
 doc/usage/cmd/mmc.rst | 26 ++++++++++++
 3 files changed, 130 insertions(+)

Comments

Jaehoon Chung Oct. 31, 2023, 6:08 a.m. UTC | #1
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@denx.de>
> ---
> Cc: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Cc: Jaehoon Chung <jh80.chung@samsung.com>
> Cc: Ramon Fried <rfried.dev@gmail.com>
> Cc: Roger Knecht <rknecht@pm.me>
> Cc: Sean Edmond <seanedmond@microsoft.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Tobias Waldekranz <tobias@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@samsung.com>
Reviewed-by: Jaehoon Chung <jh80.chung@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"
>  #endif
>  	);
>  
> diff --git a/doc/usage/cmd/mmc.rst b/doc/usage/cmd/mmc.rst
> index 71a0303109c..c0924ba5769 100644
> --- a/doc/usage/cmd/mmc.rst
> +++ b/doc/usage/cmd/mmc.rst
> @@ -21,6 +21,7 @@ Synopsis
>      mmc bootpart-resize <dev> <dev part size MB> <RPMB part size MB>
>      mmc partconf <dev> [[varname] | [<boot_ack> <boot_partition> <partition_access>]]
>      mmc rst-function <dev> <value>
> +    mmc reg read <reg> <offset> [env]
>  
>  Description
>  -----------
> @@ -183,6 +184,31 @@ The 'mmc rst-function' command changes the RST_n_FUNCTION field.
>          0x3
>              Reserved
>  
> +The 'mmc reg read <reg> <offset> [env]' reads eMMC card register and
> +either print it to standard output, or store the value in environment
> +variable.
> +
> +<reg> with
> +optional offset <offset> into the register array, and print it to
> +standard output or store it into environment variable [env].
> +
> +    reg
> +        cid
> +            The Device IDentification (CID) register. Uses offset.
> +        csd
> +            The Device-Specific Data (CSD) register. Uses offset.
> +        dsr
> +            The driver stage register (DSR).
> +        ocr
> +            The operation conditions register (OCR).
> +        rca
> +            The relative Device address (RCA) register.
> +        extcsd
> +            The Extended CSD register. Uses offset.
> +    offset
> +        For 'cid'/'csd' 128 bit registers '[0..3]' in 32-bit increments. For 'extcsd' 512 bit register '[0..512,all]' in 8-bit increments, or 'all' to read the entire register.
> +    env
> +        Optional environment variable into which 32-bit value read from register should be stored.
>  
>  Examples
>  --------
Jaehoon Chung Oct. 31, 2023, 6:29 a.m. UTC | #2
Hi,

> -----Original Message-----
> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Jaehoon Chung
> Sent: Tuesday, October 31, 2023 3:08 PM
> To: Marek Vasut <marex@denx.de>; u-boot@lists.denx.de
> Cc: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>; Heinrich Schuchardt <xypron.glpk@gmx.de>;
> Ilias Apalodimas <ilias.apalodimas@linaro.org>; Ramon Fried <rfried.dev@gmail.com>; Roger Knecht
> <rknecht@pm.me>; Sean Edmond <seanedmond@microsoft.com>; Simon Glass <sjg@chromium.org>; Tobias
> Waldekranz <tobias@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@denx.de>
> > ---
> > Cc: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > Cc: Jaehoon Chung <jh80.chung@samsung.com>
> > Cc: Ramon Fried <rfried.dev@gmail.com>
> > Cc: Roger Knecht <rknecht@pm.me>
> > Cc: Sean Edmond <seanedmond@microsoft.com>
> > Cc: Simon Glass <sjg@chromium.org>
> > Cc: Tobias Waldekranz <tobias@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@samsung.com>
> Reviewed-by: Jaehoon Chung <jh80.chung@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?

Best Regards,
Jaehoon Chung

> >  #endif
> >  	);
> >
> > diff --git a/doc/usage/cmd/mmc.rst b/doc/usage/cmd/mmc.rst
> > index 71a0303109c..c0924ba5769 100644
> > --- a/doc/usage/cmd/mmc.rst
> > +++ b/doc/usage/cmd/mmc.rst
> > @@ -21,6 +21,7 @@ Synopsis
> >      mmc bootpart-resize <dev> <dev part size MB> <RPMB part size MB>
> >      mmc partconf <dev> [[varname] | [<boot_ack> <boot_partition> <partition_access>]]
> >      mmc rst-function <dev> <value>
> > +    mmc reg read <reg> <offset> [env]
> >
> >  Description
> >  -----------
> > @@ -183,6 +184,31 @@ The 'mmc rst-function' command changes the RST_n_FUNCTION field.
> >          0x3
> >              Reserved
> >
> > +The 'mmc reg read <reg> <offset> [env]' reads eMMC card register and
> > +either print it to standard output, or store the value in environment
> > +variable.
> > +
> > +<reg> with
> > +optional offset <offset> into the register array, and print it to
> > +standard output or store it into environment variable [env].
> > +
> > +    reg
> > +        cid
> > +            The Device IDentification (CID) register. Uses offset.
> > +        csd
> > +            The Device-Specific Data (CSD) register. Uses offset.
> > +        dsr
> > +            The driver stage register (DSR).
> > +        ocr
> > +            The operation conditions register (OCR).
> > +        rca
> > +            The relative Device address (RCA) register.
> > +        extcsd
> > +            The Extended CSD register. Uses offset.
> > +    offset
> > +        For 'cid'/'csd' 128 bit registers '[0..3]' in 32-bit increments. For 'extcsd' 512 bit
> register '[0..512,all]' in 8-bit increments, or 'all' to read the entire register.
> > +    env
> > +        Optional environment variable into which 32-bit value read from register should be stored.
> >
> >  Examples
> >  --------
Marek Vasut Oct. 31, 2023, 7:52 a.m. UTC | #3
On 10/31/23 07:29, Jaehoon Chung wrote:
> Hi,
> 
>> -----Original Message-----
>> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Jaehoon Chung
>> Sent: Tuesday, October 31, 2023 3:08 PM
>> To: Marek Vasut <marex@denx.de>; u-boot@lists.denx.de
>> Cc: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>; Heinrich Schuchardt <xypron.glpk@gmx.de>;
>> Ilias Apalodimas <ilias.apalodimas@linaro.org>; Ramon Fried <rfried.dev@gmail.com>; Roger Knecht
>> <rknecht@pm.me>; Sean Edmond <seanedmond@microsoft.com>; Simon Glass <sjg@chromium.org>; Tobias
>> Waldekranz <tobias@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@denx.de>
>>> ---
>>> Cc: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
>>> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
>>> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>>> Cc: Jaehoon Chung <jh80.chung@samsung.com>
>>> Cc: Ramon Fried <rfried.dev@gmail.com>
>>> Cc: Roger Knecht <rknecht@pm.me>
>>> Cc: Sean Edmond <seanedmond@microsoft.com>
>>> Cc: Simon Glass <sjg@chromium.org>
>>> Cc: Tobias Waldekranz <tobias@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@samsung.com>
>> Reviewed-by: Jaehoon Chung <jh80.chung@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' .
Lothar Waßmann Oct. 31, 2023, 8:26 a.m. UTC | #4
Hi,

On Tue, 10 Oct 2023 14:47:28 +0200 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@denx.de>
> ---
> Cc: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Cc: Jaehoon Chung <jh80.chung@samsung.com>
> Cc: Ramon Fried <rfried.dev@gmail.com>
> Cc: Roger Knecht <rknecht@pm.me>
> Cc: Sean Edmond <seanedmond@microsoft.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Tobias Waldekranz <tobias@waldekranz.com>
> ---
> V2: - Update documentation
> ---
>  cmd/Kconfig           |  8 ++++
>  cmd/mmc.c             | 96 +++++++++++++++++++++++++++++++++++++++++++
>  doc/usage/cmd/mmc.rst | 26 ++++++++++++
>  3 files changed, 130 insertions(+)
> 
[...]
> +	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;
			return CMD_RET_FAILURE;


Lothar Waßmann
Marek Vasut Oct. 31, 2023, 11:33 a.m. UTC | #5
On 10/31/23 09:26, Lothar Waßmann wrote:
> Hi,
> 
> On Tue, 10 Oct 2023 14:47:28 +0200 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@denx.de>
>> ---
>> Cc: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
>> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>> Cc: Jaehoon Chung <jh80.chung@samsung.com>
>> Cc: Ramon Fried <rfried.dev@gmail.com>
>> Cc: Roger Knecht <rknecht@pm.me>
>> Cc: Sean Edmond <seanedmond@microsoft.com>
>> Cc: Simon Glass <sjg@chromium.org>
>> Cc: Tobias Waldekranz <tobias@waldekranz.com>
>> ---
>> V2: - Update documentation
>> ---
>>   cmd/Kconfig           |  8 ++++
>>   cmd/mmc.c             | 96 +++++++++++++++++++++++++++++++++++++++++++
>>   doc/usage/cmd/mmc.rst | 26 ++++++++++++
>>   3 files changed, 130 insertions(+)
>>
> [...]
>> +	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;
> 			return CMD_RET_FAILURE;
> 

Fixed in V3, thanks .
diff mbox series

Patch

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"
 #endif
 	);
 
diff --git a/doc/usage/cmd/mmc.rst b/doc/usage/cmd/mmc.rst
index 71a0303109c..c0924ba5769 100644
--- a/doc/usage/cmd/mmc.rst
+++ b/doc/usage/cmd/mmc.rst
@@ -21,6 +21,7 @@  Synopsis
     mmc bootpart-resize <dev> <dev part size MB> <RPMB part size MB>
     mmc partconf <dev> [[varname] | [<boot_ack> <boot_partition> <partition_access>]]
     mmc rst-function <dev> <value>
+    mmc reg read <reg> <offset> [env]
 
 Description
 -----------
@@ -183,6 +184,31 @@  The 'mmc rst-function' command changes the RST_n_FUNCTION field.
         0x3
             Reserved
 
+The 'mmc reg read <reg> <offset> [env]' reads eMMC card register and
+either print it to standard output, or store the value in environment
+variable.
+
+<reg> with
+optional offset <offset> into the register array, and print it to
+standard output or store it into environment variable [env].
+
+    reg
+        cid
+            The Device IDentification (CID) register. Uses offset.
+        csd
+            The Device-Specific Data (CSD) register. Uses offset.
+        dsr
+            The driver stage register (DSR).
+        ocr
+            The operation conditions register (OCR).
+        rca
+            The relative Device address (RCA) register.
+        extcsd
+            The Extended CSD register. Uses offset.
+    offset
+        For 'cid'/'csd' 128 bit registers '[0..3]' in 32-bit increments. For 'extcsd' 512 bit register '[0..512,all]' in 8-bit increments, or 'all' to read the entire register.
+    env
+        Optional environment variable into which 32-bit value read from register should be stored.
 
 Examples
 --------