All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] Fix flashing of eMMC user area with Fastboot
@ 2021-05-14 21:15 Oleh Kravchenko
  2021-05-14 21:26 ` Oleh Kravchenko
  0 siblings, 1 reply; 14+ messages in thread
From: Oleh Kravchenko @ 2021-05-14 21:15 UTC (permalink / raw)
  To: u-boot

'gpt' and 'mmc0' fastboot partitions have been treated as the same device,
but it is wrong.

Signed-off-by: Oleh Kravchenko <oleg@kaa.org.ua>
Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
Cc: Marek Vasut <marex@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)
+	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;
+	}
+#endif
+
 	if (fastboot_mmc_get_part_info(cmd, &dev_desc, &info, response) < 0)
 		return;
 
+#if CONFIG_IS_ENABLED(FASTBOOT_MMC_USER_SUPPORT)
+write_image:
+#endif
+
 	if (is_sparse_image(download_buffer)) {
 		struct fb_mmc_sparse sparse_priv;
 		struct sparse_storage sparse;
-- 
2.26.3

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3] Fix flashing of eMMC user area with Fastboot
  2021-05-14 21:15 [PATCH v3] Fix flashing of eMMC user area with Fastboot Oleh Kravchenko
@ 2021-05-14 21:26 ` Oleh Kravchenko
  2021-05-14 21:45   ` Sean Anderson
                     ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Oleh Kravchenko @ 2021-05-14 21:26 UTC (permalink / raw)
  To: u-boot

Hello guys,
Could you please review and merge this patch?

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@kaa.org.ua>
> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
> Cc: Marek Vasut <marex@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)
> +	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;
> +	}
> +#endif
> +
>  	if (fastboot_mmc_get_part_info(cmd, &dev_desc, &info, response) < 0)
>  		return;
>  
> +#if CONFIG_IS_ENABLED(FASTBOOT_MMC_USER_SUPPORT)
> +write_image:
> +#endif
> +
>  	if (is_sparse_image(download_buffer)) {
>  		struct fb_mmc_sparse sparse_priv;
>  		struct sparse_storage sparse;
> 

-- 
Best regards,
Oleh Kravchenko

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3] Fix flashing of eMMC user area with Fastboot
  2021-05-14 21:26 ` Oleh Kravchenko
@ 2021-05-14 21:45   ` Sean Anderson
  2021-05-14 22:10     ` Oleh Kravchenko
  2021-05-18 10:41   ` Oleh Kravchenko
  2021-05-18 14:33   ` Sean Anderson
  2 siblings, 1 reply; 14+ messages in thread
From: Sean Anderson @ 2021-05-14 21:45 UTC (permalink / raw)
  To: u-boot

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

 >
 > 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@kaa.org.ua>
 >> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
 >> Cc: Marek Vasut <marex@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?

 >> +	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?

 >> +	}
 >> +#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

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3] Fix flashing of eMMC user area with Fastboot
  2021-05-14 21:45   ` Sean Anderson
@ 2021-05-14 22:10     ` Oleh Kravchenko
  2021-05-14 22:26       ` Sean Anderson
  2021-05-14 22:27       ` Oleh Kravchenko
  0 siblings, 2 replies; 14+ messages in thread
From: Oleh Kravchenko @ 2021-05-14 22:10 UTC (permalink / raw)
  To: u-boot

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
> 
>>
>> 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@kaa.org.ua>
>>> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
>>> Cc: Marek Vasut <marex@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?
> 
>>> +??? 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?
> 
>>> +??? }
>>> +#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
> 
>>> +write_image:
>>> +#endif
>>> +
>>>?????? if (is_sparse_image(download_buffer)) {
>>>?????????? struct fb_mmc_sparse sparse_priv;
>>>?????????? struct sparse_storage sparse;
>>>
>>

-- 
Best regards,
Oleh Kravchenko

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3] Fix flashing of eMMC user area with Fastboot
  2021-05-14 22:10     ` Oleh Kravchenko
@ 2021-05-14 22:26       ` Sean Anderson
  2021-05-14 22:34         ` Oleh Kravchenko
  2021-05-14 22:27       ` Oleh Kravchenko
  1 sibling, 1 reply; 14+ messages in thread
From: Sean Anderson @ 2021-05-14 22:26 UTC (permalink / raw)
  To: u-boot



On 5/14/21 6:10 PM, Oleh Kravchenko wrote:
 > 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.

No, specifically I was requesting that you try "fastboot flash 0.1:0
foo.img" (or 1.1:0 depending on your mmc). And please send me the
failing output from the U-Boot side.

