Re: [PATCH v2 05/17] android: boot: kcomp: support andr_image_data

[Mattijs Korpershoek <mkorpershoek@baylibre.com>]

On Thu, Jan 26, 2023 at 17:54, Simon Glass <sjg@chromium.org> wrote:

> On Thu, 26 Jan 2023 at 09:05, Safae Ouajih <souajih@baylibre.com> wrote:
>>
>> andr_image_data structure is used as a global representation of
>> boot image header structure. This new structure is introduced to
>> support all boot header versions : v0,v1.v2.v3.v4 and will be used
>> to support v3 and v4 while maitaining support for v0,v1,v2. The need
>> of using andr_image_data comes from the change of header structure in
>> both version 3 and 4.
>>
>> android_image_get_kcomp() is reworked to support this new struct.
>
> nit: Your commit messages should be in imperative tense. See [1] for
> some thoughts on this.
>
>>
>> Signed-off-by: Safae Ouajih <souajih@baylibre.com>
>> ---
>>  boot/image-android.c    | 75 ++++++++++++++++++++++++++++++++++++++++-
>>  include/android_image.h | 27 +++++++++++++++
>>  include/image.h         |  2 ++
>>  3 files changed, 103 insertions(+), 1 deletion(-)
>>
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> But please comment struct andr_image_data.
>
> [..]

Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

>
>> diff --git a/include/android_image.h b/include/android_image.h
>> index 4fce363ff7..5b36f96d7b 100644
>> --- a/include/android_image.h
>> +++ b/include/android_image.h
>> @@ -317,4 +317,31 @@ struct andr_boot_img_hdr_v0 {
>>   *    contained outside boot and vendor boot partitions), otherwise
>>   *    jump to kernel_addr
>>   */
>> +
>> +/* Private struct */
>> +struct andr_image_data {
>> +       ulong kernel_ptr;
>> +       u32 kernel_size;
>> +       u32 ramdisk_size;
>> +       u32 boot_ramdisk_size;
>> +       ulong second_ptr;
>> +       u32 second_size;
>> +       ulong dtb_ptr;
>> +       u32 dtb_size;
>> +       ulong recovery_dtbo_ptr;
>> +       u32 recovery_dtbo_size;
>> +
>> +       const char *kcmdline;
>> +       const char *kcmdline_extra;
>> +       const char *image_name;
>> +
>> +       u32 kernel_addr;
>> +       ulong ramdisk_addr;
>> +       ulong ramdisk_ptr;
>> +       ulong dtb_load_addr;
>> +       ulong tags_addr;
>> +       u32 header_version;
>> +       u32 boot_img_total_size;
>> +};
>> +
>>  #endif
>> diff --git a/include/image.h b/include/image.h
>> index c1594ee169..9a0bd9d8f2 100644
>> --- a/include/image.h
>> +++ b/include/image.h
>> @@ -1733,7 +1733,9 @@ struct cipher_algo {
>>  int fit_image_cipher_get_algo(const void *fit, int noffset, char **algo);
>>
>>  struct cipher_algo *image_get_cipher_algo(const char *full_name);
>> +struct andr_image_data;
>>
>> +bool android_image_get_data(const void *boot_hdr, struct andr_image_data *data);
>>  struct andr_boot_img_hdr_v0;
>>  int android_image_get_kernel(const struct andr_boot_img_hdr_v0 *hdr, int verify,
>>                              ulong *os_data, ulong *os_len);
>> --
>> 2.34.1
>>
>
> [1] https://chromium.googlesource.com/chromiumos/docs/+/HEAD/contributing.md#commit-messages