[PATCH v3 RESEND 00/10] coresight: ctcu: Enable byte-cntr function for TMC ETR

[PATCH v3 RESEND 00/10] coresight: ctcu: Enable byte-cntr function for TMC ETR

[Jie Gan]

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 0x10000 > /sys/bus/coresight/devices/ctcu0/irq_val
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_val

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/

Jie Gan (10):
  coresight: core: Refactoring ctcu_get_active_port and make it generic
  coresight: core: add a new API to retrieve the helper device
  dt-bindings: arm: add an interrupt property for Coresight CTCU
  coresight: ctcu: enable byte-cntr for TMC ETR devices
  coresight: tmc: add etr_buf_list to store allocated etr_buf
  coresight: tmc: add create/delete functions for etr_buf_node
  coresight: tmc: add prepare/unprepare functions for byte-cntr
  coresight: tmc: add a switch buffer function for byte-cntr
  coresight: tmc: add read function for byte-cntr
  arm64: dts: qcom: sa8775p: Add interrupts to CTCU device

 .../testing/sysfs-bus-coresight-devices-ctcu  |   5 +
 .../bindings/arm/qcom,coresight-ctcu.yaml     |  17 ++
 arch/arm64/boot/dts/qcom/sa8775p.dtsi         |   5 +
 drivers/hwtracing/coresight/Makefile          |   2 +-
 drivers/hwtracing/coresight/coresight-core.c  |  54 ++++
 .../coresight/coresight-ctcu-byte-cntr.c      | 102 +++++++
 .../hwtracing/coresight/coresight-ctcu-core.c | 113 ++++++--
 drivers/hwtracing/coresight/coresight-ctcu.h  |  49 +++-
 drivers/hwtracing/coresight/coresight-priv.h  |   4 +
 .../hwtracing/coresight/coresight-tmc-core.c  |  70 ++++-
 .../hwtracing/coresight/coresight-tmc-etr.c   | 270 ++++++++++++++++++
 drivers/hwtracing/coresight/coresight-tmc.h   |  29 ++
 12 files changed, 688 insertions(+), 32 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight-devices-ctcu
 create mode 100644 drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c

-- 
2.34.1

[PATCH v3 RESEND 01/10] coresight: core: Refactoring ctcu_get_active_port and make it generic

[Jie Gan]

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.

Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
---
 drivers/hwtracing/coresight/coresight-core.c  | 24 +++++++++++++++++++
 .../hwtracing/coresight/coresight-ctcu-core.c | 19 +--------------
 drivers/hwtracing/coresight/coresight-priv.h  |  2 ++
 3 files changed, 27 insertions(+), 18 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 1accd7cbd54b..5297a5ff7921 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -580,6 +580,30 @@ struct coresight_device *coresight_get_sink(struct coresight_path *path)
 }
 EXPORT_SYMBOL_GPL(coresight_get_sink);
 
+/**
+ * coresight_get_port_helper: get the in-port number of the helper device
+ * that is connected to the csdev.
+ *
+ * @csdev: csdev of the device that is connected to helper.
+ * @helper: csdev of the helper device.
+ *
+ * Return: port number upson success or -EINVAL for fail.
+ */
+int coresight_get_port_helper(struct coresight_device *csdev,
+			      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 == csdev)
+			return pdata->in_conns[i]->dest_port;
+	}
+
+	return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(coresight_get_port_helper);
+
 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 c6bafc96db96..28ea4a216345 100644
--- a/drivers/hwtracing/coresight/coresight-ctcu-core.c
+++ b/drivers/hwtracing/coresight/coresight-ctcu-core.c
@@ -118,23 +118,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)
 {
@@ -147,7 +130,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_port_helper(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 33e22b1ba043..07a5f03de81d 100644
--- a/drivers/hwtracing/coresight/coresight-priv.h
+++ b/drivers/hwtracing/coresight/coresight-priv.h
@@ -156,6 +156,8 @@ void coresight_remove_links(struct coresight_device *orig,
 u32 coresight_get_sink_id(struct coresight_device *csdev);
 void coresight_path_assign_trace_id(struct coresight_path *path,
 				   enum cs_mode mode);
+int coresight_get_port_helper(struct coresight_device *csdev,
+			      struct coresight_device *helper);
 
 #if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM3X)
 int etm_readl_cp14(u32 off, unsigned int *val);
-- 
2.34.1

[PATCH v3 RESEND 02/10] coresight: core: add a new API to retrieve the helper device

[Jie Gan]

Retrieving the helper device of the specific coresight device based on
its helper_subtype because a single coresight device may has multiple types
of the helper devices.

Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
---
 drivers/hwtracing/coresight/coresight-core.c | 30 ++++++++++++++++++++
 drivers/hwtracing/coresight/coresight-priv.h |  2 ++
 2 files changed, 32 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 5297a5ff7921..76e10c36a8a1 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -580,6 +580,36 @@ struct coresight_device *coresight_get_sink(struct coresight_path *path)
 }
 EXPORT_SYMBOL_GPL(coresight_get_sink);
 
+/**
+ * coresight_get_helper: find the helper device of the assigned csdev.
+ *
+ * @csdev: The csdev the helper device is conntected to.
+ * @type:  helper_subtype of the expected helper device.
+ *
+ * Retrieve the helper device for the specific csdev based on its
+ * helper_subtype.
+ *
+ * Return: the helper's csdev upon success or NULL for fail.
+ */
+struct coresight_device *coresight_get_helper(struct coresight_device *csdev,
+					      int type)
+{
+	int i;
+	struct coresight_device *helper;
+
+	for (i = 0; i < csdev->pdata->nr_outconns; ++i) {
+		helper = csdev->pdata->out_conns[i]->dest_dev;
+		if (!helper || !coresight_is_helper(helper))
+			continue;
+
+		if (helper->subtype.helper_subtype == type)
+			return helper;
+	}
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(coresight_get_helper);
+
 /**
  * coresight_get_port_helper: get the in-port number of the helper device
  * that is connected to the csdev.
diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
index 07a5f03de81d..5b912eb60401 100644
--- a/drivers/hwtracing/coresight/coresight-priv.h
+++ b/drivers/hwtracing/coresight/coresight-priv.h
@@ -158,6 +158,8 @@ void coresight_path_assign_trace_id(struct coresight_path *path,
 				   enum cs_mode mode);
 int coresight_get_port_helper(struct coresight_device *csdev,
 			      struct coresight_device *helper);
+struct coresight_device *coresight_get_helper(struct coresight_device *csdev,
+					      int type);
 
 #if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM3X)
 int etm_readl_cp14(u32 off, unsigned int *val);
-- 
2.34.1

[PATCH v3 RESEND 03/10] dt-bindings: arm: add an interrupt property for Coresight CTCU

[Jie Gan]

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>
Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
---
 .../bindings/arm/qcom,coresight-ctcu.yaml       | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml
index 843b52eaf872..ea05ad8f3dd3 100644
--- a/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml
@@ -39,6 +39,16 @@ properties:
     items:
       - const: apb
 
+  interrupts:
+    items:
+      - description: Byte cntr interrupt for etr0
+      - description: Byte cntr interrupt for etr1
+
+  interrupt-names:
+    items:
+      - const: etr0
+      - const: etr1
+
   in-ports:
     $ref: /schemas/graph.yaml#/properties/ports
 
@@ -56,6 +66,8 @@ additionalProperties: false
 
 examples:
   - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
     ctcu@1001000 {
         compatible = "qcom,sa8775p-ctcu";
         reg = <0x1001000 0x1000>;
@@ -63,6 +75,11 @@ examples:
         clocks = <&aoss_qmp>;
         clock-names = "apb";
 
+        interrupts = <GIC_SPI 270 IRQ_TYPE_EDGE_RISING>,
+                     <GIC_SPI 262 IRQ_TYPE_EDGE_RISING>;
+        interrupt-names = "etr0",
+                          "etr1";
+
         in-ports {
             #address-cells = <1>;
             #size-cells = <0>;
-- 
2.34.1

[PATCH v3 RESEND 04/10] coresight: ctcu: enable byte-cntr for TMC ETR devices

[Jie Gan]

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@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@quicinc.com>; Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>; Jie Gan <jie.gan@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++;
+
+	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);
+
+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.
+ * @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;
+	bool			reading_buf;
+	u32			thresh_val;
+	u64			total_size;
+	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

[PATCH v3 RESEND 05/10] coresight: tmc: add etr_buf_list to store allocated etr_buf

[Jie Gan]

Add a list to store allocated etr_buf.

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.

Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
---
 .../hwtracing/coresight/coresight-tmc-core.c  |  1 +
 drivers/hwtracing/coresight/coresight-tmc.h   | 19 +++++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c
index be964656be93..4d249af93097 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-core.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-core.c
@@ -830,6 +830,7 @@ static int __tmc_probe(struct device *dev, struct resource *res)
 		idr_init(&drvdata->idr);
 		mutex_init(&drvdata->idr_mutex);
 		dev_list = &etr_devs;
+		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.h b/drivers/hwtracing/coresight/coresight-tmc.h
index 6541a27a018e..f6b05639aeca 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.h
+++ b/drivers/hwtracing/coresight/coresight-tmc.h
@@ -208,6 +208,21 @@ struct tmc_resrv_buf {
 	s64		len;
 };
 
+/**
+ * @sysfs_buf:	Allocated sysfs_buf.
+ * @is_free:	Indicates whether the buffer is free to choose.
+ * @reading:	Indicates whether the buffer is reading.
+ * @pos:	Position of the buffer.
+ * @node:	Node in etr_buf_list.
+ */
+struct etr_buf_node {
+	struct etr_buf		*sysfs_buf;
+	bool			is_free;
+	bool			reading;
+	loff_t			pos;
+	struct list_head	node;
+};
+
 /**
  * struct tmc_drvdata - specifics associated to an TMC component
  * @pclk:	APB clock if present, otherwise NULL
@@ -242,6 +257,8 @@ 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.
+ * @reading_node:  Available buffer for byte-cntr reading.
  */
 struct tmc_drvdata {
 	struct clk		*pclk;
@@ -271,6 +288,8 @@ 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_node     *reading_node;
 };
 
 struct etr_buf_operations {
-- 
2.34.1

[PATCH v3 RESEND 06/10] coresight: tmc: add create/delete functions for etr_buf_node

[Jie Gan]

Create and insert or remove the etr_buf_node to/from the etr_buf_list.

Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
---
 .../hwtracing/coresight/coresight-tmc-etr.c   | 49 +++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index b07fcdb3fe1a..4609df80ae38 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -1909,6 +1909,55 @@ const struct coresight_ops tmc_etr_cs_ops = {
 	.panic_ops	= &tmc_etr_sync_ops,
 };
 
+static void tmc_delete_etr_buf_node(struct tmc_drvdata *drvdata)
+{
+	struct etr_buf_node *nd, *next;
+
+	list_for_each_entry_safe(nd, next, &drvdata->etr_buf_list, node) {
+		if (nd->sysfs_buf == drvdata->sysfs_buf) {
+			list_del(&nd->node);
+			kfree(nd);
+		} else {
+			/* Free allocated buffers which are not utilized by ETR */
+			list_del(&nd->node);
+			tmc_free_etr_buf(nd->sysfs_buf);
+			nd->sysfs_buf = NULL;
+			kfree(nd);
+		}
+	}
+}
+
+static int tmc_create_etr_buf_node(struct tmc_drvdata *drvdata, struct etr_buf *alloc_buf)
+{
+	struct etr_buf_node *sysfs_buf_node;
+	struct etr_buf *sysfs_buf;
+
+	if (!alloc_buf) {
+		sysfs_buf = tmc_alloc_etr_buf(drvdata, drvdata->size, 0, cpu_to_node(0), NULL);
+		if (IS_ERR(sysfs_buf))
+			return PTR_ERR(sysfs_buf);
+	} else {
+		sysfs_buf = alloc_buf;
+	}
+
+	sysfs_buf_node = kzalloc(sizeof(struct etr_buf_node), GFP_KERNEL);
+	if (IS_ERR(sysfs_buf_node)) {
+		if (!alloc_buf)
+			tmc_free_etr_buf(sysfs_buf);
+		return PTR_ERR(sysfs_buf_node);
+	}
+
+	sysfs_buf_node->sysfs_buf = sysfs_buf;
+	sysfs_buf_node->reading = false;
+	if (!alloc_buf)
+		sysfs_buf_node->is_free = true;
+	else
+		sysfs_buf_node->is_free = false;
+	list_add(&sysfs_buf_node->node, &drvdata->etr_buf_list);
+
+	return 0;
+}
+
 int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
 {
 	int ret = 0;
-- 
2.34.1

[PATCH v3 RESEND 07/10] coresight: tmc: add prepare/unprepare functions for byte-cntr

[Jie Gan]

Prepare for byte-cntr reading. An additional sysfs_buf is required to
receive trace data, as byte-cntr always reads from the deactivated
and filled sysfs_buf.

The unprepare function releases the additional deactivated sysfs_buf
allocated during the prepare phase.

Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
---
 .../hwtracing/coresight/coresight-tmc-core.c  | 38 ++++++++-
 .../hwtracing/coresight/coresight-tmc-etr.c   | 79 +++++++++++++++++++
 drivers/hwtracing/coresight/coresight-tmc.h   |  7 ++
 3 files changed, 120 insertions(+), 4 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c
index 4d249af93097..354faeeddbb2 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-core.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-core.c
@@ -230,7 +230,11 @@ static int tmc_prepare_crashdata(struct tmc_drvdata *drvdata)
 
 static int tmc_read_prepare(struct tmc_drvdata *drvdata)
 {
-	int ret = 0;
+	struct coresight_device *helper = coresight_get_helper(drvdata->csdev,
+						CORESIGHT_DEV_SUBTYPE_HELPER_CTCU);
+	struct ctcu_byte_cntr *byte_cntr_data = NULL;
+	struct ctcu_drvdata *ctcu_drvdata = NULL;
+	int port, ret = 0;
 
 	switch (drvdata->config_type) {
 	case TMC_CONFIG_TYPE_ETB:
@@ -238,7 +242,18 @@ static int tmc_read_prepare(struct tmc_drvdata *drvdata)
 		ret = tmc_read_prepare_etb(drvdata);
 		break;
 	case TMC_CONFIG_TYPE_ETR:
-		ret = tmc_read_prepare_etr(drvdata);
+		if (helper) {
+			port = coresight_get_port_helper(drvdata->csdev, helper);
+			if (port >= 0) {
+				ctcu_drvdata = dev_get_drvdata(helper->dev.parent);
+				byte_cntr_data = &ctcu_drvdata->byte_cntr_data[port];
+			}
+		}
+
+		if (byte_cntr_data && byte_cntr_data->thresh_val)
+			ret = tmc_read_prepare_byte_cntr(drvdata, byte_cntr_data);
+		else
+			ret = tmc_read_prepare_etr(drvdata);
 		break;
 	default:
 		ret = -EINVAL;
@@ -252,7 +267,11 @@ static int tmc_read_prepare(struct tmc_drvdata *drvdata)
 
 static int tmc_read_unprepare(struct tmc_drvdata *drvdata)
 {
-	int ret = 0;
+	struct coresight_device *helper = coresight_get_helper(drvdata->csdev,
+						CORESIGHT_DEV_SUBTYPE_HELPER_CTCU);
+	struct ctcu_byte_cntr *byte_cntr_data = NULL;
+	struct ctcu_drvdata *ctcu_drvdata = NULL;
+	int port, ret = 0;
 
 	switch (drvdata->config_type) {
 	case TMC_CONFIG_TYPE_ETB:
@@ -260,7 +279,18 @@ static int tmc_read_unprepare(struct tmc_drvdata *drvdata)
 		ret = tmc_read_unprepare_etb(drvdata);
 		break;
 	case TMC_CONFIG_TYPE_ETR:
-		ret = tmc_read_unprepare_etr(drvdata);
+		if (helper) {
+			port = coresight_get_port_helper(drvdata->csdev, helper);
+			if (port >= 0) {
+				ctcu_drvdata = dev_get_drvdata(helper->dev.parent);
+				byte_cntr_data = &ctcu_drvdata->byte_cntr_data[port];
+			}
+		}
+
+		if (byte_cntr_data && byte_cntr_data->thresh_val)
+			ret = tmc_read_unprepare_byte_cntr(drvdata, byte_cntr_data);
+		else
+			ret = tmc_read_unprepare_etr(drvdata);
 		break;
 	default:
 		ret = -EINVAL;
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index 4609df80ae38..2b73bd8074bb 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -2032,6 +2032,85 @@ int tmc_read_unprepare_etr(struct tmc_drvdata *drvdata)
 	return 0;
 }
 
+int tmc_read_prepare_byte_cntr(struct tmc_drvdata *drvdata,
+			       struct ctcu_byte_cntr *byte_cntr_data)
+{
+	unsigned long flags;
+	int ret = 0;
+
+	/* config types are set a boot time and never change */
+	if (WARN_ON_ONCE(drvdata->config_type != TMC_CONFIG_TYPE_ETR))
+		return -EINVAL;
+
+	if (coresight_get_mode(drvdata->csdev) != CS_MODE_SYSFS)
+		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) {
+		dev_err(&drvdata->csdev->dev, "The threshold value is too large\n");
+		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);
+	/* Insert current sysfs_buf into the list */
+	ret = tmc_create_etr_buf_node(drvdata, drvdata->sysfs_buf);
+	if (!ret) {
+		/*
+		 * Add one more sysfs_buf for byte-cntr function, byte-cntr always reads
+		 * the data from the buffer which has been synced. Switch the buffer when
+		 * the used buffer is nearly full. The used buffer will be synced and made
+		 * available for reading before switch.
+		 */
+		ret = tmc_create_etr_buf_node(drvdata, NULL);
+		if (ret) {
+			dev_err(&drvdata->csdev->dev, "Failed to create etr_buf_node\n");
+			tmc_delete_etr_buf_node(drvdata);
+			byte_cntr_data->reading = false;
+			goto out;
+		}
+	}
+
+	raw_spin_lock_irqsave(&drvdata->spinlock, flags);
+	atomic_set(&byte_cntr_data->irq_cnt, 0);
+	enable_irq(byte_cntr_data->byte_cntr_irq);
+	enable_irq_wake(byte_cntr_data->byte_cntr_irq);
+	byte_cntr_data->total_size = 0;
+	byte_cntr_data->irq_num = 0;
+
+out_unlock:
+	raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
+
+out:
+	return ret;
+}
+
+int tmc_read_unprepare_byte_cntr(struct tmc_drvdata *drvdata,
+				 struct ctcu_byte_cntr *byte_cntr_data)
+{
+	struct device *dev = &drvdata->csdev->dev;
+
+	guard(raw_spinlock_irqsave)(&byte_cntr_data->spin_lock);
+	disable_irq_wake(byte_cntr_data->byte_cntr_irq);
+	disable_irq(byte_cntr_data->byte_cntr_irq);
+	byte_cntr_data->reading = false;
+	tmc_delete_etr_buf_node(drvdata);
+	dev_dbg(dev, "send data total size:%llu bytes, irq_cnt:%d\n",
+		byte_cntr_data->total_size, byte_cntr_data->irq_num);
+
+	return 0;
+}
+
 static const char *const buf_modes_str[] = {
 	[ETR_MODE_FLAT]		= "flat",
 	[ETR_MODE_ETR_SG]	= "tmc-sg",
diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
index f6b05639aeca..1dbba0bc50a3 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.h
+++ b/drivers/hwtracing/coresight/coresight-tmc.h
@@ -14,6 +14,8 @@
 #include <linux/refcount.h>
 #include <linux/crc32.h>
 
+#include "coresight-ctcu.h"
+
 #define TMC_RSZ			0x004
 #define TMC_STS			0x00c
 #define TMC_RRD			0x010
@@ -357,6 +359,11 @@ 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);
 
+/* Byte-cntr functions */
+int tmc_read_prepare_byte_cntr(struct tmc_drvdata *drvdata,
+			       struct ctcu_byte_cntr *byte_cntr_data);
+int tmc_read_unprepare_byte_cntr(struct tmc_drvdata *drvdata,
+				 struct ctcu_byte_cntr *byte_cntr_data);
 
 #define TMC_REG_PAIR(name, lo_off, hi_off)				\
 static inline u64							\
-- 
2.34.1

[PATCH v3 RESEND 08/10] coresight: tmc: add a switch buffer function for byte-cntr

[Jie Gan]

Switching the sysfs_buf when current buffer is full or the timeout is
triggered and resets rrp and rwp registers after switched the buffer.
Disable the ETR device if it cannot find an available buffer to switch.

Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
---
 .../hwtracing/coresight/coresight-tmc-etr.c   | 52 +++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index 2b73bd8074bb..3e3e1b5e78ca 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -1287,6 +1287,58 @@ static struct etr_buf *tmc_etr_get_sysfs_buffer(struct coresight_device *csdev)
 	return ret ? ERR_PTR(ret) : drvdata->sysfs_buf;
 }
 
+static bool tmc_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 we start the switching process */
+	if (coresight_get_mode(drvdata->csdev) == CS_MODE_SYSFS)
+		__tmc_etr_disable_hw(drvdata);
+
+	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;
+		}
+
+		if (!found_free_buf && nd->is_free && nd->sysfs_buf != curr_buf) {
+			if (nd->reading)
+				continue;
+
+			picked_node = nd;
+			found_free_buf = true;
+		}
+	}
+
+	if (found_free_buf) {
+		curr_node->reading = true;
+		curr_node->pos = 0;
+		drvdata->reading_node = curr_node;
+		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*/
+		CS_UNLOCK(drvdata->base);
+		tmc_write_rrp(drvdata, 0);
+		tmc_write_rwp(drvdata, 0);
+		CS_LOCK(drvdata->base);
+		/* Restart the ETR when we find a free buffer */
+		if (coresight_get_mode(drvdata->csdev) == CS_MODE_SYSFS)
+			__tmc_etr_enable_hw(drvdata);
+	}
+
+	return found_free_buf;
+}
+
 static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev)
 {
 	int ret = 0;
-- 
2.34.1

[PATCH v3 RESEND 09/10] coresight: tmc: add read function for byte-cntr

[Jie Gan]

The byte-cntr read function always reads trace data from the deactivated
and filled buffer which is already synced. The read function will fail
when the ETR cannot find a available buffer to receive trace data.

The read function terminates when the path is disabled or interrupted by a
signal.

Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
---
 .../hwtracing/coresight/coresight-tmc-core.c  | 31 ++++++-
 .../hwtracing/coresight/coresight-tmc-etr.c   | 90 +++++++++++++++++++
 drivers/hwtracing/coresight/coresight-tmc.h   |  3 +
 3 files changed, 120 insertions(+), 4 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c
index 354faeeddbb2..3ab25adc4e4d 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-core.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-core.c
@@ -318,14 +318,18 @@ static int tmc_open(struct inode *inode, struct file *file)
 	return 0;
 }
 
-static ssize_t tmc_get_sysfs_trace(struct tmc_drvdata *drvdata, loff_t pos, size_t len,
-				   char **bufpp)
+static ssize_t tmc_get_sysfs_trace(struct tmc_drvdata *drvdata,
+				   struct ctcu_byte_cntr *byte_cntr_data,
+				   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:
+		if (byte_cntr_data && byte_cntr_data->thresh_val)
+			return tmc_byte_cntr_get_data(drvdata, byte_cntr_data, len, bufpp);
+
 		return tmc_etr_get_sysfs_trace(drvdata, pos, len, bufpp);
 	}
 
@@ -339,7 +343,21 @@ static ssize_t tmc_read(struct file *file, char __user *data, size_t len,
 	ssize_t actual;
 	struct tmc_drvdata *drvdata = container_of(file->private_data,
 						   struct tmc_drvdata, miscdev);
-	actual = tmc_get_sysfs_trace(drvdata, *ppos, len, &bufp);
+	struct coresight_device *helper = coresight_get_helper(drvdata->csdev,
+						CORESIGHT_DEV_SUBTYPE_HELPER_CTCU);
+	struct ctcu_byte_cntr *byte_cntr_data = NULL;
+	struct ctcu_drvdata *ctcu_drvdata = NULL;
+	int port;
+
+	if (helper) {
+		port = coresight_get_port_helper(drvdata->csdev, helper);
+		if (port >= 0) {
+			ctcu_drvdata = dev_get_drvdata(helper->dev.parent);
+			byte_cntr_data = &ctcu_drvdata->byte_cntr_data[port];
+		}
+	}
+
+	actual = tmc_get_sysfs_trace(drvdata, byte_cntr_data, *ppos, len, &bufp);
 	if (actual <= 0)
 		return 0;
 
@@ -349,7 +367,12 @@ static ssize_t tmc_read(struct file *file, char __user *data, size_t len,
 		return -EFAULT;
 	}
 
-	*ppos += actual;
+	if (byte_cntr_data && byte_cntr_data->thresh_val) {
+		byte_cntr_data->total_size += actual;
+		drvdata->reading_node->pos += actual;
+	} else
+		*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 3e3e1b5e78ca..174411e76047 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -1163,6 +1163,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->reading_node)
+		etr_buf = drvdata->reading_node->sysfs_buf;
+
 	if (pos + actual > etr_buf->len)
 		actual = etr_buf->len - pos;
 	if (actual <= 0)
@@ -1339,6 +1343,92 @@ static bool tmc_byte_cntr_switch_buffer(struct tmc_drvdata *drvdata,
 	return found_free_buf;
 }
 
+/*
+ * tmc_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.
+ */
+ssize_t tmc_byte_cntr_get_data(struct tmc_drvdata *drvdata,
+			       struct ctcu_byte_cntr *byte_cntr_data,
+			       size_t len, char **bufpp)
+{
+	size_t thresh_val = byte_cntr_data->thresh_val;
+	atomic_t *irq_cnt = &byte_cntr_data->irq_cnt;
+	struct etr_buf *sysfs_buf = drvdata->sysfs_buf;
+	struct device *dev = &drvdata->csdev->dev;
+	struct etr_buf_node *nd, *next;
+	ssize_t size = sysfs_buf->size;
+	ssize_t actual;
+	loff_t pos;
+	int ret;
+
+wait_buffer:
+	if (!byte_cntr_data->reading_buf) {
+		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 (!tmc_byte_cntr_switch_buffer(drvdata, byte_cntr_data)) {
+				dev_err(dev, "Switch buffer failed for byte-cntr\n");
+				return -EINVAL;
+			}
+
+			byte_cntr_data->reading_buf = true;
+		} else {
+			if (!drvdata->reading_node) {
+				list_for_each_entry_safe(nd, next, &drvdata->etr_buf_list, node) {
+					if (nd->sysfs_buf == sysfs_buf) {
+						nd->pos = 0;
+						drvdata->reading_node = nd;
+						break;
+					}
+				}
+			}
+
+			pos = drvdata->reading_node->pos;
+			actual = tmc_etr_get_sysfs_trace(drvdata, pos, len, bufpp);
+			if (actual > 0)
+				return actual;
+
+			drvdata->reading_node = NULL;
+
+			/* Exit byte-cntr reading */
+			return -EINVAL;
+		}
+	}
+
+	/* Check the status of current etr_buf*/
+	if ((atomic_read(irq_cnt) + 1) * thresh_val >= size)
+		/*
+		 * Unlikely to find a free buffer to switch, so just disable
+		 * the ETR for a while.
+		 */
+		if (!tmc_byte_cntr_switch_buffer(drvdata, byte_cntr_data))
+			dev_info(dev, "No available buffer to store data, disable ETR\n");
+
+	pos = drvdata->reading_node->pos;
+	actual = tmc_etr_get_sysfs_trace(drvdata, pos, len, bufpp);
+	if (actual == 0) {
+		/* Reading work for marked buffer has finished, reset flags */
+		drvdata->reading_node->reading = false;
+		byte_cntr_data->reading_buf = false;
+		drvdata->reading_node = NULL;
+
+		/* Nothing in the buffer, wait for next buffer to be filled */
+		goto wait_buffer;
+	}
+
+	return actual;
+}
+
 static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev)
 {
 	int ret = 0;
diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
index 1dbba0bc50a3..4136ec5ecaf7 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.h
+++ b/drivers/hwtracing/coresight/coresight-tmc.h
@@ -364,6 +364,9 @@ int tmc_read_prepare_byte_cntr(struct tmc_drvdata *drvdata,
 			       struct ctcu_byte_cntr *byte_cntr_data);
 int tmc_read_unprepare_byte_cntr(struct tmc_drvdata *drvdata,
 				 struct ctcu_byte_cntr *byte_cntr_data);
+ssize_t tmc_byte_cntr_get_data(struct tmc_drvdata *drvdata,
+			       struct ctcu_byte_cntr *byte_cntr_data,
+			       size_t len, char **bufpp);
 
 #define TMC_REG_PAIR(name, lo_off, hi_off)				\
 static inline u64							\
-- 
2.34.1

[PATCH v3 RESEND 10/10] arm64: dts: qcom: sa8775p: Add interrupts to CTCU device

[Jie Gan]

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/sa8775p.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
index fed34717460f..44da72cebcf4 100644
--- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
+++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
@@ -2762,6 +2762,11 @@ ctcu@4001000 {
 			clocks = <&aoss_qmp>;
 			clock-names = "apb";
 
+			interrupts = <GIC_SPI 270 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 262 IRQ_TYPE_EDGE_RISING>;
+			interrupt-names = "etr0",
+					  "etr1";
+
 			in-ports {
 				#address-cells = <1>;
 				#size-cells = <0>;
-- 
2.34.1

Re: [PATCH v3 RESEND 01/10] coresight: core: Refactoring ctcu_get_active_port and make it generic

[Mike Leach]

Hi,

On Mon, 14 Jul 2025 at 07:31, Jie Gan <jie.gan@oss.qualcomm.com> wrote:
>
> 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.
>
> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
> ---
>  drivers/hwtracing/coresight/coresight-core.c  | 24 +++++++++++++++++++
>  .../hwtracing/coresight/coresight-ctcu-core.c | 19 +--------------
>  drivers/hwtracing/coresight/coresight-priv.h  |  2 ++
>  3 files changed, 27 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 1accd7cbd54b..5297a5ff7921 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -580,6 +580,30 @@ struct coresight_device *coresight_get_sink(struct coresight_path *path)
>  }
>  EXPORT_SYMBOL_GPL(coresight_get_sink);
>
> +/**
> + * coresight_get_port_helper: get the in-port number of the helper device
> + * that is connected to the csdev.
> + *

As written this looks at all connections, not just those that are
helpers. That is fine, so perhaps rename as such.

e.g. coresight_get_in_port_dest

and name the input parameters src , dest respectively.

> + * @csdev: csdev of the device that is connected to helper.
> + * @helper: csdev of the helper device.
> + *
> + * Return: port number upson success or -EINVAL for fail.

sp: upon/upson

> + */
> +int coresight_get_port_helper(struct coresight_device *csdev,
> +                             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 == csdev)
> +                       return pdata->in_conns[i]->dest_port;
> +       }
> +
> +       return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(coresight_get_port_helper);
> +
>  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 c6bafc96db96..28ea4a216345 100644
> --- a/drivers/hwtracing/coresight/coresight-ctcu-core.c
> +++ b/drivers/hwtracing/coresight/coresight-ctcu-core.c
> @@ -118,23 +118,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)
>  {
> @@ -147,7 +130,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_port_helper(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 33e22b1ba043..07a5f03de81d 100644
> --- a/drivers/hwtracing/coresight/coresight-priv.h
> +++ b/drivers/hwtracing/coresight/coresight-priv.h
> @@ -156,6 +156,8 @@ void coresight_remove_links(struct coresight_device *orig,
>  u32 coresight_get_sink_id(struct coresight_device *csdev);
>  void coresight_path_assign_trace_id(struct coresight_path *path,
>                                    enum cs_mode mode);
> +int coresight_get_port_helper(struct coresight_device *csdev,
> +                             struct coresight_device *helper);
>
rename here too

>  #if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM3X)
>  int etm_readl_cp14(u32 off, unsigned int *val);
> --
> 2.34.1
>

regards

Mike
-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK

Re: [PATCH v3 RESEND 01/10] coresight: core: Refactoring ctcu_get_active_port and make it generic

[Jie Gan]

On 7/16/2025 6:20 PM, Mike Leach wrote:
> Hi,
> 
> On Mon, 14 Jul 2025 at 07:31, Jie Gan <jie.gan@oss.qualcomm.com> wrote:
>>
>> 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.
>>
>> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
>> ---
>>   drivers/hwtracing/coresight/coresight-core.c  | 24 +++++++++++++++++++
>>   .../hwtracing/coresight/coresight-ctcu-core.c | 19 +--------------
>>   drivers/hwtracing/coresight/coresight-priv.h  |  2 ++
>>   3 files changed, 27 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
>> index 1accd7cbd54b..5297a5ff7921 100644
>> --- a/drivers/hwtracing/coresight/coresight-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>> @@ -580,6 +580,30 @@ struct coresight_device *coresight_get_sink(struct coresight_path *path)
>>   }
>>   EXPORT_SYMBOL_GPL(coresight_get_sink);
>>
>> +/**
>> + * coresight_get_port_helper: get the in-port number of the helper device
>> + * that is connected to the csdev.
>> + *
> 
> As written this looks at all connections, not just those that are
> helpers. That is fine, so perhaps rename as such.
> 
> e.g. coresight_get_in_port_dest
> 
> and name the input parameters src , dest respectively.

Make sense for me, will update in next version.
Thanks for the suggestion.

> 
>> + * @csdev: csdev of the device that is connected to helper.
>> + * @helper: csdev of the helper device.
>> + *
>> + * Return: port number upson success or -EINVAL for fail.
> 
> sp: upon/upson
> 
>> + */
>> +int coresight_get_port_helper(struct coresight_device *csdev,
>> +                             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 == csdev)
>> +                       return pdata->in_conns[i]->dest_port;
>> +       }
>> +
>> +       return -EINVAL;
>> +}
>> +EXPORT_SYMBOL_GPL(coresight_get_port_helper);
>> +
>>   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 c6bafc96db96..28ea4a216345 100644
>> --- a/drivers/hwtracing/coresight/coresight-ctcu-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-ctcu-core.c
>> @@ -118,23 +118,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)
>>   {
>> @@ -147,7 +130,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_port_helper(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 33e22b1ba043..07a5f03de81d 100644
>> --- a/drivers/hwtracing/coresight/coresight-priv.h
>> +++ b/drivers/hwtracing/coresight/coresight-priv.h
>> @@ -156,6 +156,8 @@ void coresight_remove_links(struct coresight_device *orig,
>>   u32 coresight_get_sink_id(struct coresight_device *csdev);
>>   void coresight_path_assign_trace_id(struct coresight_path *path,
>>                                     enum cs_mode mode);
>> +int coresight_get_port_helper(struct coresight_device *csdev,
>> +                             struct coresight_device *helper);
>>
> rename here too

will do.

Best Regards,
Jie

> 
>>   #if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM3X)
>>   int etm_readl_cp14(u32 off, unsigned int *val);
>> --
>> 2.34.1
>>
> 
> regards
> 
> Mike

Re: [PATCH v3 RESEND 02/10] coresight: core: add a new API to retrieve the helper device

[Mike Leach]

Hi,

On Mon, 14 Jul 2025 at 07:31, Jie Gan <jie.gan@oss.qualcomm.com> wrote:
>
> Retrieving the helper device of the specific coresight device based on
> its helper_subtype because a single coresight device may has multiple types
> of the helper devices.
>
> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
> ---
>  drivers/hwtracing/coresight/coresight-core.c | 30 ++++++++++++++++++++
>  drivers/hwtracing/coresight/coresight-priv.h |  2 ++
>  2 files changed, 32 insertions(+)
>
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 5297a5ff7921..76e10c36a8a1 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -580,6 +580,36 @@ struct coresight_device *coresight_get_sink(struct coresight_path *path)
>  }
>  EXPORT_SYMBOL_GPL(coresight_get_sink);
>
> +/**
> + * coresight_get_helper: find the helper device of the assigned csdev.
> + *
> + * @csdev: The csdev the helper device is conntected to.
> + * @type:  helper_subtype of the expected helper device.
> + *
> + * Retrieve the helper device for the specific csdev based on its
> + * helper_subtype.
> + *
> + * Return: the helper's csdev upon success or NULL for fail.
> + */
> +struct coresight_device *coresight_get_helper(struct coresight_device *csdev,
> +                                             int type)
> +{
> +       int i;
> +       struct coresight_device *helper;
> +
> +       for (i = 0; i < csdev->pdata->nr_outconns; ++i) {
> +               helper = csdev->pdata->out_conns[i]->dest_dev;
> +               if (!helper || !coresight_is_helper(helper))
> +                       continue;
> +

Manipulating the connections list almost certainly requires some
locking. See other functions in this file

Mike


> +               if (helper->subtype.helper_subtype == type)
> +                       return helper;
> +       }
> +
> +       return NULL;
> +}
> +EXPORT_SYMBOL_GPL(coresight_get_helper);
> +
>  /**
>   * coresight_get_port_helper: get the in-port number of the helper device
>   * that is connected to the csdev.
> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
> index 07a5f03de81d..5b912eb60401 100644
> --- a/drivers/hwtracing/coresight/coresight-priv.h
> +++ b/drivers/hwtracing/coresight/coresight-priv.h
> @@ -158,6 +158,8 @@ void coresight_path_assign_trace_id(struct coresight_path *path,
>                                    enum cs_mode mode);
>  int coresight_get_port_helper(struct coresight_device *csdev,
>                               struct coresight_device *helper);
> +struct coresight_device *coresight_get_helper(struct coresight_device *csdev,
> +                                             int type);
>
>  #if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM3X)
>  int etm_readl_cp14(u32 off, unsigned int *val);
> --
> 2.34.1
>


-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK

Re: [PATCH v3 RESEND 02/10] coresight: core: add a new API to retrieve the helper device

[Jie Gan]

On 7/18/2025 4:37 PM, Mike Leach wrote:
> Hi,
> 
> On Mon, 14 Jul 2025 at 07:31, Jie Gan <jie.gan@oss.qualcomm.com> wrote:
>>
>> Retrieving the helper device of the specific coresight device based on
>> its helper_subtype because a single coresight device may has multiple types
>> of the helper devices.
>>
>> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
>> ---
>>   drivers/hwtracing/coresight/coresight-core.c | 30 ++++++++++++++++++++
>>   drivers/hwtracing/coresight/coresight-priv.h |  2 ++
>>   2 files changed, 32 insertions(+)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
>> index 5297a5ff7921..76e10c36a8a1 100644
>> --- a/drivers/hwtracing/coresight/coresight-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>> @@ -580,6 +580,36 @@ struct coresight_device *coresight_get_sink(struct coresight_path *path)
>>   }
>>   EXPORT_SYMBOL_GPL(coresight_get_sink);
>>
>> +/**
>> + * coresight_get_helper: find the helper device of the assigned csdev.
>> + *
>> + * @csdev: The csdev the helper device is conntected to.
>> + * @type:  helper_subtype of the expected helper device.
>> + *
>> + * Retrieve the helper device for the specific csdev based on its
>> + * helper_subtype.
>> + *
>> + * Return: the helper's csdev upon success or NULL for fail.
>> + */
>> +struct coresight_device *coresight_get_helper(struct coresight_device *csdev,
>> +                                             int type)
>> +{
>> +       int i;
>> +       struct coresight_device *helper;
>> +
>> +       for (i = 0; i < csdev->pdata->nr_outconns; ++i) {
>> +               helper = csdev->pdata->out_conns[i]->dest_dev;
>> +               if (!helper || !coresight_is_helper(helper))
>> +                       continue;
>> +
> 
> Manipulating the connections list almost certainly requires some
> locking. See other functions in this file
> 

Thanks for pointing out. I will fix it in next version.

Thanks,
Jie

> Mike
> 
> 
>> +               if (helper->subtype.helper_subtype == type)
>> +                       return helper;
>> +       }
>> +
>> +       return NULL;
>> +}
>> +EXPORT_SYMBOL_GPL(coresight_get_helper);
>> +
>>   /**
>>    * coresight_get_port_helper: get the in-port number of the helper device
>>    * that is connected to the csdev.
>> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
>> index 07a5f03de81d..5b912eb60401 100644
>> --- a/drivers/hwtracing/coresight/coresight-priv.h
>> +++ b/drivers/hwtracing/coresight/coresight-priv.h
>> @@ -158,6 +158,8 @@ void coresight_path_assign_trace_id(struct coresight_path *path,
>>                                     enum cs_mode mode);
>>   int coresight_get_port_helper(struct coresight_device *csdev,
>>                                struct coresight_device *helper);
>> +struct coresight_device *coresight_get_helper(struct coresight_device *csdev,
>> +                                             int type);
>>
>>   #if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM3X)
>>   int etm_readl_cp14(u32 off, unsigned int *val);
>> --
>> 2.34.1
>>
> 
> 

Re: [PATCH v3 RESEND 08/10] coresight: tmc: add a switch buffer function for byte-cntr

[Mike Leach]

Hi,

This buffer swap code looks OK in principle. The ETR is stopped,
memory synced and set to be read.
See other comments inline.

On Mon, 14 Jul 2025 at 07:31, Jie Gan <jie.gan@oss.qualcomm.com> wrote:
>
> Switching the sysfs_buf when current buffer is full or the timeout is
> triggered and resets rrp and rwp registers after switched the buffer.
> Disable the ETR device if it cannot find an available buffer to switch.
>
> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
> ---
>  .../hwtracing/coresight/coresight-tmc-etr.c   | 52 +++++++++++++++++++
>  1 file changed, 52 insertions(+)
>
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index 2b73bd8074bb..3e3e1b5e78ca 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> @@ -1287,6 +1287,58 @@ static struct etr_buf *tmc_etr_get_sysfs_buffer(struct coresight_device *csdev)
>         return ret ? ERR_PTR(ret) : drvdata->sysfs_buf;
>  }
>
> +static bool tmc_byte_cntr_switch_buffer(struct tmc_drvdata *drvdata,
> +                                       struct ctcu_byte_cntr *byte_cntr_data)
> +{

This entire function should be in one of the byte_cntr source files,
not in the main etr files. Please keep byte cntr code separate as far
as possible

> +       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 we start the switching process */
> +       if (coresight_get_mode(drvdata->csdev) == CS_MODE_SYSFS)

Can this function be called if the mode is not CS_MODE_SYSFS?

> +               __tmc_etr_disable_hw(drvdata);
> +
> +       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;
> +               }
> +
> +               if (!found_free_buf && nd->is_free && nd->sysfs_buf != curr_buf) {
> +                       if (nd->reading)
> +                               continue;
> +
> +                       picked_node = nd;
> +                       found_free_buf = true;
> +               }
> +       }
> +
> +       if (found_free_buf) {
> +               curr_node->reading = true;
> +               curr_node->pos = 0;
> +               drvdata->reading_node = curr_node;
> +               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*/
> +               CS_UNLOCK(drvdata->base);
> +               tmc_write_rrp(drvdata, 0);
> +               tmc_write_rwp(drvdata, 0);

This cannot possibly be correct. RWP / RRP are pointers into the
system memory where the ETR stores data.

> +               CS_LOCK(drvdata->base);
> +               /* Restart the ETR when we find a free buffer */
> +               if (coresight_get_mode(drvdata->csdev) == CS_MODE_SYSFS)
> +                       __tmc_etr_enable_hw(drvdata);

What happens if the ETR is not restarted? Using __tmc_etr_disable_hw()
is correct for this use case, but if you do not restart then the extra
shutdown that would ordinarily happen in tmc_etr_disable_hw() does not
occur. How is this handled in the rest of the update?

> +       }
> +
> +       return found_free_buf;
> +}
> +
>  static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev)
>  {
>         int ret = 0;
> --
> 2.34.1
>

Regards

Mike

--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK

Re: [PATCH v3 RESEND 09/10] coresight: tmc: add read function for byte-cntr

[Mike Leach]

Hi,

On Mon, 14 Jul 2025 at 07:32, Jie Gan <jie.gan@oss.qualcomm.com> wrote:
>
> The byte-cntr read function always reads trace data from the deactivated
> and filled buffer which is already synced. The read function will fail
> when the ETR cannot find a available buffer to receive trace data.
>
> The read function terminates when the path is disabled or interrupted by a
> signal.
>
> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
> ---
>  .../hwtracing/coresight/coresight-tmc-core.c  | 31 ++++++-
>  .../hwtracing/coresight/coresight-tmc-etr.c   | 90 +++++++++++++++++++
>  drivers/hwtracing/coresight/coresight-tmc.h   |  3 +
>  3 files changed, 120 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c
> index 354faeeddbb2..3ab25adc4e4d 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-core.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c
> @@ -318,14 +318,18 @@ static int tmc_open(struct inode *inode, struct file *file)
>         return 0;
>  }
>
> -static ssize_t tmc_get_sysfs_trace(struct tmc_drvdata *drvdata, loff_t pos, size_t len,
> -                                  char **bufpp)
> +static ssize_t tmc_get_sysfs_trace(struct tmc_drvdata *drvdata,
> +                                  struct ctcu_byte_cntr *byte_cntr_data,
> +                                  loff_t pos, size_t len, char **bufpp)

