* [PATCH v19 0/7] coresight: ctcu: Enable byte-cntr function for TMC ETR
@ 2026-06-25 10:45 Jie Gan
2026-06-25 10:45 ` [PATCH v19 1/7] coresight: core: refactor ctcu_get_active_port and make it generic Jie Gan
` (6 more replies)
0 siblings, 7 replies; 13+ messages in thread
From: Jie Gan @ 2026-06-25 10:45 UTC (permalink / raw)
To: Suzuki K Poulose, Mike Leach, James Clark, Leo Yan,
Alexander Shishkin, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Tingwei Zhang, Jie Gan, Bjorn Andersson,
Konrad Dybcio, Yuanfang Zhang, Mao Jinlong
Cc: coresight, linux-arm-kernel, linux-kernel, linux-arm-msm,
devicetree, Konrad Dybcio, Mike Leach, Krzysztof Kozlowski
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
---
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 | 327 +++++++++++++++++++++
drivers/hwtracing/coresight/coresight-ctcu-core.c | 146 +++++++--
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 | 265 ++++++++++++++++-
drivers/hwtracing/coresight/coresight-tmc.h | 42 +++
12 files changed, 896 insertions(+), 73 deletions(-)
---
base-commit: 4e5dfb7c84012007c3c7061126491bbc92d71bf1
change-id: 20260309-enable-byte-cntr-for-ctcu-ff86e6198b7f
Best regards,
--
Jie Gan <jie.gan@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH v19 1/7] coresight: core: refactor ctcu_get_active_port and make it generic 2026-06-25 10:45 [PATCH v19 0/7] coresight: ctcu: Enable byte-cntr function for TMC ETR Jie Gan @ 2026-06-25 10:45 ` Jie Gan 2026-06-25 11:07 ` sashiko-bot 2026-06-25 10:45 ` [PATCH v19 2/7] coresight: tmc: add create/clean functions for etr_buf_list Jie Gan ` (5 subsequent siblings) 6 siblings, 1 reply; 13+ messages in thread From: Jie Gan @ 2026-06-25 10:45 UTC (permalink / raw) To: Suzuki K Poulose, Mike Leach, James Clark, Leo Yan, Alexander Shishkin, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Tingwei Zhang, Jie Gan, Bjorn Andersson, Konrad Dybcio, Yuanfang Zhang, Mao Jinlong Cc: coresight, linux-arm-kernel, linux-kernel, linux-arm-msm, devicetree, Mike Leach Remove ctcu_get_active_port from CTCU module and add it to the core framework. The port number is crucial for the CTCU device to identify which ETR it serves. With the port number we can correctly get required parameters of the CTCU device in TMC module. Reviewed-by: Mike Leach <mike.leach@linaro.org> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com> --- drivers/hwtracing/coresight/coresight-core.c | 27 +++++++++++++++++++++++ drivers/hwtracing/coresight/coresight-ctcu-core.c | 19 +--------------- drivers/hwtracing/coresight/coresight-priv.h | 2 ++ 3 files changed, 30 insertions(+), 18 deletions(-) diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index 6d65c43d574f..7a7a85acdca0 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -773,6 +773,33 @@ struct coresight_device *coresight_get_sink(struct coresight_path *path) } EXPORT_SYMBOL_GPL(coresight_get_sink); +/** + * coresight_get_in_port: Find the input port number at @remote where the @csdev + * device is connected to. + * + * @csdev: csdev of the device. + * @remote: csdev of the remote device which is connected to @csdev. + * + * Return: port number upon success or -EINVAL for fail. + */ +int coresight_get_in_port(struct coresight_device *csdev, + struct coresight_device *remote) +{ + struct coresight_platform_data *pdata = remote->pdata; + int i; + + for (i = 0; i < pdata->nr_inconns; ++i) { + if (!pdata->in_conns[i]) + continue; + + if (pdata->in_conns[i]->src_dev == csdev) + return pdata->in_conns[i]->dest_port; + } + + return -EINVAL; +} +EXPORT_SYMBOL_GPL(coresight_get_in_port); + u32 coresight_get_sink_id(struct coresight_device *csdev) { if (!csdev->ea) diff --git a/drivers/hwtracing/coresight/coresight-ctcu-core.c b/drivers/hwtracing/coresight/coresight-ctcu-core.c index 9043cad42f01..e8720026c9e3 100644 --- a/drivers/hwtracing/coresight/coresight-ctcu-core.c +++ b/drivers/hwtracing/coresight/coresight-ctcu-core.c @@ -116,23 +116,6 @@ static int __ctcu_set_etr_traceid(struct coresight_device *csdev, u8 traceid, in return 0; } -/* - * Searching the sink device from helper's view in case there are multiple helper devices - * connected to the sink device. - */ -static int ctcu_get_active_port(struct coresight_device *sink, struct coresight_device *helper) -{ - struct coresight_platform_data *pdata = helper->pdata; - int i; - - for (i = 0; i < pdata->nr_inconns; ++i) { - if (pdata->in_conns[i]->src_dev == sink) - return pdata->in_conns[i]->dest_port; - } - - return -EINVAL; -} - static int ctcu_set_etr_traceid(struct coresight_device *csdev, struct coresight_path *path, bool enable) { @@ -145,7 +128,7 @@ static int ctcu_set_etr_traceid(struct coresight_device *csdev, struct coresight return -EINVAL; } - port_num = ctcu_get_active_port(sink, csdev); + port_num = coresight_get_in_port(sink, csdev); if (port_num < 0) return -EINVAL; diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h index dddac946659f..854c0a3cb080 100644 --- a/drivers/hwtracing/coresight/coresight-priv.h +++ b/drivers/hwtracing/coresight/coresight-priv.h @@ -155,6 +155,8 @@ void coresight_remove_links(struct coresight_device *orig, u32 coresight_get_sink_id(struct coresight_device *csdev); int coresight_path_assign_trace_id(struct coresight_path *path, enum cs_mode mode); +int coresight_get_in_port(struct coresight_device *csdev, + struct coresight_device *remote); #if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM3X) int etm_readl_cp14(u32 off, unsigned int *val); -- 2.34.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v19 1/7] coresight: core: refactor ctcu_get_active_port and make it generic 2026-06-25 10:45 ` [PATCH v19 1/7] coresight: core: refactor ctcu_get_active_port and make it generic Jie Gan @ 2026-06-25 11:07 ` sashiko-bot 0 siblings, 0 replies; 13+ messages in thread From: sashiko-bot @ 2026-06-25 11:07 UTC (permalink / raw) To: Jie Gan; +Cc: devicetree, conor+dt, robh Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: New issues: - [High] Lockless iteration over `pdata->in_conns` array introduces a TOCTOU race condition and potential Use-After-Free. Pre-existing issues: - [High] Missing bounds check for Device Tree-provided `port_num` leads to out-of-bounds array access in CTCU trace ID configuration. - [High] Unsafe direct dereference of sparse array elements in `tpda_get_element_size`. -- commit f655e6df0885045f179728cb52f455717556ac67 Author: Jie Gan <jie.gan@oss.qualcomm.com> coresight: core: refactor ctcu_get_active_port and make it generic This patch removes ctcu_get_active_port from the CTCU module and adds it to the core framework as coresight_get_in_port. It allows retrieving the active input port number generically for a given device. > diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c > index 6d65c43d574fd..7a7a85acdca0d 100644 > --- a/drivers/hwtracing/coresight/coresight-core.c > +++ b/drivers/hwtracing/coresight/coresight-core.c > @@ -773,6 +773,33 @@ struct coresight_device *coresight_get_sink(struct coresight_path *path) [ ... ] > +int coresight_get_in_port(struct coresight_device *csdev, > + struct coresight_device *remote) > +{ > + struct coresight_platform_data *pdata = remote->pdata; > + int i; > + > + for (i = 0; i < pdata->nr_inconns; ++i) { > + if (!pdata->in_conns[i]) > + continue; > + > + if (pdata->in_conns[i]->src_dev == csdev) [Severity: High] Can this sequence cause a race condition leading to a NULL pointer dereference? The array element is read twice. First, to check if it's NULL, and then again to access src_dev. If a concurrent device unbind operation like coresight_remove_conns() sets the element to NULL between these reads, the second access might trigger a panic. Should we use READ_ONCE() to load the pointer into a local variable first? > + return pdata->in_conns[i]->dest_port; > + } > + > + return -EINVAL; > +} > +EXPORT_SYMBOL_GPL(coresight_get_in_port); > + > u32 coresight_get_sink_id(struct coresight_device *csdev) > { > if (!csdev->ea) > diff --git a/drivers/hwtracing/coresight/coresight-ctcu-core.c b/drivers/hwtracing/coresight/coresight-ctcu-core.c > index 9043cad42f01e..e8720026c9e3f 100644 > --- a/drivers/hwtracing/coresight/coresight-ctcu-core.c > +++ b/drivers/hwtracing/coresight/coresight-ctcu-core.c [ ... ] > @@ -145,7 +128,7 @@ static int ctcu_set_etr_traceid(struct coresight_device *csdev, struct coresight > return -EINVAL; > } > > - port_num = ctcu_get_active_port(sink, csdev); > + port_num = coresight_get_in_port(sink, csdev); > if (port_num < 0) > return -EINVAL; [Severity: High] This isn't a bug introduced by this patch, but does the returned port_num need to be bounds-checked before being used? The port_num originates from device tree properties, and if it exceeds ETR_MAX_NUM (2), it leads to out-of-bounds array accesses in __ctcu_set_etr_traceid() when indexing into drvdata->atid_offset and drvdata->traceid_refcnt: __ctcu_set_etr_traceid() ... atid_offset = drvdata->atid_offset[port_num]; ... refcnt = drvdata->traceid_refcnt[port_num][traceid]; Would it be safer to validate port_num against the array bounds after retrieving it? > [ ... ] [Severity: High] This is a pre-existing issue not introduced by this patch, but while looking at the sparse in_conns array usage, I noticed a similar issue in tpda_get_element_size(). Does this code dereference the sparse array element before checking if it is valid? drivers/hwtracing/coresight/coresight-tpda.c:tpda_get_element_size() { ... for (i = 0; i < csdev->pdata->nr_inconns; i++) { in = csdev->pdata->in_conns[i]->src_dev; if (!in) continue; ... } If a component is unbound and the connection is removed by coresight_remove_conns(), csdev->pdata->in_conns[i] could be NULL, causing a panic before the check is reached. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260625-enable-byte-cntr-for-ctcu-v19-0-8fbbf22e8381@oss.qualcomm.com?part=1 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v19 2/7] coresight: tmc: add create/clean functions for etr_buf_list 2026-06-25 10:45 [PATCH v19 0/7] coresight: ctcu: Enable byte-cntr function for TMC ETR Jie Gan 2026-06-25 10:45 ` [PATCH v19 1/7] coresight: core: refactor ctcu_get_active_port and make it generic Jie Gan @ 2026-06-25 10:45 ` Jie Gan 2026-06-25 11:06 ` sashiko-bot 2026-06-25 10:45 ` [PATCH v19 3/7] coresight: tmc: introduce tmc_sysfs_ops to wrap sysfs read operations Jie Gan ` (4 subsequent siblings) 6 siblings, 1 reply; 13+ messages in thread From: Jie Gan @ 2026-06-25 10:45 UTC (permalink / raw) To: Suzuki K Poulose, Mike Leach, James Clark, Leo Yan, Alexander Shishkin, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Tingwei Zhang, Jie Gan, Bjorn Andersson, Konrad Dybcio, Yuanfang Zhang, Mao Jinlong Cc: coresight, linux-arm-kernel, linux-kernel, linux-arm-msm, devicetree, Mike Leach Introduce functions for creating and inserting or removing the etr_buf_node to/from the etr_buf_list. The byte-cntr functionality requires two etr_buf to receive trace data. The active etr_buf collects the trace data from source device, while the byte-cntr reading function accesses the deactivated etr_buf after is has been filled and synced, transferring data to the userspace. Reviewed-by: Mike Leach <mike.leach@linaro.org> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com> --- drivers/hwtracing/coresight/coresight-tmc-core.c | 1 + drivers/hwtracing/coresight/coresight-tmc-etr.c | 126 +++++++++++++++++++++++ drivers/hwtracing/coresight/coresight-tmc.h | 17 +++ 3 files changed, 144 insertions(+) diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c index bc5a133ada3e..bc7dd676da47 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-core.c +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c @@ -835,6 +835,7 @@ static int __tmc_probe(struct device *dev, struct resource *res) idr_init(&drvdata->idr); mutex_init(&drvdata->idr_mutex); dev_list = "tmc_etr"; + INIT_LIST_HEAD(&drvdata->etr_buf_list); break; case TMC_CONFIG_TYPE_ETF: desc.groups = coresight_etf_groups; diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index 361a433e6f0c..9b3ef73e9cf2 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -1918,6 +1918,132 @@ const struct coresight_ops tmc_etr_cs_ops = { .panic_ops = &tmc_etr_sync_ops, }; +/** + * tmc_clean_etr_buf_list - clean the etr_buf_list. + * @drvdata: driver data of the TMC device. + * + * Remove all nodes from @drvdata->etr_buf_list and free their buffers. + * If a node holds the live sysfs_buf and the device is active, the node is + * removed but the buffer is not freed; ownership stays with drvdata->sysfs_buf. + * + * Locking: callers must guarantee exclusive access to @drvdata->etr_buf_list + * and must not hold @drvdata->spinlock. The spinlock is taken internally only + * to serialise the @drvdata->sysfs_buf accesses against the ETR sink + * enable/disable paths. Must be called from process context: buffers are freed + * with the lock released. + */ +void tmc_clean_etr_buf_list(struct tmc_drvdata *drvdata) +{ + struct etr_buf_node *nd, *next; + unsigned long flags; + + list_for_each_entry_safe(nd, next, &drvdata->etr_buf_list, link) { + raw_spin_lock_irqsave(&drvdata->spinlock, flags); + if (nd->sysfs_buf == drvdata->sysfs_buf) { + if (coresight_get_mode(drvdata->csdev) != CS_MODE_DISABLED) + /* + * The device is still active. Keep the live + * buffer owned by drvdata->sysfs_buf and only + * drop the list's reference to it. + */ + nd->sysfs_buf = NULL; + else + /* Free the buffer below through nd->sysfs_buf */ + drvdata->sysfs_buf = NULL; + } + raw_spin_unlock_irqrestore(&drvdata->spinlock, flags); + + /* Free the buffer (NULL is ignored) and the node out of the lock */ + tmc_etr_free_sysfs_buf(nd->sysfs_buf); + list_del(&nd->link); + kfree(nd); + } +} +EXPORT_SYMBOL_GPL(tmc_clean_etr_buf_list); + +/** + * tmc_create_etr_buf_list - create a list to manage the etr_buf_node. + * @drvdata: driver data of the TMC device. + * @num_nodes: number of nodes want to create with the list. + * + * Locking: callers must guarantee exclusive access to @drvdata->etr_buf_list + * and must not hold @drvdata->spinlock. The spinlock is taken internally only + * to serialise the @drvdata->sysfs_buf accesses against the ETR sink + * enable/disable paths. Must be called from process context: buffers and nodes + * are allocated with the lock released. + * + * Return 0 upon success and return the error number if fail. + */ +int tmc_create_etr_buf_list(struct tmc_drvdata *drvdata, int num_nodes) +{ + struct etr_buf_node *new_node; + struct etr_buf *sysfs_buf; + unsigned long flags; + int i = 0, ret = 0; + + /* We don't need a list if there is only one node */ + if (num_nodes < 2) + return -EINVAL; + + /* + * We expect that sysfs_buf in drvdata has already been allocated. + * Wrap the live sysfs_buf into the first node so the captured trace + * data is preserved. The list is owned by the caller, so no lock is + * needed to read sysfs_buf or to add the node here. + */ + if (drvdata->sysfs_buf) { + new_node = kzalloc_obj(*new_node, GFP_KERNEL); + if (!new_node) + return -ENOMEM; + + new_node->sysfs_buf = drvdata->sysfs_buf; + new_node->is_free = false; + list_add(&new_node->link, &drvdata->etr_buf_list); + i++; + } + + while (i < num_nodes) { + new_node = kzalloc_obj(*new_node, GFP_KERNEL); + if (!new_node) { + ret = -ENOMEM; + break; + } + + /* Allocate the buffer with the lock released */ + sysfs_buf = tmc_alloc_etr_buf(drvdata, drvdata->size, 0, cpu_to_node(0), NULL); + if (IS_ERR(sysfs_buf)) { + kfree(new_node); + ret = PTR_ERR(sysfs_buf); + break; + } + + new_node->sysfs_buf = sysfs_buf; + /* + * Only the drvdata->sysfs_buf write needs the spinlock, to + * serialise against the ETR sink enable/disable paths. + */ + raw_spin_lock_irqsave(&drvdata->spinlock, flags); + /* We don't have an available sysfs_buf in drvdata, set one up */ + if (!drvdata->sysfs_buf) { + drvdata->sysfs_buf = sysfs_buf; + new_node->is_free = false; + } else { + new_node->is_free = true; + } + raw_spin_unlock_irqrestore(&drvdata->spinlock, flags); + + list_add_tail(&new_node->link, &drvdata->etr_buf_list); + i++; + } + + /* Clean the list if there is an error */ + if (ret) + tmc_clean_etr_buf_list(drvdata); + + return ret; +} +EXPORT_SYMBOL_GPL(tmc_create_etr_buf_list); + int tmc_read_prepare_etr(struct tmc_drvdata *drvdata) { int ret = 0; diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h index 319a354ede9f..6e994678f926 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.h +++ b/drivers/hwtracing/coresight/coresight-tmc.h @@ -208,6 +208,19 @@ struct tmc_resrv_buf { s64 len; }; +/** + * @sysfs_buf: Allocated sysfs_buf. + * @is_free: Indicates whether the buffer is free to choose. + * @pos: Offset to the start of the buffer. + * @link: list_head of the node. + */ +struct etr_buf_node { + struct etr_buf *sysfs_buf; + bool is_free; + loff_t pos; + struct list_head link; +}; + /** * struct tmc_drvdata - specifics associated to an TMC component * @atclk: optional clock for the core parts of the TMC. @@ -245,6 +258,7 @@ struct tmc_resrv_buf { * (after crash) by default. * @crash_mdata: Reserved memory for storing tmc crash metadata. * Used by ETR/ETF. + * @etr_buf_list: List that is used to manage allocated etr_buf. */ struct tmc_drvdata { struct clk *atclk; @@ -275,6 +289,7 @@ struct tmc_drvdata { struct etr_buf *perf_buf; struct tmc_resrv_buf resrv_buf; struct tmc_resrv_buf crash_mdata; + struct list_head etr_buf_list; }; struct etr_buf_operations { @@ -447,5 +462,7 @@ struct etr_buf *tmc_etr_get_buffer(struct coresight_device *csdev, enum cs_mode mode, struct coresight_path *path); extern const struct attribute_group coresight_etr_group; +void tmc_clean_etr_buf_list(struct tmc_drvdata *drvdata); +int tmc_create_etr_buf_list(struct tmc_drvdata *drvdata, int num_nodes); #endif -- 2.34.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v19 2/7] coresight: tmc: add create/clean functions for etr_buf_list 2026-06-25 10:45 ` [PATCH v19 2/7] coresight: tmc: add create/clean functions for etr_buf_list Jie Gan @ 2026-06-25 11:06 ` sashiko-bot 0 siblings, 0 replies; 13+ messages in thread From: sashiko-bot @ 2026-06-25 11:06 UTC (permalink / raw) To: Jie Gan; +Cc: conor+dt, robh, devicetree Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] Lockless read of `drvdata->sysfs_buf` in `tmc_create_etr_buf_list` leads to a Time-of-Check to Time-of-Use race condition, resulting in a Double-Free or Use-After-Free. - [Medium] The error path in `tmc_create_etr_buf_list` destructively frees the pre-existing `drvdata->sysfs_buf` if the device becomes inactive, leading to the irreversible loss of previously captured trace data. - [Medium] `tmc_clean_etr_buf_list` leaks `drvdata->sysfs_buf` if called when the device is active in a non-SYSFS mode (e.g., `CS_MODE_PERF`). -- commit efe04b42620438e6e268cf24ebf0538327ca8629 Author: Jie Gan <jie.gan@oss.qualcomm.com> coresight: tmc: add create/clean functions for etr_buf_list This commit introduces functions for creating and cleaning a list of etr_buf_node elements to manage etr_buf entries. It is intended to support byte-counter functionality which requires multiple trace data buffers. > diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c > index 361a433e6f0c5..9b3ef73e9cf27 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c > +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c [ ... ] > @@ -1918,6 +1918,132 @@ const struct coresight_ops tmc_etr_cs_ops = { > .panic_ops = &tmc_etr_sync_ops, > }; > > +/** > + * tmc_clean_etr_buf_list - clean the etr_buf_list. > + * @drvdata: driver data of the TMC device. > + * > + * Remove all nodes from @drvdata->etr_buf_list and free their buffers. > + * If a node holds the live sysfs_buf and the device is active, the node is > + * removed but the buffer is not freed; ownership stays with drvdata->sysfs_buf. > + * > + * Locking: callers must guarantee exclusive access to @drvdata->etr_buf_list > + * and must not hold @drvdata->spinlock. The spinlock is taken internally only > + * to serialise the @drvdata->sysfs_buf accesses against the ETR sink > + * enable/disable paths. Must be called from process context: buffers are freed > + * with the lock released. > + */ > +void tmc_clean_etr_buf_list(struct tmc_drvdata *drvdata) > +{ > + struct etr_buf_node *nd, *next; > + unsigned long flags; > + > + list_for_each_entry_safe(nd, next, &drvdata->etr_buf_list, link) { > + raw_spin_lock_irqsave(&drvdata->spinlock, flags); > + if (nd->sysfs_buf == drvdata->sysfs_buf) { > + if (coresight_get_mode(drvdata->csdev) != CS_MODE_DISABLED) > + /* > + * The device is still active. Keep the live > + * buffer owned by drvdata->sysfs_buf and only > + * drop the list's reference to it. > + */ > + nd->sysfs_buf = NULL; [Severity: Medium] Will this leak drvdata->sysfs_buf if the device mode is active in a non-SYSFS mode (such as CS_MODE_PERF)? If the mode changes to CS_MODE_PERF concurrently during the tmc_create_etr_buf_list() error path, this condition evaluates to true and abandons the pointer by setting nd->sysfs_buf to NULL. Since PERF mode tracing exclusively manages drvdata->perf_buf, does its teardown path ever free drvdata->sysfs_buf? > + else > + /* Free the buffer below through nd->sysfs_buf */ > + drvdata->sysfs_buf = NULL; [Severity: Medium] If tmc_create_etr_buf_list() encounters an allocation failure and calls tmc_clean_etr_buf_list() to roll back, and the device happens to be inactive, will this unconditionally clear and free the pre-existing drvdata->sysfs_buf? Should an initialization error path only clean up the resources it allocated, rather than destroying a preserved external resource and losing previously captured trace data? > + } > + raw_spin_unlock_irqrestore(&drvdata->spinlock, flags); > + > + /* Free the buffer (NULL is ignored) and the node out of the lock */ > + tmc_etr_free_sysfs_buf(nd->sysfs_buf); > + list_del(&nd->link); > + kfree(nd); > + } > +} > +EXPORT_SYMBOL_GPL(tmc_clean_etr_buf_list); > + > +/** > + * tmc_create_etr_buf_list - create a list to manage the etr_buf_node. > + * @drvdata: driver data of the TMC device. > + * @num_nodes: number of nodes want to create with the list. > + * > + * Locking: callers must guarantee exclusive access to @drvdata->etr_buf_list > + * and must not hold @drvdata->spinlock. The spinlock is taken internally only > + * to serialise the @drvdata->sysfs_buf accesses against the ETR sink > + * enable/disable paths. Must be called from process context: buffers and nodes > + * are allocated with the lock released. > + * > + * Return 0 upon success and return the error number if fail. > + */ > +int tmc_create_etr_buf_list(struct tmc_drvdata *drvdata, int num_nodes) > +{ > + struct etr_buf_node *new_node; > + struct etr_buf *sysfs_buf; > + unsigned long flags; > + int i = 0, ret = 0; > + > + /* We don't need a list if there is only one node */ > + if (num_nodes < 2) > + return -EINVAL; > + > + /* > + * We expect that sysfs_buf in drvdata has already been allocated. > + * Wrap the live sysfs_buf into the first node so the captured trace > + * data is preserved. The list is owned by the caller, so no lock is > + * needed to read sysfs_buf or to add the node here. > + */ > + if (drvdata->sysfs_buf) { > + new_node = kzalloc_obj(*new_node, GFP_KERNEL); > + if (!new_node) > + return -ENOMEM; > + > + new_node->sysfs_buf = drvdata->sysfs_buf; [Severity: High] Does this lockless read of drvdata->sysfs_buf introduce a race condition? If a concurrent hardware disable/enable path (like tmc_etr_get_sysfs_buffer()) clears and frees drvdata->sysfs_buf immediately after this lockless check, wouldn't new_node->sysfs_buf be left as a dangling pointer, leading to a use-after-free or double free? > + new_node->is_free = false; > + list_add(&new_node->link, &drvdata->etr_buf_list); > + i++; > + } -- Sashiko AI review · https://sashiko.dev/#/patchset/20260625-enable-byte-cntr-for-ctcu-v19-0-8fbbf22e8381@oss.qualcomm.com?part=2 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v19 3/7] coresight: tmc: introduce tmc_sysfs_ops to wrap sysfs read operations 2026-06-25 10:45 [PATCH v19 0/7] coresight: ctcu: Enable byte-cntr function for TMC ETR Jie Gan 2026-06-25 10:45 ` [PATCH v19 1/7] coresight: core: refactor ctcu_get_active_port and make it generic Jie Gan 2026-06-25 10:45 ` [PATCH v19 2/7] coresight: tmc: add create/clean functions for etr_buf_list Jie Gan @ 2026-06-25 10:45 ` Jie Gan 2026-06-25 11:00 ` sashiko-bot 2026-06-25 10:45 ` [PATCH v19 4/7] coresight: etr: add a new function to retrieve the CTCU device Jie Gan ` (3 subsequent siblings) 6 siblings, 1 reply; 13+ messages in thread From: Jie Gan @ 2026-06-25 10:45 UTC (permalink / raw) To: Suzuki K Poulose, Mike Leach, James Clark, Leo Yan, Alexander Shishkin, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Tingwei Zhang, Jie Gan, Bjorn Andersson, Konrad Dybcio, Yuanfang Zhang, Mao Jinlong Cc: coresight, linux-arm-kernel, linux-kernel, linux-arm-msm, devicetree, Mike Leach Introduce tmc_sysfs_ops as a wrapper, wrap sysfs read operations, for reading trace data from the TMC buffer. Reviewed-by: Mike Leach <mike.leach@linaro.org> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com> --- drivers/hwtracing/coresight/coresight-tmc-core.c | 51 ++++++++++-------------- drivers/hwtracing/coresight/coresight-tmc.h | 15 +++++++ 2 files changed, 37 insertions(+), 29 deletions(-) diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c index bc7dd676da47..4b40b692be4d 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-core.c +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c @@ -228,17 +228,10 @@ static int tmc_read_prepare(struct tmc_drvdata *drvdata) { int ret = 0; - switch (drvdata->config_type) { - case TMC_CONFIG_TYPE_ETB: - case TMC_CONFIG_TYPE_ETF: - ret = tmc_read_prepare_etb(drvdata); - break; - case TMC_CONFIG_TYPE_ETR: - ret = tmc_read_prepare_etr(drvdata); - break; - default: + if (drvdata->sysfs_ops) + ret = drvdata->sysfs_ops->read_prepare(drvdata); + else ret = -EINVAL; - } if (!ret) dev_dbg(&drvdata->csdev->dev, "TMC read start\n"); @@ -250,17 +243,10 @@ static int tmc_read_unprepare(struct tmc_drvdata *drvdata) { int ret = 0; - switch (drvdata->config_type) { - case TMC_CONFIG_TYPE_ETB: - case TMC_CONFIG_TYPE_ETF: - ret = tmc_read_unprepare_etb(drvdata); - break; - case TMC_CONFIG_TYPE_ETR: - ret = tmc_read_unprepare_etr(drvdata); - break; - default: + if (drvdata->sysfs_ops) + ret = drvdata->sysfs_ops->read_unprepare(drvdata); + else ret = -EINVAL; - } if (!ret) dev_dbg(&drvdata->csdev->dev, "TMC read end\n"); @@ -287,15 +273,7 @@ static int tmc_open(struct inode *inode, struct file *file) static ssize_t tmc_get_sysfs_trace(struct tmc_drvdata *drvdata, loff_t pos, size_t len, char **bufpp) { - switch (drvdata->config_type) { - case TMC_CONFIG_TYPE_ETB: - case TMC_CONFIG_TYPE_ETF: - return tmc_etb_get_sysfs_trace(drvdata, pos, len, bufpp); - case TMC_CONFIG_TYPE_ETR: - return tmc_etr_get_sysfs_trace(drvdata, pos, len, bufpp); - } - - return -EINVAL; + return drvdata->sysfs_ops->get_trace_data(drvdata, pos, len, bufpp); } static ssize_t tmc_read(struct file *file, char __user *data, size_t len, @@ -764,6 +742,18 @@ static void register_crash_dev_interface(struct tmc_drvdata *drvdata, "Valid crash tracedata found\n"); } +static const struct tmc_sysfs_ops etb_sysfs_ops = { + .read_prepare = tmc_read_prepare_etb, + .read_unprepare = tmc_read_unprepare_etb, + .get_trace_data = tmc_etb_get_sysfs_trace, +}; + +static const struct tmc_sysfs_ops etr_sysfs_ops = { + .read_prepare = tmc_read_prepare_etr, + .read_unprepare = tmc_read_unprepare_etr, + .get_trace_data = tmc_etr_get_sysfs_trace, +}; + static int __tmc_probe(struct device *dev, struct resource *res) { int ret = 0; @@ -823,6 +813,7 @@ static int __tmc_probe(struct device *dev, struct resource *res) desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_BUFFER; desc.ops = &tmc_etb_cs_ops; dev_list = "tmc_etb"; + drvdata->sysfs_ops = &etb_sysfs_ops; break; case TMC_CONFIG_TYPE_ETR: desc.groups = coresight_etr_groups; @@ -835,6 +826,7 @@ static int __tmc_probe(struct device *dev, struct resource *res) idr_init(&drvdata->idr); mutex_init(&drvdata->idr_mutex); dev_list = "tmc_etr"; + drvdata->sysfs_ops = &etr_sysfs_ops; INIT_LIST_HEAD(&drvdata->etr_buf_list); break; case TMC_CONFIG_TYPE_ETF: @@ -844,6 +836,7 @@ static int __tmc_probe(struct device *dev, struct resource *res) desc.subtype.link_subtype = CORESIGHT_DEV_SUBTYPE_LINK_FIFO; desc.ops = &tmc_etf_cs_ops; dev_list = "tmc_etf"; + drvdata->sysfs_ops = &etb_sysfs_ops; break; default: pr_err("%s: Unsupported TMC config\n", desc.name); diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h index 6e994678f926..a14645b04624 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.h +++ b/drivers/hwtracing/coresight/coresight-tmc.h @@ -259,6 +259,7 @@ struct etr_buf_node { * @crash_mdata: Reserved memory for storing tmc crash metadata. * Used by ETR/ETF. * @etr_buf_list: List that is used to manage allocated etr_buf. + * @sysfs_ops: Read operations for the sysfs mode. */ struct tmc_drvdata { struct clk *atclk; @@ -290,6 +291,20 @@ struct tmc_drvdata { struct tmc_resrv_buf resrv_buf; struct tmc_resrv_buf crash_mdata; struct list_head etr_buf_list; + const struct tmc_sysfs_ops *sysfs_ops; +}; + +/** + * struct tmc_sysfs_ops - read operations for TMC and its helper devices + * @read_prepare: prepare operation. + * @read_unprepare: unprepare operation. + * @get_trace_data: read operation. + */ +struct tmc_sysfs_ops { + int (*read_prepare)(struct tmc_drvdata *drvdata); + int (*read_unprepare)(struct tmc_drvdata *drvdata); + ssize_t (*get_trace_data)(struct tmc_drvdata *drvdata, loff_t pos, + size_t len, char **bufpp); }; struct etr_buf_operations { -- 2.34.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v19 3/7] coresight: tmc: introduce tmc_sysfs_ops to wrap sysfs read operations 2026-06-25 10:45 ` [PATCH v19 3/7] coresight: tmc: introduce tmc_sysfs_ops to wrap sysfs read operations Jie Gan @ 2026-06-25 11:00 ` sashiko-bot 0 siblings, 0 replies; 13+ messages in thread From: sashiko-bot @ 2026-06-25 11:00 UTC (permalink / raw) To: Jie Gan; +Cc: devicetree, conor+dt, robh Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Pre-existing issues: - [High] The error handling path in `__tmc_probe()` at the `out:` label unconditionally accesses `drvdata->csdev` and attempts to register the crash metadata interface, leading to NULL pointer dereference, error pointer dereference, or Use-After-Free. -- commit 0e2c5d1f36c04454695412377951547f5c57c845 Author: Jie Gan <jie.gan@oss.qualcomm.com> coresight: tmc: introduce tmc_sysfs_ops to wrap sysfs read operations Introduce tmc_sysfs_ops as a wrapper, wrap sysfs read operations, for reading trace data from the TMC buffer. > diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c > index bc7dd676da473..4b40b692be4d8 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc-core.c > +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c [ ... ] > @@ -835,6 +826,7 @@ static int __tmc_probe(struct device *dev, struct resource *res) > idr_init(&drvdata->idr); > mutex_init(&drvdata->idr_mutex); > dev_list = "tmc_etr"; > + drvdata->sysfs_ops = &etr_sysfs_ops; > INIT_LIST_HEAD(&drvdata->etr_buf_list); > break; > case TMC_CONFIG_TYPE_ETF: > @@ -844,6 +836,7 @@ static int __tmc_probe(struct device *dev, struct resource *res) > desc.subtype.link_subtype = CORESIGHT_DEV_SUBTYPE_LINK_FIFO; > desc.ops = &tmc_etf_cs_ops; > dev_list = "tmc_etf"; > + drvdata->sysfs_ops = &etb_sysfs_ops; > break; > default: > pr_err("%s: Unsupported TMC config\n", desc.name); [Severity: High] This isn't a bug introduced by this patch, but looking at __tmc_probe(), is it safe to unconditionally call the crashdata functions at the out: label if probe fails? If an error occurs during probe (for example, coresight_alloc_device_name() returns NULL, or coresight_register() returns an ERR_PTR), the code jumps to out: out: if (is_tmc_crashdata_valid(drvdata) && !tmc_prepare_crashdata(drvdata)) register_crash_dev_interface(drvdata, desc.name); is_tmc_crashdata_valid() then attempts to access drvdata->csdev->dev: drivers/hwtracing/coresight/coresight-tmc-core.c:is_tmc_crashdata_valid() { ... /* Check for valid metadata */ if (!mdata->valid) { dev_dbg(&drvdata->csdev->dev, "Data invalid in tmc crash metadata\n"); ... } Will this cause a NULL pointer or ERR_PTR dereference if drvdata->csdev hasn't been successfully registered? Furthermore, if misc_register() fails, coresight_unregister() frees csdev, potentially leading to a use-after-free. If register_crash_dev_interface() succeeds on an error path, could the driver core subsequently free drvdata via devres cleanup, leaving a dangling pointer in the global miscdevice list? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260625-enable-byte-cntr-for-ctcu-v19-0-8fbbf22e8381@oss.qualcomm.com?part=3 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v19 4/7] coresight: etr: add a new function to retrieve the CTCU device 2026-06-25 10:45 [PATCH v19 0/7] coresight: ctcu: Enable byte-cntr function for TMC ETR Jie Gan ` (2 preceding siblings ...) 2026-06-25 10:45 ` [PATCH v19 3/7] coresight: tmc: introduce tmc_sysfs_ops to wrap sysfs read operations Jie Gan @ 2026-06-25 10:45 ` Jie Gan 2026-06-25 10:45 ` [PATCH v19 5/7] dt-bindings: arm: add an interrupt property for Coresight CTCU Jie Gan ` (2 subsequent siblings) 6 siblings, 0 replies; 13+ messages in thread From: Jie Gan @ 2026-06-25 10:45 UTC (permalink / raw) To: Suzuki K Poulose, Mike Leach, James Clark, Leo Yan, Alexander Shishkin, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Tingwei Zhang, Jie Gan, Bjorn Andersson, Konrad Dybcio, Yuanfang Zhang, Mao Jinlong Cc: coresight, linux-arm-kernel, linux-kernel, linux-arm-msm, devicetree Add tmc_etr_get_ctcu_device function to find the ptr of the coresight_device of the CTCU device if the CTCU device is connected to the TMC ETR device. Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com> --- drivers/hwtracing/coresight/coresight-tmc-etr.c | 24 ++++++++++++++++++++++++ drivers/hwtracing/coresight/coresight-tmc.h | 1 + 2 files changed, 25 insertions(+) diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index 9b3ef73e9cf2..2b26ce6455a7 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -865,6 +865,30 @@ tmc_etr_get_catu_device(struct tmc_drvdata *drvdata) } EXPORT_SYMBOL_GPL(tmc_etr_get_catu_device); +/* + * TMC ETR could be connected to a CTCU device, which can provide ATID filter + * and byte-cntr service. This is represented by the output port of the TMC + * (ETR) connected to the input port of the CTCU. + * + * Returns : coresight_device ptr for the CTCU device if a CTCU is found. + * : NULL otherwise. + */ +struct coresight_device * +tmc_etr_get_ctcu_device(struct tmc_drvdata *drvdata) +{ + struct coresight_device *etr = drvdata->csdev; + union coresight_dev_subtype ctcu_subtype = { + .helper_subtype = CORESIGHT_DEV_SUBTYPE_HELPER_CTCU + }; + + if (!IS_ENABLED(CONFIG_CORESIGHT_CTCU)) + return NULL; + + return coresight_find_output_type(etr->pdata, CORESIGHT_DEV_TYPE_HELPER, + ctcu_subtype); +} +EXPORT_SYMBOL_GPL(tmc_etr_get_ctcu_device); + static const struct etr_buf_operations *etr_buf_ops[] = { [ETR_MODE_FLAT] = &etr_flat_buf_ops, [ETR_MODE_ETR_SG] = &etr_sg_buf_ops, diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h index a14645b04624..fbb015079872 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.h +++ b/drivers/hwtracing/coresight/coresight-tmc.h @@ -470,6 +470,7 @@ static inline uint32_t find_crash_tracedata_crc(struct tmc_drvdata *drvdata, } struct coresight_device *tmc_etr_get_catu_device(struct tmc_drvdata *drvdata); +struct coresight_device *tmc_etr_get_ctcu_device(struct tmc_drvdata *drvdata); void tmc_etr_set_catu_ops(const struct etr_buf_operations *catu); void tmc_etr_remove_catu_ops(void); -- 2.34.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v19 5/7] dt-bindings: arm: add an interrupt property for Coresight CTCU 2026-06-25 10:45 [PATCH v19 0/7] coresight: ctcu: Enable byte-cntr function for TMC ETR Jie Gan ` (3 preceding siblings ...) 2026-06-25 10:45 ` [PATCH v19 4/7] coresight: etr: add a new function to retrieve the CTCU device Jie Gan @ 2026-06-25 10:45 ` Jie Gan 2026-06-25 10:45 ` [PATCH v19 6/7] coresight: ctcu: enable byte-cntr for TMC ETR devices Jie Gan 2026-06-25 10:45 ` [PATCH v19 7/7] arm64: dts: qcom: lemans: add interrupts to CTCU device Jie Gan 6 siblings, 0 replies; 13+ messages in thread From: Jie Gan @ 2026-06-25 10:45 UTC (permalink / raw) To: Suzuki K Poulose, Mike Leach, James Clark, Leo Yan, Alexander Shishkin, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Tingwei Zhang, Jie Gan, Bjorn Andersson, Konrad Dybcio, Yuanfang Zhang, Mao Jinlong Cc: coresight, linux-arm-kernel, linux-kernel, linux-arm-msm, devicetree, Krzysztof Kozlowski, Mike Leach 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> --- Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml index e002f87361ad..2981001a7d7f 100644 --- a/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml @@ -44,6 +44,11 @@ properties: items: - const: apb + interrupts: + items: + - description: Interrupt for the ETR device connected to in-port0. + - description: Interrupt for the ETR device connected to in-port1. + label: description: Description of a coresight device. @@ -65,6 +70,8 @@ additionalProperties: false examples: - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + ctcu@1001000 { compatible = "qcom,sa8775p-ctcu"; reg = <0x1001000 0x1000>; @@ -72,6 +79,9 @@ examples: clocks = <&aoss_qmp>; clock-names = "apb"; + interrupts = <GIC_SPI 270 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 262 IRQ_TYPE_EDGE_RISING>; + in-ports { #address-cells = <1>; #size-cells = <0>; -- 2.34.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v19 6/7] coresight: ctcu: enable byte-cntr for TMC ETR devices 2026-06-25 10:45 [PATCH v19 0/7] coresight: ctcu: Enable byte-cntr function for TMC ETR Jie Gan ` (4 preceding siblings ...) 2026-06-25 10:45 ` [PATCH v19 5/7] dt-bindings: arm: add an interrupt property for Coresight CTCU Jie Gan @ 2026-06-25 10:45 ` Jie Gan 2026-06-25 11:09 ` sashiko-bot 2026-06-25 10:45 ` [PATCH v19 7/7] arm64: dts: qcom: lemans: add interrupts to CTCU device Jie Gan 6 siblings, 1 reply; 13+ messages in thread From: Jie Gan @ 2026-06-25 10:45 UTC (permalink / raw) To: Suzuki K Poulose, Mike Leach, James Clark, Leo Yan, Alexander Shishkin, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Tingwei Zhang, Jie Gan, Bjorn Andersson, Konrad Dybcio, Yuanfang Zhang, Mao Jinlong Cc: coresight, linux-arm-kernel, linux-kernel, linux-arm-msm, devicetree The byte-cntr function provided by the CTCU device is used to transfer data from the ETR buffer to the userspace. 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 and the read function will read the data from the synced ETR buffer. Switching the sysfs_buf when current buffer is full or the timeout is triggered and resets rrp and rwp registers after switched the buffer. The synced buffer will become available for reading after the switch. Byte-cntr workflow: start -> ctcu_enable(ctcu_byte_cntr_start) -> tmc_enable_etr_sink -> tmc_read_prepare_etr(jump to tmc_read_prepare_byte_cntr) -> tmc_etr_get_sysfs_trace(jump to tmc_byte_cntr_get_data) -> tmc_disable_etr_sink -> ctcu_disable(ctcu_byte_cntr_stop) -> tmc_read_unprepare_etr(jump to tmc_read_unprepare_byte_cntr) -> finish Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com> --- .../ABI/testing/sysfs-bus-coresight-devices-ctcu | 9 + drivers/hwtracing/coresight/Makefile | 2 +- .../hwtracing/coresight/coresight-ctcu-byte-cntr.c | 327 +++++++++++++++++++++ drivers/hwtracing/coresight/coresight-ctcu-core.c | 127 +++++++- drivers/hwtracing/coresight/coresight-ctcu.h | 81 ++++- drivers/hwtracing/coresight/coresight-tmc-core.c | 3 +- drivers/hwtracing/coresight/coresight-tmc-etr.c | 115 +++++++- drivers/hwtracing/coresight/coresight-tmc.h | 9 + 8 files changed, 647 insertions(+), 26 deletions(-) diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-ctcu b/Documentation/ABI/testing/sysfs-bus-coresight-devices-ctcu new file mode 100644 index 000000000000..beef0be21969 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-ctcu @@ -0,0 +1,9 @@ +What: /sys/bus/coresight/devices/<ctcu-name>/irq_enabled[0:1] +Date: June 2026 +KernelVersion: 7.3 +Contact: Tingwei Zhang <tingwei.zhang@oss.qualcomm.com>; Jinlong Mao <jinlong.mao@oss.qualcomm.com>; Jie Gan <jie.gan@oss.qualcomm.com> +Description: + (RW) Configure the flag to enable interrupt to count data during CTCU enablement. + An interrupt is generated when the data size exceeds the value set in the IRQ register. + 0 : disable + 1 : enable diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile index ab16d06783a5..821a1b06b20c 100644 --- a/drivers/hwtracing/coresight/Makefile +++ b/drivers/hwtracing/coresight/Makefile @@ -55,5 +55,5 @@ coresight-cti-y := coresight-cti-core.o coresight-cti-platform.o \ obj-$(CONFIG_ULTRASOC_SMB) += ultrasoc-smb.o obj-$(CONFIG_CORESIGHT_DUMMY) += coresight-dummy.o obj-$(CONFIG_CORESIGHT_CTCU) += coresight-ctcu.o -coresight-ctcu-y := coresight-ctcu-core.o +coresight-ctcu-y := coresight-ctcu-core.o coresight-ctcu-byte-cntr.o obj-$(CONFIG_CORESIGHT_KUNIT_TESTS) += coresight-kunit-tests.o diff --git a/drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c b/drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c new file mode 100644 index 000000000000..5ab97a71f02f --- /dev/null +++ b/drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c @@ -0,0 +1,327 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries. + */ + +#include <linux/coresight.h> +#include <linux/device.h> +#include <linux/fs.h> +#include <linux/interrupt.h> +#include <linux/of_irq.h> +#include <linux/uaccess.h> + +#include "coresight-ctcu.h" +#include "coresight-priv.h" +#include "coresight-tmc.h" + +static irqreturn_t byte_cntr_handler(int irq, void *data) +{ + struct ctcu_byte_cntr *byte_cntr_data = data; + + atomic_inc(&byte_cntr_data->irq_cnt); + wake_up(&byte_cntr_data->wq); + + return IRQ_HANDLED; +} + +static void ctcu_cfg_byte_cntr_reg(struct ctcu_drvdata *drvdata, u32 val, + u32 offset) +{ + /* A one value for IRQCTRL register represents 8 bytes */ + ctcu_program_register(drvdata, val / 8, offset); +} + +static struct ctcu_byte_cntr *ctcu_get_byte_cntr(struct coresight_device *ctcu, + struct coresight_device *etr) +{ + struct ctcu_drvdata *drvdata = dev_get_drvdata(ctcu->dev.parent); + int port; + + port = coresight_get_in_port(etr, ctcu); + if (port < 0 || port > 1) + return NULL; + + return &drvdata->byte_cntr_data[port]; +} + +static bool ctcu_byte_cntr_switch_buffer(struct tmc_drvdata *etr_drvdata, + struct ctcu_byte_cntr *byte_cntr_data) +{ + struct etr_buf_node *nd, *next, *curr_node = NULL, *picked_node = NULL; + struct etr_buf *curr_buf = etr_drvdata->sysfs_buf; + bool found_free_buf = false; + unsigned long flags; + + if (WARN_ON(!etr_drvdata || !byte_cntr_data)) + return false; + + /* Stop the ETR before initiating the switch */ + if (coresight_get_mode(etr_drvdata->csdev) != CS_MODE_DISABLED) + tmc_etr_enable_disable_hw(etr_drvdata, false); + + /* + * Serialise the sysfs_buf/etr_buf swap against the ETR sink + * enable/disable paths which also touch these fields under the + * spinlock. tmc_etr_enable_disable_hw() takes the same lock, so it + * must be called outside this critical section. + */ + raw_spin_lock_irqsave(&etr_drvdata->spinlock, flags); + list_for_each_entry_safe(nd, next, &etr_drvdata->etr_buf_list, link) { + /* curr_buf is free for next round */ + if (nd->sysfs_buf == curr_buf) { + nd->is_free = true; + curr_node = nd; + } else if (!found_free_buf && nd->is_free) { + picked_node = nd; + found_free_buf = true; + } + } + + if (found_free_buf) { + curr_node->pos = 0; + curr_node->reading = true; + byte_cntr_data->buf_node = curr_node; + etr_drvdata->sysfs_buf = picked_node->sysfs_buf; + etr_drvdata->etr_buf = picked_node->sysfs_buf; + picked_node->is_free = false; + /* Reset irq_cnt for next etr_buf */ + atomic_set(&byte_cntr_data->irq_cnt, 0); + } + raw_spin_unlock_irqrestore(&etr_drvdata->spinlock, flags); + + /* Restart the ETR once a free buffer is available */ + if (found_free_buf && + coresight_get_mode(etr_drvdata->csdev) != CS_MODE_DISABLED) + tmc_etr_enable_disable_hw(etr_drvdata, true); + + return found_free_buf; +} + +/* + * ctcu_byte_cntr_get_data() - reads data from the deactivated and filled buffer. + * The byte-cntr reading work reads data from the deactivated and filled buffer. + * The read operation waits for a buffer to become available, either filled or + * upon timeout, and then reads trace data from the synced buffer. + */ +static ssize_t tmc_byte_cntr_get_data(struct tmc_drvdata *etr_drvdata, loff_t pos, + size_t len, char **bufpp) +{ + struct coresight_device *ctcu = tmc_etr_get_ctcu_device(etr_drvdata); + struct device *dev = &etr_drvdata->csdev->dev; + struct ctcu_byte_cntr *byte_cntr_data; + struct etr_buf *sysfs_buf; + atomic_t *irq_cnt; + ssize_t actual; + int ret; + + byte_cntr_data = ctcu_get_byte_cntr(ctcu, etr_drvdata->csdev); + if (!byte_cntr_data || !byte_cntr_data->irq_enabled) + return -EINVAL; + + irq_cnt = &byte_cntr_data->irq_cnt; + +wait_buffer: + if (!byte_cntr_data->buf_node) { + ret = wait_event_interruptible_timeout(byte_cntr_data->wq, + (atomic_read(irq_cnt) >= MAX_IRQ_CNT - 1) || + !byte_cntr_data->enable, + BYTE_CNTR_TIMEOUT); + if (ret < 0) + return ret; + /* + * The current etr_buf is almost full or timeout is triggered, + * so switch the buffer and mark the switched buffer as reading. + */ + if (byte_cntr_data->enable) { + if (!ctcu_byte_cntr_switch_buffer(etr_drvdata, byte_cntr_data)) { + dev_err(dev, "Switch buffer failed for the byte-cntr\n"); + return -ENOMEM; + } + } else { + /* Exit byte-cntr reading */ + return 0; + } + } + + /* Check the status of current etr_buf */ + if (atomic_read(irq_cnt) >= MAX_IRQ_CNT) + dev_warn(dev, "Data overwrite happened\n"); + + pos = byte_cntr_data->buf_node->pos; + sysfs_buf = byte_cntr_data->buf_node->sysfs_buf; + actual = tmc_etr_read_sysfs_buf(sysfs_buf, pos, len, bufpp); + if (actual <= 0) { + /* Reset buf_node upon reading is finished or failed */ + byte_cntr_data->buf_node->reading = false; + byte_cntr_data->buf_node = NULL; + + /* + * Nothing in the buffer, waiting for the next buffer + * to be filled. + */ + if (actual == 0) + goto wait_buffer; + } + + return actual; +} + +static int tmc_read_prepare_byte_cntr(struct tmc_drvdata *etr_drvdata) +{ + struct coresight_device *ctcu = tmc_etr_get_ctcu_device(etr_drvdata); + struct ctcu_byte_cntr *byte_cntr_data; + unsigned long flags; + int ret = 0; + + /* byte-cntr is operating with SYSFS mode being enabled only */ + if (coresight_get_mode(etr_drvdata->csdev) != CS_MODE_SYSFS) + return -EINVAL; + + byte_cntr_data = ctcu_get_byte_cntr(ctcu, etr_drvdata->csdev); + if (!byte_cntr_data || !byte_cntr_data->irq_enabled) + return -EINVAL; + + raw_spin_lock_irqsave(&byte_cntr_data->spin_lock, flags); + if (byte_cntr_data->reading) { + raw_spin_unlock_irqrestore(&byte_cntr_data->spin_lock, flags); + return -EBUSY; + } + + /* byte_cntr_data->enable may race with ctcu_platform_remove() */ + if (!byte_cntr_data->enable) { + raw_spin_unlock_irqrestore(&byte_cntr_data->spin_lock, flags); + return -ENODEV; + } + + byte_cntr_data->reading = true; + raw_spin_unlock_irqrestore(&byte_cntr_data->spin_lock, flags); + /* Setup an available etr_buf_list for byte-cntr */ + ret = tmc_create_etr_buf_list(etr_drvdata, 2); + if (ret) { + byte_cntr_data->reading = false; + return ret; + } + + scoped_guard(raw_spinlock_irqsave, &byte_cntr_data->spin_lock) { + atomic_set(&byte_cntr_data->irq_cnt, 0); + /* + * Configure the byte-cntr register to enable IRQ. The + * configured size is 5% of the buffer_size. + */ + ctcu_cfg_byte_cntr_reg(byte_cntr_data->ctcu_drvdata, + etr_drvdata->size / MAX_IRQ_CNT, + byte_cntr_data->irq_ctrl_offset); + byte_cntr_data->buf_node = NULL; + } + /* enable_irq_wake() may sleep on slow-bus irqchips, call it unlocked */ + enable_irq_wake(byte_cntr_data->irq); + + return 0; +} + +static int tmc_read_unprepare_byte_cntr(struct tmc_drvdata *etr_drvdata) +{ + struct coresight_device *ctcu = tmc_etr_get_ctcu_device(etr_drvdata); + struct ctcu_byte_cntr *byte_cntr_data; + + /* + * Do the unprepare operation only when the byte_cntr_data->reading + * is truly set + */ + byte_cntr_data = ctcu_get_byte_cntr(ctcu, etr_drvdata->csdev); + if (!byte_cntr_data || !byte_cntr_data->irq_enabled || + !byte_cntr_data->reading) + return -EINVAL; + + tmc_clean_etr_buf_list(etr_drvdata); + scoped_guard(raw_spinlock_irqsave, &byte_cntr_data->spin_lock) { + /* Configure the byte-cntr register to disable IRQ */ + ctcu_cfg_byte_cntr_reg(byte_cntr_data->ctcu_drvdata, 0, + byte_cntr_data->irq_ctrl_offset); + byte_cntr_data->buf_node = NULL; + byte_cntr_data->reading = false; + } + /* + * The threshold IRQ is already disabled by the register write above, + * so no wake event can arrive here. disable_irq_wake() may sleep on + * slow-bus irqchips, so call it outside the spin_lock. + */ + disable_irq_wake(byte_cntr_data->irq); + wake_up(&byte_cntr_data->wq); + + return 0; +} + +const struct tmc_sysfs_ops byte_cntr_sysfs_ops = { + .read_prepare = tmc_read_prepare_byte_cntr, + .read_unprepare = tmc_read_unprepare_byte_cntr, + .get_trace_data = tmc_byte_cntr_get_data, +}; + +/* Start the byte-cntr function when the path is enabled. */ +void ctcu_byte_cntr_start(struct coresight_device *csdev, struct coresight_path *path) +{ + struct coresight_device *sink = coresight_get_sink(path); + struct ctcu_byte_cntr *byte_cntr_data; + + byte_cntr_data = ctcu_get_byte_cntr(csdev, sink); + if (!byte_cntr_data) + return; + + /* Don't start byte-cntr function when irq_enabled is not set. */ + if (!byte_cntr_data->irq_enabled || byte_cntr_data->enable) + return; + + guard(raw_spinlock_irqsave)(&byte_cntr_data->spin_lock); + byte_cntr_data->enable = true; +} + +/* Stop the byte-cntr function when the path is disabled. */ +void ctcu_byte_cntr_stop(struct coresight_device *csdev, struct coresight_path *path) +{ + struct coresight_device *sink = coresight_get_sink(path); + struct ctcu_byte_cntr *byte_cntr_data; + + if (coresight_get_mode(sink) == CS_MODE_SYSFS) + return; + + byte_cntr_data = ctcu_get_byte_cntr(csdev, sink); + if (!byte_cntr_data) + return; + + guard(raw_spinlock_irqsave)(&byte_cntr_data->spin_lock); + byte_cntr_data->enable = false; +} + +void ctcu_byte_cntr_init(struct device *dev, struct ctcu_drvdata *drvdata, int etr_num) +{ + struct ctcu_byte_cntr *byte_cntr_data; + struct device_node *nd = dev->of_node; + int irq_num, ret, i, irq_registered = 0; + + for (i = 0; i < etr_num; i++) { + byte_cntr_data = &drvdata->byte_cntr_data[i]; + irq_num = of_irq_get(nd, i); + if (irq_num < 0) { + dev_err(dev, "Failed to get IRQ from DT for port%d\n", i); + continue; + } + + ret = devm_request_irq(dev, irq_num, byte_cntr_handler, + IRQF_TRIGGER_RISING | IRQF_SHARED, + dev_name(dev), byte_cntr_data); + if (ret) { + dev_err(dev, "Failed to register IRQ for port%d\n", i); + continue; + } + + byte_cntr_data->irq = irq_num; + byte_cntr_data->ctcu_drvdata = drvdata; + init_waitqueue_head(&byte_cntr_data->wq); + raw_spin_lock_init(&byte_cntr_data->spin_lock); + irq_registered++; + } + + if (irq_registered) + tmc_etr_set_byte_cntr_sysfs_ops(&byte_cntr_sysfs_ops); +} diff --git a/drivers/hwtracing/coresight/coresight-ctcu-core.c b/drivers/hwtracing/coresight/coresight-ctcu-core.c index e8720026c9e3..2da1a6f3d29f 100644 --- a/drivers/hwtracing/coresight/coresight-ctcu-core.c +++ b/drivers/hwtracing/coresight/coresight-ctcu-core.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0-only /* - * Copyright (c) 2024-2025 Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) 2024-2026 Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries. */ #include <linux/clk.h> @@ -18,6 +19,7 @@ #include "coresight-ctcu.h" #include "coresight-priv.h" +#include "coresight-tmc.h" #define ctcu_writel(drvdata, val, offset) __raw_writel((val), drvdata->base + offset) #define ctcu_readl(drvdata, offset) __raw_readl(drvdata->base + offset) @@ -43,17 +45,21 @@ #define CTCU_ATID_REG_BIT(traceid) (traceid % 32) #define CTCU_ATID_REG_SIZE 0x10 +#define CTCU_ETR0_IRQCTRL 0x6c +#define CTCU_ETR1_IRQCTRL 0x70 #define CTCU_ETR0_ATID0 0xf8 #define CTCU_ETR1_ATID0 0x108 static const struct ctcu_etr_config sa8775p_etr_cfgs[] = { { - .atid_offset = CTCU_ETR0_ATID0, - .port_num = 0, + .atid_offset = CTCU_ETR0_ATID0, + .irq_ctrl_offset = CTCU_ETR0_IRQCTRL, + .port_num = 0, }, { - .atid_offset = CTCU_ETR1_ATID0, - .port_num = 1, + .atid_offset = CTCU_ETR1_ATID0, + .irq_ctrl_offset = CTCU_ETR1_IRQCTRL, + .port_num = 1, }, }; @@ -62,6 +68,85 @@ static const struct ctcu_config sa8775p_cfgs = { .num_etr_config = ARRAY_SIZE(sa8775p_etr_cfgs), }; +void ctcu_program_register(struct ctcu_drvdata *drvdata, u32 val, u32 offset) +{ + CS_UNLOCK(drvdata->base); + ctcu_writel(drvdata, val, offset); + CS_LOCK(drvdata->base); +} + +static ssize_t irq_enabled_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct ctcu_byte_cntr_irq_attribute *irq_attr = + container_of(attr, struct ctcu_byte_cntr_irq_attribute, attr); + struct ctcu_drvdata *drvdata = dev_get_drvdata(dev->parent); + u8 port = irq_attr->port; + + if (!drvdata->byte_cntr_data[port].irq_ctrl_offset) + return -EINVAL; + + return sysfs_emit(buf, "%u\n", + (unsigned int)drvdata->byte_cntr_data[port].irq_enabled); +} + +static ssize_t irq_enabled_store(struct device *dev, + struct device_attribute *attr, + const char *buf, + size_t size) +{ + struct ctcu_byte_cntr_irq_attribute *irq_attr = + container_of(attr, struct ctcu_byte_cntr_irq_attribute, attr); + struct ctcu_drvdata *drvdata = dev_get_drvdata(dev->parent); + u8 port = irq_attr->port; + unsigned long val; + + if (kstrtoul(buf, 0, &val)) + return -EINVAL; + + guard(raw_spinlock_irqsave)(&drvdata->byte_cntr_data[port].spin_lock); + if (drvdata->byte_cntr_data[port].reading) + return -EBUSY; + else if (drvdata->byte_cntr_data[port].irq_ctrl_offset) + drvdata->byte_cntr_data[port].irq_enabled = !!val; + + return size; +} + +static umode_t irq_enabled_is_visible(struct kobject *kobj, + struct attribute *attr, int n) +{ + struct device_attribute *dev_attr = + container_of(attr, struct device_attribute, attr); + struct ctcu_byte_cntr_irq_attribute *irq_attr = + container_of(dev_attr, struct ctcu_byte_cntr_irq_attribute, attr); + struct device *dev = kobj_to_dev(kobj); + struct ctcu_drvdata *drvdata = dev_get_drvdata(dev->parent); + u8 port = irq_attr->port; + + if (drvdata && drvdata->byte_cntr_data[port].irq_ctrl_offset) + return attr->mode; + + return 0; +} + +static struct attribute *ctcu_attrs[] = { + ctcu_byte_cntr_irq_rw(0), + ctcu_byte_cntr_irq_rw(1), + NULL, +}; + +static struct attribute_group ctcu_attr_grp = { + .attrs = ctcu_attrs, + .is_visible = irq_enabled_is_visible, +}; + +static const struct attribute_group *ctcu_attr_grps[] = { + &ctcu_attr_grp, + NULL, +}; + static void ctcu_program_atid_register(struct ctcu_drvdata *drvdata, u32 reg_offset, u8 bit, bool enable) { @@ -140,11 +225,15 @@ static int ctcu_set_etr_traceid(struct coresight_device *csdev, struct coresight static int ctcu_enable(struct coresight_device *csdev, enum cs_mode mode, struct coresight_path *path) { + ctcu_byte_cntr_start(csdev, path); + return ctcu_set_etr_traceid(csdev, path, true); } static int ctcu_disable(struct coresight_device *csdev, struct coresight_path *path) { + ctcu_byte_cntr_stop(csdev, path); + return ctcu_set_etr_traceid(csdev, path, false); } @@ -195,7 +284,10 @@ static int ctcu_probe(struct platform_device *pdev) for (i = 0; i < cfgs->num_etr_config; i++) { etr_cfg = &cfgs->etr_cfgs[i]; drvdata->atid_offset[i] = etr_cfg->atid_offset; + drvdata->byte_cntr_data[i].irq_ctrl_offset = + etr_cfg->irq_ctrl_offset; } + ctcu_byte_cntr_init(dev, drvdata, cfgs->num_etr_config); } } @@ -209,6 +301,7 @@ static int ctcu_probe(struct platform_device *pdev) desc.dev = dev; desc.ops = &ctcu_ops; desc.access = CSDEV_ACCESS_IOMEM(base); + desc.groups = ctcu_attr_grps; raw_spin_lock_init(&drvdata->spin_lock); drvdata->csdev = coresight_register(&desc); @@ -244,10 +337,34 @@ static int ctcu_platform_probe(struct platform_device *pdev) static void ctcu_platform_remove(struct platform_device *pdev) { struct ctcu_drvdata *drvdata = platform_get_drvdata(pdev); + struct ctcu_byte_cntr *byte_cntr_data; + unsigned long flags; + int i; if (WARN_ON(!drvdata)) return; + /* + * Signal all active byte-cntr readers to exit, then wait for them to + * finish before resetting the ops pointer and freeing driver data. + * Without this, a reader blocked in wait_event_interruptible_timeout() + * would access the freed ctcu_drvdata wait-queue head (use-after-free). + */ + for (i = 0; i < ETR_MAX_NUM; i++) { + byte_cntr_data = &drvdata->byte_cntr_data[i]; + raw_spin_lock_irqsave(&byte_cntr_data->spin_lock, flags); + /* Set enable=false for all ports to signal teardown to racing readers */ + byte_cntr_data->enable = false; + if (!byte_cntr_data->reading) { + raw_spin_unlock_irqrestore(&byte_cntr_data->spin_lock, flags); + continue; + } + raw_spin_unlock_irqrestore(&byte_cntr_data->spin_lock, flags); + wake_up_all(&byte_cntr_data->wq); + wait_event(byte_cntr_data->wq, !byte_cntr_data->reading); + } + + tmc_etr_reset_byte_cntr_sysfs_ops(); ctcu_remove(pdev); pm_runtime_disable(&pdev->dev); } diff --git a/drivers/hwtracing/coresight/coresight-ctcu.h b/drivers/hwtracing/coresight/coresight-ctcu.h index e9594c38dd91..a2ae0a0d91d0 100644 --- a/drivers/hwtracing/coresight/coresight-ctcu.h +++ b/drivers/hwtracing/coresight/coresight-ctcu.h @@ -1,23 +1,31 @@ /* SPDX-License-Identifier: GPL-2.0-only */ /* - * Copyright (c) 2024-2025 Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) 2024-2026 Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries. */ #ifndef _CORESIGHT_CTCU_H #define _CORESIGHT_CTCU_H + +#include <linux/time.h> #include "coresight-trace-id.h" /* Maximum number of supported ETR devices for a single CTCU. */ #define ETR_MAX_NUM 2 +#define BYTE_CNTR_TIMEOUT (3 * HZ) +#define MAX_IRQ_CNT 20 + /** * struct ctcu_etr_config * @atid_offset: offset to the ATID0 Register. - * @port_num: in-port number of CTCU device that connected to ETR. + * @port_num: in-port number of the CTCU device that connected to ETR. + * @irq_ctrl_offset: offset to the BYTECNTRVAL register. */ struct ctcu_etr_config { const u32 atid_offset; const u32 port_num; + const u32 irq_ctrl_offset; }; struct ctcu_config { @@ -25,15 +33,68 @@ struct ctcu_config { int num_etr_config; }; -struct ctcu_drvdata { - void __iomem *base; - struct clk *apb_clk; - struct device *dev; - struct coresight_device *csdev; +/** + * struct ctcu_byte_cntr + * @enable: indicates that byte_cntr function is enabled or not. + * @irq_enabled: indicates that the interruption is enabled. + * @reading: indicates that byte_cntr is reading. + * @irq: allocated number of the IRQ. + * @irq_cnt: IRQ count number of the triggered interruptions. + * @wq: waitqueue for reading data from ETR buffer. + * @spin_lock: spinlock of the byte_cntr_data. + * @irq_ctrl_offset: offset to the BYTECNTVAL Register. + * @ctcu_drvdata: drvdata of the CTCU device. + * @buf_node: etr_buf_node for reading. + */ +struct ctcu_byte_cntr { + bool enable; + bool irq_enabled; + bool reading; + int irq; + atomic_t irq_cnt; + wait_queue_head_t wq; raw_spinlock_t spin_lock; - u32 atid_offset[ETR_MAX_NUM]; - /* refcnt for each traceid of each sink */ - u8 traceid_refcnt[ETR_MAX_NUM][CORESIGHT_TRACE_ID_RES_TOP]; + u32 irq_ctrl_offset; + struct ctcu_drvdata *ctcu_drvdata; + struct etr_buf_node *buf_node; }; +struct ctcu_drvdata { + void __iomem *base; + struct clk *apb_clk; + struct device *dev; + struct coresight_device *csdev; + struct ctcu_byte_cntr byte_cntr_data[ETR_MAX_NUM]; + raw_spinlock_t spin_lock; + u32 atid_offset[ETR_MAX_NUM]; + /* refcnt for each traceid of each sink */ + u8 traceid_refcnt[ETR_MAX_NUM][CORESIGHT_TRACE_ID_RES_TOP]; +}; + +/** + * struct ctcu_byte_cntr_irq_attribute + * @attr: The device attribute. + * @port: port number. + */ +struct ctcu_byte_cntr_irq_attribute { + struct device_attribute attr; + u8 port; +}; + +#define ctcu_byte_cntr_irq_rw(port) \ + (&((struct ctcu_byte_cntr_irq_attribute[]) { \ + { \ + __ATTR(irq_enabled##port, 0644, irq_enabled_show, \ + irq_enabled_store), \ + port, \ + } \ + })[0].attr.attr) + +void ctcu_program_register(struct ctcu_drvdata *drvdata, u32 val, u32 offset); + +/* Byte-cntr functions */ +void ctcu_byte_cntr_start(struct coresight_device *csdev, struct coresight_path *path); +void ctcu_byte_cntr_stop(struct coresight_device *csdev, struct coresight_path *path); +void ctcu_byte_cntr_init(struct device *dev, struct ctcu_drvdata *drvdata, int port_num); + #endif diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c index 4b40b692be4d..6ad09995ba87 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-core.c +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c @@ -293,7 +293,8 @@ static ssize_t tmc_read(struct file *file, char __user *data, size_t len, return -EFAULT; } - *ppos += actual; + if (!tmc_etr_update_buf_node_pos(drvdata, actual)) + *ppos += actual; dev_dbg(&drvdata->csdev->dev, "%zu bytes copied\n", actual); return actual; diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index 2b26ce6455a7..e78f8891f11e 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -1168,6 +1168,9 @@ static int tmc_etr_enable_hw(struct tmc_drvdata *drvdata, return rc; } +/* Assumes a single CTCU instance per system, as on all current Qualcomm SoCs. */ +static const struct tmc_sysfs_ops *byte_cntr_sysfs_ops; + /* * Return the available trace data in the buffer (starts at etr_buf->offset, * limited by etr_buf->len) from @pos, with a maximum limit of @len, @@ -1178,23 +1181,39 @@ static int tmc_etr_enable_hw(struct tmc_drvdata *drvdata, * We are protected here by drvdata->reading != 0, which ensures the * sysfs_buf stays alive. */ -ssize_t tmc_etr_get_sysfs_trace(struct tmc_drvdata *drvdata, - loff_t pos, size_t len, char **bufpp) +ssize_t tmc_etr_read_sysfs_buf(struct etr_buf *sysfs_buf, loff_t pos, + size_t len, char **bufpp) { s64 offset; ssize_t actual = len; - struct etr_buf *etr_buf = drvdata->sysfs_buf; - if (pos + actual > etr_buf->len) - actual = etr_buf->len - pos; + if (pos + actual > sysfs_buf->len) + actual = sysfs_buf->len - pos; if (actual <= 0) return actual; /* Compute the offset from which we read the data */ - offset = etr_buf->offset + pos; - if (offset >= etr_buf->size) - offset -= etr_buf->size; - return tmc_etr_buf_get_data(etr_buf, offset, actual, bufpp); + offset = sysfs_buf->offset + pos; + if (offset >= sysfs_buf->size) + offset -= sysfs_buf->size; + return tmc_etr_buf_get_data(sysfs_buf, offset, actual, bufpp); +} +EXPORT_SYMBOL_GPL(tmc_etr_read_sysfs_buf); + +ssize_t tmc_etr_get_sysfs_trace(struct tmc_drvdata *drvdata, + loff_t pos, size_t len, char **bufpp) +{ + ssize_t ret; + const struct tmc_sysfs_ops *byte_cntr_ops = READ_ONCE(byte_cntr_sysfs_ops); + + if (byte_cntr_ops) { + ret = byte_cntr_ops->get_trace_data(drvdata, pos, len, bufpp); + /* Return the filled buffer */ + if (ret > 0 || ret == -ENOMEM) + return ret; + } + + return tmc_etr_read_sysfs_buf(drvdata->sysfs_buf, pos, len, bufpp); } static struct etr_buf * @@ -1248,6 +1267,39 @@ static void __tmc_etr_disable_hw(struct tmc_drvdata *drvdata) } +static void tmc_etr_reset_sysfs_buf(struct tmc_drvdata *drvdata) +{ + u32 sts; + + CS_UNLOCK(drvdata->base); + tmc_write_rrp(drvdata, drvdata->sysfs_buf->hwaddr); + tmc_write_rwp(drvdata, drvdata->sysfs_buf->hwaddr); + sts = readl_relaxed(drvdata->base + TMC_STS) & ~TMC_STS_FULL; + writel_relaxed(sts, drvdata->base + TMC_STS); + CS_LOCK(drvdata->base); +} + +/** + * tmc_etr_enable_disable_hw - enable/disable the ETR hw. + * @drvdata: drvdata of the TMC device. + * @enable: indicates enable/disable. + */ +void tmc_etr_enable_disable_hw(struct tmc_drvdata *drvdata, bool enable) +{ + unsigned long flags; + + raw_spin_lock_irqsave(&drvdata->spinlock, flags); + if (enable) { + tmc_etr_reset_sysfs_buf(drvdata); + __tmc_etr_enable_hw(drvdata); + } else { + __tmc_etr_disable_hw(drvdata); + } + + raw_spin_unlock_irqrestore(&drvdata->spinlock, flags); +} +EXPORT_SYMBOL_GPL(tmc_etr_enable_disable_hw); + void tmc_etr_disable_hw(struct tmc_drvdata *drvdata) { __tmc_etr_disable_hw(drvdata); @@ -2068,15 +2120,54 @@ int tmc_create_etr_buf_list(struct tmc_drvdata *drvdata, int num_nodes) } EXPORT_SYMBOL_GPL(tmc_create_etr_buf_list); +void tmc_etr_set_byte_cntr_sysfs_ops(const struct tmc_sysfs_ops *sysfs_ops) +{ + WRITE_ONCE(byte_cntr_sysfs_ops, sysfs_ops); +} +EXPORT_SYMBOL_GPL(tmc_etr_set_byte_cntr_sysfs_ops); + +void tmc_etr_reset_byte_cntr_sysfs_ops(void) +{ + WRITE_ONCE(byte_cntr_sysfs_ops, NULL); +} +EXPORT_SYMBOL_GPL(tmc_etr_reset_byte_cntr_sysfs_ops); + +bool tmc_etr_update_buf_node_pos(struct tmc_drvdata *drvdata, ssize_t size) +{ + struct etr_buf_node *nd, *next; + + if (drvdata->config_type != TMC_CONFIG_TYPE_ETR) + return false; + + list_for_each_entry_safe(nd, next, &drvdata->etr_buf_list, link) { + if (nd && nd->reading) { + nd->pos += size; + return true; + } + } + + return false; +} + int tmc_read_prepare_etr(struct tmc_drvdata *drvdata) { int ret = 0; unsigned long flags; + const struct tmc_sysfs_ops *byte_cntr_ops; /* config types are set a boot time and never change */ if (WARN_ON_ONCE(drvdata->config_type != TMC_CONFIG_TYPE_ETR)) return -EINVAL; + byte_cntr_ops = READ_ONCE(byte_cntr_sysfs_ops); + if (byte_cntr_ops) { + ret = byte_cntr_ops->read_prepare(drvdata); + if (!ret || ret == -EBUSY) + return ret; + + ret = 0; + } + raw_spin_lock_irqsave(&drvdata->spinlock, flags); if (drvdata->reading) { ret = -EBUSY; @@ -2108,11 +2199,17 @@ int tmc_read_unprepare_etr(struct tmc_drvdata *drvdata) { unsigned long flags; struct etr_buf *sysfs_buf = NULL; + const struct tmc_sysfs_ops *byte_cntr_ops; /* config types are set a boot time and never change */ if (WARN_ON_ONCE(drvdata->config_type != TMC_CONFIG_TYPE_ETR)) return -EINVAL; + byte_cntr_ops = READ_ONCE(byte_cntr_sysfs_ops); + if (byte_cntr_ops) + if (!byte_cntr_ops->read_unprepare(drvdata)) + return 0; + raw_spin_lock_irqsave(&drvdata->spinlock, flags); /* RE-enable the TMC if need be */ diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h index fbb015079872..a15e2f93f16a 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.h +++ b/drivers/hwtracing/coresight/coresight-tmc.h @@ -211,12 +211,15 @@ struct tmc_resrv_buf { /** * @sysfs_buf: Allocated sysfs_buf. * @is_free: Indicates whether the buffer is free to choose. + * @reading: Indicates byte_cntr is reading the buffer attached to + * the node. * @pos: Offset to the start of the buffer. * @link: list_head of the node. */ struct etr_buf_node { struct etr_buf *sysfs_buf; bool is_free; + bool reading; loff_t pos; struct list_head link; }; @@ -480,5 +483,11 @@ struct etr_buf *tmc_etr_get_buffer(struct coresight_device *csdev, extern const struct attribute_group coresight_etr_group; void tmc_clean_etr_buf_list(struct tmc_drvdata *drvdata); int tmc_create_etr_buf_list(struct tmc_drvdata *drvdata, int num_nodes); +void tmc_etr_set_byte_cntr_sysfs_ops(const struct tmc_sysfs_ops *sysfs_ops); +void tmc_etr_reset_byte_cntr_sysfs_ops(void); +void tmc_etr_enable_disable_hw(struct tmc_drvdata *drvdata, bool enable); +bool tmc_etr_update_buf_node_pos(struct tmc_drvdata *drvdata, ssize_t size); +ssize_t tmc_etr_read_sysfs_buf(struct etr_buf *sysfs_buf, loff_t pos, + size_t len, char **bufpp); #endif -- 2.34.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v19 6/7] coresight: ctcu: enable byte-cntr for TMC ETR devices 2026-06-25 10:45 ` [PATCH v19 6/7] coresight: ctcu: enable byte-cntr for TMC ETR devices Jie Gan @ 2026-06-25 11:09 ` sashiko-bot 2026-06-25 13:51 ` Jie Gan 0 siblings, 1 reply; 13+ messages in thread From: sashiko-bot @ 2026-06-25 11:09 UTC (permalink / raw) To: Jie Gan; +Cc: robh, conor+dt, devicetree Thank you for your contribution! Sashiko AI review found 11 potential issue(s) to consider: - [Critical] Global function pointer accessed locklessly leads to Use-After-Free on driver unload - [Critical] Global sysfs ops pointer triggers NULL pointer dereference on ETR without CTCU - [High] Driver remove/unbind blocks indefinitely waiting for userspace file closure - [High] ctcu_byte_cntr_stop() skips setting enable=false for SYSFS mode, causing infinite read hangs - [High] TOCTOU race re-enables ETR hardware after framework disables it - [High] Missing wake_up() in error path deadlocks driver remove() - [High] Unsynchronized checks of irq_enabled lead to resource leaks and driver unbind hangs - [High] Data race on concurrent read() calls causes NULL pointer dereference - [Medium] Spurious Interrupt Handling due to missing hardware status check in shared IRQ - [Medium] Error codes (-ERESTARTSYS) from byte_cntr_ops are swallowed and convert to EOF - [Medium] Masked error code breaks sysfs reading fallback on memory failure -- commit 128ffef0065b7b8a8c73e5e3f9b0ce8bebea0593 Author: Jie Gan <jie.gan@oss.qualcomm.com> coresight: ctcu: enable byte-cntr for TMC ETR devices This patch introduces the byte-cntr function provided by the CTCU device to transfer data from the ETR buffer to userspace. It sets up an interrupt handler to track data size thresholds and switches buffers when full or on timeout. It also integrates with the TMC ETR framework via sysfs ops. > diff --git a/drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c b/drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c > new file mode 100644 > index 0000000000000..5ab97a71f02f6 > --- /dev/null > +++ b/drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c [ ... ] > +static irqreturn_t byte_cntr_handler(int irq, void *data) > +{ > + struct ctcu_byte_cntr *byte_cntr_data = data; > + > + atomic_inc(&byte_cntr_data->irq_cnt); > + wake_up(&byte_cntr_data->wq); > + > + return IRQ_HANDLED; > +} [Severity: Medium] Is this handler missing a hardware status check? Since the IRQ is registered with IRQF_SHARED, another device sharing the line could fire an interrupt. By unconditionally returning IRQ_HANDLED without verifying the interrupt source, the kernel's spurious interrupt detection might be suppressed, and the CTCU buffer switch logic could be triggered prematurely. [ ... ] > +static struct ctcu_byte_cntr *ctcu_get_byte_cntr(struct coresight_device *ctcu, > + struct coresight_device *etr) > +{ > + struct ctcu_drvdata *drvdata = dev_get_drvdata(ctcu->dev.parent); [Severity: Critical] Could this dereference a NULL pointer if an ETR without a connected CTCU is used? If the system has multiple ETRs and one lacks a CTCU, the globally registered byte_cntr_sysfs_ops might pass a NULL ctcu pointer here. This would cause a panic when ctcu->dev.parent is accessed. [ ... ] > +static bool ctcu_byte_cntr_switch_buffer(struct tmc_drvdata *etr_drvdata, > + struct ctcu_byte_cntr *byte_cntr_data) > +{ [ ... ] > + raw_spin_unlock_irqrestore(&etr_drvdata->spinlock, flags); > + > + /* Restart the ETR once a free buffer is available */ > + if (found_free_buf && > + coresight_get_mode(etr_drvdata->csdev) != CS_MODE_DISABLED) > + tmc_etr_enable_disable_hw(etr_drvdata, true); > + > + return found_free_buf; > +} [Severity: High] Does this lockless check introduce a race condition? If a concurrent operation disables the trace session and drops the lock before setting the mode to CS_MODE_DISABLED, this code might incorrectly re-enable the hardware after the framework has explicitly disabled it. [ ... ] > +static ssize_t tmc_byte_cntr_get_data(struct tmc_drvdata *etr_drvdata, loff_t pos, > + size_t len, char **bufpp) > +{ [ ... ] > + pos = byte_cntr_data->buf_node->pos; > + sysfs_buf = byte_cntr_data->buf_node->sysfs_buf; > + actual = tmc_etr_read_sysfs_buf(sysfs_buf, pos, len, bufpp); > + if (actual <= 0) { > + /* Reset buf_node upon reading is finished or failed */ > + byte_cntr_data->buf_node->reading = false; > + byte_cntr_data->buf_node = NULL; [Severity: High] Can concurrent read operations cause a NULL pointer dereference here? If two threads read from the same ETR file descriptor, they could concurrently access and modify byte_cntr_data->buf_node without serialization, causing one thread to clear the pointer while the other is still using it. [ ... ] > +static int tmc_read_prepare_byte_cntr(struct tmc_drvdata *etr_drvdata) > +{ > + struct coresight_device *ctcu = tmc_etr_get_ctcu_device(etr_drvdata); > + struct ctcu_byte_cntr *byte_cntr_data; > + unsigned long flags; > + int ret = 0; > + > + /* byte-cntr is operating with SYSFS mode being enabled only */ > + if (coresight_get_mode(etr_drvdata->csdev) != CS_MODE_SYSFS) > + return -EINVAL; > + > + byte_cntr_data = ctcu_get_byte_cntr(ctcu, etr_drvdata->csdev); > + if (!byte_cntr_data || !byte_cntr_data->irq_enabled) > + return -EINVAL; > + > + raw_spin_lock_irqsave(&byte_cntr_data->spin_lock, flags); [Severity: High] Does checking irq_enabled outside the spinlock create a race? A concurrent sysfs write could set irq_enabled to 0 immediately before the lock is acquired. If reading is then set to true, read_unprepare() will later abort early upon seeing irq_enabled is 0. This would leak the buffer list and leave reading permanently true, causing the driver unbind to hang forever. [ ... ] > + byte_cntr_data->reading = true; > + raw_spin_unlock_irqrestore(&byte_cntr_data->spin_lock, flags); > + /* Setup an available etr_buf_list for byte-cntr */ > + ret = tmc_create_etr_buf_list(etr_drvdata, 2); > + if (ret) { > + byte_cntr_data->reading = false; > + return ret; > + } [Severity: High] Is this error path missing a wake_up() call? If this allocation fails, the reading flag is cleared, but concurrent readers waiting in the driver's remove function won't be notified, potentially deadlocking the unbind process. [ ... ] > +void ctcu_byte_cntr_stop(struct coresight_device *csdev, struct coresight_path *path) > +{ > + struct coresight_device *sink = coresight_get_sink(path); > + struct ctcu_byte_cntr *byte_cntr_data; > + > + if (coresight_get_mode(sink) == CS_MODE_SYSFS) > + return; [Severity: High] Should the enable flag be cleared for SYSFS mode here? By returning early, enable remains true. Readers waiting in tmc_byte_cntr_get_data() might never receive the EOF condition when the trace is stopped, causing them to loop indefinitely. > diff --git a/drivers/hwtracing/coresight/coresight-ctcu-core.c b/drivers/hwtracing/coresight/coresight-ctcu-core.c > index e8720026c9e3f..2da1a6f3d29f4 100644 > --- a/drivers/hwtracing/coresight/coresight-ctcu-core.c > +++ b/drivers/hwtracing/coresight/coresight-ctcu-core.c [ ... ] > +static void ctcu_platform_remove(struct platform_device *pdev) > +{ > + struct ctcu_drvdata *drvdata = platform_get_drvdata(pdev); > + struct ctcu_byte_cntr *byte_cntr_data; > + unsigned long flags; > + int i; > + > + if (WARN_ON(!drvdata)) > + return; > + > + /* > + * Signal all active byte-cntr readers to exit, then wait for them to > + * finish before resetting the ops pointer and freeing driver data. > + * Without this, a reader blocked in wait_event_interruptible_timeout() > + * would access the freed ctcu_drvdata wait-queue head (use-after-free). > + */ > + for (i = 0; i < ETR_MAX_NUM; i++) { > + byte_cntr_data = &drvdata->byte_cntr_data[i]; > + raw_spin_lock_irqsave(&byte_cntr_data->spin_lock, flags); > + /* Set enable=false for all ports to signal teardown to racing readers */ > + byte_cntr_data->enable = false; > + if (!byte_cntr_data->reading) { > + raw_spin_unlock_irqrestore(&byte_cntr_data->spin_lock, flags); > + continue; > + } > + raw_spin_unlock_irqrestore(&byte_cntr_data->spin_lock, flags); > + wake_up_all(&byte_cntr_data->wq); > + wait_event(byte_cntr_data->wq, !byte_cntr_data->reading); > + } [Severity: High] Could this wait indefinitely during driver remove? The reading flag is only cleared when the userspace file is closed. A userspace process leaving the file descriptor open could permanently block module unload or device unbind. > diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c > index 2b26ce6455a7b..e78f8891f11e0 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c > +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c [ ... ] > +ssize_t tmc_etr_get_sysfs_trace(struct tmc_drvdata *drvdata, > + loff_t pos, size_t len, char **bufpp) > +{ > + ssize_t ret; > + const struct tmc_sysfs_ops *byte_cntr_ops = READ_ONCE(byte_cntr_sysfs_ops); > + > + if (byte_cntr_ops) { > + ret = byte_cntr_ops->get_trace_data(drvdata, pos, len, bufpp); > + /* Return the filled buffer */ > + if (ret > 0 || ret == -ENOMEM) > + return ret; > + } > + > + return tmc_etr_read_sysfs_buf(drvdata->sysfs_buf, pos, len, bufpp); > +} [Severity: Medium] Does this silently swallow -ERESTARTSYS? If a signal interrupts the wait inside tmc_byte_cntr_get_data(), it returns -ERESTARTSYS, which falls through here to a normal sysfs buffer read. This might convert the signal into an EOF or return unsynced data instead of letting userspace handle the signal cleanly. [ ... ] > +int tmc_read_prepare_etr(struct tmc_drvdata *drvdata) > +{ > + int ret = 0; > + unsigned long flags; > + const struct tmc_sysfs_ops *byte_cntr_ops; > > /* config types are set a boot time and never change */ > if (WARN_ON_ONCE(drvdata->config_type != TMC_CONFIG_TYPE_ETR)) > return -EINVAL; > > + byte_cntr_ops = READ_ONCE(byte_cntr_sysfs_ops); > + if (byte_cntr_ops) { > + ret = byte_cntr_ops->read_prepare(drvdata); [Severity: Critical] Is it safe to access this global function pointer locklessly? If the trace file is opened and the CTCU module is concurrently unloaded, the memory backing byte_cntr_ops could be freed before read_prepare() executes, leading to a use-after-free regression. > + if (!ret || ret == -EBUSY) > + return ret; > + > + ret = 0; > + } [Severity: Medium] Why are errors like -ENOMEM from read_prepare() masked here? If the memory allocation fails inside tmc_read_prepare_byte_cntr(), hiding the error breaks the fallback sysfs reading path, leading to delays and hanging operations instead of propagating the failure properly. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260625-enable-byte-cntr-for-ctcu-v19-0-8fbbf22e8381@oss.qualcomm.com?part=6 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v19 6/7] coresight: ctcu: enable byte-cntr for TMC ETR devices 2026-06-25 11:09 ` sashiko-bot @ 2026-06-25 13:51 ` Jie Gan 0 siblings, 0 replies; 13+ messages in thread From: Jie Gan @ 2026-06-25 13:51 UTC (permalink / raw) To: sashiko-reviews; +Cc: robh, conor+dt, devicetree On 6/25/2026 7:09 PM, sashiko-bot@kernel.org wrote: > Thank you for your contribution! Sashiko AI review found 11 potential issue(s) to consider: > - [Critical] Global function pointer accessed locklessly leads to Use-After-Free on driver unload > - [Critical] Global sysfs ops pointer triggers NULL pointer dereference on ETR without CTCU > - [High] Driver remove/unbind blocks indefinitely waiting for userspace file closure > - [High] ctcu_byte_cntr_stop() skips setting enable=false for SYSFS mode, causing infinite read hangs > - [High] TOCTOU race re-enables ETR hardware after framework disables it > - [High] Missing wake_up() in error path deadlocks driver remove() > - [High] Unsynchronized checks of irq_enabled lead to resource leaks and driver unbind hangs > - [High] Data race on concurrent read() calls causes NULL pointer dereference > - [Medium] Spurious Interrupt Handling due to missing hardware status check in shared IRQ > - [Medium] Error codes (-ERESTARTSYS) from byte_cntr_ops are swallowed and convert to EOF > - [Medium] Masked error code breaks sysfs reading fallback on memory failure > -- > > commit 128ffef0065b7b8a8c73e5e3f9b0ce8bebea0593 > Author: Jie Gan <jie.gan@oss.qualcomm.com> > > coresight: ctcu: enable byte-cntr for TMC ETR devices > > This patch introduces the byte-cntr function provided by the CTCU device to > transfer data from the ETR buffer to userspace. It sets up an interrupt > handler to track data size thresholds and switches buffers when full or on > timeout. It also integrates with the TMC ETR framework via sysfs ops. > >> diff --git a/drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c b/drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c >> new file mode 100644 >> index 0000000000000..5ab97a71f02f6 >> --- /dev/null >> +++ b/drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c > [ ... ] >> +static irqreturn_t byte_cntr_handler(int irq, void *data) >> +{ >> + struct ctcu_byte_cntr *byte_cntr_data = data; >> + >> + atomic_inc(&byte_cntr_data->irq_cnt); >> + wake_up(&byte_cntr_data->wq); >> + >> + return IRQ_HANDLED; >> +} > > [Severity: Medium] > Is this handler missing a hardware status check? > > Since the IRQ is registered with IRQF_SHARED, another device sharing the line > could fire an interrupt. By unconditionally returning IRQ_HANDLED without > verifying the interrupt source, the kernel's spurious interrupt detection > might be suppressed, and the CTCU buffer switch logic could be triggered > prematurely. Although the driver requests the interrupt with IRQF_SHARED, the underlying hardware/DT topology provides a dedicated IRQ line on supported platforms. So, no other interrupt sources are present, and the handler can safely treat all interrupts as CTCU byte-cntr events. > > [ ... ] >> +static struct ctcu_byte_cntr *ctcu_get_byte_cntr(struct coresight_device *ctcu, >> + struct coresight_device *etr) >> +{ >> + struct ctcu_drvdata *drvdata = dev_get_drvdata(ctcu->dev.parent); > > [Severity: Critical] > Could this dereference a NULL pointer if an ETR without a connected CTCU > is used? > > If the system has multiple ETRs and one lacks a CTCU, the globally registered > byte_cntr_sysfs_ops might pass a NULL ctcu pointer here. This would cause a > panic when ctcu->dev.parent is accessed. This scenario assumes an invalid hardware topology. Byte-cntr functionality is only applicable when an ETR is connected to a CTCU via the CoreSight graph. tmc_etr_get_ctcu_device() discovers the CTCU using this hardware/DT linkage. In the absence of such a connection, no byte-byte path exists. Supported platforms guarantee this connection for byte-cntr capable ETRs; therefore, the reported NULL-CTCU case is outside valid configurations. > > [ ... ] >> +static bool ctcu_byte_cntr_switch_buffer(struct tmc_drvdata *etr_drvdata, >> + struct ctcu_byte_cntr *byte_cntr_data) >> +{ > [ ... ] >> + raw_spin_unlock_irqrestore(&etr_drvdata->spinlock, flags); >> + >> + /* Restart the ETR once a free buffer is available */ >> + if (found_free_buf && >> + coresight_get_mode(etr_drvdata->csdev) != CS_MODE_DISABLED) >> + tmc_etr_enable_disable_hw(etr_drvdata, true); >> + >> + return found_free_buf; >> +} > > [Severity: High] > Does this lockless check introduce a race condition? > > If a concurrent operation disables the trace session and drops the lock before > setting the mode to CS_MODE_DISABLED, this code might incorrectly re-enable > the hardware after the framework has explicitly disabled it. > Disabling the trace session implies that the ETR is already disabled in the byte-cntr read flow, in which case the switch-buffer function cannot be entered. The only possible scenario is that ETR becomes disabled in the middle of a buffer switch operation. However, the code explicitly checks the CoreSight mode (CS_MODE) before re-enabling ETR, ensuring that it is only re-enabled when appropriate. The byte-cntr read path drains the ping-pong buffers regardless of whether the ETR is enabled or disabled. > [ ... ] >> +static ssize_t tmc_byte_cntr_get_data(struct tmc_drvdata *etr_drvdata, loff_t pos, >> + size_t len, char **bufpp) >> +{ > [ ... ] >> + pos = byte_cntr_data->buf_node->pos; >> + sysfs_buf = byte_cntr_data->buf_node->sysfs_buf; >> + actual = tmc_etr_read_sysfs_buf(sysfs_buf, pos, len, bufpp); >> + if (actual <= 0) { >> + /* Reset buf_node upon reading is finished or failed */ >> + byte_cntr_data->buf_node->reading = false; >> + byte_cntr_data->buf_node = NULL; > > [Severity: High] > Can concurrent read operations cause a NULL pointer dereference here? > > If two threads read from the same ETR file descriptor, they could concurrently > access and modify byte_cntr_data->buf_node without serialization, causing one > thread to clear the pointer while the other is still using it. > Byte-cntr supports a single active read session. This is enforced via byte_cntr_data->reading in tmc_read_prepare_byte_cntr(), with concurrent open attempts returning -EBUSY. The buf_node is exclusively owned by this session and cleared during unprepare. The reported race assumes unsupported concurrent consumption of the same session. > [ ... ] >> +static int tmc_read_prepare_byte_cntr(struct tmc_drvdata *etr_drvdata) >> +{ >> + struct coresight_device *ctcu = tmc_etr_get_ctcu_device(etr_drvdata); >> + struct ctcu_byte_cntr *byte_cntr_data; >> + unsigned long flags; >> + int ret = 0; >> + >> + /* byte-cntr is operating with SYSFS mode being enabled only */ >> + if (coresight_get_mode(etr_drvdata->csdev) != CS_MODE_SYSFS) >> + return -EINVAL; >> + >> + byte_cntr_data = ctcu_get_byte_cntr(ctcu, etr_drvdata->csdev); >> + if (!byte_cntr_data || !byte_cntr_data->irq_enabled) >> + return -EINVAL; >> + >> + raw_spin_lock_irqsave(&byte_cntr_data->spin_lock, flags); > > [Severity: High] > Does checking irq_enabled outside the spinlock create a race? > > A concurrent sysfs write could set irq_enabled to 0 immediately before the > lock is acquired. If reading is then set to true, read_unprepare() will later > abort early upon seeing irq_enabled is 0. This would leak the buffer list and > leave reading permanently true, causing the driver unbind to hang forever. > irq_enabled serves as a configuration gate prior to starting byte-cntr and is not a runtime control for active sessions. Once prepare succeeds, operation is governed by enable, reading, hardware registers, waitqueue/timeout, and buffer state. Sysfs prevents modification after reading is set, and toggling irq_enabled during an active session is not a supported use case. > [ ... ] >> + byte_cntr_data->reading = true; >> + raw_spin_unlock_irqrestore(&byte_cntr_data->spin_lock, flags); >> + /* Setup an available etr_buf_list for byte-cntr */ >> + ret = tmc_create_etr_buf_list(etr_drvdata, 2); >> + if (ret) { >> + byte_cntr_data->reading = false; >> + return ret; >> + } > > [Severity: High] > Is this error path missing a wake_up() call? > > If this allocation fails, the reading flag is cleared, but concurrent readers > waiting in the driver's remove function won't be notified, potentially > deadlocking the unbind process. > No wakeup is required in this case. Allocation failure occurs before establishing any byte-cntr read session. The prepare function returns failure to TMC open, meaning no reader exists. wake_up() is only necessary for active sessions that need to be terminated or completed, not for failed prepare paths. > [ ... ] >> +void ctcu_byte_cntr_stop(struct coresight_device *csdev, struct coresight_path *path) >> +{ >> + struct coresight_device *sink = coresight_get_sink(path); >> + struct ctcu_byte_cntr *byte_cntr_data; >> + >> + if (coresight_get_mode(sink) == CS_MODE_SYSFS) >> + return; > > [Severity: High] > Should the enable flag be cleared for SYSFS mode here? > > By returning early, enable remains true. Readers waiting in > tmc_byte_cntr_get_data() might never receive the EOF condition when the > trace is stopped, causing them to loop indefinitely. > The early return is intentional. In SYSFS mode, ETR remains enabled and byte-cntr may actively serve the read path. Forcing enable=false in this state would prematurely terminate the read session and disrupt trace transfer. The enable=false transition is reserved for teardown paths (e.g., CTCU removal or non-SYSFS disable). >> diff --git a/drivers/hwtracing/coresight/coresight-ctcu-core.c b/drivers/hwtracing/coresight/coresight-ctcu-core.c >> index e8720026c9e3f..2da1a6f3d29f4 100644 >> --- a/drivers/hwtracing/coresight/coresight-ctcu-core.c >> +++ b/drivers/hwtracing/coresight/coresight-ctcu-core.c > [ ... ] >> +static void ctcu_platform_remove(struct platform_device *pdev) >> +{ >> + struct ctcu_drvdata *drvdata = platform_get_drvdata(pdev); >> + struct ctcu_byte_cntr *byte_cntr_data; >> + unsigned long flags; >> + int i; >> + >> + if (WARN_ON(!drvdata)) >> + return; >> + >> + /* >> + * Signal all active byte-cntr readers to exit, then wait for them to >> + * finish before resetting the ops pointer and freeing driver data. >> + * Without this, a reader blocked in wait_event_interruptible_timeout() >> + * would access the freed ctcu_drvdata wait-queue head (use-after-free). >> + */ >> + for (i = 0; i < ETR_MAX_NUM; i++) { >> + byte_cntr_data = &drvdata->byte_cntr_data[i]; >> + raw_spin_lock_irqsave(&byte_cntr_data->spin_lock, flags); >> + /* Set enable=false for all ports to signal teardown to racing readers */ >> + byte_cntr_data->enable = false; >> + if (!byte_cntr_data->reading) { >> + raw_spin_unlock_irqrestore(&byte_cntr_data->spin_lock, flags); >> + continue; >> + } >> + raw_spin_unlock_irqrestore(&byte_cntr_data->spin_lock, flags); >> + wake_up_all(&byte_cntr_data->wq); >> + wait_event(byte_cntr_data->wq, !byte_cntr_data->reading); >> + } > > [Severity: High] > Could this wait indefinitely during driver remove? > > The reading flag is only cleared when the userspace file is closed. A > userspace process leaving the file descriptor open could permanently block > module unload or device unbind. This behavior is expected and not a driver defect. During removal, ctcu_platform_remove() sets enable=false and wakes byte-cntr readers, releasing blocked reads from the wait queue. Any remaining wait corresponds to userspace retaining open TMC file descriptors. The kernel cannot forcibly close user file descriptors without violating lifetime rules; thus, waiting ensures that CTCU state is not freed while still potentially referenced. > >> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c >> index 2b26ce6455a7b..e78f8891f11e0 100644 >> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c >> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c > [ ... ] >> +ssize_t tmc_etr_get_sysfs_trace(struct tmc_drvdata *drvdata, >> + loff_t pos, size_t len, char **bufpp) >> +{ >> + ssize_t ret; >> + const struct tmc_sysfs_ops *byte_cntr_ops = READ_ONCE(byte_cntr_sysfs_ops); >> + >> + if (byte_cntr_ops) { >> + ret = byte_cntr_ops->get_trace_data(drvdata, pos, len, bufpp); >> + /* Return the filled buffer */ >> + if (ret > 0 || ret == -ENOMEM) >> + return ret; >> + } >> + >> + return tmc_etr_read_sysfs_buf(drvdata->sysfs_buf, pos, len, bufpp); >> +} > > [Severity: Medium] > Does this silently swallow -ERESTARTSYS? > > If a signal interrupts the wait inside tmc_byte_cntr_get_data(), it returns > -ERESTARTSYS, which falls through here to a normal sysfs buffer read. This > might convert the signal into an EOF or return unsynced data instead of > letting userspace handle the signal cleanly. > This is intentional. Byte-cntr operates as an optimized read path layered over the existing TMC sysfs read. If byte-cntr does not yield valid data, the implementation falls back to the generic TMC path. As such, propagating -ERESTARTSYS to userspace is unnecessary; it simply results in bypassing byte-cntr for that read attempt. > [ ... ] >> +int tmc_read_prepare_etr(struct tmc_drvdata *drvdata) >> +{ >> + int ret = 0; >> + unsigned long flags; >> + const struct tmc_sysfs_ops *byte_cntr_ops; >> >> /* config types are set a boot time and never change */ >> if (WARN_ON_ONCE(drvdata->config_type != TMC_CONFIG_TYPE_ETR)) >> return -EINVAL; >> >> + byte_cntr_ops = READ_ONCE(byte_cntr_sysfs_ops); >> + if (byte_cntr_ops) { >> + ret = byte_cntr_ops->read_prepare(drvdata); > > [Severity: Critical] > Is it safe to access this global function pointer locklessly? > > If the trace file is opened and the CTCU module is concurrently unloaded, the > memory backing byte_cntr_ops could be freed before read_prepare() executes, > leading to a use-after-free regression. > The reported use-after-free is not substantiated. byte_cntr_sysfs_ops is registered during CTCU probe and reset during CTCU remove. Existing byte-cntr readers are first signaled with enable=false, then woken and fully drained before the CTCU state is freed. After unregister, new callers observe NULL ops and fall back to the standard TMC read path. As such, this concern conflates global ops registration with an actual post-remove data/session UAF, which does not occur. >> + if (!ret || ret == -EBUSY) >> + return ret; >> + >> + ret = 0; >> + } > > [Severity: Medium] > Why are errors like -ENOMEM from read_prepare() masked here? > > If the memory allocation fails inside tmc_read_prepare_byte_cntr(), hiding the > error breaks the fallback sysfs reading path, leading to delays and hanging > operations instead of propagating the failure properly. This is an intential fallback design. Byte-cntr is an optional enhancement to the generic TMC ETR sysfs read path. When prepare fails for reasons other than an active session (-EBUSY), the driver falls back to the generic read-prepare path. Masking non--EBUSY errors is therefore intentional and not indicative of a defect. Thanks, Jie > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v19 7/7] arm64: dts: qcom: lemans: add interrupts to CTCU device 2026-06-25 10:45 [PATCH v19 0/7] coresight: ctcu: Enable byte-cntr function for TMC ETR Jie Gan ` (5 preceding siblings ...) 2026-06-25 10:45 ` [PATCH v19 6/7] coresight: ctcu: enable byte-cntr for TMC ETR devices Jie Gan @ 2026-06-25 10:45 ` Jie Gan 6 siblings, 0 replies; 13+ messages in thread From: Jie Gan @ 2026-06-25 10:45 UTC (permalink / raw) To: Suzuki K Poulose, Mike Leach, James Clark, Leo Yan, Alexander Shishkin, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Tingwei Zhang, Jie Gan, Bjorn Andersson, Konrad Dybcio, Yuanfang Zhang, Mao Jinlong Cc: coresight, linux-arm-kernel, linux-kernel, linux-arm-msm, devicetree, Konrad Dybcio Add interrupts to enable byte-cntr function for TMC ETR devices. Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com> --- arch/arm64/boot/dts/qcom/lemans.dtsi | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/lemans.dtsi b/arch/arm64/boot/dts/qcom/lemans.dtsi index 353a6e6fd3ac..2b4debc39db0 100644 --- a/arch/arm64/boot/dts/qcom/lemans.dtsi +++ b/arch/arm64/boot/dts/qcom/lemans.dtsi @@ -3150,6 +3150,9 @@ ctcu@4001000 { clocks = <&aoss_qmp>; clock-names = "apb"; + interrupts = <GIC_SPI 270 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 262 IRQ_TYPE_EDGE_RISING>; + in-ports { #address-cells = <1>; #size-cells = <0>; -- 2.34.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2026-06-25 13:52 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-06-25 10:45 [PATCH v19 0/7] coresight: ctcu: Enable byte-cntr function for TMC ETR Jie Gan 2026-06-25 10:45 ` [PATCH v19 1/7] coresight: core: refactor ctcu_get_active_port and make it generic Jie Gan 2026-06-25 11:07 ` sashiko-bot 2026-06-25 10:45 ` [PATCH v19 2/7] coresight: tmc: add create/clean functions for etr_buf_list Jie Gan 2026-06-25 11:06 ` sashiko-bot 2026-06-25 10:45 ` [PATCH v19 3/7] coresight: tmc: introduce tmc_sysfs_ops to wrap sysfs read operations Jie Gan 2026-06-25 11:00 ` sashiko-bot 2026-06-25 10:45 ` [PATCH v19 4/7] coresight: etr: add a new function to retrieve the CTCU device Jie Gan 2026-06-25 10:45 ` [PATCH v19 5/7] dt-bindings: arm: add an interrupt property for Coresight CTCU Jie Gan 2026-06-25 10:45 ` [PATCH v19 6/7] coresight: ctcu: enable byte-cntr for TMC ETR devices Jie Gan 2026-06-25 11:09 ` sashiko-bot 2026-06-25 13:51 ` Jie Gan 2026-06-25 10:45 ` [PATCH v19 7/7] arm64: dts: qcom: lemans: add interrupts to CTCU device Jie Gan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox