CoreSight

coresight@lists.linaro.org

3 participants
2706 discussions

[PATCH v11 00/27] 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 - 09: Preparison for CPU PM: 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 10 - 12: Refactor CPU idle flow managed in the CoreSight core layer. Patches 13 - 22: Refactor path enable / disable with range, control path during CPU idle. Patches 23 - 24: Support the sink (TRBE) control during CPU idle. Patches 25 - 27: 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 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… Changes in v9: - Changed to use per-CPU path pointer with lockless access. - Removed the change for adding csdev->path, the related refactoring will be sent separately. - Re-orged patches to avoid intermediate breakage (sashiko). - Link to v8: https://lore.kernel.org/r/20260325-arm_coresight_path_power_management_impr… Changes in v8: - Moved the "cpu" field in coresight_device for better pack with new patch 01 (Suzuki). - Added check if not set CPU for per_cpu_source/per_cpu_sink (Suzuki). - Renamed spinlock name in syscfg (Suzuki). - Refactored paired enable and disable path with new patches 10 and 12 (Suzuki). - Link to v7: https://lore.kernel.org/r/20260320-arm_coresight_path_power_management_impr… Signed-off-by: Leo Yan <leo.yan(a)arm.com> --- 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: Grab per-CPU source device 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: Move source helper disabling to 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 system tracers 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 | 530 ++++++++++++++++++--- drivers/hwtracing/coresight/coresight-cti-core.c | 9 +- drivers/hwtracing/coresight/coresight-etm-perf.c | 268 ++++++----- 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 | 133 ++---- drivers/hwtracing/coresight/coresight-trbe.c | 61 ++- include/linux/coresight.h | 15 +- include/linux/cpuhotplug.h | 2 +- 13 files changed, 833 insertions(+), 472 deletions(-) --- base-commit: 971f3474f8898ae8bbab19a9b547819a5e6fbcf1 change-id: 20251104-arm_coresight_path_power_management_improvement-dab4966f8280 Best regards, -- Leo Yan <leo.yan(a)arm.com>

1 day

[PATCH 0/2] coresight: ete: Always save state on power down

by James Clark

Fix PM save on ETE, which is an issue that showed up on the FVP when booted with ACPI and the newly enabled idle states. Then refactor to clarify and avoid any probe order issues. Signed-off-by: James Clark <james.clark(a)linaro.org> --- James Clark (2): coresight: ete: Always save state on power down coresight: etm4x: Refactor pm_save_enable handling drivers/hwtracing/coresight/coresight-etm4x-core.c | 55 ++++++++++++++++------ drivers/hwtracing/coresight/coresight-etm4x.h | 1 + 2 files changed, 42 insertions(+), 14 deletions(-) --- base-commit: 971f3474f8898ae8bbab19a9b547819a5e6fbcf1 change-id: 20260420-james-cs-ete-pm_save_enable-4e994e35cdac Best regards, -- James Clark <james.clark(a)linaro.org>

1 day

Re: [PATCH v6 00/13] fix several inconsistencies with sysfs configuration in etmX

by Leo Yan

On Fri, May 01, 2026 at 01:55:46PM +0100, Yeoreum Yun wrote: > Gentle ping in case of forgotten. This is on my todo list - let me finish the task in hand and then I will play a bit this series. Thanks, Leo

1 day, 3 hours

Re: [PATCH v8 1/4] coresight: cti: Convert trigger usage fields to dynamic bitmaps and arrays

by Leo Yan

On Tue, Apr 28, 2026 at 10:25:11AM +0800, Yingchao Deng (Consultant) wrote: [...] > > tg->used_mask = bitmap_zalloc(nr_filter_sigs, GFP_KERNEL); > "nr_filter_sigs" is the count of entries in the DT property array, if the DT > property is: > arm,trig-filters = <22 23>; > Here nr_filter_sigs=2, so bitmap_zalloc(2) allocates only 1 unsigned long > (64 bits). set_bit(22/23, used_mask) still fits, but it's logically an OOB, > and any index >=64 would > write past the end. Thanks for explanation. It is correct for me.

