From: Jie Gan <jie.gan@oss.qualcomm.com>
To: Suzuki K Poulose <suzuki.poulose@arm.com>,
Mike Leach <mike.leach@arm.com>,
James Clark <james.clark@linaro.org>, Leo Yan <leo.yan@arm.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Rob Herring <robh@kernel.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Conor Dooley <conor+dt@kernel.org>,
Tingwei Zhang <tingwei.zhang@oss.qualcomm.com>,
Bjorn Andersson <andersson@kernel.org>,
Konrad Dybcio <konradybcio@kernel.org>,
Yuanfang Zhang <yuanfang.zhang@oss.qualcomm.com>,
Mao Jinlong <jinlong.mao@oss.qualcomm.com>,
Abel Vesa <abelvesa@kernel.org>
Cc: 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,
Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>,
Krzysztof Kozlowski <krzk@kernel.org>
Subject: Re: [PATCH v20 0/7] coresight: ctcu: Enable byte-cntr function for TMC ETR
Date: Fri, 30 Oct 2026 15:22:07 +0800 [thread overview]
Message-ID: <86390538-2dce-42d5-80fe-7a3e6c887828@oss.qualcomm.com> (raw)
In-Reply-To: <20261030-enable-byte-cntr-for-ctcu-v20-0-bd5c3ac9f916@oss.qualcomm.com>
On 10/30/2026 2:00 PM, Jie Gan 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 1 > /sys/bus/coresight/devices/ctcu0/irq_enabled0
> 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_enabled0
>
Sashiko reported a lot of the potential issues on the series. I am
checking all comments and will send V21 to address these comments.
https://lore.kernel.org/all/sashiko-outbox-190593@kernel.org/
https://lore.kernel.org/all/sashiko-outbox-190601@kernel.org/
Thanks,
Jie
> ---
> Changes in v20:
> 1. coresight: core: refactor ctcu_get_active_port and make it generic:
> - Check the port number returned by coresight_get_in_port() against
> ETR_MAX_NUM in ctcu_set_etr_traceid() to avoid an out-of-bounds
> access of the per-port arrays with a bad DT port number.
> 2. coresight: tmc: add create/clean functions for etr_buf_list:
> - Take drvdata->sysfs_buf and add the first node to etr_buf_list in
> one critical section, and do all etr_buf_list updates (add and
> delete) with drvdata->spinlock held.
> - Return -EBUSY from tmc_etr_get_sysfs_buffer() while etr_buf_list is
> not empty, so the ETR sink enable path cannot free or replace a
> buffer that is still owned by the list during a byte-cntr session.
> 3. coresight: ctcu: enable byte-cntr for TMC ETR devices:
> - Fixed a NULL pointer dereference in ctcu_get_byte_cntr() when the
> opened ETR is not connected to a CTCU device, since the byte-cntr
> sysfs ops are global to all the TMC ETRs.
> - Fixed the sysfs read fallback: tmc_byte_cntr_get_data() now returns
> -EINVAL unless a byte-cntr read session is active, and
> tmc_etr_get_sysfs_trace() falls back to the generic read only for
> -EINVAL. Errors and EOF of the byte-cntr read are returned as-is
> instead of reading the live buffer.
> - Fixed a race between the buffer switch and the sink disable path:
> tmc_etr_enable_disable_hw() now checks the sink mode with
> drvdata->spinlock held, so the ETR is not restarted after the sink
> has been disabled.
> - tmc_read_prepare_byte_cntr(): check irq_enabled with the
> byte_cntr_data->spin_lock held, and clear reading under the lock
> and wake up the waiter on the buffer list allocation failure path.
> - Pin the module with try_module_get() for the duration of a byte-cntr
> read session, so unloading the module fails with -EBUSY instead of
> blocking in ctcu_platform_remove(). Documented that a manual unbind
> still waits for the tmc_etr file to be closed.
> - Dropped IRQF_SHARED from the byte-cntr interrupt: the handler does
> not check the interrupt source.
> - Read curr_buf with drvdata->spinlock held and handle a missing
> current node in ctcu_byte_cntr_switch_buffer().
> - Initialize the lock and waitqueue of every port before requesting
> its interrupt, skip the ports without a registered IRQ in
> ctcu_platform_remove(), and only expose irq_enabled for the ports
> with a registered IRQ.
> - Refcount the byte-cntr sysfs ops registration across CTCU devices
> with a devm action, so removing one CTCU no longer disables
> byte-cntr for the others. Replaced the single-CTCU-instance comment
> on byte_cntr_sysfs_ops accordingly.
> - Link to v19: https://lore.kernel.org/r/20260625-enable-byte-cntr-for-ctcu-v19-0-8fbbf22e8381@oss.qualcomm.com
>
> Changes in v19:
> 1. coresight: tmc: add create/clean functions for etr_buf_list:
> - Simplified the kernel-doc "Locking:" note on tmc_create_etr_buf_list()
> and tmc_clean_etr_buf_list() to state only the required contract
> (caller must guarantee exclusive access to drvdata->etr_buf_list),
> dropping the over-specific call-path details.
> - Lock the required code block in create/clean functions.
> 2. coresight: ctcu: enable byte-cntr for TMC ETR devices:
> - Fixed a stale reading-state / double-free when irq_enabled was
> cleared between prepare and unprepare: the byte-cntr read_unprepare
> would return an error and tmc_read_unprepare_etr() would fall back
> to the normal unprepare path, freeing drvdata->sysfs_buf while it
> was still owned by the byte-cntr buffer list. irq_enabled_store()
> now returns -EBUSY while byte_cntr_data->reading is set, so the flag
> can no longer change mid-session, and tmc_read_unprepare_byte_cntr()
> additionally guards on byte_cntr_data->reading.
> - Moved enable_irq_wake()/disable_irq_wake() out of the
> byte_cntr_data->spin_lock critical section in
> tmc_read_prepare_byte_cntr()/tmc_read_unprepare_byte_cntr().
> irq_set_irq_wake() may sleep on slow-bus irqchips, so it must not
> be called under a raw spinlock. The threshold IRQ is already
> disabled by the in-lock register write before disable_irq_wake() on
> the unprepare path, so no wake event can race in the gap.
> - Added a comment on the byte_cntr_sysfs_ops pointer documenting the
> single-CTCU-instance-per-system assumption.
> - Link to v18: https://lore.kernel.org/r/20260507-enable-byte-cntr-for-ctcu-v18-0-2b2d590463a3@oss.qualcomm.com
>
> Changes in v18:
> 1. add a NULL check for the in_conns instance in patch 1.
> 2. fix a bug in patch 2: the tmc_alloc_etr_buf never return NULL and the
> previous check for the return value is incorrect.
> 3. add more kernel_doc description for tmc_clean_etr_buf_list function
> in patch 2
> - Link to v17: https://lore.kernel.org/r/20260421-enable-byte-cntr-for-ctcu-v17-0-9cf36ff55fc0@oss.qualcomm.com
>
> Changes in v17:
> 1. fix race issue during allocat buffer.
> 2. fix user after free issue observed when remove module.
> - Link to v16: https://lore.kernel.org/r/20260323-enable-byte-cntr-for-ctcu-v16-0-7a413d211b8d@oss.qualcomm.com
>
> Changes in v16:
> 1. Remove lock/unlock processes in patch "coresight: tmc: add create/clean
> functions for etr_buf_list" because we are allocating/freeing memory.
> - Link to v15: https://lore.kernel.org/r/20260313-enable-byte-cntr-for-ctcu-v15-0-1777f14ed319@oss.qualcomm.com
>
> Changes in v15:
> 1. add lockdep_assert_held in patch "coresight: tmc: add create/clean
> functions for etr_buf_list"
> 2. optimize tmc_clean_etr_buf_list function
> 3. optimize the patch "enable byte-cntr for TMC ETR devices" according
> to Suzuki's comments
> - call byte_cntr_sysfs_ops from etr_sysfs_ops
> - optimize the lock usage in all functions
> - remove the buf_node parameter in etr_drvdata, move it to
> byte_cntr_data
> - move the tmc_reset_sysfs_buf function to tmc-etr.c
> - add a read flag to struct etr_buf_node to allow updating pos while
> traversing etr_buf_list during data reads.
> Link to v14: https://lore.kernel.org/r/20260309-enable-byte-cntr-for-ctcu-v14-0-c08823e5a8e6@oss.qualcomm.com
>
> Changes in V14:
> 1. Drop the patch: integrate byte-cntr's sysfs_ops with tmc sysfs file_ops
> 2. Replace tmc_sysfs_ops with byte_cntr_sysfs_ops in byte_cntr_start
> function and restore etr_sysfs_ops in byte_cntr_unprepare function.
> 3. Remove redundant checks in byte‑cntr functions.
> Link to V13: https://lore.kernel.org/all/20260223-enable-byte-cntr-for-ctcu-v13-0-9cb44178b250@oss.qualcomm.com/
>
> Changes in v13:
> 1. initilize the byte_cntr_data->raw_spin_lock before using.
> 2. replace kzalloc with kzalloc_obj.
> Link to V12: https://lore.kernel.org/all/20260203-enable-byte-cntr-for-ctcu-v12-0-7bf81b86b70e@oss.qualcomm.com/
>
> Changes in v12:
> 1. Add a new function for retrieving the CTCU's coresight_dev instead of
> refactor the existing function.
> Link to v11: https://lore.kernel.org/r/20260126-enable-byte-cntr-for-ctcu-v11-0-c0af66ba15cf@oss.qualcomm.com
>
> Changes in v11:
> 1. Correct the description in patch1 for the function coresight_get_in_port.
> 2. Renaming the sysfs_ops to tmc_sysfs_ops per Suzuki's suggestion.
> Link to v10: https://lore.kernel.org/r/20260122-enable-byte-cntr-for-ctcu-v10-0-22978e3c169f@oss.qualcomm.com
>
> Changes in v10:
> 1. fix a free memory issue that is reported by robot for patch 2.
> Link to v9: https://lore.kernel.org/r/20251224-enable-byte-cntr-for-ctcu-v9-0-886c4496fed4@oss.qualcomm.com
>
> Changes in v9:
> 1. Drop the patch: add a new API to retrieve the helper device
> 2. Add a new patch to refactor the tmc_etr_get_catu_device function,
> making it generic to support all types of helper devices associated with ETR.
> 3. Optimizing the code for creating irq_threshold sysfs node.
> 4. Remove interrupt-name property and obtain the IRQ based on the
> in-port number.
> Link to v8: https://lore.kernel.org/r/20251211-enable-byte-cntr-for-ctcu-v8-0-3e12ff313191@oss.qualcomm.com
>
> Changes in V8:
> 1. Optimizing the patch 1 and patch 2 according to Suzuki's comments.
> 2. Combine the patch 3 and patch 4 together.
> 3. Rename the interrupt-name to prevent confusion, for example:etr0->etrirq0.
> Link to V7 - https://lore.kernel.org/all/20251013-enable-byte-cntr-for-ctcu-v7-0-e1e8f41e15dd@oss.qualcomm.com/
>
> Changes in V7:
> 1. rebased on tag next-20251010
> 2. updated info for sysfs node document
> Link to V6 - https://lore.kernel.org/all/20250908-enable-byte-cntr-for-tmc-v6-0-1db9e621441a@oss.qualcomm.com/
>
> Changes in V6:
> 1. rebased on next-20250905.
> 2. fixed the issue that the dtsi file has re-named from sa8775p.dtsi to
> lemans.dtsi.
> 3. fixed some minor issues about comments.
> Link to V5 - https://lore.kernel.org/all/20250812083731.549-1-jie.gan@oss.qualcomm.com/
>
> Changes in V5:
> 1. Add Mike's reviewed-by tag for patchset 1,2,5.
> 2. Remove the function pointer added to helper_ops according to Mike's
> comment, it also results the patchset has been removed.
> 3. Optimizing the paired create/clean functions for etr_buf_list.
> 4. Remove the unneeded parameter "reading" from the etr_buf_node.
> Link to V4 - https://lore.kernel.org/all/20250725100806.1157-1-jie.gan@oss.qualcomm.com/
>
> Changes in V4:
> 1. Rename the function to coresight_get_in_port_dest regarding to Mike's
> comment (patch 1/10).
> 2. Add lock to protect the connections regarding to Mike's comment
> (patch 2/10).
> 3. Move all byte-cntr functions to coresight-ctcu-byte-cntr file.
> 4. Add tmc_read_ops to wrap all read operations for TMC device.
> 5. Add a function in helper_ops to check whether the byte-cntr is
> enabkled.
> 6. Call byte-cntr's read_ops if byte-cntr is enabled when reading data
> from the sysfs node.
> Link to V3 resend - https://lore.kernel.org/all/20250714063109.591-1-jie.gan@oss.qualcomm.com/
>
> 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/
>
> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
>
> ---
> Jie Gan (7):
> coresight: core: refactor ctcu_get_active_port and make it generic
> coresight: tmc: add create/clean functions for etr_buf_list
> coresight: tmc: introduce tmc_sysfs_ops to wrap sysfs read operations
> coresight: etr: add a new function to retrieve the CTCU device
> dt-bindings: arm: add an interrupt property for Coresight CTCU
> coresight: ctcu: enable byte-cntr for TMC ETR devices
> arm64: dts: qcom: lemans: add interrupts to CTCU device
>
> .../ABI/testing/sysfs-bus-coresight-devices-ctcu | 9 +
> .../bindings/arm/qcom,coresight-ctcu.yaml | 10 +
> arch/arm64/boot/dts/qcom/lemans.dtsi | 3 +
> drivers/hwtracing/coresight/Makefile | 2 +-
> drivers/hwtracing/coresight/coresight-core.c | 27 ++
> .../hwtracing/coresight/coresight-ctcu-byte-cntr.c | 392 +++++++++++++++++++++
> drivers/hwtracing/coresight/coresight-ctcu-core.c | 158 +++++++--
> drivers/hwtracing/coresight/coresight-ctcu.h | 81 ++++-
> drivers/hwtracing/coresight/coresight-priv.h | 2 +
> drivers/hwtracing/coresight/coresight-tmc-core.c | 55 ++-
> drivers/hwtracing/coresight/coresight-tmc-etr.c | 289 ++++++++++++++-
> drivers/hwtracing/coresight/coresight-tmc.h | 42 +++
> 12 files changed, 995 insertions(+), 75 deletions(-)
> ---
> base-commit: a39c950806213f6861d00f90729acaaa21f2d95a
> change-id: 20260309-enable-byte-cntr-for-ctcu-ff86e6198b7f
>
> Best regards,
prev parent reply other threads:[~2026-10-30 7:22 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-30 6:00 Jie Gan
2026-10-30 6:00 ` [PATCH v20 1/7] coresight: core: refactor ctcu_get_active_port and make it generic Jie Gan
2026-10-30 6:00 ` [PATCH v20 2/7] coresight: tmc: add create/clean functions for etr_buf_list Jie Gan
2026-10-30 6:00 ` [PATCH v20 3/7] coresight: tmc: introduce tmc_sysfs_ops to wrap sysfs read operations Jie Gan
2026-10-30 6:00 ` [PATCH v20 4/7] coresight: etr: add a new function to retrieve the CTCU device Jie Gan
2026-10-30 6:00 ` [PATCH v20 5/7] dt-bindings: arm: add an interrupt property for Coresight CTCU Jie Gan
2026-10-30 6:00 ` [PATCH v20 6/7] coresight: ctcu: enable byte-cntr for TMC ETR devices Jie Gan
2026-10-30 6:00 ` [PATCH v20 7/7] arm64: dts: qcom: lemans: add interrupts to CTCU device Jie Gan
2026-10-30 7:22 ` Jie Gan [this message]
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=86390538-2dce-42d5-80fe-7a3e6c887828@oss.qualcomm.com \
--to=jie.gan@oss.qualcomm.com \
--cc=abelvesa@kernel.org \
--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=konrad.dybcio@oss.qualcomm.com \
--cc=konradybcio@kernel.org \
--cc=krzk+dt@kernel.org \
--cc=krzk@kernel.org \
--cc=leo.yan@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mike.leach@arm.com \
--cc=robh@kernel.org \
--cc=suzuki.poulose@arm.com \
--cc=tingwei.zhang@oss.qualcomm.com \
--cc=yuanfang.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
all inboxes | Powered by JetHome®