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