Re: [PATCH] coresight-tpdm: add trace_id sysfs node

[Leo Yan <leo.yan@arm.com>]

On Fri, May 30, 2025 at 03:29:14PM +0800, Yuanfang Zhang wrote:
> The trace ID of TPMD is the trace ID of the TPDA or TNOC
> which it is connected to, this change adds trace_id sysfs
> node to expose this trace id to userspace.
> 
> Signed-off-by: Yuanfang Zhang <quic_yuanfang@quicinc.com>
> ---
>  drivers/hwtracing/coresight/coresight-tpdm.c | 16 ++++++++++++++++
>  drivers/hwtracing/coresight/coresight-tpdm.h |  2 ++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c
> index 7214e65097ec9ac69f6c7c9278bcd28d25945c9e..8a5d115157924f39b09f8e3005827d7d64aa376c 100644
> --- a/drivers/hwtracing/coresight/coresight-tpdm.c
> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c
> @@ -497,6 +497,9 @@ static int tpdm_enable(struct coresight_device *csdev, struct perf_event *event,
>  
>  	__tpdm_enable(drvdata);
>  	drvdata->enable = true;
> +
> +	if (path)
> +		drvdata->traceid = path->trace_id;

In Sysfs mode, the core layer calls coresight_path_assign_trace_id().
Eventually, the source driver's trace_id() callback is invoked to
retrieve the trace ID.

I don't see TPDM driver provides trace_id() callback, so here
"path->trace_id" is an invalid value?

Please refer to STM driver (see stm_trace_id()) for this part.

>  	spin_unlock(&drvdata->spinlock);
>  
>  	dev_dbg(drvdata->dev, "TPDM tracing enabled\n");
> @@ -554,6 +557,7 @@ static void tpdm_disable(struct coresight_device *csdev,
>  	__tpdm_disable(drvdata);
>  	coresight_set_mode(csdev, CS_MODE_DISABLED);
>  	drvdata->enable = false;
> +	drvdata->traceid = 0;

Seems to me, a source device can reserve a trace ID until the module
is unloaded.  So it is not necessary to clean up trace ID when
disabling it.

BTW, my understanding is that you are trying to allocate a trace ID in
the TPDM driver and propagate the ID to the TNOC driver.  It would be
helpful if you could send the patches in one go, we can review it in
global picture.

Leo.

>  	spin_unlock(&drvdata->spinlock);
>  
>  	dev_dbg(drvdata->dev, "TPDM tracing disabled\n");
> @@ -655,9 +659,21 @@ static ssize_t integration_test_store(struct device *dev,
>  }
>  static DEVICE_ATTR_WO(integration_test);
>  
> +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);
> +
> +	val = drvdata->traceid;
> +	return sprintf(buf, "%#lx\n", val);
> +}
> +static DEVICE_ATTR_RO(traceid);
> +
>  static struct attribute *tpdm_attrs[] = {
>  	&dev_attr_reset_dataset.attr,
>  	&dev_attr_integration_test.attr,
> +	&dev_attr_traceid.attr,
>  	NULL,
>  };
>  
> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h b/drivers/hwtracing/coresight/coresight-tpdm.h
> index b117543897344b689f666f6890cabb59c8ee4869..e12a64f265daa86f1b82fa3640e271e8386f99af 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:    Value of the current ID for this component.
>   */
>  
>  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: 94305e83eccb3120c921cd3a015cd74731140bac
> change-id: 20250523-showtraceid-2df91c89be8f
> 
> Best regards,
> -- 
> Yuanfang Zhang <quic_yuanfang@quicinc.com>
> 
> _______________________________________________
> CoreSight mailing list -- coresight@lists.linaro.org
> To unsubscribe send an email to coresight-leave@lists.linaro.org