[PATCH v5 02/13] binman: qcom: Add type definitions for Qualcomm binaries

[Varadarajan Narayanan]

[ . . . ]

> > diff --git a/tools/binman/etype/qcom_appsbl.py b/tools/binman/etype/qcom_appsbl.py
> > @@ -0,0 +1,23 @@
> > +    Properties / Entry arguments:
> > +        - filename: Filename of U-Boot MBN (default 'u-boot.mbn')
> > +
> > +    This is the U-Boot MBN image.
> > +    """
> > +    def __init__(self, section, etype, node):
> > +        super().__init__(section, etype, node, 'qcom-appsbl')
>
> Not quite. blob_named_by_arg looks up an entry-arg called
> <blob_fname>-path, so the actual property is qcom-appsbl-path
> (matching what your doc patch passes with -a qcom-appsbl-path=...).
> There is no filename property and no default. Please update the
> docstring to match the other two new etypes.

Ok.

> > diff --git a/tools/binman/etype/qcom_appsbl.py b/tools/binman/etype/qcom_appsbl.py
> > @@ -0,0 +1,23 @@
> > +    def __init__(self, section, etype, node):
> > +        super().__init__(section, etype, node, 'qcom-appsbl')
>
> qcom_config and qcom_lib set self.external = True, but this one
> doesn't, even though u-boot.mbn is supplied via -a qcom-appsbl-path=.
> Should this also be external? Otherwise a missing blob produces a hard
> error rather than the usual missing-blob warning.

Ok.

> > diff --git a/tools/binman/etype/qcom_config.py b/tools/binman/etype/qcom_config.py
> > @@ -0,0 +1,26 @@
> > +from binman.entry import Entry
> > +from binman.etype.blob_named_by_arg import Entry_blob_named_by_arg
>
> The Entry import is unused - same in qcom_lib.py. Please drop it.

Ok.

> > diff --git a/tools/binman/etype/qcom_appsbl.py b/tools/binman/etype/qcom_appsbl.py
> > @@ -0,0 +1,23 @@
> > +# SPDX-License-Identifier: GPL-2.0
>
> The rest of tools/binman/etype/ uses GPL-2.0+. Please align (all three
> new files).

Our legal doesn't allow GPL-2.0+. If that is not acceptable, let me know
will talk to them and see what can be done.

> > diff --git a/tools/binman/etype/qcom_appsbl.py b/tools/binman/etype/qcom_appsbl.py
> > @@ -0,0 +1,23 @@
> > +    MBN is a Qualcomm‑specific signed firmware container that packages
> > +    executable images (typically ELF) along with metadata such as hash,
> > +    signature, and certificate chain, enabling the secure boot chain and Root
> > +    of Trust (RoT) to load and authenticate images.
>
> Just to clarify - the doc patch shows this entry is fed u-boot.mbn
> produced by running elftombn.py over u-boot.elf, but the binman node
> in the dtsi treats it as an ELF (fit,operation = 'split-elf', os =
> 'U-Boot'). Is the input actually a raw MBN container or the wrap-elf?
> The docstring should describe whichever binman consumes.

Input is the MBN container. wrap-elf is for the SPL. Will update the docstring.

>
> > diff --git a/tools/binman/etype/qcom_appsbl.py b/tools/binman/etype/qcom_appsbl.py
> > @@ -0,0 +1,23 @@
> > +class Entry_qcom_appsbl(Entry_blob_named_by_arg):
> > +    """U-Boot mbn image
>
> New binman etypes normally come with an ftest entry and a small test
> data file. Please add coverage for these three.

Ok.

Thanks
-Varada