Linux ARM-MSM sub-architecture
 help / color / mirror / Atom feed
From: Suzuki K Poulose <suzuki.poulose@arm.com>
To: Jie Gan <jie.gan@oss.qualcomm.com>,
	Krzysztof Kozlowski <krzk@kernel.org>,
	Rob Herring <robh@kernel.org>
Cc: Mike Leach <mike.leach@linaro.org>,
	James Clark <james.clark@linaro.org>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Tingwei Zhang <tingwei.zhang@oss.qualcomm.com>,
	Mao Jinlong <jinlong.mao@oss.qualcomm.com>,
	Bjorn Andersson <andersson@kernel.org>,
	Konrad Dybcio <konradybcio@kernel.org>,
	coresight@lists.linaro.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	devicetree@vger.kernel.org
Subject: Re: [PATCH v8 5/8] dt-bindings: arm: add an interrupt property for Coresight CTCU
Date: Fri, 19 Dec 2025 09:54:14 +0000	[thread overview]
Message-ID: <95ac571a-1c8f-45d9-9874-648d4123ce18@arm.com> (raw)
In-Reply-To: <46afd4f6-f287-4d19-bc68-f2f6eac36e6a@oss.qualcomm.com>

On 19/12/2025 02:05, Jie Gan wrote:
> 
> 
> On 12/19/2025 7:19 AM, Suzuki K Poulose wrote:
>> On 18/12/2025 10:17, Krzysztof Kozlowski wrote:
>>> On 12/12/2025 02:12, Jie Gan wrote:
>>>>
>>>>
>>>> On 12/11/2025 9:37 PM, Rob Herring wrote:
>>>>> On Thu, Dec 11, 2025 at 02:10:44PM +0800, Jie Gan wrote:
>>>>>> Add an interrupt property to CTCU device. The interrupt will be 
>>>>>> triggered
>>>>>> when the data size in the ETR buffer exceeds the threshold of the
>>>>>> BYTECNTRVAL register. Programming a threshold in the BYTECNTRVAL 
>>>>>> register
>>>>>> of CTCU device will enable the interrupt.
>>>>>>
>>>>>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>>>> Reviewed-by: Mike Leach <mike.leach@linaro.org>
>>>>>> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
>>>>>> ---
>>>>>>    .../devicetree/bindings/arm/qcom,coresight-ctcu.yaml    | 17 ++ 
>>>>>> + ++++++++++++++
>>>>>>    1 file changed, 17 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight- 
>>>>>> ctcu.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight- 
>>>>>> ctcu.yaml
>>>>>> index c969c16c21ef..90f88cc6cd3e 100644
>>>>>> --- a/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml
>>>>>> +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml
>>>>>> @@ -39,6 +39,16 @@ properties:
>>>>>>        items:
>>>>>>          - const: apb
>>>>>> +  interrupts:
>>>>>> +    items:
>>>>>> +      - description: Byte cntr interrupt for the first etr device
>>>>>> +      - description: Byte cntr interrupt for the second etr device
>>
>> This is really vague. How do you define first vs second ? Probe order ?
>> No way. This must be the "port" number to which the ETR is connected
>> to the CTCU. IIUC, there is a config area for each ETR (e.g., trace id
>> filter) connected to the CTCU. I was under the assumption that they
>> are identified as "ports" (input ports). I don't really understand how
>> this interrupt mapping works now. Please explain it clearly.
>>
> 
> Sorry for the misunderstanding.
> 
> Each ETR device should have its own interrupt line and an IRQ register 
> within the CTCU device, as defined by the specification. In existing 
> projects, the maximum supported number of ETR devices is 2.
> 
> Each interrupt is directly mapped to a specific ETR device, for example:
> tmc@1000 → interrupt line 0
> tmc@1001 → interrupt line 1
> 
> The suggestion to identify devices by ‘ports’ is much clearer than my 
> previous explanation, as it explicitly shows which device is connected 
> to which port.

Thanks for confirming.

> 
>>>>>> +
>>>>>> +  interrupt-names:
>>>>>> +    items:
>>>>>> +      - const: etrirq0
>>>>>> +      - const: etrirq1
>>>>>
>>>>> Names are kind of pointless when it is just foo<index>.
>>>>
>>>> Hi Rob,
>>>>
>>>> I was naming them as etr0/etr1. Are these names acceptable?
>>>
>>> Obviously irq is redundant, but how does etr0 solves the problem of
>>> calling it foo0?
>>>
>>> I don't think you really read Rob's comment.
>>>
>>>> The interrupts are assigned exclusively to a specific ETR device.
>>>>
>>>> But Suzuki is concerned that this might cause confusion because the ETR
>>>> device is named randomly in the driver. Suzuki suggested using ‘port-0’
>>>> and ‘port-1’ and would also like to hear your feedback on these names.
>>>
>>> There is no confusion here. Writing bindings luckily clarifies this what
>>> the indices in the array mean.
>>
>> The point is there are "n" interrupts. Question is, could there be more
>> devices(ETRs) connected to the CTCU than "n".
>>
>> e.g., Lets CTCU can control upto 4 ETRs and on a particular system, the
>>
>> TMC-ETR0 -> CTCU-Port0
>>
>> TMC-ETR1 -> CTCU-Port2
>> TMC-ETR2 -> CTCU-Port3
>>
>> Now, how many interrupts are described in the DT ? How do we map which
>> interrupts correspond to the CTCU-Portn. (Finding the TMC-ETRx back
>> from the port is possible, with the topology).
>>
> 
> Got your point and it's much clearer.
> 
>> This is what I raised in the previous version. Again, happy to hear
>> if there is a standard way to describe the interrupts.
>>
>> Suzuki
>>
>>
>>>
>>>>
>>>> Usually, the probe sequence follows the order of the addresses. In our
>>>> specification, ‘ETR0’ is always probed before ‘ETR1’ because its 
>>>> address
>>>> is lower.
>>>
>>> How is this even relevant? You are answering to something completely
>>> different, so I don't think you really tried to understand review.
>>>
> 
> My previous explanation was definitely unclear. As Suzuki suggested, 
> mapping the interrupt to the port number (to identify the relevant 
> device based on topology) makes sense and provides a much easier way to 
> understand the relationship between the interrupt and the ETR device.
> 
> So with the suggestion, here is the new description about the interrupts:
> 
>    interrupts:
>      items:
>        - description: Interrupt for the ETR device connected to in-port0.
>        - description: Interrupt for the ETR device connected to in-port1.
> 
>   interrupt-names:
>      items:
>       - const: port0
>       - const: port1

Which brings us back to the question I posted in the previous version. 
Do we really need a "name" or are there other ways to define, a sparse
list of interrupts ?

Suzuki


> 
> Thanks,
> Jie
> 
>>>
>>>
>>> Best regards,
>>> Krzysztof
>>
> 


  reply	other threads:[~2025-12-19  9:54 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-11  6:10 [PATCH v8 0/8] coresight: ctcu: Enable byte-cntr function for TMC ETR Jie Gan
2025-12-11  6:10 ` [PATCH v8 1/8] coresight: core: Refactoring ctcu_get_active_port and make it generic Jie Gan
2025-12-11  6:10 ` [PATCH v8 2/8] coresight: core: add a new API to retrieve the helper device Jie Gan
2025-12-13 12:38   ` kernel test robot
2025-12-18 17:50   ` Suzuki K Poulose
2025-12-19  9:24     ` Jie Gan
2025-12-11  6:10 ` [PATCH v8 3/8] coresight: tmc: add create/clean functions for etr_buf_list Jie Gan
2025-12-11  6:10 ` [PATCH v8 4/8] coresight: tmc: Introduce sysfs_read_ops to wrap sysfs read operations Jie Gan
2025-12-11  6:10 ` [PATCH v8 5/8] dt-bindings: arm: add an interrupt property for Coresight CTCU Jie Gan
2025-12-11  7:27   ` Rob Herring (Arm)
2025-12-11  7:35     ` Jie Gan
2025-12-11 13:37   ` Rob Herring
2025-12-12  1:12     ` Jie Gan
2025-12-18 10:17       ` Krzysztof Kozlowski
2025-12-18 23:19         ` Suzuki K Poulose
2025-12-19  2:05           ` Jie Gan
2025-12-19  9:54             ` Suzuki K Poulose [this message]
2025-12-22  6:58               ` Jie Gan
2025-12-22  9:17                 ` Jie Gan
2025-12-11  6:10 ` [PATCH v8 6/8] coresight: ctcu: enable byte-cntr for TMC ETR devices Jie Gan
2025-12-11  6:10 ` [PATCH v8 7/8] coresight: tmc: integrate byte-cntr's read_ops with sysfs file_ops Jie Gan
2025-12-11  6:10 ` [PATCH v8 8/8] arm64: dts: qcom: lemans: Add interrupts to CTCU device Jie Gan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=95ac571a-1c8f-45d9-9874-648d4123ce18@arm.com \
    --to=suzuki.poulose@arm.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=andersson@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=coresight@lists.linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=james.clark@linaro.org \
    --cc=jie.gan@oss.qualcomm.com \
    --cc=jinlong.mao@oss.qualcomm.com \
    --cc=konradybcio@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=krzk@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mike.leach@linaro.org \
    --cc=robh@kernel.org \
    --cc=tingwei.zhang@oss.qualcomm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox