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

[Simon Glass]

Hi Amar,

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

> This patch adds commands to open, close and create partitions on eMMC
>
> Signed-off-by: Amar <amarendra.xt at 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