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

Eugeniu Rosca erosca at de.adit-jv.com
Wed May 8 14:45:57 UTC 2019


Hi Sam,

On Wed, May 08, 2019 at 05:26:04PM +0300, Sam Protsenko wrote:
> Hi Eugeniu,
> 
> Please see my comments below. Overall, we will need to think it over
> and merge 3 patch series somehow (this one, the one for A/B support
> from Ruslan, and one internal patch series Ruslan sent for reboot
> reason support). I will provide more details a bit later, working on
> this right now.

Thanks for letting me know. I am willing to cooperate, if needed.
To my understanding, this patch is the only overlapping area, so I
think the idea is to agree on its contents/structure. Then, you could
include it in your series in the agreed shape. Once it is merged, I
will rebase my work on top of u-boot/master.

> On Mon, Apr 8, 2019 at 1:02 AM Eugeniu Rosca <roscaeugeniu at gmail.com> wrote:
[..]
> > +
> > +#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;
> 
> I would argue against creating user types for structures. It breaks
> next rule from Linux kernel coding style (which we use in U-Boot):
> 
>     "It’s a mistake to use typedef for structures and pointers."
> 
> Which more important, it breaks that rule outside of this file. Also,
> as you mentioned before, it's probably wise to keep this file as close
> as possible to original, as we will need to update it at some point.

I will wait for comments from Tom and will stick to whichever policy is
established when importing out-of-tree headers.

> 
> > +#endif
> > +
> > +// Spaces used by misc partition are as below:
> > +// 0   - 2K     For bootloader_message
> > +// 2K  - 16K    Used by Vendor's bootloader (the 2K - 4K range may be optionally used
> > +//              as bootloader_message_ab struct)
> > +// 16K - 64K    Used by uncrypt and recovery to store wipe_package for A/B devices
> > +// Note that these offsets are admitted by bootloader,recovery and uncrypt, so they
> > +// are not configurable without changing all of them.
> > +
> > +#ifndef __UBOOT__
> 
> This stuff is very nice. I guess we can upstream it further to AOSP,
> then this file will be almost the same in AOSP and in U-Boot.

Sounds good to me.

-- 
Best Regards,
Eugeniu.


More information about the U-Boot mailing list