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

Wed Jun 19 12:21:39 UTC 2019

On Wed, Jun 19, 2019 at 12:10 AM Lukasz Majewski <lukma at denx.de> wrote:
>
> On Tue, 18 Jun 2019 17:34:54 +0300
> Sam Protsenko <semen.protsenko at 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 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
>
>
>
>
> 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