4 days, 10 hours

Re: [PATCH v8 4/4] coresight: cti: expose banked sysfs registers for Qualcomm extended CTI

by Leo Yan

On Sun, Apr 26, 2026 at 05:44:41PM +0800, Yingchao Deng wrote: > Qualcomm extended CTI implements banked trigger status and integration > registers, where each bank covers 32 triggers. Multiple instances of > these registers are required to expose the full trigger space. > > Add static sysfs entries for the banked CTI registers and control their > visibility based on the underlying hardware configuration. Numbered > sysfs nodes are hidden on standard ARM CTIs, preserving the existing ABI. > On Qualcomm CTIs, only banked registers backed by hardware are exposed, > with the number of visible banks derived from nr_trig_max. > > This ensures that userspace only sees registers that are actually > implemented, while maintaining compatibility with existing CTI tooling. > > Signed-off-by: Yingchao Deng <yingchao.deng(a)oss.qualcomm.com> > --- > drivers/hwtracing/coresight/coresight-cti-sysfs.c | 58 +++++++++++++++++++++++ > 1 file changed, 58 insertions(+) > > diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c b/drivers/hwtracing/coresight/coresight-cti-sysfs.c > index 8b70e7e38ea3..046757e4e9b6 100644 > --- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c > +++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c > @@ -512,18 +512,36 @@ static struct attribute *coresight_cti_regs_attrs[] = { > &dev_attr_appclear.attr, > &dev_attr_apppulse.attr, > coresight_cti_reg(triginstatus, CTITRIGINSTATUS), > + coresight_cti_reg(triginstatus1, CTI_REG_SET_NR_CONST(CTITRIGINSTATUS, 1)), How about extend the cs_off_attribute struct: struct cs_off_attribute { struct device_attribute attr; u32 off; u32 index; }; // by default, the index is 0 #define coresight_cti_reg(name, offset) \ (&((struct cs_off_attribute[]) { \ { \ __ATTR(name, 0444, coresight_cti_reg_show, NULL), \ offset \ 0 \ } \ })[0].attr.attr) // For the register with index #define coresight_cti_reg_index(name, offset, index) \ (&((struct cs_off_attribute[]) { \ { \ __ATTR(name, 0444, coresight_cti_reg_show, NULL), \ offset \ index \ } \ })[0].attr.attr) coresight_cti_reg_index(triginstatus1, CTITRIGINSTATUS, 1), > + coresight_cti_reg(triginstatus2, CTI_REG_SET_NR_CONST(CTITRIGINSTATUS, 2)), > + coresight_cti_reg(triginstatus3, CTI_REG_SET_NR_CONST(CTITRIGINSTATUS, 3)), > coresight_cti_reg(trigoutstatus, CTITRIGOUTSTATUS), > + coresight_cti_reg(trigoutstatus1, CTI_REG_SET_NR_CONST(CTITRIGOUTSTATUS, 1)), > + coresight_cti_reg(trigoutstatus2, CTI_REG_SET_NR_CONST(CTITRIGOUTSTATUS, 2)), > + coresight_cti_reg(trigoutstatus3, CTI_REG_SET_NR_CONST(CTITRIGOUTSTATUS, 3)), > coresight_cti_reg(chinstatus, CTICHINSTATUS), > coresight_cti_reg(choutstatus, CTICHOUTSTATUS), > #ifdef CONFIG_CORESIGHT_CTI_INTEGRATION_REGS > coresight_cti_reg_rw(itctrl, CORESIGHT_ITCTRL), > coresight_cti_reg(ittrigin, ITTRIGIN), > + coresight_cti_reg(ittrigin1, CTI_REG_SET_NR_CONST(ITTRIGIN, 1)), > + coresight_cti_reg(ittrigin2, CTI_REG_SET_NR_CONST(ITTRIGIN, 2)), > + coresight_cti_reg(ittrigin3, CTI_REG_SET_NR_CONST(ITTRIGIN, 3)), > coresight_cti_reg(itchin, ITCHIN), > coresight_cti_reg_rw(ittrigout, ITTRIGOUT), > + coresight_cti_reg_rw(ittrigout1, CTI_REG_SET_NR_CONST(ITTRIGOUT, 1)), > + coresight_cti_reg_rw(ittrigout2, CTI_REG_SET_NR_CONST(ITTRIGOUT, 2)), > + coresight_cti_reg_rw(ittrigout3, CTI_REG_SET_NR_CONST(ITTRIGOUT, 3)), > coresight_cti_reg_rw(itchout, ITCHOUT), > coresight_cti_reg(itchoutack, ITCHOUTACK), > coresight_cti_reg(ittrigoutack, ITTRIGOUTACK), > + coresight_cti_reg(ittrigoutack1, CTI_REG_SET_NR_CONST(ITTRIGOUTACK, 1)), > + coresight_cti_reg(ittrigoutack2, CTI_REG_SET_NR_CONST(ITTRIGOUTACK, 2)), > + coresight_cti_reg(ittrigoutack3, CTI_REG_SET_NR_CONST(ITTRIGOUTACK, 3)), > coresight_cti_reg_wo(ittriginack, ITTRIGINACK), > + coresight_cti_reg_wo(ittriginack1, CTI_REG_SET_NR_CONST(ITTRIGINACK, 1)), > + coresight_cti_reg_wo(ittriginack2, CTI_REG_SET_NR_CONST(ITTRIGINACK, 2)), > + coresight_cti_reg_wo(ittriginack3, CTI_REG_SET_NR_CONST(ITTRIGINACK, 3)), > coresight_cti_reg_wo(itchinack, ITCHINACK), > #endif > NULL, > @@ -534,10 +552,50 @@ static umode_t coresight_cti_regs_is_visible(struct kobject *kobj, > { > struct device *dev = kobj_to_dev(kobj); > struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent); > + static const char * const qcom_suffix_registers[] = { > + "triginstatus", > + "trigoutstatus", > +#ifdef CONFIG_CORESIGHT_CTI_INTEGRATION_REGS > + "ittrigin", > + "ittrigout", > + "ittriginack", > + "ittrigoutack", > +#endif > + }; > + int i, nr, max_bank; > + size_t len; > > if (attr == &dev_attr_asicctl.attr && !drvdata->config.asicctl_impl) > return 0; > > + /* > + * Banked regs are exposed as <qcom_suffix_registers><nr> (nr = 1..3). > + * - Hide them on standard CTIs. > + * - On QCOM CTIs, hide suffixes beyond the number of banks implied > + * by nr_trig_max (32 triggers per bank). > + */ > + for (i = 0; i < ARRAY_SIZE(qcom_suffix_registers); i++) { This can be general for a register with index? Like: for (i = 0; i < ARRAY_SIZE(registers_with_index); i++) { > + len = strlen(qcom_suffix_registers[i]); > + > + if (strncmp(attr->name, qcom_suffix_registers[i], len)) > + continue; > + > + if (kstrtoint(attr->name + len, 10, &nr)) > + continue; > + > + if (!drvdata->is_qcom_cti) > + return 0; > + > + if (nr < 1 || nr > 3) > + return 0; > + > + max_bank = DIV_ROUND_UP(drvdata->config.nr_trig_max, 32) - 1; > + if (nr > max_bank) > + return 0; Directly check the attr's index here? struct cs_off_attribute *cti_attr = container_of(attr, struct cs_off_attribute, attr); max_bank = DIV_ROUND_UP(drvdata->config.nr_trig_max, 32); if (cti_attr->index >= max_bank) return 0; Thanks, Leo

4 days, 22 hours

Re: [PATCH v8 2/4] coresight: cti: encode trigger register index in register offsets

by Leo Yan

On Sun, Apr 26, 2026 at 05:44:39PM +0800, Yingchao Deng wrote: > Introduce a small encoding to carry the register index together with the > base offset in a single u32, and use a common helper to compute the final > MMIO address. This refactors register access to be based on the encoded > (reg, nr) pair, reducing duplicated arithmetic and making it easier to > support variants that bank or relocate trigger-indexed registers. > > Signed-off-by: Yingchao Deng <yingchao.deng(a)oss.qualcomm.com> > --- > drivers/hwtracing/coresight/coresight-cti-core.c | 31 +++++++++++++++-------- > drivers/hwtracing/coresight/coresight-cti-sysfs.c | 4 +-- > drivers/hwtracing/coresight/coresight-cti.h | 16 ++++++++++-- > 3 files changed, 36 insertions(+), 15 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c > index 4e7d12bd2d3e..c4cbeb64365b 100644 > --- a/drivers/hwtracing/coresight/coresight-cti-core.c > +++ b/drivers/hwtracing/coresight/coresight-cti-core.c > @@ -42,6 +42,14 @@ static DEFINE_MUTEX(ect_mutex); > #define csdev_to_cti_drvdata(csdev) \ > dev_get_drvdata(csdev->dev.parent) > > +static void __iomem *cti_reg_addr(struct cti_drvdata *drvdata, int reg) > +{ > + u32 offset = CTI_REG_CLR_NR(reg); > + u32 nr = CTI_REG_GET_NR(reg); > + > + return drvdata->base + offset + sizeof(u32) * nr; > +} Could you try below change, which is more straightforward? static void __iomem *__reg_addr(struct cti_drvdata *drvdata, int off, int index) { return drvdata->base + offset + sizeof(u32) * index; } #define reg_addr(drvdata, off) \ __reg_addr((drvdata), (off), 0) #define reg_index_addr(drvdata, off, i) \ __reg_addr((drvdata), (off), (i)) > + > /* write set of regs to hardware - call with spinlock claimed */ > void cti_write_all_hw_regs(struct cti_drvdata *drvdata) > { > @@ -55,16 +63,17 @@ void cti_write_all_hw_regs(struct cti_drvdata *drvdata) > > /* write the CTI trigger registers */ > for (i = 0; i < config->nr_trig_max; i++) { > - writel_relaxed(config->ctiinen[i], drvdata->base + CTIINEN(i)); > + writel_relaxed(config->ctiinen[i], > + cti_reg_addr(drvdata, CTI_REG_SET_NR(CTIINEN, i))); writel_relaxed(config->ctiinen[i], reg_index_addr(drvdata, CTIINEN, i)); > writel_relaxed(config->ctiouten[i], > - drvdata->base + CTIOUTEN(i)); > + cti_reg_addr(drvdata, CTI_REG_SET_NR(CTIOUTEN, i))); writel_relaxed(config->ctiouten[i], reg_index_addr(drvdata, CTIOUTEN, i)); [...] > +/* > + * Encode CTI register offset and register index in one u32: > + * - bits[0:11] : base register offset (0x000 to 0xFFF) > + * - bits[24:31] : register index (nr) > + */ > +#define CTI_REG_NR_MASK GENMASK(31, 24) > +#define CTI_REG_GET_NR(reg) FIELD_GET(CTI_REG_NR_MASK, (reg)) > +#define CTI_REG_SET_NR_CONST(reg, nr) ((reg) | FIELD_PREP_CONST(CTI_REG_NR_MASK, (nr))) > +#define CTI_REG_SET_NR(reg, nr) ((reg) | FIELD_PREP(CTI_REG_NR_MASK, (nr))) > +#define CTI_REG_CLR_NR(reg) ((reg) & (~CTI_REG_NR_MASK)) I know this might come from my suggestion, and it is also will be heavily used in patch 04. We can have strightforward way to implement this, please drop these macros. I will reply in patch 04 separately. Sorry my review might cause extra effort. Thanks, Leo

4 days, 23 hours

Re: [PATCH v8 1/4] coresight: cti: Convert trigger usage fields to dynamic bitmaps and arrays

by Leo Yan

On Sun, Apr 26, 2026 at 05:44:38PM +0800, Yingchao Deng wrote: [...] > @@ -316,23 +316,33 @@ static int cti_plat_process_filter_sigs(struct cti_drvdata *drvdata, > { > struct cti_trig_grp *tg = NULL; > int err = 0, nr_filter_sigs; > + int nr_trigs = drvdata->config.nr_trig_max; > > nr_filter_sigs = cti_plat_count_sig_elements(fwnode, > CTI_DT_FILTER_OUT_SIGS); > if (nr_filter_sigs == 0) > return 0; > > - if (nr_filter_sigs > drvdata->config.nr_trig_max) > + if (nr_filter_sigs > nr_trigs) > return -EINVAL; > > tg = kzalloc_obj(*tg); > if (!tg) > return -ENOMEM; > > + tg->used_mask = bitmap_zalloc(nr_trigs, GFP_KERNEL); Here would be: tg->used_mask = bitmap_zalloc(nr_filter_sigs, GFP_KERNEL); > + if (!tg->used_mask) { > + kfree(tg); > + return -ENOMEM; > + } > + It is likely this will have merge conflict with the new patch [1]. You might need to rebase this patch on the top of [1]. We need to give [1] priority as it is a fix. [1] https://lore.kernel.org/linux-arm-kernel/20260426-nr_sigs-v1-1-3b9df99dab97… Otherwise, LGTM: Reviewed-by: Leo Yan <leo.yan(a)arm.com>

4 days, 23 hours

Re: [PATCH] coresight: cti: Fix DT filter signals silently ignored

by Leo Yan

On Sun, Apr 26, 2026 at 05:59:34PM +0800, Yingchao Deng wrote: > In cti_plat_process_filter_sigs(), after allocating a temporary > cti_trig_grp struct via kzalloc_obj(), the code never assigns tg->nr_sigs > = nr_filter_sigs. Since kzalloc zero-initialises the struct, tg->nr_sigs > remains 0. cti_plat_read_trig_group() guards with: > if (!tgrp->nr_sigs) > return 0; > > so it returns immediately without reading any signal indices from DT. > > Fix by assigning tg->nr_sigs before calling cti_plat_read_trig_group(). > > Fixes: a5614770ab97 ("coresight: cti: Add device tree support for custom CTI") > Signed-off-by: Yingchao Deng <yingchao.deng(a)oss.qualcomm.com> > --- > drivers/hwtracing/coresight/coresight-cti-platform.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/hwtracing/coresight/coresight-cti-platform.c b/drivers/hwtracing/coresight/coresight-cti-platform.c > index 4eff96f48594..d6d5388705c3 100644 > --- a/drivers/hwtracing/coresight/coresight-cti-platform.c > +++ b/drivers/hwtracing/coresight/coresight-cti-platform.c > @@ -329,6 +329,7 @@ static int cti_plat_process_filter_sigs(struct cti_drvdata *drvdata, > if (!tg) > return -ENOMEM; > > + tg->nr_sigs = nr_filter_sigs; > err = cti_plat_read_trig_group(tg, fwnode, CTI_DT_FILTER_OUT_SIGS); I checked the naming, seems tg->nr_sigs is the right field to store the number. LGTM: Reviewed-by: Leo Yan <leo.yan(a)arm.com>

5 days, 5 hours

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

by Mike Leach

Hi, On 4/21/26 11:30, Yeoreum Yun wrote: >> Hi Mike, >> >>> Hi, >>> >>> This register [bit 31] indicates if a single shot comparator has matched. So >>> read-back provides information to the user post run to determine which if >>> any of the comparators set in this way has actually matched. >> >> Okay. so after disable sysfs session, to check former session >> check whether comprator has matched. >> >>> >>> Moreover, the specification states "Software must reset this bit to 0 to >>> re-enable single-shot control" and "Reset state is unknown. STATUS must be >>> written to set an initial state...." >>> >>> Therefore this register must be written as part of any configuration so >>> should be available in the drvdata->config for both read and write, >> >> But I don't think this is the reason for locate ss_status into "config" >> since its write purpose is not to configure but the "clear" former bit. >> That's why I think it's enough to clear when the new sysfs session starts. >> > > IOW, I think it's better to remove ss_status from configfs item > and > - add field ss_cmp in etm4_cpas > - add another field ss_status under "etm4_drvdata" to show "PENDING > and STATUS" bits to sysfs after finishing session. > > Is is valid for you? > No. Why two different locations for a single register read? If I have the ETMv4 hardware manual I am going to look for a something that is recognizable as being related to the single shot comparator status register(s). So in sysfs I would expect to see all the bits from the register, displayed, without masking off the STATUS and PENDING bits as happens now. In the code I would expect to see a single location with a sensible name - ss_cmp doesn't really correlate terribly well with TRCSSCSR. If you do not like the original ss_status, then ss_cmp_status may actually be better, ss_cmp could be either the ss comparator status or control register. Regards Mike >> -- >> Sincerely, >> Yeoreum Yun >> > > -- > Sincerely, > Yeoreum Yun

1 week, 4 days

Re: [PATCH v5 07/12] coresight: etm4x: fix inconsistencies with sysfs configuration

by Leo Yan

On Tue, Apr 21, 2026 at 12:14:20PM +0100, Yeoreum Yun wrote: [...] > > > - There is a risk of corrupting drvdata->config if a perf session enables > > > tracing while cscfg_csdev_disable_active_config() is being handled in > > > etm4_disable_sysfs(). > > > > Similiar to above, cscfg_csdev_disable_active_config() is not > > protected in etm4_disable_sysfs(). > > This is not true. > at the time of etm4_disable_sysfs() "mode" is already taken > (whether sysfs or perf). In this situation, it's enough to > call cscfg_csdev_disable_active_config() before chaning > mode to DISABLED. To be clear, I am trying to understand issue _before_ your patch. Without this patch, cscfg_csdev_disable_active_config() is not protected by the mode. > > > struct etm4_enable_arg { > > > struct etmv4_drvdata *drvdata; > > > + struct etmv4_config config; > > > > We don't need this. We can defer to get drvdata->config in SMP call. > > This is not true ane make a situation more complex. > If we get config in SMP call, that would be a problem when some user is > trying to modify config. > > IOW, while user modifying config via sysfs, and SMP call happens, > it makes a dead lock. so for this if we try to grab the lock in SMP call, > we should change every sysfs raw_spin_lock() into raw_spin_lock_irqsave(). > > Instead of this it would be much clear and simpler to get config in here > rather than to add some latencies via sysfs. Thanks for info. If so, it is fine for me to add "config". > > > @@ -1386,6 +1401,7 @@ static void etm4_init_arch_data(void *info) > > > drvdata = dev_get_drvdata(init_arg->dev); > > > caps = &drvdata->caps; > > > csa = init_arg->csa; > > > + config = &drvdata->active_config; > > > > Should we init drvdata->config instead so make it has sane values ? > > > > In other words, drvdata->active_config are always set at the runtime, > > so don't need to init it at all, right? > > No. at least when the initialise, I think we should fill the its > contesnt with the "etm4_set_default()". > > That's why the consequence call etm4_set_default() called with > active_config and config is coped with the default configutation. I'm concerned that some config fields may be reused across sessions. For example, a sysfs session copies drvdata->config into drvdata->active_config, and later a perf session may reuse stale values. The same issue can happen in the reverse direction. A clean approach would be to treat drvdata->active_config as per-session state: 1) clear drvdata->active_config at session start 2) apply the session-specific config 2.1) sysfs: use drvdata->config 2.2) perf: use event config 3) then apply configfs configuration So we should clear drvdata->active_config at the start of each session and rebuild it with the correct configuration. Thanks, Leo

1 week, 4 days

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight