Dear Amarendra,
On 05/04/13 16:39, Amarendra Reddy wrote:
Hi Jaehoon,
Please find the responses below
Thanks & Regards Amarendra Reddy
On 5 April 2013 11:51, Jaehoon Chung jh80.chung@samsung.com wrote:
Hi Amar,
On 04/03/2013 11:08 PM, Amar wrote:
This patch adds commands to access(open/close) and resize boot
partitions on EMMC.
Signed-off-by: Amar amarendra.xt@samsung.com
Changes since V1: 1)Combined the common piece of code between 'open' and 'close' operations.
Changes since V2: 1)Updation of commit message and resubmition of proper patch set.
Changes since V3: No change.
Changes since V4: 1)Added a new funtion boot_part_access() to combine the common
parts of
'mmc open' and 'mmc close' functionalities. 2)Used the generic function "mmc_boot_part_access()" instead of two functions "mmc_boot_open()" and "mmc_boot_close()". By doingso user
can specify which boot partition to be accessed (opened / closed).Changes since V5: 1)Updated minor nits in response to review comments.
Changes since V6: No change.
Changes since V7: 1)The piece of code involved in open/close and resize of EMMC boot partition has been made conditional and is enabled only if the
macro
CONFIG_SUPPORT_EMMC_BOOT is defined.common/cmd_mmc.c | 110
++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 108 insertions(+), 2 deletions(-)
diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c index 8c53a10..c5f60a2 100644 --- a/common/cmd_mmc.c +++ b/common/cmd_mmc.c @@ -147,6 +147,36 @@ U_BOOT_CMD( "- display info of the current MMC device" );
+#ifdef CONFIG_SUPPORT_EMMC_BOOT +static int boot_part_access(struct mmc *mmc, u32 ack, u32 part_num, u32
access) Why do you use u32? I know that just used 8bit.
Yes, just 8bit would suffice. I will update it.
+{
int err;err = mmc_boot_part_access(mmc, ack, part_num, access);if ((err == 0) && (access != 0)) {printf("\t\t\t!!!Notice!!!\n");printf("!You must close EMMC boot Partition");printf("after all images are written\n");printf("!EMMC boot partition has continuity");printf("at image writing time.\n");printf("!So, do not close the boot partition");printf("before all images are written.\n");return 0;} else if ((err == 0) && (access == 0))return 0;else if ((err != 0) && (access != 0)) {printf("EMMC boot partition-%d OPEN Failed.\n", part_num);return 1;} else {printf("EMMC boot partition-%d CLOSE Failed.\n", part_num);return 1;}+} +#endif
static int do_mmcops(cmd_tbl_t *cmdtp, int flag, int argc, char * const
argv[])
{ enum mmc_state state; @@ -248,8 +278,75 @@ static int do_mmcops(cmd_tbl_t *cmdtp, int flag,
int argc, char * const argv[])
curr_device, mmc->part_num); return 0;
}+#ifdef CONFIG_SUPPORT_EMMC_BOOT
} else if ((strcmp(argv[1], "open") == 0) ||(strcmp(argv[1], "close") == 0)) {int dev;struct mmc *mmc;u32 ack, part_num, access = 0;if (argc == 4) {dev = simple_strtoul(argv[2], NULL, 10);part_num = simple_strtoul(argv[3], 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 ((part_num <= 0) || (part_num >MMC_NUM_BOOT_PARTITION)) {
printf("Invalid boot partition number:\n");printf("Boot partition number cannot be <= 0\n");printf("EMMC44 supports only 2 boot partitions\n");What means? EMMC44 supports only 2 boot partitions?
As per the eMMC 4.4 specification, eMMC 4.4 has 1 RPMB partition, 2 Boot partitions and 4 General Purpose partitions. As there are only 2 Boot partitions, we restrict the user not to use an invalid partition number.
return 1;
}if (strcmp(argv[1], "open") == 0)access = part_num; /* enable R/W access to bootpart*/
if (strcmp(argv[1], "close") == 0)else if?
Even 'else if' is fine, but I wanted to explicitly check the string in both cases.
Why? I think, It doesn't make sense.
access = 0; /* No access to boot partition *//* acknowledge to be sent during boot operation */ack = 1;ack is always set to 1? then Can't initialize ack value as 1. Otherwise, boot_part_access(mmc, 1, part_num, access)?
As per JEDEC Standard 'ack' can be 0 / 1.
Right now we are always using ack=1. Is it OK to use "boot_part_access(mmc, 1, part_num, access)" with a hardcoded value '1' in the function call ?
Yes, It's a constant.
Best Regards,
Jaehoon Chung
return boot_part_access(mmc, ack, part_num, access);} 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 changeFailed.\n");
return 1;}+#endif /* CONFIG_SUPPORT_EMMC_BOOT */
} state = MMC_INVALID; if (argc == 5 && strcmp(argv[1], "read") == 0) state = MMC_READ;@@ -324,5 +421,14 @@ 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"+#ifdef CONFIG_SUPPORT_EMMC_BOOT
"mmc open <dev> <boot_partition>\n"" - Enable boot_part for booting and enable R/W access ofboot_part\n"
"mmc close <dev> <boot_partition>\n"" - Enable boot_part for booting and disable access to boot_part\n""mmc bootpart <device num> <boot part size MB> <RPMB part sizeMB>\n"
" - change sizes of boot and RPMB partions of specified device\n"#endif
);+#endif /* !CONFIG_GENERIC_MMC */
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
Thanks, Minkyu Kang.