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

2023-10-10 Thread Marek Vasut

On 10/10/23 08:51, Heinrich Schuchardt wrote:



Am 10. Oktober 2023 01:14:58 MESZ schrieb Marek Vasut :

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.


Hello Marek,

could you, please, update doc/usage/cmd/mmc.rst.


Should be fixed in V2, thanks.


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

2023-10-10 Thread Heinrich Schuchardt



Am 10. Oktober 2023 01:14:58 MESZ schrieb Marek Vasut :
>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.

Hello Marek,

could you, please, update doc/usage/cmd/mmc.rst.

Best regards

Heinrich

>
>Signed-off-by: Marek Vasut 
>---
>Cc: Abdellatif El Khlifi 
>Cc: Heinrich Schuchardt 
>Cc: Ilias Apalodimas 
>Cc: Jaehoon Chung 
>Cc: Ramon Fried 
>Cc: Roger Knecht 
>Cc: Sean Edmond 
>Cc: Simon Glass 
>Cc: Tobias Waldekranz 
>---
> cmd/Kconfig |  8 +
> cmd/mmc.c   | 96 +
> 2 files changed, 104 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, "", ""),
>@@ 

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

2020-06-24 Thread Peng Fan
> Subject: Re: [PATCH] cmd: mmc: Add mmc reg read command for reading card
> registers
> 
> On 6/24/20 7:40 AM, Peng Fan wrote:
> > Hi Marek,
> 
> Hi,
> 
> >> Subject: [PATCH] cmd: mmc: Add mmc reg read command for reading card
> >> registers
> >
> > This patch breaks ci build.
> >
> > +u-boot.imx exceeds file size limit:
> > +  limit:  0x5fc00 bytes
> > +  actual: 0x60c00 bytes
> > +  excess: 0x1000 bytes
> > +make[1]: *** [Makefile:1204: u-boot.imx] Error 1
> > +make[1]: *** Deleting file 'u-boot.imx'
> > +make: *** [Makefile:167: sub-make] Error 2
> 
> On which board ?

arm:  +   tbs2910
Regards,
Peng.


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

2020-06-24 Thread Marek Vasut
On 6/24/20 7:40 AM, Peng Fan wrote:
> Hi Marek,

Hi,

>> Subject: [PATCH] cmd: mmc: Add mmc reg read command for reading card
>> registers
> 
> This patch breaks ci build.
> 
> +u-boot.imx exceeds file size limit:
> +  limit:  0x5fc00 bytes
> +  actual: 0x60c00 bytes
> +  excess: 0x1000 bytes
> +make[1]: *** [Makefile:1204: u-boot.imx] Error 1
> +make[1]: *** Deleting file 'u-boot.imx'
> +make: *** [Makefile:167: sub-make] Error 2

On which board ?


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

2020-06-23 Thread Peng Fan
Hi Marek,

> Subject: [PATCH] cmd: mmc: Add mmc reg read command for reading card
> registers

This patch breaks ci build.

+u-boot.imx exceeds file size limit:
+  limit:  0x5fc00 bytes
+  actual: 0x60c00 bytes
+  excess: 0x1000 bytes
+make[1]: *** [Makefile:1204: u-boot.imx] Error 1
+make[1]: *** Deleting file 'u-boot.imx'
+make: *** [Makefile:167: sub-make] Error 2

Regards,
Peng.

> 
> 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 
> ---
>  cmd/Kconfig |  8 +
>  cmd/mmc.c   | 94
> +
>  2 files changed, 102 insertions(+)
> 
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 192b3b262f..a0cf03c911 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -1104,6 +1104,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 y
> + 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 1529a3e05d..55fbfe822e 100644
> --- a/cmd/mmc.c
> +++ b/cmd/mmc.c
> @@ -912,6 +912,93 @@ static int do_mmc_bkops_enable(struct cmd_tbl
> *cmdtp, int flag,  }  #endif
> 
> +#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;
> + } else {
> + off = simple_strtoul(argv[3], NULL, 10);
> + if (!strcmp(argv[2], "cid")) {
> + if (off > 1)
> + 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);
> + 

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

2020-06-21 Thread Michael Nazzareno Trimarchi
Hi

On Sat, Jun 20, 2020 at 2:35 PM Michael Nazzareno Trimarchi
 wrote:
>
> Hi
>
> On Sat, Jun 20, 2020 at 2:30 PM 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 
> > ---
> >  cmd/Kconfig |  8 +
> >  cmd/mmc.c   | 94 +
> >  2 files changed, 102 insertions(+)
> >
> > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > index 192b3b262f..a0cf03c911 100644
> > --- a/cmd/Kconfig
> > +++ b/cmd/Kconfig
> > @@ -1104,6 +1104,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 y
> > +   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 1529a3e05d..55fbfe822e 100644
> > --- a/cmd/mmc.c
> > +++ b/cmd/mmc.c
> > @@ -912,6 +912,93 @@ static int do_mmc_bkops_enable(struct cmd_tbl *cmdtp, 
> > int flag,
> >  }
> >  #endif
> >
> > +#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;
> > +   } else {
>
> else is not needed
>
> > +   off = simple_strtoul(argv[3], NULL, 10);
> > +   if (!strcmp(argv[2], "cid")) {
> > +   if (off > 1)
> > +   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) {
>
> Can you factorize all the conditions?
> string, function
>
> Even use strcmp("extcsd", arg

Sorry for the last comment, I was a bit sleeping

Michael

>
> Michael
>
> > +   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);
> > +

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

2020-06-20 Thread Michael Nazzareno Trimarchi
Hi

On Sat, Jun 20, 2020 at 2:30 PM 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 
> ---
>  cmd/Kconfig |  8 +
>  cmd/mmc.c   | 94 +
>  2 files changed, 102 insertions(+)
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 192b3b262f..a0cf03c911 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -1104,6 +1104,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 y
> +   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 1529a3e05d..55fbfe822e 100644
> --- a/cmd/mmc.c
> +++ b/cmd/mmc.c
> @@ -912,6 +912,93 @@ static int do_mmc_bkops_enable(struct cmd_tbl *cmdtp, 
> int flag,
>  }
>  #endif
>
> +#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;
> +   } else {

else is not needed

> +   off = simple_strtoul(argv[3], NULL, 10);
> +   if (!strcmp(argv[2], "cid")) {
> +   if (off > 1)
> +   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) {

Can you factorize all the conditions?
string, function

Even use strcmp("extcsd", arg

Michael

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