HI Jaehoon,
On Thu, Jan 10, 2013 at 7:54 PM, Jaehoon Chung jh80.chung@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.xt@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.xt@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);elsereturn 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