Message-ID: <01072c94-3a13-459c-baba-0cc2f3611ff9@oss.qualcomm.com>
Date: Mon, 22 Dec 2025 17:17:48 +0800
From: Jie Gan <jie.gan@....qualcomm.com>
To: Suzuki K Poulose <suzuki.poulose@....com>,
        Krzysztof Kozlowski <krzk@...nel.org>, Rob Herring <robh@...nel.org>,
        Mike Leach <mike.leach@...aro.org>,
        Tingwei Zhang <tingwei.zhang@....qualcomm.com>
Cc: 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>,
        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 12/22/2025 2:58 PM, Jie Gan wrote:
> 
> 
> On 12/19/2025 5:54 PM, Suzuki K Poulose wrote:
>> 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 ?
>>
> 
> Each interrupt is dedicated to a specific ETR device. While we can 
> retrieve the list of interrupts using of_irq_get, we cannot guarantee 
> that the obtained interrupt corresponds to the correct ETR device?
> I believe it would be better to have an interrupt-name property, so we 
> can assign the name in the data structure and retrieve the interrupt by 
> its name, ensuring it maps to the correct ETR device.

I have tried to get the interrupt with of_irq_get(nd, port_number).

If we define the interrupts in the correct sequence aligned with the 
port numbers, we can then retrieve each interrupt by its corresponding 
port number.

Is that ok to drop the interrupt-name with the solution?

Thanks,
Jie

> 
> Thanks,
> Jie
> 
> 
> 

Powered by blists - more mailing lists