Re: [PATCH 2/3] dt-bindings: remoteproc: add Versal-NET platform

Krzysztof Kozlowski
2024-03-17T18:53:39Z

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@amd.com>
> ---
> .../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.

> + 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.

Best regards,
Krzysztof