RE: [EXT] Re: [PATCH 1/7] dt-bindings: arm: coresight-tmc: Add "memory-region" property

Hi Mike,

-----Original Message-----
From: Linu Cherian
Sent: Saturday, October 14, 2023 5:07 PM
To: Mike Leach <redacted>
Cc: suzuki.poulose@arm.com; james.clark@arm.com; leo.yan@linaro.org;
linux-arm-kernel@lists.infradead.org; coresight@lists.linaro.org; linux-
kernel@vger.kernel.org; robh+dt@kernel.org;
krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org;
devicetree@vger.kernel.org; Sunil Kovvuri Goutham
[off-list ref]; George Cherian [off-list ref]
Subject: RE: [EXT] Re: [PATCH 1/7] dt-bindings: arm: coresight-tmc: Add
"memory-region" property

Hi Mike,

quoted

-----Original Message-----
From: Mike Leach <redacted>
Sent: Friday, October 6, 2023 4:33 PM
To: Linu Cherian <redacted>
Cc: suzuki.poulose@arm.com; james.clark@arm.com; leo.yan@linaro.org;
linux-arm-kernel@lists.infradead.org; coresight@lists.linaro.org;
linux- kernel@vger.kernel.org; robh+dt@kernel.org;
krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org;
devicetree@vger.kernel.org; Sunil Kovvuri Goutham
[off-list ref]; George Cherian [off-list ref]
Subject: [EXT] Re: [PATCH 1/7] dt-bindings: arm: coresight-tmc: Add
"memory-region" property

External Email

----------------------------------------------------------------------
Hi Linu

On Fri, 29 Sept 2023 at 14:38, Linu Cherian [off-list ref] wrote:

quoted

memory-region 0: Reserved trace buffer memory

  TMC ETR: When available, use this reserved memory region for
  trace data capture. Same region is used for trace data
  retention after a panic or watchdog reset.

  TMC ETF: When available, use this reserved memory region for
  trace data retention synced from internal SRAM after a panic or
  watchdog reset.

memory-region 1: Reserved meta data memory

  TMC ETR, ETF: When available, use this memory for register
  snapshot retention synced from hardware registers after a panic
  or watchdog reset.

Signed-off-by: Linu Cherian <redacted>
---
 .../bindings/arm/arm,coresight-tmc.yaml       | 19

+++++++++++++++++++

quoted

quoted

 1 file changed, 19 insertions(+)

diff --git
a/Documentation/devicetree/bindings/arm/arm,coresight-tmc.yaml
b/Documentation/devicetree/bindings/arm/arm,coresight-tmc.yaml
index cb8dceaca70e..45ca4d02d73e 100644

--- a/Documentation/devicetree/bindings/arm/arm,coresight-tmc.yaml
+++ b/Documentation/devicetree/bindings/arm/arm,coresight-tmc.yaml

@@ -101,6 +101,22 @@ properties:
           and ETF configurations.
         $ref: /schemas/graph.yaml#/properties/port

+  memory-region:
+    items:
+      - description: Reserved trace buffer memory for ETR and ETF sinks.
+          For ETR, this reserved memory region is used for trace data

capture.

quoted

quoted

+          Same region is used for trace data retention as well after a panic
+          or watchdog reset.
+          For ETF, this reserved memory region is used for retention of trace
+          data synced from internal SRAM after a panic or watchdog reset.
+

Is there a valid use case for ETR where we use these areas when there
is not a panic/reset situation?

Currently its meant to be used only for the panic/reset situations and gets
enabled only with a special buffer mode.

quoted

Either way - the description should perhaps mention that these areas
are only used if specifically selected by the driver - the default
memory usage models for ETR / perf are otherwise unaltered.

Ack.

quoted

quoted

+      - description: Reserved meta data memory. Used for ETR and ETF

sinks.

quoted

quoted

+
+  memory-region-names:
+    items:
+      - const: trace-mem
+      - const: metadata-mem
+

Is there a constraint required here? If we are using the memory area
for trace in a panic situation, then we must have the meta data memory
area defined?
Perhaps a set of names such as "etr-trace-mem", "panic-trace-mem" ,
"panic-metadata-mem", were the first is for general ETR trace in
non-panic situation and then constrain the "panic-" areas to appear

together.

quoted

The "etr-trace-mem", "panic-trace-mem" could easily point to the same
area.

As noted above, we do not have other generic use case for these reserved
regions now.
Hence two regions/names, panic-trace-mem and panic-metadata-mem with
constraints kept as
 minItems: 2 and maxItems: 2 would suffice ?

Looking this further, realized that by default minItems and maxItems are 2 in this case.
IIUC, nothing additional required to enforce this constraint.

Also tested this, with the below dts change with just one memory-region,

diff --git a/arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi b/arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi
index 09d2b692e9e1..03ff6b2f7269 100644
--- a/arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi
+++ b/arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi

@@ -30,6 +30,9 @@ etf_sys1: etf@20140000 { /* etf1 */
                clocks = <&soc_smc50mhz>;
                clock-names = "apb_pclk";
                power-domains = <&scpi_devpd 0>;
+               memory-region = <&etf1_trace>;
+               memory-region-names = "tracedata";
+
                in-ports {

Validation of DTS files using below command identified this.
# make CHECK_DTBS=y ARCH=arm64  arm/juno-r2-scmi.dtb 

     from schema $id: http://devicetree.org/schemas/thermal/thermal-zones.yaml#
/home/lcherian/scode/mainline/linux/arch/arm64/boot/dts/arm/juno-r2-scmi.dtb: etf@20140000: memory-region: [[72]] is too short
        from schema $id: http://devicetree.org/schemas/arm/arm,coresight-tmc.yaml#
/home/lcherian/scode/mainline/linux/arch/arm64/boot/dts/arm/juno-r2-scmi.dtb: etf@20140000: memory-region-names: ['tracedata'] is too short
        from schema $id: http://devicetree.org/schemas/arm/arm,coresight-tmc.yaml#
/home/lcherian/scode/mainline/linux/arch/arm64/boot/dts/arm/juno-r2-scmi.dtb: etf@20140000: Unevaluated properties are not allowed ('memory-region', 'memory-region-names' were unexp
ected)


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel