All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] cmd: mmc: add mmc partboot
@ 2021-02-12 10:32 ` grygorii tertychnyi
  2021-02-14 22:08   ` Jaehoon Chung
  2021-02-17 16:35   ` [PATCH v2] cmd: mmc: add mmc bootpart-check grygorii tertychnyi
  0 siblings, 2 replies; 9+ messages in thread
From: grygorii tertychnyi @ 2021-02-12 10:32 UTC (permalink / raw)
  To: u-boot

This patch allows to determine active boot partition in boot script:

if mmc partboot ${mmcdev} 2; then
    echo "booted from eMMC boot1 partition"
fi

Signed-off-by: Grygorii Tertychnyi <grygorii.tertychnyi@leica-geosystems.com>
---
 cmd/mmc.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/cmd/mmc.c b/cmd/mmc.c
index 1529a3e05ddd..010d6ab9aa19 100644
--- a/cmd/mmc.c
+++ b/cmd/mmc.c
@@ -771,6 +771,41 @@ static int do_mmc_boot_resize(struct cmd_tbl *cmdtp, int flag,
 	return CMD_RET_SUCCESS;
 }
 
+static int do_mmc_partboot(struct cmd_tbl *cmdtp, int flag,
+			   int argc, char *const argv[])
+{
+	int dev;
+	struct mmc *mmc;
+	u8 part_args, part_emmc;
+
+	if (argc != 3)
+		return CMD_RET_USAGE;
+
+	dev = simple_strtoul(argv[1], NULL, 10);
+
+	mmc = init_mmc_device(dev, false);
+	if (!mmc)
+		return CMD_RET_FAILURE;
+
+	if (IS_SD(mmc)) {
+		puts("PARTITION_CONFIG only exists on eMMC\n");
+		return CMD_RET_FAILURE;
+	}
+
+	if (mmc->part_config == MMCPART_NOAVAILABLE) {
+		printf("No part_config info for ver. 0x%x\n", mmc->version);
+		return CMD_RET_FAILURE;
+	}
+
+	part_emmc = EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config);
+	part_args = simple_strtoul(argv[2], NULL, 10);
+
+	if (part_emmc == part_args)
+		return CMD_RET_SUCCESS;
+	else
+		return CMD_RET_FAILURE;
+}
+
 static int mmc_partconf_print(struct mmc *mmc)
 {
 	u8 ack, access, part;
@@ -953,6 +988,7 @@ static struct cmd_tbl cmd_mmc[] = {
 #ifdef CONFIG_SUPPORT_EMMC_BOOT
 	U_BOOT_CMD_MKENT(bootbus, 5, 0, do_mmc_bootbus, "", ""),
 	U_BOOT_CMD_MKENT(bootpart-resize, 4, 0, do_mmc_boot_resize, "", ""),
+	U_BOOT_CMD_MKENT(partboot, 3, 0, do_mmc_partboot, "", ""),
 	U_BOOT_CMD_MKENT(partconf, 5, 0, do_mmc_partconf, "", ""),
 	U_BOOT_CMD_MKENT(rst-function, 3, 0, do_mmc_rst_func, "", ""),
 #endif
@@ -1021,6 +1057,9 @@ U_BOOT_CMD(
 	" - Set the BOOT_BUS_WIDTH field of the specified device\n"
 	"mmc bootpart-resize <dev> <boot part size MB> <RPMB part size MB>\n"
 	" - Change sizes of boot and RPMB partitions of specified device\n"
+	"mmc partboot dev boot_partition\n"
+	" - Return success if the given boot_partition value matches BOOT_PARTITION_ENABLE\n"
+	"   bit field of the specified device\n"
 	"mmc partconf dev [boot_ack boot_partition partition_access]\n"
 	" - Show or change the bits of the PARTITION_CONFIG field of the specified device\n"
 	"mmc rst-function dev value\n"
-- 
2.25.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH] cmd: mmc: add mmc partboot
  2021-02-12 10:32 ` [PATCH] cmd: mmc: add mmc partboot grygorii tertychnyi
@ 2021-02-14 22:08   ` Jaehoon Chung
  2021-02-15 13:04     ` gr embeter
  2021-02-17 16:35   ` [PATCH v2] cmd: mmc: add mmc bootpart-check grygorii tertychnyi
  1 sibling, 1 reply; 9+ messages in thread
From: Jaehoon Chung @ 2021-02-14 22:08 UTC (permalink / raw)
  To: u-boot

Dear Grygorii,

On 2/12/21 7:32 PM, grygorii tertychnyi wrote:
> This patch allows to determine active boot partition in boot script:
> 
> if mmc partboot ${mmcdev} 2; then
>     echo "booted from eMMC boot1 partition"
> fi

I don't know what purpose this patch has.

Best Regards,
Jaehoon Chung

> 
> Signed-off-by: Grygorii Tertychnyi <grygorii.tertychnyi@leica-geosystems.com>
> ---
>  cmd/mmc.c | 39 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/cmd/mmc.c b/cmd/mmc.c
> index 1529a3e05ddd..010d6ab9aa19 100644
> --- a/cmd/mmc.c
> +++ b/cmd/mmc.c
> @@ -771,6 +771,41 @@ static int do_mmc_boot_resize(struct cmd_tbl *cmdtp, int flag,
>  	return CMD_RET_SUCCESS;
>  }
>  
> +static int do_mmc_partboot(struct cmd_tbl *cmdtp, int flag,
> +			   int argc, char *const argv[])
> +{
> +	int dev;
> +	struct mmc *mmc;
> +	u8 part_args, part_emmc;
> +
> +	if (argc != 3)
> +		return CMD_RET_USAGE;
> +
> +	dev = simple_strtoul(argv[1], NULL, 10);
> +
> +	mmc = init_mmc_device(dev, false);
> +	if (!mmc)
> +		return CMD_RET_FAILURE;
> +
> +	if (IS_SD(mmc)) {
> +		puts("PARTITION_CONFIG only exists on eMMC\n");
> +		return CMD_RET_FAILURE;
> +	}
> +
> +	if (mmc->part_config == MMCPART_NOAVAILABLE) {
> +		printf("No part_config info for ver. 0x%x\n", mmc->version);
> +		return CMD_RET_FAILURE;
> +	}
> +
> +	part_emmc = EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config);
> +	part_args = simple_strtoul(argv[2], NULL, 10);
> +
> +	if (part_emmc == part_args)
> +		return CMD_RET_SUCCESS;
> +	else
> +		return CMD_RET_FAILURE;
> +}
> +
>  static int mmc_partconf_print(struct mmc *mmc)
>  {
>  	u8 ack, access, part;
> @@ -953,6 +988,7 @@ static struct cmd_tbl cmd_mmc[] = {
>  #ifdef CONFIG_SUPPORT_EMMC_BOOT
>  	U_BOOT_CMD_MKENT(bootbus, 5, 0, do_mmc_bootbus, "", ""),
>  	U_BOOT_CMD_MKENT(bootpart-resize, 4, 0, do_mmc_boot_resize, "", ""),
> +	U_BOOT_CMD_MKENT(partboot, 3, 0, do_mmc_partboot, "", ""),
>  	U_BOOT_CMD_MKENT(partconf, 5, 0, do_mmc_partconf, "", ""),
>  	U_BOOT_CMD_MKENT(rst-function, 3, 0, do_mmc_rst_func, "", ""),
>  #endif
> @@ -1021,6 +1057,9 @@ U_BOOT_CMD(
>  	" - Set the BOOT_BUS_WIDTH field of the specified device\n"
>  	"mmc bootpart-resize <dev> <boot part size MB> <RPMB part size MB>\n"
>  	" - Change sizes of boot and RPMB partitions of specified device\n"
> +	"mmc partboot dev boot_partition\n"
> +	" - Return success if the given boot_partition value matches BOOT_PARTITION_ENABLE\n"
> +	"   bit field of the specified device\n"
>  	"mmc partconf dev [boot_ack boot_partition partition_access]\n"
>  	" - Show or change the bits of the PARTITION_CONFIG field of the specified device\n"
>  	"mmc rst-function dev value\n"
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] cmd: mmc: add mmc partboot
  2021-02-14 22:08   ` Jaehoon Chung
