Thread (13 messages) 13 messages, 3 authors, 2020-07-02

Re: [PATCH v5 4/5] coresight: sysfs: Allow select default sink on source enable.

From: Mathieu Poirier <mathieu.poirier@linaro.org>
Date: 2020-06-29 19:07:45
Also in: linux-doc

Hi Mike,

I have applied patches 1 to 3 of this set.  Please see below for comments on
this patch.

On Tue, Jun 16, 2020 at 05:40:05PM +0100, Mike Leach wrote:
quoted hunk ↗ jump to hunk
When enabling a trace source using sysfs, allow the CoreSight system to
auto-select a default sink if none has been enabled by the user.

Uses the sink select algorithm that uses the default select priorities
set when sinks are registered with the system. At present this will
prefer ETR over ETB / ETF.

Adds a new attribute 'last_sink' to source CoreSight devices. This is set
when a source is enabled using sysfs, to the sink that the device will
trace into. This applies for both user enabled and default enabled sinks.

Signed-off-by: Mike Leach <redacted>
---
 drivers/hwtracing/coresight/coresight.c | 39 +++++++++++++++++++++++--
 include/linux/coresight.h               |  3 ++
 2 files changed, 40 insertions(+), 2 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c
index e9c90f2de34a..db39e0b56994 100644
--- a/drivers/hwtracing/coresight/coresight.c
+++ b/drivers/hwtracing/coresight/coresight.c
@@ -934,6 +934,16 @@ static void coresight_clear_default_sink(struct coresight_device *csdev)
 	}
 }
 
+static void coresight_set_last_sink_name(struct coresight_device *source,
+					 struct coresight_device *sink)
+{
+	/* remove current value and set new one if *sink not NULL */
+	kfree(source->last_sink);
+	source->last_sink = NULL;
+	if (sink)
+		source->last_sink = kstrdup(dev_name(&sink->dev), GFP_KERNEL);
+}
+
 /** coresight_validate_source - make sure a source has the right credentials
  *  @csdev:	the device structure for a source.
  *  @function:	the function this was called from.
@@ -994,8 +1004,15 @@ int coresight_enable(struct coresight_device *csdev)
 	 */
 	sink = coresight_get_enabled_sink(false);
 	if (!sink) {
-		ret = -EINVAL;
-		goto out;
+		/* look for a default sink if nothing enabled */
+		sink = coresight_find_default_sink(csdev);
+		if (!sink) {
+			ret = -EINVAL;
+			goto out;
+		}
+		/* mark the default as enabled */
+		sink->activated = true;
+		dev_info(&sink->dev, "Enabled default sink.");
I'm very ambivalent about extending the automatic sink selection to the sysfs
interface, mainly because of the new sysfs entry it requires.  I find it
clunky that users don't have to specify the sink to use but have to explicitly
disable it after the trace session.  We could automatically disable the sink
after a trace session but that would break with the current sysfs heuristic
where sinks have to be explicitly enabled and disabled.

Thanks,
Mathieu 
quoted hunk ↗ jump to hunk
 	}
 
 	path = coresight_build_path(csdev, sink);
@@ -1033,6 +1050,9 @@ int coresight_enable(struct coresight_device *csdev)
 		break;
 	}
 
+	/* record name of sink used for this session */
+	coresight_set_last_sink_name(csdev, sink);
+
 out:
 	mutex_unlock(&coresight_mutex);
 	return ret;
@@ -1145,6 +1165,19 @@ static ssize_t enable_source_store(struct device *dev,
 }
 static DEVICE_ATTR_RW(enable_source);
 
+static ssize_t last_sink_show(struct device *dev,
+			      struct device_attribute *attr, char *buf)
+{
+	struct coresight_device *csdev = to_coresight_device(dev);
+	ssize_t size = 0;
+
+	if (csdev->last_sink)
+		size = scnprintf(buf, PAGE_SIZE, "%s\n", csdev->last_sink);
+	return size;
+}
+static DEVICE_ATTR_RO(last_sink);
+
+
 static struct attribute *coresight_sink_attrs[] = {
 	&dev_attr_enable_sink.attr,
 	NULL,
@@ -1153,6 +1186,7 @@ ATTRIBUTE_GROUPS(coresight_sink);
 
 static struct attribute *coresight_source_attrs[] = {
 	&dev_attr_enable_source.attr,
+	&dev_attr_last_sink.attr,
 	NULL,
 };
 ATTRIBUTE_GROUPS(coresight_source);
@@ -1524,6 +1558,7 @@ void coresight_unregister(struct coresight_device *csdev)
 	/* Remove references of that device in the topology */
 	coresight_remove_conns(csdev);
 	coresight_clear_default_sink(csdev);
+	coresight_set_last_sink_name(csdev, NULL);
 	coresight_release_platform_data(csdev, csdev->pdata);
 	device_unregister(&csdev->dev);
 }
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index 58fffdecdbfd..fc320dd2cedc 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -184,6 +184,8 @@ struct coresight_sysfs_link {
  *		from source to that sink.
  * @ea:		Device attribute for sink representation under PMU directory.
  * @def_sink:	cached reference to default sink found for this device.
+ * @last_sink:	Name of last sink used for this source to trace into. Set when
+ *		enabling a source using sysfs - only set on a source device.
  * @ect_dev:	Associated cross trigger device. Not part of the trace data
  *		path or connections.
  * @nr_links:   number of sysfs links created to other components from this
@@ -203,6 +205,7 @@ struct coresight_device {
 	bool activated;	/* true only if a sink is part of a path */
 	struct dev_ext_attribute *ea;
 	struct coresight_device *def_sink;
+	const char *last_sink;
 	/* cross trigger handling */
 	struct coresight_device *ect_dev;
 	/* sysfs links between components */
-- 
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help