Don't change "core" functionalilty to add in bytecntr parameters.

Use helper functions to have a pattern such as:

if (bytecntr_active())
    call_byte_cntr_fn()
else
    call_standard_fn()

>  {
>         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:
> +               if (byte_cntr_data && byte_cntr_data->thresh_val)
> +                       return tmc_byte_cntr_get_data(drvdata, byte_cntr_data, len, bufpp);
> +
>                 return tmc_etr_get_sysfs_trace(drvdata, pos, len, bufpp);
>         }
>
> @@ -339,7 +343,21 @@ static ssize_t tmc_read(struct file *file, char __user *data, size_t len,
>         ssize_t actual;
>         struct tmc_drvdata *drvdata = container_of(file->private_data,
>                                                    struct tmc_drvdata, miscdev);
> -       actual = tmc_get_sysfs_trace(drvdata, *ppos, len, &bufp);
> +       struct coresight_device *helper = coresight_get_helper(drvdata->csdev,
> +                                               CORESIGHT_DEV_SUBTYPE_HELPER_CTCU);
> +       struct ctcu_byte_cntr *byte_cntr_data = NULL;
> +       struct ctcu_drvdata *ctcu_drvdata = NULL;
> +       int port;
> +
> +       if (helper) {
> +               port = coresight_get_port_helper(drvdata->csdev, helper);
> +               if (port >= 0) {
> +                       ctcu_drvdata = dev_get_drvdata(helper->dev.parent);
> +                       byte_cntr_data = &ctcu_drvdata->byte_cntr_data[port];
> +               }
> +       }
> +
> +       actual = tmc_get_sysfs_trace(drvdata, byte_cntr_data, *ppos, len, &bufp);
>         if (actual <= 0)
>                 return 0;
>
> @@ -349,7 +367,12 @@ static ssize_t tmc_read(struct file *file, char __user *data, size_t len,
>                 return -EFAULT;
>         }
>
> -       *ppos += actual;
> +       if (byte_cntr_data && byte_cntr_data->thresh_val) {
> +               byte_cntr_data->total_size += actual;
> +               drvdata->reading_node->pos += actual;
> +       } else
> +               *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 3e3e1b5e78ca..174411e76047 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> @@ -1163,6 +1163,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->reading_node)
> +               etr_buf = drvdata->reading_node->sysfs_buf;
> +
>         if (pos + actual > etr_buf->len)
>                 actual = etr_buf->len - pos;
>         if (actual <= 0)
> @@ -1339,6 +1343,92 @@ static bool tmc_byte_cntr_switch_buffer(struct tmc_drvdata *drvdata,
>         return found_free_buf;
>  }
>
> +/*
> + * tmc_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.
> + */

