Hello Igor, Thanks for the series. Some questions below.
First, my understanding is that the patches replace the deprecated libavb_ab and make it trully obsolete, i.e. there should be no need to import libavb_ab into U-Boot (unlike some of our suppliers still do). Can you please confirm? On Mon, Feb 18, 2019 at 5:25 PM Igor Opaniuk <igor.opan...@linaro.org> wrote: > [..] > diff --git a/include/android_bl_msg.h b/include/android_bl_msg.h > new file mode 100644 > index 0000000..f37e01a > --- /dev/null > +++ b/include/android_bl_msg.h > @@ -0,0 +1,169 @@ > +/* 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 I won't object on it and it's not my purpose to do any teaching/mentoring, but I think it makes sense to decouple (i.e. allocate standalone commits for) these two activities: - integration of external headers/libraries (e.g. libavb, dtc, headers imported from linux/avb/recovery/etc trees) - in-tree U-Boot development around those headers/libraries I think mixing these two activities creates more overhead for the reviewers, so it's easy for various mistakes to slip in unnoticed. See next comment as example. > + * Commit: 8b309f6970ab3b7c53cc529c51a2cb44e1c7a7e1 The contents of out-of-tree "bootloader_message.h" at this commit doesn't appear to contain any "Omaha" references: https://android.googlesource.com/platform/bootable/recovery.git/+/8b309f6970ab3b/bootloader_message/include/bootloader_message/bootloader_message.h The addition of "Omaha" comment is done in commit: $ git log --oneline -G Omaha 8b309f6970ab..recovery/master -- bootloader_message/include/bootloader_message/bootloader_message.h 7191bf049216 Add update_channel field to bootloader_message_ab. So, I believe there is a mismatch between the contents of the newly created file and its documented version. [..] > + u8 priority : 4; > + /* Number of times left attempting to boot this slot */ > + u8 tries_remaining : 3; > + /* 1 if this slot has booted successfully, 0 otherwise */ > + u8 successful_boot : 1; The s/uint8_t/u8/ and s/uint32_t/u32/ conversion creates noise comparing the in-tree versus out-of-tree files and will add some overhead during integration. I still see a lot of U-Boot code saying uint8_t/uint32_t, so I wonder if these can be preserved. BTW, some of the patches from this series add code using uint32_t. Looking forward to pick the patches from mainline! Thanks and best regards, Eugeniu. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot