Re: [PATCH] dt-bindings: sram: Tightly Coupled Memory (TCM) bindings
From: Tanmay Shah
Date: Mon Jan 16 2023 - 13:32:03 EST
Hi Kryzsztop Thanks for reviews. Please find my comments below.
On 1/15/23 6:45 AM, Krzysztof Kozlowski wrote:
On 13/01/2023 19:04, Tanmay Shah wrote:
Hi Krzysztof Thanks for your reviews.
Please find my comments below.
On 1/12/23 11:52 PM, Krzysztof Kozlowski wrote:
On 13/01/2023 08:30, Tanmay Shah wrote:
This patch introduces bindings for TCM memory address space on AMD-xilinx
platforms. As of now TCM addresses are hardcoded in xilinx remoteproc
driver. This bindings will help in defining TCM in device-tree and
make it's access platform agnostic and data-driven from the driver.
Signed-off-by: Tanmay Shah <tanmay.shah@xxxxxxx>
---
.../devicetree/bindings/sram/xlnx,tcm.yaml | 137 ++++++++++++++++++
1 file changed, 137 insertions(+)
create mode 100644 Documentation/devicetree/bindings/sram/xlnx,tcm.yaml
diff --git a/Documentation/devicetree/bindings/sram/xlnx,tcm.yaml b/Documentation/devicetree/bindings/sram/xlnx,tcm.yaml
new file mode 100644
index 000000000000..02d17026fb1f
--- /dev/null
+++ b/Documentation/devicetree/bindings/sram/xlnx,tcm.yaml
@@ -0,0 +1,137 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sram/xlnx,tcm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Tightly Coupled Memory (TCM)
+
+maintainers:
+ - Tanmay Shah <tanmay.shah@xxxxxxx>
+
+description: |
+ Tightly Coupled Memory(TCM) is available on AMD-Xilinx paltforms for ARM
+ cortex remote processors to use. It is low-latency memory that provide
+ predictable instruction execution and predictable data load/store timing.
+ TCM can be configured in lockstep mode or split mode. In split mode
+ configuration each RPU core has its own set of ATCM and BTCM memories and in
+ lockstep mode redundant processor's TCM become available to lockstep
+ processor. So In lockstep mode ATCM and BTCM size is increased.
+
+properties:
+ $nodename:
+ pattern: "sram-[0-9a-f]+$"
Drop node name requirement.
Why do you need sram node at all?
I will remove sram- node. However, it device-tree I was planning to put
all TCM nodes under single node for example:
tcm {
tcm-lockstep {
};
tcm-core@0 {
Mix of nodes with and without unit address is pointing to some design
issues.
The design currently tries to accommodate physical relocation of the
memory. May be there is another way to represent this.
Here is address space of TCM memory on zynqmp platform:
https://docs.xilinx.com/r/en-US/ug1085-zynq-ultrascale-trm/Tightly-Coupled-Memory-Address-Map
As per above address map, there are 4 TCM banks, each 64KB ( 0x10000 )
size at following addresses:
*In split mode*:
ATCM0: 0xFFE00000---|---> Assigned to RPU core0
BTCM0: 0xFFE20000---|
ATCM1: 0xFFE90000---|---> Assigned to RPU 1
BTCM1: 0xFFEB0000---|
However, In lockstep mode, ATCM1 and BTCM1 relocates to different
addresses (i.e. 0xFFE10000 and 0xFFE30000 respectively)
and becomes available for RPU core 0:
*In lockstep mode Both used by RPU core 0*:
ATCM0: 0xFFE00000-----|
|----> ATCM: 0xFFE00000 size:
128KB
ATCM1: 0xFFE10000-----|
BTCM0: 0xFFE20000-----|
|----> BTCM: 0xFFE20000 size:
128KB
BTCM1: 0xFFE30000-----|
I am not sure how to represent this physical relocation of addresses in
device-tree.
Ideally such sram nodes can be represented as following:
[1] Representation of TCM in split mode:
[ a|b ]tcm[ 0|1 ] {
compatible = "xlnx,zynqmp-tcm";
reg <>;
ranges <>;
power-domain: (only 1 power domain for current bank)
}
However, to represent TCM in lockstep mode as well, I might have to add
platform specific optional reg and ranges property which optionally
represent address space of lockstep mode for atcm and btcm.
For example, ATCM0 and BTCM0 will be represented as above [1] However,
ATCM1 and BTCM1 will have following extra properties:
[a|b]tcm1 {
compatible = "xlnx,zynqmp-tcm";
reg <>;
lockstep-reg <>; /* represent address space of this bank in
lockstep mode */
ranges <>;
lockstep-ranges <>; /* represent address space ranges of this bank
in lockstep mode */
power-domain: (only 1 power domain for current bank)
};
Does above approach looks good? If some other standard way is already
available to represent this could you please suggest?
Thanks,
Tanmay
};
};
The top-most tcm node I assumed sram node. So I kept sram@xxxx
+
+patternProperties:
+ "^tcm-[a-z]+@[0-9a-f]+$":
+ type: object
+ description: |
+ During the split mode, each RPU core has its own set of ATCM and BTCM memory
+
+ During the lock-step operation, the TCMs that are associated with the
+ redundant processor become available to the lock-step processor.
+ For example if each individual processor has 64KB ATCM, then in lockstep mode
+ The size of ATCM become 128KB. Same for BTCM. tcm-lockstep node represents
+ TCM address space in lockstep mode. tcm-core@x node specfies each core's
+ TCM address space in split mode.
+
+ properties:
+ compatible:
+ oneOf:
This is not oneOf.
+ - items:
and you do not have more than one item.
+ - enum:
+ - xlnx,tcm-lockstep
+ - xlnx,tcm-split
compatible describes hardware, not configuration. What you encode here
does not fit compatible.
I see. So, only xlnx,tcm is enough.
No, it must be specific to SoC.
Ok. Then xlnx,zynqmp-tcm. I will change file name accordingly as well.
+
+ "#address-cells":
Use consistent quotes, either " or '
Ack.
+ const: 1
+
+ "#size-cells":
+ const: 1
+
+ reg:
+ items:
+ - description: |
+ ATCM Memory address space. An ATCM typically holds interrupt or
+ exception code that must be accessed at high speed, without any
+ potential delay resulting from a cache miss.
+ RPU on AMD-Xilinx platform can also fetch data from ATCM
+ - description: |
+ BTCM Memory address space. A BTCM typically holds a block of data
+ for intensive processing, such as audio or video processing. RPU on
+ AMD-Xilinx Platforms can also fetch Code (Instructions) from BTCM
+
+ reg-names:
+ items:
+ - const: atcm
+ - const: btcm
+
+ ranges: true
+
+ power-domains:
+ maxItems: 8
+ items:
+ - description: list of ATCM Power domains
+ - description: list of BTCM Power domains
+ additionalItems: true
And what are the rest?
As both items are list, we should be able to include more than one
power-domain I believe.
So first item I am trying to create list of ATCM power domains.
In split mode, first item is ATCM power-domain and second item is BTCM
power domain.
However, In lockstep mode, second core's TCM physically relocates and
two ATCM combines and
Why power domains of a device depend on the mode? This does not look
like binding describing hardware.
makes single region of ATCM. However, their power-domains remains same.
So, In lockstep mode, first two banks are ATCM and so, first two items
are ATCM power-domains.
I am not sure best way to represent this. But, first itmes is list.
So, I am assuming list of all ATCM power-domains possible.
List all items. Order is fixed, you cannot say BTCM is second item and
then put here something else.
Best regards,
Krzysztof