This entire function should be moved to one of the byte-cntr source files.

> +ssize_t tmc_byte_cntr_get_data(struct tmc_drvdata *drvdata,
> +                              struct ctcu_byte_cntr *byte_cntr_data,
> +                              size_t len, char **bufpp)
> +{
> +       size_t thresh_val = byte_cntr_data->thresh_val;
> +       atomic_t *irq_cnt = &byte_cntr_data->irq_cnt;
> +       struct etr_buf *sysfs_buf = drvdata->sysfs_buf;
> +       struct device *dev = &drvdata->csdev->dev;
> +       struct etr_buf_node *nd, *next;
> +       ssize_t size = sysfs_buf->size;
> +       ssize_t actual;
> +       loff_t pos;
> +       int ret;
> +
> +wait_buffer:
> +       if (!byte_cntr_data->reading_buf) {
> +               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 (!tmc_byte_cntr_switch_buffer(drvdata, byte_cntr_data)) {
> +                               dev_err(dev, "Switch buffer failed for byte-cntr\n");
> +                               return -EINVAL;
> +                       }
> +
> +                       byte_cntr_data->reading_buf = true;
> +               } else {
> +                       if (!drvdata->reading_node) {
> +                               list_for_each_entry_safe(nd, next, &drvdata->etr_buf_list, node) {
> +                                       if (nd->sysfs_buf == sysfs_buf) {
> +                                               nd->pos = 0;
> +                                               drvdata->reading_node = nd;
> +                                               break;
> +                                       }
> +                               }
> +                       }
> +
> +                       pos = drvdata->reading_node->pos;
> +                       actual = tmc_etr_get_sysfs_trace(drvdata, pos, len, bufpp);
> +                       if (actual > 0)
> +                               return actual;
> +
> +                       drvdata->reading_node = NULL;
> +
> +                       /* Exit byte-cntr reading */
> +                       return -EINVAL;
> +               }
> +       }
> +
> +       /* Check the status of current etr_buf*/
> +       if ((atomic_read(irq_cnt) + 1) * thresh_val >= size)
> +               /*
> +                * Unlikely to find a free buffer to switch, so just disable
> +                * the ETR for a while.
> +                */
> +               if (!tmc_byte_cntr_switch_buffer(drvdata, byte_cntr_data))
> +                       dev_info(dev, "No available buffer to store data, disable ETR\n");
> +
> +       pos = drvdata->reading_node->pos;
> +       actual = tmc_etr_get_sysfs_trace(drvdata, pos, len, bufpp);
> +       if (actual == 0) {
> +               /* Reading work for marked buffer has finished, reset flags */
> +               drvdata->reading_node->reading = false;
> +               byte_cntr_data->reading_buf = false;
> +               drvdata->reading_node = NULL;
> +
> +               /* Nothing in the buffer, wait for next buffer to be filled */
> +               goto wait_buffer;
> +       }
> +
> +       return actual;
> +}
> +
>  static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev)
>  {
>         int ret = 0;
> diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
> index 1dbba0bc50a3..4136ec5ecaf7 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc.h
> +++ b/drivers/hwtracing/coresight/coresight-tmc.h
> @@ -364,6 +364,9 @@ int tmc_read_prepare_byte_cntr(struct tmc_drvdata *drvdata,
>                                struct ctcu_byte_cntr *byte_cntr_data);
>  int tmc_read_unprepare_byte_cntr(struct tmc_drvdata *drvdata,
>                                  struct ctcu_byte_cntr *byte_cntr_data);

Declare this in a byte_cntr header file, not here.

> +ssize_t tmc_byte_cntr_get_data(struct tmc_drvdata *drvdata,
> +                              struct ctcu_byte_cntr *byte_cntr_data,
> +                              size_t len, char **bufpp);
>
>  #define TMC_REG_PAIR(name, lo_off, hi_off)                             \
>  static inline u64                                                      \
> --
> 2.34.1
>


-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK

Re: [PATCH v3 RESEND 00/10] coresight: ctcu: Enable byte-cntr function for TMC ETR

[Mike Leach]

Hi

I have had a look at a few of the patches. The buffer swap mechanism
appears to be good.

However there is a lot of byte-ctr code in the "core" tmc / etr source
files. As much of this code as possible needs to be moved to the
byte-cntr specifc source.

I suggest having a helper function such as qcom_byte_ctr_in_use() to
call from the core code, and if true then call back into the byte-cntr
specific code to do the specialist functionality.

One other possibility is to have a flag / enum in the tmc->drvdata
structure to indicate a variant. - e.g. TMC_STD, TMC_QCOM_BYTE_CTR,
set at initialisation stage to remove the need for checking the device
tree every call.

Regards

Mike

On Mon, 14 Jul 2025 at 07:31, Jie Gan <jie.gan@oss.qualcomm.com> wrote:
>
> The byte-cntr function provided by the CTCU device is used to count the
> trace data entering the ETR. An interrupt is triggered if the data size
> exceeds the threshold set in the BYTECNTRVAL register. The interrupt
> handler counts the number of triggered interruptions.
>
> Based on this concept, the irq_cnt can be used to determine whether
> the etr_buf is full. The ETR device will be disabled when the active
> etr_buf is nearly full or a timeout occurs. The nearly full buffer will
> be switched to background after synced. A new buffer will be picked from
> the etr_buf_list, then restart the ETR device.
>
> The byte-cntr reading functions can access data from the synced and
> deactivated buffer, transferring trace data from the etr_buf to userspace
> without stopping the ETR device.
>
> The byte-cntr read operation has integrated with the file node tmc_etr,
> for example:
> /dev/tmc_etr0
> /dev/tmc_etr1
>
> There are two scenarios for the tmc_etr file node with byte-cntr function:
> 1. BYTECNTRVAL register is configured and byte-cntr is enabled -> byte-cntr read
> 2. BYTECNTRVAL register is reset or byte-cntr is disabled -> original behavior
>
> Shell commands to enable byte-cntr reading for etr0:
> echo 0x10000 > /sys/bus/coresight/devices/ctcu0/irq_val
> 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_val
>
> 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/
>
> Jie Gan (10):
>   coresight: core: Refactoring ctcu_get_active_port and make it generic
>   coresight: core: add a new API to retrieve the helper device
>   dt-bindings: arm: add an interrupt property for Coresight CTCU
>   coresight: ctcu: enable byte-cntr for TMC ETR devices
>   coresight: tmc: add etr_buf_list to store allocated etr_buf
>   coresight: tmc: add create/delete functions for etr_buf_node
>   coresight: tmc: add prepare/unprepare functions for byte-cntr
>   coresight: tmc: add a switch buffer function for byte-cntr
>   coresight: tmc: add read function for byte-cntr
>   arm64: dts: qcom: sa8775p: Add interrupts to CTCU device
>
>  .../testing/sysfs-bus-coresight-devices-ctcu  |   5 +
>  .../bindings/arm/qcom,coresight-ctcu.yaml     |  17 ++
>  arch/arm64/boot/dts/qcom/sa8775p.dtsi         |   5 +
>  drivers/hwtracing/coresight/Makefile          |   2 +-
>  drivers/hwtracing/coresight/coresight-core.c  |  54 ++++
>  .../coresight/coresight-ctcu-byte-cntr.c      | 102 +++++++
>  .../hwtracing/coresight/coresight-ctcu-core.c | 113 ++++++--
>  drivers/hwtracing/coresight/coresight-ctcu.h  |  49 +++-
>  drivers/hwtracing/coresight/coresight-priv.h  |   4 +
>  .../hwtracing/coresight/coresight-tmc-core.c  |  70 ++++-
>  .../hwtracing/coresight/coresight-tmc-etr.c   | 270 ++++++++++++++++++
>  drivers/hwtracing/coresight/coresight-tmc.h   |  29 ++
>  12 files changed, 688 insertions(+), 32 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight-devices-ctcu
>  create mode 100644 drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c
>
> --
> 2.34.1
>


-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK

Re: [PATCH v3 RESEND 04/10] coresight: ctcu: enable byte-cntr for TMC ETR devices

[Mike Leach]

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.

Regards

Mike

On Mon, 14 Jul 2025 at 07:31, Jie Gan <jie.gan@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@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@quicinc.com>; Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>; Jie Gan <jie.gan@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
>


-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK

Re: [PATCH v3 RESEND 04/10] coresight: ctcu: enable byte-cntr for TMC ETR devices

[Jie Gan]

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@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@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@quicinc.com>; Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>; Jie Gan <jie.gan@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
>>
> 
> 

Re: [PATCH v3 RESEND 00/10] coresight: ctcu: Enable byte-cntr function for TMC ETR

[Jie Gan]

On 7/22/2025 11:09 PM, Mike Leach wrote:
> Hi
> 

Hi Mike

Thanks for your comments.

> I have had a look at a few of the patches. The buffer swap mechanism
> appears to be good.
> 
> However there is a lot of byte-ctr code in the "core" tmc / etr source
> files. As much of this code as possible needs to be moved to the
> byte-cntr specifc source.
> 

I will try move byte-cntr related codes to the ctcu-byte-cntr specific 
source file.

> I suggest having a helper function such as qcom_byte_ctr_in_use() to
> call from the core code, and if true then call back into the byte-cntr
> specific code to do the specialist functionality.

Sounds a good idea, call the prepare/read function regards to whether 
there is a valid helper callback defined. I will try this solution in 
next version.

I have a minor question about the solution. We expect enable/disable the 
byte-cntr function at anytime. So when we jump to the byte-cntr 
function, we have to check whether the byte-cntr function is enabled or 
not first, jump back to the standard workflow if not, am right?

> 
> One other possibility is to have a flag / enum in the tmc->drvdata
> structure to indicate a variant. - e.g. TMC_STD, TMC_QCOM_BYTE_CTR,
> set at initialisation stage to remove the need for checking the device
> tree every call.
> 

Also will check once, see which solution is better.

Thanks,
Jie

> Regards
> 
> Mike
> 
> On Mon, 14 Jul 2025 at 07:31, Jie Gan <jie.gan@oss.qualcomm.com> wrote:
>>
>> The byte-cntr function provided by the CTCU device is used to count the
>> trace data entering the ETR. An interrupt is triggered if the data size
>> exceeds the threshold set in the BYTECNTRVAL register. The interrupt
>> handler counts the number of triggered interruptions.
>>
>> Based on this concept, the irq_cnt can be used to determine whether
>> the etr_buf is full. The ETR device will be disabled when the active
>> etr_buf is nearly full or a timeout occurs. The nearly full buffer will
>> be switched to background after synced. A new buffer will be picked from
>> the etr_buf_list, then restart the ETR device.
>>
>> The byte-cntr reading functions can access data from the synced and
>> deactivated buffer, transferring trace data from the etr_buf to userspace
>> without stopping the ETR device.
>>
>> The byte-cntr read operation has integrated with the file node tmc_etr,
>> for example:
>> /dev/tmc_etr0
>> /dev/tmc_etr1
>>
>> There are two scenarios for the tmc_etr file node with byte-cntr function:
>> 1. BYTECNTRVAL register is configured and byte-cntr is enabled -> byte-cntr read
>> 2. BYTECNTRVAL register is reset or byte-cntr is disabled -> original behavior
>>
>> Shell commands to enable byte-cntr reading for etr0:
>> echo 0x10000 > /sys/bus/coresight/devices/ctcu0/irq_val
>> 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_val
>>
>> 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/
>>
>> Jie Gan (10):
>>    coresight: core: Refactoring ctcu_get_active_port and make it generic
>>    coresight: core: add a new API to retrieve the helper device
>>    dt-bindings: arm: add an interrupt property for Coresight CTCU
>>    coresight: ctcu: enable byte-cntr for TMC ETR devices
>>    coresight: tmc: add etr_buf_list to store allocated etr_buf
>>    coresight: tmc: add create/delete functions for etr_buf_node
>>    coresight: tmc: add prepare/unprepare functions for byte-cntr
>>    coresight: tmc: add a switch buffer function for byte-cntr
>>    coresight: tmc: add read function for byte-cntr
>>    arm64: dts: qcom: sa8775p: Add interrupts to CTCU device
>>
>>   .../testing/sysfs-bus-coresight-devices-ctcu  |   5 +
>>   .../bindings/arm/qcom,coresight-ctcu.yaml     |  17 ++
>>   arch/arm64/boot/dts/qcom/sa8775p.dtsi         |   5 +
>>   drivers/hwtracing/coresight/Makefile          |   2 +-
>>   drivers/hwtracing/coresight/coresight-core.c  |  54 ++++
>>   .../coresight/coresight-ctcu-byte-cntr.c      | 102 +++++++
>>   .../hwtracing/coresight/coresight-ctcu-core.c | 113 ++++++--
>>   drivers/hwtracing/coresight/coresight-ctcu.h  |  49 +++-
>>   drivers/hwtracing/coresight/coresight-priv.h  |   4 +
>>   .../hwtracing/coresight/coresight-tmc-core.c  |  70 ++++-
>>   .../hwtracing/coresight/coresight-tmc-etr.c   | 270 ++++++++++++++++++
>>   drivers/hwtracing/coresight/coresight-tmc.h   |  29 ++
>>   12 files changed, 688 insertions(+), 32 deletions(-)
>>   create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight-devices-ctcu
>>   create mode 100644 drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c
>>
>> --
>> 2.34.1
>>
> 
> 

Re: [PATCH v3 RESEND 09/10] coresight: tmc: add read function for byte-cntr

[Jie Gan]

On 7/22/2025 11:01 PM, Mike Leach wrote:
> Hi,
> 
> On Mon, 14 Jul 2025 at 07:32, Jie Gan <jie.gan@oss.qualcomm.com> wrote:
>>
>> The byte-cntr read function always reads trace data from the deactivated
>> and filled buffer which is already synced. The read function will fail
>> when the ETR cannot find a available buffer to receive trace data.
>>
>> The read function terminates when the path is disabled or interrupted by a
>> signal.
>>
>> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
>> ---
>>   .../hwtracing/coresight/coresight-tmc-core.c  | 31 ++++++-
>>   .../hwtracing/coresight/coresight-tmc-etr.c   | 90 +++++++++++++++++++
>>   drivers/hwtracing/coresight/coresight-tmc.h   |  3 +
>>   3 files changed, 120 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c
>> index 354faeeddbb2..3ab25adc4e4d 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c
>> @@ -318,14 +318,18 @@ static int tmc_open(struct inode *inode, struct file *file)
>>          return 0;
>>   }
>>
>> -static ssize_t tmc_get_sysfs_trace(struct tmc_drvdata *drvdata, loff_t pos, size_t len,
>> -                                  char **bufpp)
>> +static ssize_t tmc_get_sysfs_trace(struct tmc_drvdata *drvdata,
>> +                                  struct ctcu_byte_cntr *byte_cntr_data,
>> +                                  loff_t pos, size_t len, char **bufpp)
> 
> Don't change "core" functionalilty to add in bytecntr parameters.
> 
> Use helper functions to have a pattern such as:
> 
> if (bytecntr_active())
>      call_byte_cntr_fn()
> else
>      call_standard_fn()

