Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 6 Jul 2023 13:02:04 -0600
From:   Rob Herring <robh@kernel.org>
To:     Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
Cc:     andersson@kernel.org, mathieu.poirier@linaro.org,
        krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
        michal.simek@amd.com, ben.levinsky@amd.com, tanmay.shah@amd.com,
        linux-remoteproc@vger.kernel.org, devicetree@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        git@amd.com
Subject: Re: [PATCH v2] dt-bindings: remoteproc: add Tightly Coupled Memory
 (TCM) bindings
Message-ID: <20230706190204.GA144696-robh@kernel.org>
References: <1687892226-3784452-1-git-send-email-radhey.shyam.pandey@amd.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1687892226-3784452-1-git-send-email-radhey.shyam.pandey@amd.com>
Precedence: bulk

On Wed, Jun 28, 2023 at 12:27:06AM +0530, Radhey Shyam Pandey wrote:
> Introduce bindings for TCM memory address space on AMD-xilinx Zynq
> UltraScale+ platform. As of now TCM addresses are hardcoded in xilinx
> remoteproc driver. This binding will help in defining TCM in device-tree
> and make it's access platform agnostic and data-driven from the driver.
> 
> Tightly-coupled memories(TCMs) are low-latency memory that provides
> predictable instruction execution and predictable data load/store
> timing. Each Cortex-R5F processor contains two 64-bit wide 64 KB memory
> banks on the ATCM and BTCM ports, for a total of 128 KB of memory.
> 
> The TCM resources(reg and power-domain) are documented in each R5 node.
> It also extends the examples for TCM split and lockstep modes.
> 
> Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
> ---
> Changes for v2:
> - Add ranges property to r5fss cluster node.
> - Use regex "^r5f(@[0-9a-f]+|-[a-f0-9]+)$".
> - Drop address/size-cells and ranges from r5f core node.
> - Mention "reg" and "reg names" as r5f core node required properties.
> - Mention address/size-cells and ranges as r5fss required node properties.
> - Modify commit description to remove ranges from R5 node.
> - Rename r5f node labels(r5f_0 -> r5f_0_split/lockstep and
>   r5f_1->r5f_1_split/lockstep)
> 
> The inspiration for integrating TCM nodes in R5 nodes is taken from
> "5ee79c2ed5bd dt-bindings: remoteproc: Add bindings for R5F subsystem
> on TI K3 SoCs".Once the binding is reviewed/accepted will send out
> driver changes in follow-up series.
> ---
>  .../remoteproc/xlnx,zynqmp-r5fss.yaml         | 90 +++++++++++++++++--
>  1 file changed, 83 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/xlnx,zynqmp-r5fss.yaml b/Documentation/devicetree/bindings/remoteproc/xlnx,zynqmp-r5fss.yaml
> index 9f677367dd9f..958044b08e86 100644
> --- a/Documentation/devicetree/bindings/remoteproc/xlnx,zynqmp-r5fss.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/xlnx,zynqmp-r5fss.yaml
> @@ -20,6 +20,17 @@ properties:
>    compatible:
>      const: xlnx,zynqmp-r5fss
>  
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 1
> +
> +  ranges:
> +    description: |
> +      Standard ranges definition providing address translations for
> +      local R5F TCM address spaces to bus addresses.
> +
>    xlnx,cluster-mode:
>      $ref: /schemas/types.yaml#/definitions/uint32
>      enum: [0, 1, 2]
> @@ -37,7 +48,7 @@ properties:
>        2: single cpu mode
>  
>  patternProperties:
> -  "^r5f-[a-f0-9]+$":
> +  "^r5f(@[0-9a-f]+|-[a-f0-9]+)$":

If reg is required, then you don't need '-[a-f0-9]+'. Though new 
required properties is an ABI change which needs justification.

>      type: object
>      description: |
>        The RPU is located in the Low Power Domain of the Processor Subsystem.
> @@ -54,8 +65,19 @@ patternProperties:
>        compatible:
>          const: xlnx,zynqmp-r5f
>  
> +      reg:
> +        items:
> +          - description: Address and Size of the ATCM internal memory region
> +          - description: Address and Size of the BTCM internal memory region

Drop 'Address and Size of '

