[PATCH] fastboot: getvar: fix partition-size return value

The size returned by 'getvar partition-size' should be in bytes, not in blocks as fastboot uses that value to generate empty partition when running format [1].
[1] https://android.googlesource.com/platform/system/core/+/refs/heads/android10...
Signed-off-by: Gary Bisson gary.bisson@boundarydevices.com --- Hi,
Another test was to run 'fastboot getvar partition-size:system' on a shipping Android phone, it will give you the size in bytes as well.
Regards, Gary --- drivers/fastboot/fb_getvar.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c index 95cb434189..51a2bea86d 100644 --- a/drivers/fastboot/fb_getvar.c +++ b/drivers/fastboot/fb_getvar.c @@ -94,7 +94,7 @@ static const struct { * * @param[in] part_name Info for which partition name to look for * @param[in,out] response Pointer to fastboot response buffer - * @param[out] size If not NULL, will contain partition size (in blocks) + * @param[out] size If not NULL, will contain partition size * @return Partition number or negative value on error */ static int getvar_get_part_info(const char *part_name, char *response, @@ -108,13 +108,13 @@ static int getvar_get_part_info(const char *part_name, char *response, r = fastboot_mmc_get_part_info(part_name, &dev_desc, &part_info, response); if (r >= 0 && size) - *size = part_info.size; + *size = part_info.size * part_info.blksz; # elif CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND) struct part_info *part_info;
r = fastboot_nand_get_part_info(part_name, &part_info, response); if (r >= 0 && size) - *size = part_info->size; + *size = part_info->size * part_info.blksz; # else fastboot_fail("this storage is not supported in bootloader", response); r = -ENODEV;

Hi,
Gentle ping on this patch. Anyone had a chance to review?
Regards, Gary
On Wed, May 06, 2020 at 10:12:28AM +0200, Gary Bisson wrote:
The size returned by 'getvar partition-size' should be in bytes, not in blocks as fastboot uses that value to generate empty partition when running format [1].
[1] https://android.googlesource.com/platform/system/core/+/refs/heads/android10...
Signed-off-by: Gary Bisson gary.bisson@boundarydevices.com
Hi,
Another test was to run 'fastboot getvar partition-size:system' on a shipping Android phone, it will give you the size in bytes as well.
Regards, Gary
drivers/fastboot/fb_getvar.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c index 95cb434189..51a2bea86d 100644 --- a/drivers/fastboot/fb_getvar.c +++ b/drivers/fastboot/fb_getvar.c @@ -94,7 +94,7 @@ static const struct {
- @param[in] part_name Info for which partition name to look for
- @param[in,out] response Pointer to fastboot response buffer
- @param[out] size If not NULL, will contain partition size (in blocks)
*/
- @param[out] size If not NULL, will contain partition size
- @return Partition number or negative value on error
static int getvar_get_part_info(const char *part_name, char *response, @@ -108,13 +108,13 @@ static int getvar_get_part_info(const char *part_name, char *response, r = fastboot_mmc_get_part_info(part_name, &dev_desc, &part_info, response); if (r >= 0 && size)
*size = part_info.size;
*size = part_info.size * part_info.blksz;
# elif CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND) struct part_info *part_info;
r = fastboot_nand_get_part_info(part_name, &part_info, response); if (r >= 0 && size)
*size = part_info->size;
*size = part_info->size * part_info.blksz;
# else fastboot_fail("this storage is not supported in bootloader", response); r = -ENODEV; -- 2.26.2

Hi,
Gentle ping on this patch. Hopefully Sam's email won't bounce this time.
Let me know if you have any questions.
Regards, Gary
On Wed, Jun 24, 2020 at 11:00:17AM +0200, Gary Bisson wrote:
Hi,
Gentle ping on this patch. Anyone had a chance to review?
Regards, Gary
On Wed, May 06, 2020 at 10:12:28AM +0200, Gary Bisson wrote:
The size returned by 'getvar partition-size' should be in bytes, not in blocks as fastboot uses that value to generate empty partition when running format [1].
[1] https://android.googlesource.com/platform/system/core/+/refs/heads/android10...
Signed-off-by: Gary Bisson gary.bisson@boundarydevices.com
Hi,
Another test was to run 'fastboot getvar partition-size:system' on a shipping Android phone, it will give you the size in bytes as well.
Regards, Gary
drivers/fastboot/fb_getvar.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c index 95cb434189..51a2bea86d 100644 --- a/drivers/fastboot/fb_getvar.c +++ b/drivers/fastboot/fb_getvar.c @@ -94,7 +94,7 @@ static const struct {
- @param[in] part_name Info for which partition name to look for
- @param[in,out] response Pointer to fastboot response buffer
- @param[out] size If not NULL, will contain partition size (in blocks)
*/
- @param[out] size If not NULL, will contain partition size
- @return Partition number or negative value on error
static int getvar_get_part_info(const char *part_name, char *response, @@ -108,13 +108,13 @@ static int getvar_get_part_info(const char *part_name, char *response, r = fastboot_mmc_get_part_info(part_name, &dev_desc, &part_info, response); if (r >= 0 && size)
*size = part_info.size;
*size = part_info.size * part_info.blksz;
# elif CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND) struct part_info *part_info;
r = fastboot_nand_get_part_info(part_name, &part_info, response); if (r >= 0 && size)
*size = part_info->size;
*size = part_info->size * part_info.blksz;
# else fastboot_fail("this storage is not supported in bootloader", response); r = -ENODEV; -- 2.26.2

Hi Gary,
Hi,
Gentle ping on this patch. Hopefully Sam's email won't bounce this time.
You couldn't have better timing than now :-)
I'm now testing PR for Tom [1] and your original patch was causing some issues (probably it was correct when it was posted, but it was my fault that I'm going to pull it in now - my apologizes).
I've fixed it [2] - please check if this fix is OK.
Now I'm hunting another issues with sandbox [3]. When fixed I will send the PR.
Links:
[1] - https://github.com/lmajewski/u-boot-dfu/commits/testing [2] - https://github.com/lmajewski/u-boot-dfu/commit/a2491ed5dd7bade94328f58ae0739... [3] - https://travis-ci.org/github/lmajewski/u-boot-dfu/jobs/641984520
Let me know if you have any questions.
Regards, Gary
On Wed, Jun 24, 2020 at 11:00:17AM +0200, Gary Bisson wrote:
Hi,
Gentle ping on this patch. Anyone had a chance to review?
Regards, Gary
On Wed, May 06, 2020 at 10:12:28AM +0200, Gary Bisson wrote:
The size returned by 'getvar partition-size' should be in bytes, not in blocks as fastboot uses that value to generate empty partition when running format [1].
[1] https://android.googlesource.com/platform/system/core/+/refs/heads/android10...
Signed-off-by: Gary Bisson gary.bisson@boundarydevices.com
Hi,
Another test was to run 'fastboot getvar partition-size:system' on a shipping Android phone, it will give you the size in bytes as well.
Regards, Gary
drivers/fastboot/fb_getvar.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c index 95cb434189..51a2bea86d 100644 --- a/drivers/fastboot/fb_getvar.c +++ b/drivers/fastboot/fb_getvar.c @@ -94,7 +94,7 @@ static const struct {
- @param[in] part_name Info for which partition name to look for
- @param[in,out] response Pointer to fastboot response buffer
- @param[out] size If not NULL, will contain partition size (in
blocks)
*/
- @param[out] size If not NULL, will contain partition size
- @return Partition number or negative value on error
static int getvar_get_part_info(const char *part_name, char *response, @@ -108,13 +108,13 @@ static int getvar_get_part_info(const char *part_name, char *response, r = fastboot_mmc_get_part_info(part_name, &dev_desc, &part_info, response); if (r >= 0 && size)
*size = part_info.size;
*size = part_info.size * part_info.blksz;
# elif CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND) struct part_info *part_info;
r = fastboot_nand_get_part_info(part_name, &part_info, response); if (r >= 0 && size)
*size = part_info->size;
*size = part_info->size * part_info.blksz;
# else fastboot_fail("this storage is not supported in bootloader", response); r = -ENODEV; -- 2.26.2
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

Hi Lukasz,
On Wed, Aug 26, 2020 at 11:36:51AM +0200, Lukasz Majewski wrote:
Hi Gary,
Hi,
Gentle ping on this patch. Hopefully Sam's email won't bounce this time.
You couldn't have better timing than now :-)
I'm now testing PR for Tom [1] and your original patch was causing some issues (probably it was correct when it was posted, but it was my fault that I'm going to pull it in now - my apologizes).
I've fixed it [2] - please check if this fix is OK.
Actually it was wrong before too, thanks for catching it! Reason is that I didn't build with FASTBOOT_FLASH_NAND config enabled which I should have done to check the second part of the change...
Now I'm hunting another issues with sandbox [3]. When fixed I will send the PR.
Sounds good. Let me know if you need anything from me.
Regards, Gary

Hi Gary,
Hi Lukasz,
On Wed, Aug 26, 2020 at 11:36:51AM +0200, Lukasz Majewski wrote:
Hi Gary,
Hi,
Gentle ping on this patch. Hopefully Sam's email won't bounce this time.
You couldn't have better timing than now :-)
I'm now testing PR for Tom [1] and your original patch was causing some issues (probably it was correct when it was posted, but it was my fault that I'm going to pull it in now - my apologizes).
I've fixed it [2] - please check if this fix is OK.
Actually it was wrong before too, thanks for catching it! Reason is that I didn't build with FASTBOOT_FLASH_NAND config enabled which I should have done to check the second part of the change...
Ok. I've found another issue with this patch - it has some issues with sunxi:
drivers/fastboot/fb_getvar.c: In function 'getvar_get_part_info':
+drivers/fastboot/fb_getvar.c:118:38: error: 'struct part_info' has no member named 'blksz'
+ 118 | *size = part_info->size * part_info->blksz;
+ | ^~
+make[3]: *** [drivers/fastboot/fb_getvar.o] Error 1
The whole CI run can be found here: https://travis-ci.org/github/lmajewski/u-boot-dfu/builds/721449368
Now I'm hunting another issues with sandbox [3]. When fixed I will send the PR.
Sounds good. Let me know if you need anything from me.
I think that the best solution would be if I drop this patch from the series and send PR (after some CI testing) without it. If you manage to fix it ASAP, then I will pull it immediately.
Regards, Gary
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

Hi Lukasz,
On Thu, Aug 27, 2020 at 08:25:51AM +0200, Lukasz Majewski wrote:
Hi Gary,
Hi Lukasz,
On Wed, Aug 26, 2020 at 11:36:51AM +0200, Lukasz Majewski wrote:
Hi Gary,
Hi,
Gentle ping on this patch. Hopefully Sam's email won't bounce this time.
You couldn't have better timing than now :-)
I'm now testing PR for Tom [1] and your original patch was causing some issues (probably it was correct when it was posted, but it was my fault that I'm going to pull it in now - my apologizes).
I've fixed it [2] - please check if this fix is OK.
Actually it was wrong before too, thanks for catching it! Reason is that I didn't build with FASTBOOT_FLASH_NAND config enabled which I should have done to check the second part of the change...
Ok. I've found another issue with this patch - it has some issues with sunxi:
drivers/fastboot/fb_getvar.c: In function 'getvar_get_part_info':
+drivers/fastboot/fb_getvar.c:118:38: error: 'struct part_info' has no member named 'blksz'
118 | *size = part_info->size * part_info->blksz;
| ^~
+make[3]: *** [drivers/fastboot/fb_getvar.o] Error 1
The whole CI run can be found here: https://travis-ci.org/github/lmajewski/u-boot-dfu/builds/721449368
Thanks, I'll take a look.
Now I'm hunting another issues with sandbox [3]. When fixed I will send the PR.
Sounds good. Let me know if you need anything from me.
I think that the best solution would be if I drop this patch from the series and send PR (after some CI testing) without it. If you manage to fix it ASAP, then I will pull it immediately.
Sure let's do this, drop my patch for now, I'll re-submit when possible.
Regards, Gary
participants (2)
-
Gary Bisson
-
Lukasz Majewski