[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aef09e24-bdd0-4638-8ddc-55c4d39e37ec@quicinc.com>
Date: Mon, 2 Sep 2024 15:24:13 +0800
From: songchai <quic_songchai@...cinc.com>
To: Krzysztof Kozlowski <krzk@...nel.org>,
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 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.
>
>
>
> Best regards,
> Krzysztof
>
Powered by blists - more mailing lists