[U-Boot] [PATCH 1/2] include: android_bl_msg.h: Initial import

[Tom Rini]

On Wed, May 08, 2019 at 05:38:13PM +0300, Sam Protsenko wrote:
> Hi Tom,
> 
> Have a generic architecture related question regarding this code
> (below, inline).
> 
> On Mon, Apr 8, 2019 at 1:02 AM Eugeniu Rosca <roscaeugeniu at gmail.com> wrote:
> >
> > Import the bootloader_message.h (former bootloader.h) from AOSP.
> >
> > Repository: https://android.googlesource.com/platform/bootable/recovery
> > Commit: 8e0821e6907d6016c19565b83319aff90ad1a034
> > Author: Tao Bao <tbao at google.com>
> > Date:   Wed Apr 3 23:48:21 2019 +0000
> >
> > 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]
> >
> > 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 [3]
> >  - the struct/macro names have been shaped by [3-4], where the two main
> >    criterias are:
> >    - Improve the syntax/readability in the global U-Boot namespace
> >    - 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)
> >
> > [1] https://source.android.com/devices/bootloader
> > [2] https://source.android.com/devices/tech/ota/ab/
> > [3] https://patchwork.ozlabs.org/patch/1003998/#2046141
> > [4] https://patchwork.ozlabs.org/patch/1003998/#2144955
> >
> > Signed-off-by: Eugeniu Rosca <erosca at de.adit-jv.com>
> > ---
> >  include/android_bl_msg.h | 273 +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 273 insertions(+)
> >  create mode 100644 include/android_bl_msg.h
> >
> > diff --git a/include/android_bl_msg.h b/include/android_bl_msg.h
> > new file mode 100644
> > index 000000000000..c48a1de2762b
> > --- /dev/null
> > +++ b/include/android_bl_msg.h
> > @@ -0,0 +1,273 @@
> > +// 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: see U-Boot commit importing/updating the file in-tree

Please include the branch / hash this matches in the commit message at
least too.

> > + *
> > + * Copyright (C) 2008 The Android Open Source Project
> > + *
> > + * Licensed under the Apache License, Version 2.0 (the "License");
> > + * you may not use this file except in compliance with the License.
> > + * You may obtain a copy of the License at
> > + *
> > + *      http://www.apache.org/licenses/LICENSE-2.0
> > + *
> > + * Unless required by applicable law or agreed to in writing, software
> > + * distributed under the License is distributed on an "AS IS" BASIS,
> > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> > + * See the License for the specific language governing permissions and
> > + * limitations under the License.
> > + */
> > +
> > +#ifndef _BOOTLOADER_MESSAGE_H
> > +#define _BOOTLOADER_MESSAGE_H
> > +
> > +#ifndef __UBOOT__
> > +#include <assert.h>
> > +#include <stddef.h>
> > +#include <stdint.h>
> > +#else
> > +#include <compiler.h>
> > +#include <linux/sizes.h>
> > +#endif
> > +
> > +#ifdef __UBOOT__
> > +/* U-Boot-specific types for improved syntax/readability */
> > +typedef struct bootloader_message      andr_bl_msg;
> > +typedef struct bootloader_message_ab   andr_bl_msg_ab;
> > +typedef struct bootloader_control      andr_bl_control;
> 
> In files like this, which we copy from another projects, should we:
>   a) keep file as close as possible to original, so that it's easy to
> keep it in sync with upstream
>   b) change structures names (like bootloader_message ->
> andr_bootloader_message, slot_metadata -> andr_slot_metadata, etc), so
> that we keep namespace clear?

First, to be clear, I don't see adding typedefs as making the namespace
clear.  I can see the argument for making the code easier to read, but
typedefs make the namespace less, not more, clear.

> Also, this file contains a lot of C++ related code, which is right now
> discarded by #ifdef __UBOOT__. And it also not in kernel style, so
> checkpatch is not happy. Should we keep it like this, or it's better
> to remove all not needed code to keep this file clear, and fix coding
> style?

But otherwise, yes, I agree with option a, we want this to match up as
much as possible with the external authoritative file to make future
syncs easy and clear about what changed.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190508/359ffe60/attachment.sig>