Re: [PATCH] fastboot: getvar: fix partition-size return value
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
Re: [PATCH] fastboot: getvar: fix partition-size return value
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: lu...@denx.de pgp06UxoHS3cD.pgp Description: OpenPGP digital signature
Re: [PATCH] fastboot: getvar: fix partition-size return value
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
Re: [PATCH] fastboot: getvar: fix partition-size return value
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/a2491ed5dd7bade94328f58ae0739e6950055660 [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-release/fastboot/fastboot.cpp#1500 > > > > > > Signed-off-by: Gary Bisson > > > --- > > > 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: lu...@denx.de pgpbj0pOYYURC.pgp Description: OpenPGP digital signature
Re: [PATCH] fastboot: getvar: fix partition-size return value
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-release/fastboot/fastboot.cpp#1500 > > > > Signed-off-by: Gary Bisson > > --- > > 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 > >
Re: [PATCH] fastboot: getvar: fix partition-size return value
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-release/fastboot/fastboot.cpp#1500 > > Signed-off-by: Gary Bisson > --- > 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 >