HI Jaehoon, On Thu, Jan 10, 2013 at 7:54 PM, Jaehoon Chung <jh80.ch...@samsung.com> wrote: > On 01/11/2013 01:46 AM, Simon Glass wrote: >> Hi Amar, >> >> On Fri, Jan 4, 2013 at 1:34 AM, Amar <amarendra...@samsung.com> wrote: >>> This patch adds commands to open, close and resize boot partitions on EMMC. >>> >>> Changes from V1: >>> 1)Combined the common piece of code between 'open' and 'close' >>> operations. >>> >>> Changes from V2: >>> 1)Updation of commit message and resubmition of proper patch set. >>> >>> Changes from V3: >>> No change. >>> >>> Signed-off-by: Amar <amarendra...@samsung.com> >>> --- >>> common/cmd_mmc.c | 84 >>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++- >>> 1 file changed, 83 insertions(+), 1 deletion(-) >>> >>> diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c >>> index 7dacd51..1dabb5b 100644 >>> --- a/common/cmd_mmc.c >>> +++ b/common/cmd_mmc.c >>> @@ -248,6 +248,84 @@ 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) || >>> + (strcmp(argv[1], "close") == 0)) { >> >> How about putting this block in its own function? >> >>> + int dev; >>> + struct mmc *mmc; >>> + >>> + if (argc == 2) >>> + dev = curr_device; >>> + else if (argc == 3) >>> + dev = simple_strtoul(argv[2], NULL, 10); >>> + 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 (strcmp(argv[1], "open") == 0) { >>> + 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" >>> + " images are written\n"); >> >> Do you need to split these strings so much? Perhaps when it is in a >> function the indenting will be less? >> >>> + printf("!EMMC boot partition" >>> + " has continuity at" >>> + " image writing time.\n"); >>> + printf("!So, Do not close boot" >>> + " partition, Before, all" >>> + " images are written.\n"); >>> + return 0; >>> + } else { >>> + printf("EMMC OPEN Failed.\n"); >>> + return 1; >> >> You could put this above the other block and reduce indenting: >> >> if (mmc_boot_open(mmc)) { >> printf("EMMC OPEN Failed.\n"); >> return 1; >> } >> ...code continues >> >>> + } >>> + } >>> + >>> + if (strcmp(argv[1], "close") == 0) { >>> + if (!(mmc_boot_close(mmc))) { >>> + printf("EMMC CLOSE Success.\n"); >> >> Shouldn't print a message on success >> >>> + return 0; >>> + } else { >>> + printf("EMMC CLOSE Failed.\n"); >>> + return 1; >>> + } >>> + } >>> + } else if (strcmp(argv[1], "bootpart") == 0) { >>> + int dev; >>> + dev = simple_strtoul(argv[2], NULL, 10); >>> + >>> + u32 bootsize = simple_strtoul(argv[3], NULL, 10); >>> + u32 rpmbsize = simple_strtoul(argv[4], NULL, 10); >>> + struct mmc *mmc = find_mmc_device(dev); >>> + 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); >>> + return 0; >>> + } else { >>> + printf("EMMC boot partition Size change Failed.\n"); >>> + return 1; >>> + } >>> } >>> >>> state = MMC_INVALID; >>> @@ -317,5 +395,9 @@ 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" >>> + " - change sizes of boot and RPMB partions of specified device\n"); >>> #endif >> >> Also did you see Wolfgang's suggestion that we put the partition stuff >> in the 'part' command (at least that's what I think he said). You >> could have 'part open', 'part close' and maybe 'part resize'? > How about using "mmc bootpart <device_num> <ack> <enable> <access>"
Maybe - what do these parameters mean? > Also i think that we can reduce the code line. OK good. Regards, Simon > > Best Regards, > Jaehoon Chung >> >> Regards, >> Simon >> >>> -- >>> 1.8.0 >>> >> > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot