Re: [U-Boot] [PATCH 9/9] COMMON: MMC: Command to support eMMC booting

Hi Amar,

On Mon, Dec 17, 2012 at 3:19 AM, Amar amarendra.xt@samsung.com wrote:

This patch adds commands to open, close and create partitions on eMMC

Signed-off-by: Amar amarendra.xt@samsung.com

common/cmd_mmc.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 100 insertions(+), 1 deletions(-)

diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c index 62a1c22..1fd6b94 100644 --- a/common/cmd_mmc.c +++ b/common/cmd_mmc.c @@ -248,6 +248,102 @@ static int do_mmcops(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) curr_device, mmc->part_num);

            return 0;

•   } else if (strcmp(argv[1], "open") == 0) {
  

•           int dev;
  

•           struct mmc *mmc;
  

•           if (argc == 2)
  

•                   dev = curr_device;
  

•           else if (argc == 3)
  

•                   dev = simple_strtoul(argv[2], NULL, 10);
  

•           else if (argc == 4)
  

•                   return 1;
  
  

Should this be CMD_RET_USAGE? What is returning 1 for?

•           else
  

•                   return CMD_RET_USAGE;
  

•           mmc = find_mmc_device(dev);
  

•           if (!mmc) {
  

•                   printf("no mmc device at slot %x\n", dev);
  

•                   return 1;
  

•           }
  

•           if (IS_SD(mmc)) {
  

•                   printf("SD device cannot be opened/closed\n");
  

•                   return 1;
  

•           }
  

•           if (!(mmc_boot_open(mmc))) {
  

•                   printf("eMMC OPEN Success.!!\n");
  

•                   printf("\t\t\t!!!Notice!!!\n");
  

•                   printf("!You must close eMMC boot Partition"
  

•                                           "after all image
  

writing!\n");

•                   printf("!eMMC boot partition has continuity"
  

•                                           "at image writing
  

time.!\n");

•                   printf("!So, Do not close boot partition, Before,"
  

•                                           "all images is
  

written.!\n");

Maybe:

So, do not close boot partition before all images are written

+ } else {

•                   printf("eMMC OPEN Failed.!!\n");
  
  

Is it eMMC or MMC? Lower case or upper case? Your messages should be consistent. And you don't need the !!! I think.

•           }
  

•           return 0;
  

•   } else if (strcmp(argv[1], "close") == 0) {
  

•           int dev;
  

•           struct mmc *mmc;
  

•           if (argc == 2)
  

•                   dev = curr_device;
  

•           else if (argc == 3)
  

•                   dev = simple_strtoul(argv[2], NULL, 10);
  

•           else if (argc == 4)
  

•                   return 1;
  
  

Same Q here as above.

•           else
  

•                   return CMD_RET_USAGE;
  

•           mmc = find_mmc_device(dev);
  

•           if (!mmc) {
  

•                   printf("no mmc device at slot %x\n", dev);
  

•                   return 1;
  

•           }
  

•           if (IS_SD(mmc)) {
  

•                   printf("SD device cannot be opened/closed\n");
  

•                   return 1;
  

•           }
  

Seems the open/close code is almost the same. Can you combine it?

•           if (!(mmc_boot_close(mmc)))
  

•                   printf("eMMC CLOSE Success.!!\n");
  

•           else
  

•                   printf("eMMC CLOSE Failed.!!\n");
  

•           return 0;
  

•   } else if (strcmp(argv[1], "bootpart") == 0) {
  

•           int dev;
  

•           dev = simple_strtoul(argv[2], NULL, 10);
  

•           struct mmc *mmc = find_mmc_device(dev);
  

•           u32 bootsize = simple_strtoul(argv[3], NULL, 10);
  

•           u32 rpmbsize = simple_strtoul(argv[4], NULL, 10);
  

•           if (!mmc) {
  

•                   printf("no mmc device at slot %x\n", dev);
  

•                   return 1;
  

•           }
  

•           if (IS_SD(mmc)) {
  

•                   printf("It is not a eMMC device\n");
  

•                   return 1;
  

•           }
  

•           if (0 == mmc_boot_partition_size_change(mmc,
  

•                                           bootsize, rpmbsize)) {
  

•                   printf("eMMC boot partition Size %d MB!!\n",
  

bootsize);

•                   printf("eMMC RPMB partition Size %d MB!!\n",
  

rpmbsize);

•           } else {
  

•                   printf("eMMC boot partition Size change
  

Failed.!!\n");

return 1 here I think

•           }
  

•           return 0;
    }
  
    if (strcmp(argv[1], "read") == 0)
  

@@ -318,5 +414,8 @@ U_BOOT_CMD( "mmc rescan\n" "mmc part - lists available partition on current mmc device\n" "mmc dev [dev] [part] - show or set current mmc device [partition]\n"

•   "mmc list - lists available devices");
  

•   "mmc list - lists available devices\n"
  

•   "mmc open <device num> - opens the specified device\n"
  

•   "mmc close <device num> - closes the specified device\n"
  

•   "mmc bootpart <device num> <boot part size MB> <RPMB part size
  

MB>\n");

Can you add another line of help here explaining what this does?

#endif

1.7.0.4

Regards,

Simon