[PATCH v3 01/19] binman: ti-board-config: Add support for TI board config binaries

[Simon Glass]

Hi Neha,

On Tue, 25 Apr 2023 at 00:24, Neha Malcom Francis <n-francis at ti.com> wrote:
>
> Hi Simon
>
> On 25/04/23 01:12, Simon Glass wrote:
> > Hi Neha,
> >
> > On Fri, 21 Apr 2023 at 06:32, Neha Malcom Francis <n-francis at ti.com> wrote:
> >>
> >> The ti-board-config entry loads and validates a given YAML config file
> >> against a given schema, and generates the board config binary. K3
> >> devices require these binaries to be packed into the final system
> >> firmware images.
> >>
> >> Signed-off-by: Neha Malcom Francis <n-francis at ti.com>
> >> ---
> >>   tools/binman/entries.rst                      |  48 ++++
> >>   tools/binman/etype/ti_board_config.py         | 269 ++++++++++++++++++
> >>   tools/binman/ftest.py                         |  32 +++
> >>   tools/binman/pyproject.toml                   |   2 +-
> >>   tools/binman/test/277_ti_board_cfg.dts        |  11 +
> >>   .../binman/test/278_ti_board_cfg_combined.dts |  25 ++
> >>   .../binman/test/279_ti_board_cfg_no_type.dts  |  11 +
> >>   .../binman/test/280_ti_board_cfg_no_file.dts  |  11 +
> >>   .../281_ti_board_cfg_combined_no_file.dts     |  13 +
> >>   tools/binman/test/yaml/config.yaml            |  19 ++
> >>   tools/binman/test/yaml/schema.yaml            |  51 ++++
> >>   tools/binman/test/yaml/schema_notype.yaml     |  40 +++
> >>   12 files changed, 531 insertions(+), 1 deletion(-)
> >>   create mode 100644 tools/binman/etype/ti_board_config.py
> >>   create mode 100644 tools/binman/test/277_ti_board_cfg.dts
> >>   create mode 100644 tools/binman/test/278_ti_board_cfg_combined.dts
> >>   create mode 100644 tools/binman/test/279_ti_board_cfg_no_type.dts
> >>   create mode 100644 tools/binman/test/280_ti_board_cfg_no_file.dts
> >>   create mode 100644 tools/binman/test/281_ti_board_cfg_combined_no_file.dts
> >>   create mode 100644 tools/binman/test/yaml/config.yaml
> >>   create mode 100644 tools/binman/test/yaml/schema.yaml
> >>   create mode 100644 tools/binman/test/yaml/schema_notype.yaml
> >>
> >
> > Reviewed-by: Simon Glass <sjg at chromium.org>
> >
> > My only real comment is that errors should produce an error rather
> > than just a warning. E.g. a schema-validation error should be fatal,
> > since it won't work.
> >
> > You can call self.Raise() when something goes wrong. The tests should
> > check for that instead of a warning.
> >
>
> Makes sense. But I'm not sure I understand when we create an fake binary
> and when we choose to throw errors? Either case we end up with
> non-working binary or no binary at all. I see both styles in existing
> etypes and I can't form a reasoning for when to do what.

OK I see.

If -M is not provided, we should produce an error
If -M is provided, entries with missing external blobs should set
'self.missing' to True, so binman knows and can report it.

When you output a warning, it really doesn't do anything. So far as
the user is concerned the thing succeeded. We need the exit code to
indicate the right thing here.

In other words, we have support in Binman for handling missing blobs
and we should use that.

Regards,
Simon