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

[Krzysztof Kozlowski <krzk@kernel.org>]

On 02/09/2024 09:24, songchai wrote:
> 
> On 9/2/2024 3:02 PM, Krzysztof Kozlowski wrote:
>> On 02/09/2024 05:14, songchai wrote:
>>> On 8/30/2024 6:11 PM, Krzysztof Kozlowski wrote:
>>>> On 30/08/2024 11:23, songchai wrote:
>>>>> 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    |
>>>>>    |                         |                     |     |             |
>>>>>    |-------------------------|                     |     |-------------|
>>>>>    |                         |                     |     | prioroty[3] |
>>>>>    |         step[7]         |<--                  |     |-------------|
>>>>>    |-------------------------|   |                 |     | prioroty[2] |
>>>>>    |                         |   |                 |     |-------------|
>>>>>    |           ...           |   |Steps region     |     | prioroty[1] |
>>>>>    |                         |   |                 |     |-------------|
>>>>>    |-------------------------|   |                 |     | prioroty[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: songchai<quic_songchai@quicinc.com>
>>>> It feels like you are using login name as real name. Please investigate
>>>> this and confirm whether latin transcription/transliteration of your
>>>> name is like above.
>>> yes.. it's my login name. Will use my real name in next version.
>>>>> ---
>>>>>    .../bindings/arm/qcom,coresight-tgu.yaml      | 136 ++++++++++++++++++
>>>>>    1 file changed, 136 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..c261252e33e0
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-tgu.yaml
>>>>> @@ -0,0 +1,136 @@
>>>>> +# 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-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:
>>>>> +  $nodename:
>>>>> +    pattern: "^tgu(@[0-9a-f]+)$"
>>>> Drop the pattern (and anyway @ is not optional).
>>> There will be several TGUs in the SoC, and we want to use the address of
>>> each to distinguish them.
>> How is this related?
>>
>>> This is why we added the nodename pattern here.
>> How is this related?
>>
>>> Additionally, I noticed that both TPDM and CTI also use this format to
>>> define the nodename.
>>>
>>> Could you please share any concerns you have about using this pattern?
>> I don't get why you insist, but sure:
>> The name does not follow DT spec recommendation, plus child schema is
>> not really supposed to define the names.
> Thanks for you clarification, will drop in the next version.
>>
>>>>> +  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.
>>>> Why this is board or SoC level property? All below also feel like
>>>> unnecessary stuff from downstream.
>>> There are actually four properties used to describe the number of
>>> registers with different functionality for TGUs at the SoC level.
>>>
>>> Each TGU may have a different number of registers, so we need to add
>>> these four properties to describe each TGU’s hardware design.
>> Each TGU on the same SoC?
> yes, in other words, there will be several TGUs in the SoC.

This I understood, but I am asking if each TGU on the same SoC will have
different number of registers and other properties?

Best regards,
Krzysztof