CoreSight March 2025

coresight@lists.linaro.org

8 participants
57 discussions

[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

1 month, 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

1 month, 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

1 month, 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

1 month, 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

1 month, 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

1 month, 3 weeks

Re: [PATCH v3 2/7] coresight: Add coresight TGU driver

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 driver to support Coresight device TGU (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. Add probe/enable/disable functions for tgu. > > 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 | 9 + > drivers/hwtracing/coresight/Kconfig | 11 + > drivers/hwtracing/coresight/Makefile | 1 + > drivers/hwtracing/coresight/coresight-tgu.c | 218 ++++++++++++++++++ > drivers/hwtracing/coresight/coresight-tgu.h | 36 +++ > 5 files changed, 275 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight-devices-tgu > create mode 100644 drivers/hwtracing/coresight/coresight-tgu.c > create mode 100644 drivers/hwtracing/coresight/coresight-tgu.h > > diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tgu b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tgu > new file mode 100644 > index 000000000000..741bc9fd9df5 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tgu > @@ -0,0 +1,9 @@ > +What: /sys/bus/coresight/devices/<tgu-name>/enable_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: > + (RW) Set/Get the enable/disable status of TGU > + Accepts only one of the 2 values - 0 or 1. > + 0 : disable TGU. > + 1 : enable TGU. > diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig > index 06f0a7594169..3fe59c745dd4 100644 > --- a/drivers/hwtracing/coresight/Kconfig > +++ b/drivers/hwtracing/coresight/Kconfig > @@ -247,4 +247,15 @@ config CORESIGHT_DUMMY > > To compile this driver as a module, choose M here: the module will be > called coresight-dummy. > + > +config CORESIGHT_TGU > + tristate "CoreSight Trigger Generation Unit driver" > + help > + This driver provides support for Trigger Generation Unit that is > + used to detect patterns or sequences on a given set of signals. > + TGU is used to monitor a particular bus within a given region to > + detect illegal transaction sequences or slave responses. It is also > + used to monitor a data stream to detect protocol violations and to > + provide a trigger point for centering data around a specific event > + within the trace data buffer. > endif > diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile > index 4ba478211b31..7c2b9e9cf1cd 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_TGU) += coresight-tgu.o > diff --git a/drivers/hwtracing/coresight/coresight-tgu.c b/drivers/hwtracing/coresight/coresight-tgu.c > new file mode 100644 > index 000000000000..da4c04ac1097 > --- /dev/null > +++ b/drivers/hwtracing/coresight/coresight-tgu.c > @@ -0,0 +1,218 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2024-2025 Qualcomm Innovation Center, Inc. All rights reserved. > + */ > + > +#include <linux/amba/bus.h> > +#include <linux/coresight.h> > +#include <linux/device.h> > +#include <linux/err.h> > +#include <linux/io.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/of.h> > + > +#include "coresight-priv.h" > +#include "coresight-tgu.h" > + > +DEFINE_CORESIGHT_DEVLIST(tgu_devs, "tgu"); > + > +static void tgu_write_all_hw_regs(struct tgu_drvdata *drvdata) > +{ > + CS_UNLOCK(drvdata->base); > + /* Enable TGU to program the triggers */ > + tgu_writel(drvdata, 1, TGU_CONTROL); > + CS_LOCK(drvdata->base); > +} > + > +static int tgu_enable(struct coresight_device *csdev, enum cs_mode mode, > + void *data) > +{ > + struct tgu_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > + > + spin_lock(&drvdata->spinlock); > + > + if (drvdata->enable) { > + spin_unlock(&drvdata->spinlock); > + return -EBUSY; > + } > + tgu_write_all_hw_regs(drvdata); > + drvdata->enable = true; > + > + spin_unlock(&drvdata->spinlock); > + return 0; > +} > + > +static int tgu_disable(struct coresight_device *csdev, void *data) > +{ > + struct tgu_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > + > + spin_lock(&drvdata->spinlock); > + > + if (!drvdata->enable) { Could simplify by changing logic here - if (enable) { do disable stuff } and have a single return point > + spin_unlock(&drvdata->spinlock); > + return 0; > + } > + > + CS_UNLOCK(drvdata->base); > + tgu_writel(drvdata, 0, TGU_CONTROL); > + CS_LOCK(drvdata->base); > + > + drvdata->enable = false; > + spin_unlock(&drvdata->spinlock); > + return 0; > +} > + > +static ssize_t enable_tgu_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + bool enabled; > + > + struct tgu_drvdata *drvdata = dev_get_drvdata(dev->parent); > + > + spin_lock(&drvdata->spinlock); > + enabled = drvdata->enable; > + spin_unlock(&drvdata->spinlock); > + > + return sprintf(buf, "%d\n", enabled); sysfs_emit() should be used here. > +} > + > +/* enable_tgu_store - Configure Trace and Gating Unit (TGU) triggers. */ > +static ssize_t enable_tgu_store(struct device *dev, > + struct device_attribute *attr, const char *buf, > + size_t size) > +{ > + int ret = 0; > + unsigned long val; > + struct tgu_drvdata *drvdata = dev_get_drvdata(dev->parent); > + > + ret = kstrtoul(buf, 0, &val); > + if (ret) > + return ret; > + > + if (val) { > + ret = pm_runtime_resume_and_get(dev->parent); > + if (ret) > + return ret; > + ret = tgu_enable(drvdata->csdev, CS_MODE_SYSFS, NULL); > + if (ret) > + pm_runtime_put(dev->parent); > + } else { > + ret = tgu_disable(drvdata->csdev, NULL); > + if (!ret) redundant - tgu_disable always returns 0. > + pm_runtime_put(dev->parent); > + } > + > + if (ret) > + return ret; > + return size; > +} > +static DEVICE_ATTR_RW(enable_tgu); > + > +static const struct coresight_ops_helper tgu_helper_ops = { > + .enable = tgu_enable, > + .disable = tgu_disable, > +}; > + > +static const struct coresight_ops tgu_ops = { > + .helper_ops = &tgu_helper_ops, > +}; > + > +static struct attribute *tgu_common_attrs[] = { > + &dev_attr_enable_tgu.attr, > + NULL, > +}; > + > +static const struct attribute_group tgu_common_grp = { > + .attrs = tgu_common_attrs, > + NULL, > +}; > + > +static const struct attribute_group *tgu_attr_groups[] = { > + &tgu_common_grp, > + NULL, > +}; > + > +static int tgu_probe(struct amba_device *adev, const struct amba_id *id) > +{ > + int ret = 0; > + struct device *dev = &adev->dev; > + struct coresight_desc desc = { 0 }; > + struct coresight_platform_data *pdata; > + struct tgu_drvdata *drvdata; > + > + desc.name = coresight_alloc_device_name(&tgu_devs, dev); > + if (!desc.name) > + return -ENOMEM; > + > + pdata = coresight_get_platform_data(dev); > + if (IS_ERR(pdata)) > + return PTR_ERR(pdata); > + > + adev->dev.platform_data = pdata; > + > + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); > + if (!drvdata) > + return -ENOMEM; > + > + drvdata->dev = &adev->dev; > + dev_set_drvdata(dev, drvdata); > + > + drvdata->base = devm_ioremap_resource(dev, &adev->res); > + if (!drvdata->base) > + return -ENOMEM; > + > + spin_lock_init(&drvdata->spinlock); > + > + drvdata->enable = false; > + desc.type = CORESIGHT_DEV_TYPE_HELPER; > + desc.pdata = adev->dev.platform_data; > + desc.dev = &adev->dev; > + desc.ops = &tgu_ops; > + desc.groups = tgu_attr_groups; > + > + drvdata->csdev = coresight_register(&desc); > + if (IS_ERR(drvdata->csdev)) { > + ret = PTR_ERR(drvdata->csdev); > + goto err; > + } > + > + pm_runtime_put(&adev->dev); > + return 0; > +err: > + pm_runtime_put(&adev->dev); > + return ret; > +} > + > +static void tgu_remove(struct amba_device *adev) > +{ > + struct tgu_drvdata *drvdata = dev_get_drvdata(&adev->dev); > + > + coresight_unregister(drvdata->csdev); > +} > + > +static const struct amba_id tgu_ids[] = { > + { > + .id = 0x000f0e00, > + .mask = 0x000fffff, > + .data = "TGU", > + }, > + { 0, 0, NULL }, > +}; > + > +MODULE_DEVICE_TABLE(amba, tgu_ids); > + > +static struct amba_driver tgu_driver = { > + .drv = { > + .name = "coresight-tgu", > + .suppress_bind_attrs = true, > + }, > + .probe = tgu_probe, > + .remove = tgu_remove, > + .id_table = tgu_ids, > +}; > + > +module_amba_driver(tgu_driver); > + > +MODULE_LICENSE("GPL"); > +MODULE_DESCRIPTION("CoreSight TGU driver"); > diff --git a/drivers/hwtracing/coresight/coresight-tgu.h b/drivers/hwtracing/coresight/coresight-tgu.h > new file mode 100644 > index 000000000000..380686f94130 > --- /dev/null > +++ b/drivers/hwtracing/coresight/coresight-tgu.h > @@ -0,0 +1,36 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (c) 2024-2025 Qualcomm Innovation Center, Inc. All rights reserved. > + */ > + > +#ifndef _CORESIGHT_TGU_H > +#define _CORESIGHT_TGU_H > + > +/* Register addresses */ > +#define TGU_CONTROL 0x0000 > + > +/* Register read/write */ > +#define tgu_writel(drvdata, val, off) __raw_writel((val), drvdata->base + off) > +#define tgu_readl(drvdata, off) __raw_readl(drvdata->base + off) > + > +/** > + * struct tgu_drvdata - Data structure for a TGU (Trigger Generator Unit) device > + * @base: Memory-mapped base address of the TGU device > + * @dev: Pointer to the associated device structure > + * @csdev: Pointer to the associated coresight device > + * @spinlock: Spinlock for handling concurrent access > + * @enable: Flag indicating whether the TGU device is enabled > + * > + * This structure defines the data associated with a TGU device, including its base > + * address, device pointers, clock, spinlock for synchronization, trigger data pointers, I don't see any trigger data pointers or limits here. Comment on what is there, if more is added later, expand the comment later. > + * maximum limits for various trigger-related parameters, and enable status. > + */ > +struct tgu_drvdata { > + void __iomem *base; > + struct device *dev; > + struct coresight_device *csdev; > + spinlock_t spinlock; > + bool enable; > +}; > + > +#endif > Regards Mike -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

1 month, 3 weeks

Re: [PATCH v6 2/9] coresight-etm4x: change etmv4_drvdata spinlock type to raw_spinlock_t

by Suzuki K Poulose

Hi Levi On 06/03/2025 12:11, Yeoreum Yun wrote: > In coresight-etm4x drivers, etmv4_drvdata->spinlock can be held during > __schedule() by perf_event_task_sched_out()/in(). > > Since etmv4_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. > > To address this, change type etmv4_drvdata->spinlock > in coresight-etm4x drivers, which can be called > by perf_event_task_sched_out()/in(), from spinlock_t to raw_spinlock_t. > > Signed-off-by: Yeoreum Yun <yeoreum.yun(a)arm.com> > Reviewed-by: James Clark <james.clark(a)linaro.org> > --- > .../hwtracing/coresight/coresight-config.c | 8 +- > .../hwtracing/coresight/coresight-config.h | 2 +- > .../coresight/coresight-etm4x-core.c | 18 +- > .../coresight/coresight-etm4x-sysfs.c | 250 +++++++++--------- > drivers/hwtracing/coresight/coresight-etm4x.h | 2 +- > 5 files changed, 140 insertions(+), 140 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-config.c b/drivers/hwtracing/coresight/coresight-config.c > index 4723bf7402a2..4f72ae71b696 100644 > --- a/drivers/hwtracing/coresight/coresight-config.c > +++ b/drivers/hwtracing/coresight/coresight-config.c > @@ -76,10 +76,10 @@ static int cscfg_set_on_enable(struct cscfg_feature_csdev *feat_csdev) > unsigned long flags; > int i; > > - spin_lock_irqsave(feat_csdev->drv_spinlock, flags); > + raw_spin_lock_irqsave(feat_csdev->drv_spinlock, flags); > for (i = 0; i < feat_csdev->nr_regs; i++) > cscfg_set_reg(&feat_csdev->regs_csdev[i]); > - spin_unlock_irqrestore(feat_csdev->drv_spinlock, flags); > + raw_spin_unlock_irqrestore(feat_csdev->drv_spinlock, flags); > dev_dbg(&feat_csdev->csdev->dev, "Feature %s: %s", > feat_csdev->feat_desc->name, "set on enable"); > return 0; > @@ -91,10 +91,10 @@ static void cscfg_save_on_disable(struct cscfg_feature_csdev *feat_csdev) > unsigned long flags; > int i; > > - spin_lock_irqsave(feat_csdev->drv_spinlock, flags); > + raw_spin_lock_irqsave(feat_csdev->drv_spinlock, flags); > for (i = 0; i < feat_csdev->nr_regs; i++) > cscfg_save_reg(&feat_csdev->regs_csdev[i]); > - spin_unlock_irqrestore(feat_csdev->drv_spinlock, flags); > + raw_spin_unlock_irqrestore(feat_csdev->drv_spinlock, flags); > dev_dbg(&feat_csdev->csdev->dev, "Feature %s: %s", > feat_csdev->feat_desc->name, "save on disable"); > } > diff --git a/drivers/hwtracing/coresight/coresight-config.h b/drivers/hwtracing/coresight/coresight-config.h > index 6ba013975741..b9ebc9fcfb7f 100644 > --- a/drivers/hwtracing/coresight/coresight-config.h > +++ b/drivers/hwtracing/coresight/coresight-config.h > @@ -206,7 +206,7 @@ struct cscfg_feature_csdev { > const struct cscfg_feature_desc *feat_desc; > struct coresight_device *csdev; > struct list_head node; > - spinlock_t *drv_spinlock; > + raw_spinlock_t *drv_spinlock; > int nr_params; > struct cscfg_parameter_csdev *params_csdev; > int nr_regs; The changes above seems to be unrelated etm4x and not called for in the patch description. Is there any reason why this cannot be separated from etm4x changes ? Suzuki > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c > index bb1e80df2914..e5972f16abff 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c > @@ -830,7 +830,7 @@ static int etm4_enable_sysfs(struct coresight_device *csdev, struct coresight_pa > return ret; > } > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > > drvdata->trcid = path->trace_id; > > @@ -849,7 +849,7 @@ static int etm4_enable_sysfs(struct coresight_device *csdev, struct coresight_pa > if (ret) > etm4_release_trace_id(drvdata); > > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > > if (!ret) > dev_dbg(&csdev->dev, "ETM tracing enabled\n"); > @@ -1011,7 +1011,7 @@ static void etm4_disable_sysfs(struct coresight_device *csdev) > * DYING hotplug callback is serviced by the ETM driver. > */ > cpus_read_lock(); > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > > /* > * Executing etm4_disable_hw on the cpu whose ETM is being disabled > @@ -1019,7 +1019,7 @@ static void etm4_disable_sysfs(struct coresight_device *csdev) > */ > smp_call_function_single(drvdata->cpu, etm4_disable_hw, drvdata, 1); > > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > cpus_read_unlock(); > > /* > @@ -1698,13 +1698,13 @@ static int etm4_starting_cpu(unsigned int cpu) > if (!etmdrvdata[cpu]) > return 0; > > - spin_lock(&etmdrvdata[cpu]->spinlock); > + raw_spin_lock(&etmdrvdata[cpu]->spinlock); > if (!etmdrvdata[cpu]->os_unlock) > etm4_os_unlock(etmdrvdata[cpu]); > > if (coresight_get_mode(etmdrvdata[cpu]->csdev)) > etm4_enable_hw(etmdrvdata[cpu]); > - spin_unlock(&etmdrvdata[cpu]->spinlock); > + raw_spin_unlock(&etmdrvdata[cpu]->spinlock); > return 0; > } > > @@ -1713,10 +1713,10 @@ static int etm4_dying_cpu(unsigned int cpu) > if (!etmdrvdata[cpu]) > return 0; > > - spin_lock(&etmdrvdata[cpu]->spinlock); > + raw_spin_lock(&etmdrvdata[cpu]->spinlock); > if (coresight_get_mode(etmdrvdata[cpu]->csdev)) > etm4_disable_hw(etmdrvdata[cpu]); > - spin_unlock(&etmdrvdata[cpu]->spinlock); > + raw_spin_unlock(&etmdrvdata[cpu]->spinlock); > return 0; > } > > @@ -2160,7 +2160,7 @@ static int etm4_probe(struct device *dev) > return -ENOMEM; > } > > - spin_lock_init(&drvdata->spinlock); > + raw_spin_lock_init(&drvdata->spinlock); > > drvdata->cpu = coresight_get_cpu(dev); > if (drvdata->cpu < 0) > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c > index e5216c0f60da..fdd0956fecb3 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c > @@ -175,7 +175,7 @@ static ssize_t reset_store(struct device *dev, > if (kstrtoul(buf, 16, &val)) > return -EINVAL; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > if (val) > config->mode = 0x0; > > @@ -267,7 +267,7 @@ static ssize_t reset_store(struct device *dev, > config->vmid_mask0 = 0x0; > config->vmid_mask1 = 0x0; > > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > > /* for sysfs - only release trace id when resetting */ > etm4_release_trace_id(drvdata); > @@ -301,7 +301,7 @@ static ssize_t mode_store(struct device *dev, > if (kstrtoul(buf, 16, &val)) > return -EINVAL; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > config->mode = val & ETMv4_MODE_ALL; > > if (drvdata->instrp0 == true) { > @@ -438,7 +438,7 @@ static ssize_t mode_store(struct device *dev, > if (config->mode & (ETM_MODE_EXCL_KERN | ETM_MODE_EXCL_USER)) > etm4_config_trace_mode(config); > > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > > return size; > } > @@ -467,14 +467,14 @@ static ssize_t pe_store(struct device *dev, > if (kstrtoul(buf, 16, &val)) > return -EINVAL; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > if (val > drvdata->nr_pe) { > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return -EINVAL; > } > > config->pe_sel = val; > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return size; > } > static DEVICE_ATTR_RW(pe); > @@ -502,7 +502,7 @@ static ssize_t event_store(struct device *dev, > if (kstrtoul(buf, 16, &val)) > return -EINVAL; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > switch (drvdata->nr_event) { > case 0x0: > /* EVENT0, bits[7:0] */ > @@ -523,7 +523,7 @@ static ssize_t event_store(struct device *dev, > default: > break; > } > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return size; > } > static DEVICE_ATTR_RW(event); > @@ -551,7 +551,7 @@ static ssize_t event_instren_store(struct device *dev, > if (kstrtoul(buf, 16, &val)) > return -EINVAL; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > /* start by clearing all instruction event enable bits */ > config->eventctrl1 &= ~TRCEVENTCTL1R_INSTEN_MASK; > switch (drvdata->nr_event) { > @@ -579,7 +579,7 @@ static ssize_t event_instren_store(struct device *dev, > default: > break; > } > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return size; > } > static DEVICE_ATTR_RW(event_instren); > @@ -740,11 +740,11 @@ static ssize_t event_vinst_store(struct device *dev, > if (kstrtoul(buf, 16, &val)) > return -EINVAL; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > val &= TRCVICTLR_EVENT_MASK >> __bf_shf(TRCVICTLR_EVENT_MASK); > config->vinst_ctrl &= ~TRCVICTLR_EVENT_MASK; > config->vinst_ctrl |= FIELD_PREP(TRCVICTLR_EVENT_MASK, val); > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return size; > } > static DEVICE_ATTR_RW(event_vinst); > @@ -772,13 +772,13 @@ static ssize_t s_exlevel_vinst_store(struct device *dev, > if (kstrtoul(buf, 16, &val)) > return -EINVAL; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > /* clear all EXLEVEL_S bits */ > config->vinst_ctrl &= ~TRCVICTLR_EXLEVEL_S_MASK; > /* enable instruction tracing for corresponding exception level */ > val &= drvdata->s_ex_level; > config->vinst_ctrl |= val << __bf_shf(TRCVICTLR_EXLEVEL_S_MASK); > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return size; > } > static DEVICE_ATTR_RW(s_exlevel_vinst); > @@ -807,13 +807,13 @@ static ssize_t ns_exlevel_vinst_store(struct device *dev, > if (kstrtoul(buf, 16, &val)) > return -EINVAL; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > /* clear EXLEVEL_NS bits */ > config->vinst_ctrl &= ~TRCVICTLR_EXLEVEL_NS_MASK; > /* enable instruction tracing for corresponding exception level */ > val &= drvdata->ns_ex_level; > config->vinst_ctrl |= val << __bf_shf(TRCVICTLR_EXLEVEL_NS_MASK); > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return size; > } > static DEVICE_ATTR_RW(ns_exlevel_vinst); > @@ -847,9 +847,9 @@ static ssize_t addr_idx_store(struct device *dev, > * Use spinlock to ensure index doesn't change while it gets > * dereferenced multiple times within a spinlock block elsewhere. > */ > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > config->addr_idx = val; > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return size; > } > static DEVICE_ATTR_RW(addr_idx); > @@ -863,7 +863,7 @@ static ssize_t addr_instdatatype_show(struct device *dev, > struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); > struct etmv4_config *config = &drvdata->config; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > idx = config->addr_idx; > val = FIELD_GET(TRCACATRn_TYPE_MASK, config->addr_acc[idx]); > len = scnprintf(buf, PAGE_SIZE, "%s\n", > @@ -871,7 +871,7 @@ static ssize_t addr_instdatatype_show(struct device *dev, > (val == TRCACATRn_TYPE_DATA_LOAD_ADDR ? "data_load" : > (val == TRCACATRn_TYPE_DATA_STORE_ADDR ? "data_store" : > "data_load_store"))); > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return len; > } > > @@ -889,13 +889,13 @@ static ssize_t addr_instdatatype_store(struct device *dev, > if (sscanf(buf, "%s", str) != 1) > return -EINVAL; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > idx = config->addr_idx; > if (!strcmp(str, "instr")) > /* TYPE, bits[1:0] */ > config->addr_acc[idx] &= ~TRCACATRn_TYPE_MASK; > > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return size; > } > static DEVICE_ATTR_RW(addr_instdatatype); > @@ -910,14 +910,14 @@ static ssize_t addr_single_show(struct device *dev, > struct etmv4_config *config = &drvdata->config; > > idx = config->addr_idx; > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > if (!(config->addr_type[idx] == ETM_ADDR_TYPE_NONE || > config->addr_type[idx] == ETM_ADDR_TYPE_SINGLE)) { > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return -EPERM; > } > val = (unsigned long)config->addr_val[idx]; > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return scnprintf(buf, PAGE_SIZE, "%#lx\n", val); > } > > @@ -933,17 +933,17 @@ static ssize_t addr_single_store(struct device *dev, > if (kstrtoul(buf, 16, &val)) > return -EINVAL; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > idx = config->addr_idx; > if (!(config->addr_type[idx] == ETM_ADDR_TYPE_NONE || > config->addr_type[idx] == ETM_ADDR_TYPE_SINGLE)) { > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return -EPERM; > } > > config->addr_val[idx] = (u64)val; > config->addr_type[idx] = ETM_ADDR_TYPE_SINGLE; > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return size; > } > static DEVICE_ATTR_RW(addr_single); > @@ -957,23 +957,23 @@ static ssize_t addr_range_show(struct device *dev, > struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); > struct etmv4_config *config = &drvdata->config; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > idx = config->addr_idx; > if (idx % 2 != 0) { > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return -EPERM; > } > if (!((config->addr_type[idx] == ETM_ADDR_TYPE_NONE && > config->addr_type[idx + 1] == ETM_ADDR_TYPE_NONE) || > (config->addr_type[idx] == ETM_ADDR_TYPE_RANGE && > config->addr_type[idx + 1] == ETM_ADDR_TYPE_RANGE))) { > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return -EPERM; > } > > val1 = (unsigned long)config->addr_val[idx]; > val2 = (unsigned long)config->addr_val[idx + 1]; > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return scnprintf(buf, PAGE_SIZE, "%#lx %#lx\n", val1, val2); > } > > @@ -996,10 +996,10 @@ static ssize_t addr_range_store(struct device *dev, > if (val1 > val2) > return -EINVAL; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > idx = config->addr_idx; > if (idx % 2 != 0) { > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return -EPERM; > } > > @@ -1007,7 +1007,7 @@ static ssize_t addr_range_store(struct device *dev, > config->addr_type[idx + 1] == ETM_ADDR_TYPE_NONE) || > (config->addr_type[idx] == ETM_ADDR_TYPE_RANGE && > config->addr_type[idx + 1] == ETM_ADDR_TYPE_RANGE))) { > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return -EPERM; > } > > @@ -1024,7 +1024,7 @@ static ssize_t addr_range_store(struct device *dev, > exclude = config->mode & ETM_MODE_EXCLUDE; > etm4_set_mode_exclude(drvdata, exclude ? true : false); > > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return size; > } > static DEVICE_ATTR_RW(addr_range); > @@ -1038,17 +1038,17 @@ static ssize_t addr_start_show(struct device *dev, > struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); > struct etmv4_config *config = &drvdata->config; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > idx = config->addr_idx; > > if (!(config->addr_type[idx] == ETM_ADDR_TYPE_NONE || > config->addr_type[idx] == ETM_ADDR_TYPE_START)) { > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return -EPERM; > } > > val = (unsigned long)config->addr_val[idx]; > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return scnprintf(buf, PAGE_SIZE, "%#lx\n", val); > } > > @@ -1064,22 +1064,22 @@ static ssize_t addr_start_store(struct device *dev, > if (kstrtoul(buf, 16, &val)) > return -EINVAL; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > idx = config->addr_idx; > if (!drvdata->nr_addr_cmp) { > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return -EINVAL; > } > if (!(config->addr_type[idx] == ETM_ADDR_TYPE_NONE || > config->addr_type[idx] == ETM_ADDR_TYPE_START)) { > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return -EPERM; > } > > config->addr_val[idx] = (u64)val; > config->addr_type[idx] = ETM_ADDR_TYPE_START; > config->vissctlr |= BIT(idx); > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return size; > } > static DEVICE_ATTR_RW(addr_start); > @@ -1093,17 +1093,17 @@ static ssize_t addr_stop_show(struct device *dev, > struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); > struct etmv4_config *config = &drvdata->config; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > idx = config->addr_idx; > > if (!(config->addr_type[idx] == ETM_ADDR_TYPE_NONE || > config->addr_type[idx] == ETM_ADDR_TYPE_STOP)) { > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return -EPERM; > } > > val = (unsigned long)config->addr_val[idx]; > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return scnprintf(buf, PAGE_SIZE, "%#lx\n", val); > } > > @@ -1119,22 +1119,22 @@ static ssize_t addr_stop_store(struct device *dev, > if (kstrtoul(buf, 16, &val)) > return -EINVAL; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > idx = config->addr_idx; > if (!drvdata->nr_addr_cmp) { > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return -EINVAL; > } > if (!(config->addr_type[idx] == ETM_ADDR_TYPE_NONE || > config->addr_type[idx] == ETM_ADDR_TYPE_STOP)) { > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return -EPERM; > } > > config->addr_val[idx] = (u64)val; > config->addr_type[idx] = ETM_ADDR_TYPE_STOP; > config->vissctlr |= BIT(idx + 16); > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return size; > } > static DEVICE_ATTR_RW(addr_stop); > @@ -1148,14 +1148,14 @@ static ssize_t addr_ctxtype_show(struct device *dev, > struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); > struct etmv4_config *config = &drvdata->config; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > idx = config->addr_idx; > /* CONTEXTTYPE, bits[3:2] */ > val = FIELD_GET(TRCACATRn_CONTEXTTYPE_MASK, config->addr_acc[idx]); > len = scnprintf(buf, PAGE_SIZE, "%s\n", val == ETM_CTX_NONE ? "none" : > (val == ETM_CTX_CTXID ? "ctxid" : > (val == ETM_CTX_VMID ? "vmid" : "all"))); > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return len; > } > > @@ -1173,7 +1173,7 @@ static ssize_t addr_ctxtype_store(struct device *dev, > if (sscanf(buf, "%s", str) != 1) > return -EINVAL; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > idx = config->addr_idx; > if (!strcmp(str, "none")) > /* start by clearing context type bits */ > @@ -1200,7 +1200,7 @@ static ssize_t addr_ctxtype_store(struct device *dev, > if (drvdata->numvmidc) > config->addr_acc[idx] |= TRCACATRn_CONTEXTTYPE_VMID; > } > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return size; > } > static DEVICE_ATTR_RW(addr_ctxtype); > @@ -1214,11 +1214,11 @@ static ssize_t addr_context_show(struct device *dev, > struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); > struct etmv4_config *config = &drvdata->config; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > idx = config->addr_idx; > /* context ID comparator bits[6:4] */ > val = FIELD_GET(TRCACATRn_CONTEXT_MASK, config->addr_acc[idx]); > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return scnprintf(buf, PAGE_SIZE, "%#lx\n", val); > } > > @@ -1239,12 +1239,12 @@ static ssize_t addr_context_store(struct device *dev, > drvdata->numcidc : drvdata->numvmidc)) > return -EINVAL; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > idx = config->addr_idx; > /* clear context ID comparator bits[6:4] */ > config->addr_acc[idx] &= ~TRCACATRn_CONTEXT_MASK; > config->addr_acc[idx] |= val << __bf_shf(TRCACATRn_CONTEXT_MASK); > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return size; > } > static DEVICE_ATTR_RW(addr_context); > @@ -1258,10 +1258,10 @@ static ssize_t addr_exlevel_s_ns_show(struct device *dev, > struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); > struct etmv4_config *config = &drvdata->config; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > idx = config->addr_idx; > val = FIELD_GET(TRCACATRn_EXLEVEL_MASK, config->addr_acc[idx]); > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return scnprintf(buf, PAGE_SIZE, "%#lx\n", val); > } > > @@ -1280,12 +1280,12 @@ static ssize_t addr_exlevel_s_ns_store(struct device *dev, > if (val & ~(TRCACATRn_EXLEVEL_MASK >> __bf_shf(TRCACATRn_EXLEVEL_MASK))) > return -EINVAL; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > idx = config->addr_idx; > /* clear Exlevel_ns & Exlevel_s bits[14:12, 11:8], bit[15] is res0 */ > config->addr_acc[idx] &= ~TRCACATRn_EXLEVEL_MASK; > config->addr_acc[idx] |= val << __bf_shf(TRCACATRn_EXLEVEL_MASK); > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return size; > } > static DEVICE_ATTR_RW(addr_exlevel_s_ns); > @@ -1308,7 +1308,7 @@ static ssize_t addr_cmp_view_show(struct device *dev, > int size = 0; > bool exclude = false; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > idx = config->addr_idx; > addr_v = config->addr_val[idx]; > addr_ctrl = config->addr_acc[idx]; > @@ -1323,7 +1323,7 @@ static ssize_t addr_cmp_view_show(struct device *dev, > } > exclude = config->viiectlr & BIT(idx / 2 + 16); > } > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > if (addr_type) { > size = scnprintf(buf, PAGE_SIZE, "addr_cmp[%i] %s %#lx", idx, > addr_type_names[addr_type], addr_v); > @@ -1367,9 +1367,9 @@ static ssize_t vinst_pe_cmp_start_stop_store(struct device *dev, > if (!drvdata->nr_pe_cmp) > return -EINVAL; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > config->vipcssctlr = val; > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return size; > } > static DEVICE_ATTR_RW(vinst_pe_cmp_start_stop); > @@ -1403,9 +1403,9 @@ static ssize_t seq_idx_store(struct device *dev, > * Use spinlock to ensure index doesn't change while it gets > * dereferenced multiple times within a spinlock block elsewhere. > */ > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > config->seq_idx = val; > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return size; > } > static DEVICE_ATTR_RW(seq_idx); > @@ -1449,10 +1449,10 @@ static ssize_t seq_event_show(struct device *dev, > struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); > struct etmv4_config *config = &drvdata->config; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > idx = config->seq_idx; > val = config->seq_ctrl[idx]; > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return scnprintf(buf, PAGE_SIZE, "%#lx\n", val); > } > > @@ -1468,11 +1468,11 @@ static ssize_t seq_event_store(struct device *dev, > if (kstrtoul(buf, 16, &val)) > return -EINVAL; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > idx = config->seq_idx; > /* Seq control has two masks B[15:8] F[7:0] */ > config->seq_ctrl[idx] = val & 0xFFFF; > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return size; > } > static DEVICE_ATTR_RW(seq_event); > @@ -1536,9 +1536,9 @@ static ssize_t cntr_idx_store(struct device *dev, > * Use spinlock to ensure index doesn't change while it gets > * dereferenced multiple times within a spinlock block elsewhere. > */ > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > config->cntr_idx = val; > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return size; > } > static DEVICE_ATTR_RW(cntr_idx); > @@ -1552,10 +1552,10 @@ static ssize_t cntrldvr_show(struct device *dev, > struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); > struct etmv4_config *config = &drvdata->config; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > idx = config->cntr_idx; > val = config->cntrldvr[idx]; > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return scnprintf(buf, PAGE_SIZE, "%#lx\n", val); > } > > @@ -1573,10 +1573,10 @@ static ssize_t cntrldvr_store(struct device *dev, > if (val > ETM_CNTR_MAX_VAL) > return -EINVAL; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > idx = config->cntr_idx; > config->cntrldvr[idx] = val; > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return size; > } > static DEVICE_ATTR_RW(cntrldvr); > @@ -1590,10 +1590,10 @@ static ssize_t cntr_val_show(struct device *dev, > struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); > struct etmv4_config *config = &drvdata->config; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > idx = config->cntr_idx; > val = config->cntr_val[idx]; > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return scnprintf(buf, PAGE_SIZE, "%#lx\n", val); > } > > @@ -1611,10 +1611,10 @@ static ssize_t cntr_val_store(struct device *dev, > if (val > ETM_CNTR_MAX_VAL) > return -EINVAL; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > idx = config->cntr_idx; > config->cntr_val[idx] = val; > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return size; > } > static DEVICE_ATTR_RW(cntr_val); > @@ -1628,10 +1628,10 @@ static ssize_t cntr_ctrl_show(struct device *dev, > struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); > struct etmv4_config *config = &drvdata->config; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > idx = config->cntr_idx; > val = config->cntr_ctrl[idx]; > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return scnprintf(buf, PAGE_SIZE, "%#lx\n", val); > } > > @@ -1647,10 +1647,10 @@ static ssize_t cntr_ctrl_store(struct device *dev, > if (kstrtoul(buf, 16, &val)) > return -EINVAL; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > idx = config->cntr_idx; > config->cntr_ctrl[idx] = val; > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return size; > } > static DEVICE_ATTR_RW(cntr_ctrl); > @@ -1688,9 +1688,9 @@ static ssize_t res_idx_store(struct device *dev, > * Use spinlock to ensure index doesn't change while it gets > * dereferenced multiple times within a spinlock block elsewhere. > */ > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > config->res_idx = val; > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return size; > } > static DEVICE_ATTR_RW(res_idx); > @@ -1704,10 +1704,10 @@ static ssize_t res_ctrl_show(struct device *dev, > struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); > struct etmv4_config *config = &drvdata->config; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > idx = config->res_idx; > val = config->res_ctrl[idx]; > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return scnprintf(buf, PAGE_SIZE, "%#lx\n", val); > } > > @@ -1723,7 +1723,7 @@ static ssize_t res_ctrl_store(struct device *dev, > if (kstrtoul(buf, 16, &val)) > return -EINVAL; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > idx = config->res_idx; > /* For odd idx pair inversal bit is RES0 */ > if (idx % 2 != 0) > @@ -1733,7 +1733,7 @@ static ssize_t res_ctrl_store(struct device *dev, > TRCRSCTLRn_INV | > TRCRSCTLRn_GROUP_MASK | > TRCRSCTLRn_SELECT_MASK); > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return size; > } > static DEVICE_ATTR_RW(res_ctrl); > @@ -1762,9 +1762,9 @@ static ssize_t sshot_idx_store(struct device *dev, > if (val >= drvdata->nr_ss_cmp) > return -EINVAL; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > config->ss_idx = val; > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return size; > } > static DEVICE_ATTR_RW(sshot_idx); > @@ -1777,9 +1777,9 @@ static ssize_t sshot_ctrl_show(struct device *dev, > struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); > struct etmv4_config *config = &drvdata->config; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > val = config->ss_ctrl[config->ss_idx]; > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return scnprintf(buf, PAGE_SIZE, "%#lx\n", val); > } > > @@ -1795,12 +1795,12 @@ static ssize_t sshot_ctrl_store(struct device *dev, > if (kstrtoul(buf, 16, &val)) > return -EINVAL; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > idx = config->ss_idx; > config->ss_ctrl[idx] = FIELD_PREP(TRCSSCCRn_SAC_ARC_RST_MASK, val); > /* must clear bit 31 in related status register on programming */ > config->ss_status[idx] &= ~TRCSSCSRn_STATUS; > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return size; > } > static DEVICE_ATTR_RW(sshot_ctrl); > @@ -1812,9 +1812,9 @@ static ssize_t sshot_status_show(struct device *dev, > struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); > struct etmv4_config *config = &drvdata->config; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > val = config->ss_status[config->ss_idx]; > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return scnprintf(buf, PAGE_SIZE, "%#lx\n", val); > } > static DEVICE_ATTR_RO(sshot_status); > @@ -1827,9 +1827,9 @@ static ssize_t sshot_pe_ctrl_show(struct device *dev, > struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); > struct etmv4_config *config = &drvdata->config; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > val = config->ss_pe_cmp[config->ss_idx]; > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return scnprintf(buf, PAGE_SIZE, "%#lx\n", val); > } > > @@ -1845,12 +1845,12 @@ static ssize_t sshot_pe_ctrl_store(struct device *dev, > if (kstrtoul(buf, 16, &val)) > return -EINVAL; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > idx = config->ss_idx; > config->ss_pe_cmp[idx] = FIELD_PREP(TRCSSPCICRn_PC_MASK, val); > /* must clear bit 31 in related status register on programming */ > config->ss_status[idx] &= ~TRCSSCSRn_STATUS; > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return size; > } > static DEVICE_ATTR_RW(sshot_pe_ctrl); > @@ -1884,9 +1884,9 @@ static ssize_t ctxid_idx_store(struct device *dev, > * Use spinlock to ensure index doesn't change while it gets > * dereferenced multiple times within a spinlock block elsewhere. > */ > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > config->ctxid_idx = val; > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return size; > } > static DEVICE_ATTR_RW(ctxid_idx); > @@ -1907,10 +1907,10 @@ static ssize_t ctxid_pid_show(struct device *dev, > if (task_active_pid_ns(current) != &init_pid_ns) > return -EINVAL; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > idx = config->ctxid_idx; > val = (unsigned long)config->ctxid_pid[idx]; > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return scnprintf(buf, PAGE_SIZE, "%#lx\n", val); > } > > @@ -1945,10 +1945,10 @@ static ssize_t ctxid_pid_store(struct device *dev, > if (kstrtoul(buf, 16, &pid)) > return -EINVAL; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > idx = config->ctxid_idx; > config->ctxid_pid[idx] = (u64)pid; > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return size; > } > static DEVICE_ATTR_RW(ctxid_pid); > @@ -1968,10 +1968,10 @@ static ssize_t ctxid_masks_show(struct device *dev, > if (task_active_pid_ns(current) != &init_pid_ns) > return -EINVAL; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > val1 = config->ctxid_mask0; > val2 = config->ctxid_mask1; > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return scnprintf(buf, PAGE_SIZE, "%#lx %#lx\n", val1, val2); > } > > @@ -2004,7 +2004,7 @@ static ssize_t ctxid_masks_store(struct device *dev, > if ((drvdata->numcidc > 4) && (nr_inputs != 2)) > return -EINVAL; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > /* > * each byte[0..3] controls mask value applied to ctxid > * comparator[0..3] > @@ -2076,7 +2076,7 @@ static ssize_t ctxid_masks_store(struct device *dev, > mask >>= 0x8; > } > > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return size; > } > static DEVICE_ATTR_RW(ctxid_masks); > @@ -2110,9 +2110,9 @@ static ssize_t vmid_idx_store(struct device *dev, > * Use spinlock to ensure index doesn't change while it gets > * dereferenced multiple times within a spinlock block elsewhere. > */ > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > config->vmid_idx = val; > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return size; > } > static DEVICE_ATTR_RW(vmid_idx); > @@ -2132,9 +2132,9 @@ static ssize_t vmid_val_show(struct device *dev, > if (!task_is_in_init_pid_ns(current)) > return -EINVAL; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > val = (unsigned long)config->vmid_val[config->vmid_idx]; > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return scnprintf(buf, PAGE_SIZE, "%#lx\n", val); > } > > @@ -2162,9 +2162,9 @@ static ssize_t vmid_val_store(struct device *dev, > if (kstrtoul(buf, 16, &val)) > return -EINVAL; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > config->vmid_val[config->vmid_idx] = (u64)val; > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return size; > } > static DEVICE_ATTR_RW(vmid_val); > @@ -2183,10 +2183,10 @@ static ssize_t vmid_masks_show(struct device *dev, > if (!task_is_in_init_pid_ns(current)) > return -EINVAL; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > val1 = config->vmid_mask0; > val2 = config->vmid_mask1; > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return scnprintf(buf, PAGE_SIZE, "%#lx %#lx\n", val1, val2); > } > > @@ -2218,7 +2218,7 @@ static ssize_t vmid_masks_store(struct device *dev, > if ((drvdata->numvmidc > 4) && (nr_inputs != 2)) > return -EINVAL; > > - spin_lock(&drvdata->spinlock); > + raw_spin_lock(&drvdata->spinlock); > > /* > * each byte[0..3] controls mask value applied to vmid > @@ -2291,7 +2291,7 @@ static ssize_t vmid_masks_store(struct device *dev, > else > mask >>= 0x8; > } > - spin_unlock(&drvdata->spinlock); > + raw_spin_unlock(&drvdata->spinlock); > return size; > } > static DEVICE_ATTR_RW(vmid_masks); > diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h > index 2b92de17b5a2..bd7db36ba197 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x.h > +++ b/drivers/hwtracing/coresight/coresight-etm4x.h > @@ -989,7 +989,7 @@ struct etmv4_drvdata { > struct clk *pclk; > void __iomem *base; > struct coresight_device *csdev; > - spinlock_t spinlock; > + raw_spinlock_t spinlock; > int cpu; > u8 arch; > u8 nr_pe;

1 month, 3 weeks

Re: [PATCH v5 8/9] coresight-tmc: change tmc_drvdata spinlock's type to raw_spinlock_t

by Suzuki K Poulose

Hi Levi, On 06/03/2025 09:47, Yeoreum Yun wrote: > In coresight-tmc drivers, tmc_drvdata->spinlock can be held > during __schedule() by perf_event_task_sched_out()/in(). > > Since tmc_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. > > To address this, change type tmc_drvdata->spinlock in coresight-tmc drivers, > which can be called by perf_event_task_sched_out()/in(), > from spinlock_t to raw_spinlock_t. > > Reviewed-by: James Clark <james.clark(a)linaro.org> > Reviewed-by: Mike Leach <mike.leach(a)linaro.org> > Signed-off-by: Yeoreum Yun <yeoreum.yun(a)arm.com> Unfortunately, this still doesn't cover the current coresight next branch. I get build errors as below : "[PATCH] coresight-tmc: change tmc_drvdata spinlock's type to" has style problems, please review. This is because, we merged the coresight panic trace support in the coresight next, on 21st Feb. NOTE: If any of the errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS. CALL scripts/checksyscalls.sh CC [M] drivers/hwtracing/coresight/coresight-tmc-core.o CC [M] drivers/hwtracing/coresight/coresight-tmc-etf.o CC [M] drivers/hwtracing/coresight/coresight-tmc-etr.o CC [M] drivers/hwtracing/coresight/coresight-catu.o In file included from ./include/linux/mmzone.h:8, from ./include/linux/gfp.h:7, from ./include/linux/slab.h:16, from ./include/linux/resource_ext.h:11, from ./include/linux/acpi.h:13, from drivers/hwtracing/coresight/coresight-tmc-core.c:7: drivers/hwtracing/coresight/coresight-tmc-core.c: In function ‘tmc_crashdata_open’: drivers/hwtracing/coresight/coresight-tmc-core.c:361:20: error: passing argument 1 of ‘spinlock_check’ from incompatible pointer type [-Werror=incompatible-pointer-types] 361 | spin_lock_irqsave(&drvdata->spinlock, flags); | ^~~~~~~~~~~~~~~~~~ | | | raw_spinlock_t * {aka struct raw_spinlock *} ./include/linux/spinlock.h:244:34: note: in definition of macro ‘raw_spin_lock_irqsave’ 244 | flags = _raw_spin_lock_irqsave(lock); \ | ^~~~ drivers/hwtracing/coresight/coresight-tmc-core.c:361:2: note: in expansion of macro ‘spin_lock_irqsave’ 361 | spin_lock_irqsave(&drvdata->spinlock, flags); | ^~~~~~~~~~~~~~~~~ In file included from ./include/linux/mmzone.h:8, from ./include/linux/gfp.h:7, from ./include/linux/slab.h:16, from ./include/linux/resource_ext.h:11, from ./include/linux/acpi.h:13, from drivers/hwtracing/coresight/coresight-tmc-core.c:7: ./include/linux/spinlock.h:324:67: note: expected ‘spinlock_t *’ {aka ‘struct spinlock *’} but argument is of type ‘raw_spinlock_t *’ {aka ‘struct raw_spinlock *’} 324 | static __always_inline raw_spinlock_t *spinlock_check(spinlock_t *lock) Cheers Suzuki

1 month, 3 weeks

Re: [PATCH v15 02/10] Coresight: Add trace_id function to retrieving the trace ID

by Mike Leach

Hi Jie On Wed, 5 Mar 2025 at 13:27, Jie Gan <quic_jiegan(a)quicinc.com> wrote: > > > > On 3/5/2025 7:07 PM, Mike Leach wrote: > > Hi, > > > > On Mon, 3 Mar 2025 at 03:30, Jie Gan <quic_jiegan(a)quicinc.com> wrote: > >> > >> Add 'trace_id' function pointer in coresight_ops. It's responsible for retrieving > >> the device's trace ID. > >> > >> Co-developed-by: James Clark <james.clark(a)linaro.org> > >> Signed-off-by: James Clark <james.clark(a)linaro.org> > >> Reviewed-by: James Clark <james.clark(a)linaro.org> > >> Signed-off-by: Jie Gan <quic_jiegan(a)quicinc.com> > >> --- > >> drivers/hwtracing/coresight/coresight-core.c | 30 +++++++++++++++++++ > >> drivers/hwtracing/coresight/coresight-dummy.c | 13 +++++++- > >> .../coresight/coresight-etm3x-core.c | 1 + > >> .../coresight/coresight-etm4x-core.c | 1 + > >> drivers/hwtracing/coresight/coresight-stm.c | 11 +++++++ > >> drivers/hwtracing/coresight/coresight-tpda.c | 11 +++++++ > >> include/linux/coresight.h | 5 ++++ > >> 7 files changed, 71 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c > >> index ab55e10d4b79..32aa07f4f8c1 100644 > >> --- a/drivers/hwtracing/coresight/coresight-core.c > >> +++ b/drivers/hwtracing/coresight/coresight-core.c > >> @@ -24,6 +24,7 @@ > >> #include "coresight-etm-perf.h" > >> #include "coresight-priv.h" > >> #include "coresight-syscfg.h" > >> +#include "coresight-trace-id.h" > >> > >> /* > >> * Mutex used to lock all sysfs enable and disable actions and loading and > >> @@ -1557,6 +1558,35 @@ void coresight_remove_driver(struct amba_driver *amba_drv, > >> } > >> 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); > >> + > > > > This is a global funciton so need to check that this csdev is a > > source,. and does provide a cpu function before calling it. > > > > Hi Mike, > > I put this function here because it's required by etm3x and etm4x. It's > intended to be called only by ETM devices, which are definitely source > devices and have a cpu function. > I fully understand the intent, but for a function that can be accessed from anywhere, it is safer to validate input rather than assume any caller will always respect the input conditions. Lots of other places in the coresight drivers check that these functions exist before calling them. Regards Mike > Jie > > >> + switch (mode) { > >> + case CS_MODE_SYSFS: > >> + trace_id = coresight_trace_id_get_cpu_id(cpu); > >> + break; > >> + case CS_MODE_PERF: > >> + if (WARN_ON(!sink)) > >> + return -EINVAL; > >> + > >> + trace_id = coresight_trace_id_get_cpu_id_map(cpu, &sink->perf_sink_id_map); > >> + break; > >> + default: > >> + trace_id = -EINVAL; > >> + break; > >> + } > >> + > >> + if (!IS_VALID_CS_TRACE_ID(trace_id)) > >> + dev_err(&csdev->dev, > >> + "Failed to allocate trace ID on CPU%d\n", cpu); > >> + > >> + return trace_id; > >> +} > >> +EXPORT_SYMBOL_GPL(coresight_etm_get_trace_id); > >> + > >> MODULE_LICENSE("GPL v2"); > >> MODULE_AUTHOR("Pratik Patel <pratikp(a)codeaurora.org>"); > >> MODULE_AUTHOR("Mathieu Poirier <mathieu.poirier(a)linaro.org>"); > >> diff --git a/drivers/hwtracing/coresight/coresight-dummy.c b/drivers/hwtracing/coresight/coresight-dummy.c > >> index 9be53be8964b..b5692ba358c1 100644 > >> --- a/drivers/hwtracing/coresight/coresight-dummy.c > >> +++ b/drivers/hwtracing/coresight/coresight-dummy.c > >> @@ -41,6 +41,16 @@ static void dummy_source_disable(struct coresight_device *csdev, > >> dev_dbg(csdev->dev.parent, "Dummy source disabled\n"); > >> } > >> > >> +static int dummy_source_trace_id(struct coresight_device *csdev, __maybe_unused enum cs_mode mode, > >> + __maybe_unused struct coresight_device *sink) > >> +{ > >> + struct dummy_drvdata *drvdata; > >> + > >> + drvdata = dev_get_drvdata(csdev->dev.parent); > >> + > >> + return drvdata->traceid; > >> +} > >> + > >> static int dummy_sink_enable(struct coresight_device *csdev, enum cs_mode mode, > >> void *data) > >> { > >> @@ -62,7 +72,8 @@ static const struct coresight_ops_source dummy_source_ops = { > >> }; > >> > >> static const struct coresight_ops dummy_source_cs_ops = { > >> - .source_ops = &dummy_source_ops, > >> + .trace_id = dummy_source_trace_id, > >> + .source_ops = &dummy_source_ops, > >> }; > >> > >> static const struct coresight_ops_sink dummy_sink_ops = { > >> diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c > >> index c103f4c70f5d..c1dda4bc4a2f 100644 > >> --- a/drivers/hwtracing/coresight/coresight-etm3x-core.c > >> +++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c > >> @@ -704,6 +704,7 @@ static const struct coresight_ops_source etm_source_ops = { > >> }; > >> > >> static const struct coresight_ops etm_cs_ops = { > >> + .trace_id = coresight_etm_get_trace_id, > >> .source_ops = &etm_source_ops, > >> }; > >> > >> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c > >> index 2c1a60577728..cfd116b87460 100644 > >> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c > >> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c > >> @@ -1067,6 +1067,7 @@ static const struct coresight_ops_source etm4_source_ops = { > >> }; > >> > >> static const struct coresight_ops etm4_cs_ops = { > >> + .trace_id = coresight_etm_get_trace_id, > >> .source_ops = &etm4_source_ops, > >> }; > >> > >> diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c > >> index b581a30a1cd9..aca25b5e3be2 100644 > >> --- a/drivers/hwtracing/coresight/coresight-stm.c > >> +++ b/drivers/hwtracing/coresight/coresight-stm.c > >> @@ -281,12 +281,23 @@ static void stm_disable(struct coresight_device *csdev, > >> } > >> } > >> > >> +static int stm_trace_id(struct coresight_device *csdev, __maybe_unused enum cs_mode mode, > >> + __maybe_unused struct coresight_device *sink) > >> +{ > >> + struct stm_drvdata *drvdata; > >> + > >> + drvdata = dev_get_drvdata(csdev->dev.parent); > >> + > >> + return drvdata->traceid; > >> +} > >> + > >> static const struct coresight_ops_source stm_source_ops = { > >> .enable = stm_enable, > >> .disable = stm_disable, > >> }; > >> > >> static const struct coresight_ops stm_cs_ops = { > >> + .trace_id = stm_trace_id, > >> .source_ops = &stm_source_ops, > >> }; > >> > >> diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c > >> index 573da8427428..94c2201fc8d3 100644 > >> --- a/drivers/hwtracing/coresight/coresight-tpda.c > >> +++ b/drivers/hwtracing/coresight/coresight-tpda.c > >> @@ -241,12 +241,23 @@ static void tpda_disable(struct coresight_device *csdev, > >> dev_dbg(drvdata->dev, "TPDA inport %d disabled\n", in->dest_port); > >> } > >> > >> +static int tpda_trace_id(struct coresight_device *csdev, __maybe_unused enum cs_mode mode, > >> + __maybe_unused struct coresight_device *sink) > >> +{ > >> + struct tpda_drvdata *drvdata; > >> + > >> + drvdata = dev_get_drvdata(csdev->dev.parent); > >> + > >> + return drvdata->atid; > >> +} > >> + > >> static const struct coresight_ops_link tpda_link_ops = { > >> .enable = tpda_enable, > >> .disable = tpda_disable, > >> }; > >> > >> static const struct coresight_ops tpda_cs_ops = { > >> + .trace_id = tpda_trace_id, > >> .link_ops = &tpda_link_ops, > >> }; > >> > >> diff --git a/include/linux/coresight.h b/include/linux/coresight.h > >> index c7cd5886c908..ce9a5e71b261 100644 > >> --- a/include/linux/coresight.h > >> +++ b/include/linux/coresight.h > >> @@ -335,6 +335,7 @@ enum cs_mode { > >> CS_MODE_PERF, > >> }; > >> > >> +#define coresight_ops(csdev) csdev->ops > >> #define source_ops(csdev) csdev->ops->source_ops > >> #define sink_ops(csdev) csdev->ops->sink_ops > >> #define link_ops(csdev) csdev->ops->link_ops > >> @@ -421,6 +422,8 @@ struct coresight_ops_panic { > >> }; > >> > >> struct coresight_ops { > >> + int (*trace_id)(struct coresight_device *csdev, enum cs_mode mode, > >> + struct coresight_device *sink); > >> const struct coresight_ops_sink *sink_ops; > >> const struct coresight_ops_link *link_ops; > >> const struct coresight_ops_source *source_ops; > >> @@ -709,4 +712,6 @@ int coresight_init_driver(const char *drv, struct amba_driver *amba_drv, > >> > >> void coresight_remove_driver(struct amba_driver *amba_drv, > >> struct platform_driver *pdev_drv); > >> +int coresight_etm_get_trace_id(struct coresight_device *csdev, enum cs_mode mode, > >> + struct coresight_device *sink); > >> #endif /* _LINUX_COREISGHT_H */ > >> -- > >> 2.34.1 > >> > > > > > -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

1 month, 3 weeks

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight March 2025