Re: [PATCH V2] coresight: Make sysFS functional on topologies with per core sink

On Fri, Aug 07, 2020 at 10:37:00PM +0530, Linu Cherian wrote:

Hi Mathieu,

Any comments on this patch ?

Let's see... You're poking me to review a patch that doesn't clear checkpatch (with a blatant violation) and that, 6 days after it was sent out?

On Sat, Aug 1, 2020 at 2:40 PM Linu Cherian lcherian@marvell.com wrote:

...

Coresight driver assumes sink is common across all the ETMs, and tries to build a path between ETM and the first enabled sink found using bus based search. This breaks sysFS usage on implementations that has multiple per core sinks in enabled state.

For this,

• coresight_find_sink API is updated with an additional flag so that it is able to return an enabled sink
• coresight_get_enabled_sink API is updated to do a connection based search, when a source reference is given.

Change-Id: I6cc91ddb3ef8936a8f41a5f7c7c455b0ece9d85d Signed-off-by: Linu Cherian lcherian@marvell.com

---

Applies on https://git.linaro.org/kernel/coresight.git/log/?h=next

Changes in V2:

• Fixed few typos in commit message
• Rephrased commit message

.../hwtracing/coresight/coresight-etm-perf.c | 2 +- drivers/hwtracing/coresight/coresight-priv.h | 5 +- drivers/hwtracing/coresight/coresight.c | 51 +++++++++++++++++-- 3 files changed, 51 insertions(+), 7 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index 1a3169e69bb1..25041d2654e3 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -223,7 +223,7 @@ static void *etm_setup_aux(struct perf_event *event, void **pages, id = (u32)event->attr.config2;

             sink = coresight_get_sink_by_id(id);

...

    } else {

•           sink = coresight_get_enabled_sink(true);
  

•           sink = coresight_get_enabled_sink(NULL, true);
    }
  
    mask = &event_data->mask;
  

diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h index f2dc625ea585..010ed26db340 100644 --- a/drivers/hwtracing/coresight/coresight-priv.h +++ b/drivers/hwtracing/coresight/coresight-priv.h @@ -148,10 +148,13 @@ static inline void coresight_write_reg_pair(void __iomem *addr, u64 val, void coresight_disable_path(struct list_head *path); 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_enabled_sink(struct coresight_device *source, bool reset); struct coresight_device *coresight_get_sink_by_id(u32 id); struct coresight_device * coresight_find_default_sink(struct coresight_device *csdev); +struct coresight_device * +coresight_find_enabled_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 e9c90f2de34a..ae69169c58d3 100644 --- a/drivers/hwtracing/coresight/coresight.c +++ b/drivers/hwtracing/coresight/coresight.c @@ -566,6 +566,10 @@ static int coresight_enabled_sink(struct device *dev, const void *data)

/**

• coresight_get_enabled_sink - returns the first enabled sink found on the bus

• • When a source reference is given, enabled sink is found using connection based
• • search.
• • @source: Coresight source device reference
  • @deactivate: Whether the 'enable_sink' flag should be reset
  • When operated from perf the deactivate parameter should be set to 'true'.

@@ -576,10 +580,21 @@ static int coresight_enabled_sink(struct device *dev, const void *data)

• parameter should be set to 'false', hence mandating users to explicitly
• clear the flag.

*/ -struct coresight_device *coresight_get_enabled_sink(bool deactivate) +struct coresight_device * +coresight_get_enabled_sink(struct coresight_device *source, bool deactivate) { struct device *dev = NULL;

•   struct coresight_device *sink;
  

•   if (!source)
  

•           goto bus_search;
  

•   sink = coresight_find_enabled_sink(source);
  

•   if (sink && deactivate)
  

•           sink->activated = false;
  

•   return sink;
  
  

+bus_search: dev = bus_find_device(&coresight_bustype, NULL, &deactivate, coresight_enabled_sink);

@@ -828,6 +843,7 @@ coresight_select_best_sink(struct coresight_device *sink, int *depth,

• @csdev: source / current device to check.
• @depth: [in] search depth of calling dev, [out] depth of found sink.

• • @enabled: flag to search only enabled sinks
  • This will walk the connection path from a source (ETM) till a suitable
  • sink is encountered and return that sink to the original caller.

@@ -839,7 +855,7 @@ coresight_select_best_sink(struct coresight_device *sink, int *depth,

• 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) +coresight_find_sink(struct coresight_device *csdev, int *depth, bool enabled) { int i, curr_depth = *depth + 1, found_depth = 0; struct coresight_device *found_sink = NULL; @@ -862,7 +878,8 @@ coresight_find_sink(struct coresight_device *csdev, int *depth)

            child_dev = csdev->pdata->conns[i].child_dev;
            if (child_dev)

•                   sink = coresight_find_sink(child_dev, &child_depth);
  

•                   sink = coresight_find_sink(child_dev, &child_depth,
  

•                                              enabled);
  
            if (sink)
                    found_sink = coresight_select_best_sink(found_sink,
  

@@ -872,6 +889,10 @@ coresight_find_sink(struct coresight_device *csdev, int *depth) }

return_def_sink:

•   /* Check if we need to return an enabled sink */
  

•   if (enabled && found_sink)
  

•           if (!found_sink->activated)
  

•                   found_sink = NULL;
    /* return found sink and depth */
    if (found_sink)
            *depth = found_depth;
  

@@ -901,10 +922,30 @@ coresight_find_default_sink(struct coresight_device *csdev)

    /* 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);
  

•           csdev->def_sink = coresight_find_sink(csdev, &depth, false);
    return csdev->def_sink;
  

}

+/**

• • coresight_find_enabled_sink: Find the suitable enabled sink
• • @csdev: starting source to find a connected sink.
• • Walks connections graph looking for a suitable sink to enable for the
• • supplied source. Uses CoreSight device subtypes and distance from source
• • to select the best sink.
• • Used in cases where the CoreSight user (sysfs) has selected a sink.
• */

+struct coresight_device * +coresight_find_enabled_sink(struct coresight_device *csdev) +{

•   int depth = 0;
  

•   /* look for the enabled sink */
  

•   return coresight_find_sink(csdev, &depth, true);
  

+}

static int coresight_remove_sink_ref(struct device *dev, void *data) { struct coresight_device *sink = data; @@ -992,7 +1033,7 @@ int coresight_enable(struct coresight_device *csdev) * Search for a valid sink for this session but don't reset the * "enable_sink" flag in sysFS. Users get to do that explicitly. */

•   sink = coresight_get_enabled_sink(false);
  

•   sink = coresight_get_enabled_sink(csdev, false);
    if (!sink) {
            ret = -EINVAL;
            goto out;
  

-- 2.25.1