diff mbox series

[v3,3/5] dt-bindings: arm: Add Coresight TMC Control Unit hardware

Message ID 20240812024141.2867655-4-quic_jiegan@quicinc.com
State Superseded
Headers show
Series Coresight: Add Coresight TMC Control Unit driver | expand

Commit Message

Jie Gan Aug. 12, 2024, 2:41 a.m. UTC
Add binding file to specify how to define a Coresight TMC
Control Unit device in device tree.

It is responsible for controlling the data filter function
based on the source device's Trace ID for TMC ETR device.
The trace data with that Trace id can get into ETR's buffer
while other trace data gets ignored.

Signed-off-by: Jie Gan <quic_jiegan@quicinc.com>
---
 .../bindings/arm/qcom,coresight-ctcu.yaml     | 79 +++++++++++++++++++
 1 file changed, 79 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml

Comments

Rob Herring (Arm) Aug. 18, 2024, 2:28 p.m. UTC | #1
On Mon, Aug 12, 2024 at 10:41:39AM +0800, Jie Gan wrote:
> Add binding file to specify how to define a Coresight TMC
> Control Unit device in device tree.
> 
> It is responsible for controlling the data filter function
> based on the source device's Trace ID for TMC ETR device.
> The trace data with that Trace id can get into ETR's buffer
> while other trace data gets ignored.
> 
> Signed-off-by: Jie Gan <quic_jiegan@quicinc.com>
> ---
>  .../bindings/arm/qcom,coresight-ctcu.yaml     | 79 +++++++++++++++++++
>  1 file changed, 79 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml
> 
> diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml
> new file mode 100644
> index 000000000000..7a9580007942
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml
> @@ -0,0 +1,79 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/arm/qcom,coresight-ctcu.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: CoreSight TMC Control Unit
> +
> +maintainers:
> +  - Yuanfang Zhang <quic_yuanfang@quicinc.com>
> +  - Mao Jinlong <quic_jinlmao@quicinc.com>
> +  - Jie Gan <quic_jiegan@quicinc.com>
> +
> +description:
> +  The Coresight TMC Control unit controls various Coresight behaviors.
> +  It works as a helper device when connected to TMC ETR device.
> +  It is responsible for controlling the data filter function based on
> +  the source device's Trace ID for TMC ETR device. The trace data with
> +  that Trace id can get into ETR's buffer while other trace data gets
> +  ignored.

Nowhere is TMC defined.

> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,sa8775p-ctcu
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    items:
> +      - const: apb
> +
> +  in-ports:

Use 'ports' unless you have both in and out ports.

> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    patternProperties:
> +      '^port(@[0-7])?$':
> +        description: Input connections from CoreSight Trace bus
> +        $ref: /schemas/graph.yaml#/properties/port
> +
> +required:
> +  - compatible
> +  - reg
> +  - in-ports
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    ctcu@1001000 {
> +        compatible = "qcom,sa8775p-ctcu";
> +        reg = <0x1001000 0x1000>;
> +
> +        clocks = <&aoss_qmp>;
> +        clock-names = "apb";
> +
> +        in-ports {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            port@0 {
> +                reg = <0>;
> +                ctcu_in_port0: endpoint {
> +                    remote-endpoint = <&etr0_out_port>;
> +                };
> +            };
> +
> +            port@1 {
> +                reg = <1>;
> +                ctcu_in_port1: endpoint {
> +                    remote-endpoint = <&etr1_out_port>;
> +                };
> +            };
> +        };
> +    };
> -- 
> 2.34.1
>
Jie Gan Aug. 19, 2024, 1:49 a.m. UTC | #2
On Sun, Aug 18, 2024 at 08:28:34AM -0600, Rob Herring wrote:
> On Mon, Aug 12, 2024 at 10:41:39AM +0800, Jie Gan wrote:
> > Add binding file to specify how to define a Coresight TMC
> > Control Unit device in device tree.
> > 
> > It is responsible for controlling the data filter function
> > based on the source device's Trace ID for TMC ETR device.
> > The trace data with that Trace id can get into ETR's buffer
> > while other trace data gets ignored.
> > 
> > Signed-off-by: Jie Gan <quic_jiegan@quicinc.com>
> > ---
> >  .../bindings/arm/qcom,coresight-ctcu.yaml     | 79 +++++++++++++++++++
> >  1 file changed, 79 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml
> > new file mode 100644
> > index 000000000000..7a9580007942
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml
> > @@ -0,0 +1,79 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/arm/qcom,coresight-ctcu.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: CoreSight TMC Control Unit
> > +
> > +maintainers:
> > +  - Yuanfang Zhang <quic_yuanfang@quicinc.com>
> > +  - Mao Jinlong <quic_jinlmao@quicinc.com>
> > +  - Jie Gan <quic_jiegan@quicinc.com>
> > +
> > +description:
> > +  The Coresight TMC Control unit controls various Coresight behaviors.
> > +  It works as a helper device when connected to TMC ETR device.
> > +  It is responsible for controlling the data filter function based on
> > +  the source device's Trace ID for TMC ETR device. The trace data with
> > +  that Trace id can get into ETR's buffer while other trace data gets
> > +  ignored.
> 
> Nowhere is TMC defined.
The Coresight TMC control unit(CTCU) connected to Coresight TMC device via replicator and
works as a helper device to TMC device.

The in-ports listed below illustrate their connection to TMC devices.

> 
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - qcom,sa8775p-ctcu
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  clock-names:
> > +    items:
> > +      - const: apb
> > +
> > +  in-ports:
> 
> Use 'ports' unless you have both in and out ports.
The ‘in-ports’ and ‘out-ports’ properties will be parsed by ‘of_coresight_get_port_parent’
and their relationships to other devices will be stored in the coresight_platform_data structure.

for example:
struct coresight_platform_data {
	int nr_inconns;
	int nr_outconns;
	struct coresight_connection **out_conns;
	struct coresight_connection **in_conns;
};

https://elixir.bootlin.com/linux/v6.11-rc4/source/drivers/hwtracing/coresight/coresight-platform.c#L147

> 
> > +    $ref: /schemas/graph.yaml#/properties/ports
> > +
> > +    patternProperties:
> > +      '^port(@[0-7])?$':
> > +        description: Input connections from CoreSight Trace bus
> > +        $ref: /schemas/graph.yaml#/properties/port
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - in-ports
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    ctcu@1001000 {
> > +        compatible = "qcom,sa8775p-ctcu";
> > +        reg = <0x1001000 0x1000>;
> > +
> > +        clocks = <&aoss_qmp>;
> > +        clock-names = "apb";
> > +
> > +        in-ports {
> > +            #address-cells = <1>;
> > +            #size-cells = <0>;
> > +
> > +            port@0 {
> > +                reg = <0>;
> > +                ctcu_in_port0: endpoint {
> > +                    remote-endpoint = <&etr0_out_port>;
> > +                };
> > +            };
> > +
> > +            port@1 {
> > +                reg = <1>;
> > +                ctcu_in_port1: endpoint {
> > +                    remote-endpoint = <&etr1_out_port>;
> > +                };
> > +            };
> > +        };
> > +    };
> > -- 
> > 2.34.1
> > 
> 

Thanks,
Jie
Krzysztof Kozlowski Aug. 19, 2024, 6:25 a.m. UTC | #3
On 19/08/2024 03:49, JieGan wrote:
> On Sun, Aug 18, 2024 at 08:28:34AM -0600, Rob Herring wrote:
>> On Mon, Aug 12, 2024 at 10:41:39AM +0800, Jie Gan wrote:
>>> Add binding file to specify how to define a Coresight TMC
>>> Control Unit device in device tree.
>>>
>>> It is responsible for controlling the data filter function
>>> based on the source device's Trace ID for TMC ETR device.
>>> The trace data with that Trace id can get into ETR's buffer
>>> while other trace data gets ignored.
>>>
>>> Signed-off-by: Jie Gan <quic_jiegan@quicinc.com>
>>> ---
>>>  .../bindings/arm/qcom,coresight-ctcu.yaml     | 79 +++++++++++++++++++
>>>  1 file changed, 79 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml
>>> new file mode 100644
>>> index 000000000000..7a9580007942
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml
>>> @@ -0,0 +1,79 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/arm/qcom,coresight-ctcu.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: CoreSight TMC Control Unit
>>> +
>>> +maintainers:
>>> +  - Yuanfang Zhang <quic_yuanfang@quicinc.com>
>>> +  - Mao Jinlong <quic_jinlmao@quicinc.com>
>>> +  - Jie Gan <quic_jiegan@quicinc.com>
>>> +
>>> +description:
>>> +  The Coresight TMC Control unit controls various Coresight behaviors.
>>> +  It works as a helper device when connected to TMC ETR device.
>>> +  It is responsible for controlling the data filter function based on
>>> +  the source device's Trace ID for TMC ETR device. The trace data with
>>> +  that Trace id can get into ETR's buffer while other trace data gets
>>> +  ignored.
>>
>> Nowhere is TMC defined.
> The Coresight TMC control unit(CTCU) connected to Coresight TMC device via replicator and
> works as a helper device to TMC device.

Did you understand the feedback or just responding with whatever to get
rid of reviewers?

> 
> The in-ports listed below illustrate their connection to TMC devices.
> 
>>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - qcom,sa8775p-ctcu
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  clocks:
>>> +    maxItems: 1
>>> +
>>> +  clock-names:
>>> +    items:
>>> +      - const: apb
>>> +
>>> +  in-ports:
>>
>> Use 'ports' unless you have both in and out ports.
> The ‘in-ports’ and ‘out-ports’ properties will be parsed by ‘of_coresight_get_port_parent’
> and their relationships to other devices will be stored in the coresight_platform_data structure.
> 
> for example:
> struct coresight_platform_data {
> 	int nr_inconns;
> 	int nr_outconns;
> 	struct coresight_connection **out_conns;
> 	struct coresight_connection **in_conns;
> };
> 
> https://elixir.bootlin.com/linux/v6.11-rc4/source/drivers/hwtracing/coresight/coresight-platform.c#L147

and? If you respond with some unrelated argument, we will respond with
the same: Use 'ports' unless you have both in and out ports.

Best regards,
Krzysztof
Krzysztof Kozlowski Aug. 19, 2024, 6:26 a.m. UTC | #4
On 12/08/2024 04:41, Jie Gan wrote:
> +
> +maintainers:
> +  - Yuanfang Zhang <quic_yuanfang@quicinc.com>
> +  - Mao Jinlong <quic_jinlmao@quicinc.com>
> +  - Jie Gan <quic_jiegan@quicinc.com>
> +
> +description:
> +  The Coresight TMC Control unit controls various Coresight behaviors.
> +  It works as a helper device when connected to TMC ETR device.
> +  It is responsible for controlling the data filter function based on
> +  the source device's Trace ID for TMC ETR device. The trace data with
> +  that Trace id can get into ETR's buffer while other trace data gets
> +  ignored.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,sa8775p-ctcu
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    items:
> +      - const: apb
> +
> +  in-ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    patternProperties:
> +      '^port(@[0-7])?$':

I see only two ports in the example. How many are there in reality?

