From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tom Rini Date: Wed, 8 May 2019 13:40:43 -0400 Subject: [U-Boot] [PATCH 1/2] include: android_bl_msg.h: Initial import In-Reply-To: References: <20190407220206.7655-1-erosca@de.adit-jv.com> <20190407220206.7655-2-erosca@de.adit-jv.com> Message-ID: <20190508174043.GP25571@bill-the-cat> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 wrote: > > > > Import the bootloader_message.h (former bootloader.h) from AOSP. > > > > Repository: https://android.googlesource.com/platform/bootable/recovery > > Commit: 8e0821e6907d6016c19565b83319aff90ad1a034 > > Author: Tao Bao > > 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 > > --- > > 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 > > +#include > > +#include > > +#else > > +#include > > +#include > > +#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: