Thanks I think this looks basically good now... just think matching the new header_version variable as a minimum (rather than an exact match) will help make it easier to expand this script in the future.

View Change

9 comments:

• File util/qualcomm/createxbl.py:

  • Patch Set #17, Line 269: secure_type = image_header_secflag, header_version = header_version )

    nit: on a new line?

  • Patch Set #17, Line 673: if header_version == 5 or header_version == 6:

    nit: probably makes more sense to check < 4 (since presumably this will stay this way for future versions, unless it explicitly changes again?)

  • Patch Set #17, Line 687: if header_version == 5 or header_version == 6:

    same

• File util/qualcomm/mbn_tools.py:

  • Patch Set #17, Line 74: MBN_HEADER_VERSION_6 = 6 # Mbn header_version 6

    nit: not really sure these constants are that useful as opposed to just using the numbers

  • Patch Set #17, Line 578: if self.flash_parti_ver == MBN_HEADER_VERSION_6:

    nit: maybe > 5?

  • Patch Set #17, Line 593: if self.flash_parti_ver == MBN_HEADER_VERSION_6:

    same

  • Patch Set #17, Line 1009: if header_version == MBN_HEADER_VERSION_5:

    This should probably be

     if header_version > 4:
       boot_header.image_src = 0
       boot_header.image_dest_ptr = 0

     if header_version > 5:
       boot_header.metadata_size_qti = 0
       boot_header.metadata_size = 0

  • Patch Set #17, Line 1050: global MI_PROG_BOOT_DIGEST_SIZE

    Can we please change this from a global to a normal local variable (and no ALL_CAPS name)? It's only used inside this function.

  • Patch Set #17, Line 2134: if header_version == MBN_HEADER_VERSION_6:

    5?

To view, visit change 35506. To unsubscribe, or for help writing mail filters, visit settings.