[PATCH v3] Fix flashing of eMMC user area with Fastboot

[Oleh Kravchenko]

Hello Sean,
Please see my comments below:

15.05.21 01:10, Oleh Kravchenko пише:
> Hello Sean,
> Could you please clarify what you have mean?
> 
> I think you pointing to this?
>> fastboot_raw_partition_<raw partition name>=<offset> <size> [mmcpart <num>]
> 
> Because I don't have idea how aliases will help to flash.
> 
> 15.05.21 00:45, Sean Anderson пише:
>> On 5/14/21 5:26 PM, Oleh Kravchenko wrote:
>>> Hello guys,
>>> Could you please review and merge this patch?
>>
>> Did you have any luck with the second suggestion [1] I made for your
>> original patch?
>>
>> [1] https://lists.denx.de/pipermail/u-boot/2021-May/449778.html
>>
>> --Sean

Hey, I've just tested these:

uboot> setenv fastboot_raw_partition_user '0 3850371072 mmcpart 0'
uboot> setenv fastboot_raw_partition_boot0 '0 16777216 mmcpart 1'
uboot> setenv fastboot_raw_partition_boot1 '0 16777216 mmcpart 2'

linux> fastboot flash core-image-minimal.wic
linux> fastboot flash boot0 u-boot.bin 
linux> fastboot flash boot1 u-boot.bin 

And it works!

-- Oleh

>>
>>>
>>> PR successfully passed CI:
>>> https://github.com/u-boot/u-boot/pull/75
>>>
>>> 15.05.21 00:15, Oleh Kravchenko пише:
>>>> 'gpt' and 'mmc0' fastboot partitions have been treated as the same device,
>>>> but it is wrong.
>>>>
>>>> Signed-off-by: Oleh Kravchenko <oleg at kaa.org.ua>
>>>> Cc: Pantelis Antoniou <panto at antoniou-consulting.com>
>>>> Cc: Marek Vasut <marex at denx.de>
>>>> ---
>>>> Changes for v2:
>>>>      - code cleanup;
>>>> Changes for v3:
>>>>      - QA passed at https://github.com/u-boot/u-boot/pull/75;
>>>>
>>>>    drivers/fastboot/fb_mmc.c | 25 ++++++++++++++++++++-----
>>>>    1 file changed, 20 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/fastboot/fb_mmc.c b/drivers/fastboot/fb_mmc.c
>>>> index 2f3837e559..647d3f6c1b 100644
>>>> --- a/drivers/fastboot/fb_mmc.c
>>>> +++ b/drivers/fastboot/fb_mmc.c
>>>> @@ -532,12 +532,7 @@ void fastboot_mmc_flash_write(const char *cmd, void *download_buffer,
>>>>    #endif
>>>>
>>>>    #if CONFIG_IS_ENABLED(EFI_PARTITION)
>>>> -#ifndef CONFIG_FASTBOOT_MMC_USER_SUPPORT
>>>>        if (strcmp(cmd, CONFIG_FASTBOOT_GPT_NAME) == 0) {
>>>> -#else
>>>> -    if (strcmp(cmd, CONFIG_FASTBOOT_GPT_NAME) == 0 ||
>>>> -        strcmp(cmd, CONFIG_FASTBOOT_MMC_USER_NAME) == 0) {
>>>> -#endif
>>>>            dev_desc = fastboot_mmc_get_dev(response);
>>>>            if (!dev_desc)
>>>>                return;
>>>> @@ -599,9 +594,29 @@ void fastboot_mmc_flash_write(const char *cmd, void *download_buffer,
>>>>        }
>>>>    #endif
>>>>
>>>> +#if CONFIG_IS_ENABLED(FASTBOOT_MMC_USER_SUPPORT)
>>
>> Is it possible to use
>>
>> if (CONFIG_IS_ENABLED(...)) { ... }
>>
>> here?
>>

No,
It will cause an error where CONFIG_FASTBOOT_MMC_USER_NAME not defined.

-- Oleh

>>>> +    if (strcmp(cmd, CONFIG_FASTBOOT_MMC_USER_NAME) == 0) {
>>>> +        dev_desc = fastboot_mmc_get_dev(response);
>>>> +        if (!dev_desc)
>>>> +            return;
>>>> +
>>>> +        memset(&info, 0, sizeof(info));
>>>> +        info.start    = 0;
>>>> +        info.size    = dev_desc->lba;
>>>> +        info.blksz    = dev_desc->blksz;
>>>> +        strlcpy((char *)&info.name, cmd, sizeof(info.name));
>>>> +
>>>> +        goto write_image;
>>
>> Why do we need to skip get_part_info?
>>

fastboot_mmc_get_part_info() doesn't check for CONFIG_FASTBOOT_MMC_USER_NAME.

>>>> +    }
>>>> +#endif
>>>> +
>>>>        if (fastboot_mmc_get_part_info(cmd, &dev_desc, &info, response) < 0)
>>>>            return;
>>>>
>>>> +#if CONFIG_IS_ENABLED(FASTBOOT_MMC_USER_SUPPORT)
>>
>> Is conditionally defining this label necessary?
>>
>> --Sean
>>

Yes,
in other case some configuration causes error goto label is not used.

-- Oleh


>>>> +write_image:
>>>> +#endif
>>>> +
>>>>        if (is_sparse_image(download_buffer)) {
>>>>            struct fb_mmc_sparse sparse_priv;
>>>>            struct sparse_storage sparse;
>>>>
>>>
> 

-- 
Best regards,
Oleh Kravchenko