linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] coresight: add support to enable more coresight paths
@ 2021-04-15  3:08 Tao Zhang
  0 siblings, 0 replies; 4+ messages in thread
From: Tao Zhang @ 2021-04-15  3:08 UTC (permalink / raw)
  To: Tingwei Zhang, Tao Zhang, Mao Jinlong, Yuanfang Zhang
  Cc: linux-kernel, linux-arm-kernel, Satyajit Desai,
	Rama Aparna Mallavarapu, Mulu He

Current coresight implementation only supports enabling source
ETMs or STM. This patch adds support to enable more kinds of
coresight source to sink paths. We build a path from source to
sink when any source is enabled and store it in a list. When the
source is disabled, we fetch the corresponding path from the list
and decrement the refcount on each device in the path. The device
is disabled if the refcount reaches zero. Don't store path to
coresight data structure of source to avoid unnecessary change to
ABI.
Since some targets may have coresight sources other than STM and
ETMs, we need to add this change to support these coresight
devices.

Signed-off-by: Satyajit Desai <sadesai@codeaurora.org>
Signed-off-by: Rama Aparna Mallavarapu <aparnam@codeaurora.org>
Signed-off-by: Mulu He <muluhe@codeaurora.org>
Signed-off-by: Tingwei Zhang <tingwei@codeaurora.org>
Signed-off-by: Tao Zhang <taozha@codeaurora.org>
---
 drivers/hwtracing/coresight/coresight-core.c | 101 +++++++++++++++------------
 1 file changed, 56 insertions(+), 45 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 4ba801d..7dfadb6 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -35,18 +35,16 @@ struct coresight_node {
 };
 
 /*
- * When operating Coresight drivers from the sysFS interface, only a single
- * path can exist from a tracer (associated to a CPU) to a sink.
+ * struct coresight_path - path from source to sink
+ * @path:	Address of path list.
+ * @link:	hook to the list.
  */
-static DEFINE_PER_CPU(struct list_head *, tracer_path);
+struct coresight_path {
+	struct list_head *path;
+	struct list_head link;
+};
 
-/*
- * As of this writing only a single STM can be found in CS topologies.  Since
- * there is no way to know if we'll ever see more and what kind of
- * configuration they will enact, for the time being only define a single path
- * for STM.
- */
-static struct list_head *stm_path;
+static LIST_HEAD(cs_active_paths);
 
 /*
  * When losing synchronisation a new barrier packet needs to be inserted at the
@@ -326,7 +324,7 @@ static void coresight_disable_sink(struct coresight_device *csdev)
 	if (ret)
 		return;
 	coresight_control_assoc_ectdev(csdev, false);
-	csdev->enable = false;
+	csdev->activated = false;
 }
 
 static int coresight_enable_link(struct coresight_device *csdev,
@@ -562,6 +560,20 @@ int coresight_enable_path(struct list_head *path, u32 mode, void *sink_data)
 	goto out;
 }
 
+static struct coresight_device *coresight_get_source(struct list_head *path)
+{
+	struct coresight_device *csdev;
+
+	if (!path)
+		return NULL;
+
+	csdev = list_first_entry(path, struct coresight_node, link)->csdev;
+	if (csdev->type != CORESIGHT_DEV_TYPE_SOURCE)
+		return NULL;
+
+	return csdev;
+}
+
 struct coresight_device *coresight_get_sink(struct list_head *path)
 {
 	struct coresight_device *csdev;
@@ -1047,9 +1059,23 @@ static int coresight_validate_source(struct coresight_device *csdev,
 	return 0;
 }
 
+static int coresight_store_path(struct list_head *path)
+{
+	struct coresight_path *node;
+
+	node = kzalloc(sizeof(struct coresight_path), GFP_KERNEL);
+	if (!node)
+		return -ENOMEM;
+
+	node->path = path;
+	list_add(&node->link, &cs_active_paths);
+
+	return 0;
+}
+
 int coresight_enable(struct coresight_device *csdev)
 {
-	int cpu, ret = 0;
+	int ret = 0;
 	struct coresight_device *sink;
 	struct list_head *path;
 	enum coresight_dev_subtype_source subtype;
@@ -1094,25 +1120,9 @@ int coresight_enable(struct coresight_device *csdev)
 	if (ret)
 		goto err_source;
 
-	switch (subtype) {
-	case CORESIGHT_DEV_SUBTYPE_SOURCE_PROC:
-		/*
-		 * When working from sysFS it is important to keep track
-		 * of the paths that were created so that they can be
-		 * undone in 'coresight_disable()'.  Since there can only
-		 * be a single session per tracer (when working from sysFS)
-		 * a per-cpu variable will do just fine.
-		 */
-		cpu = source_ops(csdev)->cpu_id(csdev);
-		per_cpu(tracer_path, cpu) = path;
-		break;
-	case CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE:
-		stm_path = path;
-		break;
-	default:
-		/* We can't be here */
-		break;
-	}
+	ret = coresight_store_path(path);
+	if (ret)
+		goto err_source;
 
 out:
 	mutex_unlock(&coresight_mutex);
