Re: [PATCH v2 3/5] coresight-tnoc: add nodes to configure flush

Yuanfang Zhang
2025-03-06T08:43:57Z

On 2/28/2025 12:23 AM, Leo Yan wrote:
> On Wed, Feb 26, 2025 at 07:05:52PM +0800, Yuanfang Zhang wrote:
>>
>> Two nodes for configure flush are added here:
>> 1. flush_req: write 1 to initiates a flush sequence.
>>
>> 2. flush_state: read this node to get flush status. 0: sequence in
>> progress; 1: sequence has been completed.
>>
>> Signed-off-by: Yuanfang Zhang <quic_yuanfang@quicinc.com>
>> ---
>> drivers/hwtracing/coresight/coresight-tnoc.c | 73 ++++++++++++++++++++++++++++
>> drivers/hwtracing/coresight/coresight-tnoc.h | 4 ++
>> 2 files changed, 77 insertions(+)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-tnoc.c b/drivers/hwtracing/coresight/coresight-tnoc.c
>> index fad8e61f05ef25989aba1be342c547f835e8953a..20231f28ddcb6a60d9b3c1ca3e0ca4d731dac39c 100644
>> --- a/drivers/hwtracing/coresight/coresight-tnoc.c
>> +++ b/drivers/hwtracing/coresight/coresight-tnoc.c
>> @@ -16,6 +16,78 @@
>> #include "coresight-tnoc.h"
>> #include "coresight-trace-id.h"
>>
>> +static ssize_t flush_req_store(struct device *dev,
>> + struct device_attribute *attr,
>> + const char *buf,
>> + size_t size)
>> +{
>> + struct trace_noc_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> + struct coresight_device *csdev = drvdata->csdev;
>> + unsigned long val;
>> + u32 reg;
>> +
>> + if (kstrtoul(buf, 10, &val))
>> + return -EINVAL;
>> +
>> + if (val != 1)
>> + return -EINVAL;
>> +
>> + spin_lock(&drvdata->spinlock);
>> + if (csdev->refcnt == 0) {
>> + spin_unlock(&drvdata->spinlock);
>> + return -EPERM;
>> + }
>> +
>> + reg = readl_relaxed(drvdata->base + TRACE_NOC_CTRL);
>> + reg = reg | TRACE_NOC_CTRL_FLUSHREQ;
>> + writel_relaxed(reg, drvdata->base + TRACE_NOC_CTRL);
>
> How can userspace determine when to trigger a flush?
It can be triggered under any circumstances.
>
> Generally, a driver kicks off a flush operation for a hardware before
> reading data from buffer or when disable a link path. I don't know the
> hardware mechanism of TNOC, but seems to me, it does not make sense to
> let the userspace to trigger a hardware flush, given the userspace has
> no knowledge for device's state.
TNOC supports the aforementioned flush operation, and it also adds this
flush functionality, allowing users to set the flush themselves.
>
> Furthermore, based on my understanding for patch 02 and 03, the working
> flow is also concerned me. IIUC, you want to use the driver to create
> a linkage and then use userspace program to poll state and trigger
> flushing. Could you explain why use this way for managing the device?
>
TNOC support flush just like other links. This interface simply provides
customers with an additional option to trigger the flush.

> Thanks,
> Leo
>
>> +
>> + spin_unlock(&drvdata->spinlock);
>> +
>> + return size;
>> +}
>> +static DEVICE_ATTR_WO(flush_req);
>> +
>> +/*
>> + * flush-sequence status:
>> + * value 0: sequence in progress;
>> + * value 1: sequence has been completed.
>> + */
>> +static ssize_t flush_status_show(struct device *dev,
>> + struct device_attribute *attr,
>> + char *buf)
>> +{
>> + struct trace_noc_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> + struct coresight_device *csdev = drvdata->csdev;
>> + u32 val;
>> +
>> + spin_lock(&drvdata->spinlock);
>> + if (csdev->refcnt == 0) {
>> + spin_unlock(&drvdata->spinlock);
>> + return -EPERM;
>> + }
>> +
>> + val = readl_relaxed(drvdata->base + TRACE_NOC_CTRL);
>> + spin_unlock(&drvdata->spinlock);
>> + return sysfs_emit(buf, "%lu\n", BMVAL(val, 2, 2));
>> +}
>> +static DEVICE_ATTR_RO(flush_status);
>> +
>> +static struct attribute *trace_noc_attrs[] = {
>> + &dev_attr_flush_req.attr,
>> + &dev_attr_flush_status.attr,
>> + NULL,
>> +};
>> +
>> +static struct attribute_group trace_noc_attr_grp = {
>> + .attrs = trace_noc_attrs,
>> +};
>> +
>> +static const struct attribute_group *trace_noc_attr_grps[] = {
>> + &trace_noc_attr_grp,
>> + NULL,
>> +};
>> +
>> static void trace_noc_enable_hw(struct trace_noc_drvdata *drvdata)
>> {
>> u32 val;
>> @@ -142,6 +214,7 @@ static int trace_noc_probe(struct amba_device *adev, const struct amba_id *id)
>> return ret;
>>
>> desc.ops = &trace_noc_cs_ops;
>> + desc.groups = trace_noc_attr_grps;
>> desc.type = CORESIGHT_DEV_TYPE_LINK;
>> desc.subtype.link_subtype = CORESIGHT_DEV_SUBTYPE_LINK_MERG;
>> desc.pdata = adev->dev.platform_data;
>> diff --git a/drivers/hwtracing/coresight/coresight-tnoc.h b/drivers/hwtracing/coresight/coresight-tnoc.h
>> index b6bd1ef659897d8e0994c5e8514e8cbdd16eebd8..d0fe8f52709ff4147d66dbf90987595012cfaa4e 100644
>> --- a/drivers/hwtracing/coresight/coresight-tnoc.h
>> +++ b/drivers/hwtracing/coresight/coresight-tnoc.h
>> @@ -10,6 +10,10 @@
>>
>> /* Enable generation of output ATB traffic.*/
>> #define TRACE_NOC_CTRL_PORTEN BIT(0)
>> +/* Writing 1 to initiate a flush sequence.*/
>> +#define TRACE_NOC_CTRL_FLUSHREQ BIT(1)
>> +/* 0: sequence in progress; 1: sequence has been completed.*/
>> +#define TRACE_NOC_CTRL_FLUSHSTATUS BIT(2)
>> /* Writing 1 to issue a FREQ or FREQ_TS packet*/
>> #define TRACE_NOC_CTRL_FREQTSREQ BIT(5)
>> /* Sets the type of issued ATB FLAG packets. 0: 'FLAG' packets; 1: 'FLAG_TS' packets.*/
>>
>> --
>> 2.34.1
>>
>> _______________________________________________
>> CoreSight mailing list -- coresight@lists.linaro.org
>> To unsubscribe send an email to coresight-leave@lists.linaro.org