Hi Jaehoon, Please find the responses below
Thanks & Regards Amarendra Reddy On 5 April 2013 11:51, Jaehoon Chung <jh80.ch...@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...@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 doing > so 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 boot > part*/ > > + if (strcmp(argv[1], "close") == 0) > else if? > Even 'else if' is fine, but I wanted to explicitly check the string in both cases. > > + 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 ? 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 change > Failed.\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 of > boot_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 size > MB>\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