CoreSight

coresight@lists.linaro.org

7 participants
2448 discussions

Re: [PATCH v2] coresight: add verification process for coresight_etm_get_trace_id

by Suzuki K Poulose

On Mon, 10 Mar 2025 18:27:24 +0800, Jie Gan wrote: > The coresight_etm_get_trace_id function is a global function. The > verification process for 'csdev' is required prior to its usage. > > Applied, thanks! [1/1] coresight: add verification process for coresight_etm_get_trace_id https://git.kernel.org/coresight/c/ab37128a Best regards, -- Suzuki K Poulose <suzuki.poulose(a)arm.com>

5 months, 2 weeks

Re: [PATCH v6 0/9] coresight: change some driver' spinlock type to raw_spinlock_t

by Suzuki K Poulose

On Thu, 06 Mar 2025 12:11:01 +0000, Yeoreum Yun wrote: > In some coresight drivers, drvdata->spinlock can be held during __schedule() > by perf_event_task_sched_out()/in(). > > Since drvdata->spinlock type is spinlock_t and > perf_event_task_sched_out()/in() is called after acquiring rq_lock, > which is raw_spinlock_t (an unsleepable lock), > this poses an issue in PREEMPT_RT kernel where spinlock_t is sleepable. > > [...] Applied, thanks! [1/9] coresight: change coresight_device lock type to raw_spinlock_t https://git.kernel.org/coresight/c/26f060c1 [2/9] coresight-etm4x: change etmv4_drvdata spinlock type to raw_spinlock_t https://git.kernel.org/coresight/c/743c5a97 [3/9] coresight: change coresight_trace_id_map's lock type to raw_spinlock_t https://git.kernel.org/coresight/c/4cf364ca [4/9] coresight-cti: change cti_drvdata spinlock's type to raw_spinlock_t https://git.kernel.org/coresight/c/e3044065 [5/9] coresight-etb10: change etb_drvdata spinlock's type to raw_spinlock_t https://git.kernel.org/coresight/c/6b80c0ab [6/9] coresight-funnel: change funnel_drvdata spinlock's type to raw_spinlock_t https://git.kernel.org/coresight/c/56eb02f0 [7/9] coresight-replicator: change replicator_drvdata spinlock's type to raw_spinlock_t https://git.kernel.org/coresight/c/982d0a0e [8/9] coresight-tmc: change tmc_drvdata spinlock's type to raw_spinlock_t https://git.kernel.org/coresight/c/db11f75b [9/9] coresight/ultrasoc: change smb_drv_data spinlock's type to raw_spinlock_t https://git.kernel.org/coresight/c/d11eb31d Best regards, -- Suzuki K Poulose <suzuki.poulose(a)arm.com>

5 months, 2 weeks

Re: [PATCH v2 2/5] coresight: add coresight Trace NOC driver

by Leo Yan

On Thu, Mar 06, 2025 at 04:22:20PM +0800, Yuanfang Zhang wrote: [...] > >> +static int trace_noc_init_default_data(struct trace_noc_drvdata *drvdata) > >> +{ > >> + int atid; > >> + > >> + atid = coresight_trace_id_get_system_id(); > >> + if (atid < 0) > >> + return atid; > >> + > >> + drvdata->atid = atid; > >> + > >> + drvdata->freq_type = FREQ_TS; > > > > I don't see anywhere uses FREQ. Please remove the unused definitions > > and related code. > > it is used in trace_noc_enable_hw(). I understood some macros and definitions are used by seqential patches. A good practice is code should be added only when they are used. This can allow every patch in neat way and easier for review. Thanks, Leo > > > >> + drvdata->flag_type = FLAG; > > > > FLAG_TS is not used in the driver as well. Remove it. > it is used in trace_noc_enable_hw().

5 months, 2 weeks

Re: [PATCH] coresight: add verification process for coresight_etm_get_trace_id

by Suzuki K Poulose

On 10/03/2025 02:23, Jie Gan wrote: > The coresight_etm_get_trace_id function is a global function. The > verification process for 'csdev' is required prior to its usage. > > Fixes: c367a89dec26 ("Coresight: Add trace_id function to retrieving the trace ID") > Signed-off-by: Jie Gan <quic_jiegan(a)quicinc.com> > --- > drivers/hwtracing/coresight/coresight-core.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c > index bd0a7edd38c9..5a7cd2376e2d 100644 > --- a/drivers/hwtracing/coresight/coresight-core.c > +++ b/drivers/hwtracing/coresight/coresight-core.c > @@ -1616,9 +1616,12 @@ EXPORT_SYMBOL_GPL(coresight_remove_driver); > int coresight_etm_get_trace_id(struct coresight_device *csdev, enum cs_mode mode, > struct coresight_device *sink) > { > - int trace_id; > - int cpu = source_ops(csdev)->cpu_id(csdev); > + int cpu, trace_id; > + > + if (csdev->type != CORESIGHT_DEV_TYPE_SOURCE && !source_ops(csdev)->cpu_id) That must be : csdev->type != CORESIGHT_DEV_TYPE_SOURCE || !source_ops(csdev)->cpu_id) Suzuki > + return -EINVAL; > > + cpu = source_ops(csdev)->cpu_id(csdev); > switch (mode) { > case CS_MODE_SYSFS: > trace_id = coresight_trace_id_get_cpu_id(cpu);

5 months, 2 weeks

[PATCH 1/1 next] perf libunwind: Fixup conversion perf_sample->user_regs to a pointer

by Arnaldo Carvalho de Melo

From: Arnaldo Carvalho de Melo <acme(a)redhat.com> The dc6d2bc2d893a878 ("perf sample: Make user_regs and intr_regs optional") misses the changes to a file, resulting in this problem: $ make LIBUNWIND=1 -C tools/perf O=/tmp/build/perf-tools-next install-bin <SNIP> CC /tmp/build/perf-tools-next/util/unwind-libunwind-local.o CC /tmp/build/perf-tools-next/util/unwind-libunwind.o <SNIP> util/unwind-libunwind-local.c: In function ‘access_mem’: util/unwind-libunwind-local.c:582:56: error: ‘ui->sample->user_regs’ is a pointer; did you mean to use ‘->’? 582 | if (__write || !stack || !ui->sample->user_regs.regs) { | ^ | -> util/unwind-libunwind-local.c:587:38: error: passing argument 2 of ‘perf_reg_value’ from incompatible pointer type [-Wincompatible-pointer-types] 587 | ret = perf_reg_value(&start, &ui->sample->user_regs, | ^~~~~~~~~~~~~~~~~~~~~~ | | | struct regs_dump ** <SNIP> ⬢ [acme@toolbox perf-tools-next]$ git bisect bad dc6d2bc2d893a878e7b58578ff01b4738708deb4 is the first bad commit commit dc6d2bc2d893a878e7b58578ff01b4738708deb4 (HEAD) Author: Ian Rogers <irogers(a)google.com> Date: Mon Jan 13 11:43:45 2025 -0800 perf sample: Make user_regs and intr_regs optional Detected using: make -C tools/perf build-test Fixes: dc6d2bc2d893a878 ("perf sample: Make user_regs and intr_regs optional") Cc: Adrian Hunter <adrian.hunter(a)intel.com> Cc: Alexander Shishkin <alexander.shishkin(a)linux.intel.com> Cc: Andi Kleen <ak(a)linux.intel.com> Cc: Charlie Jenkins <charlie(a)rivosinc.com> Cc: Ian Rogers <irogers(a)google.com> Cc: Ingo Molnar <mingo(a)redhat.com> Cc: James Clark <james.clark(a)linaro.org> Cc: John Garry <john.g.garry(a)oracle.com> Cc: Kan Liang <kan.liang(a)linux.intel.com> Cc: Leo Yan <leo.yan(a)linux.dev> Cc: Mark Rutland <mark.rutland(a)arm.com> Cc: Michael Petlan <mpetlan(a)redhat.com> Cc: Namhyung Kim <namhyung(a)kernel.org> Cc: Peter Zijlstra <peterz(a)infradead.org> Cc: Tavian Barnes <tavianator(a)tavianator.com> Cc: Veronika Molnarova <vmolnaro(a)redhat.com> Cc: coresight(a)lists.linaro.org Cc: linux-arm-kernel(a)lists.infradead.org Signed-off-by: Arnaldo Carvalho de Melo <acme(a)redhat.com> --- tools/perf/util/unwind-libunwind-local.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tools/perf/util/unwind-libunwind-local.c b/tools/perf/util/unwind-libunwind-local.c index 16c2b03831f39afc..cc0e41f2c2f2ef10 100644 --- a/tools/perf/util/unwind-libunwind-local.c +++ b/tools/perf/util/unwind-libunwind-local.c @@ -579,12 +579,12 @@ static int access_mem(unw_addr_space_t __maybe_unused as, int ret; /* Don't support write, probably not needed. */ - if (__write || !stack || !ui->sample->user_regs.regs) { + if (__write || !stack || !ui->sample->user_regs->regs) { *valp = 0; return 0; } - ret = perf_reg_value(&start, &ui->sample->user_regs, + ret = perf_reg_value(&start, perf_sample__user_regs(ui->sample), perf_arch_reg_sp(arch)); if (ret) return ret; @@ -628,7 +628,7 @@ static int access_reg(unw_addr_space_t __maybe_unused as, return 0; } - if (!ui->sample->user_regs.regs) { + if (!ui->sample->user_regs->regs) { *valp = 0; return 0; } @@ -637,7 +637,7 @@ static int access_reg(unw_addr_space_t __maybe_unused as, if (id < 0) return -EINVAL; - ret = perf_reg_value(&val, &ui->sample->user_regs, id); + ret = perf_reg_value(&val, perf_sample__user_regs(ui->sample), id); if (ret) { if (!ui->best_effort) pr_err("unwind: can't read reg %d\n", regnum); @@ -741,7 +741,7 @@ static int get_entries(struct unwind_info *ui, unwind_entry_cb_t cb, unw_cursor_t c; int ret, i = 0; - ret = perf_reg_value(&val, &ui->sample->user_regs, + ret = perf_reg_value(&val, perf_sample__user_regs(ui->sample), perf_arch_reg_ip(arch)); if (ret) return ret; @@ -808,7 +808,7 @@ static int _unwind__get_entries(unwind_entry_cb_t cb, void *arg, .best_effort = best_effort }; - if (!data->user_regs.regs) + if (!data->user_regs->regs) return -EINVAL; if (max_stack <= 0) -- 2.47.0

5 months, 3 weeks

Re: [PATCH v3 7/7] coresight-tgu: add reset node to initialize

by Mike Leach

Hi, On Thu, 27 Feb 2025 at 09:27, songchai <quic_songchai(a)quicinc.com> wrote: > > From: Songwei Chai <quic_songchai(a)quicinc.com> > > Add reset node to initialize the value of > priority/condition_decode/condition_select/timer/counter nodes > > Signed-off-by: Songwei Chai <quic_songchai(a)quicinc.com> > Signed-off-by: songchai <quic_songchai(a)quicinc.com> > --- > .../testing/sysfs-bus-coresight-devices-tgu | 7 ++ > drivers/hwtracing/coresight/coresight-tgu.c | 79 +++++++++++++++++++ > 2 files changed, 86 insertions(+) > > diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tgu b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tgu > index d88d05fbff43..8fb5afd7c655 100644 > --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tgu > +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tgu > @@ -42,3 +42,10 @@ KernelVersion 6.15 > Contact: Jinlong Mao (QUIC) <quic_jinlmao(a)quicinc.com>, Sam Chai (QUIC) <quic_songchai(a)quicinc.com> > Description: > (RW) Set/Get the counter value with specific step for TGU. > + > +What: /sys/bus/coresight/devices/<tgu-name>/reset_tgu > +Date: February 2025 > +KernelVersion 6.15 > +Contact: Jinlong Mao (QUIC) <quic_jinlmao(a)quicinc.com>, Sam Chai (QUIC) <quic_songchai(a)quicinc.com> > +Description: > + (Write) Reset the dataset for TGU. Document the value needed to initiate the reset. > diff --git a/drivers/hwtracing/coresight/coresight-tgu.c b/drivers/hwtracing/coresight/coresight-tgu.c > index 693d632fb079..b36ced761c0d 100644 > --- a/drivers/hwtracing/coresight/coresight-tgu.c > +++ b/drivers/hwtracing/coresight/coresight-tgu.c > @@ -343,6 +343,84 @@ static ssize_t enable_tgu_store(struct device *dev, > } > static DEVICE_ATTR_RW(enable_tgu); > > +/* reset_tgu_store - Reset Trace and Gating Unit (TGU) configuration. */ > +static ssize_t reset_tgu_store(struct device *dev, > + struct device_attribute *attr, const char *buf, > + size_t size) > +{ > + unsigned long value; > + struct tgu_drvdata *drvdata = dev_get_drvdata(dev->parent); > + int i, j, ret; > + > + if (kstrtoul(buf, 0, &value)) > + return -EINVAL; > + Check "value" here and bail out with an error code if 0. > + if (!drvdata->enable) { > + ret = pm_runtime_get_sync(drvdata->dev); > + if (ret < 0) { > + pm_runtime_put(drvdata->dev); > + return ret; > + } > + } > + > + spin_lock(&drvdata->spinlock); > + CS_UNLOCK(drvdata->base); > + > + if (value) { drop this line > + tgu_writel(drvdata, 0, TGU_CONTROL); > + > + if (drvdata->value_table->priority) > + memset(drvdata->value_table->priority, 0, > + MAX_PRIORITY * drvdata->max_step * > + drvdata->max_reg * sizeof(unsigned int)); > + > + if (drvdata->value_table->condition_decode) > + memset(drvdata->value_table->condition_decode, 0, > + drvdata->max_condition_decode * drvdata->max_step * > + sizeof(unsigned int)); > + > + /* Initialize all condition registers to NOT(value=0x1000000) */ > + for (i = 0; i < drvdata->max_step; i++) { > + for (j = 0; j < drvdata->max_condition_decode; j++) { > + drvdata->value_table > + ->condition_decode[calculate_array_location( > + drvdata, i, TGU_CONDITION_DECODE, j)] = > + 0x1000000; > + } > + } > + > + if (drvdata->value_table->condition_select) > + memset(drvdata->value_table->condition_select, 0, > + drvdata->max_condition_select * drvdata->max_step * > + sizeof(unsigned int)); > + > + if (drvdata->value_table->timer) > + memset(drvdata->value_table->timer, 0, > + (drvdata->max_step) * > + (drvdata->max_timer_counter) * > + sizeof(unsigned int)); > + > + if (drvdata->value_table->counter) > + memset(drvdata->value_table->counter, 0, > + (drvdata->max_step) * > + (drvdata->max_timer_counter) * > + sizeof(unsigned int)); > + > + dev_dbg(dev, "Coresight-TGU reset complete\n"); > + } else { > + dev_dbg(dev, "Coresight-TGU invalid input\n"); not needed if early exit on input errror > + } > + > + CS_LOCK(drvdata->base); > + > + drvdata->enable = false; > + spin_unlock(&drvdata->spinlock); > + pm_runtime_put(drvdata->dev); > + > + return size; > +} > +static DEVICE_ATTR_WO(reset_tgu); > + > static const struct coresight_ops_helper tgu_helper_ops = { > .enable = tgu_enable, > .disable = tgu_disable, > @@ -354,6 +432,7 @@ static const struct coresight_ops tgu_ops = { > > static struct attribute *tgu_common_attrs[] = { > &dev_attr_enable_tgu.attr, > + &dev_attr_reset_tgu.attr, > NULL, > }; > > Regards Mike -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

5 months, 3 weeks

Re: [PATCH v3 3/7] coresight-tgu: Add signal priority support

by Mike Leach

Hi On Thu, 27 Feb 2025 at 09:27, songchai <quic_songchai(a)quicinc.com> wrote: > > From: Songwei Chai <quic_songchai(a)quicinc.com> > > Like circuit of a Logic analyzer, in TGU, the requirement could be > configured in each step and the trigger will be created once the > requirements are met. Add priority functionality here to sort the > signals into different priorities. The signal which is wanted could > be configured in each step's priority node, the larger number means > the higher priority and the signal with higher priority will be sensed > more preferentially. > > Signed-off-by: Songwei Chai <quic_songchai(a)quicinc.com> > Signed-off-by: songchai <quic_songchai(a)quicinc.com> > --- > .../testing/sysfs-bus-coresight-devices-tgu | 7 + > drivers/hwtracing/coresight/coresight-tgu.c | 139 ++++++++++++++++++ > drivers/hwtracing/coresight/coresight-tgu.h | 110 ++++++++++++++ > 3 files changed, 256 insertions(+) > > diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tgu b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tgu > index 741bc9fd9df5..af7332153833 100644 > --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tgu > +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tgu > @@ -7,3 +7,10 @@ Description: > Accepts only one of the 2 values - 0 or 1. > 0 : disable TGU. > 1 : enable TGU. > + > +What: /sys/bus/coresight/devices/<tgu-name>/step[0:7]_priority[0:3]/reg[0:17] > +Date: February 2025 > +KernelVersion 6.15 > +Contact: Jinlong Mao (QUIC) <quic_jinlmao(a)quicinc.com>, Sam Chai (QUIC) <quic_songchai(a)quicinc.com> > +Description: > + (RW) Set/Get the sensed siganal with specific step and priority for TGU. sp/siganal/signal > diff --git a/drivers/hwtracing/coresight/coresight-tgu.c b/drivers/hwtracing/coresight/coresight-tgu.c > index da4c04ac1097..f28761619ebe 100644 > --- a/drivers/hwtracing/coresight/coresight-tgu.c > +++ b/drivers/hwtracing/coresight/coresight-tgu.c > @@ -17,9 +17,92 @@ > > DEFINE_CORESIGHT_DEVLIST(tgu_devs, "tgu"); > > +static int calculate_array_location(struct tgu_drvdata *drvdata, int step_index, > + int operation_index, int reg_index) > +{ > + int ret = -EINVAL; > + > + ret = operation_index * (drvdata->max_step) * > + (drvdata->max_reg) + step_index * (drvdata->max_reg) > + + reg_index; > + > + return ret; > +} > + > +static ssize_t tgu_dataset_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct tgu_drvdata *drvdata = dev_get_drvdata(dev->parent); > + 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( > + drvdata, tgu_attr->step_index, > + tgu_attr->operation_index, tgu_attr->reg_num)]); > + > +} > + > +static ssize_t tgu_dataset_store(struct device *dev, > + struct device_attribute *attr, const char *buf, > + size_t size) > +{ > + unsigned long val; > + ssize_t ret = -EINVAL; > + > + struct tgu_drvdata *tgu_drvdata = dev_get_drvdata(dev->parent); > + struct tgu_attribute *tgu_attr = > + container_of(attr, struct tgu_attribute, attr); > + > + if (kstrtoul(buf, 0, &val)) > + 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; > + > + return ret; ret is unneeded - directly return either size or -EINVAL. > +} > + > +static umode_t tgu_node_visible(struct kobject *kobject, struct attribute *attr, > + int n) > +{ > + struct device *dev = kobj_to_dev(kobject); > + struct tgu_drvdata *drvdata = dev_get_drvdata(dev->parent); > + int ret = 0; > + > + struct device_attribute *dev_attr = > + container_of(attr, struct device_attribute, attr); > + struct tgu_attribute *tgu_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; > + return ret; > + } > + return SYSFS_GROUP_INVISIBLE; > +} default ret as SYSFS_GROUP_INVISIBLE, and returnret at end > + > static void tgu_write_all_hw_regs(struct tgu_drvdata *drvdata) > { > + int i, j, k; > + > CS_UNLOCK(drvdata->base); > + > + for (i = 0; i < drvdata->max_step; i++) { > + for (j = 0; j < MAX_PRIORITY; j++) { > + for (k = 0; k < drvdata->max_reg; k++) { > + tgu_writel(drvdata, > + drvdata->value_table->priority > + [calculate_array_location(drvdata, i, j, k)], > + PRIORITY_REG_STEP(i, j, k)); > + } > + } > + } > + > /* Enable TGU to program the triggers */ > tgu_writel(drvdata, 1, TGU_CONTROL); > CS_LOCK(drvdata->base); > @@ -130,6 +213,38 @@ static const struct attribute_group tgu_common_grp = { > > static const struct attribute_group *tgu_attr_groups[] = { > &tgu_common_grp, > + PRIORITY_ATTRIBUTE_GROUP_INIT(0, 0), > + PRIORITY_ATTRIBUTE_GROUP_INIT(0, 1), > + PRIORITY_ATTRIBUTE_GROUP_INIT(0, 2), > + PRIORITY_ATTRIBUTE_GROUP_INIT(0, 3), > + PRIORITY_ATTRIBUTE_GROUP_INIT(1, 0), > + PRIORITY_ATTRIBUTE_GROUP_INIT(1, 1), > + PRIORITY_ATTRIBUTE_GROUP_INIT(1, 2), > + PRIORITY_ATTRIBUTE_GROUP_INIT(1, 3), > + PRIORITY_ATTRIBUTE_GROUP_INIT(2, 0), > + PRIORITY_ATTRIBUTE_GROUP_INIT(2, 1), > + PRIORITY_ATTRIBUTE_GROUP_INIT(2, 2), > + PRIORITY_ATTRIBUTE_GROUP_INIT(2, 3), > + PRIORITY_ATTRIBUTE_GROUP_INIT(3, 0), > + PRIORITY_ATTRIBUTE_GROUP_INIT(3, 1), > + PRIORITY_ATTRIBUTE_GROUP_INIT(3, 2), > + PRIORITY_ATTRIBUTE_GROUP_INIT(3, 3), > + PRIORITY_ATTRIBUTE_GROUP_INIT(4, 0), > + PRIORITY_ATTRIBUTE_GROUP_INIT(4, 1), > + PRIORITY_ATTRIBUTE_GROUP_INIT(4, 2), > + PRIORITY_ATTRIBUTE_GROUP_INIT(4, 3), > + PRIORITY_ATTRIBUTE_GROUP_INIT(5, 0), > + PRIORITY_ATTRIBUTE_GROUP_INIT(5, 1), > + PRIORITY_ATTRIBUTE_GROUP_INIT(5, 2), > + PRIORITY_ATTRIBUTE_GROUP_INIT(5, 3), > + PRIORITY_ATTRIBUTE_GROUP_INIT(6, 0), > + PRIORITY_ATTRIBUTE_GROUP_INIT(6, 1), > + PRIORITY_ATTRIBUTE_GROUP_INIT(6, 2), > + PRIORITY_ATTRIBUTE_GROUP_INIT(6, 3), > + PRIORITY_ATTRIBUTE_GROUP_INIT(7, 0), > + PRIORITY_ATTRIBUTE_GROUP_INIT(7, 1), > + PRIORITY_ATTRIBUTE_GROUP_INIT(7, 2), > + PRIORITY_ATTRIBUTE_GROUP_INIT(7, 3), > NULL, > }; > > @@ -164,6 +279,30 @@ static int tgu_probe(struct amba_device *adev, const struct amba_id *id) > > spin_lock_init(&drvdata->spinlock); > > + ret = of_property_read_u32(adev->dev.of_node, "tgu-regs", > + &drvdata->max_reg); > + if (ret) > + return -EINVAL; > + > + ret = of_property_read_u32(adev->dev.of_node, "tgu-steps", > + &drvdata->max_step); > + if (ret) > + return -EINVAL; > + > + drvdata->value_table = > + devm_kzalloc(dev, sizeof(*drvdata->value_table), GFP_KERNEL); > + if (!drvdata->value_table) > + return -ENOMEM; > + > + drvdata->value_table->priority = devm_kzalloc( > + dev, > + MAX_PRIORITY * drvdata->max_reg * drvdata->max_step * > + sizeof(*(drvdata->value_table->priority)), > + GFP_KERNEL); > + > + if (!drvdata->value_table->priority) > + 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 380686f94130..6e5d465117df 100644 > --- a/drivers/hwtracing/coresight/coresight-tgu.h > +++ b/drivers/hwtracing/coresight/coresight-tgu.h > @@ -13,6 +13,110 @@ > #define tgu_writel(drvdata, val, off) __raw_writel((val), drvdata->base + off) > #define tgu_readl(drvdata, off) __raw_readl(drvdata->base + off) > > +/* > + * TGU configuration space Step configuration > + * offset table space layout > + * x-------------------------x$ x-------------x$ > + * | |$ | |$ > + * | | | reserve |$ > + * | | | |$ > + * |coresight management | |-------------|base+n*0x1D8+0x1F4$ > + * | registe | |---> |prioroty[3] |$ > + * | | | |-------------|base+n*0x1D8+0x194$ > + * | | | |prioroty[2] |$ > + * |-------------------------| | |-------------|base+n*0x1D8+0x134$ > + * | | | |prioroty[1] |$ > + * | step[7] | | |-------------|base+n*0x1D8+0xD4$ > + * |-------------------------|->base+0x40+7*0x1D8 | |prioroty[0] |$ > + * | | | |-------------|base+n*0x1D8+0x74$ > + * | ... | | | condition |$ > + * | | | | select |$ > + * |-------------------------|->base+0x40+1*0x1D8 | |-------------|base+n*0x1D8+0x60$ > + * | | | | condition |$ > + * | step[0] |--------------------> | decode |$ > + * |-------------------------|-> base+0x40 |-------------|base+n*0x1D8+0x50$ > + * | | | |$ > + * | Control and status space| |Timer/Counter|$ > + * | space | | |$ > + * x-------------------------x->base x-------------x base+n*0x1D8+0x40$ > + * > + */ > + > +/* Calculate compare step addresses */ > +#define PRIORITY_REG_STEP(step, priority, reg)\ > + (0x0074 + 0x60 * priority + 0x4 * reg + 0x1D8 * step) > + use #defines + explanation instead of arbitrary magic numbers > +#define tgu_dataset_ro(name, step_index, type, reg_num) \ > + (&((struct tgu_attribute[]){ { \ > + __ATTR(name, 0444, tgu_dataset_show, NULL), \ > + step_index, \ > + type, \ > + reg_num, \ > + } })[0].attr.attr) > + This define unused in this patch, Drop till it is used. > +#define tgu_dataset_rw(name, step_index, type, reg_num) \ > + (&((struct tgu_attribute[]){ { \ > + __ATTR(name, 0644, tgu_dataset_show, tgu_dataset_store), \ > + step_index, \ > + type, \ > + reg_num, \ > + } })[0].attr.attr) > + > +#define STEP_PRIORITY(step_index, reg_num, priority) \ > + tgu_dataset_rw(reg##reg_num, step_index, TGU_PRIORITY##priority, \ > + reg_num) > + > +#define STEP_PRIORITY_LIST(step_index, priority) \ > + {STEP_PRIORITY(step_index, 0, priority), \ > + STEP_PRIORITY(step_index, 1, priority), \ > + STEP_PRIORITY(step_index, 2, priority), \ > + STEP_PRIORITY(step_index, 3, priority), \ > + STEP_PRIORITY(step_index, 4, priority), \ > + STEP_PRIORITY(step_index, 5, priority), \ > + STEP_PRIORITY(step_index, 6, priority), \ > + STEP_PRIORITY(step_index, 7, priority), \ > + STEP_PRIORITY(step_index, 8, priority), \ > + STEP_PRIORITY(step_index, 9, priority), \ > + STEP_PRIORITY(step_index, 10, priority), \ > + STEP_PRIORITY(step_index, 11, priority), \ > + STEP_PRIORITY(step_index, 12, priority), \ > + STEP_PRIORITY(step_index, 13, priority), \ > + STEP_PRIORITY(step_index, 14, priority), \ > + STEP_PRIORITY(step_index, 15, priority), \ > + STEP_PRIORITY(step_index, 16, priority), \ > + STEP_PRIORITY(step_index, 17, priority), \ > + NULL \ > + } > + > +#define PRIORITY_ATTRIBUTE_GROUP_INIT(step, priority)\ > + (&(const struct attribute_group){\ > + .attrs = (struct attribute*[])STEP_PRIORITY_LIST(step, priority),\ > + .is_visible = tgu_node_visible,\ > + .name = "step" #step "_priority" #priority \ > + }) > + > +enum operation_index { > + TGU_PRIORITY0, > + TGU_PRIORITY1, > + TGU_PRIORITY2, > + TGU_PRIORITY3 > + > +}; > + > +/* Maximum priority that TGU supports */ > +#define MAX_PRIORITY 4 > + > +struct tgu_attribute { > + struct device_attribute attr; > + u32 step_index; > + enum operation_index operation_index; > + u32 reg_num; > +}; > + > +struct value_table { > + unsigned int *priority; > +}; > + > /** > * struct tgu_drvdata - Data structure for a TGU (Trigger Generator Unit) device > * @base: Memory-mapped base address of the TGU device > @@ -20,6 +124,9 @@ > * @csdev: Pointer to the associated coresight device > * @spinlock: Spinlock for handling concurrent access > * @enable: Flag indicating whether the TGU device is enabled > + * @value_table: Store given value based on relevant parameters. > + * @max_reg: Maximum number of registers > + * @max_step: Maximum step size > * > * This structure defines the data associated with a TGU device, including its base > * address, device pointers, clock, spinlock for synchronization, trigger data pointers, > @@ -31,6 +138,9 @@ struct tgu_drvdata { > struct coresight_device *csdev; > spinlock_t spinlock; > bool enable; > + struct value_table *value_table; > + int max_reg; > + int max_step; > }; > > #endif Regards Mike -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

5 months, 3 weeks

Re: [PATCH v3 4/7] coresight-tgu: Add TGU decode support

by Mike Leach

Hi, On Thu, 27 Feb 2025 at 09:27, songchai <quic_songchai(a)quicinc.com> wrote: > > From: Songwei Chai <quic_songchai(a)quicinc.com> > > 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 <quic_songchai(a)quicinc.com> > Signed-off-by: songchai <quic_songchai(a)quicinc.com> > --- > .../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(a)quicinc.com>, Sam Chai (QUIC) <quic_songchai(a)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(a)quicinc.com>, Sam Chai (QUIC) <quic_songchai(a)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? > + } > + 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. > #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

5 months, 3 weeks

Re: [PATCH v3 1/7] dt-bindings: arm: Add support for Coresight TGU trace

by Mike Leach

Hi, On Tue, 4 Mar 2025 at 08:05, Krzysztof Kozlowski <krzk(a)kernel.org> wrote: > > On Thu, Feb 27, 2025 at 05:26:34PM +0800, songchai wrote: > > From: Songwei Chai <quic_songchai(a)quicinc.com> > > > > The Trigger Generation Unit (TGU) is designed to detect patterns or > > sequences within a specific region of the System on Chip (SoC). Once > > configured and activated, it monitors sense inputs and can detect a > > pre-programmed state or sequence across clock cycles, subsequently > > producing a trigger. > > > > TGU configuration space > > offset table > > x-------------------------x > > | | > > | | > > | | Step configuration > > | | space layout > > | coresight management | x-------------x > > | registers | |---> | | > > | | | | reserve | > > | | | | | > > |-------------------------| | |-------------| > > | | | | priority[3] | > > | step[7] |<-- | |-------------| > > |-------------------------| | | | priority[2] | > > | | | | |-------------| > > | ... | |Steps region | | priority[1] | > > | | | | |-------------| > > |-------------------------| | | | priority[0] | > > | |<-- | |-------------| > > | step[0] |--------------------> | | > > |-------------------------| | condition | > > | | | | > > | control and status | x-------------x > > | space | | | > > x-------------------------x |Timer/Counter| > > | | > > x-------------x > > TGU Configuration in Hardware > > > > The TGU provides a step region for user configuration, similar > > to a flow chart. Each step region consists of three register clusters: > > > > 1.Priority Region: Sets the required signals with priority. > > 2.Condition Region: Defines specific requirements (e.g., signal A > > reaches three times) and the subsequent action once the requirement is > > met. > > 3.Timer/Counter (Optional): Provides timing or counting functionality. > > > > Add a new coresight-tgu.yaml file to describe the bindings required to > > define the TGU in the device trees. > > > > Signed-off-by: Songwei Chai <quic_songchai(a)quicinc.com> > > Signed-off-by: songchai <quic_songchai(a)quicinc.com> > > Don't duplicate yourself. > > Anyway, this is marked as v3, I cannot find previous versions, no > changelog, no references. > > What happened here in this binding? > > > --- > > .../bindings/arm/qcom,coresight-tgu.yaml | 135 ++++++++++++++++++ > > 1 file changed, 135 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/arm/qcom,coresight-tgu.yaml > > > > diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-tgu.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-tgu.yaml > > new file mode 100644 > > index 000000000000..a41ac68a4fe7 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-tgu.yaml > > @@ -0,0 +1,135 @@ > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > > +# Copyright (c) 2023-2025 Qualcomm Innovation Center, Inc. All rights reserved. > > 2023 and 2024? Where was it published in these years? > > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/arm/qcom,coresight-tgu.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Trigger Generation Unit - TGU > > + > > +description: | > > + The Trigger Generation Unit (TGU) is a Data Engine which can be utilized > > + to sense a plurality of signals and create a trigger into the CTI or > > + generate interrupts to processors. The TGU is like the trigger circuit > > + of a Logic Analyzer. The corresponding trigger logic can be realized by > > + configuring the conditions for each step after sensing the signal. > > + Once setup and enabled, it will observe sense inputs and based upon > > + the activity of those inputs, even over clock cycles, may detect a > > + preprogrammed state/sequence and then produce a trigger or interrupt. > > + > > + The primary use case of the TGU is to detect patterns or sequences on a > > + given set of signals within some region of the SoC. > > + > > +maintainers: > > + - Mao Jinlong <quic_jinlmao(a)quicinc.com> > > + - Sam Chai <quic_songchai(a)quicinc.com> > > + > > +# Need a custom select here or 'arm,primecell' will match on lots of nodes > > +select: > > + properties: > > + compatible: > > + contains: > > + enum: > > + - qcom,coresight-tgu > > + required: > > + - compatible > > + > > +properties: > > + compatible: > > + items: > > + - const: qcom,coresight-tgu > > + - const: arm,primecell > > + > > + reg: > > + maxItems: 1 > > + > > + clocks: > > + maxItems: 1 > > + > > + clock-names: > > + items: > > + - const: apb_pclk > > + > > + qcom,tgu-steps: > > + description: > > + The trigger logic is realized by configuring each step after sensing > > + the signal. The parameter here is used to describe the maximum of steps > > + that could be configured in the current TGU. > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + minimum: 1 > > + maximum: 8 > > + Hardware features are usually defined by ID registers in coresight devices. e.g. ETM has a number of ID registers that describe the number of comparators / counters etc. Does this device not have similar registers? Is there not a unique ID for each hardware variant - hardware discoverablility is an architecture requirement for coresight devices? > > + qcom,tgu-regs: > > + description: > > + There are some "groups" register clusters in each step, which are used to > > + configure the signal that we want to detect. Meanwhile, each group has its > > + own priority, and the priority increases with number of groups. For example, > > + group3 has a higher priority than group2, the signal configured in group3 > > + will be sensed more preferentially than the signal which is configured in group2. > > + The parameter here is used to describe the signal number that each group > > + could be configured. > > And all groups are indexed by number? Or do they have names? > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + minimum: 1 > > + maximum: 18 > > + > > + qcom,tgu-conditions: > > + description: > > + A condition sets a specific requirement for a step and defines the subsequent > > + action once the requirement is met. For example, in step two, if signal A is > > + detected three times, the process jumps back to step one. The parameter describes > > + the register number for each functionality, whether it is setting a specific > > + requirement or defining a subsequent action. > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + minimum: 1 > > + maximum: 4 > > + > > + qcom,tgu-timer-counters: > > + description: > > + TGU has timer and counter which are used to set some requirement on each step. > > Wrap according to Linux coding style, so at 80. > > > + For example, we could use counter to create a trigger into CTI once TGU senses > > + the target signal three times.This parameter is used to describe the number of > > + Timers/Counters in TGU. > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + minimum: 0 > > Drop > > > + maximum: 2 > > + > > + in-ports: > > + $ref: /schemas/graph.yaml#/properties/ports > > + additionalProperties: false > > + > > + properties: > > + port: > > + description: AXI Slave connected to another Coresight component > > So this TGU can be connected to anything in coresight graph, no > restrictions? > Coresight uses APB for register access and ATB for moving trace from source to sink. The only use of AXI is on the ETR/CATU output saving trace data into system memory. > > + $ref: /schemas/graph.yaml#/properties/port > > + > > +required: > > + - compatible > > + - reg > > + - clocks > > + - clock-names > > Most likely you miss also: in-ports > > > Best regards, > Krzysztof > Regards Mike -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

5 months, 3 weeks

Re: [PATCH v3 0/7] Provides support for Trigger Generation Unit

by Mike Leach

Hi, On Thu, 27 Feb 2025 at 09:27, songchai <quic_songchai(a)quicinc.com> wrote: > > Provide support for the TGU (Trigger Generation Unit), which can be > utilized to sense a plurality of signals and create a trigger into > the CTI or generate interrupts to processors once the input signal > meets the conditions. We can treat the TGU’s workflow as a flowsheet, > it has some “steps” regions for customization. In each step region, > we can set the signals that we want with priority in priority_group, set > the conditions in each step via condition_decode, and set the resultant > action by condition_select. Meanwhile, some TGUs (not all) also provide > timer/counter functionality. Based on the characteristics described > above, we consider the TGU as a helper in the CoreSight subsystem. > Its master device is the TPDM, which can transmit signals from other > subsystems, and we reuse the existing ports mechanism to link the TPDM to > the connected TGU. > I do not believe that his component is part of the Coresight subsystem. 1) It inputs multiple signals from the SoC to process and create an trigger event - however, it can do this irrespective of CoreSight trace being operational, especially where generating interrupts for processors, or triggers for other non-coresight components. It would appear that the TPDM can send output to more than just the TDPA which generates coresight trace packets - a previously undisclosed feature. 2) The ports mechanism is a generic device tree mechanism, not something unique to the Coresight subsystem. 3) The CTI trigger connection will be defined in devicetree under the CTI component, as this is the interface between this component and coresight. As such this seems more like a general performance and debug component, with optional inputs to the coresight trigger mechanisms, rather than being a coresight component itself. Other SoCs have non-coresight component inputs to CTIs. For example the PL011 serial device on Juno has a signal into one of the system CTIs. > Here is a detailed example to explain how to use the TGU: > > In this example, the TGU is configured to use 2 conditions, 2 steps, and > the timer. The goal is to look for one of two patterns which are generated > from TPDM, giving priority to one, and then generate a trigger once the > timer reaches a certain value. In other words, two conditions are used > for the first step to look for the two patterns, where the one with the > highest priority is used in the first condition. Then, in the second step, > the timer is enabled and set to be compared to the given value at each > clock cycle. These steps are better shown below. > > > |-----------------| > | | > | TPDM | > | | > |-----------------| > | > | > --- --- --- --- --- --- --- --- --- --- --- --- --- --- --- --- ------ > | | | > | | |--------------------| | > | |---- ---> | | Go to next steps | | > | | | |--- ---> | Enable timer | | > | | v | | | | > | | |-----------------| | |--------------------| | > | | | | Yes | | | > | | | inputs==0xB | ----->| | <-------- | > | | | | | | No | | > | No | |-----------------| | v | | > | | | | |-----------------| | | > | | | | | | | | > | | | | | timer>=3 |-- | > | | v | | | | > | | |-----------------| | |-----------------| | > | | | | Yes | | | > | |--- | inputs==0xA | ----->| | Yes | > | | | | | > | |-----------------| v | > | |-----------------| | > | | | | > | | Trigger | | > | | | | > | |-----------------| | > | TGU | | > |--- --- --- --- --- --- --- --- --- --- --- --- --- --- |--- --- -- | > | > v > |-----------------| > |The controllers | > |which will use | > |triggers further | > |-----------------| > > steps: > 1. Reset TGU /*it will disable tgu and reset dataset*/ > - echo 1 > /sys/bus/coresight/devices/<tgu-name>/reset_tgu > > 2. Set the pattern match for priority0 to 0xA = 0b1010 and for > priority 1 to 0xB = 0b1011. > - echo 0x11113232 > /sys/bus/coresight/devices/<tgu-name>/step0_priority0/reg0 > - echo 0x11113233 > /sys/bus/coresight/devices/<tgu-name>/step0_priority1/reg0 > > Note: > Bit distribution diagram for each priority register > |-------------------------------------------------------------------| > | Bits | Field Nam | Description | > |-------------------------------------------------------------------| > | | | 00 = bypass for OR output | > | 29:28 | SEL_BIT7_TYPE2 | 01 = bypass for AND output | > | | | 10 = sense input '0' is true| > | | | 11 = sense input '1' is true| > |-------------------------------------------------------------------| > | | | 00 = bypass for OR output | > | 25:24 | SEL_BIT6_TYPE2 | 01 = bypass for AND output | > | | | 10 = sense input '0' is true| > | | | 11 = sense input '1' is true| > |-------------------------------------------------------------------| > | | | 00 = bypass for OR output | > | 21:20 | SEL_BIT5_TYPE2 | 01 = bypass for AND output | > | | | 10 = sense input '0' is true| > | | | 11 = sense input '1' is true| > |-------------------------------------------------------------------| > | | | 00 = bypass for OR output | > | 17:16 | SEL_BIT4_TYPE2 | 01 = bypass for AND output | > | | | 10 = sense input '0' is true| > | | | 11 = sense input '1' is true| > |-------------------------------------------------------------------| > | | | 00 = bypass for OR output | > | 13:12 | SEL_BIT3_TYPE2 | 01 = bypass for AND output | > | | | 10 = sense input '0' is true| > | | | 11 = sense input '1' is true| > |-------------------------------------------------------------------| > | | | 00 = bypass for OR output | > | 9:8 | SEL_BIT2_TYPE2 | 01 = bypass for AND output | > | | | 10 = sense input '0' is true| > | | | 11 = sense input '1' is true| > |-------------------------------------------------------------------| > | | | 00 = bypass for OR output | > | 5:4 | SEL_BIT1_TYPE2 | 01 = bypass for AND output | > | | | 10 = sense input '0' is true| > | | | 11 = sense input '1' is true| > |-------------------------------------------------------------------| > | | | 00 = bypass for OR output | > | 1:0 | SEL_BIT0_TYPE2 | 01 = bypass for AND output | > | | | 10 = sense input '0' is true| > | | | 11 = sense input '1' is true| > |-------------------------------------------------------------------| > These bits are used to identify the signals we want to sense, with > a maximum signal number of 140. For example, to sense the signal > 0xA (binary 1010), we set the value of bits 0 to 13 to 3232, which > represents 1010. The remaining bits are set to 1, as we want to use > AND gate to summarize all the signals we want to sense here. For > rising or falling edge detection of any input to the priority, set > the remaining bits to 0 to use an OR gate. > > 3. look for the pattern for priority_i i=0,1. > - echo 0x3 > /sys/bus/coresight/devices/<tgu-name>/step0_condition_decode/reg0 > - echo 0x30 > /sys/bus/coresight/devices/<tgu-name>/step0_condition_decode/reg1 > > |-------------------------------------------------------------------------------| > | Bits | Field Nam | Description | > |-------------------------------------------------------------------------------| > | | |For each decoded condition, this | > | 24 | NOT |inverts the output. If the condition | > | | |decodes to true, and the NOT field | > | | |is '1', then the output is NOT true. | > |-------------------------------------------------------------------------------| > | | |When '1' the output from the associated| > | 21 | BC0_COMP_ACTIVE |comparator will be actively included in| > | | |the decoding of this particular | > | | |condition. | > |-------------------------------------------------------------------------------| > | | |When '1' the output from the associated| > | | |comparator will need to be 1 to affect | > | 20 | BC0_COMP_HIGH |the decoding of this condition. | > | | |Conversely, a '0' here requires a '0' | > | | |from the comparator | > |-------------------------------------------------------------------------------| > | | |When '1' the output from the associated| > | 17 | |comparator will be actively included in| > | | TC0_COMP_ACTIVE |the decoding of this particular | > | | |condition. | > |-------------------------------------------------------------------------------| > | | |When '1' the output from the associated| > | | |comparator will need to be 1 to affect | > | 16 | TC0_COMP_HIGH |the decoding of this particular | > | | |condition.Conversely, a 0 here | > | | |requires a '0' from the comparator | > |-------------------------------------------------------------------------------| > | | |When '1' the output from Priority_n | > | | |OR logic will be actively | > | 4n+3 | Priority_n_OR_ACTIVE|included in the decoding of | > | | (n=0,1,2,3) |this particular condition. | > | | | | > |-------------------------------------------------------------------------------| > | | |When '1' the output from Priority_n | > | | |will need to be '1' to affect the | > | 4n+2 | Priority_n_OR_HIGH |decoding of this particular | > | | (n=0,1,2,3) |condition. Conversely, a '0' here | > | | |requires a '0' from Priority_n OR logic| > |-------------------------------------------------------------------------------| > | | |When '1' the output from Priority_n | > | | |AND logic will be actively | > | 4n+1 |Priority_n_AND_ACTIVE|included in the decoding of this | > | | (n=0,1,2,3) |particular condition. | > | | | | > |-------------------------------------------------------------------------------| > | | |When '1' the output from Priority_n | > | | |AND logic will need to be '1' to | > | 4n | Priority_n_AND_HIGH |affect the decoding of this | > | | (n=0,1,2,3) |particular condition. Conversely, | > | | |a '0' here requires a '0' from | > | | |Priority_n AND logic. | > |-------------------------------------------------------------------------------| > Since we use `priority_0` and `priority_1` with an AND output in step 2, we set `0x3` > and `0x30` here to activate them. > > 4. Set NEXT_STEP = 1 and TC0_ENABLE = 1 so that when the conditions > are met then the next step will be step 1 and the timer will be enabled. > - echo 0x20008 > /sys/bus/coresight/devices/<tgu-name>/step0_condition_select/reg0 > - echo 0x20008 > /sys/bus/coresight/devices/<tgu-name>/step0_condition_select/reg1 > > |-----------------------------------------------------------------------------| > | Bits | Field Nam | Description | > |-----------------------------------------------------------------------------| > | | |This field defines the next step the | > | 18:17 | NEXT_STEP |TGU will 'goto' for the associated | > | | |Condition and Step. | > |-----------------------------------------------------------------------------| > | | |For each possible output trigger | > | 13 | TRIGGER |available, set a '1' if you want | > | | |the trigger to go active for the | > | | |associated condition and Step. | > |-----------------------------------------------------------------------------| > | | |This will cause BC0 to increment if the| > | 9 | BC0_INC |associated Condition is decoded for | > | | |this step. | > |-----------------------------------------------------------------------------| > | | |This will cause BC0 to decrement if the| > | 8 | BC0_DEC |associated Condition is decoded for | > | | |this step. | > |-----------------------------------------------------------------------------| > | | |This will clear BC0 count value to 0 if| > | 7 | BC0_CLEAR |the associated Condition is decoded | > | | |for this step. | > |-----------------------------------------------------------------------------| > | | |This will cause TC0 to increment until | > | 3 | TC0_ENABLE |paused or cleared if the associated | > | | |Condition is decoded for this step. | > |-----------------------------------------------------------------------------| > | | |This will cause TC0 to pause until | > | 2 | TC0_PAUSE |enabled if the associated Condition | > | | |is decoded for this step. | > |-----------------------------------------------------------------------------| > | | |This will clear TC0 count value to 0 | > | 1 | TC0_CLEAR |if the associated Condition is | > | | |decoded for this step. | > |-----------------------------------------------------------------------------| > | | |This will set the done signal to the | > | 0 | DONE |TGU FSM if the associated Condition | > | | |is decoded for this step. | > |-----------------------------------------------------------------------------| > Based on the distribution diagram, we set `0x20008` for `priority0` and `priority1` to > achieve "jump to step 1 and enable TC0" once the signal is sensed. > > 5. activate the timer comparison for this step. > - echo 0x30000 > /sys/bus/coresight/devices/<tgu-name>/step1_condition_decode/reg0 > > |-------------------------------------------------------------------------------| > | | |When '1' the output from the associated| > | 17 | |comparator will be actively included in| > | | TC0_COMP_ACTIVE |the decoding of this particular | > | | |condition. | > |-------------------------------------------------------------------------------| > | | |When '1' the output from the associated| > | | |comparator will need to be 1 to affect | > | 16 | TC0_COMP_HIGH |the decoding of this particular | > | | |condition.Conversely, a 0 here | > | | |requires a '0' from the comparator | > |-------------------------------------------------------------------------------| > Accroding to the decode distribution diagram , we give 0x30000 here to set 16th&17th bit > to enable timer comparison. > > 6. Set the NEXT_STEP = 0 and TC0_PAUSE = 1 and TC0_CLEAR = 1 once the timer > has reached the given value. > - echo 0x6 > /sys/bus/coresight/devices/<tgu-name>/step1_condition_select/reg0 > > 7. Enable Trigger 0 for TGU when the condition 0 is met in step1, > i.e. when the timer reaches 3. > - echo 0x2000 > /sys/bus/coresight/devices/<tgu-name>/step1_condition_select/default > > Note: > 1. 'default' register allows for establishing the resultant action for > the default condition > > 2. Trigger:For each possible output trigger available from > the Design document, there are three triggers: interrupts, CTI, > and Cross-TGU mapping.All three triggers can occur, but > the choice of which trigger to use depends on the user's > needs. > > 8. Compare the timer to 3 in step 1. > - echo 0x3 > /sys/bus/coresight/devices/<tgu-name>/step1_timer/reg0 > > 9. enale tgu > - echo 1 > /sys/bus/coresight/devices/<tgu-name>/enable_tgu > If this is version 3 - where is the list of differences from versions 1 - 2? > Songwei Chai (7): > dt-bindings: arm: Add support for Coresight TGU trace > coresight: Add coresight TGU driver > coresight-tgu: Add signal priority support > coresight-tgu: Add TGU decode support > coresight-tgu: add support to configure next action > coresight-tgu: add timer/counter functionality for TGU > coresight-tgu: add reset node to initialize > > .../testing/sysfs-bus-coresight-devices-tgu | 51 ++ > .../bindings/arm/qcom,coresight-tgu.yaml | 135 ++++ > drivers/hwtracing/coresight/Kconfig | 11 + > drivers/hwtracing/coresight/Makefile | 1 + > drivers/hwtracing/coresight/coresight-tgu.c | 669 ++++++++++++++++++ > drivers/hwtracing/coresight/coresight-tgu.h | 242 +++++++ > 6 files changed, 1109 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight-devices-tgu > create mode 100644 Documentation/devicetree/bindings/arm/qcom,coresight-tgu.yaml > create mode 100644 drivers/hwtracing/coresight/coresight-tgu.c > create mode 100644 drivers/hwtracing/coresight/coresight-tgu.h > Regards Mike -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

5 months, 3 weeks

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight