[PATCH v3 RESEND 04/10] coresight: ctcu: enable byte-cntr for TMC ETR devices
Jie Gan
jie.gan at oss.qualcomm.com
Tue Jul 22 18:15:16 PDT 2025
On 7/22/2025 11:23 PM, Mike Leach wrote:
> Hi,
>
> There are some parameters in the new structure that are unused in this patch.
>
> please only introduce fields when they start to be used to make review easier.
>
Sure, will check carefully in future.
Thanks,
Jie
> Regards
>
> Mike
>
> On Mon, 14 Jul 2025 at 07:31, Jie Gan <jie.gan at oss.qualcomm.com> wrote:
>>
>> The byte-cntr function provided by the CTCU device is used to transfer data
>> from the ETR buffer to the userspace. An interrupt is 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 ETR buffer.
>>
>> Signed-off-by: Jie Gan <jie.gan at oss.qualcomm.com>
>> ---
>> .../testing/sysfs-bus-coresight-devices-ctcu | 5 +
>> drivers/hwtracing/coresight/Makefile | 2 +-
>> .../coresight/coresight-ctcu-byte-cntr.c | 102 ++++++++++++++++++
>> .../hwtracing/coresight/coresight-ctcu-core.c | 94 +++++++++++++++-
>> drivers/hwtracing/coresight/coresight-ctcu.h | 49 ++++++++-
>> 5 files changed, 246 insertions(+), 6 deletions(-)
>> create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight-devices-ctcu
>> create mode 100644 drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c
>>
>> 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..e21f5bcb8097
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-ctcu
>> @@ -0,0 +1,5 @@
>> +What: /sys/bus/coresight/devices/<ctcu-name>/irq_val
>> +Date: June 2025
>> +KernelVersion: 6.16
>> +Contact: Tingwei Zhang (QUIC) <quic_tingweiz at quicinc.com>; Jinlong Mao (QUIC) <quic_jinlmao at quicinc.com>; Jie Gan <jie.gan at oss.qualcomm.com>
>> +Description: (RW) Configure the IRQ value for byte-cntr register.
>> diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
>> index 4e7cc3c5bf99..3568d9768567 100644
>> --- a/drivers/hwtracing/coresight/Makefile
>> +++ b/drivers/hwtracing/coresight/Makefile
>> @@ -54,5 +54,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..d3b6eb7a89fb
>> --- /dev/null
>> +++ b/drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c
>> @@ -0,0 +1,102 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2025 Qualcomm Innovation Center, Inc. All rights reserved.
>> + */
>> +
>> +#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);
>> +
>> + byte_cntr_data->irq_num++;
>> +
>
> These two - irq_num & irq_cnt appear to count the same thing. Do not
> understand why one has to be atomic and the other does not.
>
>> + return IRQ_HANDLED;
>> +}
>> +
>> +/* 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 ctcu_byte_cntr *byte_cntr_data;
>> + int port_num;
>> +
>> + if (!sink)
>> + return;
>> +
>> + port_num = coresight_get_port_helper(sink, csdev);
>> + if (port_num < 0)
>> + return;
>> +
>> + byte_cntr_data = &drvdata->byte_cntr_data[port_num];
>> + /* 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_buf = false;
>> +}
>> +
>> +/* 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 (!sink || coresight_get_mode(sink) == CS_MODE_SYSFS)
>> + return;
>> +
>> + port_num = coresight_get_port_helper(sink, csdev);
>> + if (port_num < 0)
>> + return;
>> +
>> + byte_cntr_data = &drvdata->byte_cntr_data[port_num];
>> + 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 byte_cntr_irq, ret, i;
>> +
>> + for (i = 0; i < etr_num; i++) {
>> + byte_cntr_data = &drvdata->byte_cntr_data[i];
>> + byte_cntr_irq = of_irq_get_byname(nd, byte_cntr_data->irq_name);
>> + if (byte_cntr_irq < 0) {
>> + dev_err(dev, "Failed to get IRQ from DT for %s\n",
>> + byte_cntr_data->irq_name);
>> + continue;
>> + }
>> +
>> + ret = devm_request_irq(dev, byte_cntr_irq, byte_cntr_handler,
>> + IRQF_TRIGGER_RISING | IRQF_SHARED,
>> + dev_name(dev), byte_cntr_data);
>> + if (ret) {
>> + dev_err(dev, "Failed to register IRQ for %s\n",
>> + byte_cntr_data->irq_name);
>> + continue;
>> + }
>> +
>> + byte_cntr_data->byte_cntr_irq = byte_cntr_irq;
>> + disable_irq(byte_cntr_data->byte_cntr_irq);
>> + init_waitqueue_head(&byte_cntr_data->wq);
>> + }
>> +}
>> diff --git a/drivers/hwtracing/coresight/coresight-ctcu-core.c b/drivers/hwtracing/coresight/coresight-ctcu-core.c
>> index 28ea4a216345..721836d42523 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"
>> @@ -45,17 +46,23 @@ DEFINE_CORESIGHT_DEVLIST(ctcu_devs, "ctcu");
>>
>> #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,
>> + .irq_name = "etr0",
>> + .port_num = 0,
>> },
>> {
>> - .atid_offset = CTCU_ETR1_ATID0,
>> - .port_num = 1,
>> + .atid_offset = CTCU_ETR1_ATID0,
>> + .irq_ctrl_offset = CTCU_ETR1_IRQCTRL,
>> + .irq_name = "etr1",
>> + .port_num = 1,
>> },
>> };
>>
>> @@ -64,6 +71,76 @@ static const struct ctcu_config sa8775p_cfgs = {
>> .num_etr_config = ARRAY_SIZE(sa8775p_etr_cfgs),
>> };
>>
>> +static 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_val_show(struct device *dev, struct device_attribute *attr,
>> + char *buf)
>> +{
>> + struct ctcu_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> + int i, len = 0;
>> +
>> + for (i = 0; i < ETR_MAX_NUM; i++) {
>> + if (drvdata->byte_cntr_data[i].irq_ctrl_offset)
>> + len += scnprintf(buf + len, PAGE_SIZE - len, "%u ",
>> + drvdata->byte_cntr_data[i].thresh_val);
>> + }
>> +
>> + len += scnprintf(buf + len, PAGE_SIZE - len, "\n");
>> +
>> + return len;
>> +}
>> +
>> +/* Program a valid value into IRQCTRL register will enable byte-cntr interrupt */
>> +static ssize_t irq_val_store(struct device *dev, struct device_attribute *attr,
>> + const char *buf, size_t size)
>> +{
>> + struct ctcu_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> + u32 thresh_vals[ETR_MAX_NUM] = { 0 };
>> + u32 irq_ctrl_offset;
>> + int num, i;
>> +
>> + num = sscanf(buf, "%i %i", &thresh_vals[0], &thresh_vals[1]);
>> + if (num <= 0 || num > ETR_MAX_NUM)
>> + return -EINVAL;
>> +
>> + /* Threshold 0 disables the interruption. */
>> + guard(raw_spinlock_irqsave)(&drvdata->spin_lock);
>> + for (i = 0; i < num; i++) {
>> + /* A small threshold will result in a large number of interruptions */
>> + if (thresh_vals[i] && thresh_vals[i] < SZ_4K)
>> + return -EINVAL;
>> +
>> + if (drvdata->byte_cntr_data[i].irq_ctrl_offset) {
>> + drvdata->byte_cntr_data[i].thresh_val = thresh_vals[i];
>> + irq_ctrl_offset = drvdata->byte_cntr_data[i].irq_ctrl_offset;
>> + /* A one value for IRQCTRL register represents 8 bytes */
>> + ctcu_program_register(drvdata, thresh_vals[i] / 8, irq_ctrl_offset);
>> + }
>> + }
>> +
>> + return size;
>> +}
>> +static DEVICE_ATTR_RW(irq_val);
>> +
>
> I think it may make more sense to call this something with "threshold"
> - as it is thresholds that are being set.
>
>> +static struct attribute *ctcu_attrs[] = {
>> + &dev_attr_irq_val.attr,
>> + NULL,
>> +};
>> +
>> +static struct attribute_group ctcu_attr_grp = {
>> + .attrs = ctcu_attrs,
>> +};
>> +
>> +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)
>> {
>> @@ -143,6 +220,8 @@ static int ctcu_enable(struct coresight_device *csdev, enum cs_mode mode, void *
>> {
>> struct coresight_path *path = (struct coresight_path *)data;
>>
>> + ctcu_byte_cntr_start(csdev, path);
>> +
>> return ctcu_set_etr_traceid(csdev, path, true);
>> }
>>
>> @@ -150,6 +229,8 @@ static int ctcu_disable(struct coresight_device *csdev, void *data)
>> {
>> struct coresight_path *path = (struct coresight_path *)data;
>>
>> + ctcu_byte_cntr_stop(csdev, path);
>> +
>> return ctcu_set_etr_traceid(csdev, path, false);
>> }
>>
>> @@ -200,7 +281,11 @@ 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_name = etr_cfg->irq_name;
>> + drvdata->byte_cntr_data[i].irq_ctrl_offset =
>> + etr_cfg->irq_ctrl_offset;
>> }
>> + ctcu_byte_cntr_init(dev, drvdata, cfgs->num_etr_config);
>> }
>> }
>>
>> @@ -212,6 +297,7 @@ static int ctcu_probe(struct platform_device *pdev)
>> desc.subtype.helper_subtype = CORESIGHT_DEV_SUBTYPE_HELPER_CTCU;
>> desc.pdata = pdata;
>> desc.dev = dev;
>> + desc.groups = ctcu_attr_grps;
>> desc.ops = &ctcu_ops;
>> desc.access = CSDEV_ACCESS_IOMEM(base);
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-ctcu.h b/drivers/hwtracing/coresight/coresight-ctcu.h
>> index e9594c38dd91..71266371591b 100644
>> --- a/drivers/hwtracing/coresight/coresight-ctcu.h
>> +++ b/drivers/hwtracing/coresight/coresight-ctcu.h
>> @@ -5,19 +5,27 @@
>>
>> #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 (5 * 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;
>> + const char *irq_name;
>> };
>>
>> struct ctcu_config {
>> @@ -25,15 +33,54 @@ struct ctcu_config {
>> int num_etr_config;
>> };
>>
>> +/**
>> + * struct ctcu_byte_cntr
>> + * @enable: indicates that byte_cntr function is enabled or not.
>> + * @reading: indicates that its byte-cntr reading.
>> + * @reading_buf: indicates that byte-cntr is reading buffer.
>> + * @thresh_val: threshold to trigger a interruption.
>> + * @total_size: total size of transferred data.
>> + * @byte_cntr_irq: IRQ number.
>> + * @irq_cnt: IRQ count.
>> + * @irq_num: number of the byte_cntr IRQ for one session.
>
> the difference between byte_cntr_irg and irq_cnt is not clear.
>
>> + * @wq: workqueue of reading ETR data.
>> + * @read_work: work of reading ETR data.
>> + * @spin_lock: spinlock of byte cntr data.
>> + * the byte cntr is stopped.
>> + * @irq_ctrl_offset: offset to the BYTECNTVAL Register.
>> + * @irq_name: IRQ name in DT.
>> + */
>> +struct ctcu_byte_cntr {
>> + bool enable;
>> + bool reading;
>
> This parameter is unused in this patch
>
>> + bool reading_buf;
>> + u32 thresh_val;
>> + u64 total_size;
>
> parameter unused in this patch
>
>> + int byte_cntr_irq;
>> + atomic_t irq_cnt;
>> + int irq_num;
>> + wait_queue_head_t wq;
>> + struct work_struct read_work;
>> + raw_spinlock_t spin_lock;
>> + u32 irq_ctrl_offset;
>> + const char *irq_name;
>> +};
>> +
>> 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];
>> };
>>
>> +/* 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
>> --
>> 2.34.1
>>
>
>
More information about the linux-arm-kernel
mailing list