Re: [PATCH] cmd: mmc: add mmc partboot

2021-02-17 Thread gr embeter
On Tue, 16 Feb 2021 at 10:00 Jaehoon Chung  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 
> 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 
> >> 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=1=3829977a-6bc3-49e3-868a-0dcba5fab9e4=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.tertych...@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;
> >>>>> + }
> &

Re: [PATCH] cmd: mmc: add mmc partboot

2021-02-16 Thread gr embeter
Hi Jaehoon,
thanks for your comments.

On Tue, 16 Feb 2021 at 00:30 Jaehoon Chung  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 
> 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-grembe...@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.tertych...@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 “bootpa

Re: [PATCH] cmd: mmc: add mmc partboot

2021-02-15 Thread gr embeter
Hello Jaehoon,

On Sun, 14 Feb 2021 at 23:08 Jaehoon Chung  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.tertych...@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-resizeMB>\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"
> >
>
>


Re: [PATCH] cmd: mmc: modify partconf to be used in script

2020-12-14 Thread gr embeter
Hi Jaehoon,
thanks for replying.

On Mon, Dec 14, 2020 at 10:58 AM Jaehoon Chung  wrote:
>
> Hi,
>
> On 12/12/20 4:46 PM, grygorii tertychnyi wrote:
> > To implement dual-boot strategy we need to know what is the current
> > boot partition for U-Boot. It can be easily identified by looking at
> > the PARTITION_CONFIG value shown by "mmc partconf dev", but I didn't
> > find any way to use it in the boot script.
> >
> > Hence, modify it, so that "mmc partboot dev part_num" command returns
> > true if "part_num" is the current boot partition, otherwise it returns
> > false.
>
> Sorry. I don't understand what you want.

Here is what I meant.
We have two boot partitions on MMC: boot0 and boot1.
And two kernels on MMC: "kernel-a" and "kernel-b".

I want to have a way to determine the current boot partition to
implement the algorithm in the boot script:
if (boot0 == currect_boot_partition())
boot("kernel-a")
if (boot1 == currect_boot_partition())
boot("kernel-b")

With this patch it is possible to write:

if mmc partconf ${mmcdev} 1; then
echo ": booted from boot0"
...
fi
if mmc partconf ${mmcdev} 2; then
echo ": booted from boot1"
...
fi

> And if you want to change something about mmc partconf, then you need to 
> update the Usage.
> There is no usage about yours.
>
> And i don't want to add its. It's too dangerous because of possible to set to 
> wrong boot partition.

I will rework it and introduce a new mmc command for this specific action.



> Best Regards,
> Jaehoon Chung
>
> >
> > Signed-off-by: Grygorii Tertychnyi 
> > 
> > ---
> >  cmd/mmc.c | 23 ++-
> >  1 file changed, 22 insertions(+), 1 deletion(-)
> >
> > diff --git a/cmd/mmc.c b/cmd/mmc.c
> > index 1529a3e05ddd..c8db008bc90d 100644
> > --- a/cmd/mmc.c
> > +++ b/cmd/mmc.c
> > @@ -792,6 +792,23 @@ static int mmc_partconf_print(struct mmc *mmc)
> >   return CMD_RET_SUCCESS;
> >  }
> >
> > +static int mmc_partnum_cmp(struct mmc *mmc, u8 part_num)
> > +{
> > + u8 part;
> > +
> > + if (mmc->part_config == MMCPART_NOAVAILABLE) {
> > + printf("No part_config info for ver. 0x%x\n", mmc->version);
> > + return CMD_RET_FAILURE;
> > + }
> > +
> > + part = EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config);
> > +
> > + if (part == part_num)
> > + return CMD_RET_SUCCESS;
> > + else
> > + return CMD_RET_FAILURE;
> > +}
> > +
> >  static int do_mmc_partconf(struct cmd_tbl *cmdtp, int flag,
> >  int argc, char *const argv[])
> >  {
> > @@ -799,7 +816,7 @@ static int do_mmc_partconf(struct cmd_tbl *cmdtp, int 
> > flag,
> >   struct mmc *mmc;
> >   u8 ack, part_num, access;
> >
> > - if (argc != 2 && argc != 5)
> > + if (argc != 2 && argc != 3 && argc != 5)
> >   return CMD_RET_USAGE;
> >
> >   dev = simple_strtoul(argv[1], NULL, 10);
> > @@ -815,6 +832,10 @@ static int do_mmc_partconf(struct cmd_tbl *cmdtp, int 
> > flag,
> >
> >   if (argc == 2)
> >   return mmc_partconf_print(mmc);
> > + if (argc == 3) {
> > + part_num = simple_strtoul(argv[2], NULL, 10);
> > + return mmc_partnum_cmp(mmc, part_num);
> > + }
> >
> >   ack = simple_strtoul(argv[2], NULL, 10);
> >   part_num = simple_strtoul(argv[3], NULL, 10);
> >
>