Re: [PATCH v3 1/7] dt-bindings: arm: Add support for Coresight TGU trace

[Krzysztof Kozlowski <krzk@kernel.org>]

On Thu, Feb 27, 2025 at 05:26:34PM +0800, songchai wrote:
> From: Songwei Chai <quic_songchai@quicinc.com>
> 
> The Trigger Generation Unit (TGU) is designed to detect patterns or
> sequences within a specific region of the System on Chip (SoC). Once
> configured and activated, it monitors sense inputs and can detect a
> pre-programmed state or sequence across clock cycles, subsequently
> producing a trigger.
> 
>    TGU configuration space
>         offset table
>  x-------------------------x
>  |                         |
>  |                         |
>  |                         |                           Step configuration
>  |                         |                             space layout
>  |   coresight management  |                           x-------------x
>  |        registers        |                     |---> |             |
>  |                         |                     |     |  reserve    |
>  |                         |                     |     |             |
>  |-------------------------|                     |     |-------------|
>  |                         |                     |     | priority[3] |
>  |         step[7]         |<--                  |     |-------------|
>  |-------------------------|   |                 |     | priority[2] |
>  |                         |   |                 |     |-------------|
>  |           ...           |   |Steps region     |     | priority[1] |
>  |                         |   |                 |     |-------------|
>  |-------------------------|   |                 |     | priority[0] |
>  |                         |<--                  |     |-------------|
>  |         step[0]         |-------------------->      |             |
>  |-------------------------|                           |  condition  |
>  |                         |                           |             |
>  |     control and status  |                           x-------------x
>  |           space         |                           |             |
>  x-------------------------x                           |Timer/Counter|
>                                                        |             |
> 						       x-------------x
> TGU Configuration in Hardware
> 
> The TGU provides a step region for user configuration, similar
> to a flow chart. Each step region consists of three register clusters:
> 
> 1.Priority Region: Sets the required signals with priority.
> 2.Condition Region: Defines specific requirements (e.g., signal A
> reaches three times) and the subsequent action once the requirement is
> met.
> 3.Timer/Counter (Optional): Provides timing or counting functionality.
> 
> Add a new coresight-tgu.yaml file to describe the bindings required to
> define the TGU in the device trees.
> 
> Signed-off-by: Songwei Chai <quic_songchai@quicinc.com>
> Signed-off-by: songchai <quic_songchai@quicinc.com>

Don't duplicate yourself.

Anyway, this is marked as v3, I cannot find previous versions, no
changelog, no references.

What happened here in this binding?

> ---
>  .../bindings/arm/qcom,coresight-tgu.yaml      | 135 ++++++++++++++++++
>  1 file changed, 135 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/qcom,coresight-tgu.yaml
> 
> diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-tgu.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-tgu.yaml
> new file mode 100644
> index 000000000000..a41ac68a4fe7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-tgu.yaml
> @@ -0,0 +1,135 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +# Copyright (c) 2023-2025 Qualcomm Innovation Center, Inc. All rights reserved.

2023 and 2024? Where was it published in these years?

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/arm/qcom,coresight-tgu.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Trigger Generation Unit - TGU
> +
> +description: |
> +  The Trigger Generation Unit (TGU) is a Data Engine which can be utilized
> +  to sense a plurality of signals and create a trigger into the CTI or
> +  generate interrupts to processors. The TGU is like the trigger circuit
> +  of a Logic Analyzer. The corresponding trigger logic can be realized by
> +  configuring the conditions for each step after sensing the signal.
> +  Once setup and enabled, it will observe sense inputs and based upon
> +  the activity of those inputs, even over clock cycles, may detect a
> +  preprogrammed state/sequence and then produce a trigger or interrupt.
> +
> +  The primary use case of the TGU is to detect patterns or sequences on a
> +  given set of signals within some region of the SoC.
> +
> +maintainers:
> +  - Mao Jinlong <quic_jinlmao@quicinc.com>
> +  - Sam Chai <quic_songchai@quicinc.com>
> +
> +# Need a custom select here or 'arm,primecell' will match on lots of nodes
> +select:
> +  properties:
> +    compatible:
> +      contains:
> +        enum:
> +          - qcom,coresight-tgu
> +  required:
> +    - compatible
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: qcom,coresight-tgu
> +      - const: arm,primecell
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    items:
> +      - const: apb_pclk
> +
> +  qcom,tgu-steps:
> +    description:
> +      The trigger logic is realized by configuring each step after sensing
> +      the signal. The parameter here is used to describe the maximum of steps
> +      that could be configured in the current TGU.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 1
> +    maximum: 8
> +
> +  qcom,tgu-regs:
> +    description:
> +      There are some "groups" register clusters in each step, which are used to
> +      configure the signal that we want to detect. Meanwhile, each group has its
> +      own priority, and the priority increases with number of groups. For example,
> +      group3 has a higher priority than group2, the signal configured in group3
> +      will be sensed more preferentially than the signal which is configured in group2.
> +      The parameter here is used to describe the signal number that each group
> +      could be configured.

And all groups are indexed by number? Or do they have names?

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 1
> +    maximum: 18
> +
> +  qcom,tgu-conditions:
> +    description:
> +      A condition sets a specific requirement for a step and defines the subsequent
> +      action once the requirement is met. For example, in step two, if signal A is
> +      detected three times, the process jumps back to step one. The parameter describes
> +      the register number for each functionality, whether it is setting a specific
> +      requirement or defining a subsequent action.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 1
> +    maximum: 4
> +
> +  qcom,tgu-timer-counters:
> +    description:
> +      TGU has timer and counter which are used to set some requirement on each step.

Wrap according to Linux coding style, so at 80.

> +      For example, we could use counter to create a trigger into CTI once TGU senses
> +      the target signal three times.This parameter is used to describe the number of
> +      Timers/Counters in TGU.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0

Drop

> +    maximum: 2
> +
> +  in-ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +    additionalProperties: false
> +
> +    properties:
> +      port:
> +        description: AXI Slave connected to another Coresight component

So this TGU can be connected to anything in coresight graph, no
restrictions?

> +        $ref: /schemas/graph.yaml#/properties/port
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names

Most likely you miss also: in-ports


Best regards,
Krzysztof