[PATCH v2 01/16] board: Define GUIDs for firmware images

[Ilias Apalodimas]

Hi both

On Wed, 10 Apr 2024 at 10:36, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 09.04.24 23:05, Jon Humphreys wrote:
> > Heinrich Schuchardt <xypron.glpk at gmx.de> writes:
> >
> >> On 4/9/24 00:31, Jonathan Humphreys wrote:
> >>> Define GUIDs for the different firmware images (tiboot3.bin, tispl.bin,
> >>> u-boot.img, sysfw). >
> >>> Signed-off-by: Jonathan Humphreys <j-humphreys at ti.com>
> >>> ---
> >>>    include/configs/ti_armv7_common.h | 17 +++++++++++++++++
> >>>    1 file changed, 17 insertions(+)
> >>>
> >>> diff --git a/include/configs/ti_armv7_common.h b/include/configs/ti_armv7_common.h
> >>> index 3def7b1027e..4ce14a9b84c 100644
> >>> --- a/include/configs/ti_armv7_common.h
> >>> +++ b/include/configs/ti_armv7_common.h
> >>> @@ -16,6 +16,23 @@
> >>>    #ifndef __CONFIG_TI_ARMV7_COMMON_H__
> >>>    #define __CONFIG_TI_ARMV7_COMMON_H__
> >>>
> >>> +/* GUIDs for capsule updatable firmware images */
> >>
> >> Please, provide code comments for the GUIDs, e.g.
> >>
> >> /**
> >>    * define K3_TIBOOT3_IMAGE_GUID - firmware GUID for K3 tiboot3.bin
> >>    *
> >>    * This GUID is used in capsules updates to identify the tiboot3.bin
> >>    * binary.
> >>    */

I'd go one step further tbh.  Those GUIDs can be used by OEMs/ODMs
producing capsules for their future products.
Currently, we don't have a documented way to allow users to redefine
this (but Linaro is working on it).  If both TI and an OEM use the
same GUID and try to upload the firmware to LVFS, we will have a GUID
clash.
I think we should have that on the comment as well, until we can
properly sort it out

Thanks
/Ilias
> >>
> >> Cf.
> >> https://docs.kernel.org/doc-guide/kernel-doc.html#object-like-macro-documentation
> >>
> >> Best regards
> >>
> >> Heinrich
> >>
> >
> > Heinrich, thanks for reviewing!
> >
> > I modelled the GUID macros after how other boards and even core code
> > defined there's.  (eg, include/configs/kontron-sl-mx8mm.h or
> > include/efi_api.h).
> >
> > However, if this is the new direction, I will format as you suggest.
> > Please confirm.
>
> Hello Jon,
>
> Without properly documenting macros we make the live of developers more
> difficult. Yes, we still have a lot of missing code documentation. But
> we should not follow poor example.
>
> Best regards
>
> Heinrich
>
> >
> > thanks
> > Jon
> >
> >>> +#define K3_TIBOOT3_IMAGE_GUID \
> >>> +   EFI_GUID(0xe672b518, 0x7cd7, 0x4014, 0xbd, 0x8d, \
> >>> +            0x40, 0x72, 0x4d, 0x0a, 0xd4, 0xdc)
> >>> +
> >>> +#define K3_SPL_IMAGE_GUID \
> >>> +   EFI_GUID(0x86f710ad, 0x10cf, 0x46ea, 0xac, 0x67, \
> >>> +            0x85, 0x6a, 0xe0, 0x6e, 0xfa, 0xd2)
> >>> +
> >>> +#define K3_UBOOT_IMAGE_GUID \
> >>> +   EFI_GUID(0x81b58fb0, 0x3b00, 0x4add, 0xa2, 0x0a, \
> >>> +            0xc1, 0x85, 0xbb, 0xac, 0xa1, 0xed)
> >>> +
> >>> +#define K3_SYSFW_IMAGE_GUID \
> >>> +   EFI_GUID(0x6fd10680, 0x361b, 0x431f, 0x80, 0xaa, \
> >>> +            0x89, 0x94, 0x55, 0x81, 0x9e, 0x11)
> >>> +
> >>>    /*
> >>>     * We setup defaults based on constraints from the Linux kernel, which should
> >>>     * also be safe elsewhere.  We have the default load at 32MB into DDR (for
>