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

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..@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..@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@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;