CoreSight

coresight@lists.linaro.org

3 participants
2729 discussions

Re: [PATCH v6 02/13] coresight: etm4x: fix underflow for nrseqstate

by Leo Yan

On Fri, May 15, 2026 at 12:10:45PM +0100, Yeoreum Yun wrote: [...] > TBH, the number of transition is determinied by the MAX number of > SEQ_STATE that's why I think define the ETM_MAX_SEQ_TRANSITIONS with > > #define ETM_MAX_SEQ_TRANSITIONS (ETM_MAX_SEQ_STATE - 1) > > and uses ETM_MAX_SEQ_TRANSITIONS for the TCRSEQEVR and seq_ctrl. > > Thought? Looks good to me.

1 hour, 56 minutes

[PATCH v12 00/28] CoreSight: Refactor power management for CoreSight path

by Leo Yan

This series focuses on CoreSight path power management. The changes can be divided into four parts for review: Patches 01 - 10: Preparison for CPU PM: Fix source disabling on idr_alloc failure. Fix helper enable failure handling. Refactor CPU ID stored in csdev. Move CPU lock to sysfs layer. Move per-CPU source pointer from etm-perf to core layer. Refactor etm-perf to retrieve source via per-CPU's event data for lockless and get source reference during AUX setup. Patches 11 - 13: Refactor CPU idle flow managed in the CoreSight core layer. Patches 14 - 23: Refactor path enable / disable with range, control path during CPU idle. Patches 24 - 25: Support the sink (TRBE) control during CPU idle. Patches 26 - 28: Move CPU hotplug into the core layer, and fix sysfs mode for hotplug. This series is rebased on the coresight-next branch and has been verified on Juno-r2 (ETM + ETR) and FVP RevC (ETE + TRBE). Built successfully for armv7 (ARCH=arm). --- Changes in v12: - Added comments on coresight_{get|put)_percpu_source_ref (Suzuki). - Refined failure handling in path enable (Suzuki). - Added coresight_is_software_source() helper (Suzuki). - Reordered taking ref on csdev and its parent in patch 07. - Define the enum mode with bit flags. - Minor improvements on commit logs. - Rebased on lastest coresight-next. - Link to v11: https://lore.kernel.org/r/20260501-arm_coresight_path_power_management_impr… Changes in v11: - Moved per-CPU source pointer from etm-perf to core (Suzuki). - Added grabbing/ungrabbing csdev for device reference (Suzuki). - Minor refine for error handling and logs in CPU PM (James). - Refactored etm-perf with fetching path/source from event data (Suzuki). - Fixed Helper error handling (sashiko). - Added Jie's test tag (thanks!). - Minor improvement for comments and commit logs. - Link to v10: https://lore.kernel.org/r/20260405-arm_coresight_path_power_management_impr… Changes in v10: - Removed redundant checks in ETMv4 PM callbacks (sashiko). - Added a new const structure etm4_cs_pm_ops (sashiko). - Used fine-grained spinlock on sysfs_active_config (sashiko). - Blocked notification after failures in save / restore to avoid lockups. - Changed Change CPUHP_AP_ARM_CORESIGHT_STARTING to CPUHP_AP_ARM_CORESIGHT_ONLINE so that the CPU hotplug callback runs in the thread context (sashiko). - Link to v9: https://lore.kernel.org/r/20260401-arm_coresight_path_power_management_impr… Signed-off-by: Leo Yan <leo.yan(a)arm.com> --- Jie Gan (1): coresight: Fix source not disabled on idr_alloc_u32 failure Leo Yan (26): coresight: Handle helper enable failure properly coresight: Extract device init into coresight_init_device() coresight: Populate CPU ID into coresight_device coresight: Remove .cpu_id() callback from source ops coresight: Take hotplug lock in enable_source_store() for Sysfs mode coresight: perf: Retrieve path and source from event data coresight: Take a reference on csdev coresight: Move per-CPU source pointer to core layer coresight: Take per-CPU source reference during AUX setup coresight: Register CPU PM notifier in core layer coresight: etm4x: Hook CPU PM callbacks coresight: etm4x: Remove redundant checks in PM save and restore coresight: syscfg: Use IRQ-safe spinlock to protect active variables coresight: Disable source helpers in coresight_disable_path() coresight: Control path with range coresight: Use helpers to fetch first and last nodes coresight: Introduce coresight_enable_source() helper coresight: Save active path for system tracers coresight: etm4x: Set active path on target CPU coresight: etm3x: Set active path on target CPU coresight: sysfs: Use source's path pointer for path control coresight: Control path during CPU idle coresight: Add PM callbacks for sink device coresight: sysfs: Increment refcount only for software source coresight: Move CPU hotplug callbacks to core layer coresight: sysfs: Validate CPU online status for per-CPU sources Yabin Cui (1): coresight: trbe: Save and restore state across CPU low power state drivers/hwtracing/coresight/coresight-catu.c | 2 +- drivers/hwtracing/coresight/coresight-core.c | 548 ++++++++++++++++++--- drivers/hwtracing/coresight/coresight-cti-core.c | 9 +- drivers/hwtracing/coresight/coresight-etm-perf.c | 285 ++++++----- drivers/hwtracing/coresight/coresight-etm3x-core.c | 73 +-- drivers/hwtracing/coresight/coresight-etm4x-core.c | 166 ++----- drivers/hwtracing/coresight/coresight-priv.h | 6 + drivers/hwtracing/coresight/coresight-syscfg.c | 38 +- drivers/hwtracing/coresight/coresight-syscfg.h | 2 + drivers/hwtracing/coresight/coresight-sysfs.c | 131 ++--- drivers/hwtracing/coresight/coresight-trbe.c | 61 ++- include/linux/coresight.h | 27 +- include/linux/cpuhotplug.h | 2 +- 13 files changed, 870 insertions(+), 480 deletions(-) --- base-commit: 0ec0a8785d21f63db520bd9d2a67c55e855d36a8 change-id: 20251104-arm_coresight_path_power_management_improvement-dab4966f8280 Best regards, -- Leo Yan <leo.yan(a)arm.com>

