Message-ID: <95ac571a-1c8f-45d9-9874-648d4123ce18@arm.com>
Date: Fri, 19 Dec 2025 09:54:14 +0000
From: Suzuki K Poulose <suzuki.poulose@....com>
To: Jie Gan <jie.gan@....qualcomm.com>, Krzysztof Kozlowski
 <krzk@...nel.org>, Rob Herring <robh@...nel.org>
Cc: Mike Leach <mike.leach@...aro.org>, James Clark <james.clark@...aro.org>,
 Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Tingwei Zhang <tingwei.zhang@....qualcomm.com>,
 Mao Jinlong <jinlong.mao@....qualcomm.com>,
 Bjorn Andersson <andersson@...nel.org>,
 Konrad Dybcio <konradybcio@...nel.org>, coresight@...ts.linaro.org,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH v8 5/8] dt-bindings: arm: add an interrupt property for
 Coresight CTCU

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@...aro.org>
>>>>>> Reviewed-by: Mike Leach <mike.leach@...aro.org>
>>>>>> Signed-off-by: Jie Gan <jie.gan@....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@...0 → interrupt line 0
> tmc@...1 → 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
>>
> 

Powered by blists - more mailing lists