[PATCH] cmd: mmc: add mmc partboot

[gr embeter]

Hi Jaehoon,
thanks for your comments.

On Tue, 16 Feb 2021 at 00:30 Jaehoon Chung <jh80.chung at 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 at 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@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"
> >>>
> >>
> >>
> >
>
>