4 hours, 7 minutes

Re: [PATCH v6 04/13] coresight: etm4x: exclude ss_status from drvdata->config

by Leo Yan

On Sat, May 09, 2026 at 12:55:19PM +0100, Yeoreum Yun wrote: > > > caps->nr_ss_cmp = FIELD_GET(TRCIDR4_NUMSSCC_MASK, etmidr4); > > > for (i = 0; i < caps->nr_ss_cmp; i++) { > > > - drvdata->config.ss_status[i] = > > > - etm4x_relaxed_read32(csa, TRCSSCSRn(i)); > > > + drvdata->ss_status[i] = etm4x_relaxed_read32(csa, TRCSSCSRn(i)); > > > + drvdata->ss_status[i] &= (TRCSSCSRn_PC | TRCSSCSRn_DV | > > > + TRCSSCSRn_DA | TRCSSCSRn_INST); > > > > It is fine for read these capacity bits when probe, but we need to clear > > status when a session is starting to avoid the stale value left from > > previous session: > > > > drvdata->ss_status[idx] &= ~(TRCSSCSRn_STATUS | TRCSSCSRn_PENDING); > But, I want to clarify that the perf is one of exceptional case > since the "etm4_parse_event_config()" is called at the "resume" of session > for per-thread mode event. Good point! The right way would move etm4_parse_event_config() to setup AUX phase (e.g., call it when build path). > Anyway as we discussed, since now there have been no issue > relavant for those bits, let the clear drvdata->ss_status at the > etm4_parse_event_config(). I am fine to clear status in etm4_parse_event_config() in this series. Thanks, Leo

5 hours, 20 minutes

Re: [PATCH v6 09/13] coresight: etm4x: missing cscfg_csdev_disable_active_config() in perf enable

by Leo Yan

On Wed, Apr 22, 2026 at 02:21:59PM +0100, Yeoreum Yun wrote: [...] > @@ -895,6 +895,8 @@ static int etm4_parse_event_config(struct coresight_device *csdev, > * Missing BB support could cause silent decode errors > * so fail to open if it's not supported. > */ > + if (cfg_hash) > + cscfg_csdev_disable_active_config(csdev); I prefer do a bit refactoring for this. we just save cfg_hash and cfg_preset into drvdata in etm4_parse_event_config(): drvdata->cfg_hash = ATTR_CFG_GET_FLD(attr, configid); if (drvdata->cfg_hash) drvdata->preset = ATTR_CFG_GET_FLD(attr, preset); Then create two helpers: etm4_cscfg_enable(csdev) { struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); return cscfg_csdev_enable_active_config(csdev, drvdata->cfg_hash, drvdata->preset); } etm4_cscfg_disable(csdev) { cscfg_csdev_disable_active_config(csdev); } These helpers will be used by etm4_{enable|disable}_perf() and etm4_{enable|disable}_sysfs(). This might benefit the future cscfg refactoring. Thanks, Leo P.s. I will stop review at here. I assume ETMv3 changes just mirror ETMv4's. So all ETMv4's comments would apply on ETMv3 patches. If I miss anything, please let me know.

5 hours, 46 minutes

Re: [PATCH v6 08/13] coresight: etm4x: remove redundant call etm4_enable_hw() with hotplug

by Leo Yan

On Wed, Apr 22, 2026 at 02:21:58PM +0100, Yeoreum Yun wrote: [...] > + /* > + * Take the hotplug lock to prevent redundant calls to etm4_enable_hw(). > + * > + * The cpu_online_mask is set at the CPUHP_BRINGUP_CPU step. > + * In other words, if etm4_enable_sysfs() is called between > + * CPUHP_BRINGUP_CPU and CPUHP_AP_ARM_CORESIGHT_STARTING, > + * etm4_enable_hw() may be invoked in etm4_enable_sysfs_smp_call() > + * and then executed again in etm4_starting_cpu(). > + */ > + cpus_read_lock(); > ret = smp_call_function_single(drvdata->cpu, > etm4_enable_sysfs_smp_call, &arg, 1); > + cpus_read_unlock(); This will cause double deadlock with the patch: https://lore.kernel.org/linux-arm-kernel/20260511-arm_coresight_path_power_… I think we need to drop this one. Thanks, Leo

6 hours, 17 minutes

Re: [PATCH v6 07/13] coresight: etm4x: fix inconsistencies with sysfs configuration

by Leo Yan

On Wed, Apr 22, 2026 at 02:21:57PM +0100, Yeoreum Yun wrote: [...] > - Since active_config and related fields are accessed only by the local CPU > in etm4_enable/disable_sysfs_smp_call() (similar to perf enable/disable), > remove the lock/unlock from the sysfs enable/disable path and > startup/dying_cpu except when to access config fields. Thanks for writing this up, which is helpful for understanding. [...] > @@ -918,40 +948,29 @@ static int etm4_enable_sysfs(struct coresight_device *csdev, struct coresight_pa > > /* enable any config activated by configfs */ > cscfg_config_sysfs_get_active_cfg(&cfg_hash, &preset); With the patch [1], we can move cscfg_config_sysfs_get_active_cfg() to smp call. As a result, all things for enabling cscfg can be in the same place. [1] https://lore.kernel.org/linux-arm-kernel/20260511-arm_coresight_path_power_… > - if (cfg_hash) { > - ret = cscfg_csdev_enable_active_config(csdev, cfg_hash, preset); > - if (ret) { > - etm4_release_trace_id(drvdata); > - return ret; > - } > - } > - > - raw_spin_lock(&drvdata->spinlock); > - > - drvdata->trcid = path->trace_id; > - > - /* Tracer will never be paused in sysfs mode */ > - drvdata->paused = false; > > /* > * Executing etm4_enable_hw on the cpu whose ETM is being enabled > * ensures that register writes occur when cpu is powered. > */ > arg.drvdata = drvdata; > + arg.path = path; > + arg.cfg_hash = cfg_hash; > + arg.preset = preset; Connect with the comment above , don't need to pass cfg_hash/preset anymore. > + raw_spin_lock(&drvdata->spinlock); > + arg.config = drvdata->config; > + raw_spin_unlock(&drvdata->spinlock); Seems to me, this is right way for locking - here we simply use spinlock for exclusive access config from sysfs knobs. However, we avoid the config copy and directly access in SMP call? we still can use the raw spinlock in SMP call. My suggestion is: - First use a patch to move the drvdata assignment to SMP call and remove spinlock; - Then, rebase this patch for moving cscfg into SMP call. If so, we only need add a new field "arg->path", right? > @@ -1857,13 +1875,11 @@ static int etm4_starting_cpu(unsigned int cpu) > if (!etmdrvdata[cpu]) > return 0; > > - raw_spin_lock(&etmdrvdata[cpu]->spinlock); With the change [2], the starting and dying functions have been removed. If you rebase patches on the top PM series, then you will not bother this. Anyway, this is right to remove spinlock for hotplug notifiers. [2] https://lore.kernel.org/linux-arm-kernel/20260511-arm_coresight_path_power_…

6 hours, 32 minutes

Re: [PATCH] coresight: trbe: Hide enable_sink sysfs file

by Leo Yan

On Thu, May 07, 2026 at 11:53:45AM +0100, James Clark wrote: [...] > +static umode_t coresight_attr_is_visible(struct kobject *kobj, > + struct attribute *attr, int n) > { > struct device *dev = kobj_to_dev(kobj); > + struct coresight_device *csdev = to_coresight_device(dev); > > if (attr == &dev_attr_label.attr) { > if (fwnode_property_present(dev_fwnode(dev), "label")) > return attr->mode; > else > return 0; > + } else if (attr == &dev_attr_enable_sink.attr || > + attr == &dev_attr_enable_source.attr) { > + if (csdev->no_sysfs_mode) > + return 0; > + else > + return attr->mode; I'd prefer no_sysfs_mode to work as a general flag rather than being limited to sink/source devices only. Otherwise, LGTM. Thanks, Leo

2 days, 5 hours

Re: [PATCH v4] coresight: fix missing error code when trace ID is invalid

by Suzuki K Poulose

On Tue, 12 May 2026 09:56:07 +0800, Jie Gan wrote: > When coresight_path_assign_trace_id() cannot assign a valid trace ID, > coresight_enable_sysfs() takes the err_path goto with ret still 0, > returning success to the caller despite no trace session being started. > > Change coresight_path_assign_trace_id() to return int, moving the > IS_VALID_CS_TRACE_ID() check inside it so it returns -EINVAL on failure > and 0 on success. Update both callers to propagate this return value > directly instead of inspecting path->trace_id after the call. > > [...] Applied, thanks! [1/1] coresight: fix missing error code when trace ID is invalid https://git.kernel.org/coresight/c/f4526ffee6ff Best regards, -- Suzuki K Poulose <suzuki.poulose(a)arm.com>

2 days, 23 hours

Re: [PATCH v4] coresight: fix missing error code when trace ID is invalid

by Leo Yan

On Tue, May 12, 2026 at 09:56:07AM +0800, Jie Gan wrote: > When coresight_path_assign_trace_id() cannot assign a valid trace ID, > coresight_enable_sysfs() takes the err_path goto with ret still 0, > returning success to the caller despite no trace session being started. > > Change coresight_path_assign_trace_id() to return int, moving the > IS_VALID_CS_TRACE_ID() check inside it so it returns -EINVAL on failure > and 0 on success. Update both callers to propagate this return value > directly instead of inspecting path->trace_id after the call. > > Fixes: d87d76d823d1 ("Coresight: Allocate trace ID after building the path") > Reviewed-by: James Clark <james.clark(a)linaro.org> > Reviewed-by: Richard Cheng <icheng(a)nvidia.com> > Signed-off-by: Jie Gan <jie.gan(a)oss.qualcomm.com> Reviewed-by: Leo Yan <leo.yan(a)arm.com>

3 days

Re: [PATCH v3] coresight: fix missing error code when trace ID is invalid

by Leo Yan

On Mon, May 11, 2026 at 05:27:10PM +0800, Jie Gan wrote: [...] > > > @@ -755,10 +755,16 @@ void coresight_path_assign_trace_id(struct coresight_path *path, > > > * Non 0 is either success or fail. > > > */ > > > if (trace_id != 0) { > > > - path->trace_id = trace_id; > > > - return; > > > + if (IS_VALID_CS_TRACE_ID(trace_id)) { > > > + path->trace_id = trace_id; > > > + return 0; > > > + } > > > + > > > + return -EINVAL; I'd advocate a bit early exit style, like: /* 0 means the device has no ID assignment, so keep searching */ if (trace_id == 0) continue; if (!IS_VALID_CS_TRACE_ID(trace_id)) return -EINVAL; path->trace_id = trace_id; return 0; Early exit can reduce indentation depth, and it handles simple cases first and then the complex logic. In some cases (maye not this case), we may benefit a bit from compiler optimization [1]. [1] https://xania.org/202512/18-partial-inlining [...] > The return value has been ignored in perf mode. It will introduce noisy by > adding __must_check. So I think its better without __must_check? Wouldn't it need to update perf mode as well? Regarding __must_check, I searched Documentation but didn't find guidance on when it should be used. I don't want to use this annotation randomly (some functions use it and some not), this will be hard for everyone to follow up. IMO, it's fine not to use __must_check here. I would leave this to Suzuki and other maintainers if have different opinions. Thanks, Leo

4 days

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight