Hi Suzuki,
On Mon, 4 May 2020 at 22:22, Suzuki K Poulose suzuki.poulose@arm.com wrote:
Hi Mike,
On 05/01/2020 03:02 PM, Mike Leach wrote:
Adds a method to select a suitable sink connected to a given source.
In cases where no sink is defined, the coresight_find_default_sink routine can search from a given source, through the child connections until a suitable sink is found.
The suitability is defined in a sink_select_priority parameter on the CoreSight device.
This allows for default sink to be discovered were none is specified (e.g. perf command line)
The patch looks good to me. Some comments below.
Signed-off-by: Mike Leach mike.leach@linaro.org
drivers/hwtracing/coresight/coresight-priv.h | 2 + drivers/hwtracing/coresight/coresight.c | 76 ++++++++++++++++++++ include/linux/coresight.h | 10 ++- 3 files changed, 87 insertions(+), 1 deletion(-)
diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h index 890f9a5c97c6..856d8a29658c 100644 --- a/drivers/hwtracing/coresight/coresight-priv.h +++ b/drivers/hwtracing/coresight/coresight-priv.h @@ -150,6 +150,8 @@ int coresight_enable_path(struct list_head *path, u32 mode, void *sink_data); struct coresight_device *coresight_get_sink(struct list_head *path); struct coresight_device *coresight_get_enabled_sink(bool reset); struct coresight_device *coresight_get_sink_by_id(u32 id); +struct coresight_device * +coresight_find_default_sink(struct coresight_device *csdev); struct list_head *coresight_build_path(struct coresight_device *csdev, struct coresight_device *sink); void coresight_release_path(struct list_head *path); diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c index c71553c09f8e..0564d26f5eb3 100644 --- a/drivers/hwtracing/coresight/coresight.c +++ b/drivers/hwtracing/coresight/coresight.c @@ -769,6 +769,82 @@ void coresight_release_path(struct list_head *path) path = NULL; }
+/**
- coresight_find_sink - recursive function to walk trace connections from
- source to find a suitable default sink.
- @csdev: source / current device to check.
- @first_sink: the first lower priority sink found.
- Sinks are given priority weighting, the first HI priority sink will be
- used, if not found then the next highest priority sink will be recorded
- for possible later use.
- (Higher numbers indicate lower priorities, 0 not a sink or not to be
- selected as default)
- This will walk the connection path from a source (etm) till a suitable
- sink is encountered and return that sink to the original caller.
- return HI priority sink, or NULL if not found.
- Best lower priority sink returned in first_sink.
- */
+static struct coresight_device * +coresight_find_sink(struct coresight_device *csdev,
struct coresight_device **first_sink)+{
int i;/* return if highest priority sink */if (csdev->sink_select_priority == CORESIGHT_SINK_SELECT_PRIORITY_HI)return csdev;/* record if lower priority sink */if (csdev->sink_select_priority > CORESIGHT_SINK_SELECT_PRIORITY_HI) {if ((*first_sink == NULL) ||((*first_sink)->sink_select_priority >csdev->sink_select_priority)) {*first_sink = csdev;}}/** Not a sink we want - recursively explore each port found on* this element.*/for (i = 0; i < csdev->pdata->nr_outport; i++) {struct coresight_device *child_dev, *sink;child_dev = csdev->pdata->conns[i].child_dev;if (child_dev) {sink = coresight_find_sink(child_dev, first_sink);if (sink)return sink;How do we handle multiple ETRs ? I believe we need to add some notion of distance with a "found" sink above and choose the closest one after going through each of the component paths.
At present we are using a rather basic just use the first found algorithm. So the first found of multiple ETRs will be the one used. To add depth as a factor is possible - it would make early search termination trickier, but not impossible.
}}return NULL;+}
+/**
- coresight_find_default_sink: Find a sink suitable for use as a
- default sink.
- @csdev: starting source to find a connected sink.
- Walks connections graph looking for a suitable sink to enable for the
- supplied source. Uses sink_select_priority values.
- Used in cases where the CoreSight user (perf / sysfs) has not selected a
- sink.
- */
+struct coresight_device * +coresight_find_default_sink(struct coresight_device *csdev) +{
struct coresight_device *first_sink = NULL, *sink;sink = coresight_find_sink(csdev, &first_sink);return sink ? sink : first_sink;+}
- /** 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.
diff --git a/include/linux/coresight.h b/include/linux/coresight.h index 193cc9dbf448..ff443761c8fc 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -161,10 +161,13 @@ struct coresight_connection {
- @enable: 'true' if component is currently part of an active path.
- @activated: 'true' only if a _sink_ has been activated. A sink can be
activated but not yet enabled. Enabling for a _sink_
appens when a source has been selected for that it.
happens when a source has been selected for that it.- @ea: Device attribute for sink representation under PMU directory.
- @ect_dev: Associated cross trigger device. Not part of the trace data
path or connections.
- @sink_select_priority: Used when the system is required to choose a default
sink. Higher numbers indicate lower priority. Unused (0) if not
a sink / or sink not supported as a default.minor nit: this could be sink_prio to make it shorter.
Mathieu has a similar comment - will do something.
Also, we could optimize (could be done later) the sink selection by caching "the default" sink for each ETM. This can be invalidated whenever a sink is registered.
Repeatedly walking the chain for all ETMs whenever a sink is registered seems a log of effort for relatively little gain. (ordering of registration adds a layer of complication too). However, if we cache the value at first use - i.e. when either perf or sysfs looks for the default sink for a given etm, then we cache only when needed and avoid repetition.
Regards
Mike
*/ struct coresight_device { struct coresight_platform_data *pdata; @@ -180,8 +183,13 @@ struct coresight_device { struct dev_ext_attribute *ea; /* cross trigger handling */ struct coresight_device *ect_dev;
/* mark priority when system choosing a default sink */int sink_select_priority;+/* The highest priority sink - will terminate default sink search */ +#define CORESIGHT_SINK_SELECT_PRIORITY_HI 1
- /*
- coresight_dev_list - Mapping for devices to "name" index for device
- names.