On Mon, Oct 12, 2026 at 04:05:22PM -0700, Amir Ayupov wrote:
[...]
--- a/drivers/hwtracing/coresight/coresight-config.h +++ b/drivers/hwtracing/coresight/coresight-config.h @@ -35,6 +35,10 @@
- a device that matches one of its classes and has all of its capabilities.
*/ #define CS_CFG_MATCH_CAP_MASK 0x0000ff00 +/* trace unit external output n is countable by the PMU as TRCEXTOUTn */ +#define CS_CFG_MATCH_CAP_PMU_EXTOUT_SHIFT 8 +#define CS_CFG_MATCH_CAP_PMU_EXTOUT_NR 4 /* TRCEXTOUT0-3 */ +#define CS_CFG_MATCH_CAP_PMU_EXTOUT(n) BIT(CS_CFG_MATCH_CAP_PMU_EXTOUT_SHIFT + (n))
We can just use a single capacity for all output singals. It would be good to follow the naming convention in the Arm ARM:
#define CS_CFG_MATCH_CAP_TRCEXTOUT BIT(8)
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;
- match_flags |= (u32)drvdata->pmu_extout << CS_CFG_MATCH_CAP_PMU_EXTOUT_SHIFT;
- return cscfg_register_csdev(csdev, CS_CFG_ETM4_MATCH_FLAGS, &ops);
- return cscfg_register_csdev(csdev, match_flags, &ops);
I would suggest to enable the capability bit base on drvdata->caps:
if (etm4x_is_ete(drvdata) && drvdata->caps.nr_events) match_flags |= CS_CFG_MATCH_CAP_TRCEXTOUT;
For now, we can enable TRCEXTOUT for ETE only, as this should be sufficient to detect the feature safely. I'd like to leave this to James and Mike as I got suggestions from them.
Please also rebase onto the latest CoreSight next branch [1], which moves the capability fields into drvdata->caps. I found an issue in the decoding of caps.nr_event; the attached patch fixes it and makes the field hold the actual event count.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/coresight/linux.git branch: next
---8<---
From 95307fa0574b1e0b3922ffa23fe98144b2688496 Mon Sep 17 00:00:00 2001 From: Leo Yan leo.yan@arm.com Date: Wed, 21 Oct 2026 17:05:18 +0100 Subject: [PATCH] coresight: etm4x: Fix event count decoding
TRCIDR0.NUMEVENT normally encodes the number of events minus one. From ETMv4.3 onwards, including ETE, a zero TRCIDR4.NUMRSPAIR instead means that no events are implemented and NUMEVENT is zero. The sysfs event controls currently interpret this combination as one event and allow writes to unsupported event fields.
Decode NUMEVENT after NUMRSPAIR and store the actual number of events in nr_event. Use the decoded resource count so that older ETM versions, where a zero NUMRSPAIR represents one resource pair, still report at least one event. Update both sysfs event controls to use the actual count and clear their event fields when no events are implemented.
Also correct the instruction event enable mask for the single-event case to use INSTEN[0]. The existing mask uses INSTEN[1], preventing event 0 from being enabled.
This change depends on the recent driver refactoring. With no reported failures, it is not intended for stable backporting.
Signed-off-by: Leo Yan leo.yan@arm.com --- .../coresight/coresight-etm4x-core.c | 13 +++++++-- .../coresight/coresight-etm4x-sysfs.c | 29 ++++++++++--------- 2 files changed, 27 insertions(+), 15 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index e21be6468e954..af017bd8a5477 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -1451,8 +1451,6 @@ static void etm4_init_arch_data(void *info) caps->trccci = !!(etmidr0 & TRCIDR0_TRCCCI); /* RETSTACK, bit[9] Return stack bit */ caps->retstack = !!(etmidr0 & TRCIDR0_RETSTACK); - /* NUMEVENT, bits[11:10] Number of events field */ - caps->nr_event = FIELD_GET(TRCIDR0_NUMEVENT_MASK, etmidr0); /* QSUPP, bits[16:15] Q element support field */ caps->q_support = FIELD_GET(TRCIDR0_QSUPP_MASK, etmidr0); if (caps->q_support) @@ -1519,6 +1517,17 @@ static void etm4_init_arch_data(void *info) caps->nr_resource = FIELD_GET(TRCIDR4_NUMRSPAIR_MASK, etmidr4); if ((drvdata->arch < ETM_ARCH_V4_3) || (caps->nr_resource > 0)) caps->nr_resource += 1; + + /* + * NUMEVENT encodes the number of events minus one. From ETMv4.3, + * including ETE, NUMRSPAIR == 0 means no events are implemented. + * nr_resource already accounts for the pre-ETMv4.3 encoding. + */ + if (caps->nr_resource) + caps->nr_event = FIELD_GET(TRCIDR0_NUMEVENT_MASK, etmidr0) + 1; + else + caps->nr_event = 0; + /* * NUMSSCC, bits[23:20] the number of single-shot * comparator control for tracing. Read any status regs as these diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c index db9e603f97ba8..fb38f0102290a 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c @@ -513,19 +513,22 @@ static ssize_t event_store(struct device *dev,
raw_spin_lock(&drvdata->spinlock); switch (caps->nr_event) { - case 0x0: + case 0: + sysfs_config->eventctrl0 = 0; + break; + case 1: /* EVENT0, bits[7:0] */ sysfs_config->eventctrl0 = val & 0xFF; break; - case 0x1: - /* EVENT1, bits[15:8] */ + case 2: + /* EVENT1, bits[15:8] */ sysfs_config->eventctrl0 = val & 0xFFFF; break; - case 0x2: + case 3: /* EVENT2, bits[23:16] */ sysfs_config->eventctrl0 = val & 0xFFFFFF; break; - case 0x3: + case 4: /* EVENT3, bits[31:24] */ sysfs_config->eventctrl0 = val; break; @@ -565,22 +568,22 @@ static ssize_t event_instren_store(struct device *dev, /* start by clearing all instruction event enable bits */ sysfs_config->eventctrl1 &= ~TRCEVENTCTL1R_INSTEN_MASK; switch (caps->nr_event) { - case 0x0: - /* generate Event element for event 1 */ - sysfs_config->eventctrl1 |= val & TRCEVENTCTL1R_INSTEN_1; + case 1: + /* generate Event element for event 0 */ + sysfs_config->eventctrl1 |= val & TRCEVENTCTL1R_INSTEN_0; break; - case 0x1: - /* generate Event element for event 1 and 2 */ + case 2: + /* generate Event element for events 0 and 1 */ sysfs_config->eventctrl1 |= val & (TRCEVENTCTL1R_INSTEN_0 | TRCEVENTCTL1R_INSTEN_1); break; - case 0x2: - /* generate Event element for event 1, 2 and 3 */ + case 3: + /* generate Event element for events 0, 1 and 2 */ sysfs_config->eventctrl1 |= val & (TRCEVENTCTL1R_INSTEN_0 | TRCEVENTCTL1R_INSTEN_1 | TRCEVENTCTL1R_INSTEN_2); break; - case 0x3: + case 4: /* generate Event element for all 4 events */ sysfs_config->eventctrl1 |= val & (TRCEVENTCTL1R_INSTEN_0 | TRCEVENTCTL1R_INSTEN_1 |
On 21/10/2026 17:43, Leo Yan wrote:
On Mon, Oct 12, 2026 at 04:05:22PM -0700, Amir Ayupov wrote:
[...]
--- a/drivers/hwtracing/coresight/coresight-config.h +++ b/drivers/hwtracing/coresight/coresight-config.h @@ -35,6 +35,10 @@
- a device that matches one of its classes and has all of its capabilities.
*/ #define CS_CFG_MATCH_CAP_MASK 0x0000ff00 +/* trace unit external output n is countable by the PMU as TRCEXTOUTn */ +#define CS_CFG_MATCH_CAP_PMU_EXTOUT_SHIFT 8 +#define CS_CFG_MATCH_CAP_PMU_EXTOUT_NR 4 /* TRCEXTOUT0-3 */ +#define CS_CFG_MATCH_CAP_PMU_EXTOUT(n) BIT(CS_CFG_MATCH_CAP_PMU_EXTOUT_SHIFT + (n))
Isn't this recreating what GENMASK() and FIELD_PREP() does? Can we use that instead?
We can just use a single capacity for all output singals. It would be good to follow the naming convention in the Arm ARM:
#define CS_CFG_MATCH_CAP_TRCEXTOUT BIT(8)
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;
- match_flags |= (u32)drvdata->pmu_extout << CS_CFG_MATCH_CAP_PMU_EXTOUT_SHIFT;
- return cscfg_register_csdev(csdev, CS_CFG_ETM4_MATCH_FLAGS, &ops);
- return cscfg_register_csdev(csdev, match_flags, &ops);
I would suggest to enable the capability bit base on drvdata->caps:
if (etm4x_is_ete(drvdata) && drvdata->caps.nr_events) match_flags |= CS_CFG_MATCH_CAP_TRCEXTOUT;
For now, we can enable TRCEXTOUT for ETE only, as this should be sufficient to detect the feature safely. I'd like to leave this to James and Mike as I got suggestions from them.
ETE only makes sense to me.
Please also rebase onto the latest CoreSight next branch [1], which moves the capability fields into drvdata->caps. I found an issue in the decoding of caps.nr_event; the attached patch fixes it and makes the field hold the actual event count.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/coresight/linux.git branch: next
---8<---
From 95307fa0574b1e0b3922ffa23fe98144b2688496 Mon Sep 17 00:00:00 2001 From: Leo Yan leo.yan@arm.com Date: Wed, 21 Oct 2026 17:05:18 +0100 Subject: [PATCH] coresight: etm4x: Fix event count decoding
TRCIDR0.NUMEVENT normally encodes the number of events minus one. From ETMv4.3 onwards, including ETE, a zero TRCIDR4.NUMRSPAIR instead means that no events are implemented and NUMEVENT is zero. The sysfs event controls currently interpret this combination as one event and allow writes to unsupported event fields.
Decode NUMEVENT after NUMRSPAIR and store the actual number of events in nr_event. Use the decoded resource count so that older ETM versions, where a zero NUMRSPAIR represents one resource pair, still report at least one event. Update both sysfs event controls to use the actual count and clear their event fields when no events are implemented.
Also correct the instruction event enable mask for the single-event case to use INSTEN[0]. The existing mask uses INSTEN[1], preventing event 0 from being enabled.
This change depends on the recent driver refactoring. With no reported failures, it is not intended for stable backporting.
Signed-off-by: Leo Yan leo.yan@arm.com
.../coresight/coresight-etm4x-core.c | 13 +++++++-- .../coresight/coresight-etm4x-sysfs.c | 29 ++++++++++--------- 2 files changed, 27 insertions(+), 15 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index e21be6468e954..af017bd8a5477 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -1451,8 +1451,6 @@ static void etm4_init_arch_data(void *info) caps->trccci = !!(etmidr0 & TRCIDR0_TRCCCI); /* RETSTACK, bit[9] Return stack bit */ caps->retstack = !!(etmidr0 & TRCIDR0_RETSTACK);
- /* NUMEVENT, bits[11:10] Number of events field */
- caps->nr_event = FIELD_GET(TRCIDR0_NUMEVENT_MASK, etmidr0); /* QSUPP, bits[16:15] Q element support field */ caps->q_support = FIELD_GET(TRCIDR0_QSUPP_MASK, etmidr0); if (caps->q_support)
@@ -1519,6 +1517,17 @@ static void etm4_init_arch_data(void *info) caps->nr_resource = FIELD_GET(TRCIDR4_NUMRSPAIR_MASK, etmidr4); if ((drvdata->arch < ETM_ARCH_V4_3) || (caps->nr_resource > 0)) caps->nr_resource += 1;
- /*
* NUMEVENT encodes the number of events minus one. From ETMv4.3,* including ETE, NUMRSPAIR == 0 means no events are implemented.* nr_resource already accounts for the pre-ETMv4.3 encoding.*/- if (caps->nr_resource)
caps->nr_event = FIELD_GET(TRCIDR0_NUMEVENT_MASK, etmidr0) + 1;- else
caps->nr_event = 0;- /*
- NUMSSCC, bits[23:20] the number of single-shot
- comparator control for tracing. Read any status regs as these
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c index db9e603f97ba8..fb38f0102290a 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c @@ -513,19 +513,22 @@ static ssize_t event_store(struct device *dev, raw_spin_lock(&drvdata->spinlock); switch (caps->nr_event) {
- case 0x0:
- case 0:
sysfs_config->eventctrl0 = 0;break;- case 1: /* EVENT0, bits[7:0] */ sysfs_config->eventctrl0 = val & 0xFF; break;
- case 0x1:
/* EVENT1, bits[15:8] */
- case 2:
/* EVENT1, bits[15:8] */
- case 0x2:
- case 3: /* EVENT2, bits[23:16] */ sysfs_config->eventctrl0 = val & 0xFFFFFF; break;
- case 0x3:
- case 4: /* EVENT3, bits[31:24] */ sysfs_config->eventctrl0 = val; break;
@@ -565,22 +568,22 @@ static ssize_t event_instren_store(struct device *dev, /* start by clearing all instruction event enable bits */ sysfs_config->eventctrl1 &= ~TRCEVENTCTL1R_INSTEN_MASK; switch (caps->nr_event) {
- case 0x0:
/* generate Event element for event 1 */sysfs_config->eventctrl1 |= val & TRCEVENTCTL1R_INSTEN_1;
- case 1:
/* generate Event element for event 0 */sysfs_config->eventctrl1 |= val & TRCEVENTCTL1R_INSTEN_0;
- case 0x1:
/* generate Event element for event 1 and 2 */
- case 2:
/* generate Event element for events 0 and 1 */
- case 0x2:
/* generate Event element for event 1, 2 and 3 */
- case 3:
/* generate Event element for events 0, 1 and 2 */
- case 0x3:
- case 4: /* generate Event element for all 4 events */ sysfs_config->eventctrl1 |= val & (TRCEVENTCTL1R_INSTEN_0 | TRCEVENTCTL1R_INSTEN_1 |
On Fri, Oct 23, 2026 at 10:49:40AM +0100, James Clark wrote:
[...]
- a device that matches one of its classes and has all of its capabilities.
*/ #define CS_CFG_MATCH_CAP_MASK 0x0000ff00 +/* trace unit external output n is countable by the PMU as TRCEXTOUTn */ +#define CS_CFG_MATCH_CAP_PMU_EXTOUT_SHIFT 8 +#define CS_CFG_MATCH_CAP_PMU_EXTOUT_NR 4 /* TRCEXTOUT0-3 */ +#define CS_CFG_MATCH_CAP_PMU_EXTOUT(n) BIT(CS_CFG_MATCH_CAP_PMU_EXTOUT_SHIFT + (n))
Isn't this recreating what GENMASK() and FIELD_PREP() does? Can we use that instead?
TBC, are you suggesting using separate capability bits to represent TRCEXTOUT0-3? Why can't we use a single capability bit to indicate whether the feature is supported and caps->num_event to indicate the number of external output signals?
On 23/10/2026 15:00, Leo Yan wrote:
On Fri, Oct 23, 2026 at 10:49:40AM +0100, James Clark wrote:
[...]
* a device that matches one of its classes and has all of its capabilities. */#define CS_CFG_MATCH_CAP_MASK 0x0000ff00 +/* trace unit external output n is countable by the PMU as TRCEXTOUTn */ +#define CS_CFG_MATCH_CAP_PMU_EXTOUT_SHIFT 8 +#define CS_CFG_MATCH_CAP_PMU_EXTOUT_NR 4 /* TRCEXTOUT0-3 */ +#define CS_CFG_MATCH_CAP_PMU_EXTOUT(n) BIT(CS_CFG_MATCH_CAP_PMU_EXTOUT_SHIFT + (n))
Isn't this recreating what GENMASK() and FIELD_PREP() does? Can we use that instead?
TBC, are you suggesting using separate capability bits to represent TRCEXTOUT0-3? Why can't we use a single capability bit to indicate whether the feature is supported and caps->num_event to indicate the number of external output signals?
Just the pattern of *_SHIFT and other stuff to make a bitfield is usually better done with GENMASK(). Or if there are only 4 of them, even defining four BIT()s would be clearer.
But yes if a single bit and caps->num_event works we should do that instead.
-
James Clark -
Leo Yan