Re: [PATCH v3 RESEND 00/10] coresight: ctcu: Enable byte-cntr function for TMC ETR

[Jie Gan <jie.gan@oss.qualcomm.com>]

On 7/22/2025 11:09 PM, Mike Leach wrote:
> Hi
> 

Hi Mike

Thanks for your comments.

> I have had a look at a few of the patches. The buffer swap mechanism
> appears to be good.
> 
> However there is a lot of byte-ctr code in the "core" tmc / etr source
> files. As much of this code as possible needs to be moved to the
> byte-cntr specifc source.
> 

I will try move byte-cntr related codes to the ctcu-byte-cntr specific 
source file.

> I suggest having a helper function such as qcom_byte_ctr_in_use() to
> call from the core code, and if true then call back into the byte-cntr
> specific code to do the specialist functionality.

Sounds a good idea, call the prepare/read function regards to whether 
there is a valid helper callback defined. I will try this solution in 
next version.

I have a minor question about the solution. We expect enable/disable the 
byte-cntr function at anytime. So when we jump to the byte-cntr 
function, we have to check whether the byte-cntr function is enabled or 
not first, jump back to the standard workflow if not, am right?

> 
> One other possibility is to have a flag / enum in the tmc->drvdata
> structure to indicate a variant. - e.g. TMC_STD, TMC_QCOM_BYTE_CTR,
> set at initialisation stage to remove the need for checking the device
> tree every call.
> 

Also will check once, see which solution is better.

Thanks,
Jie

> Regards
> 
> Mike
> 
> On Mon, 14 Jul 2025 at 07:31, Jie Gan <jie.gan@oss.qualcomm.com> wrote:
>>
>> The byte-cntr function provided by the CTCU device is used to count the
>> trace data entering the ETR. An interrupt is triggered if the data size
>> exceeds the threshold set in the BYTECNTRVAL register. The interrupt
>> handler counts the number of triggered interruptions.
>>
>> Based on this concept, the irq_cnt can be used to determine whether
>> the etr_buf is full. The ETR device will be disabled when the active
>> etr_buf is nearly full or a timeout occurs. The nearly full buffer will
>> be switched to background after synced. A new buffer will be picked from
>> the etr_buf_list, then restart the ETR device.
>>
>> The byte-cntr reading functions can access data from the synced and
>> deactivated buffer, transferring trace data from the etr_buf to userspace
>> without stopping the ETR device.
>>
>> The byte-cntr read operation has integrated with the file node tmc_etr,
>> for example:
>> /dev/tmc_etr0
>> /dev/tmc_etr1
>>
>> There are two scenarios for the tmc_etr file node with byte-cntr function:
>> 1. BYTECNTRVAL register is configured and byte-cntr is enabled -> byte-cntr read
>> 2. BYTECNTRVAL register is reset or byte-cntr is disabled -> original behavior
>>
>> Shell commands to enable byte-cntr reading for etr0:
>> echo 0x10000 > /sys/bus/coresight/devices/ctcu0/irq_val
>> echo 1 > /sys/bus/coresight/devices/tmc_etr0/enable_sink
>> echo 1 > /sys/bus/coresight/devices/etm0/enable_source
>> cat /dev/tmc_etr0
>>
>> Reset the BYTECNTR register for etr0:
>> echo 0 > /sys/bus/coresight/devices/ctcu0/irq_val
>>
>> Changes in V3 resend:
>> 1. rebased on next-20250711.
>> Link to V3 - https://lore.kernel.org/all/20250624060438.7469-1-jie.gan@oss.qualcomm.com/
>>
>> Changes in V3:
>> 1. The previous solution has been deprecated.
>> 2. Add a etr_buf_list to manage allcated etr buffers.
>> 3. Add a logic to switch buffer for ETR.
>> 4. Add read functions to read trace data from synced etr buffer.
>> Link to V2 - https://lore.kernel.org/all/20250410013330.3609482-1-jie.gan@oss.qualcomm.com/
>>
>> Changes in V2:
>> 1. Removed the independent file node /dev/byte_cntr.
>> 2. Integrated the byte-cntr's file operations with current ETR file
>>     node.
>> 3. Optimized the driver code of the CTCU that associated with byte-cntr.
>> 4. Add kernel document for the export API tmc_etr_get_rwp_offset.
>> 5. Optimized the way to read the rwp_offset according to Mike's
>>     suggestion.
>> 6. Removed the dependency of the dts patch.
>> Link to V1 - https://lore.kernel.org/all/20250310090407.2069489-1-quic_jiegan@quicinc.com/
>>
>> Jie Gan (10):
>>    coresight: core: Refactoring ctcu_get_active_port and make it generic
>>    coresight: core: add a new API to retrieve the helper device
>>    dt-bindings: arm: add an interrupt property for Coresight CTCU
>>    coresight: ctcu: enable byte-cntr for TMC ETR devices
>>    coresight: tmc: add etr_buf_list to store allocated etr_buf
>>    coresight: tmc: add create/delete functions for etr_buf_node
>>    coresight: tmc: add prepare/unprepare functions for byte-cntr
>>    coresight: tmc: add a switch buffer function for byte-cntr
>>    coresight: tmc: add read function for byte-cntr
>>    arm64: dts: qcom: sa8775p: Add interrupts to CTCU device
>>
>>   .../testing/sysfs-bus-coresight-devices-ctcu  |   5 +
>>   .../bindings/arm/qcom,coresight-ctcu.yaml     |  17 ++
>>   arch/arm64/boot/dts/qcom/sa8775p.dtsi         |   5 +
>>   drivers/hwtracing/coresight/Makefile          |   2 +-
>>   drivers/hwtracing/coresight/coresight-core.c  |  54 ++++
>>   .../coresight/coresight-ctcu-byte-cntr.c      | 102 +++++++
>>   .../hwtracing/coresight/coresight-ctcu-core.c | 113 ++++++--
>>   drivers/hwtracing/coresight/coresight-ctcu.h  |  49 +++-
>>   drivers/hwtracing/coresight/coresight-priv.h  |   4 +
>>   .../hwtracing/coresight/coresight-tmc-core.c  |  70 ++++-
>>   .../hwtracing/coresight/coresight-tmc-etr.c   | 270 ++++++++++++++++++
>>   drivers/hwtracing/coresight/coresight-tmc.h   |  29 ++
>>   12 files changed, 688 insertions(+), 32 deletions(-)
>>   create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight-devices-ctcu
>>   create mode 100644 drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c
>>
>> --
>> 2.34.1
>>
> 
>