linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Jie Gan <jie.gan@oss.qualcomm.com>
To: Suzuki K Poulose <suzuki.poulose@arm.com>,
	Krzysztof Kozlowski <krzk@kernel.org>,
	Rob Herring <robh@kernel.org>, Mike Leach <mike.leach@linaro.org>,
	Tingwei Zhang <tingwei.zhang@oss.qualcomm.com>
Cc: 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>,
	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: Mon, 22 Dec 2025 17:17:48 +0800	[thread overview]
Message-ID: <01072c94-3a13-459c-baba-0cc2f3611ff9@oss.qualcomm.com> (raw)
In-Reply-To: <097dbefb-f679-40da-bce4-d6e09dc4cf08@oss.qualcomm.com>



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@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 ?
>>
> 
> 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
> 
> 
> 



  reply	other threads:[~2025-12-22  9:18 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
2025-12-22  6:58               ` Jie Gan
2025-12-22  9:17                 ` Jie Gan [this message]
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=01072c94-3a13-459c-baba-0cc2f3611ff9@oss.qualcomm.com \
    --to=jie.gan@oss.qualcomm.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=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=suzuki.poulose@arm.com \
    --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;
as well as URLs for NNTP newsgroup(s).