Hi Leo,
On Sat, 23 May 2020 at 08:18, Leo Yan leo.yan@linaro.org wrote:
Hi Mike,
Several minor comments.
On Tue, May 19, 2020 at 06:31:24PM +0100, 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 by the sink coresight_dev_subtype on the CoreSight device, and the distance from the source by counting connections.
Higher value subtype is preferred - where these are equal, shorter distance from source is used as a tie-break.
This allows for default sink to be discovered were none is specified (e.g. perf command line)
Signed-off-by: Mike Leach mike.leach@linaro.org
drivers/hwtracing/coresight/coresight-priv.h | 2 + drivers/hwtracing/coresight/coresight.c | 137 +++++++++++++++++++ include/linux/coresight.h | 3 + 3 files changed, 142 insertions(+)
diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h index 36c943ae94d5..f2dc625ea585 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 f3efbb3b2b4d..96b9ad249c62 100644 --- a/drivers/hwtracing/coresight/coresight.c +++ b/drivers/hwtracing/coresight/coresight.c @@ -769,6 +769,143 @@ void coresight_release_path(struct list_head *path) path = NULL; }
+/* return true if the device is a suitable type for a default sink */ +static inline bool coresight_is_def_sink_type(struct coresight_device *csdev) +{
/* sink & correct subtype */if (((csdev->type == CORESIGHT_DEV_TYPE_SINK) ||(csdev->type == CORESIGHT_DEV_TYPE_LINKSINK)) &&(csdev->subtype.sink_subtype >= CORESIGHT_DEV_SUBTYPE_SINK_BUFFER))return true;Seems to me, we don't ned to check 'csdev->type' thus can simplify the condition checking:
if (csdev->subtype.sink_subtype == CORESIGHT_DEV_SUBTYPE_SINK_BUFFER || csdev->subtype.sink_subtype == CORESIGHT_DEV_SUBTYPE_SINK_SYSMEM) return true;
csdev->subtype is a union of all subtypes - so the explicit check for type is required otherwise we could match on source subtypes for example.
Regards
Mike
return false;+}
+/**
- coresight_select_best_sink - return the best sink for use as default from
- the two provided.
- @sink: current best sink.
- @depth: search depth where current sink was found.
- @new_sink: new sink for comparison with current sink.
- @new_depth: search depth where new sink was found.
- Sinks prioritised according to coresight_dev_subtype_sink, with only
- subtypes CORESIGHT_DEV_SUBTYPE_SINK_BUFFER or higher being used.
- Where two sinks of equal priority are found, the sink closest to the
- source is used (smallest search depth).
- return @new_sink & update @depth if better than @sink, else return @sink.
- */
+static struct coresight_device * +coresight_select_best_sink(struct coresight_device *sink, int *depth,
struct coresight_device *new_sink, int new_depth)+{
bool update = false;if (!sink) {/* first found at this level */update = true;} else if (new_sink->subtype.sink_subtype >sink->subtype.sink_subtype) {/* found better sink */update = true;} else if ((new_sink->subtype.sink_subtype ==sink->subtype.sink_subtype) &&(*depth > new_depth)) {/* found same but closer sink */update = true;}if (update)*depth = new_depth;return update ? new_sink : sink;+}
+/**
- coresight_find_sink - recursive function to walk trace connections from
- source to find a suitable default sink.
- @csdev: source / current device to check.
- @depth: [in] search depth of calling dev, [out] depth of found sink.
- This will walk the connection path from a source (ETM) till a suitable
- sink is encountered and return that sink to the original caller.
- If current device is a plain sink return that & depth, otherwise recursively
- call child connections looking for a sink. Select best possible using
- coresight_select_best_sink.
- return best sink found, or NULL if not found at this node or child nodes.
- */
+static struct coresight_device * +coresight_find_sink(struct coresight_device *csdev,
int *depth)+{
int i, curr_depth = *depth+1, found_depth = 0;struct coresight_device *found_sink = NULL;if (coresight_is_def_sink_type(csdev)) {found_depth = curr_depth;found_sink = csdev;if (csdev->type == CORESIGHT_DEV_TYPE_SINK)goto coresight_return_sink;/* look past LINKSINK for something better */}/** Not a sink we want - or possible child sink may be better.* recursively explore each port found on this element.*/for (i = 0; i < csdev->pdata->nr_outport; i++) {struct coresight_device *child_dev, *sink = NULL;int child_depth = curr_depth;child_dev = csdev->pdata->conns[i].child_dev;if (child_dev)sink = coresight_find_sink(child_dev, &child_depth);if (sink)found_sink = coresight_select_best_sink(found_sink,&found_depth,sink,child_depth);}+coresight_return_sink:
suggest to use short jump tag: s/coresight_return_sink/return_sink
/* return found sink and depth */if (found_sink)*depth = found_depth;return found_sink;+}
As Mathieu mentioned, actually I also carefully read up code for the iteration all CoreSight device nodes to find best sink, but I don't find any issue I can say. FWIW:
Reviewed-by: Leo Yan leo.yan@linaro.org
+/**
- 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 csdev->priority values.
- If a sink is found, then the default sink for this device is set and
- will be automatically used in future.
- 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) +{
int depth = 0;/* look for a default sink if we have not found for this device */if (!csdev->def_sink)csdev->def_sink = coresight_find_sink(csdev, &depth);return csdev->def_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 84dc695e87d4..58fffdecdbfd 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -48,6 +48,7 @@ enum coresight_dev_subtype_sink { CORESIGHT_DEV_SUBTYPE_SINK_NONE, CORESIGHT_DEV_SUBTYPE_SINK_PORT, CORESIGHT_DEV_SUBTYPE_SINK_BUFFER,
CORESIGHT_DEV_SUBTYPE_SINK_SYSMEM,};
enum coresight_dev_subtype_link { @@ -182,6 +183,7 @@ struct coresight_sysfs_link {
happens when a source has been selected and a path is enabledfrom source to that sink.- @ea: Device attribute for sink representation under PMU directory.
- @def_sink: cached reference to default sink found for this 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
@@ -200,6 +202,7 @@ struct coresight_device { /* sink specific fields */ bool activated; /* true only if a sink is part of a path */ struct dev_ext_attribute *ea;
struct coresight_device *def_sink; /* cross trigger handling */ struct coresight_device *ect_dev; /* sysfs links between components */-- 2.17.1
CoreSight mailing list CoreSight@lists.linaro.org https://lists.linaro.org/mailman/listinfo/coresight