CoreSight

coresight@lists.linaro.org

3 participants
2706 discussions

[issue] Data loss during fork in workload-only mode

by 梁镇

Hi CoreSight maintainers, I am currently working on adding CoreSight support for the RISC-V platform and have identified a data loss issue when using workload-only mode in conjunction with fork system calls. Issue Description When recording traces in workload-only mode, fork events lead to partial trace data loss. Specifically, trace data prior to the fork call—including the entry of the main() function—is missing. In contrast, per-thread mode correctly captures the complete trace. Test Case The following test program was used to reproduce the issue: c /* gcc -D_GNU_SOURCE test_fork.c -o test_fork */ #include <stdio.h> #include <stdlib.h> #include <unistd.h> #include <sys/types.h> #include <sys/wait.h> #include <sched.h> void child_task() {     printf("Child process: Running on CPU=%d PID=%d\n", sched_getcpu(), getpid());     sleep(1);     printf("Child process: Task finished\n"); } int main() {     pid_t pid;     printf("Parent process: Running on CPU=%d PID=%d\n", sched_getcpu(), getpid());         pid = fork();     if (pid == -1) {         perror("fork failed");         exit(1);     } else if (pid == 0) {         child_task();         exit(0);     } else {         printf("Parent process: Waiting for child to finish...\n");         wait(NULL);         printf("Parent process: Child finished\n");     }     return 0; } Code Change for Debugging To assist in diagnosing the issue, the following debug output was added to coresight-etm-perf.c: diff --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -613,6 +611,10 @@ static void etm_event_pause(struct perf_event *event,         size = sink_ops(sink)->update_buffer(sink, handle, ctxt->event_data->snk_config);   +       dev_dbg(&csdev->dev, "etm_event_pause called on CPU%d, size=%lu\n", +               smp_processor_id(), size); +         if (READ_ONCE(handle->event)) {                 if (!size)                         return; Log Output The following logs were captured during execution: root@k3:~# ~/perf record -e rvtrace/@tmc_etr0/ ./test_fork Parent process: Running on CPU=1 PID=1534 [ 2190.481643] coresight encoder1: DEBUG: CPU1 update_buffer returned size=0 Parent process: Waiting for child to finish... [ 2190.489223] coresight encoder6: DEBUG: CPU6 update_buffer returned size=0 Child process: Running on CPU=6 PID=1535 [ 2191.489372] coresight encoder6: DEBUG: CPU6 update_buffer returned size=30880 Child process: Task finished Parent process: Child finished [ 2191.496382] coresight encoder1: DEBUG: CPU1 update_buffer returned size=21504 [ perf record: Woken up 2 times to write data ] [ perf record: Captured and wrote 0.056 MB perf.data ] root@k3:~# ~/perf script > ./test_fork.log root@k3:~# grep "/root/test" test_fork.log -nrI 115: test_fork 1535 [006] 52.991808: 1 branches: 2aaacc7910 child_task+0x2e (/root/test_fork) => 2aaacc7794 puts@plt+0x4 (/root/test_fork) 116: test_fork 1535 [006] 52.991808: 1 branches: 2aaacc7798 puts@plt+0x8 (/root/test_fork) => 2aaacc7798 puts@plt+0x8 (/root/test_fork) 962: test_fork 1535 [006] 52.991809: 1 branches: 2aaacc791c child_task+0x3a (/root/test_fork) => 2aaacc7928 child_task+0x46 (/root/test_fork) 963: test_fork 1535 [006] 52.991809: 1 branches: 2aaacc7988 main+0x5e (/root/test_fork) => 2aaacc77c8 exit@plt+0x8 (/root/test_fork) 2468: test_fork 1535 [006] 52.991809: 1 branches: 2aaacc7894 __do_global_dtors_aux+0x0 (/root/test_fork) => 2aaacc7894 __do_global_dtors_aux+0x0 (/root/test_fork) 2563: test_fork 1535 [006] 52.991809: 1 branches: 2aaacc7898 __do_global_dtors_aux+0x4 (/root/test_fork) => 2aaacc7898 __do_global_dtors_aux+0x4 (/root/test_fork) 2564: test_fork 1535 [006] 52.991809: 1 branches: 2aaacc789c __do_global_dtors_aux+0x8 (/root/test_fork) => 2aaacc78b8 __do_global_dtors_aux+0x24 (/root/test_fork) 2570: test_fork 1535 [006] 52.991809: 1 branches: 2aaacc78ba __do_global_dtors_aux+0x26 (/root/test_fork) => 2aaacc7836 deregister_tm_clones+0x18 (/root/test_fork) 2571: test_fork 1535 [006] 52.991809: 1 branches: 2aaacc784c deregister_tm_clones+0x2e (/root/test_fork) => 2aaacc7852 deregister_tm_clones+0x34 (/root/test_fork) 2572: test_fork 1535 [006] 52.991809: 1 branches: 2aaacc78be __do_global_dtors_aux+0x2a (/root/test_fork) => 2aaacc78c4 __do_global_dtors_aux+0x30 (/root/test_fork) 2864: test_fork 1535 [006] 52.991809: 1 branches: 2aaacc78c8 __do_global_dtors_aux+0x34 (/root/test_fork) => 2aaacc78ce __do_global_dtors_aux+0x3a (/root/test_fork) 4163: test_fork 1534 [001] 52.991977: 1 branches: 2aaacc79a0 main+0x76 (/root/test_fork) => 2aaacc7798 puts@plt+0x8 (/root/test_fork) 4849: test_fork 1534 [001] 52.991977: 1 branches: 2aaacc79ac main+0x82 (/root/test_fork) => 2aaacc79b8 main+0x8e (/root/test_fork) 5244: test_fork 1534 [001] 52.991978: 1 branches: 2aaacc7894 __do_global_dtors_aux+0x0 (/root/test_fork) => 2aaacc78b8 __do_global_dtors_aux+0x24 (/root/test_fork) 5368: test_fork 1534 [001] 52.991978: 1 branches: 2aaacc78ba __do_global_dtors_aux+0x26 (/root/test_fork) => 2aaacc7836 deregister_tm_clones+0x18 (/root/test_fork) 5369: test_fork 1534 [001] 52.991978: 1 branches: 2aaacc784c deregister_tm_clones+0x2e (/root/test_fork) => 2aaacc7852 deregister_tm_clones+0x34 (/root/test_fork) 5370: test_fork 1534 [001] 52.991978: 1 branches: 2aaacc78be __do_global_dtors_aux+0x2a (/root/test_fork) => 2aaacc78c4 __do_global_dtors_aux+0x30 (/root/test_fork) 5478: test_fork 1534 [001] 52.991978: 1 branches: 2aaacc78c8 __do_global_dtors_aux+0x34 (/root/test_fork) => 2aaacc78ce __do_global_dtors_aux+0x3a (/root/test_fork) As shown in the logs, trace data from the parent process before the fork (including the entry to main()) is missing in workload-only mode. Questions I am currently uncertain whether this issue also affects the coresight-etm tracer, or if it is a hardware-specific problem on our platform. I would greatly appreciate any suggestions or guidance you may have. Thank you for your support! 进迭时空梁镇

1 month, 3 weeks

Re: [PATCH v14 6/7] coresight: ctcu: enable byte-cntr for TMC ETR devices

by Suzuki K Poulose

On 10/03/2026 03:01, Jie Gan wrote: > > > On 3/9/2026 8:43 PM, Suzuki K Poulose wrote: >> On 09/03/2026 09:47, Jie Gan wrote: >>> The byte-cntr function provided by the CTCU device is used to >>> transfer data >>> from the ETR buffer to the userspace. An interrupt is 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. >>> >>> Signed-off-by: Jie Gan <jie.gan(a)oss.qualcomm.com> >>> --- >>> .../ABI/testing/sysfs-bus-coresight-devices-ctcu | 8 + >>> drivers/hwtracing/coresight/Makefile | 2 +- >>> .../hwtracing/coresight/coresight-ctcu-byte-cntr.c | 351 ++++++++++ >>> + ++++++++++ >>> drivers/hwtracing/coresight/coresight-ctcu-core.c | 103 +++++- >>> drivers/hwtracing/coresight/coresight-ctcu.h | 76 ++++- >>> drivers/hwtracing/coresight/coresight-tmc-core.c | 8 +- >>> drivers/hwtracing/coresight/coresight-tmc-etr.c | 18 ++ >>> drivers/hwtracing/coresight/coresight-tmc.h | 4 + >>> 8 files changed, 555 insertions(+), 15 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..6ff1708fb944 >>> --- /dev/null >>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-ctcu >>> @@ -0,0 +1,8 @@ >>> +What: /sys/bus/coresight/devices/<ctcu-name>/ >>> irq_threshold[0:1] >>> +Date: March 2026 >>> +KernelVersion: 7.1 >>> +Contact: Tingwei Zhang <tingwei.zhang(a)oss.qualcomm.com>; >>> Jinlong Mao <jinlong.mao(a)oss.qualcomm.com>; Jie Gan >>> <jie.gan(a)oss.qualcomm.com> >>> +Description: >>> + (RW) Configure the byte-cntr IRQ register for the specified >>> ETR device >>> + based on its port number. An interrupt is generated when the >>> data size >>> + exceeds the value set in the IRQ register. >>> 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..0bf738d6c283 >>> --- /dev/null >>> +++ b/drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c >>> @@ -0,0 +1,351 @@ >>> +// 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 = (struct ctcu_byte_cntr >>> *)data; >>> + >>> + atomic_inc(&byte_cntr_data->irq_cnt); >>> + wake_up(&byte_cntr_data->wq); >>> + >>> + return IRQ_HANDLED; >>> +} >>> + >>> +static void ctcu_reset_sysfs_buf(struct tmc_drvdata *drvdata) >> >> minor nit: This has nothing to do with the CTCU. For what it is worth, >> it must be called, tmc_etr_reset_sysf_buf(). But more on this below, >> and even do we need it, further below. >> >>> +{ >>> + 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); >> >> Could we not keep this function in the tmc-etr.c and invoke from here ? >> > > Sure, will move the function tmc-etr.c > >>> +} >>> + >>> +static void ctcu_cfg_byte_cntr_reg(struct tmc_drvdata *drvdata, u32 >>> val, u32 offset) >>> +{ >>> + struct ctcu_drvdata *ctcu_drvdata; >>> + struct coresight_device *helper; >>> + >>> + helper = tmc_etr_get_ctcu_device(drvdata); >>> + if (!helper) >>> + return; >>> + >>> + ctcu_drvdata = dev_get_drvdata(helper->dev.parent); >>> + /* A one value for IRQCTRL register represents 8 bytes */ >>> + ctcu_program_register(ctcu_drvdata, val / 8, offset); >>> +} >>> + >>> +static struct ctcu_byte_cntr *ctcu_get_byte_cntr_data(struct >>> tmc_drvdata *drvdata) >>> +{ >>> + struct ctcu_byte_cntr *byte_cntr_data; >>> + struct ctcu_drvdata *ctcu_drvdata; >>> + struct coresight_device *helper; >>> + int port; >>> + >>> + helper = tmc_etr_get_ctcu_device(drvdata); >>> + if (!helper) >>> + return NULL; >>> + >> >> >> >>> + port = coresight_get_in_port(drvdata->csdev, helper); >>> + if (port < 0) >>> + return NULL; >>> + >> >> Please validate that the port_num you get is valid for the CTCU ? That >> applies to all uses of this construct. >> > > Will validate it before using. > >>> + ctcu_drvdata = dev_get_drvdata(helper->dev.parent); >>> + byte_cntr_data = &ctcu_drvdata->byte_cntr_data[port]; >>> + return byte_cntr_data; >> >> >> >> nit: >> return &ctcu_drvdata->byte_cntr_data[port]; ? >> >> Also, why not make this into a helper, as we seem to use this other >> places too ? >> > > Didnt get the point here. We may run more than one ETR devices > concurrently. So we should get the proper byte_cntr_data according to > the port number at runtime. > static struct ctcu_byte_cntr *ctcu_byte_cntr(struct coresight_device *cctcu_dev, struct coresight_device *tmc_etr, ) { port = coresight_get_in_port().. // Verify the port in this helper and everyone uses this. if (//!validate_port//) return NULL return ... } Suzuki

1 month, 3 weeks

Re: [PATCH v14 6/7] coresight: ctcu: enable byte-cntr for TMC ETR devices

by Suzuki K Poulose

On 09/03/2026 09:47, Jie Gan wrote: > The byte-cntr function provided by the CTCU device is used to transfer data > from the ETR buffer to the userspace. An interrupt is 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. > > Signed-off-by: Jie Gan <jie.gan(a)oss.qualcomm.com> > --- > .../ABI/testing/sysfs-bus-coresight-devices-ctcu | 8 + > drivers/hwtracing/coresight/Makefile | 2 +- > .../hwtracing/coresight/coresight-ctcu-byte-cntr.c | 351 +++++++++++++++++++++ > drivers/hwtracing/coresight/coresight-ctcu-core.c | 103 +++++- > drivers/hwtracing/coresight/coresight-ctcu.h | 76 ++++- > drivers/hwtracing/coresight/coresight-tmc-core.c | 8 +- > drivers/hwtracing/coresight/coresight-tmc-etr.c | 18 ++ > drivers/hwtracing/coresight/coresight-tmc.h | 4 + > 8 files changed, 555 insertions(+), 15 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..6ff1708fb944 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-ctcu > @@ -0,0 +1,8 @@ > +What: /sys/bus/coresight/devices/<ctcu-name>/irq_threshold[0:1] > +Date: March 2026 > +KernelVersion: 7.1 > +Contact: Tingwei Zhang <tingwei.zhang(a)oss.qualcomm.com>; Jinlong Mao <jinlong.mao(a)oss.qualcomm.com>; Jie Gan <jie.gan(a)oss.qualcomm.com> > +Description: > + (RW) Configure the byte-cntr IRQ register for the specified ETR device > + based on its port number. An interrupt is generated when the data size > + exceeds the value set in the IRQ register. > 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..0bf738d6c283 > --- /dev/null > +++ b/drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c > @@ -0,0 +1,351 @@ > +// 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 = (struct ctcu_byte_cntr *)data; > + > + atomic_inc(&byte_cntr_data->irq_cnt); > + wake_up(&byte_cntr_data->wq); > + > + return IRQ_HANDLED; > +} > + > +static void ctcu_reset_sysfs_buf(struct tmc_drvdata *drvdata) minor nit: This has nothing to do with the CTCU. For what it is worth, it must be called, tmc_etr_reset_sysf_buf(). But more on this below, and even do we need it, further below. > +{ > + 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); Could we not keep this function in the tmc-etr.c and invoke from here ? > +} > + > +static void ctcu_cfg_byte_cntr_reg(struct tmc_drvdata *drvdata, u32 val, u32 offset) > +{ > + struct ctcu_drvdata *ctcu_drvdata; > + struct coresight_device *helper; > + > + helper = tmc_etr_get_ctcu_device(drvdata); > + if (!helper) > + return; > + > + ctcu_drvdata = dev_get_drvdata(helper->dev.parent); > + /* A one value for IRQCTRL register represents 8 bytes */ > + ctcu_program_register(ctcu_drvdata, val / 8, offset); > +} > + > +static struct ctcu_byte_cntr *ctcu_get_byte_cntr_data(struct tmc_drvdata *drvdata) > +{ > + struct ctcu_byte_cntr *byte_cntr_data; > + struct ctcu_drvdata *ctcu_drvdata; > + struct coresight_device *helper; > + int port; > + > + helper = tmc_etr_get_ctcu_device(drvdata); > + if (!helper) > + return NULL; > + > + port = coresight_get_in_port(drvdata->csdev, helper); > + if (port < 0) > + return NULL; > + Please validate that the port_num you get is valid for the CTCU ? That applies to all uses of this construct. > + ctcu_drvdata = dev_get_drvdata(helper->dev.parent); > + byte_cntr_data = &ctcu_drvdata->byte_cntr_data[port]; > + return byte_cntr_data; nit: return &ctcu_drvdata->byte_cntr_data[port]; ? Also, why not make this into a helper, as we seem to use this other places too ? > +} > + > +static bool ctcu_byte_cntr_switch_buffer(struct tmc_drvdata *drvdata, > + struct ctcu_byte_cntr *byte_cntr_data) > +{ > + struct etr_buf_node *nd, *next, *curr_node, *picked_node; > + struct etr_buf *curr_buf = drvdata->sysfs_buf; > + bool found_free_buf = false; > + > + if (WARN_ON(!drvdata || !byte_cntr_data)) > + return found_free_buf; > + > + /* Stop the ETR before initiating the switch */ > + if (coresight_get_mode(drvdata->csdev) != CS_MODE_DISABLED) > + tmc_etr_enable_disable_hw(drvdata, false); > + > + list_for_each_entry_safe(nd, next, &drvdata->etr_buf_list, node) { > + /* curr_buf is free for next round */ > + if (nd->sysfs_buf == curr_buf) { > + nd->is_free = true; > + curr_node = nd; ---8>-- > + } > + > + if (!found_free_buf && nd->is_free && nd->sysfs_buf != curr_buf) { --<8--- } else if (!found_free_buf && nd->is_free) { > + picked_node = nd; > + found_free_buf = true; > + } > + } > + > + if (found_free_buf) { > + curr_node->pos = 0; > + drvdata->buf_node = curr_node; Why are we adding something new into the drvdata ? Could we not extend the sysfs_buf struct and add the "link" there ? That would make it much more simpler ? > + drvdata->sysfs_buf = picked_node->sysfs_buf; > + 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); > + /* Reset rrp and rwp when the system has switched the buffer*/ > + ctcu_reset_sysfs_buf(drvdata); Why do we do this ? Could we not leave this to the __tmc_etr_enable() below ? > + /* Restart the ETR once a free buffer is available */ > + if (coresight_get_mode(drvdata->csdev) != CS_MODE_DISABLED) > + tmc_etr_enable_disable_hw(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 ctcu_byte_cntr_get_data(struct tmc_drvdata *drvdata, loff_t pos, > + size_t len, char **bufpp) > +{ > + struct etr_buf *sysfs_buf = drvdata->sysfs_buf; > + struct device *dev = &drvdata->csdev->dev; > + ssize_t actual, size = sysfs_buf->size; > + struct ctcu_byte_cntr *byte_cntr_data; > + size_t thresh_val; > + atomic_t *irq_cnt; > + int ret; > + > + byte_cntr_data = ctcu_get_byte_cntr_data(drvdata); > + if (!byte_cntr_data) > + return -EINVAL; > + > + thresh_val = byte_cntr_data->thresh_val; > + irq_cnt = &byte_cntr_data->irq_cnt; > + > +wait_buffer: > + if (!byte_cntr_data->reading_data) { > + ret = wait_event_interruptible_timeout(byte_cntr_data->wq, > + ((atomic_read(irq_cnt) + 1) * thresh_val >= size) || > + !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(drvdata, byte_cntr_data)) { > + dev_err(dev, "Switch buffer failed for the byte-cntr\n"); > + return -EINVAL; > + } > + > + byte_cntr_data->reading_data = true; > + } else { > + /* > + * TMC-ETR has been disabled, so directly reads data from > + * the drvdata->sysfs_buf. > + */ > + actual = etr_sysfs_ops.get_trace_data(drvdata, pos, len, bufpp); > + if (actual > 0) { > + byte_cntr_data->total_size += actual; > + return actual; > + } > + > + /* Exit byte-cntr reading */ > + return 0; > + } > + } > + > + /* Check the status of current etr_buf*/ > + if (atomic_read(irq_cnt) * thresh_val >= size) > + dev_warn(dev, "Data overwrite happened\n"); > + > + pos = drvdata->buf_node->pos; > + actual = etr_sysfs_ops.get_trace_data(drvdata, pos, len, bufpp); > + if (actual <= 0) { > + /* Reset flags upon reading is finished or failed */ > + byte_cntr_data->reading_data = false; > + drvdata->buf_node = NULL; > + > + /* > + * Nothing in the buffer, waiting for the next buffer > + * to be filled. > + */ > + if (actual == 0) > + goto wait_buffer; > + } else > + byte_cntr_data->total_size += actual; > + > + return actual; > +} > + > +static int ctcu_read_prepare_byte_cntr(struct tmc_drvdata *drvdata) Please use "tmc_data" or even "etr_drvdata" to make it easier to read the code below > +{ > + struct ctcu_byte_cntr *byte_cntr_data; > + unsigned long flags; > + int ret = 0; > + > + byte_cntr_data = ctcu_get_byte_cntr_data(drvdata); > + if (!byte_cntr_data) > + return -EINVAL; > + > + /* > + * The threshold value must not exceed the buffer size. > + * A margin should be maintained between the two values to account > + * for the time gap between the interrupt and buffer switching. > + */ > + if (byte_cntr_data->thresh_val + SZ_16K >= drvdata->size) { Whats the magic number SZ_16K ? Comment could explain it. > + dev_err(&drvdata->csdev->dev, "The threshold value is too large\n"); Could the threshold be a percentage of the size used by the ETR (You could always align it to the nearest PAGE_SIZE)? That would make it convenient for people to change the ETR size and not having to bother about moving the threshold value with the ETR size. > + return -EINVAL; > + } > + > + raw_spin_lock_irqsave(&drvdata->spinlock, flags); > + if (byte_cntr_data->reading) { > + ret = -EBUSY; > + goto out_unlock; > + } > + > + byte_cntr_data->reading = true; > + raw_spin_unlock_irqrestore(&drvdata->spinlock, flags); All of this can be avoided by retaining the etr_sysf_ops in TMC-> and calling into the CTCU after basic checks. > + /* Setup an available etr_buf_list for byte-cntr */ > + ret = tmc_create_etr_buf_list(drvdata, 2); > + if (ret) > + goto out; > + > + raw_spin_lock_irqsave(&drvdata->spinlock, flags); > + atomic_set(&byte_cntr_data->irq_cnt, 0); > + /* Configure the byte-cntr register to enable IRQ */ > + ctcu_cfg_byte_cntr_reg(drvdata, byte_cntr_data->thresh_val, > + byte_cntr_data->irq_ctrl_offset); Is there no way of going back to the ctcu_drvdata from byte_cntr_data ? For a starter, we could easily store the ctcu_drvdata in byte_cntr_data and always write straight to the device, rather than always going to the ETR drvdata in ctcu_cfg_byte_cntr_reg() ? > + enable_irq_wake(byte_cntr_data->irq); > + byte_cntr_data->total_size = 0; > + > +out_unlock: > + raw_spin_unlock_irqrestore(&drvdata->spinlock, flags); > + > +out: > + return ret; > +} > + > +static int ctcu_read_unprepare_byte_cntr(struct tmc_drvdata *drvdata) Same as above. name > +{ > + struct device *dev = &drvdata->csdev->dev; > + struct ctcu_byte_cntr *byte_cntr_data; > + unsigned long flags; > + > + byte_cntr_data = ctcu_get_byte_cntr_data(drvdata); > + if (!byte_cntr_data) > + return -EINVAL; > + > + raw_spin_lock_irqsave(&drvdata->spinlock, flags); > + /* Configure the byte-cntr register to disable IRQ */ > + ctcu_cfg_byte_cntr_reg(drvdata, 0, byte_cntr_data->irq_ctrl_offset); > + disable_irq_wake(byte_cntr_data->irq); > + byte_cntr_data->reading = false; > + byte_cntr_data->reading_data = false; > + drvdata->buf_node = NULL; > + /* Restore the original sysfs_ops */ > + drvdata->sysfs_ops = &etr_sysfs_ops; Please see if you can avoid it. > + raw_spin_unlock_irqrestore(&drvdata->spinlock, flags); > + dev_dbg(dev, "send data total size:%llu bytes\n", byte_cntr_data->total_size); > + tmc_clean_etr_buf_list(drvdata); > + > + return 0; > +} > + > +const struct tmc_sysfs_ops byte_cntr_sysfs_ops = { > + .read_prepare = ctcu_read_prepare_byte_cntr, > + .read_unprepare = ctcu_read_unprepare_byte_cntr, > + .get_trace_data = ctcu_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 ctcu_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > + struct coresight_device *sink = coresight_get_sink(path); > + struct tmc_drvdata *tmc_drvdata = dev_get_drvdata(sink->dev.parent); > + struct ctcu_byte_cntr *byte_cntr_data; > + int port_num; > + > + port_num = coresight_get_in_port(sink, csdev); > + if (port_num < 0) > + return; > + > + byte_cntr_data = &drvdata->byte_cntr_data[port_num]; instance 2 > + /* Don't start byte-cntr function when threshold is not set. */ > + if (!byte_cntr_data->thresh_val || byte_cntr_data->enable) > + return; > + > + guard(raw_spinlock_irqsave)(&byte_cntr_data->spin_lock); > + byte_cntr_data->enable = true; > + byte_cntr_data->reading_data = false; > + /* Replace with byte-cntr's sysfs_ops */ > + tmc_drvdata->sysfs_ops = &byte_cntr_sysfs_ops; Why are we hacking this ? This looks *very ugly* and we are doing this without any locks at the tmc-etr driver !!! ?? Could we not call out from the etr_sysf_ops into the CTCU ops based some checks ? Anyways, please don't change anything in the tmc_drvdata from this driver, especially not the "operations". Also the locking seems inconsistent. Here we use byte_cntr_data->lock. But in ctcu_read_unprepare_byte_cntr(), we use the TMC-ETR spinlock :facepalm: > +} > + > +/* Stop the byte-cntr function when the path is disabled. */ > +void ctcu_byte_cntr_stop(struct coresight_device *csdev, struct coresight_path *path) > +{ > + struct ctcu_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > + struct coresight_device *sink = coresight_get_sink(path); > + struct ctcu_byte_cntr *byte_cntr_data; > + int port_num; > + > + if (coresight_get_mode(sink) == CS_MODE_SYSFS) > + return; > + > + port_num = coresight_get_in_port(sink, csdev); > + if (port_num < 0) > + return; > + > + byte_cntr_data = &drvdata->byte_cntr_data[port_num]; instance 3 > + 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; > + > + 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; > + init_waitqueue_head(&byte_cntr_data->wq); > + raw_spin_lock_init(&byte_cntr_data->spin_lock); > + } > +} > diff --git a/drivers/hwtracing/coresight/coresight-ctcu-core.c b/drivers/hwtracing/coresight/coresight-ctcu-core.c > index e8720026c9e3..60f1db3ab70d 100644 > --- a/drivers/hwtracing/coresight/coresight-ctcu-core.c > +++ b/drivers/hwtracing/coresight/coresight-ctcu-core.c > @@ -15,6 +15,7 @@ > #include <linux/platform_device.h> > #include <linux/pm_runtime.h> > #include <linux/slab.h> > +#include <linux/sizes.h> > > #include "coresight-ctcu.h" > #include "coresight-priv.h" > @@ -43,17 +44,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 +67,88 @@ 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_threshold_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].thresh_val); > +} > + > +static ssize_t irq_threshold_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; > + > + /* Threshold 0 disables the interruption. */ > + guard(raw_spinlock_irqsave)(&drvdata->spin_lock); > + /* A small threshold will result in a large number of interruptions */ > + if (val && val < SZ_4K) > + return -EINVAL; > + > + if (drvdata->byte_cntr_data[port].irq_ctrl_offset) > + drvdata->byte_cntr_data[port].thresh_val = val; > + > + return size; > +} > + > +static umode_t irq_threshold_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_threshold_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 +227,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 +286,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 +303,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); > diff --git a/drivers/hwtracing/coresight/coresight-ctcu.h b/drivers/hwtracing/coresight/coresight-ctcu.h > index e9594c38dd91..c52cf6a33d2e 100644 > --- a/drivers/hwtracing/coresight/coresight-ctcu.h > +++ b/drivers/hwtracing/coresight/coresight-ctcu.h > @@ -5,19 +5,26 @@ > > #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) > + > /** > * 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. > + * @irq_name: IRQ name in dt node. > */ > struct ctcu_etr_config { > const u32 atid_offset; > const u32 port_num; > + const u32 irq_ctrl_offset; > }; > > struct ctcu_config { > @@ -25,15 +32,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. > + * @reading: indicates that byte-cntr reading is started. > + * @reading_data: indicates that byte-cntr is reading data from the buffer. > + * @thresh_val: threshold to trigger a interruption. > + * @total_size: total size of the transferred data. > + * @irq: allocated number of the IRQ. > + * @irq_cnt: IRQ count number for triggered interruptions. > + * @wq: waitqueue for reading data from ETR buffer. > + * @spin_lock: spinlock of byte_cntr_data. > + * @irq_ctrl_offset: offset to the BYTECNTVAL Register. > + */ > +struct ctcu_byte_cntr { > + bool enable; > + bool reading; > + bool reading_data; > + u32 thresh_val; > + u64 total_size; > + int irq; > + atomic_t irq_cnt; > + wait_queue_head_t wq; > raw_spinlock_t spin_lock; > - u32 atid_offset[ETR_MAX_NUM]; > + u32 irq_ctrl_offset; > +}; > + > +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]; > + u8 traceid_refcnt[ETR_MAX_NUM][CORESIGHT_TRACE_ID_RES_TOP]; > }; > > +/** > + * struct ctcu_irq_thresh_attribute > + * @attr: The device attribute. > + * @idx: 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_threshold##port, 0644, irq_threshold_show, \ > + irq_threshold_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 110eedde077f..948ea864f2a1 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc-core.c > +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c > @@ -293,7 +293,10 @@ static ssize_t tmc_read(struct file *file, char __user *data, size_t len, > return -EFAULT; > } > > - *ppos += actual; > + if (drvdata->buf_node) > + drvdata->buf_node->pos += actual; > + else > + *ppos += actual; > dev_dbg(&drvdata->csdev->dev, "%zu bytes copied\n", actual); > > return actual; > @@ -748,11 +751,12 @@ static const struct tmc_sysfs_ops etb_sysfs_ops = { > .get_trace_data = tmc_etb_get_sysfs_trace, > }; > > -static const struct tmc_sysfs_ops etr_sysfs_ops = { > +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, > }; > +EXPORT_SYMBOL_GPL(etr_sysfs_ops); > > static int __tmc_probe(struct device *dev, struct resource *res) > { > diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c > index f4223215ef8d..8896fb7a9ed3 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c > +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c > @@ -1185,6 +1185,10 @@ ssize_t tmc_etr_get_sysfs_trace(struct tmc_drvdata *drvdata, > ssize_t actual = len; > struct etr_buf *etr_buf = drvdata->sysfs_buf; > > + /* Reading the buffer from the buf_node if it exists*/ > + if (drvdata->buf_node) > + etr_buf = drvdata->buf_node->sysfs_buf; > + > if (pos + actual > etr_buf->len) > actual = etr_buf->len - pos; > if (actual <= 0) > @@ -1248,6 +1252,20 @@ static void __tmc_etr_disable_hw(struct tmc_drvdata *drvdata) > > } > > +/** > + * 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) > +{ > + if (enable) > + __tmc_etr_enable_hw(drvdata); > + else > + __tmc_etr_disable_hw(drvdata); > +} > +EXPORT_SYMBOL_GPL(tmc_etr_enable_disable_hw); > + > void tmc_etr_disable_hw(struct tmc_drvdata *drvdata) > { > __tmc_etr_disable_hw(drvdata); > diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h > index aaa443abe8e9..183c649b982c 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc.h > +++ b/drivers/hwtracing/coresight/coresight-tmc.h > @@ -260,6 +260,7 @@ struct etr_buf_node { > * Used by ETR/ETF. > * @etr_buf_list: List that is used to manage allocated etr_buf. > * @sysfs_ops: Read operations for the sysfs mode. > + * @buf_node: Available buffer_node for byte-cntr reading. > */ > struct tmc_drvdata { > struct clk *atclk; > @@ -292,6 +293,7 @@ struct tmc_drvdata { > struct tmc_resrv_buf crash_mdata; > struct list_head etr_buf_list; > const struct tmc_sysfs_ops *sysfs_ops; > + struct etr_buf_node *buf_node; > }; > > /** > @@ -371,6 +373,7 @@ void tmc_etr_disable_hw(struct tmc_drvdata *drvdata); > extern const struct coresight_ops tmc_etr_cs_ops; > ssize_t tmc_etr_get_sysfs_trace(struct tmc_drvdata *drvdata, > loff_t pos, size_t len, char **bufpp); > +extern const struct tmc_sysfs_ops etr_sysfs_ops; > > > #define TMC_REG_PAIR(name, lo_off, hi_off) \ > @@ -480,5 +483,6 @@ 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_enable_disable_hw(struct tmc_drvdata *drvdata, bool enable); > > #endif >

1 month, 3 weeks

Re: [PATCH v14 2/7] coresight: tmc: add create/clean functions for etr_buf_list

by Suzuki K Poulose

On 09/03/2026 09:47, Jie Gan wrote: > 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(a)linaro.org> > Signed-off-by: Jie Gan <jie.gan(a)oss.qualcomm.com> > --- > drivers/hwtracing/coresight/coresight-tmc-core.c | 1 + > drivers/hwtracing/coresight/coresight-tmc-etr.c | 94 ++++++++++++++++++++++++ > drivers/hwtracing/coresight/coresight-tmc.h | 17 +++++ > 3 files changed, 112 insertions(+) > > diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c > index c89fe996af23..bac3278ef4dd 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 4dc1defe27a5..15c0874ff641 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c > +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c > @@ -1918,6 +1918,100 @@ 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 the allocated node from the list and free the extra buffer. > + */ > +void tmc_clean_etr_buf_list(struct tmc_drvdata *drvdata) > +{ ... > +} > +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. > + * > + * Return 0 upon success and return the error number if fail. > + */ > +int tmc_create_etr_buf_list(struct tmc_drvdata *drvdata, int num_nodes) > +{ ... > +EXPORT_SYMBOL_GPL(tmc_create_etr_buf_list); Given the above functions are "EXPORTED" please could you make sure that the locking requirements are documented and asserted (lockdep_assert) in the functions ? Suzuki > + > 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..5ac07e8dd5ff 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: Position of the buffer. > + * @node: Node in etr_buf_list. > + */ > +struct etr_buf_node { > + struct etr_buf *sysfs_buf; > + bool is_free; > + loff_t pos; > + struct list_head node; > +}; > + > /** > * 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 >

1 month, 3 weeks

[PATCH v2 0/2] perf cs-etm: Finish removal of ETM_OPT_*

by James Clark

Finish removal of ETM_OPT_* defines so the coresight-pmu.h header can by synced from the kernel. Signed-off-by: James Clark <james.clark(a)linaro.org> --- Changes in v2: - Access metadata through existing etm pointer (Leo) - Link to v1: https://lore.kernel.org/r/20260306-james-perf-remove-etm_opt-v1-0-03c662380… --- James Clark (2): perf cs-etm: Finish removal of ETM_OPT_* tools: Sync coresight-pmu.h header tools/include/linux/coresight-pmu.h | 24 ----------------- tools/perf/arch/arm/util/cs-etm.c | 14 ---------- tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 2 +- tools/perf/util/cs-etm.c | 36 +++++++++---------------- tools/perf/util/cs-etm.h | 15 +++++++++++ 5 files changed, 29 insertions(+), 62 deletions(-) --- base-commit: b1718b0367ba31e8db273e3896ebd1707bcbe59e change-id: 20260306-james-perf-remove-etm_opt-c0e9a768dce8 Best regards, -- James Clark <james.clark(a)linaro.org>

1 month, 4 weeks

[PATCH 0/2] perf cs-etm: Finish removal of ETM_OPT_*

by James Clark

Finish removal of ETM_OPT_* defines so the coresight-pmu.h header can by synced from the kernel. Signed-off-by: James Clark <james.clark(a)linaro.org> --- James Clark (2): perf cs-etm: Finish removal of ETM_OPT_* tools: Sync coresight-pmu.h header tools/include/linux/coresight-pmu.h | 24 ----------------- tools/perf/arch/arm/util/cs-etm.c | 14 ---------- tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 2 +- tools/perf/util/cs-etm.c | 36 +++++++++---------------- tools/perf/util/cs-etm.h | 15 +++++++++++ 5 files changed, 28 insertions(+), 63 deletions(-) --- base-commit: b1718b0367ba31e8db273e3896ebd1707bcbe59e change-id: 20260306-james-perf-remove-etm_opt-c0e9a768dce8 Best regards, -- James Clark <james.clark(a)linaro.org>

1 month, 4 weeks

RE: [PATCH v13 7/8] coresight: tmc: integrate byte-cntr's sysfs_ops with tmc sysfs file_ops

by Mike Leach

Hi, > -----Original Message----- > From: Jie Gan <jie.gan(a)oss.qualcomm.com> > Sent: Monday, February 23, 2026 6:56 AM > To: Suzuki Poulose <Suzuki.Poulose(a)arm.com>; Mike Leach > <Mike.Leach(a)arm.com>; James Clark <james.clark(a)linaro.org>; Alexander > Shishkin <alexander.shishkin(a)linux.intel.com>; Rob Herring > <robh(a)kernel.org>; Krzysztof Kozlowski <krzk+dt(a)kernel.org>; Conor Dooley > <conor+dt(a)kernel.org>; Tingwei Zhang > <tingwei.zhang(a)oss.qualcomm.com>; Mao Jinlong > <jinlong.mao(a)oss.qualcomm.com>; Bjorn Andersson > <andersson(a)kernel.org>; Konrad Dybcio <konradybcio(a)kernel.org> > Cc: coresight(a)lists.linaro.org; linux-arm-kernel(a)lists.infradead.org; linux- > kernel(a)vger.kernel.org; linux-arm-msm(a)vger.kernel.org; > devicetree(a)vger.kernel.org; Jie Gan <jie.gan(a)oss.qualcomm.com> > Subject: [PATCH v13 7/8] coresight: tmc: integrate byte-cntr's sysfs_ops with > tmc sysfs file_ops > > Add code logic to invoke byte-cntr's tmc_sysfs_ops if the byte-cntr > is enabled. > > Signed-off-by: Jie Gan <jie.gan(a)oss.qualcomm.com> > --- > drivers/hwtracing/coresight/coresight-tmc-core.c | 53 > +++++++++++++++++++++++- > 1 file changed, 52 insertions(+), 1 deletion(-) > > diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c > b/drivers/hwtracing/coresight/coresight-tmc-core.c > index 32ca2ec994de..6486bdafdddc 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc-core.c > +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c > @@ -31,6 +31,7 @@ > > #include "coresight-priv.h" > #include "coresight-tmc.h" > +#include "coresight-ctcu.h" > > DEFINE_CORESIGHT_DEVLIST(etb_devs, "tmc_etb"); > DEFINE_CORESIGHT_DEVLIST(etf_devs, "tmc_etf"); > @@ -228,15 +229,47 @@ static int tmc_prepare_crashdata(struct > tmc_drvdata *drvdata) > return 0; > } > > +/* Return the byte-cntr's tmc_sysfs_ops if in using */ > +static const struct tmc_sysfs_ops *tmc_get_byte_cntr_sysfs_ops(struct > tmc_drvdata *drvdata) > +{ > + struct ctcu_byte_cntr *byte_cntr_data; > + struct ctcu_drvdata *ctcu_drvdata; > + struct coresight_device *ctcu; > + int port; > + > + ctcu = tmc_etr_get_ctcu_device(drvdata); > + if (!ctcu) > + return NULL; > + > + port = coresight_get_in_port(drvdata->csdev, ctcu); > + if (port < 0) > + return NULL; > + > + ctcu_drvdata = dev_get_drvdata(ctcu->dev.parent); > + byte_cntr_data = &ctcu_drvdata->byte_cntr_data[port]; > + if (byte_cntr_data && byte_cntr_data->thresh_val) > + return ctcu_drvdata->byte_cntr_sysfs_ops; > + > + return NULL; > +} > + Should be in a CTCU source file, not part of the common tmc code > static int tmc_read_prepare(struct tmc_drvdata *drvdata) > { > + const struct tmc_sysfs_ops *byte_cntr_sysfs_ops; > int ret = 0; > > + byte_cntr_sysfs_ops = tmc_get_byte_cntr_sysfs_ops(drvdata); > + if (byte_cntr_sysfs_ops) { > + ret = byte_cntr_sysfs_ops->read_prepare(drvdata); > + goto out; > + } > + > if (drvdata->sysfs_ops) > ret = drvdata->sysfs_ops->read_prepare(drvdata); > else > ret = -EINVAL; > I understand ctcu usage is per session & per device, but at the start of the session would it not be better to have a function in the ctcu code that takes the drvdata->sysfs_ops and substitutes the callback directly, restoring it at the end. > +out: > if (!ret) > dev_dbg(&drvdata->csdev->dev, "TMC read start\n"); > > @@ -245,13 +278,21 @@ static int tmc_read_prepare(struct tmc_drvdata > *drvdata) > > static int tmc_read_unprepare(struct tmc_drvdata *drvdata) > { > + const struct tmc_sysfs_ops *byte_cntr_sysfs_ops; > int ret = 0; > > + byte_cntr_sysfs_ops = tmc_get_byte_cntr_sysfs_ops(drvdata); > + if (byte_cntr_sysfs_ops) { > + ret = byte_cntr_sysfs_ops->read_unprepare(drvdata); > + goto out; > + } > + > if (drvdata->sysfs_ops) > ret = drvdata->sysfs_ops->read_unprepare(drvdata); > else > ret = -EINVAL; > Again override / restore over the session. Regards Mike > +out: > if (!ret) > dev_dbg(&drvdata->csdev->dev, "TMC read end\n"); > > @@ -277,6 +318,12 @@ 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) > { > + const struct tmc_sysfs_ops *byte_cntr_sysfs_ops; > + > + byte_cntr_sysfs_ops = tmc_get_byte_cntr_sysfs_ops(drvdata); > + if (byte_cntr_sysfs_ops) > + return byte_cntr_sysfs_ops->get_trace_data(drvdata, pos, > len, bufpp); > + > return drvdata->sysfs_ops->get_trace_data(drvdata, pos, len, bufpp); > } > > @@ -297,7 +344,11 @@ static ssize_t tmc_read(struct file *file, char __user > *data, size_t len, > return -EFAULT; > } > > - *ppos += actual; > + if (drvdata->reading_node) > + drvdata->reading_node->pos += actual; > + else > + *ppos += actual; > + > dev_dbg(&drvdata->csdev->dev, "%zu bytes copied\n", actual); > > return actual; > > -- > 2.34.1

1 month, 4 weeks

[PATCH v2 0/8] coresight: cti: Miscellaneous fixes and CPU PM cleanup

by Leo Yan

The CPU power management issue in the CTI driver was first observed in series [1]; this series resolves that issue. It fixes bugs and removes CPU PM operations from the CoreSight CTI driver, the goal is to use the CoreSight core layer as the central place for CPU power management. Removing CPU PM from CTI driver can avoid conflicts with the core layer. This series can be divided into: Patches 01 ~ 02: Fix spinlock with irqsave and register read with CS lock. Patches 03 ~ 08: Access ASICCTL condintioanlly, remove CPU PM code, and refactor register access in sysfs knob. This series is based on coresight-next branch and has been validated on Juno r2 platforms, pass normal sysfs and perf test, as well as CPU PM stress testing. [1] https://lore.kernel.org/all/20250915-arm_coresight_power_management_fix-v3-… Signed-off-by: Leo Yan <leo.yan(a)arm.com> --- Changes in v2: - Rebased on coresight-next branch (v7.1). - Kept read/write cache value in sysfs knob (Mike). - Link to v1: https://lore.kernel.org/r/20260209-arm_coresight_cti_refactor_v1-v1-0-db71a… --- Leo Yan (8): coresight: cti: Make spinlock usage consistent coresight: cti: Fix register reads coresight: cti: Access ASICCTL only when implemented coresight: cti: Remove CPU power management code coresight: cti: Rename cti_active() to cti_is_active() coresight: cti: Remove hw_powered flag coresight: cti: Remove hw_enabled flag coresight: cti: Properly handle negative offsets in cti_reg32_{show|store}() drivers/hwtracing/coresight/coresight-cti-core.c | 278 ++++------------------ drivers/hwtracing/coresight/coresight-cti-sysfs.c | 171 ++++++------- drivers/hwtracing/coresight/coresight-cti.h | 13 +- 3 files changed, 137 insertions(+), 325 deletions(-) --- base-commit: eef33a7cce239783d0422526a4d786289a936f1b change-id: 20251223-arm_coresight_cti_refactor_v1-76e1bda8b716 Best regards, -- Leo Yan <leo.yan(a)arm.com>

2 months

Re: [PATCH] drivers/hwtracing/coresight: remove unneeded variable in tmc_crashdata_release()

by Suzuki K Poulose

On Sat, 28 Feb 2026 18:23:44 -0400, Elsanti wrote: > The variable 'ret' is initialized to 0, never modified, and returned > directly. Remove it and return 0 explicitly. > > Applied, thanks! [1/1] drivers/hwtracing/coresight: remove unneeded variable in tmc_crashdata_release() https://git.kernel.org/coresight/c/061c39a17136 Best regards, -- Suzuki K Poulose <suzuki.poulose(a)arm.com>

2 months

Re: [PATCH] drivers/hwtracing/coresight: remove unneeded variable in tmc_crashdata_release()

by Leo Yan

On Sat, Feb 28, 2026 at 06:23:44PM -0400, Elsanti wrote: > The variable 'ret' is initialized to 0, never modified, and returned > directly. Remove it and return 0 explicitly. > > Signed-off-by: Elsanti <santiagojoseleal27(a)gmail.com> Reviewed-by: Leo Yan <leo.yan(a)arm.com> > --- > drivers/hwtracing/coresight/coresight-tmc-core.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c > index 36599c431be6..e29012c451c5 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc-core.c > +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c > @@ -401,7 +401,6 @@ static ssize_t tmc_crashdata_read(struct file *file, char __user *data, > > static int tmc_crashdata_release(struct inode *inode, struct file *file) > { > - int ret = 0; > unsigned long flags; > struct tmc_resrv_buf *rbuf; > struct tmc_drvdata *drvdata = container_of(file->private_data, > @@ -414,7 +413,7 @@ static int tmc_crashdata_release(struct inode *inode, struct file *file) > raw_spin_unlock_irqrestore(&drvdata->spinlock, flags); > > dev_dbg(&drvdata->csdev->dev, "%s: released\n", __func__); > - return ret; > + return 0; > } > > static const struct file_operations tmc_crashdata_fops = { > -- > 2.53.0 > >

2 months

← Newer
1
...
4
5
6
7
8
9
10
...
271
Older →

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight