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

[Jie Gan]

On 3/6/2026 5:44 PM, Mike Leach wrote:
> 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.

Hi Mike,

Thanks for the suggestion. That would be a better solution, and this 
patch is no longer needed.
I will export etr_sysfs_ops in tmc header file so the CTCU driver can 
retrieve the pointer and restore to the etr_sysfs_ops.
With this solution, sysfs_ops will be switched to byte_cntr_sysfs_ops in 
ctcu_enable and restored in ctcu_disable.

Thanks,
Jie

> > +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