(and please address the comments inline below as well)

--Sean

 >
 > 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
 >>
 >>>
 >>> 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@kaa.org.ua>
 >>>> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
 >>>> Cc: Marek Vasut <marex@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?
 >>
 >>>> +    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?
 >>
 >>>> +    }
 >>>> +#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
 >>
 >>>> +write_image:
 >>>> +#endif
 >>>> +
 >>>>         if (is_sparse_image(download_buffer)) {
 >>>>             struct fb_mmc_sparse sparse_priv;
 >>>>             struct sparse_storage sparse;
 >>>>
 >>>
 >

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3] Fix flashing of eMMC user area with Fastboot
  2021-05-14 22:10     ` Oleh Kravchenko
  2021-05-14 22:26       ` Sean Anderson
@ 2021-05-14 22:27       ` Oleh Kravchenko
  1 sibling, 0 replies; 14+ messages in thread
From: Oleh Kravchenko @ 2021-05-14 22:27 UTC (permalink / raw)
  To: u-boot

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@kaa.org.ua>
>>>> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
>>>> Cc: Marek Vasut <marex@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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3] Fix flashing of eMMC user area with Fastboot
  2021-05-14 22:26       ` Sean Anderson
@ 2021-05-14 22:34         ` Oleh Kravchenko
  2021-05-14 22:43           ` Oleh Kravchenko
  0 siblings, 1 reply; 14+ messages in thread
From: Oleh Kravchenko @ 2021-05-14 22:34 UTC (permalink / raw)
  To: u-boot

This one works "0:0"

linux> fastboot flash 0:0 core-image-minimal.wic 
linux> target reported max download size of 419430400 bytes
linux> Sending '0:0' (402048 KB)...
linux> OKAY [ 12.613s]
linux> Writing '0:0'...
linux> OKAY [ 30.204s]
linux> Finished. Total time: 42.842s

u-boot> ** Bad partition specification mmc 0:0_a **
u-boot> Couldn't find partition mmc 0:0_a
u-boot> ** Unrecognized filesystem type **
u-boot> Starting download of 411697152 bytes
u-boot> downloading of 411697152 bytes finished
u-boot> Flashing Raw Image
u-boot> ........ wrote 411697152 bytes to '0:0'

15.05.21 01:26, Sean Anderson ????:
> 
> 
> On 5/14/21 6:10 PM, Oleh Kravchenko wrote:
>> 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.
> 
> No, specifically I was requesting that you try "fastboot flash 0.1:0
> foo.img" (or 1.1:0 depending on your mmc). And please send me the
> failing output from the U-Boot side.
> 
> (and please address the comments inline below as well)
> 
> --Sean
> 
>>
>> 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
>>>
>>>>
>>>> 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@kaa.org.ua>
>>>>> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
>>>>> Cc: Marek Vasut <marex@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?
>>>
>>>>> +??? 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?
>>>
>>>>> +??? }
>>>>> +#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
>>>
>>>>> +write_image:
>>>>> +#endif
>>>>> +
>>>>>???????? if (is_sparse_image(download_buffer)) {
>>>>>???????????? struct fb_mmc_sparse sparse_priv;
>>>>>???????????? struct sparse_storage sparse;
>>>>>
>>>>
>>

-- 
Best regards,
Oleh Kravchenko

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3] Fix flashing of eMMC user area with Fastboot
  2021-05-14 22:34         ` Oleh Kravchenko
@ 2021-05-14 22:43           ` Oleh Kravchenko
  0 siblings, 0 replies; 14+ messages in thread
From: Oleh Kravchenko @ 2021-05-14 22:43 UTC (permalink / raw)
  To: u-boot

Hm,

So next mapping works:
0.0:0 mmc0 user area
0.1:0 mmc0 boot 1        
0.2:0 mmc0 boot 2

linux> $ fastboot erase 0.1:0
linux> Erasing '0.1:0'...
linux> OKAY [  2.969s]
linux> Finished. Total time: 2.990s
linux> $ fastboot erase 0.2:0
linux> Erasing '0.2:0'...
linux> OKAY [  2.962s]
linux> Finished. Total time: 2.985s
linux> $ fastboot erase 0.0:0
linux> Erasing '0.0:0'...
linux> OKAY [ 48.224s]
linux> Finished. Total time: 48.239s


