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.ch...@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.ch...@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.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 “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" >>>>> >>>> >>>> >>> >> >> >