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

[Neha Malcom Francis]

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.

> Regards,
> Simon

-- 
Thanking You
Neha Malcom Francis