[PATCH v4 1/4] binman: Add nxp_imx8mcst etype for i.MX8M flash.bin signing

[Marek Vasut]

On 6/27/24 10:19 AM, Simon Glass wrote:
> Hi Marek,

Hi,

>>>>>> Add new binman etype which allows signing both the SPL and fitImage sections
>>>>>> of i.MX8M flash.bin using CST. There are multiple DT properties which govern
>>>>>> the signing process, nxp,loader-address is the only mandatory one which sets
>>>>>> the SPL signature start address without the imx8mimage header, this should be
>>>>>> SPL text base. The key material can be configured using optional DT properties
>>>>>> nxp,srk-table, nxp,csf-crt, nxp,img-crt, all of which default the key material
>>>>>> names generated by CST tool scripts. The nxp,unlock property can be used to
>>>>>> unlock CAAM access in SPL section.
>>>>>>
>>>>>> Reviewed-by: Tim Harvey <tharvey at gateworks.com>
>>>>>> Signed-off-by: Marek Vasut <marex at denx.de>
>>>>>
>>>>> Applied the series, thanks.
>>>>
>>>> This lacks tests - can you please add sufficient tests in ftest.py to
>>>> get the cover coverage back to 100%? Please try 'binman test -T' to
>>>> see this.
>>>
>>> Any thoughts on this, please? At present -master is broken for one
>>> file and -next has three problems.
>>
>> It is in the pipeline.
>>
>> What exactly is the error you observe ?
>>
>> When I run binman test -T , I get a lot of output, but no error reports?
> 
> Sorry I somehow missed this email.
> 
> The tests are in ftest.py - there are lots of examples, e.g.
> testXilinxBootgenSigning() - commit d8a2d3b29

This seems to be testing some out-of-tree tool , not binman ?

> Basically you need to create a test .dts file that uses your entry
> type, then use it in the test code. You can check error handling as
> well, e.g. by having an invalid dts too if needed.
> 
> If you run 'binman test -T' you will see the code-coverage problem.
> You may need to fetch tools with 'binman tool -f missing' to get all
> the tools*:
> 
> Coverage error: 96%, but should be 100%
> ValueError: Test coverage failure
> 
> It is normally much easier to add an etype using a test than to test
> it by using it 'for real', since you don't need to worry about the
> U-Boot integration.

Huh ...