Re: [PATCH v11 14/27] coresight: Move source helper disabling to coresight_disable_path()

On 01/05/2026 17:47, Leo Yan wrote:

Move source helper disabling to coresight_disable_path() to pair it with coresight_enable_path().

In coresight_enable_path(), if enabling a node fails, iterate to the next node (which is the last successfully enabled node) and pass it to coresight_disable_path() for rollback. If the failed node is the last node on the path, no device is actually enabled, so bail out directly.

As a result, coresight_disable_source() only controls the source, leaving the associated helpers to be managed as part of path. Update the comment to reflect this change.

Tested-by: Jie Gan jie.gan@oss.qualcomm.com Reviewed-by: Yeoreum Yun yeoreum.yun@arm.com Reviewed-by: James Clark james.clark@linaro.org Tested-by: James Clark james.clark@linaro.org Signed-off-by: Leo Yan leo.yan@arm.com

---

drivers/hwtracing/coresight/coresight-core.c | 38 +++++++++++++--------------- 1 file changed, 17 insertions(+), 21 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index 0c11e7fff1bd5cf504b17241595d83a1d11bcf40..6ab80ae7c389e9031d4ab065d83bb676fd12160b 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -447,19 +447,12 @@ static void coresight_disable_helpers(struct coresight_device *csdev, } /*

• • Helper function to call source_ops(csdev)->disable and also disable the
• • helpers.
• • There is an imbalance between coresight_enable_path() and
• • coresight_disable_path(). Enabling also enables the source's helpers as part
• • of the path, but disabling always skips the first item in the path (which is
• • the source), so sources and their helpers don't get disabled as part of that
• • function and we need the extra step here.

• • coresight_disable_source() only disables the source, but do nothing for
• • the associated helpers, which are controlled as part of the path.
  */ void coresight_disable_source(struct coresight_device *csdev, void *data) { source_ops(csdev)->disable(csdev, data);

• coresight_disable_helpers(csdev, NULL); } EXPORT_SYMBOL_GPL(coresight_disable_source);

@@ -486,9 +479,9 @@ int coresight_resume_source(struct coresight_device *csdev) EXPORT_SYMBOL_GPL(coresight_resume_source); /*

• • 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 : Disable components in the given path starting
• • from @nd in the list. If @nd is NULL, all the components, except the SOURCE
• • are disabled.
  */ static void coresight_disable_path_from(struct coresight_path *path, struct coresight_node *nd)

@@ -499,7 +492,7 @@ static void coresight_disable_path_from(struct coresight_path *path, if (!nd) nd = list_first_entry(&path->path_list, struct coresight_node, link);

• list_for_each_entry_continue(nd, &path->path_list, link) {

• list_for_each_entry_from(nd, &path->path_list, link) { csdev = nd->csdev; type = csdev->type;

@@ -519,12 +512,6 @@ static void coresight_disable_path_from(struct coresight_path *path, coresight_disable_sink(csdev); break; case CORESIGHT_DEV_TYPE_SOURCE:

• 	/*
  

• 	 * We skip the first node in the path assuming that it
  

• 	 * is the source. So we don't expect a source device in
  

• 	 * the middle of a path.
  

• 	 */
  

• 	WARN_ON(1);
  break;
  

  case CORESIGHT_DEV_TYPE_LINK: parent = list_prev_entry(nd, link)->csdev;

--8>--

@@ -580,12 +567,16 @@ int coresight_enable_path(struct coresight_path *path, enum cs_mode mode) { int ret = 0; u32 type;

• struct coresight_node *nd;

• struct coresight_node *nd, *last; struct coresight_device *csdev, *parent, *child; struct coresight_device *source;

source = coresight_get_source(path);

• list_for_each_entry_reverse(nd, &path->path_list, link) {

• last = list_last_entry(&path->path_list, struct coresight_node, link);
• nd = last;
• list_for_each_entry_from_reverse(nd, &path->path_list, link) { csdev = nd->csdev; type = csdev->type;

@@ -639,6 +630,11 @@ int coresight_enable_path(struct coresight_path *path, enum cs_mode mode) err_disable_helpers: coresight_disable_helpers(csdev, path); err_disable_path:

• /* No device is actually enabled */
• if (nd == last)

• goto out;
  

+

This hunk seems to be different from the one described in the commit and should be a different patch if at all we need it ? Moreover, this looks unnecessary. We already do the check to see if we didn't enable any device. See CORESIGHT_DEVICE_TYPE_SINK case.

Suzuki

• nd = list_next_entry(nd, link); coresight_disable_path_from(path, nd); goto out; }