* [PATCH V2] coresight: Make sysFS functional on topologies with per core sink
@ 2020-08-01 9:09 Linu Cherian
2020-08-03 9:37 ` Anshuman Khandual
2020-08-07 17:07 ` Linu Cherian
0 siblings, 2 replies; 6+ messages in thread
From: Linu Cherian @ 2020-08-01 9:09 UTC (permalink / raw)
To: mathieu.poirier, suzuki.poulose, mike.leach
Cc: coresight, linuc.decode, linux-arm-kernel, Linu Cherian
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH V2] coresight: Make sysFS functional on topologies with per core sink
2020-08-01 9:09 [PATCH V2] coresight: Make sysFS functional on topologies with per core sink Linu Cherian
@ 2020-08-03 9:37 ` Anshuman Khandual
2020-08-03 11:26 ` Linu Cherian
2020-08-07 17:07 ` Linu Cherian
1 sibling, 1 reply; 6+ messages in thread
From: Anshuman Khandual @ 2020-08-03 9:37 UTC (permalink / raw)
To: Linu Cherian, mathieu.poirier, suzuki.poulose, mike.leach
Cc: coresight, linuc.decode, linux-arm-kernel
Hello Linu,
Please do mention the tree on which this patch applies on. The coresight
API functions being changed here are not even present on v5.8 but instead
are part of linux-next (atleast next-20200731).
On 08/01/2020 02:39 PM, Linu Cherian wrote:
> Coresight driver assumes sink is common across all the ETMs,
Does 'common' just implies reachable here. Should not all the sinks on
the system be reachable from all the sources.
> 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.
Could you please explain how it breaks the sysfs usage ? Why those per
core sinks were not found through existing bus based search.
>
> 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);
> +}
> +
Why this wrapper is required ? Could not coresight_find_sink() be used
directly in the only caller i.e coresight_get_enabled_sink(). Besides,
their names are really very similar and might lead to confusion.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V2] coresight: Make sysFS functional on topologies with per core sink
2020-08-03 9:37 ` Anshuman Khandual
@ 2020-08-03 11:26 ` Linu Cherian
0 siblings, 0 replies; 6+ messages in thread
From: Linu Cherian @ 2020-08-03 11:26 UTC (permalink / raw)
To: Anshuman Khandual
Cc: mathieu.poirier, suzuki.poulose, coresight, Linu Cherian,
linux-arm-kernel, mike.leach
Hi Anshuman,
On Mon, Aug 3, 2020 at 3:07 PM Anshuman Khandual
<anshuman.khandual@arm.com> wrote:
>
> Hello Linu,
>
> Please do mention the tree on which this patch applies on. The coresight
> API functions being changed here are not even present on v5.8 but instead
> are part of linux-next (atleast next-20200731).
Will be more explicit next time, though i have mentioned the relevant
tree below.
>
> On 08/01/2020 02:39 PM, Linu Cherian wrote:
> > Coresight driver assumes sink is common across all the ETMs,
>
> Does 'common' just implies reachable here. Should not all the sinks on
> the system be reachable from all the sources.
Common implies, a topology where a sink is common to all ETMs,
a global ETR for example. Please see below for details.
>
> > 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.
>
> Could you please explain how it breaks the sysfs usage ? Why those per
> core sinks were not found through existing bus based search.
>
On an implementation with per core ETR, and when a user tries to enable
multiple sinks using the syfs interface, only the first sink on the bus would
get enabled at hardware level.
This is because coresight_get_enabled_sink would always select the
first user enabled
sink on the coresight bus, which works fine on implementations with
global ETR but
not for per core ETR/ETF.
So for example, when an user enables ETM 0 and ETR0 , followed by ETM
1 and ETR 1,
then ETR1 hardware block will never get enabled.
> >
> > 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(-)
> >
> > +/**
> > + * 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);
> > +}
> > +
>
> Why this wrapper is required ? Could not coresight_find_sink() be used
> directly in the only caller i.e coresight_get_enabled_sink(). Besides,
> their names are really very similar and might lead to confusion.
Thought that a wrapper avoids exposing the internals like "depth" in
the search API
to its users.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V2] coresight: Make sysFS functional on topologies with per core sink
2020-08-01 9:09 [PATCH V2] coresight: Make sysFS functional on topologies with per core sink Linu Cherian
2020-08-03 9:37 ` Anshuman Khandual
@ 2020-08-07 17:07 ` Linu Cherian
2020-08-10 22:25 ` Mathieu Poirier
1 sibling, 1 reply; 6+ messages in thread
From: Linu Cherian @ 2020-08-07 17:07 UTC (permalink / raw)
To: Linu Cherian
Cc: coresight, Mike Leach, linux-arm-kernel, Mathieu Poirier, suzuki.poulose
Hi Mathieu,
Any comments on this patch ?
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
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V2] coresight: Make sysFS functional on topologies with per core sink
2020-08-07 17:07 ` Linu Cherian
@ 2020-08-10 22:25 ` Mathieu Poirier
2020-08-11 2:46 ` Linu Cherian
0 siblings, 1 reply; 6+ messages in thread
From: Mathieu Poirier @ 2020-08-10 22:25 UTC (permalink / raw)
To: Linu Cherian
Cc: coresight, Mike Leach, linux-arm-kernel, Linu Cherian, suzuki.poulose
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
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V2] coresight: Make sysFS functional on topologies with per core sink
2020-08-10 22:25 ` Mathieu Poirier
@ 2020-08-11 2:46 ` Linu Cherian
0 siblings, 0 replies; 6+ messages in thread
From: Linu Cherian @ 2020-08-11 2:46 UTC (permalink / raw)
To: Mathieu Poirier
Cc: coresight, Mike Leach, linux-arm-kernel, Linu Cherian, suzuki.poulose
Hi Mathieu,
On Mon Aug 10, 2020 at 04:25:56PM -0600, Mathieu Poirier wrote:
> 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?
Apologies, it happened by mistake.
Have submitted V3 fixing this.
--
Linu cherian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-08-11 2:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-01 9:09 [PATCH V2] coresight: Make sysFS functional on topologies with per core sink Linu Cherian
2020-08-03 9:37 ` Anshuman Khandual
2020-08-03 11:26 ` Linu Cherian
2020-08-07 17:07 ` Linu Cherian
2020-08-10 22:25 ` Mathieu Poirier
2020-08-11 2:46 ` Linu Cherian
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).