RE: [PATCH v13 7/8] coresight: tmc: integrate byte-cntr's sysfs_ops with tmc sysfs file_ops

[Mike Leach]

Hi, 
> -----Original Message-----
> From: Jie Gan <jie.gan@xxxxxxxxxxxxxxxx>
> Sent: Monday, February 23, 2026 6:56 AM
> To: Suzuki Poulose <Suzuki.Poulose@xxxxxxx>; Mike Leach
> <Mike.Leach@xxxxxxx>; James Clark <james.clark@xxxxxxxxxx>; Alexander
> Shishkin <alexander.shishkin@xxxxxxxxxxxxxxx>; Rob Herring
> <robh@xxxxxxxxxx>; Krzysztof Kozlowski <krzk+dt@xxxxxxxxxx>; Conor Dooley
> <conor+dt@xxxxxxxxxx>; Tingwei Zhang
> <tingwei.zhang@xxxxxxxxxxxxxxxx>; Mao Jinlong
> <jinlong.mao@xxxxxxxxxxxxxxxx>; Bjorn Andersson
> <andersson@xxxxxxxxxx>; Konrad Dybcio <konradybcio@xxxxxxxxxx>
> Cc: coresight@xxxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; linux-arm-msm@xxxxxxxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; Jie Gan <jie.gan@xxxxxxxxxxxxxxxx>
> Subject: [PATCH v13 7/8] coresight: tmc: integrate byte-cntr's sysfs_ops with
> tmc sysfs file_ops
> 
> Add code logic to invoke byte-cntr's tmc_sysfs_ops if the byte-cntr
> is enabled.
> 
> Signed-off-by: Jie Gan <jie.gan@xxxxxxxxxxxxxxxx>
> ---
>  drivers/hwtracing/coresight/coresight-tmc-core.c | 53
> +++++++++++++++++++++++-
>  1 file changed, 52 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c
> b/drivers/hwtracing/coresight/coresight-tmc-core.c
> index 32ca2ec994de..6486bdafdddc 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-core.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c
> @@ -31,6 +31,7 @@
> 
>  #include "coresight-priv.h"
>  #include "coresight-tmc.h"
> +#include "coresight-ctcu.h"
> 
>  DEFINE_CORESIGHT_DEVLIST(etb_devs, "tmc_etb");
>  DEFINE_CORESIGHT_DEVLIST(etf_devs, "tmc_etf");
> @@ -228,15 +229,47 @@ static int tmc_prepare_crashdata(struct
> tmc_drvdata *drvdata)
>  	return 0;
>  }
> 
> +/* Return the byte-cntr's tmc_sysfs_ops if in using */
> +static const struct tmc_sysfs_ops *tmc_get_byte_cntr_sysfs_ops(struct
> tmc_drvdata *drvdata)
> +{
> +	struct ctcu_byte_cntr *byte_cntr_data;
> +	struct ctcu_drvdata *ctcu_drvdata;
> +	struct coresight_device *ctcu;
> +	int port;
> +
> +	ctcu = tmc_etr_get_ctcu_device(drvdata);
> +	if (!ctcu)
> +		return NULL;
> +
> +	port = coresight_get_in_port(drvdata->csdev, ctcu);
> +	if (port < 0)
> +		return NULL;
> +
> +	ctcu_drvdata = dev_get_drvdata(ctcu->dev.parent);
> +	byte_cntr_data = &ctcu_drvdata->byte_cntr_data[port];
> +	if (byte_cntr_data && byte_cntr_data->thresh_val)
> +		return ctcu_drvdata->byte_cntr_sysfs_ops;
> +
> +	return NULL;
> +}
> +

Should be in a CTCU source file, not part of the common tmc code

>  static int tmc_read_prepare(struct tmc_drvdata *drvdata)
>  {
> +	const struct tmc_sysfs_ops *byte_cntr_sysfs_ops;
>  	int ret = 0;
> 
> +	byte_cntr_sysfs_ops = tmc_get_byte_cntr_sysfs_ops(drvdata);
> +	if (byte_cntr_sysfs_ops) {
> +		ret = byte_cntr_sysfs_ops->read_prepare(drvdata);
> +		goto out;
> +	}
> +
>  	if (drvdata->sysfs_ops)
>  		ret = drvdata->sysfs_ops->read_prepare(drvdata);
>  	else
>  		ret = -EINVAL;
> 

I understand ctcu usage is per session & per device, but at the start of the session would it not be better to have a function in the ctcu code that takes the drvdata->sysfs_ops and substitutes the callback directly, restoring it at the end. 


> +out:
>  	if (!ret)
>  		dev_dbg(&drvdata->csdev->dev, "TMC read start\n");
> 
> @@ -245,13 +278,21 @@ static int tmc_read_prepare(struct tmc_drvdata
> *drvdata)
> 
>  static int tmc_read_unprepare(struct tmc_drvdata *drvdata)
>  {
> +	const struct tmc_sysfs_ops *byte_cntr_sysfs_ops;
>  	int ret = 0;
> 
> +	byte_cntr_sysfs_ops = tmc_get_byte_cntr_sysfs_ops(drvdata);
> +	if (byte_cntr_sysfs_ops) {
> +		ret = byte_cntr_sysfs_ops->read_unprepare(drvdata);
> +		goto out;
> +	}
> +
>  	if (drvdata->sysfs_ops)
>  		ret = drvdata->sysfs_ops->read_unprepare(drvdata);
>  	else
>  		ret = -EINVAL;
> 

Again override / restore over the session. 

Regards

Mike

> +out:
>  	if (!ret)
>  		dev_dbg(&drvdata->csdev->dev, "TMC read end\n");
> 
> @@ -277,6 +318,12 @@ static int tmc_open(struct inode *inode, struct file
> *file)
>  static ssize_t tmc_get_sysfs_trace(struct tmc_drvdata *drvdata, loff_t pos,
> size_t len,
>  				   char **bufpp)
>  {
> +	const struct tmc_sysfs_ops *byte_cntr_sysfs_ops;
> +
> +	byte_cntr_sysfs_ops = tmc_get_byte_cntr_sysfs_ops(drvdata);
> +	if (byte_cntr_sysfs_ops)
> +		return byte_cntr_sysfs_ops->get_trace_data(drvdata, pos,
> len, bufpp);
> +
>  	return drvdata->sysfs_ops->get_trace_data(drvdata, pos, len, bufpp);
>  }
> 
> @@ -297,7 +344,11 @@ static ssize_t tmc_read(struct file *file, char __user
> *data, size_t len,
>  		return -EFAULT;
>  	}
> 
> -	*ppos += actual;
> +	if (drvdata->reading_node)
> +		drvdata->reading_node->pos += actual;
> +	else
> +		*ppos += actual;
> +
>  	dev_dbg(&drvdata->csdev->dev, "%zu bytes copied\n", actual);
> 
>  	return actual;
> 
> --
> 2.34.1