[PATCH v7 14/20] coresight: Control path with range

[Suzuki K Poulose]

On 20/03/2026 17:42, Leo Yan wrote:
> Add parameters @from and @to for path enabling and disabling, allowing
> finer-grained control of a path.
> 
> Disabling a path uses the range (@from.. at to].  @from is exclusive to
> handle the case where the source is passed as the start node: the first
> node is skipped, as the source has its own disable function.
> 
> Enabling a path uses the range [@from.. at to], where both @from and @to
> are inclusive.  The enable path still needs to enable the source's
> helpers, though the source has its own enable function.
> 
> Introduce coresight_path_nodes_in_order() to validate the given range if
> is ordered.  Update callers accordingly.
> 
> Signed-off-by: Leo Yan <leo.yan at arm.com>
> ---
>   drivers/hwtracing/coresight/coresight-core.c | 96 ++++++++++++++++++++++++----
>   1 file changed, 85 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 81bf4637f788f99844e839fd9dad8802ee78eb60..874bd87da2270fdbe6b32c2fa29292d7a26b8dc7 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -492,20 +492,62 @@ int coresight_resume_source(struct coresight_device *csdev)
>   }
>   EXPORT_SYMBOL_GPL(coresight_resume_source);
>   
> +static struct coresight_node *
> +coresight_path_first_node(struct coresight_path *path)
> +{
> +	return list_first_entry(&path->path_list, struct coresight_node, link);
> +}
> +
> +static struct coresight_node *
> +coresight_path_last_node(struct coresight_path *path)
> +{
> +	return list_last_entry(&path->path_list, struct coresight_node, link);
> +}
> +

nit: This could be used for in coresight_get_sink().

> +static bool coresight_path_nodes_in_order(struct coresight_path *path,
> +					  struct coresight_node *from,
> +					  struct coresight_node *to)

I am not very clear what this is supposed to do and what it is doing ?
Are we verifying that the nodes @from and @to are in order ? As, in the
the link is from @from to @to ? In that case what you do may not be 
sufficient ?

Rather, we should walk the path from @from and return if we find @to on 
the walk. Otherwise return false ? In any case,this need a comment on
the top.

Also I don't think this check is really needed. If we don't find the
@to, simply disable the entire path.


> +{
> +	struct coresight_node *nd;
> +
> +	/* Callers must fetch nodes from the path */
> +	if (WARN_ON_ONCE(!from || !to))
> +		return false;
> +
> +	list_for_each_entry(nd, &path->path_list, link) {
> +		if (nd == from)
> +			return true;
> +		if (nd == to)
> +			return false;
> +	}
> +
> +	return false;
> +}
> +
>   /*
> - * coresight_disable_path_from : Disable components in the given path beyond
> - * @nd in the list. If @nd is NULL, all the components, except the SOURCE are
> - * disabled.
> + * coresight_disable_path_from_to : Disable components in the given @path
> + * between @from and @to.
> + *
> + * The range excludes @from but includes @to.  @from is exclusive to handle the
> + * case where it is the source (the first node in the path), as the source has
> + * its own disable function.

Why not break that "hack" and always include @from and let the caller 
now use this helper to disable everything after the @source ?

>    */
> -static void coresight_disable_path_from(struct coresight_path *path,
> -					struct coresight_node *nd)
> +static void coresight_disable_path_from_to(struct coresight_path *path,
> +					   struct coresight_node *from,
> +					   struct coresight_node *to)
>   {
>   	u32 type;
>   	struct coresight_device *csdev, *parent, *child;
> +	struct coresight_node *nd;
>   
> -	if (!nd)
> -		nd = list_first_entry(&path->path_list, struct coresight_node, link);
> +	if (!coresight_path_nodes_in_order(path, from, to))
> +		return;
>   
> +	/* @from is exclusive; nothing to do if @from == @to */
> +	if (from == to)
> +		return;

And drop this ^

> +
> +	nd = from;
>   	list_for_each_entry_continue(nd, &path->path_list, link) {
>   		csdev = nd->csdev;
>   		type = csdev->type;
> @@ -545,12 +587,18 @@ static void coresight_disable_path_from(struct coresight_path *path,
>   
>   		/* Disable all helpers adjacent along the path last */
>   		coresight_disable_helpers(csdev, path);
> +
> +		/* Iterate up to and including @to */
> +		if (nd == to)
> +			break;
>   	}
>   }
>   
>   void coresight_disable_path(struct coresight_path *path)
>   {
> -	coresight_disable_path_from(path, NULL);
> +	coresight_disable_path_from_to(path,
> +				       coresight_path_first_node(path),
> +				       coresight_path_last_node(path));
>   }
>   EXPORT_SYMBOL_GPL(coresight_disable_path);
>   
> @@ -574,7 +622,18 @@ static int coresight_enable_helpers(struct coresight_device *csdev,
>   	return 0;
>   }
>   
> -int coresight_enable_path(struct coresight_path *path, enum cs_mode mode)
> +/*
> + * coresight_enable_path_from_to : Enable components in the given @path
> + * between @from and @to with the specified mode.
> + *
> + * The range includes both @from and @to. If @from is the source (the first
> + * node in the path), its helper is enabled here but the source is enabled
> + * in a separate function.

Like above, can we cleanup everything to always include from and to 
requested in the arguments ? That way it is very clear from the code
perspective on what we are doing, rather than hiding the facts
under a comment/code.


Otherwise, looks like a great step towards cleaning this up

Cheers
Suzuki


> + */
> +static int coresight_enable_path_from_to(struct coresight_path *path,
> +					 enum cs_mode mode,
> +					 struct coresight_node *from,
> +					 struct coresight_node *to)
>   {
>   	int ret = 0;
>   	u32 type;
> @@ -582,8 +641,12 @@ int coresight_enable_path(struct coresight_path *path, enum cs_mode mode)
>   	struct coresight_device *csdev, *parent, *child;
>   	struct coresight_device *source;
>   
> +	if (!coresight_path_nodes_in_order(path, from, to))
> +		return -EINVAL;
> +
> +	nd = to;
>   	source = coresight_get_source(path);
> -	list_for_each_entry_reverse(nd, &path->path_list, link) {
> +	list_for_each_entry_from_reverse(nd, &path->path_list, link) {
>   		csdev = nd->csdev;
>   		type = csdev->type;
>   
> @@ -630,6 +693,10 @@ int coresight_enable_path(struct coresight_path *path, enum cs_mode mode)
>   			ret = -EINVAL;
>   			goto err_disable_helpers;
>   		}
> +
> +		/* Iterate down to and including @from */
> +		if (nd == from)
> +			break;
>   	}
>   
>   out:
> @@ -637,10 +704,17 @@ int coresight_enable_path(struct coresight_path *path, enum cs_mode mode)
>   err_disable_helpers:
>   	coresight_disable_helpers(csdev, path);
>   err_disable_path:
> -	coresight_disable_path_from(path, nd);
> +	coresight_disable_path_from_to(path, nd, coresight_path_last_node(path));
>   	goto out;
>   }
>   
> +int coresight_enable_path(struct coresight_path *path, enum cs_mode mode)
> +{
> +	return coresight_enable_path_from_to(path, mode,
> +					     coresight_path_first_node(path),
> +					     coresight_path_last_node(path));
> +}
> +
>   struct coresight_device *coresight_get_sink(struct coresight_path *path)
>   {
>   	struct coresight_device *csdev;
>