RE: [PATCH v2] dt-bindings: remoteproc: add Tightly Coupled Memory (TCM) bindings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: Rob Herring <robh@xxxxxxxxxx>
> Sent: Friday, July 7, 2023 12:32 AM
> To: Pandey, Radhey Shyam <radhey.shyam.pandey@xxxxxxx>
> Cc: andersson@xxxxxxxxxx; mathieu.poirier@xxxxxxxxxx;
> krzysztof.kozlowski+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx; Simek, Michal
> <michal.simek@xxxxxxx>; Levinsky, Ben <ben.levinsky@xxxxxxx>; Shah,
> Tanmay <tanmay.shah@xxxxxxx>; linux-remoteproc@xxxxxxxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; git (AMD-Xilinx) <git@xxxxxxx>
> Subject: Re: [PATCH v2] dt-bindings: remoteproc: add Tightly Coupled
> Memory (TCM) bindings
> 
> 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@xxxxxxx>
> > ---
> > 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.y
> > +++ aml
> > @@ -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.


Some background: remoteproc subsystem is currently supported on  zu+ 
platform and it's TCM addresses are hardcoded in xlnx_r5_remoteproc 
driver at path: drivers/remoteproc/xlnx_r5_remoteproc.c
With this binding change i.e making reg/reg-names required 
remoteproc driver will still ensure stable ABI and support zu+ old 
device trees (without reg/reg-names).

For newer platform (versal) we plan to make reg/reg-names as required
properties and prefer not to have new hardcoded addresses for them 
in xlnx_r5_remoteproc driver. Hope that is fine. I can add
this justification to commit description.

> 
> >      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 '

Sure, will fix it in v3.

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




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux