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.opaniuk@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.
- 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/+/8b309f6970...
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.