> +
> +      reg-names:
> +        items:
> +          - const: atcm
> +          - const: btcm
> +
>        power-domains:
> -        maxItems: 1
> +        minItems: 1
> +        maxItems: 3
>  
>        mboxes:
>          minItems: 1
> @@ -102,31 +124,85 @@ patternProperties:
>      required:
>        - compatible
>        - power-domains
> +      - reg
> +      - reg-names
>  
>      unevaluatedProperties: false
>  
>  required:
>    - compatible
> +  - "#address-cells"
> +  - "#size-cells"
> +  - ranges
>  
>  additionalProperties: false
>  
>  examples:
>    - |
> -    remoteproc {
> +    #include <dt-bindings/power/xlnx-zynqmp-power.h>
> +
> +    //Split mode configuration
> +    remoteproc@ffe00000 {
> +        compatible = "xlnx,zynqmp-r5fss";
> +        xlnx,cluster-mode = <0>;
> +
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +        ranges = <0x0 0xffe00000 0x10000>, <0x20000 0xffe20000 0x10000>,
> +                 <0x0 0xffe90000 0x10000>, <0x20000 0xffeb0000 0x10000>;
> +
> +        r5f_0_split: r5f@ffe00000 {
> +            compatible = "xlnx,zynqmp-r5f";
> +            reg = <0xffe00000 0x10000>, <0xffe20000 0x10000>;
> +            reg-names = "atcm", "btcm";
> +            power-domains = <&zynqmp_firmware PD_RPU_0>,
> +                            <&zynqmp_firmware PD_R5_0_ATCM>,
> +                            <&zynqmp_firmware PD_R5_0_BTCM>;
> +            memory-region = <&rproc_0_fw_image>, <&rpu0vdev0buffer>, <&rpu0vdev0vring0>, <&rpu0vdev0vring1>;
> +            mboxes = <&ipi_mailbox_rpu0 0>, <&ipi_mailbox_rpu0 1>;
> +            mbox-names = "tx", "rx";
> +        };
> +
> +        r5f_1_split: r5f@ffe90000 {
> +            compatible = "xlnx,zynqmp-r5f";
> +            reg = <0xffe90000 0x10000>, <0xffeb0000 0x10000>;
> +            reg-names = "atcm", "btcm";
> +            power-domains = <&zynqmp_firmware PD_RPU_1>,
> +                            <&zynqmp_firmware PD_R5_1_ATCM>,
> +                            <&zynqmp_firmware PD_R5_1_BTCM>;
> +            memory-region = <&rproc_1_fw_image>, <&rpu1vdev0buffer>, <&rpu1vdev0vring0>, <&rpu1vdev0vring1>;
> +            mboxes = <&ipi_mailbox_rpu1 0>, <&ipi_mailbox_rpu1 1>;
> +            mbox-names = "tx", "rx";
> +        };
> +    };
> +
> +  - |
> +    //Lockstep configuration
> +    remoteproc@ffe00000 {
>          compatible = "xlnx,zynqmp-r5fss";
>          xlnx,cluster-mode = <1>;
>  
> -        r5f-0 {
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +        ranges = <0x0 0xffe00000 0x20000>, <0x20000 0xffe20000 0x20000>;
> +
> +        r5f_0_lockstep: r5f@ffe00000 {
>              compatible = "xlnx,zynqmp-r5f";
> -            power-domains = <&zynqmp_firmware 0x7>;
> +            reg = <0xffe00000 0x20000>, <0xffe20000 0x20000>;
> +            reg-names = "atcm", "btcm";
> +            power-domains = <&zynqmp_firmware PD_RPU_0>,
> +                            <&zynqmp_firmware PD_R5_0_ATCM>,
> +                            <&zynqmp_firmware PD_R5_0_BTCM>;
>              memory-region = <&rproc_0_fw_image>, <&rpu0vdev0buffer>, <&rpu0vdev0vring0>, <&rpu0vdev0vring1>;
>              mboxes = <&ipi_mailbox_rpu0 0>, <&ipi_mailbox_rpu0 1>;
>              mbox-names = "tx", "rx";
>          };
>  
> -        r5f-1 {
> +        r5f_1_lockstep: r5f@ffe90000 {
>              compatible = "xlnx,zynqmp-r5f";
> -            power-domains = <&zynqmp_firmware 0x8>;
> +            reg = <0xffe90000 0x10000>, <0xffeb0000 0x10000>;
> +            reg-names = "atcm", "btcm";
> +            power-domains = <&zynqmp_firmware PD_RPU_1>;
>              memory-region = <&rproc_1_fw_image>, <&rpu1vdev0buffer>, <&rpu1vdev0vring0>, <&rpu1vdev0vring1>;
>              mboxes = <&ipi_mailbox_rpu1 0>, <&ipi_mailbox_rpu1 1>;
>              mbox-names = "tx", "rx";
> -- 
> 2.25.1
>