Hi Mathieu,
On Mon, 4 May 2020 at 21:31, Mathieu Poirier mathieu.poirier@linaro.org wrote:
On Fri, May 01, 2020 at 03:02:22PM +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 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)
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)
I think we should go for higher number == higher priority because 1) it is more intuitive and 2) it allows us to grow with minimal changes. Right now ETB10, TMC-ETF and TPIU are '0' and ETR is '1'. If new IP comes out we can assign them higher numbers with little changes.
Went for this as it matches priority scheme for linux kernel where 'nice' numbers have higher value for lower priority (only difference being we use 0 as a "don't use" value and never use -ve values) & I was thinking 1st priority is more important than 2nd priority etc.
- 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;If @csdev is a replicator and a sink is found on port '0' components connected, directly or not, to port '1' won't be searched.
Correct - by design - return the first found highest priority sink.
Also, instead of contending with a 'sink' and a 'first_sink' I would simply make coresight_find_sink() return the highest priority sink found from any @csdev port. That should simplify thing a little.
sink is used to early terminate the search if we find a sink that has the highest possible priority.
}}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.This is obviously wrong, so much I can't even figure out what I meant. Please remove the entire sentence in a cleanup patch that goes at the beginning of this set.
- @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.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;When I first looked at this set I didn't think there was a need for something like this but after reflection we do since information about the kind of sink (ETB10, TMC-ETF/ETR, TPIU) isn't available from the coresight_device. You can also forget my comment about things not working for ETB10 and TPIU, it is clearer now.
I would simply call this "priority" and move it in the sink section, i.e just under @activated. For extra credit please move @ea after @ops (also in a prequel patch) since it is used for all coresight_device.
could do this - or rename per suzukis suggestion.
};
+/* The highest priority sink - will terminate default sink search */ +#define CORESIGHT_SINK_SELECT_PRIORITY_HI 1
Why not calling this CORESIGHT_PRIORITY_ETR, so that when the next best thing comes out we can have CORESIGHT_PRIORITY_NEXT_BEST_THING. Hopefully we won't need to do that because everyone will move to 1:1 topologies but I know better.
I wanted to completely decouple the "Type" of sink from the selection priority. It is not important that it is an ETR now or maybe something else later. If something new comes along that is better then it is up to the sink drivers to allocate priorities accordingly. At present it is simple as a single driver covers the sinks of interest, but in future we may need to be more dynamic about allocating sinks.
For you next revision, please rebase on coresight-next, I couldn't apply this one properly.
Sorry - was using Linux-5.7-rc3 main branch. Hadn;t realised there had been sufficient diversion that this would not also work on your coresight branch.
Mike
Thanks, Mathieu
/*
- coresight_dev_list - Mapping for devices to "name" index for device
- names.
-- 2.17.1