lkml.org 
[lkml]   [2023]   [Mar]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v2 2/3] dt-bindings: arm: Add Coresight Dummy Trace YAML schema
From
Hi Krzysztof,

On 3/25/2023 7:49 PM, Krzysztof Kozlowski wrote:
> On 24/03/2023 07:16, Hao Zhang wrote:
>> Add new coresight-dummy.yaml file describing the bindings required
>> to define coresight dummy trace in the device trees.
>>
>
> Subject: drop second/last, redundant "YAML schema". The "dt-bindings"
> prefix is already stating that these are bindings and all new must be DT
> schema. You cannot add anything else, so this is redundant.
>
I will take your advice to drop redundant part of title in the next
version of patch.
>
>> Signed-off-by: Hao Zhang <quic_hazha@quicinc.com>
>> ---
>> .../bindings/arm/qcom,coresight-dummy.yaml | 118 ++++++++++++++++++
>> 1 file changed, 118 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/arm/qcom,coresight-dummy.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-dummy.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-dummy.yaml
>> new file mode 100644
>> index 000000000000..7b719b084d72
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-dummy.yaml
>> @@ -0,0 +1,118 @@
>> +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
>> +# Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/arm/qcom,coresight-dummy.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: QCOM Coresight Dummy component
>> +
>> +description: |
>> + The Coresight Dummy component is for the specific devices that HLOS don't have
>> + permission to access or configure. Such as Coresight sink EUD, some TPDMs etc.
>> + So there need driver to register dummy devices as Coresight devices. Provide
>> + Coresight API for dummy device operations, such as enabling and disabling
>> + dummy devices. Build the Coresight path for dummy sink or dummy source for
>> + debugging.
>> +
>> + The primary use case of the coresight dummy is to build path for dummy sink or
>> + dummy source.
>> +
>> +maintainers:
>> + - Mao Jinlong <quic_jinlmao@quicinc.com>
>> + - Tao Zhang <quic_taozha@quicinc.com>
>> + - Hao Zhang <quic_hazha@quicinc.com>
>> +
>> +select:
>> + properties:
>> + compatible:
>> + contains:
>> + enum:
>> + - qcom,coresight-dummy
>> + required:
>> + - compatible
>
> Why do you need the select?
>
This is a mistake, will remove it in the next version of patch.
>> +
>> +properties:
>> + $nodename:
>> + pattern: "^dummy_.*(sink|source)_[0-9]+.*$"
>
> We do not enforce node names in individual bindings. Why do you need it?
> Plus underscore is not even proper character...
>
I will remove this node.

>> + compatible:
>> + items:
>
> Drop items. You have only one item, so no need for list.

I will take your advice and update it in the next version of patch.

>> + - const: qcom,coresight-dummy
>> +
>> + qcom,dummy-sink:
>> + type: boolean
>> + description:
>> + Indicates that the type of this coresight node is dummy sink.
>
> You just duplicated property name. Write something useful.
>
>> +
>> + qcom,dummy-source:
>> + type: boolean
>> + description:
>> + Indicates that the type of this coresight node is dummy source.
>
> You just duplicated property name. Write something useful.
>

Sure, I will add more details for it.

>> +
>> + out-ports:
>> + description: |
>
> No need for |
>
>> + Output connections from the dummy source to Coresight Trace bus.
>> + $ref: /schemas/graph.yaml#/properties/ports
>> +
>> + properties:
>> + port:
>> + description: Output connection from the dummy source to Coresight
>> + Trace bus.
>> + $ref: /schemas/graph.yaml#/properties/port
>> +
>> + in-ports:
>> + description: |
>
> Ditto
>
I will remove it in the next version of patch.

>> + Input connections from the CoreSight Trace bus to dummy sink.
>> + $ref: /schemas/graph.yaml#/properties/ports
>> +
>> + properties:
>> + port:
>> + description: Input connection from the Coresight Trace bus to
>> + dummy sink.
>> + $ref: /schemas/graph.yaml#/properties/port
>> +
>> +required:
>> + - compatible
>> +
>> +additionalProperties: false
>> +
>> +oneOf:
>> + - required:
>> + - qcom,dummy-sink
>> + - required:
>> + - qcom,dummy-source
>> +
>> +examples:
>> + # minimum dummy sink definition. dummy sink connect to coresight replicator.
>> + - |
>> + dummy_sink_1 {
>
> Node names should be generic, so "sink"
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
>
>> + compatible = "qcom,coresight-dummy";
>> + qcom,dummy-sink;
>> +
>> + in-ports {
>> + port {
>> + eud_in_replicator_swao: endpoint {
>> + remote-endpoint =
>> + <&replicator_swao_out_eud>;
>
> Why line break after =?
>

>> + };
>> + };
>> + };
>> + };
>> +
>> + # minimum dummy source definition. dummy source connect to coresight funnel.
>
> If you use sentences, then start with capital letter.
>

I will update it according to your advice in the next version of patch.

>> + - |
>> + dummy_source_1 {
>> + compatible = "qcom,coresight-dummy";
>> + qcom,dummy-source;
>> +
>> + out-ports {
>> + port {
>> + dummy_riscv_out_funnel_swao: endpoint {
>> + remote-endpoint =
>> + <&funnel_swao_in_dummy_riscv>;
>
> Why line break?

I copy it from device tree and keep the original format, will correct
the format in the next version of patch.

Thanks,
Hao

>> + };
>> + };
>> + };
>> + };
>> +
>> +...
>
> Best regards,
> Krzysztof
>

\
 
 \ /
  Last update: 2023-03-27 09:40    [W:0.728 / U:1.492 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site