[PATCH v3] coresight: tpdm: add traceid_show for checking traceid
Jie Gan
jie.gan at oss.qualcomm.com
Tue Mar 31 02:33:45 PDT 2026
On 3/31/2026 5:26 PM, James Clark wrote:
>
>
> On 31/03/2026 4:18 am, Jie Gan wrote:
>> Hi James,
>>
>> On 3/31/2026 9:29 AM, Jie Gan wrote:
>>>
>>> Hi James,
>>>
>>> On 3/30/2026 10:55 PM, James Clark wrote:
>>>>
>>>>
>>>> On 25/03/2026 3:10 am, Jie Gan wrote:
>>>>> Save the trace ID in drvdata during TPDM enablement and expose it
>>>>> to userspace to support trace data parsing.
>>>>>
>>>>> The TPDM device’s trace ID corresponds to the trace ID allocated
>>>>> to the connected TPDA device.
>>>>>
>>>>> Signed-off-by: Jie Gan <jie.gan at oss.qualcomm.com>
>>>>> ---
>>>>> Changes in v3:
>>>>> 1. Only allow user to read the traceid while the TPDM device is
>>>>> enabled.
>>>>> - Link to v2: https://lore.kernel.org/r/20260316-add-traceid-show-
>>>>> for- tpdm-v2-1-1dec2a67e4ed at oss.qualcomm.com
>>>>>
>>>>> Changes in V2:
>>>>> 1. Use sysfs_emit instead of sprintf.
>>>>> Link to V1 - https://lore.kernel.org/all/20260306-add-traceid-show-
>>>>> for-tpdm-v1-1-0658a8edb972 at oss.qualcomm.com/
>>>>> ---
>>>>> drivers/hwtracing/coresight/coresight-tpdm.c | 34 +++++++++++++++
>>>>> + + + +++++++++-
>>>>> drivers/hwtracing/coresight/coresight-tpdm.h | 2 ++
>>>>> 2 files changed, 35 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/
>>>>> drivers/ hwtracing/coresight/coresight-tpdm.c
>>>>> index da77bdaad0a4..c8339b973bfc 100644
>>>>> --- a/drivers/hwtracing/coresight/coresight-tpdm.c
>>>>> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c
>>>>> @@ -481,7 +481,7 @@ static void __tpdm_enable(struct tpdm_drvdata
>>>>> *drvdata)
>>>>> static int tpdm_enable(struct coresight_device *csdev, struct
>>>>> perf_event *event,
>>>>> enum cs_mode mode,
>>>>> - __maybe_unused struct coresight_path *path)
>>>>> + struct coresight_path *path)
>>>>> {
>>>>> struct tpdm_drvdata *drvdata = dev_get_drvdata(csdev-
>>>>> >dev.parent);
>>>>> @@ -497,6 +497,7 @@ static int tpdm_enable(struct coresight_device
>>>>> *csdev, struct perf_event *event,
>>>>> }
>>>>> __tpdm_enable(drvdata);
>>>>> + drvdata->traceid = path->trace_id;
>>>>> drvdata->enable = true;
>>>>> spin_unlock(&drvdata->spinlock);
>>>>> @@ -693,6 +694,29 @@ static struct attribute_group tpdm_attr_grp = {
>>>>> .attrs = tpdm_attrs,
>>>>> };
>>>>> +static ssize_t traceid_show(struct device *dev,
>>>>> + struct device_attribute *attr, char *buf)
>>>>> +{
>>>>> + unsigned long val;
>>>>> + struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>>>> +
>>>>> + if (coresight_get_mode(drvdata->csdev) == CS_MODE_DISABLED)
>>>>> + return -EINVAL;
>>>>> +
>>>>> + val = drvdata->traceid;
>>>>
>>>> You probably need to take the coresight_mutex here otherwise you
>>>> could still return an invalid or stale value despite checking the mode.
>>>>
>>>
>>> Acked. I have missed this potential race condition.
>>>
>>>> There might also be some value in it returning the last used trace
>>>> ID even if the mode isn't enabled anymore. Because you can still
>>>> read out of the sink after disabling, so it makes more sense for a
>>>> script to read it at that point rather than when it's enabled. Also,
>>>> you probably don't want to be doing other things in your script in
>>>> the point between enabling and disabling.
>>>
>>> That's making sense. I shouldnt add such restriction for the read
>>> process.
>>>
>>
>> I missed one point in last message.
>>
>> Is that acceptable to export the coresight_mutex from the core module?
>> Currently, the coresight_mutex is used within the module only.
>>
>> Thanks,
>> Jie
>>
>
> If the plan is to only check for non-zero trace ID and not check the
> mode any more then you don't need to lock. Maybe lets see what Suzuki
> thinks about returning the last trace ID though as it was his idea to
> add -EINVAL.
>
Sure. The trace ID is allocated during TPDA device probe and remains
unchanged for the entire lifetime of the device.
Thanks,
Jie
>>> Scenarios for reading:
>>> 1. device is enabled -> trace ID is valid
>>> 2. device is enabled then disabled -> trace ID is valid for the last
>>> trace event
>>> 3. device is never enabled -> invalid trace ID (value 0)
>>>
>>> we only need to check the validation of the trace ID.
>>>
>>> mutex_lock(&coresight_mutex);
>>> val = drvdata->traceid;
>>> mutex_unlock(&coresight_mutex);
>>>
>>> if (!val)
>>> return -EINVAL;
>>>
>>> return sysfs_emit(buf, "%#lx\n", val);
>>>
>>> Thanks,
>>> Jie
>>>
>>>>
>>>>> + return sysfs_emit(buf, "%#lx\n", val);
>>>>> +}
>>>>> +static DEVICE_ATTR_RO(traceid);
>>>>> +
>>>>> +static struct attribute *traceid_attrs[] = {
>>>>> + &dev_attr_traceid.attr,
>>>>> + NULL,
>>>>> +};
>>>>> +
>>>>> +static struct attribute_group traceid_attr_grp = {
>>>>> + .attrs = traceid_attrs,
>>>>> +};
>>>>> +
>>>>> static ssize_t dsb_mode_show(struct device *dev,
>>>>> struct device_attribute *attr,
>>>>> char *buf)
>>>>> @@ -1367,6 +1391,12 @@ static const struct attribute_group
>>>>> *tpdm_attr_grps[] = {
>>>>> &tpdm_cmb_patt_grp,
>>>>> &tpdm_cmb_msr_grp,
>>>>> &tpdm_mcmb_attr_grp,
>>>>> + &traceid_attr_grp,
>>>>> + NULL,
>>>>> +};
>>>>> +
>>>>> +static const struct attribute_group *static_tpdm_attr_grps[] = {
>>>>> + &traceid_attr_grp,
>>>>> NULL,
>>>>> };
>>>>> @@ -1425,6 +1455,8 @@ static int tpdm_probe(struct device *dev,
>>>>> struct resource *res)
>>>>> desc.access = CSDEV_ACCESS_IOMEM(base);
>>>>> if (res)
>>>>> desc.groups = tpdm_attr_grps;
>>>>> + else
>>>>> + desc.groups = static_tpdm_attr_grps;
>>>>> drvdata->csdev = coresight_register(&desc);
>>>>> if (IS_ERR(drvdata->csdev))
>>>>> return PTR_ERR(drvdata->csdev);
>>>>> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h b/
>>>>> drivers/ hwtracing/coresight/coresight-tpdm.h
>>>>> index 2867f3ab8186..11da64e1ade8 100644
>>>>> --- a/drivers/hwtracing/coresight/coresight-tpdm.h
>>>>> +++ b/drivers/hwtracing/coresight/coresight-tpdm.h
>>>>> @@ -300,6 +300,7 @@ struct cmb_dataset {
>>>>> * @cmb Specifics associated to TPDM CMB.
>>>>> * @dsb_msr_num Number of MSR supported by DSB TPDM
>>>>> * @cmb_msr_num Number of MSR supported by CMB TPDM
>>>>> + * @traceid Trace ID of the path.
>>>>> */
>>>>> struct tpdm_drvdata {
>>>>> @@ -313,6 +314,7 @@ struct tpdm_drvdata {
>>>>> struct cmb_dataset *cmb;
>>>>> u32 dsb_msr_num;
>>>>> u32 cmb_msr_num;
>>>>> + u8 traceid;
>>>>> };
>>>>> /* Enumerate members of various datasets */
>>>>>
>>>>> ---
>>>>> base-commit: b84a0ebe421ca56995ff78b66307667b62b3a900
>>>>> change-id: 20260316-add-traceid-show-for-tpdm-88d040651f00
>>>>>
>>>>> Best regards,
>>>>
>>>
>>
>
More information about the linux-arm-kernel
mailing list