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

[Jie Gan <jie.gan@oss.qualcomm.com>]

On 3/6/2026 5:44 PM, Mike Leach wrote:
> 
> Hi,
>> -----Original Message-----
>> From: Jie Gan <jie.gan@oss.qualcomm.com>
>> Sent: Monday, February 23, 2026 6:56 AM
>> To: Suzuki Poulose <Suzuki.Poulose@arm.com>; Mike Leach
>> <Mike.Leach@arm.com>; James Clark <james.clark@linaro.org>; Alexander
>> Shishkin <alexander.shishkin@linux.intel.com>; Rob Herring
>> <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
>> <conor+dt@kernel.org>; Tingwei Zhang
>> <tingwei.zhang@oss.qualcomm.com>; Mao Jinlong
>> <jinlong.mao@oss.qualcomm.com>; Bjorn Andersson
>> <andersson@kernel.org>; Konrad Dybcio <konradybcio@kernel.org>
>> Cc: coresight@lists.linaro.org; linux-arm-kernel@lists.infradead.org; linux-
>> kernel@vger.kernel.org; linux-arm-msm@vger.kernel.org;
>> devicetree@vger.kernel.org; Jie Gan <jie.gan@oss.qualcomm.com>
>> 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@oss.qualcomm.com>
>> ---
>>   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
>