Best regards,
Krzysztof
Jie Gan Aug. 19, 2024, 8:51 a.m. UTC | #5
On Mon, Aug 19, 2024 at 08:25:33AM +0200, Krzysztof Kozlowski wrote:
> On 19/08/2024 03:49, JieGan wrote:
> > On Sun, Aug 18, 2024 at 08:28:34AM -0600, Rob Herring wrote:
> >> On Mon, Aug 12, 2024 at 10:41:39AM +0800, Jie Gan wrote:
> >>> Add binding file to specify how to define a Coresight TMC
> >>> Control Unit device in device tree.
> >>>
> >>> It is responsible for controlling the data filter function
> >>> based on the source device's Trace ID for TMC ETR device.
> >>> The trace data with that Trace id can get into ETR's buffer
> >>> while other trace data gets ignored.
> >>>
> >>> Signed-off-by: Jie Gan <quic_jiegan@quicinc.com>
> >>> ---
> >>>  .../bindings/arm/qcom,coresight-ctcu.yaml     | 79 +++++++++++++++++++
> >>>  1 file changed, 79 insertions(+)
> >>>  create mode 100644 Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml
> >>> new file mode 100644
> >>> index 000000000000..7a9580007942
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml
> >>> @@ -0,0 +1,79 @@
> >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >>> +%YAML 1.2
> >>> +---
> >>> +$id: http://devicetree.org/schemas/arm/qcom,coresight-ctcu.yaml#
> >>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>> +
> >>> +title: CoreSight TMC Control Unit
> >>> +
> >>> +maintainers:
> >>> +  - Yuanfang Zhang <quic_yuanfang@quicinc.com>
> >>> +  - Mao Jinlong <quic_jinlmao@quicinc.com>
> >>> +  - Jie Gan <quic_jiegan@quicinc.com>
> >>> +
> >>> +description:
> >>> +  The Coresight TMC Control unit controls various Coresight behaviors.
> >>> +  It works as a helper device when connected to TMC ETR device.
> >>> +  It is responsible for controlling the data filter function based on
> >>> +  the source device's Trace ID for TMC ETR device. The trace data with
> >>> +  that Trace id can get into ETR's buffer while other trace data gets
> >>> +  ignored.
> >>
> >> Nowhere is TMC defined.
> > The Coresight TMC control unit(CTCU) connected to Coresight TMC device via replicator and
> > works as a helper device to TMC device.
> 
> Did you understand the feedback or just responding with whatever to get
> rid of reviewers?

Sorry for the insufficient clarity in my response, I am just misunderstood the feedback and try
to explain the relationship between TMC and CTCU device.

I will add the TMC description to explain what TMC is as shown below:
The Trace Memory Controller(TMC) is used for Embedded Trace Buffer(ETB), Embedded Trace FIFO(ETF)
and Embedded Trace Router(ETR) configurations. The configuration mode (ETB, ETF, ETR) is
discovered at boot time when the device is probed.

> 
> > 
> > The in-ports listed below illustrate their connection to TMC devices.
> > 
> >>
> >>> +
> >>> +properties:
> >>> +  compatible:
> >>> +    enum:
> >>> +      - qcom,sa8775p-ctcu
> >>> +
> >>> +  reg:
> >>> +    maxItems: 1
> >>> +
> >>> +  clocks:
> >>> +    maxItems: 1
> >>> +
> >>> +  clock-names:
> >>> +    items:
> >>> +      - const: apb
> >>> +
> >>> +  in-ports:
> >>
> >> Use 'ports' unless you have both in and out ports.
> > The ‘in-ports’ and ‘out-ports’ properties will be parsed by ‘of_coresight_get_port_parent’
> > and their relationships to other devices will be stored in the coresight_platform_data structure.
> > 
> > for example:
> > struct coresight_platform_data {
> > 	int nr_inconns;
> > 	int nr_outconns;
> > 	struct coresight_connection **out_conns;
> > 	struct coresight_connection **in_conns;
> > };
> > 
> > https://elixir.bootlin.com/linux/v6.11-rc4/source/drivers/hwtracing/coresight/coresight-platform.c#L147
> 
> and? If you respond with some unrelated argument, we will respond with
> the same: Use 'ports' unless you have both in and out ports.

Sorry for the insufficient response.

The Coresight driver prefers using ‘in-ports’ and ‘out-ports’ instead of the ‘ports’ property, as each
Coresight component needs to specify its input and output directions.

The Coresight system operates by integrating all Coresight components and construting its data flow path
based on the defined directions. 

Consequently, the data flow direction cannot be determined when utilizing the ‘ports’ property in the
Coresight system.


> 
> Best regards,
> Krzysztof
> 

Thanks,
Jie
Jie Gan Aug. 19, 2024, 9:06 a.m. UTC | #6
On Mon, Aug 19, 2024 at 08:26:19AM +0200, Krzysztof Kozlowski wrote:
> On 12/08/2024 04:41, Jie Gan wrote:
> > +
> > +maintainers:
> > +  - Yuanfang Zhang <quic_yuanfang@quicinc.com>
> > +  - Mao Jinlong <quic_jinlmao@quicinc.com>
> > +  - Jie Gan <quic_jiegan@quicinc.com>
> > +
> > +description:
> > +  The Coresight TMC Control unit controls various Coresight behaviors.
> > +  It works as a helper device when connected to TMC ETR device.
> > +  It is responsible for controlling the data filter function based on
> > +  the source device's Trace ID for TMC ETR device. The trace data with
> > +  that Trace id can get into ETR's buffer while other trace data gets
> > +  ignored.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - qcom,sa8775p-ctcu
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  clock-names:
> > +    items:
> > +      - const: apb
> > +
> > +  in-ports:
> > +    $ref: /schemas/graph.yaml#/properties/ports
> > +
> > +    patternProperties:
> > +      '^port(@[0-7])?$':
> 
> I see only two ports in the example. How many are there in reality?
Existing projects can have a maximum of two ports. I used the range 0-7 as I consider
it unlikely to have more than 8 ports. Maybe it's intended as a large buffer for
futher design needs.

> 
> Best regards,
> Krzysztof
> 

