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

[Leo Yan <leo.yan@arm.com>]

On Mon, Mar 23, 2026 at 10:01:52AM +0000, Suzuki K Poulose wrote:

[...]

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

Will do.

> > +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 ?

My purpose is that coresight_path_nodes_in_order() checks if *from and
*to are sequential on the link.  This is a internal function to detect
any buggy code written in callers.

An assumption is all callers must obtain nodes from the path before
passing them in coresight_path_nodes_in_order().  Therefore, the check
is simplified to only verify that *from appears before *to.

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

Sure, I will refine the walk code like this way and add a comment.

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

If we allow @to to be NULL, we may also need to allow @from to be NULL
(e.g. in the enable path case).

Instead, callers can fetch the first and last nodes using helpers and
explicitly pass them to the enable and disable functions.  Although this
introduces some duplicate code in callers, it avoids complexity in the
enable and disable functions.

> > +{
> > +	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 ?

Will do for this and below suggestions.

Thanks,
Leo