got it. Will fix in next version.

Thanks,
Jie

> 
>>   {
>>          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:
>> +               if (byte_cntr_data && byte_cntr_data->thresh_val)
>> +                       return tmc_byte_cntr_get_data(drvdata, byte_cntr_data, len, bufpp);
>> +
>>                  return tmc_etr_get_sysfs_trace(drvdata, pos, len, bufpp);
>>          }
>>
>> @@ -339,7 +343,21 @@ static ssize_t tmc_read(struct file *file, char __user *data, size_t len,
>>          ssize_t actual;
>>          struct tmc_drvdata *drvdata = container_of(file->private_data,
>>                                                     struct tmc_drvdata, miscdev);
>> -       actual = tmc_get_sysfs_trace(drvdata, *ppos, len, &bufp);
>> +       struct coresight_device *helper = coresight_get_helper(drvdata->csdev,
>> +                                               CORESIGHT_DEV_SUBTYPE_HELPER_CTCU);
>> +       struct ctcu_byte_cntr *byte_cntr_data = NULL;
>> +       struct ctcu_drvdata *ctcu_drvdata = NULL;
>> +       int port;
>> +
>> +       if (helper) {
>> +               port = coresight_get_port_helper(drvdata->csdev, helper);
>> +               if (port >= 0) {
>> +                       ctcu_drvdata = dev_get_drvdata(helper->dev.parent);
>> +                       byte_cntr_data = &ctcu_drvdata->byte_cntr_data[port];
>> +               }
>> +       }
>> +
>> +       actual = tmc_get_sysfs_trace(drvdata, byte_cntr_data, *ppos, len, &bufp);
>>          if (actual <= 0)
>>                  return 0;
>>
>> @@ -349,7 +367,12 @@ static ssize_t tmc_read(struct file *file, char __user *data, size_t len,
>>                  return -EFAULT;
>>          }
>>
>> -       *ppos += actual;
>> +       if (byte_cntr_data && byte_cntr_data->thresh_val) {
>> +               byte_cntr_data->total_size += actual;
>> +               drvdata->reading_node->pos += actual;
>> +       } else
>> +               *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 3e3e1b5e78ca..174411e76047 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> @@ -1163,6 +1163,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->reading_node)
>> +               etr_buf = drvdata->reading_node->sysfs_buf;
>> +
>>          if (pos + actual > etr_buf->len)
>>                  actual = etr_buf->len - pos;
>>          if (actual <= 0)
>> @@ -1339,6 +1343,92 @@ static bool tmc_byte_cntr_switch_buffer(struct tmc_drvdata *drvdata,
>>          return found_free_buf;
>>   }
>>
>> +/*
>> + * tmc_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.
>> + */
> 
> This entire function should be moved to one of the byte-cntr source files.
> 
>> +ssize_t tmc_byte_cntr_get_data(struct tmc_drvdata *drvdata,
>> +                              struct ctcu_byte_cntr *byte_cntr_data,
>> +                              size_t len, char **bufpp)
>> +{
>> +       size_t thresh_val = byte_cntr_data->thresh_val;
>> +       atomic_t *irq_cnt = &byte_cntr_data->irq_cnt;
>> +       struct etr_buf *sysfs_buf = drvdata->sysfs_buf;
>> +       struct device *dev = &drvdata->csdev->dev;
>> +       struct etr_buf_node *nd, *next;
>> +       ssize_t size = sysfs_buf->size;
>> +       ssize_t actual;
>> +       loff_t pos;
>> +       int ret;
>> +
>> +wait_buffer:
>> +       if (!byte_cntr_data->reading_buf) {
>> +               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 (!tmc_byte_cntr_switch_buffer(drvdata, byte_cntr_data)) {
>> +                               dev_err(dev, "Switch buffer failed for byte-cntr\n");
>> +                               return -EINVAL;
>> +                       }
>> +
>> +                       byte_cntr_data->reading_buf = true;
>> +               } else {
>> +                       if (!drvdata->reading_node) {
>> +                               list_for_each_entry_safe(nd, next, &drvdata->etr_buf_list, node) {
>> +                                       if (nd->sysfs_buf == sysfs_buf) {
>> +                                               nd->pos = 0;
>> +                                               drvdata->reading_node = nd;
>> +                                               break;
>> +                                       }
>> +                               }
>> +                       }
>> +
>> +                       pos = drvdata->reading_node->pos;
>> +                       actual = tmc_etr_get_sysfs_trace(drvdata, pos, len, bufpp);
>> +                       if (actual > 0)
>> +                               return actual;
>> +
>> +                       drvdata->reading_node = NULL;
>> +
>> +                       /* Exit byte-cntr reading */
>> +                       return -EINVAL;
>> +               }
>> +       }
>> +
>> +       /* Check the status of current etr_buf*/
>> +       if ((atomic_read(irq_cnt) + 1) * thresh_val >= size)
>> +               /*
>> +                * Unlikely to find a free buffer to switch, so just disable
>> +                * the ETR for a while.
>> +                */
>> +               if (!tmc_byte_cntr_switch_buffer(drvdata, byte_cntr_data))
>> +                       dev_info(dev, "No available buffer to store data, disable ETR\n");
>> +
>> +       pos = drvdata->reading_node->pos;
>> +       actual = tmc_etr_get_sysfs_trace(drvdata, pos, len, bufpp);
>> +       if (actual == 0) {
>> +               /* Reading work for marked buffer has finished, reset flags */
>> +               drvdata->reading_node->reading = false;
>> +               byte_cntr_data->reading_buf = false;
>> +               drvdata->reading_node = NULL;
>> +
>> +               /* Nothing in the buffer, wait for next buffer to be filled */
>> +               goto wait_buffer;
>> +       }
>> +
>> +       return actual;
>> +}
>> +
>>   static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev)
>>   {
>>          int ret = 0;
>> diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
>> index 1dbba0bc50a3..4136ec5ecaf7 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc.h
>> +++ b/drivers/hwtracing/coresight/coresight-tmc.h
>> @@ -364,6 +364,9 @@ int tmc_read_prepare_byte_cntr(struct tmc_drvdata *drvdata,
>>                                 struct ctcu_byte_cntr *byte_cntr_data);
>>   int tmc_read_unprepare_byte_cntr(struct tmc_drvdata *drvdata,
>>                                   struct ctcu_byte_cntr *byte_cntr_data);
> 
> Declare this in a byte_cntr header file, not here.

I will add it to,for example, ctcu_byte_cntr_read_ops->read.
So I think I dont need define it in any header file in future.
Will remove it.

Thanks,
Jie

> 
>> +ssize_t tmc_byte_cntr_get_data(struct tmc_drvdata *drvdata,
>> +                              struct ctcu_byte_cntr *byte_cntr_data,
>> +                              size_t len, char **bufpp);
>>
>>   #define TMC_REG_PAIR(name, lo_off, hi_off)                             \
>>   static inline u64                                                      \
>> --
>> 2.34.1
>>
> 
> 

Re: [PATCH v3 RESEND 08/10] coresight: tmc: add a switch buffer function for byte-cntr

[Jie Gan]

On 7/22/2025 10:09 PM, Mike Leach wrote:
> Hi,
> 
> This buffer swap code looks OK in principle. The ETR is stopped,
> memory synced and set to be read.
> See other comments inline.
> 
> On Mon, 14 Jul 2025 at 07:31, Jie Gan <jie.gan@oss.qualcomm.com> wrote:
>>
>> Switching the sysfs_buf when current buffer is full or the timeout is
>> triggered and resets rrp and rwp registers after switched the buffer.
>> Disable the ETR device if it cannot find an available buffer to switch.
>>
>> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
>> ---
>>   .../hwtracing/coresight/coresight-tmc-etr.c   | 52 +++++++++++++++++++
>>   1 file changed, 52 insertions(+)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> index 2b73bd8074bb..3e3e1b5e78ca 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> @@ -1287,6 +1287,58 @@ static struct etr_buf *tmc_etr_get_sysfs_buffer(struct coresight_device *csdev)
>>          return ret ? ERR_PTR(ret) : drvdata->sysfs_buf;
>>   }
>>
>> +static bool tmc_byte_cntr_switch_buffer(struct tmc_drvdata *drvdata,
>> +                                       struct ctcu_byte_cntr *byte_cntr_data)
>> +{
> 
> This entire function should be in one of the byte_cntr source files,
> not in the main etr files. Please keep byte cntr code separate as far
> as possible
> 
>> +       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 we start the switching process */
>> +       if (coresight_get_mode(drvdata->csdev) == CS_MODE_SYSFS)
> 
> Can this function be called if the mode is not CS_MODE_SYSFS?

Should be ok with called if the the is DISABLED.
I will check the condition.

> 
>> +               __tmc_etr_disable_hw(drvdata);
>> +
>> +       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;
>> +               }
>> +
>> +               if (!found_free_buf && nd->is_free && nd->sysfs_buf != curr_buf) {
>> +                       if (nd->reading)
>> +                               continue;
>> +
>> +                       picked_node = nd;
>> +                       found_free_buf = true;
>> +               }
>> +       }
>> +
>> +       if (found_free_buf) {
>> +               curr_node->reading = true;
>> +               curr_node->pos = 0;
>> +               drvdata->reading_node = curr_node;
>> +               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*/
>> +               CS_UNLOCK(drvdata->base);
>> +               tmc_write_rrp(drvdata, 0);
>> +               tmc_write_rwp(drvdata, 0);
> 
> This cannot possibly be correct. RWP / RRP are pointers into the
> system memory where the ETR stores data.

It should be sysfs_buf->hwaddr here.
I made a mistake.

will fix it.

> 
>> +               CS_LOCK(drvdata->base);
>> +               /* Restart the ETR when we find a free buffer */
>> +               if (coresight_get_mode(drvdata->csdev) == CS_MODE_SYSFS)
>> +                       __tmc_etr_enable_hw(drvdata);
> 
> What happens if the ETR is not restarted? Using __tmc_etr_disable_hw()
> is correct for this use case, but if you do not restart then the extra
> shutdown that would ordinarily happen in tmc_etr_disable_hw() does not
> occur. How is this handled in the rest of the update?

Yes, there is a problem here. It's incorrectly put a strict condition here.

I will check the logic here and fix in next version.

Thanks,
Jie

> 
>> +       }
>> +
>> +       return found_free_buf;
>> +}
>> +
>>   static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev)
>>   {
>>          int ret = 0;
>> --
>> 2.34.1
>>
> 
> Regards
> 
> Mike
> 
> --
> Mike Leach
> Principal Engineer, ARM Ltd.
> Manchester Design Centre. UK

Re: [PATCH v3 RESEND 08/10] coresight: tmc: add a switch buffer function for byte-cntr

[Jie Gan]

On 7/22/2025 10:09 PM, Mike Leach wrote:
> Hi,
> 
> This buffer swap code looks OK in principle. The ETR is stopped,
> memory synced and set to be read.
> See other comments inline.
> 
> On Mon, 14 Jul 2025 at 07:31, Jie Gan <jie.gan@oss.qualcomm.com> wrote:
>>
>> Switching the sysfs_buf when current buffer is full or the timeout is
>> triggered and resets rrp and rwp registers after switched the buffer.
>> Disable the ETR device if it cannot find an available buffer to switch.
>>
>> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
>> ---
>>   .../hwtracing/coresight/coresight-tmc-etr.c   | 52 +++++++++++++++++++
>>   1 file changed, 52 insertions(+)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> index 2b73bd8074bb..3e3e1b5e78ca 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> @@ -1287,6 +1287,58 @@ static struct etr_buf *tmc_etr_get_sysfs_buffer(struct coresight_device *csdev)
>>          return ret ? ERR_PTR(ret) : drvdata->sysfs_buf;
>>   }
>>
>> +static bool tmc_byte_cntr_switch_buffer(struct tmc_drvdata *drvdata,
>> +                                       struct ctcu_byte_cntr *byte_cntr_data)
>> +{
> 
> This entire function should be in one of the byte_cntr source files,
> not in the main etr files. Please keep byte cntr code separate as far
> as possible
> 
>> +       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 we start the switching process */
>> +       if (coresight_get_mode(drvdata->csdev) == CS_MODE_SYSFS)
> 
> Can this function be called if the mode is not CS_MODE_SYSFS?
> 
>> +               __tmc_etr_disable_hw(drvdata);

Hi Mike,

Since I need move the whole function to byte-cntr source file, is that 
acceptable to export __tmc_etr_disable_hw from tmc-etr?

Same with __tmc_etr_enable_hw.

Thanks,
Jie

>> +
>> +       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;
>> +               }
>> +
>> +               if (!found_free_buf && nd->is_free && nd->sysfs_buf != curr_buf) {
>> +                       if (nd->reading)
>> +                               continue;
>> +
>> +                       picked_node = nd;
>> +                       found_free_buf = true;
>> +               }
>> +       }
>> +
>> +       if (found_free_buf) {
>> +               curr_node->reading = true;
>> +               curr_node->pos = 0;
>> +               drvdata->reading_node = curr_node;
>> +               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*/
>> +               CS_UNLOCK(drvdata->base);
>> +               tmc_write_rrp(drvdata, 0);
>> +               tmc_write_rwp(drvdata, 0);
> 
> This cannot possibly be correct. RWP / RRP are pointers into the
> system memory where the ETR stores data.
> 
>> +               CS_LOCK(drvdata->base);
>> +               /* Restart the ETR when we find a free buffer */
>> +               if (coresight_get_mode(drvdata->csdev) == CS_MODE_SYSFS)
>> +                       __tmc_etr_enable_hw(drvdata);
> 
> What happens if the ETR is not restarted? Using __tmc_etr_disable_hw()
> is correct for this use case, but if you do not restart then the extra
> shutdown that would ordinarily happen in tmc_etr_disable_hw() does not
> occur. How is this handled in the rest of the update?
> 
>> +       }
>> +
>> +       return found_free_buf;
>> +}
>> +
>>   static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev)
>>   {
>>          int ret = 0;
>> --
>> 2.34.1
>>
> 
> Regards
> 
> Mike
> 
> --
> Mike Leach
> Principal Engineer, ARM Ltd.
> Manchester Design Centre. UK