Thanks,
Jie
Krzysztof Kozlowski Aug. 19, 2024, 9:41 a.m. UTC | #7
On 19/08/2024 11:06, JieGan wrote:
> On Mon, Aug 19, 2024 at 08:26:19AM +0200, Krzysztof Kozlowski wrote:
>> On 12/08/2024 04:41, Jie Gan wrote:
>>> +
>>> +maintainers:
>>> +  - Yuanfang Zhang <quic_yuanfang@quicinc.com>
>>> +  - Mao Jinlong <quic_jinlmao@quicinc.com>
>>> +  - Jie Gan <quic_jiegan@quicinc.com>
>>> +
>>> +description:
>>> +  The Coresight TMC Control unit controls various Coresight behaviors.
>>> +  It works as a helper device when connected to TMC ETR device.
>>> +  It is responsible for controlling the data filter function based on
>>> +  the source device's Trace ID for TMC ETR device. The trace data with
>>> +  that Trace id can get into ETR's buffer while other trace data gets
>>> +  ignored.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - qcom,sa8775p-ctcu
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  clocks:
>>> +    maxItems: 1
>>> +
>>> +  clock-names:
>>> +    items:
>>> +      - const: apb
>>> +
>>> +  in-ports:
>>> +    $ref: /schemas/graph.yaml#/properties/ports
>>> +
>>> +    patternProperties:
>>> +      '^port(@[0-7])?$':
>>
>> I see only two ports in the example. How many are there in reality?
> Existing projects can have a maximum of two ports. I used the range 0-7 as I consider
> it unlikely to have more than 8 ports. Maybe it's intended as a large buffer for
> futher design needs.

Please do not create buffers in the bindings. This must be specific, so [01]

Best regards,
Krzysztof
Jie Gan Aug. 19, 2024, 9:50 a.m. UTC | #8
On Mon, Aug 19, 2024 at 11:41:59AM +0200, Krzysztof Kozlowski wrote:
> On 19/08/2024 11:06, JieGan wrote:
> > On Mon, Aug 19, 2024 at 08:26:19AM +0200, Krzysztof Kozlowski wrote:
> >> On 12/08/2024 04:41, Jie Gan wrote:
> >>> +
> >>> +maintainers:
> >>> +  - Yuanfang Zhang <quic_yuanfang@quicinc.com>
> >>> +  - Mao Jinlong <quic_jinlmao@quicinc.com>
> >>> +  - Jie Gan <quic_jiegan@quicinc.com>
> >>> +
> >>> +description:
> >>> +  The Coresight TMC Control unit controls various Coresight behaviors.
> >>> +  It works as a helper device when connected to TMC ETR device.
> >>> +  It is responsible for controlling the data filter function based on
> >>> +  the source device's Trace ID for TMC ETR device. The trace data with
> >>> +  that Trace id can get into ETR's buffer while other trace data gets
> >>> +  ignored.
> >>> +
> >>> +properties:
> >>> +  compatible:
> >>> +    enum:
> >>> +      - qcom,sa8775p-ctcu
> >>> +
> >>> +  reg:
> >>> +    maxItems: 1
> >>> +
> >>> +  clocks:
> >>> +    maxItems: 1
> >>> +
> >>> +  clock-names:
> >>> +    items:
> >>> +      - const: apb
> >>> +
> >>> +  in-ports:
> >>> +    $ref: /schemas/graph.yaml#/properties/ports
> >>> +
> >>> +    patternProperties:
> >>> +      '^port(@[0-7])?$':
> >>
> >> I see only two ports in the example. How many are there in reality?
> > Existing projects can have a maximum of two ports. I used the range 0-7 as I consider
> > it unlikely to have more than 8 ports. Maybe it's intended as a large buffer for
> > futher design needs.
> 
> Please do not create buffers in the bindings. This must be specific, so [01]

Got it. I will update it to '^port(@[0-1])?$' in next version.

> 
> 

Thanks,
Jie
Krzysztof Kozlowski Aug. 19, 2024, 9:51 a.m. UTC | #9
On 19/08/2024 10:51, JieGan wrote:
> On Mon, Aug 19, 2024 at 08:25:33AM +0200, Krzysztof Kozlowski wrote:
>> On 19/08/2024 03:49, JieGan wrote:
>>> On Sun, Aug 18, 2024 at 08:28:34AM -0600, Rob Herring wrote:
>>>> On Mon, Aug 12, 2024 at 10:41:39AM +0800, Jie Gan wrote:
>>>>> Add binding file to specify how to define a Coresight TMC
>>>>> Control Unit device in device tree.
>>>>>
>>>>> It is responsible for controlling the data filter function
>>>>> based on the source device's Trace ID for TMC ETR device.
>>>>> The trace data with that Trace id can get into ETR's buffer
>>>>> while other trace data gets ignored.
>>>>>
>>>>> Signed-off-by: Jie Gan <quic_jiegan@quicinc.com>
>>>>> ---
>>>>>  .../bindings/arm/qcom,coresight-ctcu.yaml     | 79 +++++++++++++++++++
>>>>>  1 file changed, 79 insertions(+)
>>>>>  create mode 100644 Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml
>>>>> new file mode 100644
>>>>> index 000000000000..7a9580007942
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml
>>>>> @@ -0,0 +1,79 @@
>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>>> +%YAML 1.2
>>>>> +---
>>>>> +$id: http://devicetree.org/schemas/arm/qcom,coresight-ctcu.yaml#
>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>> +
>>>>> +title: CoreSight TMC Control Unit
>>>>> +
>>>>> +maintainers:
>>>>> +  - Yuanfang Zhang <quic_yuanfang@quicinc.com>
>>>>> +  - Mao Jinlong <quic_jinlmao@quicinc.com>
>>>>> +  - Jie Gan <quic_jiegan@quicinc.com>
>>>>> +
>>>>> +description:
>>>>> +  The Coresight TMC Control unit controls various Coresight behaviors.
>>>>> +  It works as a helper device when connected to TMC ETR device.
>>>>> +  It is responsible for controlling the data filter function based on
>>>>> +  the source device's Trace ID for TMC ETR device. The trace data with
>>>>> +  that Trace id can get into ETR's buffer while other trace data gets
>>>>> +  ignored.
>>>>
>>>> Nowhere is TMC defined.
>>> The Coresight TMC control unit(CTCU) connected to Coresight TMC device via replicator and
>>> works as a helper device to TMC device.
>>
>> Did you understand the feedback or just responding with whatever to get
>> rid of reviewers?
> 
> Sorry for the insufficient clarity in my response, I am just misunderstood the feedback and try
> to explain the relationship between TMC and CTCU device.
> 
> I will add the TMC description to explain what TMC is as shown below:
> The Trace Memory Controller(TMC) is used for Embedded Trace Buffer(ETB), Embedded Trace FIFO(ETF)
> and Embedded Trace Router(ETR) configurations. The configuration mode (ETB, ETF, ETR) is
> discovered at boot time when the device is probed.

Thanks.

> 
>>
>>>
>>> The in-ports listed below illustrate their connection to TMC devices.
>>>
>>>>
>>>>> +
>>>>> +properties:
>>>>> +  compatible:
>>>>> +    enum:
>>>>> +      - qcom,sa8775p-ctcu
>>>>> +
>>>>> +  reg:
>>>>> +    maxItems: 1
>>>>> +
>>>>> +  clocks:
>>>>> +    maxItems: 1
>>>>> +
>>>>> +  clock-names:
>>>>> +    items:
>>>>> +      - const: apb
>>>>> +
>>>>> +  in-ports:
>>>>
>>>> Use 'ports' unless you have both in and out ports.
>>> The ‘in-ports’ and ‘out-ports’ properties will be parsed by ‘of_coresight_get_port_parent’
>>> and their relationships to other devices will be stored in the coresight_platform_data structure.
>>>
>>> for example:
>>> struct coresight_platform_data {
>>> 	int nr_inconns;
>>> 	int nr_outconns;
>>> 	struct coresight_connection **out_conns;
>>> 	struct coresight_connection **in_conns;
>>> };
>>>
>>> https://elixir.bootlin.com/linux/v6.11-rc4/source/drivers/hwtracing/coresight/coresight-platform.c#L147
>>
>> and? If you respond with some unrelated argument, we will respond with
>> the same: Use 'ports' unless you have both in and out ports.
> 
> Sorry for the insufficient response.
> 
> The Coresight driver prefers using ‘in-ports’ and ‘out-ports’ instead of the ‘ports’ property, as each
> Coresight component needs to specify its input and output directions.
> 
> The Coresight system operates by integrating all Coresight components and construting its data flow path
> based on the defined directions. 
> 
> Consequently, the data flow direction cannot be determined when utilizing the ‘ports’ property in the
> Coresight system.

It can be determined. Driver knows that there are only in-ports, so you
cannot have here other direction. Maybe the drivers have somehow this
hard-coded? But that's a bit annoying limitation.

Best regards,
Krzysztof
Jie Gan Aug. 20, 2024, 6:36 a.m. UTC | #10
On Mon, Aug 19, 2024 at 11:51:41AM +0200, Krzysztof Kozlowski wrote:
> On 19/08/2024 10:51, JieGan wrote:
> > On Mon, Aug 19, 2024 at 08:25:33AM +0200, Krzysztof Kozlowski wrote:
> >> On 19/08/2024 03:49, JieGan wrote:
> >>> On Sun, Aug 18, 2024 at 08:28:34AM -0600, Rob Herring wrote:
> >>>> On Mon, Aug 12, 2024 at 10:41:39AM +0800, Jie Gan wrote:
> >>>>> Add binding file to specify how to define a Coresight TMC
> >>>>> Control Unit device in device tree.
> >>>>>
> >>>>> It is responsible for controlling the data filter function
> >>>>> based on the source device's Trace ID for TMC ETR device.
> >>>>> The trace data with that Trace id can get into ETR's buffer
> >>>>> while other trace data gets ignored.
> >>>>>
> >>>>> Signed-off-by: Jie Gan <quic_jiegan@quicinc.com>
> >>>>> ---
> >>>>>  .../bindings/arm/qcom,coresight-ctcu.yaml     | 79 +++++++++++++++++++
> >>>>>  1 file changed, 79 insertions(+)
> >>>>>  create mode 100644 Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml
> >>>>> new file mode 100644
> >>>>> index 000000000000..7a9580007942
> >>>>> --- /dev/null
> >>>>> +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml
> >>>>> @@ -0,0 +1,79 @@
> >>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >>>>> +%YAML 1.2
> >>>>> +---
> >>>>> +$id: http://devicetree.org/schemas/arm/qcom,coresight-ctcu.yaml#
> >>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>>>> +
> >>>>> +title: CoreSight TMC Control Unit
> >>>>> +
> >>>>> +maintainers:
> >>>>> +  - Yuanfang Zhang <quic_yuanfang@quicinc.com>
> >>>>> +  - Mao Jinlong <quic_jinlmao@quicinc.com>
> >>>>> +  - Jie Gan <quic_jiegan@quicinc.com>
> >>>>> +
> >>>>> +description:
> >>>>> +  The Coresight TMC Control unit controls various Coresight behaviors.
> >>>>> +  It works as a helper device when connected to TMC ETR device.
> >>>>> +  It is responsible for controlling the data filter function based on
> >>>>> +  the source device's Trace ID for TMC ETR device. The trace data with
> >>>>> +  that Trace id can get into ETR's buffer while other trace data gets
> >>>>> +  ignored.
> >>>>
> >>>> Nowhere is TMC defined.
> >>> The Coresight TMC control unit(CTCU) connected to Coresight TMC device via replicator and
> >>> works as a helper device to TMC device.
> >>
> >> Did you understand the feedback or just responding with whatever to get
> >> rid of reviewers?
> > 
> > Sorry for the insufficient clarity in my response, I am just misunderstood the feedback and try
> > to explain the relationship between TMC and CTCU device.
> > 
> > I will add the TMC description to explain what TMC is as shown below:
> > The Trace Memory Controller(TMC) is used for Embedded Trace Buffer(ETB), Embedded Trace FIFO(ETF)
> > and Embedded Trace Router(ETR) configurations. The configuration mode (ETB, ETF, ETR) is
> > discovered at boot time when the device is probed.
> 
> Thanks.
> 
> > 
> >>
> >>>
> >>> The in-ports listed below illustrate their connection to TMC devices.
> >>>
> >>>>
> >>>>> +
> >>>>> +properties:
> >>>>> +  compatible:
> >>>>> +    enum:
> >>>>> +      - qcom,sa8775p-ctcu
> >>>>> +
> >>>>> +  reg:
> >>>>> +    maxItems: 1
> >>>>> +
> >>>>> +  clocks:
> >>>>> +    maxItems: 1
> >>>>> +
> >>>>> +  clock-names:
> >>>>> +    items:
> >>>>> +      - const: apb
> >>>>> +
> >>>>> +  in-ports:
> >>>>
> >>>> Use 'ports' unless you have both in and out ports.
> >>> The ‘in-ports’ and ‘out-ports’ properties will be parsed by ‘of_coresight_get_port_parent’
> >>> and their relationships to other devices will be stored in the coresight_platform_data structure.
> >>>
> >>> for example:
> >>> struct coresight_platform_data {
> >>> 	int nr_inconns;
> >>> 	int nr_outconns;
> >>> 	struct coresight_connection **out_conns;
> >>> 	struct coresight_connection **in_conns;
> >>> };
> >>>
> >>> https://elixir.bootlin.com/linux/v6.11-rc4/source/drivers/hwtracing/coresight/coresight-platform.c#L147
> >>
> >> and? If you respond with some unrelated argument, we will respond with
> >> the same: Use 'ports' unless you have both in and out ports.
> > 
> > Sorry for the insufficient response.
> > 
> > The Coresight driver prefers using ‘in-ports’ and ‘out-ports’ instead of the ‘ports’ property, as each
> > Coresight component needs to specify its input and output directions.
> > 
> > The Coresight system operates by integrating all Coresight components and construting its data flow path
> > based on the defined directions. 
> > 
> > Consequently, the data flow direction cannot be determined when utilizing the ‘ports’ property in the
> > Coresight system.
> 
> It can be determined. Driver knows that there are only in-ports, so you
> cannot have here other direction. Maybe the drivers have somehow this
> hard-coded? But that's a bit annoying limitation.
> 
In Coresight platform driver, the of_coresight_get_port_parent function is used to retrieve the parent of the 'ports' node.
The function is specifically hard-coded to recognize 'in-ports' and 'out-ports'. I think that's the limitation for
'ports' property.

