[PATCH v3 1/2] binman: openssl: x509: ti_secure_rom: Add support for bootcore_opts

[Neha Malcom Francis]

Hi Simon

On 18/10/23 09:03, Simon Glass wrote:
> Hi Neha,
> 
> On Tue, 17 Oct 2023 at 03:58, Neha Malcom Francis <n-francis at ti.com> wrote:
>>
>> Hi Simon
>>
>> On 08/10/23 04:39, Simon Glass wrote:
>>> Hi Neha,
>>>
>>> On Fri, 6 Oct 2023 at 04:07, Neha Malcom Francis <n-francis at ti.com> wrote:
>>>>
>>>> According to the TRMs of K3 platform of devices, the ROM boot image
>>>> format specifies a "Core Options Field" that provides the capability to
>>>> set the boot core in lockstep when set to 0 or to split mode when set
>>>> to 2. Add support for providing the same from the binman DTS. Also
>>>> modify existing test case for ensuring future coverage.
>>>>
>>>> Signed-off-by: Neha Malcom Francis <n-francis at ti.com>
>>>> ---
>>>> Link to J721E TRM: https://www.ti.com/lit/zip/spruil1
>>>> Section 4.5.4.1 Boot Info
>>>>
>>>> Changes in v3:
>>>>           - updated function comments
>>>>           - removed inconsistency in setting bootcore_opts to 32
>>>>
>>>> Changes in v2:
>>>>           - included TRM link in commit message
>>>>
>>>>    tools/binman/btool/openssl.py           |  6 ++++--
>>>>    tools/binman/entries.rst                |  1 +
>>>>    tools/binman/etype/ti_secure_rom.py     | 11 +++++++++--
>>>>    tools/binman/etype/x509_cert.py         |  3 ++-
>>>>    tools/binman/test/297_ti_secure_rom.dts |  1 +
>>>>    5 files changed, 17 insertions(+), 5 deletions(-)
>>>>
>>
>> [...]
>>
>>>> diff --git a/tools/binman/test/297_ti_secure_rom.dts b/tools/binman/test/297_ti_secure_rom.dts
>>>> index d1313769f4..1a3eca9425 100644
>>>> --- a/tools/binman/test/297_ti_secure_rom.dts
>>>> +++ b/tools/binman/test/297_ti_secure_rom.dts
>>>> @@ -9,6 +9,7 @@
>>>>           binman {
>>>>                   ti-secure-rom {
>>>>                           content = <&unsecure_binary>;
>>>> +                       core-opts = <2>;
>>>
>>> here ^
>>>
>>> Do you think there could be a binding file in dt-bindings/ for this value?
>>>
>>
>> Sorry for getting back to this patch so late, but wanted to ask about this. I
>> had seen a last version of getting the binman compatible in dt-bindings but I
>> don't see it merged. Not sure where I would add this property in.
> 
> Yes it is difficult to do anything genuinely new.
> 
> I am anticipating something like compatible = "ti,secure-rom"
> 
> so I suppose you could create a header file with suitable values for
> this property.
> 
> On the other hand, you could wait until there is progress with the bindings.
> 
> 

I can try adding a header file for now, but follow up question; shouldn't it be 
added to arch/arm/dts rather than include/dt-bindings? There was traction by DT 
maintainers to move all such constants not directly used by the driver (in this 
case binman) to arch/arm/dts? [1] If you mean to use these constants in binman, 
will need to see how we can do that.

>>
>>>>                   };
>>>>                   unsecure_binary: blob-ext {
>>>>                           filename = "ti_unsecure.bin";
>>>> --
>>>> 2.34.1
>>>>
> Regards,
> Simon

[1] https://lore.kernel.org/all/c4d53e9c-dac0-8ccc-dc86-faada324beba@linaro.org/

-- 
Thanking You
Neha Malcom Francis