linux> $ fastboot flash -raw2sparse 0.0:0 core-image-minimal.wic 
linux> target reported max download size of 268435456 bytes
linux> Invalid sparse file format at header magic
linux> Sending sparse '0.0:0' 1/1 (221985 KB)...
linux> OKAY [  7.301s]
linux> Writing '0.0:0' 1/1...
linux> OKAY [ 31.904s]
linux> Finished. Total time: 39.298s
linux> $ fastboot flash -raw2sparse 0.1:0 u-boot.bin 
linux> target reported max download size of 268435456 bytes
linux> Sending '0.1:0' (440 KB)...
linux> OKAY [  0.020s]
linux> Writing '0.1:0'...
linux> OKAY [  0.113s]
linux> Finished. Total time: 0.166s
linux> $ fastboot flash -raw2sparse 0.2:0 u-boot.bin 
linux> target reported max download size of 268435456 bytes
linux> Sending '0.2:0' (440 KB)...
linux> OKAY [  0.020s]
linux> Writing '0.2:0'...
linux> OKAY [  0.113s]
linux> Finished. Total time: 0.168s

15.05.21 01:34, Oleh Kravchenko ????:
> This one works "0:0"
> 
> linux> fastboot flash 0:0 core-image-minimal.wic 
> linux> target reported max download size of 419430400 bytes
> linux> Sending '0:0' (402048 KB)...
> linux> OKAY [ 12.613s]
> linux> Writing '0:0'...
> linux> OKAY [ 30.204s]
> linux> Finished. Total time: 42.842s
> 
> u-boot> ** Bad partition specification mmc 0:0_a **
> u-boot> Couldn't find partition mmc 0:0_a
> u-boot> ** Unrecognized filesystem type **
> u-boot> Starting download of 411697152 bytes
> u-boot> downloading of 411697152 bytes finished
> u-boot> Flashing Raw Image
> u-boot> ........ wrote 411697152 bytes to '0:0'
> 
> 15.05.21 01:26, Sean Anderson ????:
>>
>>
>> On 5/14/21 6:10 PM, Oleh Kravchenko wrote:
>>> 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.
>>
>> No, specifically I was requesting that you try "fastboot flash 0.1:0
>> foo.img" (or 1.1:0 depending on your mmc). And please send me the
>> failing output from the U-Boot side.
>>
>> (and please address the comments inline below as well)
>>
>> --Sean
>>
>>>
>>> 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
>>>>
>>>>>
>>>>> 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@kaa.org.ua>
>>>>>> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
>>>>>> Cc: Marek Vasut <marex@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?
>>>>
>>>>>> +??? 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?
>>>>
>>>>>> +??? }
>>>>>> +#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
>>>>
>>>>>> +write_image:
>>>>>> +#endif
>>>>>> +
>>>>>> ???????? if (is_sparse_image(download_buffer)) {
>>>>>> ???????????? struct fb_mmc_sparse sparse_priv;
>>>>>> ???????????? struct sparse_storage sparse;
>>>>>>
>>>>>
>>>
> 

-- 
Best regards,
Oleh Kravchenko

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3] Fix flashing of eMMC user area with Fastboot
  2021-05-14 21:26 ` Oleh Kravchenko
  2021-05-14 21:45   ` Sean Anderson
@ 2021-05-18 10:41   ` Oleh Kravchenko
  2021-05-18 12:21     ` Tom Rini
  2021-05-18 14:33   ` Sean Anderson
  2 siblings, 1 reply; 14+ messages in thread
From: Oleh Kravchenko @ 2021-05-18 10:41 UTC (permalink / raw)
  To: u-boot

Any updates on these?

15.05.21 00:26, Oleh Kravchenko ????:
> Hello guys,
> Could you please review and merge this patch?
>
> 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@kaa.org.ua>
>> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
>> Cc: Marek Vasut <marex@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)
>> +	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;
>> +	}
>> +#endif
>> +
>>  	if (fastboot_mmc_get_part_info(cmd, &dev_desc, &info, response) < 0)
>>  		return;
>>  
>> +#if CONFIG_IS_ENABLED(FASTBOOT_MMC_USER_SUPPORT)
>> +write_image:
>> +#endif
>> +
>>  	if (is_sparse_image(download_buffer)) {
>>  		struct fb_mmc_sparse sparse_priv;
>>  		struct sparse_storage sparse;
>>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3] Fix flashing of eMMC user area with Fastboot
  2021-05-18 10:41   ` Oleh Kravchenko
@ 2021-05-18 12:21     ` Tom Rini
  2021-05-18 13:24       ` Oleh Kravchenko
  0 siblings, 1 reply; 14+ messages in thread
From: Tom Rini @ 2021-05-18 12:21 UTC (permalink / raw)
  To: u-boot

On Tue, May 18, 2021 at 01:41:06PM +0300, Oleh Kravchenko wrote:

> Any updates on these?

Sorry, my reading of what you and Sean were saying left me with the
impression no code changes were needed in the end, is that not the case?

> 
> 15.05.21 00:26, Oleh Kravchenko ????:
> > Hello guys,
> > Could you please review and merge this patch?
> >
> > 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@kaa.org.ua>
> >> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
> >> Cc: Marek Vasut <marex@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)
> >> +	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;
> >> +	}
> >> +#endif
> >> +
> >>  	if (fastboot_mmc_get_part_info(cmd, &dev_desc, &info, response) < 0)
> >>  		return;
> >>  
> >> +#if CONFIG_IS_ENABLED(FASTBOOT_MMC_USER_SUPPORT)
> >> +write_image:
> >> +#endif
> >> +
> >>  	if (is_sparse_image(download_buffer)) {
> >>  		struct fb_mmc_sparse sparse_priv;
> >>  		struct sparse_storage sparse;
> >>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210518/41155df3/attachment.sig>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3] Fix flashing of eMMC user area with Fastboot
  2021-05-18 12:21     ` Tom Rini
@ 2021-05-18 13:24       ` Oleh Kravchenko
  2021-05-18 13:40         ` Tom Rini
  0 siblings, 1 reply; 14+ messages in thread
From: Oleh Kravchenko @ 2021-05-18 13:24 UTC (permalink / raw)
  To: u-boot

Hello Tom,

18.05.21 15:21, Tom Rini ????:
> On Tue, May 18, 2021 at 01:41:06PM +0300, Oleh Kravchenko wrote:
>
>> Any updates on these?
> Sorry, my reading of what you and Sean were saying left me with the
> impression no code changes were needed in the end, is that not the case?

Sean has proposed to flash and erase by mapping through 0.0:0 (user),
0.1:0 (boot0), 0.2:0 (boot1).
This works.

But as I understand, we also can flash and erase eMMC hardware
partitions by configuration defined labels:
- CONFIG_FASTBOOT_MMC_BOOT1_NAME
- CONFIG_FASTBOOT_MMC_BOOT2_NAME
- CONFIG_FASTBOOT_MMC_USER_NAME
Currently, this feature is broken, and these patches fix that.

>> 15.05.21 00:26, Oleh Kravchenko ????:
>>> Hello guys,
>>> Could you please review and merge this patch?
>>>
>>> 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@kaa.org.ua>
>>>> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
>>>> Cc: Marek Vasut <marex@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)
>>>> +	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;
>>>> +	}
>>>> +#endif
>>>> +
>>>>  	if (fastboot_mmc_get_part_info(cmd, &dev_desc, &info, response) < 0)
>>>>  		return;
>>>>  
>>>> +#if CONFIG_IS_ENABLED(FASTBOOT_MMC_USER_SUPPORT)
>>>> +write_image:
>>>> +#endif
>>>> +
>>>>  	if (is_sparse_image(download_buffer)) {
>>>>  		struct fb_mmc_sparse sparse_priv;
>>>>  		struct sparse_storage sparse;
>>>>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3] Fix flashing of eMMC user area with Fastboot
  2021-05-18 13:24       ` Oleh Kravchenko
@ 2021-05-18 13:40         ` Tom Rini
  2021-05-18 14:32           ` Oleh Kravchenko
  0 siblings, 1 reply; 14+ messages in thread
From: Tom Rini @ 2021-05-18 13:40 UTC (permalink / raw)
  To: u-boot

On Tue, May 18, 2021 at 04:24:49PM +0300, Oleh Kravchenko wrote:
> Hello Tom,
> 
> 18.05.21 15:21, Tom Rini ????:
> > On Tue, May 18, 2021 at 01:41:06PM +0300, Oleh Kravchenko wrote:
> >
> >> Any updates on these?
> > Sorry, my reading of what you and Sean were saying left me with the
> > impression no code changes were needed in the end, is that not the case?
> 
> Sean has proposed to flash and erase by mapping through 0.0:0 (user),
> 0.1:0 (boot0), 0.2:0 (boot1).
> This works.
> 
> But as I understand, we also can flash and erase eMMC hardware
> partitions by configuration defined labels:
> - CONFIG_FASTBOOT_MMC_BOOT1_NAME
> - CONFIG_FASTBOOT_MMC_BOOT2_NAME
> - CONFIG_FASTBOOT_MMC_USER_NAME
> Currently, this feature is broken, and these patches fix that.

Ah, OK.  And are both
https://patchwork.ozlabs.org/project/uboot/patch/20210514210620.24715-1-oleg at kaa.org.ua/
and
https://patchwork.ozlabs.org/project/uboot/patch/20210514211505.26722-1-oleg at kaa.org.ua/
relevant still or is the latter v3 of the former?  Thanks.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210518/5d2bd6b7/attachment.sig>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3] Fix flashing of eMMC user area with Fastboot
  2021-05-18 13:40         ` Tom Rini
@ 2021-05-18 14:32           ` Oleh Kravchenko
  0 siblings, 0 replies; 14+ messages in thread
From: Oleh Kravchenko @ 2021-05-18 14:32 UTC (permalink / raw)
  To: u-boot

Hello Tom,

18.05.21 16:40, Tom Rini ????:
> On Tue, May 18, 2021 at 04:24:49PM +0300, Oleh Kravchenko wrote:
>> Hello Tom,
>>
>> 18.05.21 15:21, Tom Rini ????:
>>> On Tue, May 18, 2021 at 01:41:06PM +0300, Oleh Kravchenko wrote:
>>>
>>>> Any updates on these?
>>> Sorry, my reading of what you and Sean were saying left me with the
>>> impression no code changes were needed in the end, is that not the case?
>>
>> Sean has proposed to flash and erase by mapping through 0.0:0 (user),
>> 0.1:0 (boot0), 0.2:0 (boot1).
>> This works.
>>
>> But as I understand, we also can flash and erase eMMC hardware
>> partitions by configuration defined labels:
>> - CONFIG_FASTBOOT_MMC_BOOT1_NAME
>> - CONFIG_FASTBOOT_MMC_BOOT2_NAME
>> - CONFIG_FASTBOOT_MMC_USER_NAME
>> Currently, this feature is broken, and these patches fix that.
> 
> Ah, OK.  And are both
> https://patchwork.ozlabs.org/project/uboot/patch/20210514210620.24715-1-oleg at kaa.org.ua/
> and
> https://patchwork.ozlabs.org/project/uboot/patch/20210514211505.26722-1-oleg at kaa.org.ua/
> relevant still or is the latter v3 of the former?  Thanks.
> 

Yes, they are still relevant.

-- 
Best regards,
Oleh Kravchenko

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3] Fix flashing of eMMC user area with Fastboot
  2021-05-14 21:26 ` Oleh Kravchenko
  2021-05-14 21:45   ` Sean Anderson
  2021-05-18 10:41   ` Oleh Kravchenko
@ 2021-05-18 14:33   ` Sean Anderson
  2 siblings, 0 replies; 14+ messages in thread
From: Sean Anderson @ 2021-05-18 14:33 UTC (permalink / raw)
  To: u-boot



On 5/14/21 5:26 PM, Oleh Kravchenko wrote:
 > Hello guys,
 > Could you please review and merge this patch?
 >
 > 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@kaa.org.ua>
 >> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
 >> Cc: Marek Vasut <marex@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)
 >> +	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;
 >> +	}
 >> +#endif
 >> +
 >>   	if (fastboot_mmc_get_part_info(cmd, &dev_desc, &info, response) < 0)
 >>   		return;
 >>
 >> +#if CONFIG_IS_ENABLED(FASTBOOT_MMC_USER_SUPPORT)
 >> +write_image:
 >> +#endif
 >> +

This is still ugly; perhaps we can combine this with the above if statement. E.g.

	if (!info.size
  	    && fastboot_mmc_get_part_info(cmd, &dev_desc, &info, response) < 0)
  		return;

--Sean

 >>   	if (is_sparse_image(download_buffer)) {
 >>   		struct fb_mmc_sparse sparse_priv;
 >>   		struct sparse_storage sparse;
 >>
 >

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2021-05-18 14:33 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-14 21:15 [PATCH v3] Fix flashing of eMMC user area with Fastboot Oleh Kravchenko
2021-05-14 21:26 ` Oleh Kravchenko
2021-05-14 21:45   ` Sean Anderson
2021-05-14 22:10     ` Oleh Kravchenko
2021-05-14 22:26       ` Sean Anderson
2021-05-14 22:34         ` Oleh Kravchenko
2021-05-14 22:43           ` Oleh Kravchenko
2021-05-14 22:27       ` Oleh Kravchenko
2021-05-18 10:41   ` Oleh Kravchenko
2021-05-18 12:21     ` Tom Rini
2021-05-18 13:24       ` Oleh Kravchenko
2021-05-18 13:40         ` Tom Rini
2021-05-18 14:32           ` Oleh Kravchenko
2021-05-18 14:33   ` Sean Anderson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.