@ 2021-02-15 13:04     ` gr embeter
  2021-02-15 23:30       ` Jaehoon Chung
  0 siblings, 1 reply; 9+ messages in thread
From: gr embeter @ 2021-02-15 13:04 UTC (permalink / raw)
  To: u-boot

Hello Jaehoon,

On Sun, 14 Feb 2021 at 23:08 Jaehoon Chung <jh80.chung@samsung.com> wrote:

> Dear Grygorii,
>
> On 2/12/21 7:32 PM, grygorii tertychnyi wrote:
> > This patch allows to determine active boot partition in boot script:
> >
> > if mmc partboot ${mmcdev} 2; then
> >     echo "booted from eMMC boot1 partition"
> > fi
>
> I don't know what purpose this patch has.


With an eMMC as a boot source I have two boot partitions, i.e. ?boot1? and
?boot2?, with two different versions of U-Boot on each partition.

I also have two different kernels located on eMMC ?user? partition, let?s
say ?image1? and ?image2?.

So, I want to boot ?image1? kernel if it is U-boot from ?boot1? partition
is booted now,
and to boot ?image2? kernel if it is U-boot from ?boot2? partition is
booted.

It is easy to do it manually. I just added possibility to do it with U-boot
script now,
because ?mmc partconf ...? does not let you check the current boot
partition with hush.

Hope, I explained the purpose.



