Re: [PATCH v3 4/7] coresight-tgu: Add TGU decode support
From: Mike Leach <hidden>
Date: 2025-03-07 10:58:34
Also in:
linux-arm-msm, linux-devicetree, lkml
Hi, On Thu, 27 Feb 2025 at 09:27, songchai [off-list ref] wrote:
quoted hunk ↗ jump to hunk
From: Songwei Chai <redacted> Decoding is when all the potential pieces for creating a trigger are brought together for a given step. Example - there may be a counter keeping track of some occurrences and a priority-group that is being used to detect a pattern on the sense inputs. These 2 inputs to condition_decode must be programmed, for a given step, to establish the condition for the trigger, or movement to another step. Signed-off-by: Songwei Chai <redacted> Signed-off-by: songchai <redacted> --- .../testing/sysfs-bus-coresight-devices-tgu | 7 ++ drivers/hwtracing/coresight/coresight-tgu.c | 113 ++++++++++++++++-- drivers/hwtracing/coresight/coresight-tgu.h | 29 ++++- 3 files changed, 136 insertions(+), 13 deletions(-)diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tgu b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tgu index af7332153833..dd6cc1184d52 100644 --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tgu +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tgu@@ -14,3 +14,10 @@ KernelVersion 6.15 Contact: Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Sam Chai (QUIC) <quic_songchai@quicinc.com> Description: (RW) Set/Get the sensed siganal with specific step and priority for TGU. + +What: /sys/bus/coresight/devices/<tgu-name>/step[0:7]_condition_decode/reg[0:3] +Date: February 2025 +KernelVersion 6.15 +Contact: Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Sam Chai (QUIC) <quic_songchai@quicinc.com> +Description: + (RW) Set/Get the decode mode with specific step for TGU.diff --git a/drivers/hwtracing/coresight/coresight-tgu.c b/drivers/hwtracing/coresight/coresight-tgu.c index f28761619ebe..5eebf5eecbbb 100644 --- a/drivers/hwtracing/coresight/coresight-tgu.c +++ b/drivers/hwtracing/coresight/coresight-tgu.c@@ -22,9 +22,21 @@ static int calculate_array_location(struct tgu_drvdata *drvdata, int step_index, { int ret = -EINVAL; - ret = operation_index * (drvdata->max_step) * - (drvdata->max_reg) + step_index * (drvdata->max_reg) - + reg_index; + switch (operation_index) { + case TGU_PRIORITY0: + case TGU_PRIORITY1: + case TGU_PRIORITY2: + case TGU_PRIORITY3: + ret = operation_index * (drvdata->max_step) * + (drvdata->max_reg) + step_index * (drvdata->max_reg) + + reg_index; + break; + case TGU_CONDITION_DECODE: + ret = step_index * (drvdata->max_condition_decode) + reg_index; + break; + default: + break; + } return ret; }@@ -36,10 +48,23 @@ static ssize_t tgu_dataset_show(struct device *dev, struct tgu_attribute *tgu_attr = container_of(attr, struct tgu_attribute, attr); - return sysfs_emit(buf, "0x%x\n", - drvdata->value_table->priority[calculate_array_location( + switch (tgu_attr->operation_index) { + case TGU_PRIORITY0: + case TGU_PRIORITY1: + case TGU_PRIORITY2: + case TGU_PRIORITY3: + return sysfs_emit(buf, "0x%x\n", + drvdata->value_table->priority[calculate_array_location(
calculate_array_location() can return -EINVAL - you could be referencing array value drvdata->value_table->priority[-EINVAL] here. Test the return before referencing the array. Same for all following occurrences.
drvdata, tgu_attr->step_index,
tgu_attr->operation_index, tgu_attr->reg_num)]);
+ case TGU_CONDITION_DECODE:
+ return sysfs_emit(buf, "0x%x\n",
+ drvdata->value_table->condition_decode[calculate_array_location(
+ drvdata, tgu_attr->step_index, tgu_attr->operation_index,
+ tgu_attr->reg_num)]);
+missing default - did this code compile without warnings?
quoted hunk ↗ jump to hunk
+ } + return -EINVAL; }@@ -58,11 +83,25 @@ static ssize_t tgu_dataset_store(struct device *dev, return ret; guard(spinlock)(&tgu_drvdata->spinlock); - tgu_drvdata->value_table->priority[calculate_array_location( - tgu_drvdata, tgu_attr->step_index, tgu_attr->operation_index, - tgu_attr->reg_num)] = val; - ret = size; - + switch (tgu_attr->operation_index) { + case TGU_PRIORITY0: + case TGU_PRIORITY1: + case TGU_PRIORITY2: + case TGU_PRIORITY3: + tgu_drvdata->value_table->priority[calculate_array_location( + tgu_drvdata, tgu_attr->step_index, tgu_attr->operation_index, + tgu_attr->reg_num)] = val; + ret = size; + break; + case TGU_CONDITION_DECODE: + tgu_drvdata->value_table->condition_decode[calculate_array_location( + tgu_drvdata, tgu_attr->step_index, tgu_attr->operation_index, + tgu_attr->reg_num)] = val; + ret = size; + break; + default: + break; + } return ret; }@@ -79,8 +118,23 @@ static umode_t tgu_node_visible(struct kobject *kobject, struct attribute *attr, container_of(dev_attr, struct tgu_attribute, attr); if (tgu_attr->step_index < drvdata->max_step) { - ret = (tgu_attr->reg_num < drvdata->max_reg) ? - attr->mode : 0; + switch (tgu_attr->operation_index) { + case TGU_PRIORITY0: + case TGU_PRIORITY1: + case TGU_PRIORITY2: + case TGU_PRIORITY3: + ret = (tgu_attr->reg_num < drvdata->max_reg) ? + attr->mode : 0; + break; + case TGU_CONDITION_DECODE: + ret = (tgu_attr->reg_num < + drvdata->max_condition_decode) ? + attr->mode : 0; + break; + default: + break; + } + return ret; } return SYSFS_GROUP_INVISIBLE;@@ -103,6 +157,17 @@ static void tgu_write_all_hw_regs(struct tgu_drvdata *drvdata) } } + for (i = 0; i < drvdata->max_step; i++) { + for (j = 0; j < drvdata->max_condition_decode; j++) { + tgu_writel(drvdata, + drvdata->value_table + ->condition_decode[calculate_array_location( + drvdata, i, + TGU_CONDITION_DECODE, j)], + CONDITION_DECODE_STEP(i, j)); + } + } + /* Enable TGU to program the triggers */ tgu_writel(drvdata, 1, TGU_CONTROL); CS_LOCK(drvdata->base);@@ -245,6 +310,14 @@ static const struct attribute_group *tgu_attr_groups[] = { PRIORITY_ATTRIBUTE_GROUP_INIT(7, 1), PRIORITY_ATTRIBUTE_GROUP_INIT(7, 2), PRIORITY_ATTRIBUTE_GROUP_INIT(7, 3), + CONDITION_DECODE_ATTRIBUTE_GROUP_INIT(0), + CONDITION_DECODE_ATTRIBUTE_GROUP_INIT(1), + CONDITION_DECODE_ATTRIBUTE_GROUP_INIT(2), + CONDITION_DECODE_ATTRIBUTE_GROUP_INIT(3), + CONDITION_DECODE_ATTRIBUTE_GROUP_INIT(4), + CONDITION_DECODE_ATTRIBUTE_GROUP_INIT(5), + CONDITION_DECODE_ATTRIBUTE_GROUP_INIT(6), + CONDITION_DECODE_ATTRIBUTE_GROUP_INIT(7), NULL, };@@ -289,6 +362,13 @@ static int tgu_probe(struct amba_device *adev, const struct amba_id *id) if (ret) return -EINVAL; + ret = of_property_read_u32(adev->dev.of_node, "tgu-conditions", + &drvdata->max_condition); + if (ret) + return -EINVAL; + + drvdata->max_condition_decode = drvdata->max_condition; + drvdata->value_table = devm_kzalloc(dev, sizeof(*drvdata->value_table), GFP_KERNEL); if (!drvdata->value_table)@@ -303,6 +383,15 @@ static int tgu_probe(struct amba_device *adev, const struct amba_id *id) if (!drvdata->value_table->priority) return -ENOMEM; + drvdata->value_table->condition_decode = devm_kzalloc( + dev, + drvdata->max_condition_decode * drvdata->max_step * + sizeof(*(drvdata->value_table->condition_decode)), + GFP_KERNEL); + + if (!drvdata->value_table->condition_decode) + return -ENOMEM; + drvdata->enable = false; desc.type = CORESIGHT_DEV_TYPE_HELPER; desc.pdata = adev->dev.platform_data;diff --git a/drivers/hwtracing/coresight/coresight-tgu.h b/drivers/hwtracing/coresight/coresight-tgu.h index 6e5d465117df..c2a9ce38b44f 100644 --- a/drivers/hwtracing/coresight/coresight-tgu.h +++ b/drivers/hwtracing/coresight/coresight-tgu.h@@ -46,6 +46,9 @@ #define PRIORITY_REG_STEP(step, priority, reg)\ (0x0074 + 0x60 * priority + 0x4 * reg + 0x1D8 * step) +#define CONDITION_DECODE_STEP(step, decode) \ + (0x0050 + 0x4 * decode + 0x1D8 * step) +
use #define constants with explanations of what they are rather than arbitrary magic numbers.
quoted hunk ↗ jump to hunk
#define tgu_dataset_ro(name, step_index, type, reg_num) \ (&((struct tgu_attribute[]){ { \ __ATTR(name, 0444, tgu_dataset_show, NULL), \@@ -66,6 +69,9 @@ tgu_dataset_rw(reg##reg_num, step_index, TGU_PRIORITY##priority, \ reg_num) +#define STEP_DECODE(step_index, reg_num) \ + tgu_dataset_rw(reg##reg_num, step_index, TGU_CONDITION_DECODE, reg_num) + #define STEP_PRIORITY_LIST(step_index, priority) \ {STEP_PRIORITY(step_index, 0, priority), \ STEP_PRIORITY(step_index, 1, priority), \@@ -88,6 +94,14 @@ NULL \ } +#define STEP_DECODE_LIST(n) \ + {STEP_DECODE(n, 0), \ + STEP_DECODE(n, 1), \ + STEP_DECODE(n, 2), \ + STEP_DECODE(n, 3), \ + NULL \ + } + #define PRIORITY_ATTRIBUTE_GROUP_INIT(step, priority)\ (&(const struct attribute_group){\ .attrs = (struct attribute*[])STEP_PRIORITY_LIST(step, priority),\@@ -95,11 +109,19 @@ .name = "step" #step "_priority" #priority \ }) +#define CONDITION_DECODE_ATTRIBUTE_GROUP_INIT(step)\ + (&(const struct attribute_group){\ + .attrs = (struct attribute*[])STEP_DECODE_LIST(step),\ + .is_visible = tgu_node_visible,\ + .name = "step" #step "_condition_decode" \ + }) + enum operation_index { TGU_PRIORITY0, TGU_PRIORITY1, TGU_PRIORITY2, - TGU_PRIORITY3 + TGU_PRIORITY3, + TGU_CONDITION_DECODE };@@ -115,6 +137,7 @@ struct tgu_attribute { struct value_table { unsigned int *priority; + unsigned int *condition_decode; }; /**@@ -127,6 +150,8 @@ struct value_table { * @value_table: Store given value based on relevant parameters. * @max_reg: Maximum number of registers * @max_step: Maximum step size + * @max_condition: Maximum number of condition + * @max_condition_decode: Maximum number of condition_decode * * This structure defines the data associated with a TGU device, including its base * address, device pointers, clock, spinlock for synchronization, trigger data pointers,@@ -141,6 +166,8 @@ struct tgu_drvdata { struct value_table *value_table; int max_reg; int max_step; + int max_condition; + int max_condition_decode; }; #endif
Regards Mike -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK