[U-Boot] [PATCH v2] include: android_bl_msg.h: Initial import

Sam Protsenko semen.protsenko at linaro.org
Fri May 10 21:54:24 UTC 2019


Hi Eugeniu,

On Fri, May 10, 2019 at 11:41 PM Eugeniu Rosca <roscaeugeniu at gmail.com> wrote:
>
> Hi Sam,
>
> On Fri, May 10, 2019 at 04:12:10PM +0300, Sam Protsenko wrote:
> > From: Eugeniu Rosca <roscaeugeniu at gmail.com>
> >
> > Import the bootloader_message.h (former bootloader.h) from AOSP.
> >
> > The bootloader_message.h basically defines the flash layout of a
> > dedicated partition (usually called 'misc') and is needed in U-Boot
> > in order to be able to implement a subset of Android Bootloader
> > Requirements [1], specifically dealing with:
> >  - Communication between the bootloader and recovery
> >  - Handling of A/B (Seamless) System Updates [2]
> >  - Passing the reboot reason [3]
>
> Well, getting the reboot reason is a subset of
> "Communication between the bootloader and recovery" or
> "Communication between the bootloader and Android userspace" in general,
> but if you want to stress it via a dedicated bullet, fine with me.
>
> >
> > With respect to the in-tree vs out-of-tree file differences:
> >  - license matches https://patchwork.ozlabs.org/patch/1003998/
> >  - filename is changed to android_bl_msg.h, as per Simon's comment [4]
> >  - minimize the future integration/update efforts from the source.
> >    Particularly, the __UBOOT__ macro helps with isolating the
> >    U-Boot-unrelated parts (e.g. includes/function prototypes/etc)
>
> There is a dissonance added by this last newly added item, since it
> takes an imperative form compared to the previous two. I close my eyes
> on that.
>
> [..]
>
> > diff --git a/include/android_bl_msg.h b/include/android_bl_msg.h
> > new file mode 100644
> > index 0000000000..7bb69ef431
> > --- /dev/null
> > +++ b/include/android_bl_msg.h
> > @@ -0,0 +1,264 @@
> > +// SPDX-License-Identifier: BSD-2-Clause
> > +/*
> > + * This file was taken from the AOSP Project.
> > + * Repository: https://android.googlesource.com/platform/bootable/recovery/
> > + * File: bootloader_message/include/bootloader_message/bootloader_message.h
> > + * Commit: 9423d2f6b7ef ("Merge "Track libziparchive API change."")
>
> Here is miscommunication, again. Tom is saying in
> https://patchwork.ozlabs.org/patch/1080394/#2168595:
>  ----
>  Please include the branch / hash this matches in the commit message at
> least too.
>  ---
>
> which means he agrees with the v1 of this line, i.e.:
>  ---
>  Commit: see U-Boot commit importing/updating the file in-tree
>  ---
>
> and suggests documenting the current AOSP version in the description
> of U-Boot commit importing/updating the file. The problem with
> documenting the AOSP commit in the U-Boot file is that people will
> forget updating the commit id on next AOSP backports, which will create
> stale references. This actually happened between two consecutive
> iterations of A/B series from Igor.
>
> > + *
> > + * Please keep this file with minimal changes with respect to AOSP version!
>
> Agree with this new line. No other comments.
>

Agreed with all you comments. Please send the next version along with
BCB patch as you mentioned. I just wanted to accelerate things a bit,
hence submitting v2.

> Since the next A/B patches will apparently depend on the 'bcb' command,
> I can resolve the above minor nit-picks in my next iteration of the
> 'bcb' command, freeing you from any more work for this patch.
>

Thanks, Eugeniu. Please make it so. Basically, for this patch the main
change is typedefs removal. Once you send v2 for BCB, I can try and
implement reboot reason on TI X15 board by using it, and Igor will be
unblocked w.r.t. A/B patches.

> --
> Best regards,
> Eugeniu.


More information about the U-Boot mailing list