Re: [PATCH 1/2] coresight: etm4x: Report whether the PMU counts external output 1

On 07/10/2026 17:24, Leo Yan wrote:

Hi Amir,

Thanks for the patch. I have a few initial comments below. We may have further feedback after our internal review.

On Wed, Sep 30, 2026 at 11:37:43PM -0700, Amir Ayupov wrote:

[...]

...

#define CS_CFG_MATCH_CLASS_SRC_ALL 0x0001 /* match any source */ #define CS_CFG_MATCH_CLASS_SRC_ETM4 0x0002 /* match any ETMv4 device */ +/* ETM external output 1 is countable by the PMU as TRCEXTOUT1 */ +#define CS_CFG_MATCH_CAP_PMU_EXTOUT1 0x0004

Do we need to tie this capability to TRCEXTOUT1? For example, Neoverse V2 exposes TRCEXTOUT0 through TRCEXTOUT3 as PMU events.

Shouldn't we also use TRCEXTOUT0 instead of 1? Isn't 1 for devices that have two external outputs, but some devices might only have 1 output so only have TRCEXTOUT0?

...

/* 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-etm4x-cfg.c b/drivers/hwtracing/coresight/coresight-etm4x-cfg.c index e1a59b4345052..2847d2d7f7bed 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-cfg.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-cfg.c @@ -174,9 +174,14 @@ 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;

• return cscfg_register_csdev(csdev, CS_CFG_ETM4_MATCH_FLAGS, &ops);

• if (drvdata->pmu_extout1)

• match_flags |= CS_CFG_MATCH_CAP_PMU_EXTOUT1;
  

• return cscfg_register_csdev(csdev, match_flags, &ops);

The current matching logic succeeds if a device and feature share any flag bit. If the feature sets both CS_CFG_MATCH_CLASS_SRC_ETM4 and CS_CFG_MATCH_CAP_PMU_EXTOUT1, it will still load on an ETM4 device that lacks the capability.

Could we check the capability separately when loading the feature, perhaps in cscfg_load_feat_csdev(), and skip it on unsupported devices?

...

+static bool etm4_pmu_has_extout1(struct etmv4_drvdata *drvdata) +{

• int pmuver = read_pmuver();
• if (!is_midr_in_range_list(etm4_pmu_extout1_cpus))

• return false;
  

Based on specific CPU variant, we should already have identified TRCEXTOUT has supported.

So either we only base on MIDR list or we can figure out a reliable way to detect the feature dynamically.

At least with ETE the ARM says:

D4.6.12 External Outputs

SRBKWBThe TRCIDR0.NUMEVENT field shows how many ETEEvents are for the particular implementation

0x4011, TRCEXTOUT1, Trace unit external output 1

D14 PMU Event Descriptions

The counter counts each event signaled by the trace unit on external event 1.

It is IMPLEMENTATION DEFINED whether this event is available as an external input to the ETE.

PMCEID0_EL0[49] reads as 1 if this event is implemented and 0 otherwise.

The number of outputs and the PMU event are both discoverable. It specifically says that only the external input is implementation defined, implying that if it's available it's always connected as an output.

We could leave the MIDR list to only support errata when the external output isn't connected to the PMU event.

...

• /* PMCEID0_EL0[63:32] describe events 0x4000-0x401f from PMUv3p1 */
• if (!pmuv3_implemented(pmuver) || pmuver < ID_AA64DFR0_EL1_PMUVer_V3P1)

• return false;
  

• if (!(read_pmceid0() & BIT_ULL(32 + ARMV8_PMUV3_PERFCTR_TRCEXTOUT1 -

• 		       ARMV8_PMUV3_EXT_COMMON_EVENT_BASE)))
  

• return false;
  

• /* nr_event is TRCIDR0.NUMEVENT, the number of events minus one */
• return drvdata->nr_event >= 1;

+}

[...]

...

@@ -1116,6 +1116,13 @@ int cscfg_csdev_enable_active_config(struct coresight_device *csdev, if (err) cscfg_config_desc_put(config_desc);

• } else {

• /*
  

•  * The configuration is active but was not loaded on this
  

•  * device, for example because the device lacks a capability
  

•  * its features require. Fail rather than trace without it.
  

•  */
  

• err = -EINVAL;
  

  }

This fixes a pre-existing issue. It is worther to put it in a separate patch with fixes tag:

An early return would also avoid the normal enable path's indentation:

if (!config_csdev_active) return -EINVAL;

err = cscfg_csdev_enable_config(config_csdev_active, preset); ...

Thanks, Leo