CoreSight

coresight@lists.linaro.org

4 participants
2607 discussions

Re: [PATCH] coresight: Fix possible deadlock in coresight_panic_cb

by Suzuki K Poulose

Hi Sean On 28/08/2025 19:17, Sean Anderson wrote: > coresight_panic_cb is called with interrupts disabled during panics. > However, bus_for_each_dev calls bus_to_subsys which takes > bus_kset->list_lock without disabling IRQs. This will cause a deadlock > if a panic occurs while one of the other coresight functions that uses > bus_for_each_dev is running. > > Maintain a separate list of coresight devices to access during a panic. Thanks for the patch. I have a minor comment. > > Fixes: 46006ceb5d02 ("coresight: core: Add provision for panic callbacks") > Signed-off-by: Sean Anderson <sean.anderson(a)linux.dev> > --- > > drivers/hwtracing/coresight/coresight-core.c | 40 ++++++++++---------- > include/linux/coresight.h | 1 + > 2 files changed, 22 insertions(+), 19 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c > index fa758cc21827..1f1bf0e2bf92 100644 > --- a/drivers/hwtracing/coresight/coresight-core.c > +++ b/drivers/hwtracing/coresight/coresight-core.c > @@ -1315,6 +1315,9 @@ void coresight_release_platform_data(struct coresight_device *csdev, > coresight_remove_conns_sysfs_group(csdev); > } > > +static DEFINE_SPINLOCK(csdev_lock); > +static LIST_HEAD(csdev_list); May be add a comment here to mention why we maintain this list ? > + > struct coresight_device *coresight_register(struct coresight_desc *desc) > { > int ret; > @@ -1374,11 +1377,16 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) > goto out_unlock; > } > > + scoped_guard(spinlock_irq, &csdev_lock) > + list_add(&csdev->csdev_list, &csdev_list); > + > if (csdev->type == CORESIGHT_DEV_TYPE_SINK || > csdev->type == CORESIGHT_DEV_TYPE_LINKSINK) { > ret = etm_perf_add_symlink_sink(csdev); > > if (ret) { > + scoped_guard(spinlock_irq, &csdev_lock) > + list_del(&csdev->csdev_list); Could this be moved to coresight_device_release(), which will be called when the device gets unregistered ? That way, you don't need it here and in coresight_unregister() too. Rest looks good to me Suzuki > device_unregister(&csdev->dev); > /* > * As with the above, all resources are free'd > @@ -1427,6 +1435,8 @@ void coresight_unregister(struct coresight_device *csdev) > coresight_remove_conns(csdev); > coresight_clear_default_sink(csdev); > coresight_release_platform_data(csdev, csdev->dev.parent, csdev->pdata); > + scoped_guard(spinlock_irq, &csdev_lock) > + list_del(&csdev->csdev_list); > device_unregister(&csdev->dev); > } > EXPORT_SYMBOL_GPL(coresight_unregister); > @@ -1563,28 +1573,20 @@ const struct bus_type coresight_bustype = { > .name = "coresight", > }; > > -static int coresight_panic_sync(struct device *dev, void *data) > -{ > - int mode; > - struct coresight_device *csdev; > - > - /* Run through panic sync handlers for all enabled devices */ > - csdev = container_of(dev, struct coresight_device, dev); > - mode = coresight_get_mode(csdev); > - > - if ((mode == CS_MODE_SYSFS) || (mode == CS_MODE_PERF)) { > - if (panic_ops(csdev)) > - panic_ops(csdev)->sync(csdev); > - } > - > - return 0; > -} > - > static int coresight_panic_cb(struct notifier_block *self, > unsigned long v, void *p) > { > - bus_for_each_dev(&coresight_bustype, NULL, NULL, > - coresight_panic_sync); > + struct coresight_device *csdev; > + > + guard(spinlock)(&csdev_lock); > + list_for_each_entry(csdev, &csdev_list, csdev_list) { > + /* Run through panic sync handlers for all enabled devices */ > + int mode = coresight_get_mode(csdev); > + > + if ((mode == CS_MODE_SYSFS || mode == CS_MODE_PERF) && > + panic_ops(csdev)) > + panic_ops(csdev)->sync(csdev); > + } > > return 0; > } > diff --git a/include/linux/coresight.h b/include/linux/coresight.h > index 4ac65c68bbf4..a5e62ebd03b5 100644 > --- a/include/linux/coresight.h > +++ b/include/linux/coresight.h > @@ -302,6 +302,7 @@ struct coresight_device { > /* system configuration and feature lists */ > struct list_head feature_csdev_list; > struct list_head config_csdev_list; > + struct list_head csdev_list; > raw_spinlock_t cscfg_csdev_lock; > void *active_cscfg_ctxt; > };

4 months, 1 week

Re: [PATCH v2] coresight: tpda: fix the logic to setup the element size

by Suzuki K Poulose

On Sat, 06 Sep 2025 07:53:04 +0800, Jie Gan wrote: > Some TPDM devices support both CMB and DSB datasets, requiring > the system to enable the port with both corresponding element sizes. > > Currently, the logic treats tpdm_read_element_size as successful if > the CMB element size is retrieved correctly, regardless of whether > the DSB element size is obtained. This behavior causes issues > when parsing data from TPDM devices that depend on both element sizes. > > [...] Applied, thanks! [1/1] coresight: tpda: fix the logic to setup the element size https://git.kernel.org/coresight/c/fd6974c19eb3 Best regards, -- Suzuki K Poulose <suzuki.poulose(a)arm.com>

4 months, 1 week

Re: [PATCH -next] coresight: Fix incorrect handling for return value of devm_kzalloc

by James Clark

On 08/09/2025 1:20 pm, Lin Yujun wrote: > The return value of devm_kzalloc could be an null pointer, > use "!desc.pdata" to fix incorrect handling return value > of devm_kzalloc. > > Fixes: 4277f035d227 ("coresight: trbe: Add a representative coresight_platform_data for TRBE") > Signed-off-by: Lin Yujun <linyujun809(a)h-partners.com> > --- > drivers/hwtracing/coresight/coresight-trbe.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c > index 8267dd1a2130..caf873adfc3a 100644 > --- a/drivers/hwtracing/coresight/coresight-trbe.c > +++ b/drivers/hwtracing/coresight/coresight-trbe.c > @@ -1277,11 +1277,11 @@ static void arm_trbe_register_coresight_cpu(struct trbe_drvdata *drvdata, int cp > * ends up failing. Instead let's allocate a dummy zeroed > * coresight_platform_data structure and assign that back > * into the device for that purpose. > */ > desc.pdata = devm_kzalloc(dev, sizeof(*desc.pdata), GFP_KERNEL); > - if (IS_ERR(desc.pdata)) > + if (!desc.pdata) > goto cpu_clear; > > desc.type = CORESIGHT_DEV_TYPE_SINK; > desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_PERCPU_SYSMEM; > desc.ops = &arm_trbe_cs_ops; Reviewed-by: James Clark <james.clark(a)linaro.org>

4 months, 1 week

[PATCH v4 0/6] Refactoring finding CPU phandles in DT

by Alireza Sanaee

This series refactor the way CPU IDs are retrieved from the device tree. Usually, there is a for loop that goes over every single CPU that can be avoided. This also reduces the amount of NULL pointer checks in drivers. I have abstracted away that loop and introduced a new function (of_cpu_node_to_id) for this. This patchset is a subset of [1], where I removed content and patches relevant to hyper-threaded cores for DT. Based on the discussion, the code refactor is still useful, hence this patchset. [1] https://lore.kernel.org/all/20250512080715.82-1-alireza.sanaee@huawei.co Changes since v3: - Rebased to d69eb204c255c35abd9e8cb621484e8074c75eaa Changes since v2: - Addressed Jonathan Cameron's comments. - Added reviewed-by tags. - Added a new commit where I suggest simplfing the code in arch_topology.c to remove the use of cpu_node on failure path. - Improve documentation. - Caught a bug in patch 1. - Commit message fixed for patch 2. Changes since v1: - Rebased on top of the latest mainline. - Addressed Krzysztof Kozlowski's comments -- Hopefully :-) - Addressed Jonathan Cameron's comments. Alireza Sanaee (6): of: add infra for finding CPU id from phandle arch_topology: drop the use of cpu_node in the pr_info arch_topology: update CPU map to use of_cpu_phandle_to_id coresight: cti: Use of_cpu_phandle_to_id for grabbing CPU id coresight: Use of_cpu_phandle_to_id for grabbing CPU ID perf/arm-dsu: refactor cpu id retrieval via new API of_cpu_phandle_to_id drivers/base/arch_topology.c | 22 ++++----- .../coresight/coresight-cti-platform.c | 13 +---- .../hwtracing/coresight/coresight-platform.c | 15 +----- drivers/of/cpu.c | 48 +++++++++++++++++++ drivers/perf/arm_dsu_pmu.c | 7 +-- include/linux/of.h | 9 ++++ 6 files changed, 71 insertions(+), 43 deletions(-) -- 2.43.0

4 months, 1 week

[PATCH v2] coresight: trbe: Return NULL pointer for allocation failures

by Leo Yan

When the TRBE driver fails to allocate a buffer, it currently returns the error code "-ENOMEM". However, the caller etm_setup_aux() only checks for a NULL pointer, so it misses the error. As a result, the driver continues and eventually causes a kernel panic. Fix this by returning a NULL pointer from arm_trbe_alloc_buffer() on allocation failures. This allows that the callers can properly handle the failure. Fixes: 3fbf7f011f24 ("coresight: sink: Add TRBE driver") Reported-by: Tamas Zsoldos <tamas.zsoldos(a)arm.com> Signed-off-by: Leo Yan <leo.yan(a)arm.com> --- Changes in v2: - Fix TRBE driver instead of changing coresight-etm-perf.c. - Link to v1: https://lore.kernel.org/r/20250904-cs_etm_auxsetup_fix_error_handling-v1-1-… --- drivers/hwtracing/coresight/coresight-trbe.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index 10f3fb401edf6a00b24b38cdaa7c2865e7a191ac..8f9bbef71f236b327d35a288689df9b0dd8ff3f4 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -748,12 +748,12 @@ static void *arm_trbe_alloc_buffer(struct coresight_device *csdev, buf = kzalloc_node(sizeof(*buf), GFP_KERNEL, trbe_alloc_node(event)); if (!buf) - return ERR_PTR(-ENOMEM); + return NULL; pglist = kcalloc(nr_pages, sizeof(*pglist), GFP_KERNEL); if (!pglist) { kfree(buf); - return ERR_PTR(-ENOMEM); + return NULL; } for (i = 0; i < nr_pages; i++) @@ -763,7 +763,7 @@ static void *arm_trbe_alloc_buffer(struct coresight_device *csdev, if (!buf->trbe_base) { kfree(pglist); kfree(buf); - return ERR_PTR(-ENOMEM); + return NULL; } buf->trbe_limit = buf->trbe_base + nr_pages * PAGE_SIZE; buf->trbe_write = buf->trbe_base; --- base-commit: fa71e9cb4cfa59abb196229667ec84929bdc18fe change-id: 20250904-cs_etm_auxsetup_fix_error_handling-cb7e07ed9adf Best regards, -- Leo Yan <leo.yan(a)arm.com>

4 months, 2 weeks

[PATCH v6 00/10] coresight: Fix and improve clock usage

by Leo Yan

This series fixes and improves clock usage in the Arm CoreSight drivers. Based on the DT binding documents, the trace clock (atclk) is defined in some CoreSight modules, but support is absent. In most cases, the issue is hidden because the atclk clock is shared by multiple CoreSight modules and the clock is enabled anyway by other drivers. The first three patches address this issue. The programming clock (pclk) management in CoreSight drivers does not use the devm_XXX() variant APIs, the drivers needs to manually disable and release clocks for errors and for normal module exit. However, the drivers miss to disable clocks during module exit. The atclk may also not be disabled in CoreSight drivers during module exit. By using devm APIs, patches 04 and 05 fix clock disabling issues. Another issue is pclk might be enabled twice in init phase - once by AMBA bus driver, and again by CoreSight drivers. This is fixed in patch 06. Patches 07 to 10 refactor the clock related code. Patch 07 consolidates the clock initialization into a central place. Patch 08 polishes driver data allocation. Patch 09 makes the clock enabling sequence consistent. Patch 09 removes redundant condition checks and adds error handling in runtime PM. This series has been verified on Arm64 Juno platform, for both DT and ACPI modes. Also tested on Hikey960 for emulating static funnel and replicator without clock nodes. Mark kindly tested on imx8mp board. --- Changes in v6: - Changed to devm_clk_get_optional_enabled() for pclk, for fixing the clock initialization failure reported by Mark. - Added James test tags as no change for ACPI. - Link to v5: https://lore.kernel.org/r/20250724-arm_cs_fix_clock_v4-v5-0-63f648dae021@ar… Changes in v5: - Skip clock management for ACPI devices (Suzuki). - Link to v4: https://lore.kernel.org/r/20250627-arm_cs_fix_clock_v4-v4-0-0ce0009c38f8@ar… Changes in v4: - Separated patch 07 into two patches, one is for clock consolidation and another is for polishing driver data allocation (Anshuman). Changes in v3: - Updated subjects for patches 04 and 05 (Anshuman). - Refined condition checking "if (dev_is_amba(dev))" in patch 07 (Anshuman). --- Leo Yan (10): coresight: tmc: Support atclk coresight: catu: Support atclk coresight: etm4x: Support atclk coresight: Appropriately disable programming clocks coresight: Appropriately disable trace bus clocks coresight: Avoid enable programming clock duplicately coresight: Consolidate clock enabling coresight: Refactor driver data allocation coresight: Make clock sequence consistent coresight: Refactor runtime PM drivers/hwtracing/coresight/coresight-catu.c | 53 ++++++++--------- drivers/hwtracing/coresight/coresight-catu.h | 1 + drivers/hwtracing/coresight/coresight-core.c | 48 ++++++++++++++++ drivers/hwtracing/coresight/coresight-cpu-debug.c | 41 +++++--------- drivers/hwtracing/coresight/coresight-ctcu-core.c | 24 +++----- drivers/hwtracing/coresight/coresight-etb10.c | 18 ++---- drivers/hwtracing/coresight/coresight-etm3x-core.c | 17 ++---- drivers/hwtracing/coresight/coresight-etm4x-core.c | 32 ++++++----- drivers/hwtracing/coresight/coresight-etm4x.h | 4 +- drivers/hwtracing/coresight/coresight-funnel.c | 66 ++++++++-------------- drivers/hwtracing/coresight/coresight-replicator.c | 63 ++++++++------------- drivers/hwtracing/coresight/coresight-stm.c | 34 +++++------ drivers/hwtracing/coresight/coresight-tmc-core.c | 48 ++++++++-------- drivers/hwtracing/coresight/coresight-tmc.h | 2 + drivers/hwtracing/coresight/coresight-tpiu.c | 36 +++++------- include/linux/coresight.h | 31 +--------- 16 files changed, 228 insertions(+), 290 deletions(-) --- base-commit: a80198ba650f50d266d7fc4a6c5262df9970f9f2 change-id: 20250627-arm_cs_fix_clock_v4-e24b1e1f8920 Best regards, -- Leo Yan <leo.yan(a)arm.com>

4 months, 2 weeks

[PATCH v2 00/28] CoreSight: Address CPU Power Management Issues

by Leo Yan

This series addresses CPU power management issues in the CoreSight drivers. For easier review, the patches are organized into two categories: o Patches 01 ~ 10 focus on CPU power management within the ETM drivers. These patches fix SMP-safe access to the mode, correct context synchronization, and refactor the CPU suspend/resume flows. o Patches 11 ~ 28 extend CPU power management to cover activated paths, including helpers, links, and sinks. These changes move CPU PM and hotplug notifiers from the ETMv4 driver into the CoreSight core layer. Summary of Changes: - Patches 01 ~ 03: Fix device mode access in the SMP mode. - Patch 04 : A minor fix for polling bit. - Patches 05 ~ 07: Improve the context synchronization based on the ETM specification (IHI0064H.b) and Arm ARM (ARM DDI 0487 L.a). - Patches 08 ~ 10: Refactor the context save/restore flow in the ETMv4 driver, in the end, the CPU PM callbacks reuse the normal enabling and disabling flows. - Patches 11 ~ 17: Move CPU PM code from ETMv4 driver to the core layer. - Patches 18 ~ 25: Enhance device mode handling in the CTI driver, distinguishes debug mode from trace modes (Perf or SysFS mode). Extend support activated paths during CPU idle. Support save and restore context for per-CPU sink (TRBE). - Patches 26 ~ 28: Move CPU hotplug notifier from the ETMv4 driver to the core layer. The full path is now managed in CPU hotplug. Verification: This series has been verified on the following platforms: - Hikey960, Juno-r0 and Juno-r2: Note that the firmware on Hikey960 and Juno-r0 is unstable. While no CoreSight failures were observed, but the CPU hotplug or CPU idle itself may fail intermittently. Juno-r2 is much stable and can pass 24+ hours stress test. - FVP RevC: Verified TRBE changes. Stress test script: #!/usr/bin/bash echo 1 > /sys/bus/coresight/devices/tmc_etr0/enable_sink while true; do echo 0 > /sys/devices/system/cpu/cpu2/online; echo 1 > /sys/devices/system/cpu/cpu2/online; done & while true; do echo 1 > /sys/bus/coresight/devices/etm2/enable_source; echo 0 > /sys/bus/coresight/devices/etm2/enable_source; done & --- Changes in v2: - Refactored ETMv4 suspend and resume for reusing the normal enabling and disabling flows (James). - Used a per-CPU structure to maintain path pointers (James). - Supported helpers in CPU PM flows (James). - Fixed the SMP-safe access to device mode. - Fixed the context synchronization in ETMv4x driver. - Link to v1: https://lore.kernel.org/linux-arm-kernel/20250516160742.1200904-1-leo.yan@a… Signed-off-by: Leo Yan <leo.yan(a)arm.com> --- Leo Yan (27): coresight: Change device mode to atomic type coresight: etm4x: Always set tracer's device mode on target CPU coresight: etm3x: Always set tracer's device mode on target CPU coresight: etm4x: Correct polling IDLE bit coresight: etm4x: Ensure context synchronization is not ignored coresight: etm4x: Add context synchronization before enabling trace coresight: etm4x: Properly control filter in CPU idle with FEAT_TRF coresight: etm4x: Remove the state_needs_restore flag coresight: etm4x: Add flag to control single-shot restart coresight: etm4x: Reuse normal enable and disable logic in CPU idle coresight: Populate CPU ID into the coresight_device structure coresight: sysfs: Validate CPU online status for per-CPU sources coresight: Set per CPU source pointer coresight: Register CPU PM notifier in core layer coresight: etm4x: Hook CPU PM callbacks coresight: Add callback to determine if context save/restore is needed coresight: etm4x: Remove redundant condition checks in save and restore coresight: cti: Fix race condition by using device mode coresight: cti: Introduce CS_MODE_DEBUG mode coresight: cti: Properly handle modes in CPU PM notifiers coresight: Add per-CPU path pointer coresight: Add 'in_idle' argument to path enable/disable functions coresight: Control path during CPU idle coresight: Add PM callbacks for percpu sink coresight: Take hotplug lock in enable_source_store() for Sysfs mode coresight: Move CPU hotplug callbacks to core layer coresight: Manage activated path during CPU hotplug Yabin Cui (1): coresight: trbe: Save and restore state across CPU low power state drivers/hwtracing/coresight/coresight-catu.c | 1 + drivers/hwtracing/coresight/coresight-core.c | 337 ++++++++++++-- drivers/hwtracing/coresight/coresight-ctcu-core.c | 1 + drivers/hwtracing/coresight/coresight-cti-core.c | 40 +- drivers/hwtracing/coresight/coresight-cti-sysfs.c | 2 +- drivers/hwtracing/coresight/coresight-dummy.c | 1 + drivers/hwtracing/coresight/coresight-etb10.c | 1 + drivers/hwtracing/coresight/coresight-etm3x-core.c | 61 ++- drivers/hwtracing/coresight/coresight-etm4x-core.c | 488 ++++++--------------- drivers/hwtracing/coresight/coresight-etm4x.h | 62 --- drivers/hwtracing/coresight/coresight-funnel.c | 1 + drivers/hwtracing/coresight/coresight-replicator.c | 1 + drivers/hwtracing/coresight/coresight-stm.c | 1 + drivers/hwtracing/coresight/coresight-sysfs.c | 10 + drivers/hwtracing/coresight/coresight-tmc-core.c | 1 + drivers/hwtracing/coresight/coresight-tpda.c | 1 + drivers/hwtracing/coresight/coresight-tpdm.c | 1 + drivers/hwtracing/coresight/coresight-tpiu.c | 1 + drivers/hwtracing/coresight/coresight-trbe.c | 85 ++++ drivers/hwtracing/coresight/ultrasoc-smb.c | 1 + include/linux/coresight.h | 55 ++- 21 files changed, 665 insertions(+), 487 deletions(-) --- base-commit: 66701750d5565c574af42bef0b789ce0203e3071 change-id: 20250611-arm_cs_pm_fix_v3-f4ae29bb7d81 Best regards, -- Leo Yan <leo.yan(a)arm.com>

4 months, 2 weeks

[PATCH] coresight: perf: Fix pointer check with IS_ERR_OR_NULL()

by Leo Yan

The returned pointer from .alloc_buffer() callback can be an error, if only checking NULL pointer the driver cannot capture errors. The driver will proceed even after failure and cause kernel panic. Change to use IS_ERR_OR_NULL() check for capture error cases. Fixes: 0bcbf2e30ff2 ("coresight: etm-perf: new PMU driver for ETM tracers") Reported-by: Tamas Zsoldos <tamas.zsoldos(a)arm.com> Signed-off-by: Leo Yan <leo.yan(a)arm.com> --- drivers/hwtracing/coresight/coresight-etm-perf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index f677c08233ba1a28b277674662c6e6db904873dd..440d967f5d0962df187a81b0dd69a7d82a8b62ba 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -198,7 +198,7 @@ static void free_sink_buffer(struct etm_event_data *event_data) cpumask_t *mask = &event_data->mask; struct coresight_device *sink; - if (!event_data->snk_config) + if (IS_ERR_OR_NULL(event_data->snk_config)) return; if (WARN_ON(cpumask_empty(mask))) @@ -450,7 +450,7 @@ static void *etm_setup_aux(struct perf_event *event, void **pages, event_data->snk_config = sink_ops(sink)->alloc_buffer(sink, event, pages, nr_pages, overwrite); - if (!event_data->snk_config) + if (IS_ERR_OR_NULL(event_data->snk_config)) goto err; out: --- base-commit: fa71e9cb4cfa59abb196229667ec84929bdc18fe change-id: 20250904-cs_etm_auxsetup_fix_error_handling-cb7e07ed9adf Best regards, -- Leo Yan <leo.yan(a)arm.com>

4 months, 2 weeks

Re: [PATCH v1] coresight: tpda: fix the logic to setup the element size

by Suzuki K Poulose

On 04/09/2025 02:12, Jie Gan wrote: > > > On 9/3/2025 5:45 PM, Jie Gan wrote: >> >> >> On 9/3/2025 4:57 PM, Suzuki K Poulose wrote: >>> On 06/08/2025 09:09, Jie Gan wrote: >>>> Some TPDM devices support both CMB and DSB datasets, requiring >>>> the system to enable the port with both corresponding element sizes. >>>> >>>> Currently, the logic treats tpdm_read_element_size as successful if >>>> the CMB element size is retrieved correctly, regardless of whether >>>> the DSB element size is obtained. This behavior causes issues >>>> when parsing data from TPDM devices that depend on both element sizes. >>>> >>>> To address this, the function should explicitly fail if the DSB >>>> element size cannot be read correctly. >>> >>> But what is the device only has CMB ? Back when this was originally >> >> We have CMB TPDM, DSB TPDM and CMB&&DSB TPDM. >> >>> merged, we raised this question and the answer was, "Only one is >>> supported, not both." But this sounds like that is wrong. >> >> I think we may not answer the previous question clearly. But it >> definitely has issue here. >> >>> Could we defer the "Warning" to the caller. i.e., Let the caller >>> figure out the if the DSB size is found and predicate that on the >>> DSB support on the TPDM. >> >> Understood, below codes will be added in the caller to check the error: >> if ((tpdm_data->dsb && !drvdata->dsb_esize) || >> (tpdm_data->cmb && !drvdata->cmb_esize)) >> goto err; >> >> Thanks, >> Jie >> > > Hi Suzuki, > > I've reviewed the logic here. It's not feasible for the caller to > perform the check, since we first retrieve TPDM's drvdata, which adds > complexity to the code. I believe it's better to handle this within the > function itself. > > We are expecting the element_size for cmb if the condition is true, as > well as dsb: > if (tpdm_data->dsb) > ... > should obtain a valid element size for dsb. > ... > > if (tpdm_data->cmb) > ... > should obtain a valid element size for cmb. > ... > Ok, fair enough. Please resend the patch without the dependency on the static TPDM patch. Given this is a fix, this could go in without waiting for the new series. Suzuki > Thanks, > Jie > >>> >>> Suzuki >>> >>>> >>>> Fixes: e6d7f5252f73 ("coresight-tpda: Add support to configure CMB >>>> element") >>>> Signed-off-by: Jie Gan <jie.gan(a)oss.qualcomm.com> >>>> --- >>>> drivers/hwtracing/coresight/coresight-tpda.c | 3 +++ >>>> 1 file changed, 3 insertions(+) >>>> >>>> diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/ >>>> hwtracing/coresight/coresight-tpda.c >>>> index 0633f04beb24..333b3cb23685 100644 >>>> --- a/drivers/hwtracing/coresight/coresight-tpda.c >>>> +++ b/drivers/hwtracing/coresight/coresight-tpda.c >>>> @@ -71,6 +71,8 @@ static int tpdm_read_element_size(struct >>>> tpda_drvdata *drvdata, >>>> if (tpdm_data->dsb) { >>>> rc = fwnode_property_read_u32(dev_fwnode(csdev->dev.parent), >>>> "qcom,dsb-element-bits", &drvdata->dsb_esize); >>>> + if (rc) >>>> + goto out; >>>> } >>>> if (tpdm_data->cmb) { >>>> @@ -78,6 +80,7 @@ static int tpdm_read_element_size(struct >>>> tpda_drvdata *drvdata, >>>> "qcom,cmb-element-bits", &drvdata->cmb_esize); >>>> } >>>> +out: >>>> if (rc) >>>> dev_warn_once(&csdev->dev, >>>> "Failed to read TPDM Element size: %d\n", rc); >>> >>> >> >

4 months, 2 weeks

Re: [PATCH v1] coresight: tpda: fix the logic to setup the element size

by Suzuki K Poulose

On 06/08/2025 09:09, Jie Gan wrote: > Some TPDM devices support both CMB and DSB datasets, requiring > the system to enable the port with both corresponding element sizes. > > Currently, the logic treats tpdm_read_element_size as successful if > the CMB element size is retrieved correctly, regardless of whether > the DSB element size is obtained. This behavior causes issues > when parsing data from TPDM devices that depend on both element sizes. > > To address this, the function should explicitly fail if the DSB > element size cannot be read correctly. But what is the device only has CMB ? Back when this was originally merged, we raised this question and the answer was, "Only one is supported, not both." But this sounds like that is wrong. Could we defer the "Warning" to the caller. i.e., Let the caller figure out the if the DSB size is found and predicate that on the DSB support on the TPDM. Suzuki > > Fixes: e6d7f5252f73 ("coresight-tpda: Add support to configure CMB element") > Signed-off-by: Jie Gan <jie.gan(a)oss.qualcomm.com> > --- > drivers/hwtracing/coresight/coresight-tpda.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c > index 0633f04beb24..333b3cb23685 100644 > --- a/drivers/hwtracing/coresight/coresight-tpda.c > +++ b/drivers/hwtracing/coresight/coresight-tpda.c > @@ -71,6 +71,8 @@ static int tpdm_read_element_size(struct tpda_drvdata *drvdata, > if (tpdm_data->dsb) { > rc = fwnode_property_read_u32(dev_fwnode(csdev->dev.parent), > "qcom,dsb-element-bits", &drvdata->dsb_esize); > + if (rc) > + goto out; > } > > if (tpdm_data->cmb) { > @@ -78,6 +80,7 @@ static int tpdm_read_element_size(struct tpda_drvdata *drvdata, > "qcom,cmb-element-bits", &drvdata->cmb_esize); > } > > +out: > if (rc) > dev_warn_once(&csdev->dev, > "Failed to read TPDM Element size: %d\n", rc);

4 months, 2 weeks

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight