[<prev] [next>] [day] [month] [year] [list]
Message-ID: <b6d0a93b-24bd-4af3-9294-7b069e819b14@kernel.org>
Date: Mon, 2 Sep 2024 12:24:48 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: songchai <quic_songchai@...cinc.com>,
Suzuki K Poulose <suzuki.poulose@....com>, Mike Leach
<mike.leach@...aro.org>, 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>
Cc: 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 v1 1/7] dt-bindings: arm: Add support for Coresight TGU
trace
On 02/09/2024 12:10, songchai wrote:
>
> On 9/2/2024 4:05 PM, Krzysztof Kozlowski wrote:
>> 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@...cinc.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@...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:
>>>>>>> + $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?
>
> yes, each TGU has a different number of registers.
>
> For example, some TGUs support 6 steps, while others support only 4 steps.
>
> These variations depend on the hardware design, so we need properties to
> describe.
You keep avoiding the answer. ON THE SAME SOC.
Point to your upstream DTS using this.
Best regards,
Krzysztof
Powered by blists - more mailing lists