[PATCH] cmd: mmc: add mmc partboot
Jaehoon Chung
jh80.chung at samsung.com
Tue Feb 16 10:00:35 CET 2021
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 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://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"
>>>>>
>>>>
>>>>
>>>
>>
>>
>
More information about the U-Boot
mailing list