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@xxxxxxxxxxx> >> --- >> 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@xxxxxxxxxxxxxxxx >> To unsubscribe send an email to coresight-leave@xxxxxxxxxxxxxxxx