>
> Best Regards,
> Jaehoon Chung
>
> >
> > Signed-off-by: Grygorii Tertychnyi <
> grygorii.tertychnyi at leica-geosystems.com>
> > ---
> >  cmd/mmc.c | 39 +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 39 insertions(+)
> >
> > diff --git a/cmd/mmc.c b/cmd/mmc.c
> > index 1529a3e05ddd..010d6ab9aa19 100644
> > --- a/cmd/mmc.c
> > +++ b/cmd/mmc.c
> > @@ -771,6 +771,41 @@ static int do_mmc_boot_resize(struct cmd_tbl
> *cmdtp, int flag,
> >       return CMD_RET_SUCCESS;
> >  }
> >
> > +static int do_mmc_partboot(struct cmd_tbl *cmdtp, int flag,
> > +                        int argc, char *const argv[])
> > +{
> > +     int dev;
> > +     struct mmc *mmc;
> > +     u8 part_args, part_emmc;
> > +
> > +     if (argc != 3)
> > +             return CMD_RET_USAGE;
> > +
> > +     dev = simple_strtoul(argv[1], NULL, 10);
> > +
> > +     mmc = init_mmc_device(dev, false);
> > +     if (!mmc)
> > +             return CMD_RET_FAILURE;
> > +
> > +     if (IS_SD(mmc)) {
> > +             puts("PARTITION_CONFIG only exists on eMMC\n");
> > +             return CMD_RET_FAILURE;
> > +     }
> > +
> > +     if (mmc->part_config == MMCPART_NOAVAILABLE) {
> > +             printf("No part_config info for ver. 0x%x\n",
> mmc->version);
> > +             return CMD_RET_FAILURE;
> > +     }
> > +
> > +     part_emmc = EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config);
> > +     part_args = simple_strtoul(argv[2], NULL, 10);
> > +
> > +     if (part_emmc == part_args)
> > +             return CMD_RET_SUCCESS;
> > +     else
> > +             return CMD_RET_FAILURE;
> > +}
> > +
> >  static int mmc_partconf_print(struct mmc *mmc)
> >  {
> >       u8 ack, access, part;
> > @@ -953,6 +988,7 @@ static struct cmd_tbl cmd_mmc[] = {
> >  #ifdef CONFIG_SUPPORT_EMMC_BOOT
> >       U_BOOT_CMD_MKENT(bootbus, 5, 0, do_mmc_bootbus, "", ""),
> >       U_BOOT_CMD_MKENT(bootpart-resize, 4, 0, do_mmc_boot_resize, "",
> ""),
> > +     U_BOOT_CMD_MKENT(partboot, 3, 0, do_mmc_partboot, "", ""),
> >       U_BOOT_CMD_MKENT(partconf, 5, 0, do_mmc_partconf, "", ""),
> >       U_BOOT_CMD_MKENT(rst-function, 3, 0, do_mmc_rst_func, "", ""),
> >  #endif
> > @@ -1021,6 +1057,9 @@ U_BOOT_CMD(
> >       " - Set the BOOT_BUS_WIDTH field of the specified device\n"
> >       "mmc bootpart-resize <dev> <boot part size MB> <RPMB part size
> MB>\n"
> >       " - Change sizes of boot and RPMB partitions of specified device\n"
> > +     "mmc partboot dev boot_partition\n"
> > +     " - Return success if the given boot_partition value matches
> BOOT_PARTITION_ENABLE\n"
> > +     "   bit field of the specified device\n"
> >       "mmc partconf dev [boot_ack boot_partition partition_access]\n"
> >       " - Show or change the bits of the PARTITION_CONFIG field of the
> specified device\n"
> >       "mmc rst-function dev value\n"
> >
>
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] cmd: mmc: add mmc partboot
  2021-02-15 13:04     ` gr embeter
@ 2021-02-15 23:30       ` Jaehoon Chung
  2021-02-16  8:18         ` gr embeter
  0 siblings, 1 reply; 9+ messages in thread
From: Jaehoon Chung @ 2021-02-15 23:30 UTC (permalink / raw)
  To: u-boot

Hi Grygorii,

On 2/15/21 10:04 PM, gr embeter wrote:
> Hello Jaehoon,
> 
> On Sun, 14 Feb 2021 at 23:08 Jaehoon Chung <jh80.chung@samsung.com> wrote:
> 
>> Dear Grygorii,
>>
>> On 2/12/21 7:32 PM, grygorii tertychnyi wrote:
>>> This patch allows to determine active boot partition in boot script:
>>>
>>> if mmc partboot ${mmcdev} 2; then
>>>     echo "booted from eMMC boot1 partition"
>>> fi
>>
>> I don't know what purpose this patch has.
> 
> 
> With an eMMC as a boot source I have two boot partitions, i.e. ?boot1? and
> ?boot2?, with two different versions of U-Boot on each partition.
> 
> I also have two different kernels located on eMMC ?user? partition, let?s
> say ?image1? and ?image2?.
> 
> So, I want to boot ?image1? kernel if it is U-boot from ?boot1? partition
> is booted now,
> and to boot ?image2? kernel if it is U-boot from ?boot2? partition is
> booted.
> 
> It is easy to do it manually. I just added possibility to do it with U-boot
> script now,
> because ?mmc partconf ...? does not let you check the current boot
> partition with hush.
> 
> Hope, I explained the purpose.

I remembered. Because i feel to see similar patch to use bootpart.
At that time, i also asked same question. Sorry for not remembered yours.

https://patchwork.ozlabs.org/project/uboot/patch/20201212074633.891704-1-grembeter at outlook.com/


> 
> 
> 
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>>
>>> Signed-off-by: Grygorii Tertychnyi <
>> grygorii.tertychnyi at leica-geosystems.com>
>>> ---
>>>  cmd/mmc.c | 39 +++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 39 insertions(+)
>>>
>>> diff --git a/cmd/mmc.c b/cmd/mmc.c
>>> index 1529a3e05ddd..010d6ab9aa19 100644
>>> --- a/cmd/mmc.c
>>> +++ b/cmd/mmc.c
>>> @@ -771,6 +771,41 @@ static int do_mmc_boot_resize(struct cmd_tbl
>> *cmdtp, int flag,
>>>       return CMD_RET_SUCCESS;
>>>  }
>>>
>>> +static int do_mmc_partboot(struct cmd_tbl *cmdtp, int flag,
>>> +                        int argc, char *const argv[])
>>> +{
>>> +     int dev;
>>> +     struct mmc *mmc;
>>> +     u8 part_args, part_emmc;
>>> +
>>> +     if (argc != 3)
>>> +             return CMD_RET_USAGE;
>>> +
>>> +     dev = simple_strtoul(argv[1], NULL, 10);
>>> +
>>> +     mmc = init_mmc_device(dev, false);
>>> +     if (!mmc)
>>> +             return CMD_RET_FAILURE;
>>> +
>>> +     if (IS_SD(mmc)) {
>>> +             puts("PARTITION_CONFIG only exists on eMMC\n");
>>> +             return CMD_RET_FAILURE;
>>> +     }
>>> +
>>> +     if (mmc->part_config == MMCPART_NOAVAILABLE) {
>>> +             printf("No part_config info for ver. 0x%x\n",
>> mmc->version);
>>> +             return CMD_RET_FAILURE;
>>> +     }
>>> +
>>> +     part_emmc = EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config);
>>> +     part_args = simple_strtoul(argv[2], NULL, 10);
>>> +
>>> +     if (part_emmc == part_args)
>>> +             return CMD_RET_SUCCESS;
>>> +     else
>>> +             return CMD_RET_FAILURE;
>>> +}
>>> +
>>>  static int mmc_partconf_print(struct mmc *mmc)
>>>  {
>>>       u8 ack, access, part;
>>> @@ -953,6 +988,7 @@ static struct cmd_tbl cmd_mmc[] = {
>>>  #ifdef CONFIG_SUPPORT_EMMC_BOOT
>>>       U_BOOT_CMD_MKENT(bootbus, 5, 0, do_mmc_bootbus, "", ""),
>>>       U_BOOT_CMD_MKENT(bootpart-resize, 4, 0, do_mmc_boot_resize, "",
>> ""),
>>> +     U_BOOT_CMD_MKENT(partboot, 3, 0, do_mmc_partboot, "", ""),

partboot can be confused. how about changing more clear name, like mmc_bootpart_check?

>>>       U_BOOT_CMD_MKENT(partconf, 5, 0, do_mmc_partconf, "", ""),
>>>       U_BOOT_CMD_MKENT(rst-function, 3, 0, do_mmc_rst_func, "", ""),
>>>  #endif
>>> @@ -1021,6 +1057,9 @@ U_BOOT_CMD(
>>>       " - Set the BOOT_BUS_WIDTH field of the specified device\n"
>>>       "mmc bootpart-resize <dev> <boot part size MB> <RPMB part size
>> MB>\n"
>>>       " - Change sizes of boot and RPMB partitions of specified device\n"
>>> +     "mmc partboot dev boot_partition\n"

why it needs to pass "boot_partition"?
And use more clear usage..with optional or mandatory.
<> - mandatory
[] - optional

mmc partboot <dev> 

>>> +     " - Return success if the given boot_partition value matches
>> BOOT_PARTITION_ENABLE\n"
>>> +     "   bit field of the specified device\n"
>>>       "mmc partconf dev [boot_ack boot_partition partition_access]\n"
>>>       " - Show or change the bits of the PARTITION_CONFIG field of the
>> specified device\n"
>>>       "mmc rst-function dev value\n"
>>>
>>
>>
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] cmd: mmc: add mmc partboot
  2021-02-15 23:30       ` Jaehoon Chung
@ 2021-02-16  8:18         ` gr embeter
  2021-02-16  9:00           ` Jaehoon Chung
  0 siblings, 1 reply; 9+ messages in thread
From: gr embeter @ 2021-02-16  8:18 UTC (permalink / raw)
  To: u-boot

Hi Jaehoon,
thanks for your comments.

On Tue, 16 Feb 2021 at 00:30 Jaehoon Chung <jh80.chung@samsung.com> wrote:

> Hi Grygorii,
>
> On 2/15/21 10:04 PM, gr embeter wrote:
> > Hello Jaehoon,
> >
> > On Sun, 14 Feb 2021 at 23:08 Jaehoon Chung <jh80.chung@samsung.com>
> wrote:
> >
> >> Dear Grygorii,
> >>
> >> On 2/12/21 7:32 PM, grygorii tertychnyi wrote:
> >>> This patch allows to determine active boot partition in boot script:
> >>>
> >>> if mmc partboot ${mmcdev} 2; then
> >>>     echo "booted from eMMC boot1 partition"
> >>> fi
> >>
> >> I don't know what purpose this patch has.
> >
> >
> > With an eMMC as a boot source I have two boot partitions, i.e. ?boot1?
> and
> > ?boot2?, with two different versions of U-Boot on each partition.
> >
> > I also have two different kernels located on eMMC ?user? partition, let?s
> > say ?image1? and ?image2?.
> >
> > So, I want to boot ?image1? kernel if it is U-boot from ?boot1? partition
> > is booted now,
> > and to boot ?image2? kernel if it is U-boot from ?boot2? partition is
> > booted.
> >
> > It is easy to do it manually. I just added possibility to do it with
> U-boot
> > script now,
> > because ?mmc partconf ...? does not let you check the current boot
> > partition with hush.
> >
> > Hope, I explained the purpose.
>
> I remembered. Because i feel to see similar patch to use bootpart.
> At that time, i also asked same question. Sorry for not remembered yours.
>
>
> https://patchwork.ozlabs.org/project/uboot/patch/20201212074633.891704-1-grembeter at outlook.com/
>

Indeed. I should?ve mentioned this link. Sorry. I?ll extend commit message.


> >
> >
> >
> >>
> >> Best Regards,
> >> Jaehoon Chung
> >>
> >>>
> >>> Signed-off-by: Grygorii Tertychnyi <
> >> grygorii.tertychnyi at leica-geosystems.com>
> >>> ---
> >>>  cmd/mmc.c | 39 +++++++++++++++++++++++++++++++++++++++
> >>>  1 file changed, 39 insertions(+)
> >>>
> >>> diff --git a/cmd/mmc.c b/cmd/mmc.c
> >>> index 1529a3e05ddd..010d6ab9aa19 100644
> >>> --- a/cmd/mmc.c
> >>> +++ b/cmd/mmc.c
> >>> @@ -771,6 +771,41 @@ static int do_mmc_boot_resize(struct cmd_tbl
> >> *cmdtp, int flag,
> >>>       return CMD_RET_SUCCESS;
> >>>  }
> >>>
> >>> +static int do_mmc_partboot(struct cmd_tbl *cmdtp, int flag,
> >>> +                        int argc, char *const argv[])
> >>> +{
> >>> +     int dev;
> >>> +     struct mmc *mmc;
> >>> +     u8 part_args, part_emmc;
> >>> +
> >>> +     if (argc != 3)
> >>> +             return CMD_RET_USAGE;
> >>> +
> >>> +     dev = simple_strtoul(argv[1], NULL, 10);
> >>> +
> >>> +     mmc = init_mmc_device(dev, false);
> >>> +     if (!mmc)
> >>> +             return CMD_RET_FAILURE;
> >>> +
> >>> +     if (IS_SD(mmc)) {
> >>> +             puts("PARTITION_CONFIG only exists on eMMC\n");
> >>> +             return CMD_RET_FAILURE;
> >>> +     }
> >>> +
> >>> +     if (mmc->part_config == MMCPART_NOAVAILABLE) {
> >>> +             printf("No part_config info for ver. 0x%x\n",
> >> mmc->version);
> >>> +             return CMD_RET_FAILURE;
> >>> +     }
> >>> +
> >>> +     part_emmc = EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config);
> >>> +     part_args = simple_strtoul(argv[2], NULL, 10);
> >>> +
> >>> +     if (part_emmc == part_args)
> >>> +             return CMD_RET_SUCCESS;
> >>> +     else
> >>> +             return CMD_RET_FAILURE;
> >>> +}
> >>> +
> >>>  static int mmc_partconf_print(struct mmc *mmc)
> >>>  {
> >>>       u8 ack, access, part;
> >>> @@ -953,6 +988,7 @@ static struct cmd_tbl cmd_mmc[] = {
> >>>  #ifdef CONFIG_SUPPORT_EMMC_BOOT
> >>>       U_BOOT_CMD_MKENT(bootbus, 5, 0, do_mmc_bootbus, "", ""),
> >>>       U_BOOT_CMD_MKENT(bootpart-resize, 4, 0, do_mmc_boot_resize, "",
> >> ""),
> >>> +     U_BOOT_CMD_MKENT(partboot, 3, 0, do_mmc_partboot, "", ""),
>
> partboot can be confused. how about changing more clear name, like
> mmc_bootpart_check?


Agree. We have ?bootpart-resize?, and ?bootpart-check? looks very natural.

I?ll prepare V2 for
mmc bootpart-check <dev> <boot_partition>


>
> >>>       U_BOOT_CMD_MKENT(partconf, 5, 0, do_mmc_partconf, "", ""),
> >>>       U_BOOT_CMD_MKENT(rst-function, 3, 0, do_mmc_rst_func, "", ""),
> >>>  #endif
> >>> @@ -1021,6 +1057,9 @@ U_BOOT_CMD(
> >>>       " - Set the BOOT_BUS_WIDTH field of the specified device\n"
> >>>       "mmc bootpart-resize <dev> <boot part size MB> <RPMB part size
> >> MB>\n"
> >>>       " - Change sizes of boot and RPMB partitions of specified
> device\n"
> >>> +     "mmc partboot dev boot_partition\n"
>
> why it needs to pass "boot_partition"?


We need to return true or false to be able to use ?if? in boot script
(hush).
Comparing the real value with the given one is the only way to return true
or false. So, this argument is mandatory.


> And use more clear usage..with optional or mandatory.
> <> - mandatory
> [] - optional


OK.


>
> mmc partboot <dev>


>
> >>> +     " - Return success if the given boot_partition value matches
> >> BOOT_PARTITION_ENABLE\n"
> >>> +     "   bit field of the specified device\n"
> >>>       "mmc partconf dev [boot_ack boot_partition partition_access]\n"
> >>>       " - Show or change the bits of the PARTITION_CONFIG field of the
> >> specified device\n"
> >>>       "mmc rst-function dev value\n"
> >>>
> >>
> >>
> >
>
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] cmd: mmc: add mmc partboot
  2021-02-16  8:18         ` gr embeter
@ 2021-02-16  9:00           ` Jaehoon Chung
  2021-02-17 15:35             ` gr embeter
  0 siblings, 1 reply; 9+ messages in thread
From: Jaehoon Chung @ 2021-02-16  9:00 UTC (permalink / raw)
  To: u-boot

On 2/16/21 5:18 PM, gr embeter wrote:
> Hi Jaehoon,
> thanks for your comments.
> 
> On Tue, 16 Feb 2021 at 00:30 Jaehoon Chung <jh80.chung@samsung.com> wrote:
> 
>> Hi Grygorii,
>>
>> On 2/15/21 10:04 PM, gr embeter wrote:
>>> Hello Jaehoon,
>>>
>>> On Sun, 14 Feb 2021 at 23:08 Jaehoon Chung <jh80.chung@samsung.com>
>> wrote:
>>>
>>>> Dear Grygorii,
>>>>
>>>> On 2/12/21 7:32 PM, grygorii tertychnyi wrote:
>>>>> This patch allows to determine active boot partition in boot script:
>>>>>
>>>>> if mmc partboot ${mmcdev} 2; then
>>>>>     echo "booted from eMMC boot1 partition"
>>>>> fi
>>>>
>>>> I don't know what purpose this patch has.
>>>
>>>
>>> With an eMMC as a boot source I have two boot partitions, i.e. ?boot1?
>> and
>>> ?boot2?, with two different versions of U-Boot on each partition.
>>>
>>> I also have two different kernels located on eMMC ?user? partition, let?s
>>> say ?image1? and ?image2?.
>>>
>>> So, I want to boot ?image1? kernel if it is U-boot from ?boot1? partition
>>> is booted now,
>>> and to boot ?image2? kernel if it is U-boot from ?boot2? partition is
>>> booted.
>>>
>>> It is easy to do it manually. I just added possibility to do it with
>> U-boot
>>> script now,
>>> because ?mmc partconf ...? does not let you check the current boot
>>> partition with hush.
>>>
>>> Hope, I explained the purpose.
>>
>> I remembered. Because i feel to see similar patch to use bootpart.
>> At that time, i also asked same question. Sorry for not remembered yours.
>>
>>
>> https://protect2.fireeye.com/v1/url?k=dac52f49-855e1643-dac4a406-0cc47a31384a-4f6e07c1e040623a&q=1&e=3829977a-6bc3-49e3-868a-0dcba5fab9e4&u=https%3A%2F%2Fpatchwork.ozlabs.org%2Fproject%2Fuboot%2Fpatch%2F20201212074633.891704-1-grembeter%40outlook.com%2F
>>
> 
> Indeed. I should?ve mentioned this link. Sorry. I?ll extend commit message.
> 
> 
>>>
>>>
>>>
>>>>
>>>> Best Regards,
>>>> Jaehoon Chung
>>>>
>>>>>
>>>>> Signed-off-by: Grygorii Tertychnyi <
>>>> grygorii.tertychnyi at leica-geosystems.com>
>>>>> ---
>>>>>  cmd/mmc.c | 39 +++++++++++++++++++++++++++++++++++++++
>>>>>  1 file changed, 39 insertions(+)
>>>>>
>>>>> diff --git a/cmd/mmc.c b/cmd/mmc.c
>>>>> index 1529a3e05ddd..010d6ab9aa19 100644
>>>>> --- a/cmd/mmc.c
>>>>> +++ b/cmd/mmc.c
>>>>> @@ -771,6 +771,41 @@ static int do_mmc_boot_resize(struct cmd_tbl
>>>> *cmdtp, int flag,
>>>>>       return CMD_RET_SUCCESS;
>>>>>  }
>>>>>
>>>>> +static int do_mmc_partboot(struct cmd_tbl *cmdtp, int flag,
>>>>> +                        int argc, char *const argv[])
>>>>> +{
>>>>> +     int dev;
>>>>> +     struct mmc *mmc;
>>>>> +     u8 part_args, part_emmc;
>>>>> +
>>>>> +     if (argc != 3)
>>>>> +             return CMD_RET_USAGE;
>>>>> +
>>>>> +     dev = simple_strtoul(argv[1], NULL, 10);
>>>>> +
>>>>> +     mmc = init_mmc_device(dev, false);
>>>>> +     if (!mmc)
>>>>> +             return CMD_RET_FAILURE;
>>>>> +
>>>>> +     if (IS_SD(mmc)) {
>>>>> +             puts("PARTITION_CONFIG only exists on eMMC\n");
>>>>> +             return CMD_RET_FAILURE;
>>>>> +     }
>>>>> +
>>>>> +     if (mmc->part_config == MMCPART_NOAVAILABLE) {
>>>>> +             printf("No part_config info for ver. 0x%x\n",
>>>> mmc->version);
>>>>> +             return CMD_RET_FAILURE;
>>>>> +     }
>>>>> +
>>>>> +     part_emmc = EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config);
>>>>> +     part_args = simple_strtoul(argv[2], NULL, 10);
>>>>> +
>>>>> +     if (part_emmc == part_args)
>>>>> +             return CMD_RET_SUCCESS;
>>>>> +     else
>>>>> +             return CMD_RET_FAILURE;
>>>>> +}
>>>>> +
>>>>>  static int mmc_partconf_print(struct mmc *mmc)
>>>>>  {
>>>>>       u8 ack, access, part;
>>>>> @@ -953,6 +988,7 @@ static struct cmd_tbl cmd_mmc[] = {
>>>>>  #ifdef CONFIG_SUPPORT_EMMC_BOOT
>>>>>       U_BOOT_CMD_MKENT(bootbus, 5, 0, do_mmc_bootbus, "", ""),
>>>>>       U_BOOT_CMD_MKENT(bootpart-resize, 4, 0, do_mmc_boot_resize, "",
>>>> ""),
>>>>> +     U_BOOT_CMD_MKENT(partboot, 3, 0, do_mmc_partboot, "", ""),
>>
>> partboot can be confused. how about changing more clear name, like
>> mmc_bootpart_check?
> 
> 
> Agree. We have ?bootpart-resize?, and ?bootpart-check? looks very natural.
> 
> I?ll prepare V2 for
> mmc bootpart-check <dev> <boot_partition>
> 
> 
>>
>>>>>       U_BOOT_CMD_MKENT(partconf, 5, 0, do_mmc_partconf, "", ""),
>>>>>       U_BOOT_CMD_MKENT(rst-function, 3, 0, do_mmc_rst_func, "", ""),
>>>>>  #endif
>>>>> @@ -1021,6 +1057,9 @@ U_BOOT_CMD(
>>>>>       " - Set the BOOT_BUS_WIDTH field of the specified device\n"
>>>>>       "mmc bootpart-resize <dev> <boot part size MB> <RPMB part size
>>>> MB>\n"
>>>>>       " - Change sizes of boot and RPMB partitions of specified
>> device\n"
>>>>> +     "mmc partboot dev boot_partition\n"
>>
>> why it needs to pass "boot_partition"?
> 
> 
> We need to return true or false to be able to use ?if? in boot script
> (hush).
> Comparing the real value with the given one is the only way to return true
> or false. So, this argument is mandatory.

How about setting environment variable in checking command? With optional arguments.
e,g) mmc partconf-check <dev> [varname] ?

- mmc partconf-check 0
Then just returned BOOT_PARTITON_ENABLE[5:3] value.
- mmc partconf-check 0 boot_partition
Then it will be set to boot_partition as PARTITION_ENABLE[5:3] value.

boot_partition=0x2

I think that you can use %boot_partition variable in your boot script.

Refer to cmd/part.c.

How about this? It's just my opinion.

Best Regards,
Jaehoon Chung

> 
> 
>> And use more clear usage..with optional or mandatory.
>> <> - mandatory
>> [] - optional
> 
> 
> OK.
> 
> 
>>
>> mmc partboot <dev>
> 
> 
>>
>>>>> +     " - Return success if the given boot_partition value matches
>>>> BOOT_PARTITION_ENABLE\n"
>>>>> +     "   bit field of the specified device\n"
>>>>>       "mmc partconf dev [boot_ack boot_partition partition_access]\n"
>>>>>       " - Show or change the bits of the PARTITION_CONFIG field of the
>>>> specified device\n"
>>>>>       "mmc rst-function dev value\n"
>>>>>
>>>>
>>>>
>>>
>>
>>
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] cmd: mmc: add mmc partboot
  2021-02-16  9:00           ` Jaehoon Chung
@ 2021-02-17 15:35             ` gr embeter
  0 siblings, 0 replies; 9+ messages in thread
From: gr embeter @ 2021-02-17 15:35 UTC (permalink / raw)
  To: u-boot

On Tue, 16 Feb 2021 at 10:00 Jaehoon Chung <jh80.chung@samsung.com> wrote:

> On 2/16/21 5:18 PM, gr embeter wrote:
> > Hi Jaehoon,
> > thanks for your comments.
> >
> > On Tue, 16 Feb 2021 at 00:30 Jaehoon Chung <jh80.chung@samsung.com>
> wrote:
> >
> >> Hi Grygorii,
> >>
> >> On 2/15/21 10:04 PM, gr embeter wrote:
> >>> Hello Jaehoon,
> >>>
> >>> On Sun, 14 Feb 2021 at 23:08 Jaehoon Chung <jh80.chung@samsung.com>
> >> wrote:
> >>>
> >>>> Dear Grygorii,
> >>>>
> >>>> On 2/12/21 7:32 PM, grygorii tertychnyi wrote:
> >>>>> This patch allows to determine active boot partition in boot script:
> >>>>>
> >>>>> if mmc partboot ${mmcdev} 2; then
> >>>>>     echo "booted from eMMC boot1 partition"
> >>>>> fi
> >>>>
> >>>> I don't know what purpose this patch has.
> >>>
> >>>
> >>> With an eMMC as a boot source I have two boot partitions, i.e. ?boot1?
> >> and
> >>> ?boot2?, with two different versions of U-Boot on each partition.
> >>>
> >>> I also have two different kernels located on eMMC ?user? partition,
> let?s
> >>> say ?image1? and ?image2?.
> >>>
> >>> So, I want to boot ?image1? kernel if it is U-boot from ?boot1?
> partition
> >>> is booted now,
> >>> and to boot ?image2? kernel if it is U-boot from ?boot2? partition is
> >>> booted.
> >>>
> >>> It is easy to do it manually. I just added possibility to do it with
> >> U-boot
> >>> script now,
> >>> because ?mmc partconf ...? does not let you check the current boot
> >>> partition with hush.
> >>>
> >>> Hope, I explained the purpose.
> >>
> >> I remembered. Because i feel to see similar patch to use bootpart.
> >> At that time, i also asked same question. Sorry for not remembered
> yours.
> >>
> >>
> >>
> https://protect2.fireeye.com/v1/url?k=dac52f49-855e1643-dac4a406-0cc47a31384a-4f6e07c1e040623a&q=1&e=3829977a-6bc3-49e3-868a-0dcba5fab9e4&u=https%3A%2F%2Fpatchwork.ozlabs.org%2Fproject%2Fuboot%2Fpatch%2F20201212074633.891704-1-grembeter%40outlook.com%2F
> >>
> >
> > Indeed. I should?ve mentioned this link. Sorry. I?ll extend commit
> message.
> >
> >
> >>>
> >>>
> >>>
> >>>>
> >>>> Best Regards,
> >>>> Jaehoon Chung
> >>>>
> >>>>>
> >>>>> Signed-off-by: Grygorii Tertychnyi <
> >>>> grygorii.tertychnyi at leica-geosystems.com>
> >>>>> ---
> >>>>>  cmd/mmc.c | 39 +++++++++++++++++++++++++++++++++++++++
> >>>>>  1 file changed, 39 insertions(+)
> >>>>>
> >>>>> diff --git a/cmd/mmc.c b/cmd/mmc.c
> >>>>> index 1529a3e05ddd..010d6ab9aa19 100644
> >>>>> --- a/cmd/mmc.c
> >>>>> +++ b/cmd/mmc.c
> >>>>> @@ -771,6 +771,41 @@ static int do_mmc_boot_resize(struct cmd_tbl
> >>>> *cmdtp, int flag,
> >>>>>       return CMD_RET_SUCCESS;
> >>>>>  }
> >>>>>
> >>>>> +static int do_mmc_partboot(struct cmd_tbl *cmdtp, int flag,
> >>>>> +                        int argc, char *const argv[])
> >>>>> +{
> >>>>> +     int dev;
> >>>>> +     struct mmc *mmc;
> >>>>> +     u8 part_args, part_emmc;
> >>>>> +
> >>>>> +     if (argc != 3)
> >>>>> +             return CMD_RET_USAGE;
> >>>>> +
> >>>>> +     dev = simple_strtoul(argv[1], NULL, 10);
> >>>>> +
> >>>>> +     mmc = init_mmc_device(dev, false);
> >>>>> +     if (!mmc)
> >>>>> +             return CMD_RET_FAILURE;
> >>>>> +
> >>>>> +     if (IS_SD(mmc)) {
> >>>>> +             puts("PARTITION_CONFIG only exists on eMMC\n");
> >>>>> +             return CMD_RET_FAILURE;
> >>>>> +     }
> >>>>> +
> >>>>> +     if (mmc->part_config == MMCPART_NOAVAILABLE) {
> >>>>> +             printf("No part_config info for ver. 0x%x\n",
> >>>> mmc->version);
> >>>>> +             return CMD_RET_FAILURE;
> >>>>> +     }
> >>>>> +
> >>>>> +     part_emmc = EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config);
> >>>>> +     part_args = simple_strtoul(argv[2], NULL, 10);
> >>>>> +
> >>>>> +     if (part_emmc == part_args)
> >>>>> +             return CMD_RET_SUCCESS;
> >>>>> +     else
> >>>>> +             return CMD_RET_FAILURE;
> >>>>> +}
> >>>>> +
> >>>>>  static int mmc_partconf_print(struct mmc *mmc)
> >>>>>  {
> >>>>>       u8 ack, access, part;
> >>>>> @@ -953,6 +988,7 @@ static struct cmd_tbl cmd_mmc[] = {
> >>>>>  #ifdef CONFIG_SUPPORT_EMMC_BOOT
> >>>>>       U_BOOT_CMD_MKENT(bootbus, 5, 0, do_mmc_bootbus, "", ""),
> >>>>>       U_BOOT_CMD_MKENT(bootpart-resize, 4, 0, do_mmc_boot_resize, "",
> >>>> ""),
> >>>>> +     U_BOOT_CMD_MKENT(partboot, 3, 0, do_mmc_partboot, "", ""),
> >>
> >> partboot can be confused. how about changing more clear name, like
> >> mmc_bootpart_check?
> >
> >
> > Agree. We have ?bootpart-resize?, and ?bootpart-check? looks very
> natural.
> >
> > I?ll prepare V2 for
> > mmc bootpart-check <dev> <boot_partition>
> >
> >
> >>
> >>>>>       U_BOOT_CMD_MKENT(partconf, 5, 0, do_mmc_partconf, "", ""),
> >>>>>       U_BOOT_CMD_MKENT(rst-function, 3, 0, do_mmc_rst_func, "", ""),
> >>>>>  #endif
> >>>>> @@ -1021,6 +1057,9 @@ U_BOOT_CMD(
> >>>>>       " - Set the BOOT_BUS_WIDTH field of the specified device\n"
> >>>>>       "mmc bootpart-resize <dev> <boot part size MB> <RPMB part size
> >>>> MB>\n"
> >>>>>       " - Change sizes of boot and RPMB partitions of specified
> >> device\n"
> >>>>> +     "mmc partboot dev boot_partition\n"
> >>
> >> why it needs to pass "boot_partition"?
> >
> >
> > We need to return true or false to be able to use ?if? in boot script
> > (hush).
> > Comparing the real value with the given one is the only way to return
> true
> > or false. So, this argument is mandatory.
>
> How about setting environment variable in checking command? With optional
> arguments.
> e,g) mmc partconf-check <dev> [varname] ?
>
> - mmc partconf-check 0
> Then just returned BOOT_PARTITON_ENABLE[5:3] value.
> - mmc partconf-check 0 boot_partition
> Then it will be set to boot_partition as PARTITION_ENABLE[5:3] value.
>
> boot_partition=0x2
>
> I think that you can use %boot_partition variable in your boot script.
>
> Refer to cmd/part.c.
>
> How about this? It's just my opinion.


it looks like an overkill for this purpose.
it is additional ?if?: 1st to check the return value of bootpart-check (to
verify the value is set), 2nd to check the value of variable.
And if there is ?save env? call in the same boot script, we would need to
unset this new variable.


>
> Best Regards,
> Jaehoon Chung
>
> >
> >
> >> And use more clear usage..with optional or mandatory.
> >> <> - mandatory
> >> [] - optional
> >
> >
> > OK.
> >
> >
> >>
> >> mmc partboot <dev>
> >
> >
> >>
> >>>>> +     " - Return success if the given boot_partition value matches
> >>>> BOOT_PARTITION_ENABLE\n"
> >>>>> +     "   bit field of the specified device\n"
> >>>>>       "mmc partconf dev [boot_ack boot_partition partition_access]\n"
> >>>>>       " - Show or change the bits of the PARTITION_CONFIG field of
> the
> >>>> specified device\n"
> >>>>>       "mmc rst-function dev value\n"
> >>>>>
> >>>>
> >>>>
> >>>
> >>
> >>
> >
>
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2] cmd: mmc: add mmc bootpart-check
  2021-02-12 10:32 ` [PATCH] cmd: mmc: add mmc partboot grygorii tertychnyi
  2021-02-14 22:08   ` Jaehoon Chung
@ 2021-02-17 16:35   ` grygorii tertychnyi
  2021-02-17 23:15     ` Jaehoon Chung
  1 sibling, 1 reply; 9+ messages in thread
From: grygorii tertychnyi @ 2021-02-17 16:35 UTC (permalink / raw)
  To: u-boot

This patch allows to check the current boot partition. It is useful
when you use two different sets of (bootloader, kernel) images and
want to boot the corresponding kernel image in boot script. E.g.:

if mmc bootpart-check ${mmcdev} 1; then
    echo "booted from eMMC boot0 partition"
    ...load kernel image1
elif mmc bootpart-check ${mmcdev} 2; then
    echo "booted from eMMC boot1 partition"
    ...load kernel image2
fi

Signed-off-by: Grygorii Tertychnyi <grygorii.tertychnyi@leica-geosystems.com>
---
v2:
 - renamed the command, it is "bootpart-check" now
 - added <> to mandatory arguments help string
 - added more details to commit message

 cmd/mmc.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/cmd/mmc.c b/cmd/mmc.c
index 1529a3e05ddd..3332dee4b130 100644
--- a/cmd/mmc.c
+++ b/cmd/mmc.c
@@ -771,6 +771,41 @@ static int do_mmc_boot_resize(struct cmd_tbl *cmdtp, int flag,
 	return CMD_RET_SUCCESS;
 }
 
+static int do_mmc_boot_check(struct cmd_tbl *cmdtp, int flag,
+			     int argc, char *const argv[])
+{
+	int dev;
+	struct mmc *mmc;
+	u8 part_args, part_emmc;
+
+	if (argc != 3)
+		return CMD_RET_USAGE;
+
+	dev = simple_strtoul(argv[1], NULL, 10);
+
+	mmc = init_mmc_device(dev, false);
+	if (!mmc)
+		return CMD_RET_FAILURE;
+
+	if (IS_SD(mmc)) {
+		printf("It is not an eMMC device\n");
+		return CMD_RET_FAILURE;
+	}
+
+	if (mmc->part_config == MMCPART_NOAVAILABLE) {
+		printf("No part_config info for ver. 0x%x\n", mmc->version);
+		return CMD_RET_FAILURE;
+	}
+
+	part_emmc = EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config);
+	part_args = simple_strtoul(argv[2], NULL, 10);
+
+	if (part_emmc == part_args)
+		return CMD_RET_SUCCESS;
+	else
+		return CMD_RET_FAILURE;
+}
+
 static int mmc_partconf_print(struct mmc *mmc)
 {
 	u8 ack, access, part;
@@ -952,6 +987,7 @@ static struct cmd_tbl cmd_mmc[] = {
 #endif
 #ifdef CONFIG_SUPPORT_EMMC_BOOT
 	U_BOOT_CMD_MKENT(bootbus, 5, 0, do_mmc_bootbus, "", ""),
+	U_BOOT_CMD_MKENT(bootpart-check, 3, 0, do_mmc_boot_check, "", ""),
 	U_BOOT_CMD_MKENT(bootpart-resize, 4, 0, do_mmc_boot_resize, "", ""),
 	U_BOOT_CMD_MKENT(partconf, 5, 0, do_mmc_partconf, "", ""),
 	U_BOOT_CMD_MKENT(rst-function, 3, 0, do_mmc_rst_func, "", ""),
@@ -1019,6 +1055,9 @@ U_BOOT_CMD(
 #ifdef CONFIG_SUPPORT_EMMC_BOOT
 	"mmc bootbus dev boot_bus_width reset_boot_bus_width boot_mode\n"
 	" - Set the BOOT_BUS_WIDTH field of the specified device\n"
+	"mmc bootpart-check <dev> <boot_partition>\n"
+	" - Return success if the given boot_partition value matches BOOT_PARTITION_ENABLE\n"
+	"   bit field of the specified device\n"
 	"mmc bootpart-resize <dev> <boot part size MB> <RPMB part size MB>\n"
 	" - Change sizes of boot and RPMB partitions of specified device\n"
 	"mmc partconf dev [boot_ack boot_partition partition_access]\n"
-- 
2.25.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2] cmd: mmc: add mmc bootpart-check
  2021-02-17 16:35   ` [PATCH v2] cmd: mmc: add mmc bootpart-check grygorii tertychnyi
@ 2021-02-17 23:15     ` Jaehoon Chung
  0 siblings, 0 replies; 9+ messages in thread
From: Jaehoon Chung @ 2021-02-17 23:15 UTC (permalink / raw)
  To: u-boot

Hi,

On 2/18/21 1:35 AM, grygorii tertychnyi wrote:
> This patch allows to check the current boot partition. It is useful
> when you use two different sets of (bootloader, kernel) images and
> want to boot the corresponding kernel image in boot script. E.g.:
> 
> if mmc bootpart-check ${mmcdev} 1; then
>     echo "booted from eMMC boot0 partition"
>     ...load kernel image1
> elif mmc bootpart-check ${mmcdev} 2; then
>     echo "booted from eMMC boot1 partition"
>     ...load kernel image2
> fi

If mmc_bootpart-check <dev> <boot_partition> is used, it's just for your bootsripts.
Your patch is doing nothing...

U-boot> mmc bootpart-check 1 1
U-boot> mmc bootpart-check 1 2
U-boot> mmc bootpart-check 1 3

To use this command by other developer, it needs to be more useful than now.
So i think good that displayed the bootpart value in your patch.

Then it doesn't need to use "echo booted from.." in your bootscript.

e.g) if success..
U-boot> mmc bootpart-check 1 1 
Boot partition 1 enabled for boot 

if fail
U-boot> mmc bootpart-check 1 1
Boot partition 2 enabled for boot

Code can be the below..

switch (part_emmc) {
case 0x0:
	printf("Device not boot enabled");
	break;
case 0x1:
	printf("Boot partition 1 ...);
	break;
case 0x2:
	...
default:
	printf("BOOT_PARTITION_ENABLE is reserved\n");
}


Then other developer and you can get more information with this command.
Also when checking is failed, it can provide which bootpart is valid.
And this patch's commit-msg can be described more common, not only bootscript's case.

how about this?

> 
> Signed-off-by: Grygorii Tertychnyi <grygorii.tertychnyi@leica-geosystems.com>

I think better that Signed-off-by and Author's email address maintains to same.

Author: grygorii tertychnyi <grembeter@gmail.com>
Date:   Wed Feb 17 17:35:16 2021 +0100

    cmd: mmc: add mmc bootpart-check

Best Regards,
Jaehoon Chung

> ---
> v2:
>  - renamed the command, it is "bootpart-check" now
>  - added <> to mandatory arguments help string
>  - added more details to commit message
> 
>  cmd/mmc.c | 39 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/cmd/mmc.c b/cmd/mmc.c
> index 1529a3e05ddd..3332dee4b130 100644
> --- a/cmd/mmc.c
> +++ b/cmd/mmc.c
> @@ -771,6 +771,41 @@ static int do_mmc_boot_resize(struct cmd_tbl *cmdtp, int flag,
>  	return CMD_RET_SUCCESS;
>  }
>  
> +static int do_mmc_boot_check(struct cmd_tbl *cmdtp, int flag,
> +			     int argc, char *const argv[])
> +{
> +	int dev;
> +	struct mmc *mmc;
> +	u8 part_args, part_emmc;
> +
> +	if (argc != 3)
> +		return CMD_RET_USAGE;
> +
> +	dev = simple_strtoul(argv[1], NULL, 10);
> +
> +	mmc = init_mmc_device(dev, false);
> +	if (!mmc)
> +		return CMD_RET_FAILURE;
> +
> +	if (IS_SD(mmc)) {
> +		printf("It is not an eMMC device\n");
> +		return CMD_RET_FAILURE;
> +	}
> +
> +	if (mmc->part_config == MMCPART_NOAVAILABLE) {
> +		printf("No part_config info for ver. 0x%x\n", mmc->version);
> +		return CMD_RET_FAILURE;
> +	}
> +
> +	part_emmc = EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config);
> +	part_args = simple_strtoul(argv[2], NULL, 10);
> +
> +	if (part_emmc == part_args)
> +		return CMD_RET_SUCCESS;
> +	else
> +		return CMD_RET_FAILURE;
> +}
> +
>  static int mmc_partconf_print(struct mmc *mmc)
>  {
>  	u8 ack, access, part;
> @@ -952,6 +987,7 @@ static struct cmd_tbl cmd_mmc[] = {
>  #endif
>  #ifdef CONFIG_SUPPORT_EMMC_BOOT
>  	U_BOOT_CMD_MKENT(bootbus, 5, 0, do_mmc_bootbus, "", ""),
> +	U_BOOT_CMD_MKENT(bootpart-check, 3, 0, do_mmc_boot_check, "", ""),
>  	U_BOOT_CMD_MKENT(bootpart-resize, 4, 0, do_mmc_boot_resize, "", ""),
>  	U_BOOT_CMD_MKENT(partconf, 5, 0, do_mmc_partconf, "", ""),
>  	U_BOOT_CMD_MKENT(rst-function, 3, 0, do_mmc_rst_func, "", ""),
> @@ -1019,6 +1055,9 @@ U_BOOT_CMD(
>  #ifdef CONFIG_SUPPORT_EMMC_BOOT
>  	"mmc bootbus dev boot_bus_width reset_boot_bus_width boot_mode\n"
>  	" - Set the BOOT_BUS_WIDTH field of the specified device\n"
> +	"mmc bootpart-check <dev> <boot_partition>\n"
> +	" - Return success if the given boot_partition value matches BOOT_PARTITION_ENABLE\n"
> +	"   bit field of the specified device\n"
>  	"mmc bootpart-resize <dev> <boot part size MB> <RPMB part size MB>\n"
>  	" - Change sizes of boot and RPMB partitions of specified device\n"
>  	"mmc partconf dev [boot_ack boot_partition partition_access]\n"
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-02-17 23:15 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20210212103248epcas1p302270c25ac7ac0b5b9f86267acfe9860@epcas1p3.samsung.com>
2021-02-12 10:32 ` [PATCH] cmd: mmc: add mmc partboot grygorii tertychnyi
2021-02-14 22:08   ` Jaehoon Chung
2021-02-15 13:04     ` gr embeter
2021-02-15 23:30       ` Jaehoon Chung
2021-02-16  8:18         ` gr embeter
2021-02-16  9:00           ` Jaehoon Chung
2021-02-17 15:35             ` gr embeter
2021-02-17 16:35   ` [PATCH v2] cmd: mmc: add mmc bootpart-check grygorii tertychnyi
2021-02-17 23:15     ` Jaehoon Chung

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.