Re: [U-Boot] [PATCH 9/9] COMMON: MMC: Command to support EMMC booting
Dear Wolfgang, Thanks for the review comments. I would like to clarify that this patch resizes EMMC partitions. But does not create partitions. I think the statement *..close and create partitions on EMMC* was misleading, which I corrected in latest patch set. The EMMC4.4 chips are provided with 2 boot partitons and 1 RPMB partition. This patch merely deals with resizing Boot partitions RPMB partition, by sending proper commands to EMMC chip. Thanks Regards Amarendra Reddy. On 28 December 2012 21:11, Wolfgang Denk w...@denx.de wrote: Dear Amar, In message 1356709972-26549-10-git-send-email-amarendra...@samsung.com you wrote: This patch adds commands to open, close and create partitions on EMMC In which way are partitions on MC devices special, compare to partitions on other storage devies? I mean, does it reeally make sense to create a (E?) MMC specific command here, instead of providing general partition support hat can also be used on other storage devices? Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Never face facts; if you do, you'll never get up in the morning. - Marlo Thomas ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 9/9] COMMON: MMC: Command to support EMMC booting
Dear Amarendra Reddy, In message CAPbRUm=2t8=mojzewv98eq15nv8rqo0sbjxwm7mzj+zft2q...@mail.gmail.com you wrote: Thanks for the review comments. I would like to clarify that this patch resizes EMMC partitions. But does not create partitions. I think the statement *..close and create partitions on EMMC* was misleading, which I corrected in latest patch set. The EMMC4.4 chips are provided with 2 boot partitons and 1 RPMB partition. This patch merely deals with resizing Boot partitions RPMB partition, by sending proper commands to EMMC chip. I see, but I still think that instead of adding (E)MMC specific partition handling, we should instead start and add a generc command to create/change/delete partitions, and then provide the (E)MMC specific implementation for it. This way we can later add partition handling for other storage media if someone needs this. Otherwise we would have a growing number of completely separate implementations, with incomplatible user interfaces and all the other maintenance nightmares this would cause. That means the core of your implementation will remain the same, just the command interface should be made generic so it can be extended to add support for other storage devices. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de We all agree on the necessity of compromise. We just can't agree on when it's necessary to compromise. - Larry Wall in 1991nov13.194420.28...@netlabs.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 9/9] COMMON: MMC: Command to support EMMC booting
This patch adds commands to open, close and create partitions on EMMC
Signed-off-by: Amar amarendra...@samsung.com
---
common/cmd_mmc.c | 85 +++-
1 file changed, 84 insertions(+), 1 deletion(-)
diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c
index 62a1c22..355ab8e 100644
--- a/common/cmd_mmc.c
+++ b/common/cmd_mmc.c
@@ -248,6 +248,85 @@ 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)) {
+ 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);
+ 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;
+ }
+ }
+
+ if (strcmp(argv[1], close) == 0) {
+ if (!(mmc_boot_close(mmc))) {
+ printf(EMMC CLOSE Success.\n);
+ 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);
+
+ 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);
+ return 0;
+ } else {
+ printf(EMMC boot partition Size change Failed.\n);
+ return 1;
+ }
}
if (strcmp(argv[1], read) == 0)
@@ -318,5 +397,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
--
1.8.0
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 9/9] COMMON: MMC: Command to support EMMC booting
Dear Amar, In message 1356709972-26549-10-git-send-email-amarendra...@samsung.com you wrote: This patch adds commands to open, close and create partitions on EMMC In which way are partitions on MC devices special, compare to partitions on other storage devies? I mean, does it reeally make sense to create a (E?) MMC specific command here, instead of providing general partition support hat can also be used on other storage devices? Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Never face facts; if you do, you'll never get up in the morning. - Marlo Thomas ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 9/9] COMMON: MMC: Command to support eMMC booting
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 s...@chromium.org wrote:
Hi Amar,
On Mon, Dec 17, 2012 at 3:19 AM, Amar amarendra...@samsung.com wrote:
This patch adds commands to open, close and create partitions on eMMC
Signed-off-by: Amar amarendra...@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);
+
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...@samsung.com wrote:
This patch adds commands to open, close and create partitions on eMMC
Signed-off-by: Amar amarendra...@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
[U-Boot] [PATCH 9/9] COMMON: MMC: Command to support eMMC booting
This patch adds commands to open, close and create partitions on eMMC
Signed-off-by: Amar amarendra...@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;
+
+ 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
--
1.7.0.4
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
