Re: [U-Boot] [PATCH V8 9/9] COMMON: MMC: Command to support EMMC booting and to resize EMMC boot partition
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.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.
Why? I think, It doesn't make sense.
+ access = 0; /* No access to boot partition */
+
+ /* acknowledge
Re: [U-Boot] [PATCH V8 9/9] COMMON: MMC: Command to support EMMC booting and to resize EMMC boot partition
Dear Minkyu,
Thankyou for the review comments.
Please find my responses below.
Thanks Regards
Amarendra Reddy
On 9 April 2013 16:25, Minkyu Kang mk7.k...@samsung.com wrote:
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.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;
Re: [U-Boot] [PATCH V8 9/9] COMMON: MMC: Command to support EMMC booting and to resize EMMC boot partition
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.
+{
+ 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?
+ 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?
+ 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)?
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
Re: [U-Boot] [PATCH V8 9/9] COMMON: MMC: Command to support EMMC booting and to resize EMMC boot partition
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
