[U-Boot] [PATCH v2 2/2] fastboot: Fix slot names reported by getvar

[Sam Protsenko]

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 at 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 at 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