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

From: Pandey, Radhey Shyam
Date: Thu Jun 22 2023 - 15:17:12 EST


> -----Original Message-----
> From: Rob Herring <robh@xxxxxxxxxx>
> Sent: Tuesday, June 20, 2023 10:34 PM
> 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] dt-bindings: remoteproc: add Tightly Coupled Memory
> (TCM) bindings
>
> On Fri, Jun 16, 2023 at 06:04:25PM +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.
> >
> > In split mode, TCM resources(reg, ranges and power-domain) are
> > documented in each R5 node and in lockstep mode TCM resources are
> > documented in any of the R5 node.
> >
> > It also extends the examples for TCM split and lockstep modes.
> >
> > Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xxxxxxx>
> > ---
> > 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 | 86 +++++++++++++++++--
> > 1 file changed, 81 insertions(+), 5 deletions(-)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/remoteproc/xlnx,zynqmp-
> r5fss.yaml
> > b/Documentation/devicetree/bindings/remoteproc/xlnx,zynqmp-
> r5fss.yaml
> > index 9f677367dd9f..0bc3a8bb8374 100644
> > ---
> > a/Documentation/devicetree/bindings/remoteproc/xlnx,zynqmp-
> r5fss.yaml
> > +++ b/Documentation/devicetree/bindings/remoteproc/xlnx,zynqmp-
> r5fss.y
> > +++ aml
> > @@ -20,6 +20,12 @@ properties:
> > compatible:
> > const: xlnx,zynqmp-r5fss
> >
> > + "#address-cells":
> > + const: 1
> > +
> > + "#size-cells":
> > + const: 1
>
> If these are translatable addresses, then you are missing 'ranges'.

Thanks, will add ranges property in v2.

+ 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 +43,7 @@ properties:
> > 2: single cpu mode
> >
> > patternProperties:
> > - "^r5f-[a-f0-9]+$":
> > + "^r5f(.*)+$":
>
> That's just '^r5f'. If adding a unit-address is what you want then make the
> regex define that: "^r5f(@[0-9a-f]+|-[a-f0-9]+)$"

Will use this regex in v2.

>
> > type: object
> > description: |
> > The RPU is located in the Low Power Domain of the Processor
> Subsystem.
> > @@ -54,8 +60,27 @@ patternProperties:
> > compatible:
> > const: xlnx,zynqmp-r5f
> >
> > + "#address-cells":
> > + const: 1
> > +
> > + "#size-cells":
> > + const: 1
>
> These (and ranges) apply to child nodes, but you don't have any at this level.

Will drop address-cells and size-cells from core(xlnx,zynqmp-r5f) node
and address it in v2.

>
> > +
> > + reg:
> > + items:
> > + - description: Address and Size of the ATCM internal memory region
> > + - description: Address and Size of the BTCM internal memory
> > + region
> > +
> > + reg-names:
> > + items:
> > + - const: atcm
> > + - const: btcm
> > +
> > + ranges: true
> > +
> > power-domains:
> > - maxItems: 1
> > + minItems: 1
> > + maxItems: 3
> >
> > mboxes:
> > minItems: 1
> > @@ -112,13 +137,64 @@ additionalProperties: false
> >
> > examples:
> > - |
> > + #include <dt-bindings/power/xlnx-zynqmp-power.h>
> > +
> > + //Split mode configuration
> > + remoteproc {
> > + compatible = "xlnx,zynqmp-r5fss";
> > + xlnx,cluster-mode = <0>;
> > +
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > + r5f_0: r5f@ffe00000 {
> > + compatible = "xlnx,zynqmp-r5f";
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > + reg = <0xffe00000 0x10000>, <0xffe20000 0x10000>;
> > + reg-names = "atcm", "btcm";
> > + ranges = <0x0 0xffe00000 0x10000>, <0x20000 0xffe20000
> 0x10000>;
> > + 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@ffe90000 {
> > + compatible = "xlnx,zynqmp-r5f";
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > + reg = <0xffe90000 0x10000>, <0xffeb0000 0x10000>;
> > + reg-names = "atcm", "btcm";
> > + ranges = <0x0 0xffe90000 0x10000>, <0x20000 0xffeb0000
> 0x10000>;
> > + 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 {
> > compatible = "xlnx,zynqmp-r5fss";
> > xlnx,cluster-mode = <1>;
> >
> > - r5f-0 {
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > + r5f_00: r5f@ffe00000 {
> > compatible = "xlnx,zynqmp-r5f";
> > - power-domains = <&zynqmp_firmware 0x7>;
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > + reg = <0xffe00000 0x20000>, <0xffe20000 0x20000>;
> > + reg-names = "atcm", "btcm";
> > + ranges = <0x0 0xffe00000 0x20000>, <0x20000 0xffe20000
> 0x20000>;
> > + 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";
> > @@ -126,7 +202,7 @@ examples:
> >
> > r5f-1 {
> > compatible = "xlnx,zynqmp-r5f";
> > - power-domains = <&zynqmp_firmware 0x8>;
> > + 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
> >