Hello Krzysztof,
Thanks for reviews. Please find my comments below.
On 3/17/24 1:53 PM, Krzysztof Kozlowski wrote:
> On 15/03/2024 22:15, Tanmay Shah wrote:
>> AMD-Xilinx Versal-NET platform is successor of Versal platform. It
>> contains multiple clusters of cortex-R52 real-time processing units.
>> Each cluster contains two cores of cortex-R52 processors. Each cluster
>> can be configured in lockstep mode or split mode.
>>
>> Each R52 core is assigned 128KB of TCM memory. ATCM memory is 64KB, BTCM
>> and CTCM memoreis are 32KB each. Each TCM memory has its own dedicated
>> power-domain that needs to be requested before using it.
>>
>> Signed-off-by: Tanmay Shah <tanmay.shah@xxxxxxx>
>> ---
>> .../remoteproc/xlnx,zynqmp-r5fss.yaml | 220 +++++++++++++++---
>> 1 file changed, 184 insertions(+), 36 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/remoteproc/xlnx,zynqmp-r5fss.yaml b/Documentation/devicetree/bindings/remoteproc/xlnx,zynqmp-r5fss.yaml
>> index 711da0272250..55654ee02eef 100644
>> --- a/Documentation/devicetree/bindings/remoteproc/xlnx,zynqmp-r5fss.yaml
>> +++ b/Documentation/devicetree/bindings/remoteproc/xlnx,zynqmp-r5fss.yaml
>> @@ -18,7 +18,9 @@ description: |
>>
>> properties:
>> compatible:
>> - const: xlnx,zynqmp-r5fss
>> + enum:
>> + - xlnx,zynqmp-r5fss
>> + - xlnx,versal-net-r52fss
>>
>> "#address-cells":
>> const: 2
>> @@ -64,7 +66,9 @@ patternProperties:
>>
>> properties:
>> compatible:
>> - const: xlnx,zynqmp-r5f
>> + enum:
>> + - xlnx,zynqmp-r5f
>> + - xlnx,versal-net-r52f
>>
>> reg:
>> minItems: 1
>> @@ -135,9 +139,11 @@ required:
>> allOf:
>> - if:
>> properties:
>> - xlnx,cluster-mode:
>> - enum:
>> - - 1
>> + compatible:
>> + contains:
>> + enum:
>> + - xlnx,versal-net-r52fss
>
> Why do you touch these lines?
>
>> +
>> then:
>> patternProperties:
>> "^r5f@[0-9a-f]+$":
>> @@ -149,16 +155,14 @@ allOf:
>> items:
>> - description: ATCM internal memory
>> - description: BTCM internal memory
>> - - description: extra ATCM memory in lockstep mode
>> - - description: extra BTCM memory in lockstep mode
>> + - description: CTCM internal memory
>>
>> reg-names:
>> minItems: 1
>> items:
>> - - const: atcm0
>> - - const: btcm0
>> - - const: atcm1
>> - - const: btcm1
>> + - const: atcm
>> + - const: btcm
>> + - const: ctcm
>>
>> power-domains:
>> minItems: 2
>> @@ -166,33 +170,70 @@ allOf:
>> - description: RPU core power domain
>> - description: ATCM power domain
>> - description: BTCM power domain
>> - - description: second ATCM power domain
>> - - description: second BTCM power domain
>> + - description: CTCM power domain
>>
>> else:
>> - patternProperties:
>> - "^r5f@[0-9a-f]+$":
>> - type: object
>> -
>> - properties:
>> - reg:
>> - minItems: 1
>> - items:
>> - - description: ATCM internal memory
>> - - description: BTCM internal memory
>> -
>> - reg-names:
>> - minItems: 1
>> - items:
>> - - const: atcm0
>> - - const: btcm0
>> -
>> - power-domains:
>> - minItems: 2
>> - items:
>> - - description: RPU core power domain
>> - - description: ATCM power domain
>> - - description: BTCM power domain
>> + allOf:
>> + - if:
>> + properties:
>> + xlnx,cluster-mode:
>> + enum:
>> + - 1
>
> Whatever you did here, is not really readable. You have now multiple
> if:then:if:then embedded.
For ZynqMP platform, TCM can be configured differently in lockstep mode
and split mode.
For Versal-NET no such configuration is available, but new CTCM memory
is added.
So, I am trying to achieve following representation of TCM for both:
if: versal-net compatible
then:
ATCM - 64KB
BTCM - 32KB
CTCM - 32KB
else: (ZynqMP compatible)
if:
xlnx,cluster-mode (lockstep mode)
then:
ATCM0 - 64KB
BTCM0 - 64KB
ATCM1 - 64KB
BTCM1 - 64KB
else: (split mode)
ATCM0 - 64KB
BTCM0 - 64KB
If bindings are getting complicated, does it make sense to introduce
new file for Versal-NET bindings? Let me know how you would like me
to proceed.
Thanks,
Tanmay
>
>> + then:
>> + patternProperties:
>> + "^r5f@[0-9a-f]+$":
>> + type: object
>> +
>> + properties:
>> + reg:
>> + minItems: 1
>> + items:
>> + - description: ATCM internal memory
>> + - description: BTCM internal memory
>> + - description: extra ATCM memory in lockstep mode
>> + - description: extra BTCM memory in lockstep mode
>> +
>> + reg-names:
>> + minItems: 1
>> + items:
>> + - const: atcm0
>> + - const: btcm0
>> + - const: atcm1
>> + - const: btcm1
>> +
>> + power-domains:
>> + minItems: 2
>> + items:
>> + - description: RPU core power domain
>> + - description: ATCM power domain
>> + - description: BTCM power domain
>> + - description: second ATCM power domain
>> + - description: second BTCM power domain
>> +
>> + else:
>> + patternProperties:
>> + "^r5f@[0-9a-f]+$":
>> + type: object
>> +
>> + properties:
>> + reg:
>> + minItems: 1
>> + items:
>> + - description: ATCM internal memory
>> + - description: BTCM internal memory
>> +
>> + reg-names:
>> + minItems: 1
>> + items:
>> + - const: atcm0
>> + - const: btcm0
>> +
>> + power-domains:
>> + minItems: 2
>> + items:
>> + - description: RPU core power domain
>> + - description: ATCM power domain
>> + - description: BTCM power domain
>>
>> additionalProperties: false
>>
>> @@ -386,4 +427,111 @@ examples:
>> };
>> };
>> };
>> +
>> + - |
>> + // Versal-NET split configuration
>> + soc {
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> +
>
> Don't add examples per each platform.
Noted. If we go with different file for Versal-NET, then example should
be included?
>
> Best regards,
> Krzysztof
>