https://elixir.bootlin.com/linux/v6.11-rc4/source/drivers/hwtracing/coresight/coresight-platform.c#L147

Thnaks,
Jie
Krzysztof Kozlowski Aug. 22, 2024, 9:55 a.m. UTC | #11
On 20/08/2024 08:36, JieGan wrote:
> On Mon, Aug 19, 2024 at 11:51:41AM +0200, Krzysztof Kozlowski wrote:
>> On 19/08/2024 10:51, JieGan wrote:
>>> On Mon, Aug 19, 2024 at 08:25:33AM +0200, Krzysztof Kozlowski wrote:
>>>> On 19/08/2024 03:49, JieGan wrote:
>>>>> On Sun, Aug 18, 2024 at 08:28:34AM -0600, Rob Herring wrote:
>>>>>> On Mon, Aug 12, 2024 at 10:41:39AM +0800, Jie Gan wrote:
>>>>>>> Add binding file to specify how to define a Coresight TMC
>>>>>>> Control Unit device in device tree.
>>>>>>>
>>>>>>> It is responsible for controlling the data filter function
>>>>>>> based on the source device's Trace ID for TMC ETR device.
>>>>>>> The trace data with that Trace id can get into ETR's buffer
>>>>>>> while other trace data gets ignored.
>>>>>>>
>>>>>>> Signed-off-by: Jie Gan <quic_jiegan@quicinc.com>
>>>>>>> ---
>>>>>>>  .../bindings/arm/qcom,coresight-ctcu.yaml     | 79 +++++++++++++++++++
>>>>>>>  1 file changed, 79 insertions(+)
>>>>>>>  create mode 100644 Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml
>>>>>>> new file mode 100644
>>>>>>> index 000000000000..7a9580007942
>>>>>>> --- /dev/null
>>>>>>> +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml
>>>>>>> @@ -0,0 +1,79 @@
>>>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>>>>> +%YAML 1.2
>>>>>>> +---
>>>>>>> +$id: http://devicetree.org/schemas/arm/qcom,coresight-ctcu.yaml#
>>>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>>>> +
>>>>>>> +title: CoreSight TMC Control Unit
>>>>>>> +
>>>>>>> +maintainers:
>>>>>>> +  - Yuanfang Zhang <quic_yuanfang@quicinc.com>
>>>>>>> +  - Mao Jinlong <quic_jinlmao@quicinc.com>
>>>>>>> +  - Jie Gan <quic_jiegan@quicinc.com>
>>>>>>> +
>>>>>>> +description:
>>>>>>> +  The Coresight TMC Control unit controls various Coresight behaviors.
>>>>>>> +  It works as a helper device when connected to TMC ETR device.
>>>>>>> +  It is responsible for controlling the data filter function based on
>>>>>>> +  the source device's Trace ID for TMC ETR device. The trace data with
>>>>>>> +  that Trace id can get into ETR's buffer while other trace data gets
>>>>>>> +  ignored.
>>>>>>
>>>>>> Nowhere is TMC defined.
>>>>> The Coresight TMC control unit(CTCU) connected to Coresight TMC device via replicator and
>>>>> works as a helper device to TMC device.
>>>>
>>>> Did you understand the feedback or just responding with whatever to get
>>>> rid of reviewers?
>>>
>>> Sorry for the insufficient clarity in my response, I am just misunderstood the feedback and try
>>> to explain the relationship between TMC and CTCU device.
>>>
>>> I will add the TMC description to explain what TMC is as shown below:
>>> The Trace Memory Controller(TMC) is used for Embedded Trace Buffer(ETB), Embedded Trace FIFO(ETF)
>>> and Embedded Trace Router(ETR) configurations. The configuration mode (ETB, ETF, ETR) is
>>> discovered at boot time when the device is probed.
>>
>> Thanks.
>>
>>>
>>>>
>>>>>
>>>>> The in-ports listed below illustrate their connection to TMC devices.
>>>>>
>>>>>>
>>>>>>> +
>>>>>>> +properties:
>>>>>>> +  compatible:
>>>>>>> +    enum:
>>>>>>> +      - qcom,sa8775p-ctcu
>>>>>>> +
>>>>>>> +  reg:
>>>>>>> +    maxItems: 1
>>>>>>> +
>>>>>>> +  clocks:
>>>>>>> +    maxItems: 1
>>>>>>> +
>>>>>>> +  clock-names:
>>>>>>> +    items:
>>>>>>> +      - const: apb
>>>>>>> +
>>>>>>> +  in-ports:
>>>>>>
>>>>>> Use 'ports' unless you have both in and out ports.
>>>>> The ‘in-ports’ and ‘out-ports’ properties will be parsed by ‘of_coresight_get_port_parent’
>>>>> and their relationships to other devices will be stored in the coresight_platform_data structure.
>>>>>
>>>>> for example:
>>>>> struct coresight_platform_data {
>>>>> 	int nr_inconns;
>>>>> 	int nr_outconns;
>>>>> 	struct coresight_connection **out_conns;
>>>>> 	struct coresight_connection **in_conns;
>>>>> };
>>>>>
>>>>> https://elixir.bootlin.com/linux/v6.11-rc4/source/drivers/hwtracing/coresight/coresight-platform.c#L147
>>>>
>>>> and? If you respond with some unrelated argument, we will respond with
>>>> the same: Use 'ports' unless you have both in and out ports.
>>>
>>> Sorry for the insufficient response.
>>>
>>> The Coresight driver prefers using ‘in-ports’ and ‘out-ports’ instead of the ‘ports’ property, as each
>>> Coresight component needs to specify its input and output directions.
>>>
>>> The Coresight system operates by integrating all Coresight components and construting its data flow path
>>> based on the defined directions. 
>>>
>>> Consequently, the data flow direction cannot be determined when utilizing the ‘ports’ property in the
>>> Coresight system.
>>
>> It can be determined. Driver knows that there are only in-ports, so you
>> cannot have here other direction. Maybe the drivers have somehow this
>> hard-coded? But that's a bit annoying limitation.
>>
> In Coresight platform driver, the of_coresight_get_port_parent function is used to retrieve the parent of the 'ports' node.
> The function is specifically hard-coded to recognize 'in-ports' and 'out-ports'. I think that's the limitation for
> 'ports' property.
> 
> https://elixir.bootlin.com/linux/v6.11-rc4/source/drivers/hwtracing/coresight/coresight-platform.c#L147

That's a limitation of coresight platform driver, not bindings. Fix your
drivers.

Best regards,
Krzysztof
Suzuki K Poulose Aug. 22, 2024, 11:43 a.m. UTC | #12
On 22/08/2024 10:55, Krzysztof Kozlowski wrote:
> On 20/08/2024 08:36, JieGan wrote:
>> On Mon, Aug 19, 2024 at 11:51:41AM +0200, Krzysztof Kozlowski wrote:
>>> On 19/08/2024 10:51, JieGan wrote:
>>>> On Mon, Aug 19, 2024 at 08:25:33AM +0200, Krzysztof Kozlowski wrote:
>>>>> On 19/08/2024 03:49, JieGan wrote:
>>>>>> On Sun, Aug 18, 2024 at 08:28:34AM -0600, Rob Herring wrote:
>>>>>>> On Mon, Aug 12, 2024 at 10:41:39AM +0800, Jie Gan wrote:
>>>>>>>> Add binding file to specify how to define a Coresight TMC
>>>>>>>> Control Unit device in device tree.
>>>>>>>>
>>>>>>>> It is responsible for controlling the data filter function
>>>>>>>> based on the source device's Trace ID for TMC ETR device.
>>>>>>>> The trace data with that Trace id can get into ETR's buffer
>>>>>>>> while other trace data gets ignored.
>>>>>>>>
>>>>>>>> Signed-off-by: Jie Gan <quic_jiegan@quicinc.com>
>>>>>>>> ---
>>>>>>>>   .../bindings/arm/qcom,coresight-ctcu.yaml     | 79 +++++++++++++++++++
>>>>>>>>   1 file changed, 79 insertions(+)
>>>>>>>>   create mode 100644 Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml
>>>>>>>>
>>>>>>>> diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml
>>>>>>>> new file mode 100644
>>>>>>>> index 000000000000..7a9580007942
>>>>>>>> --- /dev/null
>>>>>>>> +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml
>>>>>>>> @@ -0,0 +1,79 @@
>>>>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>>>>>> +%YAML 1.2
>>>>>>>> +---
>>>>>>>> +$id: http://devicetree.org/schemas/arm/qcom,coresight-ctcu.yaml#
>>>>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>>>>> +
>>>>>>>> +title: CoreSight TMC Control Unit
>>>>>>>> +
>>>>>>>> +maintainers:
>>>>>>>> +  - Yuanfang Zhang <quic_yuanfang@quicinc.com>
>>>>>>>> +  - Mao Jinlong <quic_jinlmao@quicinc.com>
>>>>>>>> +  - Jie Gan <quic_jiegan@quicinc.com>
>>>>>>>> +
>>>>>>>> +description:
>>>>>>>> +  The Coresight TMC Control unit controls various Coresight behaviors.
>>>>>>>> +  It works as a helper device when connected to TMC ETR device.
>>>>>>>> +  It is responsible for controlling the data filter function based on
>>>>>>>> +  the source device's Trace ID for TMC ETR device. The trace data with
>>>>>>>> +  that Trace id can get into ETR's buffer while other trace data gets
>>>>>>>> +  ignored.
>>>>>>>
>>>>>>> Nowhere is TMC defined.
>>>>>> The Coresight TMC control unit(CTCU) connected to Coresight TMC device via replicator and
>>>>>> works as a helper device to TMC device.
>>>>>
>>>>> Did you understand the feedback or just responding with whatever to get
>>>>> rid of reviewers?
>>>>
>>>> Sorry for the insufficient clarity in my response, I am just misunderstood the feedback and try
>>>> to explain the relationship between TMC and CTCU device.
>>>>
>>>> I will add the TMC description to explain what TMC is as shown below:
>>>> The Trace Memory Controller(TMC) is used for Embedded Trace Buffer(ETB), Embedded Trace FIFO(ETF)
>>>> and Embedded Trace Router(ETR) configurations. The configuration mode (ETB, ETF, ETR) is
>>>> discovered at boot time when the device is probed.
>>>
>>> Thanks.
>>>
>>>>
>>>>>
>>>>>>
>>>>>> The in-ports listed below illustrate their connection to TMC devices.
>>>>>>
>>>>>>>
>>>>>>>> +
>>>>>>>> +properties:
>>>>>>>> +  compatible:
>>>>>>>> +    enum:
>>>>>>>> +      - qcom,sa8775p-ctcu
>>>>>>>> +
>>>>>>>> +  reg:
>>>>>>>> +    maxItems: 1
>>>>>>>> +
>>>>>>>> +  clocks:
>>>>>>>> +    maxItems: 1
>>>>>>>> +
>>>>>>>> +  clock-names:
>>>>>>>> +    items:
>>>>>>>> +      - const: apb
>>>>>>>> +
>>>>>>>> +  in-ports:
>>>>>>>
>>>>>>> Use 'ports' unless you have both in and out ports.
>>>>>> The ‘in-ports’ and ‘out-ports’ properties will be parsed by ‘of_coresight_get_port_parent’
>>>>>> and their relationships to other devices will be stored in the coresight_platform_data structure.
>>>>>>
>>>>>> for example:
>>>>>> struct coresight_platform_data {
>>>>>> 	int nr_inconns;
>>>>>> 	int nr_outconns;
>>>>>> 	struct coresight_connection **out_conns;
>>>>>> 	struct coresight_connection **in_conns;
>>>>>> };
>>>>>>
>>>>>> https://elixir.bootlin.com/linux/v6.11-rc4/source/drivers/hwtracing/coresight/coresight-platform.c#L147
>>>>>
>>>>> and? If you respond with some unrelated argument, we will respond with
>>>>> the same: Use 'ports' unless you have both in and out ports.
>>>>
>>>> Sorry for the insufficient response.
>>>>
>>>> The Coresight driver prefers using ‘in-ports’ and ‘out-ports’ instead of the ‘ports’ property, as each
>>>> Coresight component needs to specify its input and output directions.
>>>>
>>>> The Coresight system operates by integrating all Coresight components and construting its data flow path
>>>> based on the defined directions.
>>>>
>>>> Consequently, the data flow direction cannot be determined when utilizing the ‘ports’ property in the
>>>> Coresight system.
>>>
>>> It can be determined. Driver knows that there are only in-ports, so you
>>> cannot have here other direction. Maybe the drivers have somehow this
>>> hard-coded? But that's a bit annoying limitation.
>>>
>> In Coresight platform driver, the of_coresight_get_port_parent function is used to retrieve the parent of the 'ports' node.
>> The function is specifically hard-coded to recognize 'in-ports' and 'out-ports'. I think that's the limitation for
>> 'ports' property.
>>
>> https://elixir.bootlin.com/linux/v6.11-rc4/source/drivers/hwtracing/coresight/coresight-platform.c#L147
> 
> That's a limitation of coresight platform driver, not bindings. Fix your
> drivers.

