All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v2 0/2] fastboot: Support new A/B slot format
@ 2019-06-14 15:50 Sam Protsenko
  2019-06-14 15:50 ` [U-Boot] [PATCH v2 1/2] fastboot: Remove "slot-suffixes" variable Sam Protsenko
  2019-06-14 15:50 ` [U-Boot] [PATCH v2 2/2] fastboot: Fix slot names reported by getvar Sam Protsenko
  0 siblings, 2 replies; 7+ messages in thread
From: Sam Protsenko @ 2019-06-14 15:50 UTC (permalink / raw)
  To: u-boot

Android fastboot tool was changed recently:
  1. "slot-suffixes" variable was removed
  2. Now slot format is "a" instead of "_a"

That leads to issues with current U-Boot, e.g. "fastboot flash" to
slotted partitions ("boot_a", etc) is broken. This patch series fixes
that by getting U-Boot in sync with current AOSP/master w.r.t. fastboot
tool.

Sam Protsenko (2):
  fastboot: Remove "slot-suffixes" variable
  fastboot: Fix slot names reported by getvar

 drivers/fastboot/fb_getvar.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

-- 
2.20.1

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

* [U-Boot] [PATCH v2 1/2] fastboot: Remove "slot-suffixes" variable
  2019-06-14 15:50 [U-Boot] [PATCH v2 0/2] fastboot: Support new A/B slot format Sam Protsenko
@ 2019-06-14 15:50 ` Sam Protsenko
  2019-06-14 15:50 ` [U-Boot] [PATCH v2 2/2] fastboot: Fix slot names reported by getvar Sam Protsenko
  1 sibling, 0 replies; 7+ messages in thread
From: Sam Protsenko @ 2019-06-14 15:50 UTC (permalink / raw)
  To: u-boot

"slot-suffixes" variable was dropped in fastboot tool (in [1]). Let's
track AOSP/master and drop this variable in U-Boot as well.

[1] https://android.googlesource.com/platform/system/core/+/42b18a518bac85c3eea14206f6cbafbd1e98a31f

Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
---
Changes in v2:
  - add this patch to patch series
  - drop slot-suffix variable instead of returning "a,b"

 drivers/fastboot/fb_getvar.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c
index 584c7f0263..f89c7f62e6 100644
--- a/drivers/fastboot/fb_getvar.c
+++ b/drivers/fastboot/fb_getvar.c
@@ -19,7 +19,6 @@ static void getvar_version_baseband(char *var_parameter, char *response);
 static void getvar_product(char *var_parameter, char *response);
 static void getvar_platform(char *var_parameter, char *response);
 static void getvar_current_slot(char *var_parameter, char *response);
-static void getvar_slot_suffixes(char *var_parameter, char *response);
 #if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
 static void getvar_has_slot(char *var_parameter, char *response);
 #endif
@@ -64,9 +63,6 @@ static const struct {
 	}, {
 		.variable = "current-slot",
 		.dispatch = getvar_current_slot
-	}, {
-		.variable = "slot-suffixes",
-		.dispatch = getvar_slot_suffixes
 #if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
 	}, {
 		.variable = "has-slot",
@@ -182,11 +178,6 @@ static void getvar_current_slot(char *var_parameter, char *response)
 	fastboot_okay("_a", response);
 }
 
