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

Mathieu Poirier mathieu.poirier at linaro.org
Mon Aug 10 18:25:56 EDT 2020


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 at 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 at 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
> >



More information about the linux-arm-kernel mailing list