| Message ID | 20230324061608.33609-3-quic_hazha@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add support to configure Coresight Dummy subunit | expand |
On 24/03/2023 06:16, Hao Zhang wrote: > Add new coresight-dummy.yaml file describing the bindings required > to define coresight dummy trace in the device trees. > > 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 As mentioned in the previous email, please make this Arm CoreSight. This is not specific to Qcom, rather something the CoreSight driver exposes as a dummy framework. Otherwise looks good to me. Suzuki
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. > 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? > + > +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... > + compatible: > + items: Drop items. You have only one item, so no need for list. > + - 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. > + > + 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 > + 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. > + - | > + 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? > + }; > + }; > + }; > + }; > + > +... Best regards, Krzysztof
Hi Suzuki, On 3/24/2023 6:47 PM, Suzuki K Poulose wrote: > On 24/03/2023 06:16, Hao Zhang wrote: >> Add new coresight-dummy.yaml file describing the bindings required >> to define coresight dummy trace in the device trees. >> >> 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 > > As mentioned in the previous email, please make this Arm CoreSight. This > is not specific to Qcom, rather something the CoreSight driver exposes > as a dummy framework. Otherwise looks good to me. > > Suzuki > Thanks for your comment, I will take your advice and update it in the next version of patch. Thanks, Hao
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 >
Hi, As per my comments in the previous patch in this set.... On Mon, 27 Mar 2023 at 08:38, Hao Zhang <quic_hazha@quicinc.com> wrote: > > 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 Can we have coresight-dummy-source and coresight-dummy-sink? > >> + 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. > > > These properties not required if the compatible name is more specific > 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 > >> + The binding should constrain out ports to dummy-source only, and in ports to dummy sink only. Regards Mike > >> +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 > >
Hi Mike, On 3/28/2023 6:12 PM, Mike Leach wrote: > Hi, > > As per my comments in the previous patch in this set.... > > On Mon, 27 Mar 2023 at 08:38, Hao Zhang <quic_hazha@quicinc.com> wrote: >> >> 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 > > Can we have coresight-dummy-source and coresight-dummy-sink? Sure, I will take your advice in the next version of patch. > >>>> + 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. >>> >> > > These properties not required if the compatible name is more specific > I will update it in the next version of patch. >> 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 >>>> + > > The binding should constrain out ports to dummy-source only, and in > ports to dummy sink only. > > Regards > > Mike > I will update it according to your advice in the next version of patch. Thanks, Hao >>>> +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 >>> > > >
On Fri, Mar 24, 2023 at 02:16:07PM +0800, Hao Zhang wrote: > Add new coresight-dummy.yaml file describing the bindings required > to define coresight dummy trace in the device trees. The diff tells me all this. Please explain why this is needed and needs to be in DT here. > > 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. EUD? TPDM? I don't really love 'dummy' used here. Maybe the OS still wants/needs to know where the sink goes to even if not configurable. You *can* have multiple compatibles for a single generic driver if those compatibles might be useful some day. > + 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 > + > +properties: > + $nodename: > + pattern: "^dummy_.*(sink|source)_[0-9]+.*$" Don't use '_' in node names. Convention for multiple instances without 'reg' is '-[0-9]+' on the end, but you are allowing anything after that. > + compatible: > + items: > + - const: qcom,coresight-dummy > + > + qcom,dummy-sink: > + type: boolean > + description: > + Indicates that the type of this coresight node is dummy sink. > + > + qcom,dummy-source: Incorporate source or sink into the compatible strings. It's also somewhat redundant with 'in-ports' vs. 'out-ports'. > + type: boolean > + description: > + Indicates that the type of this coresight node is dummy source. > + > + out-ports: > + description: | Don't need '|' unless you need to preserve formatting. > + 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: | > + 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 { > + compatible = "qcom,coresight-dummy"; > + qcom,dummy-sink; > + > + in-ports { > + port { > + eud_in_replicator_swao: endpoint { > + remote-endpoint = > + <&replicator_swao_out_eud>; > + }; > + }; > + }; > + }; > + > + # minimum dummy source definition. dummy source connect to coresight funnel. > + - | > + 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>; > + }; > + }; > + }; > + }; > + > +... > -- > 2.17.1 >
Hi Rob, On 4/1/2023 2:47 AM, Rob Herring wrote: > On Fri, Mar 24, 2023 at 02:16:07PM +0800, Hao Zhang wrote: >> Add new coresight-dummy.yaml file describing the bindings required >> to define coresight dummy trace in the device trees. > > The diff tells me all this. Please explain why this is needed and needs > to be in DT here. > Sure, will add more details to describe it. >> >> 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. > > EUD? TPDM? The term EUD stand for Embedded USB debugger, it will be connected to coresight replicator component to receive and store coresight data. It would be configured by NON-HLOS, and need HLOS(Kernel) to configure the last coresight components. So we will use dummy sink to replace it in kernel side for building the whole path(from source to sink). The TPDM is Trace Profiling and Diagnostics Monitor, it is a coresight trace source which could get hardware events from the IP subsystem. > > I don't really love 'dummy' used here. Maybe the OS still wants/needs to > know where the sink goes to even if not configurable. > > You *can* have multiple compatibles for a single generic driver if those > compatibles might be useful some day. > Yes, we want to take it as a generic framework for coresight dummy sink and source. I think we could add one more compatible to indicate the type of it. >> + 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 >> + >> +properties: >> + $nodename: >> + pattern: "^dummy_.*(sink|source)_[0-9]+.*$" > > Don't use '_' in node names. > > Convention for multiple instances without 'reg' is '-[0-9]+' on the end, > but you are allowing anything after that. > OK, I will update it in the next version of patch. >> + compatible: >> + items: >> + - const: qcom,coresight-dummy >> + >> + qcom,dummy-sink: >> + type: boolean >> + description: >> + Indicates that the type of this coresight node is dummy sink. >> + >> + qcom,dummy-source: > > Incorporate source or sink into the compatible strings. > OK, I will update it in the next version of patch. > It's also somewhat redundant with 'in-ports' vs. 'out-ports'. I think I could add more details to describe it. > >> + type: boolean >> + description: >> + Indicates that the type of this coresight node is dummy source. >> + >> + out-ports: >> + description: | > > Don't need '|' unless you need to preserve formatting. I will remove it. Thanks for your comments, I will take your advice and update it in the next version of patch. Thanks, Hao > >> + 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: | >> + 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 { >> + compatible = "qcom,coresight-dummy"; >> + qcom,dummy-sink; >> + >> + in-ports { >> + port { >> + eud_in_replicator_swao: endpoint { >> + remote-endpoint = >> + <&replicator_swao_out_eud>; >> + }; >> + }; >> + }; >> + }; >> + >> + # minimum dummy source definition. dummy source connect to coresight funnel. >> + - | >> + 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>; >> + }; >> + }; >> + }; >> + }; >> + >> +... >> -- >> 2.17.1 >>
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 + +properties: + $nodename: + pattern: "^dummy_.*(sink|source)_[0-9]+.*$" + compatible: + items: + - const: qcom,coresight-dummy + + qcom,dummy-sink: + type: boolean + description: + Indicates that the type of this coresight node is dummy sink. + + qcom,dummy-source: + type: boolean + description: + Indicates that the type of this coresight node is dummy source. + + out-ports: + description: | + 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: | + 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 { + compatible = "qcom,coresight-dummy"; + qcom,dummy-sink; + + in-ports { + port { + eud_in_replicator_swao: endpoint { + remote-endpoint = + <&replicator_swao_out_eud>; + }; + }; + }; + }; + + # minimum dummy source definition. dummy source connect to coresight funnel. + - | + 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>; + }; + }; + }; + }; + +...
Add new coresight-dummy.yaml file describing the bindings required to define coresight dummy trace in the device trees. 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