-static void getvar_slot_suffixes(char *var_parameter, char *response)
-{
-	fastboot_okay("_a,_b", response);
-}
-
 #if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
 static void getvar_has_slot(char *part_name, char *response)
 {
-- 
2.20.1

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

* [U-Boot] [PATCH v2 2/2] fastboot: Fix slot names reported by getvar
  2019-06-14 15:50 [U-Boot] [PATCH v2 0/2] fastboot: Support new A/B slot format Sam Protsenko
  2019-06-14 15:50 ` [U-Boot] [PATCH v2 1/2] fastboot: Remove "slot-suffixes" variable Sam Protsenko
@ 2019-06-14 15:50 ` Sam Protsenko
  2019-06-15 15:28   ` Lukasz Majewski
  1 sibling, 1 reply; 7+ messages in thread
From: Sam Protsenko @ 2019-06-14 15:50 UTC (permalink / raw)
  To: u-boot

Fastboot tool recently underwent changes w.r.t. A/B slot format:
  1. In commit [1] the new slot format was introduced, according to new
     A/B specification [2]. New slot format is "a", and old format "_a"
     is now considered legacy.
  2. In commit [3] the legacy format "_a" was fixed and fastboot tool
     should support both "a" and "_a" slot formats now.
  3. Finally, commit [4] drops the legacy slot format ("_a") completely.
     That makes the latest fastboot tool incompatible with "_a" format.

Last change leads to next error, when running "fastboot flash" with
current U-Boot:

    $ fastboot flash boot boot.img
    Sending 'boot__a' (11301 KB)    OKAY [  0.451s]
    Writing 'boot__a'               FAILED (remote: 'cannot find partition')
    fastboot: error: Command failed

To overcome this issue we should report slot names in "a" format instead
of "_a". Of course, this change breaks U-Boot compatibility with older
fastboot tools, but that shouldn't be a problem as A/B is not
implemented in U-Boot yet, and there are not users for slotted
partitions out there anyway. This fact can be checked like this:

    $ grep -Ir \b'boot_a\b' *

Let's use new slot format in order to fix "fastboot flash" with slotted
partitions and to be in sync with AOSP master.

With this patch, U-Boot depends on most recent fastboot tool (patch [1]
or later), for working with slotted partitions.

[1] https://android.googlesource.com/platform/system/core/+/8091947847d5e5130b09d2ac0a4bdc900f3b77c5
[2] https://source.android.com/devices/tech/ota/ab/ab_implement#partitions
[3] https://android.googlesource.com/platform/system/core/+/04396f62da6150b94e02d50e5302fd980048833d
[4] https://android.googlesource.com/platform/system/core/+/42b18a518bac85c3eea14206f6cbafbd1e98a31f

Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
---
Changes in v2:
  - don't change slot format in "slot-suffixes" variable (it's now
    dropped in [PATCH 1/2])
  - improve commit message

 drivers/fastboot/fb_getvar.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c
index f89c7f62e6..9ee5054485 100644
--- a/drivers/fastboot/fb_getvar.c
+++ b/drivers/fastboot/fb_getvar.c
@@ -174,8 +174,8 @@ static void getvar_platform(char *var_parameter, char *response)
 
 static void getvar_current_slot(char *var_parameter, char *response)
 {
-	/* A/B not implemented, for now always return _a */
-	fastboot_okay("_a", response);
+	/* A/B not implemented, for now always return "a" */
+	fastboot_okay("a", response);
 }
 
 #if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
-- 
2.20.1

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

* [U-Boot] [PATCH v2 2/2] fastboot: Fix slot names reported by getvar
  2019-06-14 15:50 ` [U-Boot] [PATCH v2 2/2] fastboot: Fix slot names reported by getvar Sam Protsenko
@ 2019-06-15 15:28   ` Lukasz Majewski
  2019-06-18 14:34     ` Sam Protsenko
  0 siblings, 1 reply; 7+ messages in thread
From: Lukasz Majewski @ 2019-06-15 15:28 UTC (permalink / raw)
  To: u-boot

Hi Sam,

> Fastboot tool recently underwent changes w.r.t. A/B slot format:
>   1. In commit [1] the new slot format was introduced, according to
> new A/B specification [2]. New slot format is "a", and old format "_a"
>      is now considered legacy.
>   2. In commit [3] the legacy format "_a" was fixed and fastboot tool
>      should support both "a" and "_a" slot formats now.
>   3. Finally, commit [4] drops the legacy slot format ("_a")
> completely. That makes the latest fastboot tool incompatible with
> "_a" format.
> 
> Last change leads to next error, when running "fastboot flash" with
> current U-Boot:
> 
>     $ fastboot flash boot boot.img
>     Sending 'boot__a' (11301 KB)    OKAY [  0.451s]
>     Writing 'boot__a'               FAILED (remote: 'cannot find
> partition') fastboot: error: Command failed
> 
> To overcome this issue we should report slot names in "a" format
> instead of "_a". Of course, this change breaks U-Boot compatibility
> with older fastboot tools, but that shouldn't be a problem as A/B is
> not implemented in U-Boot yet, and there are not users for slotted
> partitions out there anyway. This fact can be checked like this:
> 
>     $ grep -Ir \b'boot_a\b' *
> 
> Let's use new slot format in order to fix "fastboot flash" with
> slotted partitions and to be in sync with AOSP master.
> 
> With this patch, U-Boot depends on most recent fastboot tool (patch
> [1] or later), for working with slotted partitions.

Is there any "version" number scheme for the fastboot protocol
specification?

I do remember that in the past there were some mismatches for some
"fastboot" specification depending on vendor. Now it turns out that
there is a problem between AOSP versions ...

(My point is that for example even lthor had version number for
specification update. Is something similar for fastboot?). 

> 
> [1]
> https://android.googlesource.com/platform/system/core/+/8091947847d5e5130b09d2ac0a4bdc900f3b77c5
> [2]
> https://source.android.com/devices/tech/ota/ab/ab_implement#partitions
> [3]
> https://android.googlesource.com/platform/system/core/+/04396f62da6150b94e02d50e5302fd980048833d
> [4]
> https://android.googlesource.com/platform/system/core/+/42b18a518bac85c3eea14206f6cbafbd1e98a31f
> 
> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> ---
> Changes in v2:
>   - don't change slot format in "slot-suffixes" variable (it's now
>     dropped in [PATCH 1/2])
>   - improve commit message
> 
>  drivers/fastboot/fb_getvar.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/fastboot/fb_getvar.c
> b/drivers/fastboot/fb_getvar.c index f89c7f62e6..9ee5054485 100644
> --- a/drivers/fastboot/fb_getvar.c
> +++ b/drivers/fastboot/fb_getvar.c
> @@ -174,8 +174,8 @@ static void getvar_platform(char *var_parameter,
> char *response) 
>  static void getvar_current_slot(char *var_parameter, char *response)
>  {
> -	/* A/B not implemented, for now always return _a */
> -	fastboot_okay("_a", response);
> +	/* A/B not implemented, for now always return "a" */
> +	fastboot_okay("a", response);
>  }
>  
>  #if CONFIG_IS_ENABLED(FASTBOOT_FLASH)




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: <http://lists.denx.de/pipermail/u-boot/attachments/20190615/02fbfd14/attachment.sig>

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

* [U-Boot] [PATCH v2 2/2] fastboot: Fix slot names reported by getvar
  2019-06-15 15:28   ` Lukasz Majewski
@ 2019-06-18 14:34     ` Sam Protsenko
  2019-06-18 21:10       ` Lukasz Majewski
  0 siblings, 1 reply; 7+ messages in thread
From: Sam Protsenko @ 2019-06-18 14:34 UTC (permalink / raw)
  To: u-boot

Hi Lukasz,

Seems like the versioning is basically useless in fastboot USB
protocol. It exists, but only host requests the version from device
(bootloader), like "fastboot getvar version". It always returns "0.4"
[1], and it was unchanged for 5 years or so. I didn't find any
handshake with protocol version exchange. Alas, as I mentioned before,
we probably can't keep the backward compatibility in universal way.
The only way I see to make U-Boot work with both "a" and "_a"
prefixes, is to introduce some new config option or dedicated
environment variable, so that user can specify which format should be
used.

I suggest merging this as is, as we don't have any users for slotted
partitions in upstream U-Boot anyway, at this moment. If need arises,
we can add option or variable later. Of course, I don't suggest to
merge this right now, as codebase is frozen, and this is not an ideal
bugfix and can even break backward compatibility. Let's take it when
merge window opens.

Please tell me what is the preferred course of actions on this one,
from your point of view.

Thanks!

[1] http://git.denx.de/?p=u-boot.git;a=blob;f=drivers/fastboot/fb_getvar.c;h=4268628f5ef0210507c5d23f2e4209b2afc07029;hb=refs/heads/master#l84
[2] https://android.googlesource.com/platform/system/core/+/9bfecb0e3444306ec57d7fafe4a99a47d3848c17%5E%21/#F0

On Sat, Jun 15, 2019 at 6:29 PM Lukasz Majewski <lukma@denx.de> wrote:
>
> Hi Sam,
>
> > Fastboot tool recently underwent changes w.r.t. A/B slot format:
> >   1. In commit [1] the new slot format was introduced, according to
> > new A/B specification [2]. New slot format is "a", and old format "_a"
> >      is now considered legacy.
> >   2. In commit [3] the legacy format "_a" was fixed and fastboot tool
> >      should support both "a" and "_a" slot formats now.
> >   3. Finally, commit [4] drops the legacy slot format ("_a")
> > completely. That makes the latest fastboot tool incompatible with
> > "_a" format.
> >
> > Last change leads to next error, when running "fastboot flash" with
> > current U-Boot:
> >
> >     $ fastboot flash boot boot.img
> >     Sending 'boot__a' (11301 KB)    OKAY [  0.451s]
> >     Writing 'boot__a'               FAILED (remote: 'cannot find
> > partition') fastboot: error: Command failed
> >
> > To overcome this issue we should report slot names in "a" format
> > instead of "_a". Of course, this change breaks U-Boot compatibility
> > with older fastboot tools, but that shouldn't be a problem as A/B is
> > not implemented in U-Boot yet, and there are not users for slotted
> > partitions out there anyway. This fact can be checked like this:
> >
> >     $ grep -Ir \b'boot_a\b' *
> >
> > Let's use new slot format in order to fix "fastboot flash" with
> > slotted partitions and to be in sync with AOSP master.
> >
> > With this patch, U-Boot depends on most recent fastboot tool (patch
> > [1] or later), for working with slotted partitions.
>
> Is there any "version" number scheme for the fastboot protocol
> specification?
>
> I do remember that in the past there were some mismatches for some
> "fastboot" specification depending on vendor. Now it turns out that
> there is a problem between AOSP versions ...
>
> (My point is that for example even lthor had version number for
> specification update. Is something similar for fastboot?).
>
> >
> > [1]
> > https://android.googlesource.com/platform/system/core/+/8091947847d5e5130b09d2ac0a4bdc900f3b77c5
> > [2]
> > https://source.android.com/devices/tech/ota/ab/ab_implement#partitions
> > [3]
> > https://android.googlesource.com/platform/system/core/+/04396f62da6150b94e02d50e5302fd980048833d
> > [4]
> > https://android.googlesource.com/platform/system/core/+/42b18a518bac85c3eea14206f6cbafbd1e98a31f
> >
> > Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> > ---
> > Changes in v2:
> >   - don't change slot format in "slot-suffixes" variable (it's now
> >     dropped in [PATCH 1/2])
> >   - improve commit message
> >
> >  drivers/fastboot/fb_getvar.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/fastboot/fb_getvar.c
> > b/drivers/fastboot/fb_getvar.c index f89c7f62e6..9ee5054485 100644
> > --- a/drivers/fastboot/fb_getvar.c
> > +++ b/drivers/fastboot/fb_getvar.c
> > @@ -174,8 +174,8 @@ static void getvar_platform(char *var_parameter,
> > char *response)
> >  static void getvar_current_slot(char *var_parameter, char *response)
> >  {
> > -     /* A/B not implemented, for now always return _a */
> > -     fastboot_okay("_a", response);
> > +     /* A/B not implemented, for now always return "a" */
> > +     fastboot_okay("a", response);
> >  }
> >
> >  #if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
>
>
>
>
> 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

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

* [U-Boot] [PATCH v2 2/2] fastboot: Fix slot names reported by getvar
  2019-06-18 14:34     ` Sam Protsenko
@ 2019-06-18 21:10       ` Lukasz Majewski
  2019-06-19 12:21         ` Sam Protsenko
  0 siblings, 1 reply; 7+ messages in thread
From: Lukasz Majewski @ 2019-06-18 21:10 UTC (permalink / raw)
  To: u-boot

On Tue, 18 Jun 2019 17:34:54 +0300
Sam Protsenko <semen.protsenko@linaro.org> wrote:

> Hi Lukasz,
> 
> Seems like the versioning is basically useless in fastboot USB
> protocol. It exists, but only host requests the version from device
> (bootloader), like "fastboot getvar version". It always returns "0.4"
> [1], and it was unchanged for 5 years or so. I didn't find any
> handshake with protocol version exchange. Alas, as I mentioned before,
> we probably can't keep the backward compatibility in universal way.
> The only way I see to make U-Boot work with both "a" and "_a"
> prefixes, is to introduce some new config option or dedicated
> environment variable, so that user can specify which format should be
> used.
> 
> I suggest merging this as is, as we don't have any users for slotted
> partitions in upstream U-Boot anyway, at this moment. If need arises,
> we can add option or variable later. Of course, I don't suggest to
> merge this right now, as codebase is frozen, and this is not an ideal
> bugfix and can even break backward compatibility. Let's take it when
> merge window opens.
> 
> Please tell me what is the preferred course of actions on this one,
> from your point of view.

As you stated in the other mail - you shall prepare some fix for
current code base (without the _a suffix) for this release. Please
proceed.

Regarding the supported features - maybe we shall introduce new env
variable - like fastboot_version = "0.5" (then - 0.6, 0.7).

And in U-Boot just decide which set of features would be 0.5 or 0.6.

Then, we can also use Kconfig option to enable needed features.

Otherwise, we will have regressions all the time as the "fastboot"
protocol changes without any notice in the versioning (inside the
protocol).

> 
> Thanks!
> 
> [1]
> http://git.denx.de/?p=u-boot.git;a=blob;f=drivers/fastboot/fb_getvar.c;h=4268628f5ef0210507c5d23f2e4209b2afc07029;hb=refs/heads/master#l84
> [2]
> https://android.googlesource.com/platform/system/core/+/9bfecb0e3444306ec57d7fafe4a99a47d3848c17%5E%21/#F0
> 
> On Sat, Jun 15, 2019 at 6:29 PM Lukasz Majewski <lukma@denx.de> wrote:
> >
> > Hi Sam,
> >  
> > > Fastboot tool recently underwent changes w.r.t. A/B slot format:
> > >   1. In commit [1] the new slot format was introduced, according
> > > to new A/B specification [2]. New slot format is "a", and old
> > > format "_a" is now considered legacy.
> > >   2. In commit [3] the legacy format "_a" was fixed and fastboot
> > > tool should support both "a" and "_a" slot formats now.
> > >   3. Finally, commit [4] drops the legacy slot format ("_a")
> > > completely. That makes the latest fastboot tool incompatible with
> > > "_a" format.
> > >
> > > Last change leads to next error, when running "fastboot flash"
> > > with current U-Boot:
> > >
> > >     $ fastboot flash boot boot.img
> > >     Sending 'boot__a' (11301 KB)    OKAY [  0.451s]
> > >     Writing 'boot__a'               FAILED (remote: 'cannot find
> > > partition') fastboot: error: Command failed
> > >
> > > To overcome this issue we should report slot names in "a" format
> > > instead of "_a". Of course, this change breaks U-Boot
> > > compatibility with older fastboot tools, but that shouldn't be a
> > > problem as A/B is not implemented in U-Boot yet, and there are
> > > not users for slotted partitions out there anyway. This fact can
> > > be checked like this:
> > >
> > >     $ grep -Ir \b'boot_a\b' *
> > >
> > > Let's use new slot format in order to fix "fastboot flash" with
> > > slotted partitions and to be in sync with AOSP master.
> > >
> > > With this patch, U-Boot depends on most recent fastboot tool
> > > (patch [1] or later), for working with slotted partitions.  
> >
> > Is there any "version" number scheme for the fastboot protocol
> > specification?
> >
> > I do remember that in the past there were some mismatches for some
> > "fastboot" specification depending on vendor. Now it turns out that
> > there is a problem between AOSP versions ...
> >
> > (My point is that for example even lthor had version number for
> > specification update. Is something similar for fastboot?).
> >  
> > >
> > > [1]
> > > https://android.googlesource.com/platform/system/core/+/8091947847d5e5130b09d2ac0a4bdc900f3b77c5
> > > [2]
> > > https://source.android.com/devices/tech/ota/ab/ab_implement#partitions
> > > [3]
> > > https://android.googlesource.com/platform/system/core/+/04396f62da6150b94e02d50e5302fd980048833d
> > > [4]
> > > https://android.googlesource.com/platform/system/core/+/42b18a518bac85c3eea14206f6cbafbd1e98a31f
> > >
> > > Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> > > ---
> > > Changes in v2:
> > >   - don't change slot format in "slot-suffixes" variable (it's now
> > >     dropped in [PATCH 1/2])
> > >   - improve commit message
> > >
> > >  drivers/fastboot/fb_getvar.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/fastboot/fb_getvar.c
> > > b/drivers/fastboot/fb_getvar.c index f89c7f62e6..9ee5054485 100644
> > > --- a/drivers/fastboot/fb_getvar.c
> > > +++ b/drivers/fastboot/fb_getvar.c
> > > @@ -174,8 +174,8 @@ static void getvar_platform(char
> > > *var_parameter, char *response)
> > >  static void getvar_current_slot(char *var_parameter, char
> > > *response) {
> > > -     /* A/B not implemented, for now always return _a */
> > > -     fastboot_okay("_a", response);
> > > +     /* A/B not implemented, for now always return "a" */
> > > +     fastboot_okay("a", response);
> > >  }
> > >
> > >  #if CONFIG_IS_ENABLED(FASTBOOT_FLASH)  
> >
> >
> >
> >
> > 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  




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: <http://lists.denx.de/pipermail/u-boot/attachments/20190618/a4c40073/attachment.sig>

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

* [U-Boot] [PATCH v2 2/2] fastboot: Fix slot names reported by getvar
  2019-06-18 21:10       ` Lukasz Majewski
@ 2019-06-19 12:21         ` Sam Protsenko
  0 siblings, 0 replies; 7+ messages in thread
From: Sam Protsenko @ 2019-06-19 12:21 UTC (permalink / raw)
  To: u-boot

On Wed, Jun 19, 2019 at 12:10 AM Lukasz Majewski <lukma@denx.de> wrote:
>
> On Tue, 18 Jun 2019 17:34:54 +0300
> Sam Protsenko <semen.protsenko@linaro.org> wrote:
>
> > Hi Lukasz,
> >
> > Seems like the versioning is basically useless in fastboot USB
> > protocol. It exists, but only host requests the version from device
> > (bootloader), like "fastboot getvar version". It always returns "0.4"
> > [1], and it was unchanged for 5 years or so. I didn't find any
> > handshake with protocol version exchange. Alas, as I mentioned before,
> > we probably can't keep the backward compatibility in universal way.
> > The only way I see to make U-Boot work with both "a" and "_a"
> > prefixes, is to introduce some new config option or dedicated
> > environment variable, so that user can specify which format should be
> > used.
> >
> > I suggest merging this as is, as we don't have any users for slotted
> > partitions in upstream U-Boot anyway, at this moment. If need arises,
> > we can add option or variable later. Of course, I don't suggest to
> > merge this right now, as codebase is frozen, and this is not an ideal
> > bugfix and can even break backward compatibility. Let's take it when
> > merge window opens.
> >
> > Please tell me what is the preferred course of actions on this one,
> > from your point of view.
>
> As you stated in the other mail - you shall prepare some fix for
> current code base (without the _a suffix) for this release. Please
> proceed.
>

Already did, in this patch:

    [PATCH v3] fastboot: Remove "slot-suffixes" variable

Hope it's what we have agreed on (it's not for v2019.07, but for next
release, should be applied when merge window opens). If no, please
correct me. There was a lot of confusion going on w.r.t. this patch
series :)

