CoreSight

coresight@lists.linaro.org

5 participants
2730 discussions

Re: [PATCH v16 7/7] qcom-tgu: Add reset node to initialize

by Suzuki K Poulose

On 30/07/2026 09:07, Songwei Chai wrote: > Add reset node to initialize the value of > priority/condition_decode/condition_select/timer/counter nodes. > > Signed-off-by: Songwei Chai <songwei.chai(a)oss.qualcomm.com> > Reviewed-by: Jie Gan <jie.gan(a)oss.qualcomm.com> > --- > .../ABI/testing/sysfs-bus-amba-devices-tgu | 7 ++ > drivers/hwtracing/qcom/tgu.c | 84 +++++++++++++++++++ > 2 files changed, 91 insertions(+) > > diff --git a/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu b/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu > index a0cad2f16310..5c928a5fa033 100644 > --- a/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu > +++ b/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu > @@ -51,3 +51,10 @@ KernelVersion: 7.2 > Contact: Jinlong Mao <jinlong.mao(a)oss.qualcomm.com>, Songwei Chai <songwei.chai(a)oss.qualcomm.com> > Description: > (RW) Set/Get the counter value with specific step for TGU. > + > +What: /sys/bus/amba/devices/<tgu-name>/reset_tgu > +Date: August 2026 > +KernelVersion: 7.2 > +Contact: Jinlong Mao <jinlong.mao(a)oss.qualcomm.com>, Songwei Chai <songwei.chai(a)oss.qualcomm.com> > +Description: > + (Write) Write 1 to reset the dataset for TGU. > diff --git a/drivers/hwtracing/qcom/tgu.c b/drivers/hwtracing/qcom/tgu.c > index ea83273a628c..d65a68faa741 100644 > --- a/drivers/hwtracing/qcom/tgu.c > +++ b/drivers/hwtracing/qcom/tgu.c > @@ -228,6 +228,18 @@ static ssize_t tgu_write_all_hw_regs(struct tgu_drvdata *drvdata) > int i, j, k, index = 0; > > TGU_UNLOCK(drvdata->base); > + > + /* > + * Quiesce the detection engine before mutating any comparison input. > + * This function is also reached from reset_tgu_store() while the TGU is > + * already running (TGU_CONTROL == 1); rewriting the priority/condition/ > + * timer/counter registers register-by-register on a live engine could > + * feed a transient, half-updated match into CTI/interrupt consumers. > + * Programming with CONTROL == 0 and only re-enabling at the end keeps the > + * program-then-enable ordering for every caller. > + */ > + writel(0, drvdata->base + TGU_CONTROL); > + > for (i = 0; i < drvdata->num_step; i++) { > for (j = 0; j < MAX_PRIORITY; j++) { > for (k = 0; k < drvdata->num_reg; k++) { > @@ -459,8 +471,80 @@ static ssize_t enable_tgu_store(struct device *dev, > } > static DEVICE_ATTR_RW(enable_tgu); > > +static ssize_t reset_tgu_store(struct device *dev, > + struct device_attribute *attr, const char *buf, > + size_t size) > +{ > + struct tgu_drvdata *drvdata = dev_get_drvdata(dev); > + struct value_table *vt = drvdata->value_table; > + unsigned long value; > + int i, j, ret; > + > + if (kstrtoul(buf, 0, &value) || value != 1) > + return -EINVAL; > + > + /* > + * Hold the mutex across the whole sequence so it cannot race with > + * enable_tgu_store()/tgu_dataset_store(): the enabled check and the > + * hardware access stay consistent. No PM reference is taken or dropped > + * here - the shadow tables are plain memory, and the hardware is only > + * touched when the device is already resumed via the enable reference. > + */ > + guard(mutex)(&drvdata->lock); > + > + if (vt->priority) { > + size_t bytes = MAX_PRIORITY * drvdata->num_step * > + drvdata->num_reg * sizeof(unsigned int); Please could you add a macro to compute "number" of each array ? Something like : #define TGU_PRIORITY_SIZE(drvdata) (MAX_PRIORITY * (drvdata)->num_step * (drvdata)->num_reg) So that we use the same size calculation for allocation and reset ? You may scale it by sizeof(*vt->priority) > + memset(vt->priority, 0, bytes); > + } > + > + if (vt->condition_decode) { > + size_t bytes = drvdata->num_condition_decode * > + drvdata->num_step * sizeof(unsigned int); Same as above > + memset(vt->condition_decode, 0, bytes); > + > + /* Initialize all condition decode registers to NOT (0x1000000) */ then why do the memset above ? > + for (i = 0; i < drvdata->num_step; i++) > + for (j = 0; j < drvdata->num_condition_decode; j++) > + vt->condition_decode[calculate_array_location( > + drvdata, i, TGU_CONDITION_DECODE, j)] = > + TGU_CONDITION_DECODE_NOT; Then, we could simplify the above to: for (i = 0; i < TGU_CONDITION_DECODE(drvdata); i++) vt->condition_decode[i] = TGU_CONDITION_DECODE_NOT; Same for everything below. Suzuki > + } > + > + if (vt->condition_select) { > + size_t bytes = drvdata->num_condition_select * > + drvdata->num_step * sizeof(unsigned int); > + memset(vt->condition_select, 0, bytes); > + } > + > + if (vt->timer) { > + size_t bytes = drvdata->num_step * drvdata->num_timer * > + sizeof(unsigned int); > + memset(vt->timer, 0, bytes); > + } > + > + if (vt->counter) { > + size_t bytes = drvdata->num_step * drvdata->num_counter * > + sizeof(unsigned int); > + memset(vt->counter, 0, bytes); > + } > + > + /* If the TGU is running, apply the cleared configuration immediately. */ > + if (drvdata->enabled) { > + ret = tgu_write_all_hw_regs(drvdata); > + if (ret) > + return ret; > + } > + > + dev_dbg(dev, "Qualcomm-TGU reset complete\n"); > + > + return size; > +} > +static DEVICE_ATTR_WO(reset_tgu); > + > static struct attribute *tgu_common_attrs[] = { > &dev_attr_enable_tgu.attr, > + &dev_attr_reset_tgu.attr, > NULL, > }; >

1 week, 1 day

Re: [PATCH v16 3/7] qcom-tgu: Add signal priority support

by Suzuki K Poulose

On 30/07/2026 09:07, Songwei Chai wrote: > 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. > > Reviewed-by: Jie Gan <jie.gan(a)oss.qualcomm.com> > Signed-off-by: Songwei Chai <songwei.chai(a)oss.qualcomm.com> > --- > .../ABI/testing/sysfs-bus-amba-devices-tgu | 7 + > drivers/hwtracing/qcom/tgu.c | 189 +++++++++++++++++- > drivers/hwtracing/qcom/tgu.h | 133 ++++++++++++ > 3 files changed, 328 insertions(+), 1 deletion(-) > > diff --git a/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu b/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu > index 36ab64f1e301..7dd24314592d 100644 > --- a/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu > +++ b/Documentation/ABI/testing/sysfs-bus-amba-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/amba/devices/<tgu-name>/step[0:7]_priority[0:3]/reg[0:17] > +Date: August 2026 > +KernelVersion: 7.2 > +Contact: Jinlong Mao <jinlong.mao(a)oss.qualcomm.com>, Songwei Chai <songwei.chai(a)oss.qualcomm.com> > +Description: > + (RW) Set/Get the sensed signal with specific step and priority for TGU. > diff --git a/drivers/hwtracing/qcom/tgu.c b/drivers/hwtracing/qcom/tgu.c > index 9c7dd4a09f02..0e88b437f191 100644 > --- a/drivers/hwtracing/qcom/tgu.c > +++ b/drivers/hwtracing/qcom/tgu.c > @@ -15,14 +15,144 @@ > > #include "tgu.h" > > +static int calculate_array_location(struct tgu_drvdata *drvdata, > + int step_index, int operation_index, > + int reg_index) > +{ > + return operation_index * (drvdata->num_step) * (drvdata->num_reg) + > + step_index * (drvdata->num_reg) + reg_index; > +} > + > +static ssize_t tgu_dataset_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct tgu_drvdata *drvdata = dev_get_drvdata(dev); > + struct tgu_attribute *tgu_attr = > + container_of(attr, struct tgu_attribute, attr); > + int index; > + > + index = calculate_array_location(drvdata, tgu_attr->step_index, > + tgu_attr->operation_index, > + tgu_attr->reg_num); > + > + /* > + * Serialise against tgu_dataset_store() and tgu_enable(), which read > + * and write the same priority array, so the value is not observed > + * while being updated concurrently. > + */ > + guard(mutex)(&drvdata->lock); > + > + return sysfs_emit(buf, "0x%x\n", > + drvdata->value_table->priority[index]); > +} > + > +static ssize_t tgu_dataset_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t size) > +{ > + struct tgu_drvdata *tgu_drvdata = dev_get_drvdata(dev); > + struct tgu_attribute *tgu_attr = > + container_of(attr, struct tgu_attribute, attr); > + unsigned long val; > + int index; > + int ret; > + > + ret = kstrtoul(buf, 0, &val); > + if (ret) > + return ret; > + > + guard(mutex)(&tgu_drvdata->lock); > + index = calculate_array_location(tgu_drvdata, tgu_attr->step_index, > + tgu_attr->operation_index, > + tgu_attr->reg_num); > + > + tgu_drvdata->value_table->priority[index] = val; > + > + return size; > +} > + > +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); > + 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->num_step) > + return SYSFS_GROUP_INVISIBLE; > + > + if (tgu_attr->reg_num >= drvdata->num_reg) > + return 0; > + > + return attr->mode; > +} > + > static void tgu_write_all_hw_regs(struct tgu_drvdata *drvdata) > { > + int i, j, k, index; > + > TGU_UNLOCK(drvdata->base); > + for (i = 0; i < drvdata->num_step; i++) { > + for (j = 0; j < MAX_PRIORITY; j++) { > + for (k = 0; k < drvdata->num_reg; k++) { > + index = calculate_array_location( > + drvdata, i, j, k); > + > + writel(drvdata->value_table->priority[index], > + drvdata->base + > + PRIORITY_REG_STEP(i, j, k)); > + } > + } > + } > /* Enable TGU to program the triggers */ > writel(1, drvdata->base + TGU_CONTROL); > TGU_LOCK(drvdata->base); > } > > +static void tgu_set_reg_number(struct tgu_drvdata *drvdata) > +{ > + int num_sense_input; > + int num_reg; > + u32 devid; > + > + devid = readl(drvdata->base + TGU_DEVID); > + > + num_sense_input = TGU_DEVID_SENSE_INPUT(devid); > + num_reg = (num_sense_input * TGU_BITS_PER_SIGNAL) / LENGTH_REGISTER; > + > + if ((num_sense_input * TGU_BITS_PER_SIGNAL) % LENGTH_REGISTER) > + num_reg++; > + > + if (num_reg > TGU_MAX_PRIORITY_REGS) { > + dev_warn(drvdata->dev, > + "DEVID reports %d priority regs, clamping to %d\n", > + num_reg, TGU_MAX_PRIORITY_REGS); > + num_reg = TGU_MAX_PRIORITY_REGS; > + } > + > + drvdata->num_reg = num_reg; > +} > + > +static void tgu_set_steps(struct tgu_drvdata *drvdata) > +{ > + u32 devid; > + > + devid = readl(drvdata->base + TGU_DEVID); > + > + drvdata->num_step = TGU_DEVID_STEPS(devid); > + > + if (drvdata->num_step > TGU_MAX_STEPS) { > + dev_warn(drvdata->dev, > + "DEVID reports %d steps, clamping to %d\n", > + drvdata->num_step, TGU_MAX_STEPS); > + drvdata->num_step = TGU_MAX_STEPS; > + } > +} Why not combine setting the num_step and num_regs in one go ? You are reading the DEVID anyways, and such macro level functional split is useless. > + > static void tgu_enable(struct tgu_drvdata *drvdata) > { > tgu_write_all_hw_regs(drvdata); > @@ -104,6 +234,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, > }; > > @@ -111,6 +273,8 @@ static int tgu_probe(struct amba_device *adev, const struct amba_id *id) > { > struct device *dev = &adev->dev; > struct tgu_drvdata *drvdata; > + unsigned int *priority; > + size_t priority_size; > int ret; > > drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); > @@ -128,8 +292,31 @@ static int tgu_probe(struct amba_device *adev, const struct amba_id *id) > if (ret) > return ret; > > - drvdata->enabled = false; > + tgu_set_reg_number(drvdata); > + tgu_set_steps(drvdata); As mentioned above, combine both ? > + > + drvdata->value_table = > + devm_kzalloc(dev, sizeof(*drvdata->value_table), GFP_KERNEL); Could we not embed this in the drvdata without dynamic allocation ? > + if (!drvdata->value_table) > + return -ENOMEM; > > + priority_size = MAX_PRIORITY * drvdata->num_reg * drvdata->num_step; > + > + priority = devm_kcalloc(dev, priority_size, > + sizeof(*drvdata->value_table->priority), > + GFP_KERNEL); > + if (!priority) > + return -ENOMEM; > + > + drvdata->value_table->priority = priority; > + > + drvdata->enabled = false; > + /* > + * Create the sysfs groups only after the backing storage is fully > + * allocated, otherwise userspace could open a priority attribute and > + * have tgu_dataset_show() dereference an uninitialised value_table. > + * This also keeps the error paths above free of sysfs teardown. > + */ > ret = sysfs_create_groups(&dev->kobj, tgu_attr_groups); > if (ret) { > dev_err(dev, "failed to create sysfs groups: %d\n", ret); > diff --git a/drivers/hwtracing/qcom/tgu.h b/drivers/hwtracing/qcom/tgu.h > index ebba49402a20..7a3e1161c4cf 100644 > --- a/drivers/hwtracing/qcom/tgu.h > +++ b/drivers/hwtracing/qcom/tgu.h > @@ -6,10 +6,137 @@ > #ifndef _QCOM_TGU_H > #define _QCOM_TGU_H > > +#include <linux/bitfield.h> > +#include <linux/bits.h> > + > /* Register addresses */ > #define TGU_CONTROL 0x0000 > #define TGU_LAR 0xfb0 > #define TGU_UNLOCK_OFFSET 0xc5acce55 > +#define TGU_DEVID 0xfc8 > + > +#define TGU_DEVID_SENSE_INPUT(devid_val) \ > + ((int)FIELD_GET(GENMASK(17, 10), devid_val)) > +#define TGU_DEVID_STEPS(devid_val) \ > + ((int)FIELD_GET(GENMASK(6, 3), devid_val)) > +#define TGU_BITS_PER_SIGNAL 4 > +#define LENGTH_REGISTER 32 > + > +/* > + * TGU configuration space Step configuration > + * offset table space layout > + * x-------------------------x$ x-------------x$ > + * | |$ | |$ > + * | | | reserve |$ > + * | | | |$ > + * |coresight management | |-------------|base+n*0x1D8+0x1F4$ > + * | registers | |---> |priority[3] |$ > + * | | | |-------------|base+n*0x1D8+0x194$ > + * | | | |priority[2] |$ > + * |-------------------------| | |-------------|base+n*0x1D8+0x134$ > + * | | | |priority[1] |$ > + * | step[7] | | |-------------|base+n*0x1D8+0xD4$ > + * |-------------------------|->base+0x40+7*0x1D8 | |priority[0] |$ > + * | | | |-------------|base+n*0x1D8+0x74$ > + * | ... | | | condition |$ > + * | | | | select |$ > + * |-------------------------|->base+0x40+1*0x1D8 | |-------------|base+n*0x1D8+0x60$ minor nit: ^^^ s/base+0x40+1*0x1D8/step[0] + 0x1D8 and so on, for the above > + * | | | | 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$ Also for each frame, you don't have to describe "base+n*..." that makes it unnecessarily crowded and complicated to read. > + * > + */ > +#define STEP_OFFSET 0x1D8 > +#define PRIORITY_START_OFFSET 0x0074 > +#define PRIORITY_OFFSET 0x60 From the diagram, it is CONDITION_SELECT ? Having two offsets named after PRIORITY is confusing. > +#define REG_OFFSET 0x4 > + > +/* > + * Upper bounds on the DEVID/DEVID2-derived per-region counts. The TGU register > + * layout is fixed by the offset macros above and mirrored by the static sysfs > + * attribute tables in tgu.c, so these are the largest counts the driver can > + * both address within the mapped window and expose to userspace: > + * > + * TGU_MAX_STEPS static tables define step0..step7 > + * TGU_MAX_PRIORITY_REGS STEP_PRIORITY_LIST defines reg0..reg17 What are these ? PRIORITY regs is 0-3 ? Do you mean each priority has 0-17 registers ? (See, the description using base+n*.. for the "step" frame is making this worse). You may move it down as a separate figure. From my maths: For pritority[0] -> 0xD4-0x74 = 0x60 => 0x18 => 24 registers! not 18! What am I missing ? > + * > + * A device whose DEVID reports counts above these bounds is clamped, so writes > + * never fall outside the ioremap()'d window or spill into an adjacent region. > + */ > + > +#define TGU_MAX_STEPS 8 > +#define TGU_MAX_PRIORITY_REGS 18 > + > +/* Calculate compare step addresses */ > +#define PRIORITY_REG_STEP(step, priority, reg)\ > + (PRIORITY_START_OFFSET + PRIORITY_OFFSET * priority +\ > + REG_OFFSET * reg + STEP_OFFSET * step) > + > +#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 \ > + }) Please could you move these macro magic to the .c file ? 1. Nobody else reuses them ? 2. It is really difficult to go back and forth to check what each macro means, given how complex it is. > + > +enum operation_index { why are we calling it 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; u8 priority ? > + u32 reg_num; u8 reg_num ? > +}; > + > +struct value_table { > + unsigned int *priority; > +}; > > static inline void TGU_LOCK(void __iomem *addr) > { > @@ -36,6 +163,9 @@ static inline void TGU_UNLOCK(void __iomem *addr) > * @lock: Mutex serialising enable/disable and the associated PM runtime > * reference counting against concurrent sysfs access > * @enabled: Flag indicating whether the TGU device is enabled > + * @value_table: Store given value based on relevant parameters > + * @num_reg: Maximum number of registers > + * @num_step: Maximum step size > * > * This structure defines the data associated with a TGU device, > * including its base address, device pointer, the lock serialising > @@ -46,6 +176,9 @@ struct tgu_drvdata { > struct device *dev; > struct mutex lock; > bool enabled; > + struct value_table *value_table; Why this ? why not struct value_table value_table ? Surely, we seem to be dynamically allocating the priorities etc from there. But do we need dynamic allocation for value_table field as such ? Suzuki > + int num_reg; > + int num_step; > }; > > #endif

1 week, 1 day

Re: [PATCH v16 2/7] qcom-tgu: Add TGU driver

by Suzuki K Poulose

On 30/07/2026 09:06, Songwei Chai wrote: > Add driver to support 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 <songwei.chai(a)oss.qualcomm.com> > Acked-by: Konrad Dybcio <konrad.dybcio(a)oss.qualcomm.com> > Reviewed-by: Jie Gan <jie.gan(a)oss.qualcomm.com> > --- > .../ABI/testing/sysfs-bus-amba-devices-tgu | 9 + > MAINTAINERS | 11 + > drivers/Makefile | 1 + > drivers/hwtracing/Kconfig | 2 + > drivers/hwtracing/qcom/Kconfig | 20 ++ > drivers/hwtracing/qcom/Makefile | 3 + > drivers/hwtracing/qcom/tgu.c | 192 ++++++++++++++++++ > drivers/hwtracing/qcom/tgu.h | 51 +++++ > 8 files changed, 289 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-bus-amba-devices-tgu > create mode 100644 drivers/hwtracing/qcom/Kconfig > create mode 100644 drivers/hwtracing/qcom/Makefile > create mode 100644 drivers/hwtracing/qcom/tgu.c > create mode 100644 drivers/hwtracing/qcom/tgu.h > > diff --git a/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu b/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu > new file mode 100644 > index 000000000000..36ab64f1e301 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu > @@ -0,0 +1,9 @@ > +What: /sys/bus/amba/devices/<tgu-name>/enable_tgu > +Date: August 2026 > +KernelVersion: 7.2 > +Contact: Jinlong Mao <jinlong.mao(a)oss.qualcomm.com>, Songwei Chai <songwei.chai(a)oss.qualcomm.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/MAINTAINERS b/MAINTAINERS > index ce5c5185c773..4789321cca6d 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -22360,6 +22360,17 @@ S: Maintained > F: Documentation/tee/qtee.rst > F: drivers/tee/qcomtee/ > > +QUALCOMM HARDWARETRACING DRIVER > +M: Songwei Chai <songwei.chai(a)oss.qualcomm.com> > +M: Jie Gan <jie.gan(a)oss.qualcomm.com> > +M: Suzuki K Poulose <suzuki.poulose(a)arm.com> > +L: linux-arm-msm(a)vger.kernel.org > +S: Maintained > +T: git git://git.kernel.org/pub/scm/linux/kernel/git/coresight/linux.git > +F: Documentation/ABI/testing/sysfs-bus-amba-devices-tgu > +F: Documentation/devicetree/bindings/arm/qcom,tgu.yaml > +F: drivers/hwtracing/qcom/ > + > QUALCOMM TRUST ZONE MEMORY ALLOCATOR > M: Bartosz Golaszewski <brgl(a)kernel.org> > L: linux-arm-msm(a)vger.kernel.org > diff --git a/drivers/Makefile b/drivers/Makefile > index 0841ea851847..6a2cbe5b340f 100644 > --- a/drivers/Makefile > +++ b/drivers/Makefile > @@ -176,6 +176,7 @@ obj-$(CONFIG_RAS) += ras/ > obj-$(CONFIG_USB4) += thunderbolt/ > obj-$(CONFIG_CORESIGHT) += hwtracing/coresight/ > obj-y += hwtracing/intel_th/ > +obj-y += hwtracing/qcom/ > obj-$(CONFIG_STM) += hwtracing/stm/ > obj-$(CONFIG_HISI_PTT) += hwtracing/ptt/ > obj-y += android/ > diff --git a/drivers/hwtracing/Kconfig b/drivers/hwtracing/Kconfig > index 911ee977103c..8a640218eed8 100644 > --- a/drivers/hwtracing/Kconfig > +++ b/drivers/hwtracing/Kconfig > @@ -7,4 +7,6 @@ source "drivers/hwtracing/intel_th/Kconfig" > > source "drivers/hwtracing/ptt/Kconfig" > > +source "drivers/hwtracing/qcom/Kconfig" > + > endmenu > diff --git a/drivers/hwtracing/qcom/Kconfig b/drivers/hwtracing/qcom/Kconfig > new file mode 100644 > index 000000000000..5c94c75ffa39 > --- /dev/null > +++ b/drivers/hwtracing/qcom/Kconfig > @@ -0,0 +1,20 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +# > +# QCOM specific hwtracing drivers > +# > +menu "Qualcomm specific hwtracing drivers" > + > +config QCOM_TGU > + tristate "QCOM Trigger Generation Unit driver" > + depends on ARCH_QCOM || COMPILE_TEST > + depends on ARM_AMBA > + 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. > + > +endmenu > diff --git a/drivers/hwtracing/qcom/Makefile b/drivers/hwtracing/qcom/Makefile > new file mode 100644 > index 000000000000..5a0a868c1ea0 > --- /dev/null > +++ b/drivers/hwtracing/qcom/Makefile > @@ -0,0 +1,3 @@ > +# SPDX-License-Identifier: GPL-2.0 > + > +obj-$(CONFIG_QCOM_TGU) += tgu.o > diff --git a/drivers/hwtracing/qcom/tgu.c b/drivers/hwtracing/qcom/tgu.c > new file mode 100644 > index 000000000000..9c7dd4a09f02 > --- /dev/null > +++ b/drivers/hwtracing/qcom/tgu.c > @@ -0,0 +1,192 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries. > + */ > + > +#include <linux/amba/bus.h> > +#include <linux/device.h> > +#include <linux/err.h> > +#include <linux/io.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/mutex.h> > +#include <linux/of.h> > +#include <linux/pm_runtime.h> > + > +#include "tgu.h" > + > +static void tgu_write_all_hw_regs(struct tgu_drvdata *drvdata) > +{ > + TGU_UNLOCK(drvdata->base); > + /* Enable TGU to program the triggers */ > + writel(1, drvdata->base + TGU_CONTROL); > + TGU_LOCK(drvdata->base); > +} > + > +static void tgu_enable(struct tgu_drvdata *drvdata) > +{ > + tgu_write_all_hw_regs(drvdata); > + drvdata->enabled = true; > +} > + > +static void tgu_disable(struct tgu_drvdata *drvdata) > +{ > + TGU_UNLOCK(drvdata->base); > + writel(0, drvdata->base + TGU_CONTROL); > + TGU_LOCK(drvdata->base); > + > + drvdata->enabled = false; > +} > + > +static ssize_t enable_tgu_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct tgu_drvdata *drvdata = dev_get_drvdata(dev); > + bool enabled; > + > + scoped_guard(mutex, &drvdata->lock) > + enabled = drvdata->enabled; > + > + return sysfs_emit(buf, "%d\n", enabled); > +} > + > +/* enable_tgu_store - Enable or disable the Trigger Generation Unit (TGU). */ > +static ssize_t enable_tgu_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, > + size_t size) > +{ > + struct tgu_drvdata *drvdata = dev_get_drvdata(dev); > + unsigned long val; > + int ret; > + > + ret = kstrtoul(buf, 0, &val); > + if (ret || val > 1) > + return -EINVAL; > + > + /* > + * Hold the mutex across the whole check-and-act sequence: the check of > + * drvdata->enabled, the PM runtime reference counting and the state > + * update must be atomic, otherwise concurrent writers could each > + * imbalance the PM runtime usage counter. > + */ > + guard(mutex)(&drvdata->lock); > + > + if (val) { > + if (drvdata->enabled) > + return -EBUSY; > + > + ret = pm_runtime_resume_and_get(dev); > + if (ret) > + return ret; > + > + tgu_enable(drvdata); > + } else { > + if (!drvdata->enabled) > + return -EINVAL; > + > + tgu_disable(drvdata); > + pm_runtime_put(dev); > + } > + > + return size; > +} > +static DEVICE_ATTR_RW(enable_tgu); > + > +static struct attribute *tgu_common_attrs[] = { > + &dev_attr_enable_tgu.attr, > + NULL, > +}; > + > +static const struct attribute_group tgu_common_grp = { > + .attrs = tgu_common_attrs, > +}; > + > +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) > +{ > + struct device *dev = &adev->dev; > + struct tgu_drvdata *drvdata; > + int ret; > + > + 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 (IS_ERR(drvdata->base)) > + return PTR_ERR(drvdata->base); > + > + ret = devm_mutex_init(dev, &drvdata->lock); > + if (ret) > + return ret; > + > + drvdata->enabled = false; > + > + ret = sysfs_create_groups(&dev->kobj, tgu_attr_groups); > + if (ret) { > + dev_err(dev, "failed to create sysfs groups: %d\n", ret); > + return ret; > + } > + > + pm_runtime_put(&adev->dev); > + > + return 0; > +} > + > +static void tgu_remove(struct amba_device *adev) > +{ > + struct device *dev = &adev->dev; > + struct tgu_drvdata *drvdata = dev_get_drvdata(dev); > + > + sysfs_remove_groups(&dev->kobj, tgu_attr_groups); > + > + /* > + * If the device is still enabled through sysfs, disable the hardware > + * and drop the PM runtime reference taken in enable_tgu_store() so the > + * usage counter stays balanced. Use the synchronous put: the driver core > + * disables runtime PM right after ->remove() returns, and the barrier in > + * pm_runtime_disable() would cancel a still-queued async idle/suspend, > + * leaving the runtime-PM state inconsistent. > + */ > + scoped_guard(mutex, &drvdata->lock) { > + if (drvdata->enabled) { > + tgu_disable(drvdata); > + pm_runtime_put_sync(dev); > + } > + } > +} > + > +static const struct amba_id tgu_ids[] = { > + { > + .id = 0x000f0e00, > + .mask = 0x000fffff, > + }, > + { 0, 0, NULL }, > +}; > + > +MODULE_DEVICE_TABLE(amba, tgu_ids); > + > +static struct amba_driver tgu_driver = { > + .drv = { > + .name = "qcom-tgu", > + .suppress_bind_attrs = true, > + }, > + .probe = tgu_probe, > + .remove = tgu_remove, > + .id_table = tgu_ids, > +}; > + > +module_amba_driver(tgu_driver); > + > +MODULE_AUTHOR("Songwei Chai <songwei.chai(a)oss.qualcomm.com>"); > +MODULE_AUTHOR("Jinlong Mao <jinlong.mao(a)oss.qualcomm.com>"); > +MODULE_DESCRIPTION("Qualcomm Trigger Generation Unit driver"); > +MODULE_LICENSE("GPL"); > diff --git a/drivers/hwtracing/qcom/tgu.h b/drivers/hwtracing/qcom/tgu.h > new file mode 100644 > index 000000000000..ebba49402a20 > --- /dev/null > +++ b/drivers/hwtracing/qcom/tgu.h > @@ -0,0 +1,51 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries. > + */ > + > +#ifndef _QCOM_TGU_H > +#define _QCOM_TGU_H > + > +/* Register addresses */ > +#define TGU_CONTROL 0x0000 > +#define TGU_LAR 0xfb0 > +#define TGU_UNLOCK_OFFSET 0xc5acce55 > + > +static inline void TGU_LOCK(void __iomem *addr) > +{ > + do { > + /* Wait for things to settle */ > + mb(); > + writel_relaxed(0x0, addr + TGU_LAR); > + } while (0); > +} This is simply replicating the CS_LOCK/CS_UNLOCK. I don't mind those (and relevant definitions) to be moved into include/linux/coresight.h and reused for your driver. The rest looks fine by me. Suzuki > + > +static inline void TGU_UNLOCK(void __iomem *addr) > +{ > + do { > + writel_relaxed(TGU_UNLOCK_OFFSET, addr + TGU_LAR); > + /* Make sure everyone has seen this */ > + mb(); > + } while (0); > +} > + > +/** > + * struct tgu_drvdata - Data structure for a TGU (Trigger Generation Unit) > + * @base: Memory-mapped base address of the TGU device > + * @dev: Pointer to the associated device structure > + * @lock: Mutex serialising enable/disable and the associated PM runtime > + * reference counting against concurrent sysfs access > + * @enabled: Flag indicating whether the TGU device is enabled > + * > + * This structure defines the data associated with a TGU device, > + * including its base address, device pointer, the lock serialising > + * enable/disable, and the enable status. > + */ > +struct tgu_drvdata { > + void __iomem *base; > + struct device *dev; > + struct mutex lock; > + bool enabled; > +}; > + > +#endif

1 week, 1 day

Re: [PATCH v4 3/7] coresight: syscfg: Match feature capabilities separately from device class

by Suzuki K Poulose

On 26/10/2026 05:48, Amir Ayupov wrote: > A feature loads on a device when the two share any match flag. That works > for device classes, where a feature wants any one of them, but not for > capabilities a device may or may not have: a feature that sets a class > and a capability would still load on a device of that class without the > capability. > > Split the match flags into CS_CFG_MATCH_CLASS_MASK for device classes and > CS_CFG_MATCH_CAP_MASK for capabilities. A feature now loads on a device > that shares one of its classes and has all of its capabilities. Check > this once in cscfg_load_feat_csdev(), which both the feature and the > device registration paths call. No capability flags are defined yet, so > matching is unchanged. > > Assisted-by: Devmate:Opus-5.5 > Signed-off-by: Amir Ayupov <aaupov(a)fb.com> > --- > .../hwtracing/coresight/coresight-config.h | 8 ++++ > .../hwtracing/coresight/coresight-syscfg.c | 37 +++++++++++++------ > 2 files changed, 34 insertions(+), 11 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-config.h b/drivers/hwtracing/coresight/coresight-config.h > index ae6295e308131..f04e73b91d9a6 100644 > --- a/drivers/hwtracing/coresight/coresight-config.h > +++ b/drivers/hwtracing/coresight/coresight-config.h > @@ -7,6 +7,7 @@ > #ifndef _CORESIGHT_CORESIGHT_CONFIG_H > #define _CORESIGHT_CORESIGHT_CONFIG_H > > +#include <linux/bits.h> > #include <linux/cleanup.h> > #include <linux/coresight.h> > #include <linux/types.h> > @@ -30,6 +31,13 @@ > */ > #define CS_CFG_MATCH_CLASS_SRC_ALL 0x0001 /* match any source */ > #define CS_CFG_MATCH_CLASS_SRC_ETM4 0x0002 /* match any ETMv4 device */ > +#define CS_CFG_MATCH_CLASS_MASK GENMASK_U32(7, 0) > + > +/* > + * flags defining device capabilities a feature can require. A feature loads on > + * a device that matches one of its classes and has all of its capabilities. > + */ > +#define CS_CFG_MATCH_CAP_MASK GENMASK_U32(15, 8) > > /* flags defining device instance matching - used in config match desc data. */ > #define CS_CFG_MATCH_INST_ANY 0x80000000 /* any instance of a class */ > diff --git a/drivers/hwtracing/coresight/coresight-syscfg.c b/drivers/hwtracing/coresight/coresight-syscfg.c > index a816c22c87bc8..3ee104e94a4cd 100644 > --- a/drivers/hwtracing/coresight/coresight-syscfg.c > +++ b/drivers/hwtracing/coresight/coresight-syscfg.c > @@ -171,8 +171,22 @@ cscfg_alloc_csdev_feat(struct coresight_device *csdev, struct cscfg_feature_desc > return feat_csdev; > } > > -/* load one feature into one coresight device */ > +static bool cscfg_match_feat_csdev(u32 feat_flags, u32 csdev_flags) > +{ > + /* The device must be in one of the feature's classes */ > + if (!(feat_flags & csdev_flags & CS_CFG_MATCH_CLASS_MASK)) > + return false; > + > + /* The device must have every capability the feature requires */ > + if (feat_flags & ~csdev_flags & CS_CFG_MATCH_CAP_MASK) > + return false; Please could you address the following comment ? https://lore.kernel.org/linux-arm-kernel/82adee03-a169-4106-99f2-7624a49260… Cheers Suzuki > + > + return true; > +} > + > +/* Load the feature into the device if it matches, otherwise skip it. */ > static int cscfg_load_feat_csdev(struct coresight_device *csdev, > + u32 match_flags, > struct cscfg_feature_desc *feat_desc, > struct cscfg_csdev_feat_ops *ops) > { > @@ -183,6 +197,9 @@ static int cscfg_load_feat_csdev(struct coresight_device *csdev, > if (!ops->load_feat) > return -EINVAL; > > + if (!cscfg_match_feat_csdev(feat_desc->match_flags, match_flags)) > + return 0; > + > feat_csdev = cscfg_alloc_csdev_feat(csdev, feat_desc); > if (!feat_csdev) > return -ENOMEM; > @@ -212,11 +229,11 @@ static int cscfg_add_feat_to_csdevs(struct cscfg_feature_desc *feat_desc) > int err; > > list_for_each_entry(csdev_item, &cscfg_mgr->csdev_desc_list, item) { > - if (csdev_item->match_flags & feat_desc->match_flags) { > - err = cscfg_load_feat_csdev(csdev_item->csdev, feat_desc, &csdev_item->ops); > - if (err) > - return err; > - } > + err = cscfg_load_feat_csdev(csdev_item->csdev, > + csdev_item->match_flags, feat_desc, > + &csdev_item->ops); > + if (err) > + return err; > } > return 0; > } > @@ -739,11 +756,9 @@ static int cscfg_add_feats_csdev(struct coresight_device *csdev, > return -EINVAL; > > list_for_each_entry(feat_desc, &cscfg_mgr->feat_desc_list, item) { > - if (feat_desc->match_flags & match_flags) { > - err = cscfg_load_feat_csdev(csdev, feat_desc, ops); > - if (err) > - break; > - } > + err = cscfg_load_feat_csdev(csdev, match_flags, feat_desc, ops); > + if (err) > + break; > } > return err; > }

1 week, 1 day

Re: [PATCH v4 6/7] coresight: etm4x: Report whether the PMU counts the external outputs

by Suzuki K Poulose

On 26/10/2026 05:48, Amir Ayupov wrote: > Some CPUs count the trace unit's external outputs as the PMU common > events TRCEXTOUT0-3 (0x4010-0x4013), which lets a configuration drive a > PMU event from trace unit events. Other CPUs, Neoverse N1 for example, > have no such events, and a feature relying on them would silently do > nothing there. > > Detect at probe, on the trace unit's CPU, whether the PMU counts every > external output: the trace unit has an external output for each of its > events (caps.nr_event), and pmuv3_event_implemented() reports whether > the PMU implements TRCEXTOUTn. Only check ETE: per its specification, an > implemented TRCEXTOUTn event counts external output n, while whether that > output is also an external input is IMPLEMENTATION DEFINED. > > Add a CS_CFG_MATCH_CAP_TRCEXTOUT capability match flag, set it on those > trace units, and show it in the feature's configfs 'matches' attribute. > With capabilities matched separately from the device class, a feature > that requires it only loads on those trace units. > > Assisted-by: Devmate:Opus-5.5 > Signed-off-by: Amir Ayupov <aaupov(a)fb.com> > --- > .../hwtracing/coresight/coresight-config.h | 2 ++ > .../hwtracing/coresight/coresight-etm4x-cfg.c | 6 ++++- > .../coresight/coresight-etm4x-core.c | 26 +++++++++++++++++++ > drivers/hwtracing/coresight/coresight-etm4x.h | 3 +++ > .../coresight/coresight-syscfg-configfs.c | 3 +++ > 5 files changed, 39 insertions(+), 1 deletion(-) > > diff --git a/drivers/hwtracing/coresight/coresight-config.h b/drivers/hwtracing/coresight/coresight-config.h > index 220bc54f8e5e1..4cf53230854d0 100644 > --- a/drivers/hwtracing/coresight/coresight-config.h > +++ b/drivers/hwtracing/coresight/coresight-config.h > @@ -38,6 +38,8 @@ > * a device that matches one of its classes and has all of its capabilities. > */ > #define CS_CFG_MATCH_CAP_MASK GENMASK_U32(15, 8) > +/* the PMU counts the trace unit's external outputs as TRCEXTOUTn */ > +#define CS_CFG_MATCH_CAP_TRCEXTOUT BIT(8) > Please could we rename this to : CS_CFG_MATCH_CAP_ETE_TRCEXTOUT as this is only applicable for ETEs ? > /* > * Limit number of presets in a configuration > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-cfg.c b/drivers/hwtracing/coresight/coresight-etm4x-cfg.c > index 52cb036bb04b9..9e4dc5950c13d 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-cfg.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-cfg.c > @@ -176,9 +176,13 @@ static int etm4_cfg_load_feature(struct coresight_device *csdev, > > int etm4_cscfg_register(struct coresight_device *csdev) > { > + struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > struct cscfg_csdev_feat_ops ops; > + u32 match_flags = CS_CFG_ETM4_MATCH_FLAGS; > > ops.load_feat = &etm4_cfg_load_feature; > + if (drvdata->caps.trcextout) > + match_flags |= CS_CFG_MATCH_CAP_TRCEXTOUT; > > - return cscfg_register_csdev(csdev, CS_CFG_ETM4_MATCH_FLAGS, &ops); > + return cscfg_register_csdev(csdev, match_flags, &ops); > } > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c > index af017bd8a5477..cb5fbb0279b88 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c > @@ -30,6 +30,7 @@ > #include <linux/uaccess.h> > #include <linux/perf_event.h> > #include <linux/perf/arm_pmu.h> > +#include <linux/perf/arm_pmuv3.h> > #include <linux/platform_device.h> > #include <linux/pm_runtime.h> > #include <linux/property.h> > @@ -1399,6 +1400,29 @@ static void etm4_fixup_wrong_ccitmin(struct etmv4_caps *caps) > } > } > > +/* > + * Whether the PMU counts each external output of this trace unit as > + * TRCEXTOUTn. The trace unit has an external output for each of its events. > + * Only ETE is checked: per its specification, an implemented TRCEXTOUTn event > + * counts external output n. > + * > + * Must run on the CPU that owns the trace unit. > + */ > +static bool etm4_pmu_counts_extout(struct etmv4_drvdata *drvdata) > +{ > + int n; > + > + if (!etm4x_is_ete(drvdata) || !drvdata->caps.nr_event) > + return false; > + > + for (n = 0; n < drvdata->caps.nr_event; n++) { > + if (!pmuv3_event_implemented(ARMV8_PMUV3_PERFCTR_TRCEXTOUT0 + n)) > + return false; > + } > + > + return true; > +} > + > static void etm4_init_arch_data(void *info) > { > u32 etmidr0; > @@ -1562,6 +1586,8 @@ static void etm4_init_arch_data(void *info) > /* NUMCNTR, bits[30:28] number of counters available for tracing */ > caps->nr_cntr = FIELD_GET(TRCIDR5_NUMCNTR_MASK, etmidr5); > > + caps->trcextout = etm4_pmu_counts_extout(drvdata); > + > coresight_clear_self_claim_tag_unlocked(csa); > etm4_cs_lock(drvdata, csa); > cpu_detect_trace_filtering(drvdata); > diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h > index 5417280cdb274..33994ad4b2641 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x.h > +++ b/drivers/hwtracing/coresight/coresight-etm4x.h > @@ -868,6 +868,8 @@ enum etm_impdef_type { > * @lpoverride: If the implementation can support low-power state over. > * @skip_power_up: Indicates if an implementation can skip powering up > * the trace unit. > + * @trcextout: The trace unit is an ETE whose PMU counts its external > + * outputs as TRCEXTOUTn. We don't seem to be using this after the CSFG flag initialisation, in which case could we directly call etm4_pmu_counts_extout() from etm4_cscfg_register() ? Suzuki > */ > struct etmv4_caps { > u8 nr_pe; > @@ -907,6 +909,7 @@ struct etmv4_caps { > bool atbtrig : 1; > bool lpoverride : 1; > bool skip_power_up : 1; > + bool trcextout : 1; > }; > > /** > diff --git a/drivers/hwtracing/coresight/coresight-syscfg-configfs.c b/drivers/hwtracing/coresight/coresight-syscfg-configfs.c > index 2b40e556be872..7b3800138f625 100644 > --- a/drivers/hwtracing/coresight/coresight-syscfg-configfs.c > +++ b/drivers/hwtracing/coresight/coresight-syscfg-configfs.c > @@ -250,6 +250,9 @@ static ssize_t cscfg_feat_matches_show(struct config_item *item, char *page) > if (match_flags & CS_CFG_MATCH_CLASS_SRC_ETM4) > used += scnprintf(page + used, PAGE_SIZE - used, "SRC_ETMV4 "); > > + if (match_flags & CS_CFG_MATCH_CAP_TRCEXTOUT) > + used += scnprintf(page + used, PAGE_SIZE - used, "TRCEXTOUT "); > + > used += scnprintf(page + used, PAGE_SIZE - used, "\n"); > return used; > }

1 week, 1 day

[PATCH v4 00/19] perf cs-etm: Per-thread mode fixes and snapshot wrap support

by James Clark

This series depends on: https://lore.kernel.org/linux-perf-users/20261030-perf_cs_etm_fix_non_taken… Fix a few issues with per-thread mode: - On nVHE, everything looks like guest trace instead of host - Tracing multiple threads didn't really work After adding a tests for those, it turns out there were some snapshot bugs not picked up by the other snapshot tests. That showed that the different snapshot searches for Intel BTS and Arm SPE were inconsistent and I think all tracers can benefit from using the same pointer fixup logic, so that gets refactored into the common auxtrace.c. Then SPE can just share the Intel BTS search as it was obviously copied from it originally. IntelPT keeps the more advanced duplicate data search, but in the future that could probably be made the common one and all tracers would benefit from using it (except maybe BTS if it always has a very small buffer, but I doubt the overhead of the duplicate search would be an issue). For now this is more of a refactor rather than behavioral change so don't do that yet. Signed-off-by: James Clark <james.clark(a)linaro.org> --- Changes in v4: - Change test snapshot size to 128K for 64K machines - Put patchset depencency in cover letter text - Link to v3: https://patch.msgid.link/20261102-james-cs-unformatted-per-thread-fix-v3-0-… Changes in v3: - Keep the find_snapshot() callback instead of adding snapshot_has_wrapped() and monotonic_snapshot_head boolean (Adrian) - Add auxtrace_record__default_find_snapshot() for the simple search for BTS and SPE (Adrian) - Add auxtrace_record__snapshot_fixup() for any snapshot search implementation to use to update the pointers (Adrian) - Reword UINT64_MAX timestamp comment (Leo) - Fix disassembly test after rebasing onto exception sample fixes - Only take 'perf_guest' into account if there is no PIDFMT_CTXTID2 - Improve decoding of old files by hard coding queue 0 for per-thread - Link to v2: https://patch.msgid.link/20260821-james-cs-unformatted-per-thread-fix-v2-0-… Changes in v2: - Remove stale "may be reset to -1 in per-thread mode" comment - Set CPU when queueing auxtrace fragments otherwise auxtrace_queues__add_event() overwrites the queue's CPU - Link to v1: https://patch.msgid.link/20260811-james-cs-unformatted-per-thread-fix-v1-0-… --- James Clark (19): perf cs-etm: Fix nVHE per-thread decoding perf cs-etm: Warn for invalid timestamp option perf cs-etm: Turn on context packet timestamps in per-thread mode perf cs-etm: Use per-CPU queues for per-thread mode perf test: cs-etm: Increase window size in disassembly test perf cs-etm: Don't drop previous target for kernel samples perf python: Expose sample instruction length perf cs-etm: Use instruction length in disassembly script perf cs-etm: Increase default timestamp generation period perf auxtrace: Turn Intel BTS snapshot search into a generic one perf arm-spe: Use generic snapshot search perf auxtrace: intel-pt: Use common snapshot pointer fixup perf cs-etm: Queue partial AUX records perf cs-etm: Don't print missing buffers in snapshot mode perf auxtrace: cs-etm: Capture wrapped snapshots perf test: Allow infinite named_thread loops perf test: cs-etm: Add test for per-thread mode perf test: cs-etm: Test multiple per-thread threads perf test: cs-etm: Increase snapshot size tools/perf/Documentation/perf-test.txt | 2 +- tools/perf/arch/arm/util/cs-etm.c | 34 ++++- tools/perf/arch/arm64/util/arm-spe.c | 147 +-------------------- tools/perf/arch/x86/util/intel-bts.c | 115 +--------------- tools/perf/arch/x86/util/intel-pt.c | 21 +-- tools/perf/python/arm-cs-trace-disasm.py | 22 +-- tools/perf/python/perf.pyi | 1 + .../perf/tests/shell/coresight/per-thread-multi.sh | 76 +++++++++++ tools/perf/tests/shell/coresight/per-thread.sh | 48 +++++++ .../perf/tests/shell/coresight/raw_dump_stress.sh | 5 - .../shell/coresight/test_arm_coresight_disasm.sh | 8 +- tools/perf/tests/workloads/named_threads.c | 7 +- tools/perf/util/auxtrace.c | 145 +++++++++++++++++++- tools/perf/util/auxtrace.h | 18 +++ tools/perf/util/cs-etm.c | 113 +++++++++++----- tools/perf/util/python.c | 1 + 16 files changed, 420 insertions(+), 343 deletions(-) --- base-commit: 28ef93607ad52acc8f8be25199711153ba11a0c2 change-id: 20260605-james-cs-unformatted-per-thread-fix-50e723aa7f0e prerequisite-message-id: <20261030-perf_cs_etm_fix_non_taken-v3-0-7629b52ab047(a)arm.com> prerequisite-patch-id: 4ef889ad751e344ae47c1d75a2a2e6cb2eadf6e1 prerequisite-patch-id: 636b39730334cbc5a76642c070e704ffe178e7be prerequisite-patch-id: fb58f6daebdf5a194f8aa56ccc4022f932a5a6e5 prerequisite-patch-id: d240c10c6824f8ae6c3f10a1da01226a957f05fb prerequisite-patch-id: 17b291db261e32dcb0d6f1ab2ed0adb7e0411813 prerequisite-patch-id: 45ae1d523c9e5c8c573309f1dab107a598f3d4f8 prerequisite-patch-id: 39a3eb25b3a45f4481760c25fa270773bcfd94f5 prerequisite-patch-id: 5723a15636c2a28ec40eb6b5c9836d162e971786 prerequisite-patch-id: d18054b586fc38bc832ead185e8c1fbb0ce46f39 prerequisite-patch-id: 6735b192189d7945613458abce3da871cb79b35d prerequisite-patch-id: 387cb2c6181bef8f45a8fb167e8ef86e4a54ff69 prerequisite-patch-id: 50243a8d1739141aa24aa371a8452d4d2d078a99 prerequisite-patch-id: 2ae6f1e7707ba0c71a0678331b02d738fb25cb3c prerequisite-patch-id: 1fa0a90cfbc904c4429c1427409125ac44b27aed Best regards, -- James Clark <james.clark(a)linaro.org>

1 week, 1 day

Re: [PATCH v4 0/7] coresight: Pulse an ETM external output for a PMU event

by Leo Yan

On Sun, Oct 25, 2026 at 10:48:20PM -0700, Amir Ayupov wrote: > Add an ETMv4 configuration, pmu_pulse, that drives trace unit external > output 1 on every traced cycle, so a PMU event counting it (TRCEXTOUT1) > can end a fixed-length ETM window, for example with aux-action=pause. > The feature only loads on an ETE whose PMU counts its external outputs. > > Patch 1 is Leo's fix for TRCIDR0.NUMEVENT decoding, which patch 6 relies > on for the number of external outputs. Maybe it is worth to update patch 03 for Suzuki's comment: https://lore.kernel.org/linux-arm-kernel/82adee03-a169-4106-99f2-7624a49260… This series looks good to me and I have verified it: Tested-by: Leo Yan <leo.yan(a)arm.com>

1 week, 1 day

[PATCH v3 00/18] perf cs-etm: Per-thread mode fixes and snapshot wrap support

by James Clark

Fix a few issues with per-thread mode: - On nVHE, everything looks like guest trace instead of host - Tracing multiple threads didn't really work After adding a tests for those, it turns out there were some snapshot bugs not picked up by the other snapshot tests. That showed that the different snapshot searches for Intel BTS and Arm SPE were inconsistent and I think all tracers can benefit from using the same pointer fixup logic, so that gets refactored into the common auxtrace.c. Then SPE can just share the Intel BTS search as it was obviously copied from it originally. IntelPT keeps the more advanced duplicate data search, but in the future that could probably be made the common one and all tracers would benefit from using it (except maybe BTS if it always has a very small buffer, but I doubt the overhead of the duplicate search would be an issue). For now this is more of a refactor rather than behavioral change so don't do that yet. Signed-off-by: James Clark <james.clark(a)linaro.org> --- Changes in v3: - Keep the find_snapshot() callback instead of adding snapshot_has_wrapped() and monotonic_snapshot_head boolean (Adrian) - Add auxtrace_record__default_find_snapshot() for the simple search for BTS and SPE (Adrian) - Add auxtrace_record__snapshot_fixup() for any snapshot search implementation to use to update the pointers (Adrian) - Reword UINT64_MAX timestamp comment (Leo) - Fix disassembly test after rebasing onto exception sample fixes - Only take 'perf_guest' into account if there is no PIDFMT_CTXTID2 - Improve decoding of old files by hard coding queue 0 for per-thread - Link to v2: https://patch.msgid.link/20260821-james-cs-unformatted-per-thread-fix-v2-0-… Changes in v2: - Remove stale "may be reset to -1 in per-thread mode" comment - Set CPU when queueing auxtrace fragments otherwise auxtrace_queues__add_event() overwrites the queue's CPU - Link to v1: https://patch.msgid.link/20260811-james-cs-unformatted-per-thread-fix-v1-0-… --- James Clark (18): perf cs-etm: Fix nVHE per-thread decoding perf cs-etm: Warn for invalid timestamp option perf cs-etm: Turn on context packet timestamps in per-thread mode perf cs-etm: Use per-CPU queues for per-thread mode perf test: cs-etm: Increase window size in disassembly test perf cs-etm: Don't drop previous target for kernel samples perf python: Expose sample instruction length perf cs-etm: Use instruction length in disassembly script perf cs-etm: Increase default timestamp generation period perf auxtrace: Turn Intel BTS snapshot search into a generic one perf arm-spe: Use generic snapshot search perf auxtrace: intel-pt: Use common snapshot pointer fixup perf cs-etm: Queue partial AUX records perf cs-etm: Don't print missing buffers in snapshot mode perf auxtrace: cs-etm: Capture wrapped snapshots perf test: Allow infinite named_thread loops perf test: cs-etm: Add test for per-thread mode perf test: cs-etm: Test multiple per-thread threads tools/perf/Documentation/perf-test.txt | 2 +- tools/perf/arch/arm/util/cs-etm.c | 34 ++++- tools/perf/arch/arm64/util/arm-spe.c | 147 +-------------------- tools/perf/arch/x86/util/intel-bts.c | 115 +--------------- tools/perf/arch/x86/util/intel-pt.c | 21 +-- tools/perf/python/arm-cs-trace-disasm.py | 22 +-- tools/perf/python/perf.pyi | 1 + .../perf/tests/shell/coresight/per-thread-multi.sh | 76 +++++++++++ tools/perf/tests/shell/coresight/per-thread.sh | 48 +++++++ .../perf/tests/shell/coresight/raw_dump_stress.sh | 5 - .../shell/coresight/test_arm_coresight_disasm.sh | 4 +- tools/perf/tests/workloads/named_threads.c | 7 +- tools/perf/util/auxtrace.c | 145 +++++++++++++++++++- tools/perf/util/auxtrace.h | 18 +++ tools/perf/util/cs-etm.c | 113 +++++++++++----- tools/perf/util/python.c | 1 + 16 files changed, 418 insertions(+), 341 deletions(-) --- base-commit: 790454475c15508d68ccfc4b023642305100f18f change-id: 20260605-james-cs-unformatted-per-thread-fix-50e723aa7f0e prerequisite-message-id: <20261030-perf_cs_etm_fix_non_taken-v3-0-7629b52ab047(a)arm.com> prerequisite-patch-id: 4ef889ad751e344ae47c1d75a2a2e6cb2eadf6e1 prerequisite-patch-id: 636b39730334cbc5a76642c070e704ffe178e7be prerequisite-patch-id: fb58f6daebdf5a194f8aa56ccc4022f932a5a6e5 prerequisite-patch-id: d240c10c6824f8ae6c3f10a1da01226a957f05fb prerequisite-patch-id: 17b291db261e32dcb0d6f1ab2ed0adb7e0411813 prerequisite-patch-id: 45ae1d523c9e5c8c573309f1dab107a598f3d4f8 prerequisite-patch-id: 39a3eb25b3a45f4481760c25fa270773bcfd94f5 prerequisite-patch-id: 5723a15636c2a28ec40eb6b5c9836d162e971786 prerequisite-patch-id: d18054b586fc38bc832ead185e8c1fbb0ce46f39 prerequisite-patch-id: 6735b192189d7945613458abce3da871cb79b35d prerequisite-patch-id: 387cb2c6181bef8f45a8fb167e8ef86e4a54ff69 prerequisite-patch-id: 50243a8d1739141aa24aa371a8452d4d2d078a99 prerequisite-patch-id: 2ae6f1e7707ba0c71a0678331b02d738fb25cb3c prerequisite-patch-id: 1fa0a90cfbc904c4429c1427409125ac44b27aed Best regards, -- James Clark <james.clark(a)linaro.org>

1 week, 1 day

[PATCH 0/9] coresight: Fix assorted bugs reported by Sashiko

by James Clark

Small fixes for bugs in the CoreSight drivers that were found by Sashiko. Mostly reference and resource leaks on error paths, plus a deadlock, a missing lock, and a sysfs store that should return -EBUSY while a sysfs session is enabled. This series depends on "coresight: Prevent per-thread events from sharing a sink" v4 [1]. That series takes coresight_mutex for perf AUX setup and adds coresight_release_path_locked(), which the etm_setup_aux() and coresight_build_path() fixes here are based on. [1]: https://lore.kernel.org/r/20261016-james-cs-multiple-per-threads-v4-0-dedf9… Signed-off-by: James Clark <james.clark(a)linaro.org> --- James Clark (9): coresight: cpu-debug: Fix PM usage counter leak in debug_enable_func() coresight: etm4x: Remove perf symlink if config registration fails coresight: etm4x: Fix truncation of nr_ext_inp coresight: etm-perf: Fix sink device reference leak in etm_setup_aux() coresight: syscfg: Take csdev lock when removing owned features coresight: syscfg: Fix deadlock in cscfg_create_device() error path coresight: Fix resource leaks on coresight_build_path() errors coresight: etm3x: Don't ignore etm_parse_event_config() errors coresight: etm3x: Prohibit modifying seq_curr_state while session is enabled drivers/hwtracing/coresight/coresight-core.c | 6 ++++-- drivers/hwtracing/coresight/coresight-cpu-debug.c | 4 ++-- drivers/hwtracing/coresight/coresight-etm-perf.c | 4 ++++ drivers/hwtracing/coresight/coresight-etm3x-core.c | 6 +++++- drivers/hwtracing/coresight/coresight-etm3x-sysfs.c | 5 +++++ drivers/hwtracing/coresight/coresight-etm4x-core.c | 1 + drivers/hwtracing/coresight/coresight-etm4x.h | 2 +- drivers/hwtracing/coresight/coresight-syscfg.c | 8 +++++++- 8 files changed, 29 insertions(+), 7 deletions(-) --- base-commit: 343cfcbb3ecfdb80aa576a428cfa3b65ad783842 change-id: 20261027-james-cs-sashiko-easy-fixes-1-692d80016a89 prerequisite-message-id: <20261016-james-cs-multiple-per-threads-v4-0-dedf961aca0e(a)linaro.org> prerequisite-patch-id: 81acda380ba25acd3845eb58d5ded812175b9631 prerequisite-patch-id: 430caddc9c87948d4e9a0d1967bf3d8a50bca06b prerequisite-patch-id: 633377150141300f7578b265fcc02eba52e592a8 prerequisite-patch-id: 9a82ffe5d705ac1fdf1250ff6bf242cfcd9d8fbf prerequisite-patch-id: cf211801575d6bd0aa7fc0f6e19c3b90cea446a0 prerequisite-patch-id: d22fee48bf4aa3e01536e4683ccd797b3dbe1c91 prerequisite-patch-id: 55eeffb37cae55a1f47c2827899e0512b47a6efa prerequisite-patch-id: c542d74cd4bc76158d5872bab12b8bd7a00f0c27 prerequisite-patch-id: 71cd37ed83d3e01d7a0321cdfa5cfc5ee113fe7b prerequisite-patch-id: 74c1e3ed6dba4e25bfd3c39f06d43b079eed45e1 Best regards, -- James Clark <james.clark(a)linaro.org>

1 week, 2 days

Re: [PATCH][next] perf cs-etm: Fix spelling mistake "should't" -> "shouldn't"

by James Clark

On 02/11/2026 18:01, Ian Rogers wrote: > On Mon, Nov 2, 2026 at 2:52 AM James Clark <james.clark(a)linaro.org> wrote: >> >> >> >> On 28/10/2026 18:31, Colin Ian King wrote: >>> There is a spelling mistake in a pr_err error message. Fix it. >>> >>> Signed-off-by: Colin Ian King <colin.i.king(a)gmail.com> >>> --- >>> tools/perf/util/cs-etm.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c >>> index bdcf94813129..65bce2450549 100644 >>> --- a/tools/perf/util/cs-etm.c >>> +++ b/tools/perf/util/cs-etm.c >>> @@ -434,7 +434,7 @@ static int cs_etm__map_unformatted_trace_ids(struct cs_etm_auxtrace *etm) >>> continue; >>> >>> if (!intlist__empty(etmq->traceid_list)) { >>> - pr_err("CS_ETM: Unformatted queue should't have trace ID\n"); >>> + pr_err("CS_ETM: Unformatted queue shouldn't have trace ID\n"); >>> return -EINVAL; >>> } >>> >> >> Reviewed-by: James Clark <james.clark(a)linaro.org> >> > > I was curious how this passed Sashiko's review. Did Sashiko flag the > spelling mistake but the fix was missed, or did Sashiko not flag it at > all? > > Thanks! > Ian It reviewed it, but it didn't flag it. What kind of typos is it supposed to look for? I haven't seen it flag one before. I have a spellchecker in my IDE which didn't catch it either because should is correct and t is only 1 letter. Maybe there is a setting I can tweak.

1 week, 4 days

← Newer
1
2
3
4
5
6
7
...
273
Older →

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight