[PATCH v3] Fix flashing of eMMC user area with Fastboot
'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;
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;
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;
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;
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;
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;
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;
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;
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;
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;
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;
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... and https://patchwork.ozlabs.org/project/uboot/patch/20210514211505.26722-1-oleg... relevant still or is the latter v3 of the former? Thanks.
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... and https://patchwork.ozlabs.org/project/uboot/patch/20210514211505.26722-1-oleg... relevant still or is the latter v3 of the former? Thanks.
Yes, they are still relevant.
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;
participants (3)
-
Oleh Kravchenko -
Sean Anderson -
Tom Rini