@@ -1129,8 +1139,11 @@ EXPORT_SYMBOL_GPL(coresight_enable);
 
 void coresight_disable(struct coresight_device *csdev)
 {
-	int cpu, ret;
+	int  ret;
 	struct list_head *path = NULL;
+	struct coresight_path *cspath = NULL;
+	struct coresight_path *cspath_next = NULL;
+	struct coresight_device *src_csdev = NULL;
 
 	mutex_lock(&coresight_mutex);
 
@@ -1141,20 +1154,18 @@ void coresight_disable(struct coresight_device *csdev)
 	if (!csdev->enable || !coresight_disable_source(csdev))
 		goto out;
 
-	switch (csdev->subtype.source_subtype) {
-	case CORESIGHT_DEV_SUBTYPE_SOURCE_PROC:
-		cpu = source_ops(csdev)->cpu_id(csdev);
-		path = per_cpu(tracer_path, cpu);
-		per_cpu(tracer_path, cpu) = NULL;
-		break;
-	case CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE:
-		path = stm_path;
-		stm_path = NULL;
-		break;
-	default:
-		/* We can't be here */
-		break;
+	list_for_each_entry_safe(cspath, cspath_next, &cs_active_paths, link) {
+		src_csdev = coresight_get_source(cspath->path);
+		if (!src_csdev)
+			continue;
+		if (src_csdev == csdev) {
+			path = cspath->path;
+			list_del(&cspath->link);
+			kfree(cspath);
+		}
 	}
+	if (path == NULL)
+		goto out;
 
 	coresight_disable_path(path);
 	coresight_release_path(path);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] coresight: add support to enable more coresight paths
  2021-04-15  9:49 ` Suzuki K Poulose
@ 2021-04-16 11:50   ` taozha
  0 siblings, 0 replies; 4+ messages in thread
From: taozha @ 2021-04-16 11:50 UTC (permalink / raw)
  To: Suzuki K Poulose
  Cc: Mathieu Poirier, Alexander Shishkin, Mike Leach, Leo Yan,
	Greg Kroah-Hartman, coresight, linux-arm-kernel, linux-kernel,
	Tingwei Zhang, Mao Jinlong, Yuanfang Zhang, Satyajit Desai,
	Rama Aparna Mallavarapu, Mulu He, Tao Zhang

On 2021-04-15 17:49, Suzuki K Poulose wrote:
> Hi
> 
> On 15/04/2021 10:33, Tao Zhang wrote:
>> Current coresight implementation only supports enabling source
>> ETMs or STM. This patch adds support to enable more kinds of
>> coresight source to sink paths. We build a path from source to
>> sink when any source is enabled and store it in a list. When the
>> source is disabled, we fetch the corresponding path from the list
>> and decrement the refcount on each device in the path. The device
>> is disabled if the refcount reaches zero. Don't store path to
>> coresight data structure of source to avoid unnecessary change to
>> ABI.
>> Since some targets may have coresight sources other than STM and
>> ETMs, we need to add this change to support these coresight
>> devices.
> 
> While I am not against the patch, I would like to see why this change 
> is
> needed. Which compnents are we talking about , other than STM / ETM ?
> Where is the "device" support code ? Without a legitimate user, we
> cannot add this change in.
> 
> Some other comments below.
> 
We will upload coresight source device driver code in addition to 
STM/ETM in the future. This patch is only our first step. e.g, this 
patch can support to build a path from TPDM(a kind of coresight source 
device on Qualcomm target) to sink. At the same time, this patch also 
can support coresight source devices other than STM/ETM on the other 
targets.
>> 
>> Signed-off-by: Satyajit Desai <sadesai@codeaurora.org>
>> Signed-off-by: Rama Aparna Mallavarapu <aparnam@codeaurora.org>
>> Signed-off-by: Mulu He <muluhe@codeaurora.org>
>> Signed-off-by: Tingwei Zhang <tingwei@codeaurora.org>
>> Signed-off-by: Tao Zhang <taozha@codeaurora.org>
>> ---
>>   drivers/hwtracing/coresight/coresight-core.c | 101 
>> +++++++++++++++------------
>>   1 file changed, 56 insertions(+), 45 deletions(-)
>> 
>> diff --git a/drivers/hwtracing/coresight/coresight-core.c 
>> b/drivers/hwtracing/coresight/coresight-core.c
>> index 4ba801d..7dfadb6 100644
>> --- a/drivers/hwtracing/coresight/coresight-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>> @@ -35,18 +35,16 @@ struct coresight_node {
>>   };
>>     /*
>> - * When operating Coresight drivers from the sysFS interface, only a 
>> single
>> - * path can exist from a tracer (associated to a CPU) to a sink.
>> + * struct coresight_path - path from source to sink
>> + * @path:	Address of path list.
>> + * @link:	hook to the list.
>>    */
>> -static DEFINE_PER_CPU(struct list_head *, tracer_path);
>> +struct coresight_path {
>> +	struct list_head *path;
>> +	struct list_head link;
>> +};
>>   -/*
>> - * As of this writing only a single STM can be found in CS 
>> topologies.  Since
>> - * there is no way to know if we'll ever see more and what kind of
>> - * configuration they will enact, for the time being only define a 
>> single path
>> - * for STM.
>> - */
>> -static struct list_head *stm_path;
>> +static LIST_HEAD(cs_active_paths);
>>     /*
>>    * When losing synchronisation a new barrier packet needs to be 
>> inserted at the
>> @@ -326,7 +324,7 @@ static void coresight_disable_sink(struct 
>> coresight_device *csdev)
>>   	if (ret)
>>   		return;
>>   	coresight_control_assoc_ectdev(csdev, false);
>> -	csdev->enable = false;
>> +	csdev->activated = false;
>>   }
>>     static int coresight_enable_link(struct coresight_device *csdev,
>> @@ -562,6 +560,20 @@ int coresight_enable_path(struct list_head *path, 
>> u32 mode, void *sink_data)
>>   	goto out;
>>   }
>>   +static struct coresight_device *coresight_get_source(struct 
>> list_head *path)
>> +{
>> +	struct coresight_device *csdev;
>> +
>> +	if (!path)
>> +		return NULL;
>> +
>> +	csdev = list_first_entry(path, struct coresight_node, link)->csdev;
>> +	if (csdev->type != CORESIGHT_DEV_TYPE_SOURCE)
>> +		return NULL;
>> +
>> +	return csdev;
>> +}
>> +
>>   struct coresight_device *coresight_get_sink(struct list_head *path)
>>   {
>>   	struct coresight_device *csdev;
>> @@ -1047,9 +1059,23 @@ static int coresight_validate_source(struct 
>> coresight_device *csdev,
>>   	return 0;
>>   }
>>   +static int coresight_store_path(struct list_head *path)
>> +{
>> +	struct coresight_path *node;
>> +
>> +	node = kzalloc(sizeof(struct coresight_path), GFP_KERNEL);
> 
> Have you run this with all the "DEBUG" whistles turned on ? This could
> be problematic when called from a context where you are not allowed to
> sleep. e.g, perf enable call back.
> 
Only coresight_enable function will call coresight_store_path, and seems 
like coresight_enable function should be allowed to sleep since it is 
protected by Mutex. And I find in function coresight_build_path also 
will call kzalloc. Do you have any other concerns about calling kzalloc 
here?
>> +	if (!node)
>> +		return -ENOMEM;
>> +
>> +	node->path = path;
>> +	list_add(&node->link, &cs_active_paths);
>> +
>> +	return 0;
>> +}
>> +
>>   int coresight_enable(struct coresight_device *csdev)
>>   {
>> -	int cpu, ret = 0;
>> +	int ret = 0;
>>   	struct coresight_device *sink;
>>   	struct list_head *path;
>>   	enum coresight_dev_subtype_source subtype;
>> @@ -1094,25 +1120,9 @@ int coresight_enable(struct coresight_device 
>> *csdev)
>>   	if (ret)
>>   		goto err_source;
>>   -	switch (subtype) {
>> -	case CORESIGHT_DEV_SUBTYPE_SOURCE_PROC:
>> -		/*
>> -		 * When working from sysFS it is important to keep track
>> -		 * of the paths that were created so that they can be
>> -		 * undone in 'coresight_disable()'.  Since there can only
>> -		 * be a single session per tracer (when working from sysFS)
>> -		 * a per-cpu variable will do just fine.
>> -		 */
>> -		cpu = source_ops(csdev)->cpu_id(csdev);
>> -		per_cpu(tracer_path, cpu) = path;
>> -		break;
>> -	case CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE:
>> -		stm_path = path;
>> -		break;
>> -	default:
>> -		/* We can't be here */
>> -		break;
>> -	}
>> +	ret = coresight_store_path(path);
>> +	if (ret)
>> +		goto err_source;
>>     out:
>>   	mutex_unlock(&coresight_mutex);
>> @@ -1129,8 +1139,11 @@ EXPORT_SYMBOL_GPL(coresight_enable);
>>     void coresight_disable(struct coresight_device *csdev)
>>   {
>> -	int cpu, ret;
>> +	int  ret;
>>   	struct list_head *path = NULL;
>> +	struct coresight_path *cspath = NULL;
>> +	struct coresight_path *cspath_next = NULL;
>> +	struct coresight_device *src_csdev = NULL;
>>     	mutex_lock(&coresight_mutex);
>>   @@ -1141,20 +1154,18 @@ void coresight_disable(struct 
>> coresight_device *csdev)
>>   	if (!csdev->enable || !coresight_disable_source(csdev))
>>   		goto out;
>>   -	switch (csdev->subtype.source_subtype) {
>> -	case CORESIGHT_DEV_SUBTYPE_SOURCE_PROC:
>> -		cpu = source_ops(csdev)->cpu_id(csdev);
>> -		path = per_cpu(tracer_path, cpu);
>> -		per_cpu(tracer_path, cpu) = NULL;
>> -		break;
>> -	case CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE:
>> -		path = stm_path;
>> -		stm_path = NULL;
>> -		break;
>> -	default:
>> -		/* We can't be here */
>> -		break;
> 
> --> Cut here <---
> 
>> +	list_for_each_entry_safe(cspath, cspath_next, &cs_active_paths, 
>> link) {
>> +		src_csdev = coresight_get_source(cspath->path);
>> +		if (!src_csdev)
>> +			continue;
>> +		if (src_csdev == csdev) {
>> +			path = cspath->path;
>> +			list_del(&cspath->link);
>> +			kfree(cspath);
>> +		}
>>   	}
> 
> Why not add the above to 
> coresight_{retrieve/remove/}_active_path(csdev) ?
> 
> 
> 
> Suzuki
I didn't find coresight_{retrieve/remove/}_active_path(csdev) from 
coresight next branch. Can you help point out the exact function name 
that you think this part of code should be added to?

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] coresight: add support to enable more coresight paths
  2021-04-15  9:33 Tao Zhang
@ 2021-04-15  9:49 ` Suzuki K Poulose
  2021-04-16 11:50   ` taozha
  0 siblings, 1 reply; 4+ messages in thread
From: Suzuki K Poulose @ 2021-04-15  9:49 UTC (permalink / raw)
  To: Tao Zhang, Mathieu Poirier, Alexander Shishkin
  Cc: Mike Leach, Leo Yan, Greg Kroah-Hartman, coresight,
	linux-arm-kernel, linux-kernel, Tingwei Zhang, Mao Jinlong,
	Yuanfang Zhang, Satyajit Desai, Rama Aparna Mallavarapu, Mulu He

Hi

On 15/04/2021 10:33, Tao Zhang wrote:
> Current coresight implementation only supports enabling source
> ETMs or STM. This patch adds support to enable more kinds of
> coresight source to sink paths. We build a path from source to
> sink when any source is enabled and store it in a list. When the
> source is disabled, we fetch the corresponding path from the list
> and decrement the refcount on each device in the path. The device
> is disabled if the refcount reaches zero. Don't store path to
> coresight data structure of source to avoid unnecessary change to
> ABI.
> Since some targets may have coresight sources other than STM and
> ETMs, we need to add this change to support these coresight
> devices.

While I am not against the patch, I would like to see why this change is
needed. Which compnents are we talking about , other than STM / ETM ?
Where is the "device" support code ? Without a legitimate user, we 
cannot add this change in.

Some other comments below.

> 
> Signed-off-by: Satyajit Desai <sadesai@codeaurora.org>
> Signed-off-by: Rama Aparna Mallavarapu <aparnam@codeaurora.org>
> Signed-off-by: Mulu He <muluhe@codeaurora.org>
> Signed-off-by: Tingwei Zhang <tingwei@codeaurora.org>
> Signed-off-by: Tao Zhang <taozha@codeaurora.org>
> ---
>   drivers/hwtracing/coresight/coresight-core.c | 101 +++++++++++++++------------
>   1 file changed, 56 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 4ba801d..7dfadb6 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -35,18 +35,16 @@ struct coresight_node {
>   };
>   
>   /*
> - * When operating Coresight drivers from the sysFS interface, only a single
> - * path can exist from a tracer (associated to a CPU) to a sink.
> + * struct coresight_path - path from source to sink
> + * @path:	Address of path list.
> + * @link:	hook to the list.
>    */
> -static DEFINE_PER_CPU(struct list_head *, tracer_path);
> +struct coresight_path {
> +	struct list_head *path;
> +	struct list_head link;
> +};
>   
> -/*
> - * As of this writing only a single STM can be found in CS topologies.  Since
> - * there is no way to know if we'll ever see more and what kind of
> - * configuration they will enact, for the time being only define a single path
> - * for STM.
> - */
> -static struct list_head *stm_path;
> +static LIST_HEAD(cs_active_paths);
>   
>   /*
>    * When losing synchronisation a new barrier packet needs to be inserted at the
> @@ -326,7 +324,7 @@ static void coresight_disable_sink(struct coresight_device *csdev)
>   	if (ret)
>   		return;
>   	coresight_control_assoc_ectdev(csdev, false);
> -	csdev->enable = false;
> +	csdev->activated = false;
>   }
>   
>   static int coresight_enable_link(struct coresight_device *csdev,
> @@ -562,6 +560,20 @@ int coresight_enable_path(struct list_head *path, u32 mode, void *sink_data)
>   	goto out;
>   }
>   
> +static struct coresight_device *coresight_get_source(struct list_head *path)
> +{
> +	struct coresight_device *csdev;
> +
> +	if (!path)
> +		return NULL;
> +
> +	csdev = list_first_entry(path, struct coresight_node, link)->csdev;
> +	if (csdev->type != CORESIGHT_DEV_TYPE_SOURCE)
> +		return NULL;
> +
> +	return csdev;
> +}
> +
>   struct coresight_device *coresight_get_sink(struct list_head *path)
>   {
>   	struct coresight_device *csdev;
> @@ -1047,9 +1059,23 @@ static int coresight_validate_source(struct coresight_device *csdev,
>   	return 0;
>   }
>   
> +static int coresight_store_path(struct list_head *path)
> +{
> +	struct coresight_path *node;
> +
> +	node = kzalloc(sizeof(struct coresight_path), GFP_KERNEL);

Have you run this with all the "DEBUG" whistles turned on ? This could 
be problematic when called from a context where you are not allowed to 
sleep. e.g, perf enable call back.

> +	if (!node)
> +		return -ENOMEM;
> +
> +	node->path = path;
> +	list_add(&node->link, &cs_active_paths);
> +
> +	return 0;
> +}
> +
>   int coresight_enable(struct coresight_device *csdev)
>   {
> -	int cpu, ret = 0;
> +	int ret = 0;
>   	struct coresight_device *sink;
>   	struct list_head *path;
>   	enum coresight_dev_subtype_source subtype;
> @@ -1094,25 +1120,9 @@ int coresight_enable(struct coresight_device *csdev)
>   	if (ret)
>   		goto err_source;
>   
> -	switch (subtype) {
> -	case CORESIGHT_DEV_SUBTYPE_SOURCE_PROC:
> -		/*
> -		 * When working from sysFS it is important to keep track
> -		 * of the paths that were created so that they can be
> -		 * undone in 'coresight_disable()'.  Since there can only
> -		 * be a single session per tracer (when working from sysFS)
> -		 * a per-cpu variable will do just fine.
> -		 */
> -		cpu = source_ops(csdev)->cpu_id(csdev);
> -		per_cpu(tracer_path, cpu) = path;
> -		break;
> -	case CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE:
> -		stm_path = path;
> -		break;
> -	default:
> -		/* We can't be here */
> -		break;
> -	}
> +	ret = coresight_store_path(path);
> +	if (ret)
> +		goto err_source;
>   
>   out:
>   	mutex_unlock(&coresight_mutex);
> @@ -1129,8 +1139,11 @@ EXPORT_SYMBOL_GPL(coresight_enable);
>   
>   void coresight_disable(struct coresight_device *csdev)
>   {
> -	int cpu, ret;
> +	int  ret;
>   	struct list_head *path = NULL;
> +	struct coresight_path *cspath = NULL;
> +	struct coresight_path *cspath_next = NULL;
> +	struct coresight_device *src_csdev = NULL;
>   
>   	mutex_lock(&coresight_mutex);
>   
> @@ -1141,20 +1154,18 @@ void coresight_disable(struct coresight_device *csdev)
>   	if (!csdev->enable || !coresight_disable_source(csdev))
>   		goto out;
>   
> -	switch (csdev->subtype.source_subtype) {
> -	case CORESIGHT_DEV_SUBTYPE_SOURCE_PROC:
> -		cpu = source_ops(csdev)->cpu_id(csdev);
> -		path = per_cpu(tracer_path, cpu);
> -		per_cpu(tracer_path, cpu) = NULL;
> -		break;
> -	case CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE:
> -		path = stm_path;
> -		stm_path = NULL;
> -		break;
> -	default:
> -		/* We can't be here */
> -		break;

--> Cut here <---

> +	list_for_each_entry_safe(cspath, cspath_next, &cs_active_paths, link) {
> +		src_csdev = coresight_get_source(cspath->path);
> +		if (!src_csdev)
> +			continue;
> +		if (src_csdev == csdev) {
> +			path = cspath->path;
> +			list_del(&cspath->link);
> +			kfree(cspath);
> +		}
>   	}

Why not add the above to coresight_{retrieve/remove/}_active_path(csdev) ?



Suzuki

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] coresight: add support to enable more coresight paths
@ 2021-04-15  9:33 Tao Zhang
  2021-04-15  9:49 ` Suzuki K Poulose
  0 siblings, 1 reply; 4+ messages in thread
From: Tao Zhang @ 2021-04-15  9:33 UTC (permalink / raw)
  To: Mathieu Poirier, Suzuki K Poulose, Alexander Shishkin
  Cc: Tao Zhang, Mike Leach, Leo Yan, Greg Kroah-Hartman, coresight,
	linux-arm-kernel, linux-kernel, Tingwei Zhang, Mao Jinlong,
	Yuanfang Zhang, Satyajit Desai, Rama Aparna Mallavarapu, Mulu He

Current coresight implementation only supports enabling source
ETMs or STM. This patch adds support to enable more kinds of
coresight source to sink paths. We build a path from source to
sink when any source is enabled and store it in a list. When the
source is disabled, we fetch the corresponding path from the list
and decrement the refcount on each device in the path. The device
is disabled if the refcount reaches zero. Don't store path to
coresight data structure of source to avoid unnecessary change to
ABI.
Since some targets may have coresight sources other than STM and
ETMs, we need to add this change to support these coresight
devices.

Signed-off-by: Satyajit Desai <sadesai@codeaurora.org>
Signed-off-by: Rama Aparna Mallavarapu <aparnam@codeaurora.org>
Signed-off-by: Mulu He <muluhe@codeaurora.org>
Signed-off-by: Tingwei Zhang <tingwei@codeaurora.org>
Signed-off-by: Tao Zhang <taozha@codeaurora.org>
---
 drivers/hwtracing/coresight/coresight-core.c | 101 +++++++++++++++------------
 1 file changed, 56 insertions(+), 45 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 4ba801d..7dfadb6 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -35,18 +35,16 @@ struct coresight_node {
 };
 
 /*
- * When operating Coresight drivers from the sysFS interface, only a single
- * path can exist from a tracer (associated to a CPU) to a sink.
+ * struct coresight_path - path from source to sink
+ * @path:	Address of path list.
+ * @link:	hook to the list.
  */
-static DEFINE_PER_CPU(struct list_head *, tracer_path);
+struct coresight_path {
+	struct list_head *path;
+	struct list_head link;
+};
 
-/*
- * As of this writing only a single STM can be found in CS topologies.  Since
- * there is no way to know if we'll ever see more and what kind of
- * configuration they will enact, for the time being only define a single path
- * for STM.
- */
-static struct list_head *stm_path;
+static LIST_HEAD(cs_active_paths);
 
 /*
  * When losing synchronisation a new barrier packet needs to be inserted at the
@@ -326,7 +324,7 @@ static void coresight_disable_sink(struct coresight_device *csdev)
 	if (ret)
 		return;
 	coresight_control_assoc_ectdev(csdev, false);
-	csdev->enable = false;
+	csdev->activated = false;
 }
 
 static int coresight_enable_link(struct coresight_device *csdev,
@@ -562,6 +560,20 @@ int coresight_enable_path(struct list_head *path, u32 mode, void *sink_data)
 	goto out;
 }
 
+static struct coresight_device *coresight_get_source(struct list_head *path)
+{
+	struct coresight_device *csdev;
+
+	if (!path)
+		return NULL;
+
+	csdev = list_first_entry(path, struct coresight_node, link)->csdev;
+	if (csdev->type != CORESIGHT_DEV_TYPE_SOURCE)
+		return NULL;
+
+	return csdev;
+}
+
 struct coresight_device *coresight_get_sink(struct list_head *path)
 {
 	struct coresight_device *csdev;
@@ -1047,9 +1059,23 @@ static int coresight_validate_source(struct coresight_device *csdev,
 	return 0;
 }
 
+static int coresight_store_path(struct list_head *path)
+{
+	struct coresight_path *node;
+
+	node = kzalloc(sizeof(struct coresight_path), GFP_KERNEL);
+	if (!node)
+		return -ENOMEM;
+
+	node->path = path;
+	list_add(&node->link, &cs_active_paths);
+
+	return 0;
+}
+
 int coresight_enable(struct coresight_device *csdev)
 {
-	int cpu, ret = 0;
+	int ret = 0;
 	struct coresight_device *sink;
 	struct list_head *path;
 	enum coresight_dev_subtype_source subtype;
@@ -1094,25 +1120,9 @@ int coresight_enable(struct coresight_device *csdev)
 	if (ret)
 		goto err_source;
 
-	switch (subtype) {
-	case CORESIGHT_DEV_SUBTYPE_SOURCE_PROC:
-		/*
-		 * When working from sysFS it is important to keep track
-		 * of the paths that were created so that they can be
-		 * undone in 'coresight_disable()'.  Since there can only
-		 * be a single session per tracer (when working from sysFS)
-		 * a per-cpu variable will do just fine.
-		 */
-		cpu = source_ops(csdev)->cpu_id(csdev);
-		per_cpu(tracer_path, cpu) = path;
-		break;
-	case CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE:
-		stm_path = path;
-		break;
-	default:
-		/* We can't be here */
-		break;
-	}
+	ret = coresight_store_path(path);
+	if (ret)
+		goto err_source;
 
 out:
 	mutex_unlock(&coresight_mutex);
@@ -1129,8 +1139,11 @@ EXPORT_SYMBOL_GPL(coresight_enable);
 
 void coresight_disable(struct coresight_device *csdev)
 {
-	int cpu, ret;
+	int  ret;
 	struct list_head *path = NULL;
+	struct coresight_path *cspath = NULL;
+	struct coresight_path *cspath_next = NULL;
+	struct coresight_device *src_csdev = NULL;
 
 	mutex_lock(&coresight_mutex);
 
@@ -1141,20 +1154,18 @@ void coresight_disable(struct coresight_device *csdev)
 	if (!csdev->enable || !coresight_disable_source(csdev))
 		goto out;
 
-	switch (csdev->subtype.source_subtype) {
-	case CORESIGHT_DEV_SUBTYPE_SOURCE_PROC:
-		cpu = source_ops(csdev)->cpu_id(csdev);
-		path = per_cpu(tracer_path, cpu);
-		per_cpu(tracer_path, cpu) = NULL;
-		break;
-	case CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE:
-		path = stm_path;
-		stm_path = NULL;
-		break;
-	default:
-		/* We can't be here */
-		break;
+	list_for_each_entry_safe(cspath, cspath_next, &cs_active_paths, link) {
+		src_csdev = coresight_get_source(cspath->path);
+		if (!src_csdev)
+			continue;
+		if (src_csdev == csdev) {
+			path = cspath->path;
+			list_del(&cspath->link);
+			kfree(cspath);
+		}
 	}
+	if (path == NULL)
+		goto out;
 
 	coresight_disable_path(path);
 	coresight_release_path(path);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-04-16 11:50 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-15  3:08 [PATCH] coresight: add support to enable more coresight paths Tao Zhang
2021-04-15  9:33 Tao Zhang
2021-04-15  9:49 ` Suzuki K Poulose
2021-04-16 11:50   ` taozha

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).