Message-ID: <d06214d3-1044-4455-b5c2-f28e26fec696@quicinc.com>
Date: Tue, 3 Jun 2025 09:44:46 +0800
From: Jie Gan <quic_jiegan@...cinc.com>
To: Mike Leach <mike.leach@...aro.org>, Jie Gan <jie.gan@....qualcomm.com>
CC: Suzuki K Poulose <suzuki.poulose@....com>,
        James Clark
	<james.clark@...aro.org>,
        Alexander Shishkin
	<alexander.shishkin@...ux.intel.com>,
        Maxime Coquelin
	<mcoquelin.stm32@...il.com>,
        Alexandre Torgue <alexandre.torgue@...s.st.com>,
        Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konradybcio@...nel.org>,
        Tingwei Zhang
	<quic_tingweiz@...cinc.com>,
        Jinlong Mao <quic_jinlmao@...cinc.com>, <coresight@...ts.linaro.org>,
        <linux-arm-kernel@...ts.infradead.org>, <linux-kernel@...r.kernel.org>,
        <devicetree@...r.kernel.org>, <linux-arm-msm@...r.kernel.org>,
        <linux-stm32@...md-mailman.stormreply.com>
Subject: Re: [PATCH v2 0/5] coresight: ctcu: Enable byte-cntr function for TMC
 ETR



On 4/11/2025 6:16 PM, Mike Leach wrote:
> Hi,
> 
> I can see that this patchset has fixed some of the issues raised from
> v1 - using the existing file handle for read, and stopping the ETR to
> read the RWP.
> 
> However the fundamental problem that it is still attempting to read
> memory without stopping the ETR has not been addressed.
> 
> As mentioned in mine and Suzuki's comments for v1, this means:-
> 
> 1) you cannot guarantee that the buffer has not wrapped when reading
> data back, which will corrupt the trace decode process.
> 2) The DMA buffers are not being synchronized, so the PE could be
> reading stale data rather than the new data written by the ETR.
> 

Sorry for the late reply. I am trying to gather more detailed 
information about the byte-cntr register from our hardware team.

I have confirmed the behavior of the byte-cntr function:
The byte counter counts the number of bytes that are getting written 
into the ETR fifo & generates the interrupt once this matches the 
programmed value.

You are right, the byte-cntr cannot ensure safe reads from etr buffer 
while the ETR is running, so my previous solution will be deprecated.

I will consider another safe solution for byte-cntr.

Thanks,
Jie

> 
> Regards
> 
> Mike
> 
> On Thu, 10 Apr 2025 at 02:33, Jie Gan <jie.gan@....qualcomm.com> wrote:
>>
>> The byte-cntr function provided by the CTCU device is used to transfer data
>> from the ETR buffer to the userspace. An interrupt is tiggered if the data
>> size exceeds the threshold set in the BYTECNTRVAL register. The interrupt
>> handler counts the number of triggered interruptions and the read function
>> will read the data from the ETR buffer if the IRQ count is greater than 0.
>> The read work will be conducted ASAP after the byte-cntr is started.
>> Each successful read process will decrement the IRQ count by 1.
>>
>> The byte cntr function will start when the device node is opened for reading,
>> and the IRQ count will reset when the byte cntr function has stopped. When
>> the file node is opened, the w_offset of the ETR buffer will be read and
>> stored in byte_cntr_data, serving as the original r_offset (indicating
>> where reading starts) for the byte counter function.
>>
>> The work queue for the read operation will wake up once when ETR is stopped,
>> ensuring that the remaining data in the ETR buffer has been flushed based on
>> the w_offset read at the time of stopping.
>>
>> The byte-cntr read work has integrated with the file node tmc_etr, e.g.
>> /dev/tmc_etr0
>> /dev/tmc_etr1
>>
>> There are two scenarios for the ETR file nodes with byte-cntr function:
>> 1. BYTECNTRVAL register has configured -> byte-cntr read
>> 2. BYTECNTRVAL register is disabled -> original behavior, flush the etr_buf
>>
>> We still can flush the etr buffer once after the byte-cntr function has
>> triggered.
>> 1. Enable byte-cntr
>> 2. Byte-cntr read
>> 3. Disable byte-cntr
>> 4. Flush etr buffer
>>
>> Since the ETR operates in circular buffer mode, we cannot fully guarantee
>> that no overwrites occur when the byte-cntr read function reads the data.
>> The read function will read the data ASAP when the interrupt is
>> triggered and we should not configure a threshold greater than the
>> buffer size of the ETR buffer.
>>
>> The following shell commands write threshold to BYTECNTRVAL registers.
>>
>> Only enable byte-cntr for ETR0:
>> echo 0x10000 > /sys/devices/platform/soc@...001000.ctcu/ctcu0/byte_cntr_val
>>
>> Enable byte-cntr for both ETR0 and ETR1(support both hex and decimal values):
>> echo 0x10000 4096 > /sys/devices/platform/soc@...001000.ctcu/ctcu0/byte_cntr_val
>>
>> Setting the BYTECNTRVAL registers to 0 disables the byte-cntr function.
>> Disable byte-cntr for ETR0:
>> echo 0 > /sys/devices/platform/soc@...001000.ctcu/ctcu0/byte_cntr_val
>>
>> Disable byte-cntr for both ETR0 and ETR1:
>> echo 0 0 > /sys/devices/platform/soc@...001000.ctcu/ctcu0/byte_cntr_val
>>
>> There is a minimum threshold to prevent generating too many interrupts.
>> The minimum threshold is 4096 bytes. The write process will fail if user try
>> to set the BYTECNTRVAL registers to a value less than 4096 bytes(except
>> for 0).
>>
>> Way to enable and start byte-cntr for ETR0:
>> echo 0x10000 > /sys/devices/platform/soc@...001000.ctcu/ctcu0/byte_cntr_val
>> echo 1 > /sys/bus/coresight/devices/tmc_etr0/enable_sink
>> echo 1 > /sys/bus/coresight/devices/etm0/enable_source
>> cat /dev/tmc_etr0
>>
>> Testing case has conducted for the byte-cntr read work:
>> 1. Setting the buffer_size of the ETR as large as possile, here is for ETR0
>>     echo 0x1000000 > /sys/bus/coresight/devices/tmc_etr0/buffer_size
>> 2. Setting the threshold for the ETR0 to 0x10000
>>     echo 0x10000 > /sys/bus/coresight/devices/ctcu0/byte_cntr_val
>> 3. Enable ETR0
>>     echo 1 > /sys/bus/coresight/devices/tmc_etr0/enable_sink
>> 4. Enable ETM0 as source and enable byte-cntr to read data
>>     echo 1 > /sys/bus/coresight/devices/etm0/enable_source;
>>     cat /dev/tmc_etr0 > /tmp/file_byte_cntr.bin &
>> 5. Disable ETM0
>>     echo 0 > /sys/bus/coresight/devices/etm0/enable_source
>> 6. Disable byte-cntr and flush the etr buffer
>>     echo 0 > /sys/bus/coresight/devices/ctcu0/byte_cntr_val;
>>     cat /dev/tmc_etr0 > /tmp/file_etr0.bin
>>     ls -l /tmp
>>
>> -rw-r--r--    1 root     root      12628960 Apr 28 17:44 file_byte_cntr.bin
>> -rw-r--r--    1 root     root      12669296 Apr 28 17:45 file_etr0.bin
>>
>> 7. Deal with the file_etr0.bin with following command:
>>     dd if=/tmp/file_etr0.bin of=/tmp/file_etr0_aligned.bin bs=1
>> count=12628960 skip=40336
>>     ls -l /tmp
>>
>> -rw-r--r--    1 root     root      12628960 Apr 28 17:44 file_byte_cntr.bin
>> -rw-r--r--    1 root     root      12669296 Apr 28 17:45 file_etr0.bin
>> -rw-r--r--    1 root     root      12628960 Apr 28 17:49 file_etr0_aligned.bin
>>
>> 8. Compared file_byte_cntr.bin with file_etr0_aligned.bin and identified
>> they are competely same.
>>     diff file_byte_cntr.bin file_etr0_aligned.bin
>>
>> =======================
>> 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 (5):
>>    coresight: tmc: Introduce new APIs to get the RWP offset of ETR buffer
>>    dt-bindings: arm: Add an interrupt property for Coresight CTCU
>>    coresight: ctcu: Enable byte-cntr for TMC ETR devices
>>    coresight: tmc: add functions for byte-cntr operation
>>    arm64: dts: qcom: sa8775p: Add interrupts to CTCU device
>>
>>   .../bindings/arm/qcom,coresight-ctcu.yaml     |  17 ++
>>   arch/arm64/boot/dts/qcom/sa8775p.dtsi         |   5 +
>>   drivers/hwtracing/coresight/Makefile          |   2 +-
>>   .../coresight/coresight-ctcu-byte-cntr.c      | 119 ++++++++++++
>>   .../hwtracing/coresight/coresight-ctcu-core.c |  88 ++++++++-
>>   drivers/hwtracing/coresight/coresight-ctcu.h  |  49 ++++-
>>   .../hwtracing/coresight/coresight-tmc-core.c  |  29 ++-
>>   .../hwtracing/coresight/coresight-tmc-etr.c   | 175 ++++++++++++++++++
>>   drivers/hwtracing/coresight/coresight-tmc.h   |  10 +-
>>   9 files changed, 483 insertions(+), 11 deletions(-)
>>   create mode 100644 drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c
>>
>> --
>> 2.34.1
>>
> 
> 

Powered by blists - more mailing lists