Re: [PATCH v8 4/5] Coresight: Add Coresight TMC Control Unit driver
From: Jie Gan
Date: Mon Jan 13 2025 - 20:51:29 EST
On 1/13/2025 8:05 PM, James Clark wrote:
On 26/12/2024 1:10 am, Jie Gan wrote:
The Coresight TMC Control Unit hosts miscellaneous configuration
registers
which control various features related to TMC ETR sink.
Based on the trace ID, which is programmed in the related CTCU ATID
register of a specific ETR, trace data with that trace ID gets into
the ETR buffer, while other trace data gets dropped.
Enabling source device sets one bit of the ATID register based on
source device's trace ID.
Disabling source device resets the bit according to the source
device's trace ID.
Signed-off-by: Jie Gan <quic_jiegan@xxxxxxxxxxx>
---
drivers/hwtracing/coresight/Kconfig | 8 +
drivers/hwtracing/coresight/Makefile | 1 +
drivers/hwtracing/coresight/coresight-ctcu.c | 273 +++++++++++++++++++
drivers/hwtracing/coresight/coresight-ctcu.h | 21 ++
include/linux/coresight.h | 3 +-
5 files changed, 305 insertions(+), 1 deletion(-)
create mode 100644 drivers/hwtracing/coresight/coresight-ctcu.c
create mode 100644 drivers/hwtracing/coresight/coresight-ctcu.h
diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/
coresight/Kconfig
index 06f0a7594169..152eab0b9b2a 100644
--- a/drivers/hwtracing/coresight/Kconfig
+++ b/drivers/hwtracing/coresight/Kconfig
@@ -133,6 +133,14 @@ config CORESIGHT_STM
To compile this driver as a module, choose M here: the
module will be called coresight-stm.
+config CORESIGHT_CTCU
+ tristate "CoreSight TMC Control Unit driver"
+ help
+ This driver provides support for CoreSight TMC Control Unit
+ that hosts miscellaneous configuration registers. This is
+ primarily used for controlling the behaviors of the TMC
+ ETR device.
+
config CORESIGHT_CPU_DEBUG
tristate "CoreSight CPU Debug driver"
depends on ARM || ARM64
diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/
coresight/Makefile
index 4ba478211b31..1b7869910a12 100644
--- a/drivers/hwtracing/coresight/Makefile
+++ b/drivers/hwtracing/coresight/Makefile
@@ -51,3 +51,4 @@ coresight-cti-y := coresight-cti-core.o
coresight-cti-platform.o \
coresight-cti-sysfs.o
obj-$(CONFIG_ULTRASOC_SMB) += ultrasoc-smb.o
obj-$(CONFIG_CORESIGHT_DUMMY) += coresight-dummy.o
+obj-$(CONFIG_CORESIGHT_CTCU) += coresight-ctcu.o
diff --git a/drivers/hwtracing/coresight/coresight-ctcu.c b/drivers/
hwtracing/coresight/coresight-ctcu.c
new file mode 100644
index 000000000000..7650dbe9a41e
--- /dev/null
+++ b/drivers/hwtracing/coresight/coresight-ctcu.c
@@ -0,0 +1,273 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights
reserved.
+ */
+
+#include <linux/clk.h>
+#include <linux/coresight.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#include "coresight-ctcu.h"
+#include "coresight-priv.h"
+#include "coresight-trace-id.h"
+
+DEFINE_CORESIGHT_DEVLIST(ctcu_devs, "ctcu");
+
+#define ctcu_writel(drvdata, val, offset) __raw_writel((val),
drvdata->base + offset)
+#define ctcu_readl(drvdata, offset) __raw_readl(drvdata->base
+ offset)
+
+/* The TMC Coresight Control Unit uses four ATID registers to control
the data filter function based
+ * on the trace ID for each TMC ETR sink. The length of each ATID
register is 32 bits. Therefore,
+ * the ETR has a related field in CTCU that is 128 bits long. Each
trace ID is represented by one
+ * bit in that filed.
+ * e.g. ETR0ATID0 layout, set bit 5 for traceid 5
+ * bit5
+ * ------------------------------------------------------
+ * | |28| |24| |20| |16| |12| |8| 1|4| |0|
+ * ------------------------------------------------------
+ *
+ * e.g. ETR0:
+ * 127 0 from ATID_offset for ETR0ATID0
+ * -------------------------
+ * |ATID3|ATID2|ATID1|ATID0|
+ *
+ */
+#define CTCU_ATID_REG_OFFSET(traceid, atid_offset) \
+ ((traceid / 32) * 4 + atid_offset)
+
+#define CTCU_ATID_REG_BIT(traceid) (traceid % 32)
+#define CTCU_ATID_REG_SIZE 0x10
+
+struct ctcu_atid_config {
+ const uint32_t atid_offset;
+ const uint32_t port_num;
+};
+
+struct ctcu_config {
+ const struct ctcu_atid_config *atid_config;
+ int num_atid_config;
+};
+
+static const struct ctcu_atid_config sa8775p_atid_cfgs[] = {
+ {0xf8, 0},
+ {0x108, 1},
+};
+
+static const struct ctcu_config sa8775p_cfgs = {
+ .atid_config = sa8775p_atid_cfgs,
+ .num_atid_config = ARRAY_SIZE(sa8775p_atid_cfgs),
+};
+
+/*
+ * __ctcu_set_etr_traceid: Set bit in the ATID register based on
trace ID when enable is true.
+ * Reset the bit of the ATID register based on trace ID when enable
is false.
+ *
+ * @csdev: coresight_device struct related to the device
+ * @traceid: trace ID of the source tracer.
+ * @enable: True for set bit and false for reset bit.
+ *
+ * Returns 0 indicates success. Non-zero result means failure.
+ */
+static int __ctcu_set_etr_traceid(struct coresight_device *csdev,
+ u8 traceid,
+ int port_num,
+ bool enable)
+{
+ uint32_t atid_offset, reg_offset, val;
+ struct ctcu_drvdata *drvdata;
+ int bit;
+
+ if (!IS_VALID_CS_TRACE_ID(traceid))
+ return -EINVAL;
Minor point, but this was already done in the calling function.
Thanks for comment. Totally agree with you, it's redundant codes here.
I will remove it in next version.
+
+ drvdata = dev_get_drvdata(csdev->dev.parent);
+ if (IS_ERR_OR_NULL(drvdata))
+ return -EINVAL;
+
+ atid_offset = drvdata->atid_offset[port_num];
+ if (atid_offset == 0)
+ return -EINVAL;
+
+ guard(raw_spinlock_irqsave)(&drvdata->spin_lock);
+ CS_UNLOCK(drvdata->base);
+
+ bit = CTCU_ATID_REG_BIT(traceid);
+ reg_offset = CTCU_ATID_REG_OFFSET(traceid, atid_offset);
The locks only need to be around the read/write below. bit and
reg_offset are all local and shouldn't be affected. Doesn't really make
a difference but makes the code a bit more readable.
Yes, agree with you. It makes sense and is easier to read. Will move the
CS_UNLOCK to the proper position.
+ if (reg_offset - atid_offset > CTCU_ATID_REG_SIZE) {
+ CS_LOCK(drvdata);
+ return -EINVAL;
+ }
+
+ val = ctcu_readl(drvdata, reg_offset);
+ if (enable)
+ val = val | BIT(bit);
+ else
+ val = val & ~BIT(bit);
+
+ ctcu_writel(drvdata, val, reg_offset);
+ CS_LOCK(drvdata->base);
+
+ return 0;
+}
+
[...]
Thanks,
Jie