Message-ID: <CAJ9a7VjOU2d0tGjvbMBWwWtjKp38hj_NptVYFHDy9Zrs-sdOeg@mail.gmail.com>
Date: Fri, 7 Mar 2025 09:28:29 +0000
From: Mike Leach <mike.leach@...aro.org>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: songchai <quic_songchai@...cinc.com>, Suzuki K Poulose <suzuki.poulose@....com>, 
	James Clark <james.clark@....com>, 
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Andy Gross <agross@...nel.org>, 
	Bjorn Andersson <andersson@...nel.org>, Rob Herring <robh+dt@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, linux-kernel@...r.kernel.org, 
	coresight@...ts.linaro.org, linux-arm-kernel@...ts.infradead.org, 
	linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH v3 1/7] dt-bindings: arm: Add support for Coresight TGU trace

Hi,

On Tue, 4 Mar 2025 at 08:05, Krzysztof Kozlowski <krzk@...nel.org> wrote:
>
> On Thu, Feb 27, 2025 at 05:26:34PM +0800, songchai wrote:
> > From: Songwei Chai <quic_songchai@...cinc.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@...cinc.com>
> > Signed-off-by: songchai <quic_songchai@...cinc.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@...cinc.com>
> > +  - Sam Chai <quic_songchai@...cinc.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
> > +

Hardware features are usually defined by ID registers in coresight
devices. e.g. ETM has a number of ID registers that describe the
number of comparators / counters etc.
Does this device not have similar registers? Is there not a unique ID
for each hardware variant - hardware discoverablility is an
architecture requirement for coresight devices?

> > +  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?
>


Coresight uses APB for register access and ATB for moving trace from
source to sink. The only use of AXI is on the ETR/CATU output saving
trace data into system memory.

> > +        $ref: /schemas/graph.yaml#/properties/port
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - clock-names
>
> Most likely you miss also: in-ports
>
>
> Best regards,
> Krzysztof
>

Regards

Mike

-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK

Powered by blists - more mailing lists