diff mbox series

[v1,2/2] dt-bindings: arm: Add remote etm driver

Message ID 20231107060939.13449-3-quic_jinlmao@quicinc.com
State New
Headers
Series coresight: Add remote etm support |

Commit Message

Mao Jinlong Nov. 7, 2023, 6:09 a.m. UTC 
  Add new coresight-remote-etm.yaml file describing the bindings required
to define coresight remote etm in the device trees.

Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com>
---
 .../arm/qcom,coresight-remote-etm.yaml        | 59 +++++++++++++++++++
 1 file changed, 59 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/qcom,coresight-remote-etm.yaml

Comments

Krzysztof Kozlowski Nov. 7, 2023, 3:30 p.m. UTC | #1 
On 07/11/2023 07:09, Mao Jinlong wrote:
> Add new coresight-remote-etm.yaml file describing the bindings required
> to define coresight remote etm in the device trees.

Subject: drop driver. Bindings are about hardware, not drivers.

> 
> Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com>
> ---
>  .../arm/qcom,coresight-remote-etm.yaml        | 59 +++++++++++++++++++
>  1 file changed, 59 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/qcom,coresight-remote-etm.yaml
> 
> diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-remote-etm.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-remote-etm.yaml
> new file mode 100644
> index 000000000000..04bb57b48d96
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-remote-etm.yaml
> @@ -0,0 +1,59 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/arm/qcom,coresight-remote-etm.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Coresight Remote ETM

What is ETM?

> +
> +maintainers:
> +  - Jinlong Mao <quic_jinlmao@quicinc.com>
> +  - Tao Zhang <quic_taozha@quicinc.com>
> +
> +description: |

Do not need '|' unless you need to preserve formatting.

> +  Support for ETM trace collection on remote processor using coresight
> +  framework. Enabling this will allow turning on ETM tracing on remote
> +  processor via sysfs and collecting the trace via TMC sinks.
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: qcom,coresight-remote-etm
> +
> +  qcom,inst-id:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Instance id of remote etm.

It's the first time this property appears... Why do you need to
hard-code it and why would it differ between each board? If you want to
use existing, accepted property, then you must use exactly the same.

> +
> +  out-ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +    additionalProperties: false
> +
> +    properties:
> +      port:
> +        description: Output connection to the CoreSight Trace bus.
> +        $ref: /schemas/graph.yaml#/properties/port
> +
> +required:
> +  - compatible
> +  - qcom,inst-id
> +  - out-ports
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    modem_etm0 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

Underscores are not allowed.

> +        compatible = "qcom,coresight-remote-etm";
> +        qcom,inst-id = <2>;
> +
> +        out-ports {
> +            port {
> +                modem_etm0_out_funnel_modem: endpoint {
> +                remote-endpoint =
> +                    <&funnel_modem_in_modem_etm0>;

Fix wrapping.

> +                };
> +            };
> +        };
> +    };
> +...

Best regards,
Krzysztof
Krzysztof Kozlowski Nov. 7, 2023, 3:31 p.m. UTC | #2 
On 07/11/2023 07:09, Mao Jinlong wrote:
> Add new coresight-remote-etm.yaml file describing the bindings required
> to define coresight remote etm in the device trees.
> 

...

> +title: Qualcomm Coresight Remote ETM
> +
> +maintainers:
> +  - Jinlong Mao <quic_jinlmao@quicinc.com>
> +  - Tao Zhang <quic_taozha@quicinc.com>
> +
> +description: |
> +  Support for ETM trace collection on remote processor using coresight
> +  framework. Enabling this will allow turning on ETM tracing on remote
> +  processor via sysfs and collecting the trace via TMC sinks.

BTW, your cover letter says more than commit messages and descriptions.
It should be reversed. Cover letter is meaningless afterwards.

Best regards,
Krzysztof
Krzysztof Kozlowski Nov. 16, 2023, 5:22 p.m. UTC | #3 
On 07/11/2023 07:09, Mao Jinlong wrote:
> Add new coresight-remote-etm.yaml file describing the bindings required
> to define coresight remote etm in the device trees.
> 
> Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com>

+Cc Trilok,

Several Qualcomm boards have warnings coming from Coresight bindings.
These are big, fat warnings coming usually from ARM bindings (e.g.
dynamic funnel, TMC). I don't know Coresight good enough to fix them by
myself.

I would prefer not to take any new Qualcomm specific Coresight bindings
and definitely no new Coresight device nodes in Qualcomm boards, before
these are fixed.

Therefore I kindly ask to fix all warnings in Qualcomm boards coming
from existing Coresight bindings.

Best regards,
Krzysztof
Trilok Soni Nov. 20, 2023, 10:39 p.m. UTC | #4 
On 11/16/2023 9:22 AM, Krzysztof Kozlowski wrote:
> On 07/11/2023 07:09, Mao Jinlong wrote:
>> Add new coresight-remote-etm.yaml file describing the bindings required
>> to define coresight remote etm in the device trees.
>>
>> Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com>
> 
> +Cc Trilok,
> 
> Several Qualcomm boards have warnings coming from Coresight bindings.
> These are big, fat warnings coming usually from ARM bindings (e.g.
> dynamic funnel, TMC). I don't know Coresight good enough to fix them by
> myself.
> 
> I would prefer not to take any new Qualcomm specific Coresight bindings
> and definitely no new Coresight device nodes in Qualcomm boards, before
> these are fixed.
> 
> Therefore I kindly ask to fix all warnings in Qualcomm boards coming
> from existing Coresight bindings.

Thanks Krzysztof, we can look into it as first thing to fix.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-remote-etm.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-remote-etm.yaml
new file mode 100644
index 000000000000..04bb57b48d96
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/qcom,coresight-remote-etm.yaml
@@ -0,0 +1,59 @@ 
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/qcom,coresight-remote-etm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Coresight Remote ETM
+
+maintainers:
+  - Jinlong Mao <quic_jinlmao@quicinc.com>
+  - Tao Zhang <quic_taozha@quicinc.com>
+
+description: |
+  Support for ETM trace collection on remote processor using coresight
+  framework. Enabling this will allow turning on ETM tracing on remote
+  processor via sysfs and collecting the trace via TMC sinks.
+
+properties:
+  compatible:
+    items:
+      - const: qcom,coresight-remote-etm
+
+  qcom,inst-id:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Instance id of remote etm.
+
+  out-ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+    additionalProperties: false
+
+    properties:
+      port:
+        description: Output connection to the CoreSight Trace bus.
+        $ref: /schemas/graph.yaml#/properties/port
+
+required:
+  - compatible
+  - qcom,inst-id
+  - out-ports
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    modem_etm0 {
+        compatible = "qcom,coresight-remote-etm";
+        qcom,inst-id = <2>;
+
+        out-ports {
+            port {
+                modem_etm0_out_funnel_modem: endpoint {
+                remote-endpoint =
+                    <&funnel_modem_in_modem_etm0>;
+                };
+            };
+        };
+    };
+...