* [PATCH] fastboot: getvar: fix partition-size return value
@ 2020-05-06 8:12 Gary Bisson
2020-06-24 9:00 ` Gary Bisson
0 siblings, 1 reply; 7+ messages in thread
From: Gary Bisson @ 2020-05-06 8:12 UTC (permalink / raw)
To: u-boot
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 <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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH] fastboot: getvar: fix partition-size return value
2020-05-06 8:12 [PATCH] fastboot: getvar: fix partition-size return value Gary Bisson
@ 2020-06-24 9:00 ` Gary Bisson
2020-08-26 9:01 ` Gary Bisson
0 siblings, 1 reply; 7+ messages in thread
From: Gary Bisson @ 2020-06-24 9:00 UTC (permalink / raw)
To: u-boot
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 <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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] fastboot: getvar: fix partition-size return value
2020-06-24 9:00 ` Gary Bisson
@ 2020-08-26 9:01 ` Gary Bisson
2020-08-26 9:36 ` Lukasz Majewski
0 siblings, 1 reply; 7+ messages in thread
From: Gary Bisson @ 2020-08-26 9:01 UTC (permalink / raw)
To: u-boot
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 <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
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] fastboot: getvar: fix partition-size return value
2020-08-26 9:01 ` Gary Bisson
@ 2020-08-26 9:36 ` Lukasz Majewski
2020-08-26 10:14 ` Gary Bisson
0 siblings, 1 reply; 7+ messages in thread
From: Lukasz Majewski @ 2020-08-26 9:36 UTC (permalink / raw)
To: u-boot
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 <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 at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200826/b4ddf057/attachment.sig>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] fastboot: getvar: fix partition-size return value
2020-08-26 9:36 ` Lukasz Majewski
@ 2020-08-26 10:14 ` Gary Bisson
2020-08-27 6:25 ` Lukasz Majewski
0 siblings, 1 reply; 7+ messages in thread
From: Gary Bisson @ 2020-08-26 10:14 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] fastboot: getvar: fix partition-size return value
2020-08-26 10:14 ` Gary Bisson
@ 2020-08-27 6:25 ` Lukasz Majewski
2020-08-27 7:46 ` Gary Bisson
0 siblings, 1 reply; 7+ messages in thread
From: Lukasz Majewski @ 2020-08-27 6:25 UTC (permalink / raw)
To: u-boot
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 at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200827/52a083db/attachment.sig>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] fastboot: getvar: fix partition-size return value
2020-08-27 6:25 ` Lukasz Majewski
@ 2020-08-27 7:46 ` Gary Bisson
0 siblings, 0 replies; 7+ messages in thread
From: Gary Bisson @ 2020-08-27 7:46 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-08-27 7:46 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-06 8:12 [PATCH] fastboot: getvar: fix partition-size return value Gary Bisson
2020-06-24 9:00 ` Gary Bisson
2020-08-26 9:01 ` Gary Bisson
2020-08-26 9:36 ` Lukasz Majewski
2020-08-26 10:14 ` Gary Bisson
2020-08-27 6:25 ` Lukasz Majewski
2020-08-27 7:46 ` Gary Bisson
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.