| Message ID | 1355743176-12305-10-git-send-email-amarendra.xt@samsung.com |
|---|---|
| State | New |
| Headers | show |
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
Hi SImon, Thanks for the review comments. Please find below the responses for your comments. Thanks & Regards Amarendra On 20 December 2012 08:10, Simon Glass <sjg@chromium.org> wrote: > 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? > Yes. Shall correct it. > > + > > + 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 > OK.. will change the wordings. + } 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. > > OK. Will maintain EMMC consistently every where. Will remove "!!!". > > + } > > + 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. > > Ok, will be addressed. > > > + 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? > Ok. Will combine open/close. > > > + 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 > Yes. WIll be corrected. > > > + } > > + 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? > OK. I will add another line of help for mmc bootpart. > > > #endif > > -- > > 1.7.0.4 > > > > Regards, > Simon > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot > >
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; + + 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"); + } else { + printf("eMMC OPEN Failed.!!\n"); + } + 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; + 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_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 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"); #endif
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(-)