> Regarding the supported features - maybe we shall introduce new env
> variable - like fastboot_version = "0.5" (then - 0.6, 0.7).
>
> And in U-Boot just decide which set of features would be 0.5 or 0.6.
>
> Then, we can also use Kconfig option to enable needed features.
>
> Otherwise, we will have regressions all the time as the "fastboot"
> protocol changes without any notice in the versioning (inside the
> protocol).
>

Agreed. As A/B is not implemented in U-Boot right now, and we don't
have any users for A/B slotted partitions, I presume this is not going
to be a regression, so we can skip this step right now. But on next
major change in fastboot protocol, this should be implemented.

> >
> > Thanks!
> >
> > [1]
> > http://git.denx.de/?p=u-boot.git;a=blob;f=drivers/fastboot/fb_getvar.c;h=4268628f5ef0210507c5d23f2e4209b2afc07029;hb=refs/heads/master#l84
> > [2]
> > https://android.googlesource.com/platform/system/core/+/9bfecb0e3444306ec57d7fafe4a99a47d3848c17%5E%21/#F0
> >
> > On Sat, Jun 15, 2019 at 6:29 PM Lukasz Majewski <lukma@denx.de> wrote:
> > >
> > > Hi Sam,
> > >
> > > > Fastboot tool recently underwent changes w.r.t. A/B slot format:
> > > >   1. In commit [1] the new slot format was introduced, according
> > > > to new A/B specification [2]. New slot format is "a", and old
> > > > format "_a" is now considered legacy.
> > > >   2. In commit [3] the legacy format "_a" was fixed and fastboot
> > > > tool should support both "a" and "_a" slot formats now.
> > > >   3. Finally, commit [4] drops the legacy slot format ("_a")
> > > > completely. That makes the latest fastboot tool incompatible with
> > > > "_a" format.
> > > >
> > > > Last change leads to next error, when running "fastboot flash"
> > > > with current U-Boot:
> > > >
> > > >     $ fastboot flash boot boot.img
> > > >     Sending 'boot__a' (11301 KB)    OKAY [  0.451s]
> > > >     Writing 'boot__a'               FAILED (remote: 'cannot find
> > > > partition') fastboot: error: Command failed
> > > >
> > > > To overcome this issue we should report slot names in "a" format
> > > > instead of "_a". Of course, this change breaks U-Boot
> > > > compatibility with older fastboot tools, but that shouldn't be a
> > > > problem as A/B is not implemented in U-Boot yet, and there are
> > > > not users for slotted partitions out there anyway. This fact can
> > > > be checked like this:
> > > >
> > > >     $ grep -Ir \b'boot_a\b' *
> > > >
> > > > Let's use new slot format in order to fix "fastboot flash" with
> > > > slotted partitions and to be in sync with AOSP master.
> > > >
> > > > With this patch, U-Boot depends on most recent fastboot tool
> > > > (patch [1] or later), for working with slotted partitions.
> > >
> > > Is there any "version" number scheme for the fastboot protocol
> > > specification?
> > >
> > > I do remember that in the past there were some mismatches for some
> > > "fastboot" specification depending on vendor. Now it turns out that
> > > there is a problem between AOSP versions ...
> > >
> > > (My point is that for example even lthor had version number for
> > > specification update. Is something similar for fastboot?).
> > >
> > > >
> > > > [1]
> > > > https://android.googlesource.com/platform/system/core/+/8091947847d5e5130b09d2ac0a4bdc900f3b77c5
> > > > [2]
> > > > https://source.android.com/devices/tech/ota/ab/ab_implement#partitions
> > > > [3]
> > > > https://android.googlesource.com/platform/system/core/+/04396f62da6150b94e02d50e5302fd980048833d
> > > > [4]
> > > > https://android.googlesource.com/platform/system/core/+/42b18a518bac85c3eea14206f6cbafbd1e98a31f
> > > >
> > > > Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> > > > ---
> > > > Changes in v2:
> > > >   - don't change slot format in "slot-suffixes" variable (it's now
> > > >     dropped in [PATCH 1/2])
> > > >   - improve commit message
> > > >
> > > >  drivers/fastboot/fb_getvar.c | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/fastboot/fb_getvar.c
> > > > b/drivers/fastboot/fb_getvar.c index f89c7f62e6..9ee5054485 100644
> > > > --- a/drivers/fastboot/fb_getvar.c
> > > > +++ b/drivers/fastboot/fb_getvar.c
> > > > @@ -174,8 +174,8 @@ static void getvar_platform(char
> > > > *var_parameter, char *response)
> > > >  static void getvar_current_slot(char *var_parameter, char
> > > > *response) {
> > > > -     /* A/B not implemented, for now always return _a */
> > > > -     fastboot_okay("_a", response);
> > > > +     /* A/B not implemented, for now always return "a" */
> > > > +     fastboot_okay("a", response);
> > > >  }
> > > >
> > > >  #if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
> > >
> > >
> > >
> > >
> > > 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
>
>
>
>
> 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

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

end of thread, other threads:[~2019-06-19 12:21 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-14 15:50 [U-Boot] [PATCH v2 0/2] fastboot: Support new A/B slot format Sam Protsenko
2019-06-14 15:50 ` [U-Boot] [PATCH v2 1/2] fastboot: Remove "slot-suffixes" variable Sam Protsenko
2019-06-14 15:50 ` [U-Boot] [PATCH v2 2/2] fastboot: Fix slot names reported by getvar Sam Protsenko
2019-06-15 15:28   ` Lukasz Majewski
2019-06-18 14:34     ` Sam Protsenko
2019-06-18 21:10       ` Lukasz Majewski
2019-06-19 12:21         ` Sam Protsenko

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.