A bit more of context and history.
We have a generic platform parsing code for parsing the firmware tables
(DT and ACPI) in CoreSight. This will parse most generic bits (read, the
graph connections) to avoid duplicating the code. Most of the components
have both in and out connections (except trace sources and other helper
devices like this TMC CU).

Also, the "previous" (now legacy) DT graph bindings for CoreSight relied
on using standard "ports" with a property to indicate whether a given
port is "input". The platform driver still supports this for any legacy
platform that hasn't moved over (of course with a complaint of using
obsolete bindings in the dmesg).

Now the above proposal is going to conflict with the legacy binding
handling, where "ports" indicate output ports by default, without
the obsolete property and the driver can't differentiate this from
a legacy binding vs modern binding with individual driver to decide
the direction of the port. Nothing unfixable, but I don't understand
what is wrong in calling "input ports" in-ports. But if that is
definitely a no go area, we would need to add "fixup" logic in 
individual drivers to "correct" the type of a port (i.e, all output 
ports in the parsed data to input ports) and live with it.

Kind regards
Suzuki






> 
> Best regards,
> Krzysztof
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml
new file mode 100644
index 000000000000..7a9580007942
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml
@@ -0,0 +1,79 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/qcom,coresight-ctcu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: CoreSight TMC Control Unit
+
+maintainers:
+  - Yuanfang Zhang <quic_yuanfang@quicinc.com>
+  - Mao Jinlong <quic_jinlmao@quicinc.com>
+  - Jie Gan <quic_jiegan@quicinc.com>
+
+description:
+  The Coresight TMC Control unit controls various Coresight behaviors.
+  It works as a helper device when connected to TMC ETR device.
+  It is responsible for controlling the data filter function based on
+  the source device's Trace ID for TMC ETR device. The trace data with
+  that Trace id can get into ETR's buffer while other trace data gets
+  ignored.
+
+properties:
+  compatible:
+    enum:
+      - qcom,sa8775p-ctcu
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    items:
+      - const: apb
+
+  in-ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+
+    patternProperties:
+      '^port(@[0-7])?$':
+        description: Input connections from CoreSight Trace bus
+        $ref: /schemas/graph.yaml#/properties/port
+
+required:
+  - compatible
+  - reg
+  - in-ports
+
+additionalProperties: false
+
+examples:
+  - |
+    ctcu@1001000 {
+        compatible = "qcom,sa8775p-ctcu";
+        reg = <0x1001000 0x1000>;
+
+        clocks = <&aoss_qmp>;
+        clock-names = "apb";
+
+        in-ports {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            port@0 {
+                reg = <0>;
+                ctcu_in_port0: endpoint {
+                    remote-endpoint = <&etr0_out_port>;
+                };
+            };
+
+            port@1 {
+                reg = <1>;
+                ctcu_in_port1: endpoint {
+                    remote-endpoint = <&etr1_out_port>;
+                };
+            };
+        };
+    };