[PATCH v10 00/20] CoreSight: Refactor power management for CoreSight path

[PATCH v10 00/20] CoreSight: Refactor power management for CoreSight path

[Leo Yan]

This series focuses on CoreSight path power management.  The changes can
be divided into four parts for review:

  Patches 01 - 04: Refactor the CPU ID storing in csdev, later patches
                   consume csdev->cpu.  Move CPU lock to sysfs layer so
		   it is safe for later changes.
  Patches 05 - 09: Refactor the CPU idle flow with moving common code into
                   the CoreSight core layer.
  Patches 10 - 15: Refactor path enabling and disabling with range, add
                   path control during CPU idle.
  Patches 16 - 17: Support the sink (TRBE) control during CPU idle.
  Patches 18 - 20: Move CPU hotplug into the core layer, and fix sysfs
                   mode for hotplug.

This series is rebased on the coresight-next branch and has been verified
on Juno-r2 (ETM + ETR) and FVP RevC (ETE + TRBE).  Built successfully
for armv7 (ARCH=arm).

---
Changes in v10:
- Removed redundant checks in ETMv4 PM callbacks (sashiko).
- Added a new const structure etm4_cs_pm_ops (sashiko).
- Used fine-grained spinlock on sysfs_active_config (sashiko).
- Blocked notification after failures in save / restore to avoid lockups.
- Changed Change CPUHP_AP_ARM_CORESIGHT_STARTING to
  CPUHP_AP_ARM_CORESIGHT_ONLINE so that the CPU hotplug callback runs in
  the thread context (sashiko).
- Link to v9: https://lore.kernel.org/r/20260401-arm_coresight_path_power_management_improvement-v9-0-091d73e44072@arm.com

Changes in v9:
- Changed to use per-CPU path pointer with lockless access.
- Removed the change for adding csdev->path, the related refactoring
  will be sent separately.
- Re-orged patches to avoid intermediate breakage (sashiko).
- Link to v8: https://lore.kernel.org/r/20260325-arm_coresight_path_power_management_improvement-v8-0-7b1902e18041@arm.com

Changes in v8:
- Moved the "cpu" field in coresight_device for better pack with new
  patch 01 (Suzuki).
- Added check if not set CPU for per_cpu_source/per_cpu_sink (Suzuki).
- Renamed spinlock name in syscfg (Suzuki).
- Refactored paired enable and disable path with new patches
  10 and 12 (Suzuki).
- Link to v7: https://lore.kernel.org/r/20260320-arm_coresight_path_power_management_improvement-v7-0-327ddd36b58b@arm.com

Changes in v7:
- Added a flag in coresight_desc to indicate CPU bound device (Suzuki).
- Used coresight_mutex to protect per-CPU source pointer (Suzuki).
- Added a spinlock for exclusive access per-CPU source pointer (Suzuki).
- Dropped .pm_is_needed() callback (Suzuki).
- Supported range in path enabling / disabling (Suzuki).
- Gathered test and review tags (Levi / James).
- Link to v6: https://lore.kernel.org/r/20260305-arm_coresight_path_power_management_improvement-v6-0-eff765d211a9@arm.com

Signed-off-by: Leo Yan <leo.yan@arm.com>

---
Leo Yan (19):
      coresight: Extract device init into coresight_init_device()
      coresight: Populate CPU ID into coresight_device
      coresight: Remove .cpu_id() callback from source ops
      coresight: Take hotplug lock in enable_source_store() for Sysfs mode
      coresight: etm4x: Set per-CPU path on local CPU
      coresight: etm3x: Set per-CPU path on local CPU
      coresight: Register CPU PM notifier in core layer
      coresight: etm4x: Hook CPU PM callbacks
      coresight: etm4x: Remove redundant checks in PM save and restore
      coresight: syscfg: Use IRQ-safe spinlock to protect active variables
      coresight: Move source helper disabling to coresight_disable_path()
      coresight: Control path with range
      coresight: Use helpers to fetch first and last nodes
      coresight: Introduce coresight_enable_source() helper
      coresight: Control path during CPU idle
      coresight: Add PM callbacks for sink device
      coresight: sysfs: Increment refcount only for system tracers
      coresight: Move CPU hotplug callbacks to core layer
      coresight: sysfs: Validate CPU online status for per-CPU sources

Yabin Cui (1):
      coresight: trbe: Save and restore state across CPU low power state

 drivers/hwtracing/coresight/coresight-catu.c       |   2 +-
 drivers/hwtracing/coresight/coresight-core.c       | 419 ++++++++++++++++++---
 drivers/hwtracing/coresight/coresight-cti-core.c   |   9 +-
 drivers/hwtracing/coresight/coresight-etm-perf.c   |   4 +-
 drivers/hwtracing/coresight/coresight-etm3x-core.c |  73 +---
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 168 ++-------
 drivers/hwtracing/coresight/coresight-priv.h       |   4 +
 drivers/hwtracing/coresight/coresight-syscfg.c     |  35 +-
 drivers/hwtracing/coresight/coresight-syscfg.h     |   2 +
 drivers/hwtracing/coresight/coresight-sysfs.c      |  50 ++-
 drivers/hwtracing/coresight/coresight-trbe.c       |  61 ++-
 include/linux/coresight.h                          |  13 +-
 include/linux/cpuhotplug.h                         |   2 +-
 13 files changed, 566 insertions(+), 276 deletions(-)
---
base-commit: ec687ba84000d7d50cf243558041f6729d1d8119
change-id: 20251104-arm_coresight_path_power_management_improvement-dab4966f8280

Best regards,
-- 
Leo Yan <leo.yan@arm.com>

[PATCH v10 01/20] coresight: Extract device init into coresight_init_device()

[Leo Yan]

This commit extracts the allocation and initialization of the coresight
device structure into a separate function to make future extensions
easier.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-core.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 46f247f73cf64a97b9353b84ba5b76b991676f5f..5452de9367d450de399a2107016c3fddb894fc82 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -1322,20 +1322,16 @@ void coresight_release_platform_data(struct device *dev,
 	devm_kfree(dev, pdata);
 }
 
-struct coresight_device *coresight_register(struct coresight_desc *desc)
+static struct coresight_device *
+coresight_init_device(struct coresight_desc *desc)
 {
-	int ret;
 	struct coresight_device *csdev;
-	bool registered = false;
 
 	csdev = kzalloc_obj(*csdev);
-	if (!csdev) {
-		ret = -ENOMEM;
-		goto err_out;
-	}
+	if (!csdev)
+		return ERR_PTR(-ENOMEM);
 
 	csdev->pdata = desc->pdata;
-
 	csdev->type = desc->type;
 	csdev->subtype = desc->subtype;
 	csdev->ops = desc->ops;
@@ -1348,6 +1344,21 @@ struct coresight_device *coresight_register(struct coresight_desc *desc)
 	csdev->dev.release = coresight_device_release;
 	csdev->dev.bus = &coresight_bustype;
 
+	return csdev;
+}
+
+struct coresight_device *coresight_register(struct coresight_desc *desc)
+{
+	int ret;
+	struct coresight_device *csdev;
+	bool registered = false;
+
+	csdev = coresight_init_device(desc);
+	if (IS_ERR(csdev)) {
+		ret = PTR_ERR(csdev);
+		goto err_out;
+	}
+
 	if (csdev->type == CORESIGHT_DEV_TYPE_SINK ||
 	    csdev->type == CORESIGHT_DEV_TYPE_LINKSINK) {
 		raw_spin_lock_init(&csdev->perf_sink_id_map.lock);

-- 
2.34.1

[PATCH v10 02/20] coresight: Populate CPU ID into coresight_device

[Leo Yan]

Add a new flag CORESIGHT_DESC_CPU_BOUND to indicate components that
are CPU bound.  Populate CPU ID into the coresight_device structure;
otherwise, set CPU ID to -1 for non CPU bound devices.

Use the {0} initializer to clear coresight_desc structures to avoid
uninitialized values.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-catu.c       |  2 +-
 drivers/hwtracing/coresight/coresight-core.c       | 13 +++++++++++++
 drivers/hwtracing/coresight/coresight-cti-core.c   |  9 ++++++---
 drivers/hwtracing/coresight/coresight-etm3x-core.c |  2 ++
 drivers/hwtracing/coresight/coresight-etm4x-core.c |  2 ++
 drivers/hwtracing/coresight/coresight-trbe.c       |  2 ++
 include/linux/coresight.h                          |  8 ++++++++
 7 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c
index ce71dcddfca2558eddd625de58a709b151f2e07e..43abe13995cf3c96e70dcf97856872d70f71345a 100644
--- a/drivers/hwtracing/coresight/coresight-catu.c
+++ b/drivers/hwtracing/coresight/coresight-catu.c
@@ -514,7 +514,7 @@ static int __catu_probe(struct device *dev, struct resource *res)
 	int ret = 0;
 	u32 dma_mask;
 	struct catu_drvdata *drvdata;
-	struct coresight_desc catu_desc;
+	struct coresight_desc catu_desc = { 0 };
 	struct coresight_platform_data *pdata = NULL;
 	void __iomem *base;
 
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 5452de9367d450de399a2107016c3fddb894fc82..89911fc27128b2bb78c8ba704f2af5a5d4efe83c 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -1338,6 +1338,19 @@ coresight_init_device(struct coresight_desc *desc)
 	csdev->access = desc->access;
 	csdev->orphan = true;
 
+	if (desc->flags & CORESIGHT_DESC_CPU_BOUND) {
+		csdev->cpu = desc->cpu;
+	} else {
+		/* A per-CPU source or sink must set CPU_BOUND flag */
+		if (coresight_is_percpu_source(csdev) ||
+		    coresight_is_percpu_sink(csdev)) {
+			kfree(csdev);
+			return ERR_PTR(-EINVAL);
+		}
+
+		csdev->cpu = -1;
+	}
+
 	csdev->dev.type = &coresight_dev_type[desc->type];
 	csdev->dev.groups = desc->groups;
 	csdev->dev.parent = desc->dev;
diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c
index 2f4c9362709a90b12a1aeb5016905b7d4474b912..b2c9a4db13b4e5554bca565c17ed299fdfdb30ff 100644
--- a/drivers/hwtracing/coresight/coresight-cti-core.c
+++ b/drivers/hwtracing/coresight/coresight-cti-core.c
@@ -659,7 +659,7 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id)
 	void __iomem *base;
 	struct device *dev = &adev->dev;
 	struct cti_drvdata *drvdata = NULL;
-	struct coresight_desc cti_desc;
+	struct coresight_desc cti_desc = { 0 };
 	struct coresight_platform_data *pdata = NULL;
 	struct resource *res = &adev->res;
 
@@ -702,11 +702,14 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id)
 	 * eCPU ID. System CTIs will have the name cti_sys<I> where I is an
 	 * index allocated by order of discovery.
 	 */
-	if (drvdata->ctidev.cpu >= 0)
+	if (drvdata->ctidev.cpu >= 0) {
+		cti_desc.cpu = drvdata->ctidev.cpu;
+		cti_desc.flags = CORESIGHT_DESC_CPU_BOUND;
 		cti_desc.name = devm_kasprintf(dev, GFP_KERNEL, "cti_cpu%d",
 					       drvdata->ctidev.cpu);
-	else
+	} else {
 		cti_desc.name = coresight_alloc_device_name("cti_sys", dev);
+	}
 	if (!cti_desc.name)
 		return -ENOMEM;
 
diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c
index a547a6d2e0bde84748f753e5529d316c4f5e82e2..eb665db1a37d9970f7f55395c0aa23b98a7f3118 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c
@@ -891,6 +891,8 @@ static int etm_probe(struct amba_device *adev, const struct amba_id *id)
 	desc.pdata = pdata;
 	desc.dev = dev;
 	desc.groups = coresight_etm_groups;
+	desc.cpu = drvdata->cpu;
+	desc.flags = CORESIGHT_DESC_CPU_BOUND;
 	drvdata->csdev = coresight_register(&desc);
 	if (IS_ERR(drvdata->csdev))
 		return PTR_ERR(drvdata->csdev);
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index d565a73f0042e3e0b21fcf9cb94009cc25834d3d..b1e0254a534027d7ede8591e56be28745d0b9974 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -2260,6 +2260,8 @@ static int etm4_add_coresight_dev(struct etm4_init_arg *init_arg)
 	desc.pdata = pdata;
 	desc.dev = dev;
 	desc.groups = coresight_etmv4_groups;
+	desc.cpu = drvdata->cpu;
+	desc.flags = CORESIGHT_DESC_CPU_BOUND;
 	drvdata->csdev = coresight_register(&desc);
 	if (IS_ERR(drvdata->csdev))
 		return PTR_ERR(drvdata->csdev);
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
index 1511f8eb95afb5b4610b8fbdacc8b174b6b08530..14e35b9660d76e47619cc6026b94929b3bb3e02b 100644
--- a/drivers/hwtracing/coresight/coresight-trbe.c
+++ b/drivers/hwtracing/coresight/coresight-trbe.c
@@ -1289,6 +1289,8 @@ static void arm_trbe_register_coresight_cpu(struct trbe_drvdata *drvdata, int cp
 	desc.ops = &arm_trbe_cs_ops;
 	desc.groups = arm_trbe_groups;
 	desc.dev = dev;
+	desc.cpu = cpu;
+	desc.flags = CORESIGHT_DESC_CPU_BOUND;
 	trbe_csdev = coresight_register(&desc);
 	if (IS_ERR(trbe_csdev))
 		goto cpu_clear;
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index 2131febebee93d609df1aea8534a10898b600be2..687190ca11ddeaa83193caa3903a480bac3060d1 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -141,6 +141,8 @@ struct csdev_access {
 		.base		= (_addr),	\
 	})
 
+#define CORESIGHT_DESC_CPU_BOUND	BIT(0)
+
 /**
  * struct coresight_desc - description of a component required from drivers
  * @type:	as defined by @coresight_dev_type.
@@ -153,6 +155,8 @@ struct csdev_access {
  *		in the component's sysfs sub-directory.
  * @name:	name for the coresight device, also shown under sysfs.
  * @access:	Describe access to the device
+ * @flags:	The descritpion flags.
+ * @cpu:	The CPU this component is affined to.
  */
 struct coresight_desc {
 	enum coresight_dev_type type;
@@ -163,6 +167,8 @@ struct coresight_desc {
 	const struct attribute_group **groups;
 	const char *name;
 	struct csdev_access access;
+	u32 flags;
+	int cpu;
 };
 
 /**
@@ -260,6 +266,7 @@ struct coresight_trace_id_map {
  *		device's spinlock when the coresight_mutex held and mode ==
  *		CS_MODE_SYSFS. Otherwise it must be accessed from inside the
  *		spinlock.
+ * @cpu:	The CPU this component is affined to (-1 for not CPU bound).
  * @orphan:	true if the component has connections that haven't been linked.
  * @sysfs_sink_activated: 'true' when a sink has been selected for use via sysfs
  *		by writing a 1 to the 'enable_sink' file.  A sink can be
@@ -286,6 +293,7 @@ struct coresight_device {
 	struct device dev;
 	atomic_t mode;
 	int refcnt;
+	int cpu;
 	bool orphan;
 	/* sink specific fields */
 	bool sysfs_sink_activated;

-- 
2.34.1

[PATCH v10 03/20] coresight: Remove .cpu_id() callback from source ops

[Leo Yan]

The CPU ID can be fetched directly from the coresight_device structure,
so the .cpu_id() callback is no longer needed.

Remove the .cpu_id() callback from source ops and update callers
accordingly.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-core.c       | 8 ++++----
 drivers/hwtracing/coresight/coresight-etm-perf.c   | 2 +-
 drivers/hwtracing/coresight/coresight-etm3x-core.c | 8 --------
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 8 --------
 drivers/hwtracing/coresight/coresight-sysfs.c      | 4 ++--
 include/linux/coresight.h                          | 3 ---
 6 files changed, 7 insertions(+), 26 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 89911fc27128b2bb78c8ba704f2af5a5d4efe83c..6907da35ed02fa58f8d30f5576627a6ce3b88362 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -787,7 +787,7 @@ static int _coresight_build_path(struct coresight_device *csdev,
 		goto out;
 
 	if (coresight_is_percpu_source(csdev) && coresight_is_percpu_sink(sink) &&
-	    sink == per_cpu(csdev_sink, source_ops(csdev)->cpu_id(csdev))) {
+	    sink == per_cpu(csdev_sink, csdev->cpu)) {
 		if (_coresight_build_path(sink, source, sink, path) == 0) {
 			found = true;
 			goto out;
@@ -1014,7 +1014,7 @@ coresight_find_default_sink(struct coresight_device *csdev)
 	/* look for a default sink if we have not found for this device */
 	if (!csdev->def_sink) {
 		if (coresight_is_percpu_source(csdev))
-			csdev->def_sink = per_cpu(csdev_sink, source_ops(csdev)->cpu_id(csdev));
+			csdev->def_sink = per_cpu(csdev_sink, csdev->cpu);
 		if (!csdev->def_sink)
 			csdev->def_sink = coresight_find_sink(csdev, &depth);
 	}
@@ -1752,10 +1752,10 @@ int coresight_etm_get_trace_id(struct coresight_device *csdev, enum cs_mode mode
 {
 	int cpu, trace_id;
 
-	if (csdev->type != CORESIGHT_DEV_TYPE_SOURCE || !source_ops(csdev)->cpu_id)
+	if (csdev->type != CORESIGHT_DEV_TYPE_SOURCE)
 		return -EINVAL;
 
-	cpu = source_ops(csdev)->cpu_id(csdev);
+	cpu = csdev->cpu;
 	switch (mode) {
 	case CS_MODE_SYSFS:
 		trace_id = coresight_trace_id_get_cpu_id(cpu);
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
index f85dedf89a3f9e85d568ca4c320fa6fa6d9059ff..cae6738e9906c35d291e4b0f3feae37306b95c06 100644
--- a/drivers/hwtracing/coresight/coresight-etm-perf.c
+++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
@@ -824,7 +824,7 @@ static void etm_addr_filters_sync(struct perf_event *event)
 int etm_perf_symlink(struct coresight_device *csdev, bool link)
 {
 	char entry[sizeof("cpu9999999")];
-	int ret = 0, cpu = source_ops(csdev)->cpu_id(csdev);
+	int ret = 0, cpu = csdev->cpu;
 	struct device *pmu_dev = etm_pmu.dev;
 	struct device *cs_dev = &csdev->dev;
 
diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c
index eb665db1a37d9970f7f55395c0aa23b98a7f3118..ab47f69e923fb191b48b82367dce465c79b3a93d 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c
@@ -466,13 +466,6 @@ static void etm_enable_sysfs_smp_call(void *info)
 		coresight_set_mode(csdev, CS_MODE_DISABLED);
 }
 
-static int etm_cpu_id(struct coresight_device *csdev)
-{
-	struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
-
-	return drvdata->cpu;
-}
-
 void etm_release_trace_id(struct etm_drvdata *drvdata)
 {
 	coresight_trace_id_put_cpu_id(drvdata->cpu);
@@ -684,7 +677,6 @@ static void etm_disable(struct coresight_device *csdev,
 }
 
 static const struct coresight_ops_source etm_source_ops = {
-	.cpu_id		= etm_cpu_id,
 	.enable		= etm_enable,
 	.disable	= etm_disable,
 };
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index b1e0254a534027d7ede8591e56be28745d0b9974..66a8058098376264d3f8c5815763a75ebffb352e 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -227,13 +227,6 @@ static void etm4_cs_unlock(struct etmv4_drvdata *drvdata,
 		CS_UNLOCK(csa->base);
 }
 
-static int etm4_cpu_id(struct coresight_device *csdev)
-{
-	struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
-
-	return drvdata->cpu;
-}
-
 void etm4_release_trace_id(struct etmv4_drvdata *drvdata)
 {
 	coresight_trace_id_put_cpu_id(drvdata->cpu);
@@ -1201,7 +1194,6 @@ static void etm4_pause_perf(struct coresight_device *csdev)
 }
 
 static const struct coresight_ops_source etm4_source_ops = {
-	.cpu_id		= etm4_cpu_id,
 	.enable		= etm4_enable,
 	.disable	= etm4_disable,
 	.resume_perf	= etm4_resume_perf,
diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/hwtracing/coresight/coresight-sysfs.c
index d2a6ed8bcc74d64dccc735463f14790b4e80d101..3b1e7152dd108408d837c404ce607ba511ca14a6 100644
--- a/drivers/hwtracing/coresight/coresight-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-sysfs.c
@@ -232,7 +232,7 @@ int coresight_enable_sysfs(struct coresight_device *csdev)
 		 * 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);
+		cpu = csdev->cpu;
 		per_cpu(tracer_path, cpu) = path;
 		break;
 	case CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE:
@@ -282,7 +282,7 @@ void coresight_disable_sysfs(struct coresight_device *csdev)
 
 	switch (csdev->subtype.source_subtype) {
 	case CORESIGHT_DEV_SUBTYPE_SOURCE_PROC:
-		cpu = source_ops(csdev)->cpu_id(csdev);
+		cpu = csdev->cpu;
 		path = per_cpu(tracer_path, cpu);
 		per_cpu(tracer_path, cpu) = NULL;
 		break;
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index 687190ca11ddeaa83193caa3903a480bac3060d1..e9c20ceb9016fa3db256b8c1147c1fd2027b7b0d 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -395,15 +395,12 @@ struct coresight_ops_link {
 /**
  * struct coresight_ops_source - basic operations for a source
  * Operations available for sources.
- * @cpu_id:	returns the value of the CPU number this component
- *		is associated to.
  * @enable:	enables tracing for a source.
  * @disable:	disables tracing for a source.
  * @resume_perf: resumes tracing for a source in perf session.
  * @pause_perf:	pauses tracing for a source in perf session.
  */
 struct coresight_ops_source {
-	int (*cpu_id)(struct coresight_device *csdev);
 	int (*enable)(struct coresight_device *csdev, struct perf_event *event,
 		      enum cs_mode mode, struct coresight_path *path);
 	void (*disable)(struct coresight_device *csdev,

-- 
2.34.1

[PATCH v10 04/20] coresight: Take hotplug lock in enable_source_store() for Sysfs mode

[Leo Yan]

The hotplug lock is acquired and released in etm{3|4}_disable_sysfs(),
which are low-level functions.  This prevents us from a new solution for
hotplug.

Firstly, hotplug callbacks cannot invoke etm{3|4}_disable_sysfs() to
disable the source; otherwise, a deadlock issue occurs.  The reason is
that, in the hotplug flow, the kernel acquires the hotplug lock before
calling callbacks.  Subsequently, if coresight_disable_source() is
invoked and it calls etm{3|4}_disable_sysfs(), the hotplug lock will be
acquired twice, leading to a double lock issue.

Secondly, when hotplugging a CPU on or off, if we want to manipulate all
components on a path attached to the CPU, we need to maintain atomicity
for the entire path.  Otherwise, a race condition may occur with users
setting the same path via the Sysfs knobs, ultimately causing mess
states in CoreSight components.

This patch moves the hotplug locking from etm{3|4}_disable_sysfs() into
enable_source_store().  As a result, when users control the Sysfs knobs,
the whole flow is protected by hotplug locking, ensuring it is mutual
exclusive with hotplug callbacks.

Note, the paired function etm{3|4}_enable_sysfs() does not use hotplug
locking, which is why this patch does not modify it.

Reviewed-by: Mike Leach <mike.leach@linaro.org>
Tested-by: James Clark <james.clark@linaro.org>
Reviewed-by: Yeoreum Yun <yeoreum.yun@arm.com>
Reviewed-by: James Clark <james.clark@linaro.org>
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-etm3x-core.c | 8 --------
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 9 ---------
 drivers/hwtracing/coresight/coresight-sysfs.c      | 7 +++++++
 3 files changed, 7 insertions(+), 17 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c
index ab47f69e923fb191b48b82367dce465c79b3a93d..aeeb284abdbe4b6a0960da45baa1138e203f3e3c 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c
@@ -620,13 +620,6 @@ static void etm_disable_sysfs(struct coresight_device *csdev)
 {
 	struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
 
-	/*
-	 * Taking hotplug lock here protects from clocks getting disabled
-	 * with tracing being left on (crash scenario) if user disable occurs
-	 * after cpu online mask indicates the cpu is offline but before the
-	 * DYING hotplug callback is serviced by the ETM driver.
-	 */
-	cpus_read_lock();
 	spin_lock(&drvdata->spinlock);
 
 	/*
@@ -637,7 +630,6 @@ static void etm_disable_sysfs(struct coresight_device *csdev)
 				 drvdata, 1);
 
 	spin_unlock(&drvdata->spinlock);
-	cpus_read_unlock();
 
 	/*
 	 * we only release trace IDs when resetting sysfs.
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 66a8058098376264d3f8c5815763a75ebffb352e..a776ebb3b2b0360c99a8dadacfde4c2303dd59d6 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1106,13 +1106,6 @@ static void etm4_disable_sysfs(struct coresight_device *csdev)
 {
 	struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
 
-	/*
-	 * Taking hotplug lock here protects from clocks getting disabled
-	 * with tracing being left on (crash scenario) if user disable occurs
-	 * after cpu online mask indicates the cpu is offline but before the
-	 * DYING hotplug callback is serviced by the ETM driver.
-	 */
-	cpus_read_lock();
 	raw_spin_lock(&drvdata->spinlock);
 
 	/*
@@ -1126,8 +1119,6 @@ static void etm4_disable_sysfs(struct coresight_device *csdev)
 
 	cscfg_csdev_disable_active_config(csdev);
 
-	cpus_read_unlock();
-
 	/*
 	 * we only release trace IDs when resetting sysfs.
 	 * This permits sysfs users to read the trace ID after the trace
diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/hwtracing/coresight/coresight-sysfs.c
index 3b1e7152dd108408d837c404ce607ba511ca14a6..f398dbfbcb8de0c5a873837c19b3fdcf97b64abe 100644
--- a/drivers/hwtracing/coresight/coresight-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-sysfs.c
@@ -360,6 +360,13 @@ static ssize_t enable_source_store(struct device *dev,
 	if (ret)
 		return ret;
 
+	/*
+	 * CoreSight hotplug callbacks in core layer control a activated path
+	 * from its source to sink. Taking hotplug lock here protects a race
+	 * condition with hotplug callbacks.
+	 */
+	guard(cpus_read_lock)();
+
 	if (val) {
 		ret = coresight_enable_sysfs(csdev);
 		if (ret)

-- 
2.34.1

[PATCH v10 05/20] coresight: etm4x: Set per-CPU path on local CPU

[Leo Yan]

Introduce the coresight_set_percpu_local_path() helper to set the path
pointer on the local CPU.  This helper is used during ETMv4 enable and
disable operations.

Tested-by: James Clark <james.clark@linaro.org>
Reviewed-by: Yeoreum Yun <yeoreum.yun@arm.com>
Reviewed-by: James Clark <james.clark@linaro.org>
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-core.c       |  8 ++++++++
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 18 +++++++++++++++---
 drivers/hwtracing/coresight/coresight-priv.h       |  1 +
 3 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 6907da35ed02fa58f8d30f5576627a6ce3b88362..74c9f0dd43784dd735885249c1e50fc86f610582 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -35,6 +35,8 @@
 DEFINE_MUTEX(coresight_mutex);
 static DEFINE_PER_CPU(struct coresight_device *, csdev_sink);
 
+static DEFINE_PER_CPU(struct coresight_path *, percpu_path);
+
 /**
  * struct coresight_node - elements of a path, from source to sink
  * @csdev:	Address of an element.
@@ -423,6 +425,12 @@ int coresight_resume_source(struct coresight_device *csdev)
 }
 EXPORT_SYMBOL_GPL(coresight_resume_source);
 
+void coresight_set_percpu_local_path(struct coresight_path *path)
+{
+	this_cpu_write(percpu_path, path);
+}
+EXPORT_SYMBOL_GPL(coresight_set_percpu_local_path);
+
 /*
  * 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
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index a776ebb3b2b0360c99a8dadacfde4c2303dd59d6..7b91fab9895d7b2a65ebb1161b117d9d3f5fca1b 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -234,6 +234,7 @@ void etm4_release_trace_id(struct etmv4_drvdata *drvdata)
 
 struct etm4_enable_arg {
 	struct etmv4_drvdata *drvdata;
+	struct coresight_path *path;
 	int rc;
 };
 
@@ -621,8 +622,12 @@ static void etm4_enable_sysfs_smp_call(void *info)
 	arg->rc = etm4_enable_hw(arg->drvdata);
 
 	/* The tracer didn't start */
-	if (arg->rc)
+	if (arg->rc) {
 		coresight_set_mode(csdev, CS_MODE_DISABLED);
+		return;
+	}
+
+	coresight_set_percpu_local_path(arg->path);
 }
 
 /*
@@ -890,9 +895,13 @@ static int etm4_enable_perf(struct coresight_device *csdev,
 
 out:
 	/* Failed to start tracer; roll back to DISABLED mode */
-	if (ret)
+	if (ret) {
 		coresight_set_mode(csdev, CS_MODE_DISABLED);
-	return ret;
+		return ret;
+	}
+
+	coresight_set_percpu_local_path(path);
+	return 0;
 }
 
 static int etm4_enable_sysfs(struct coresight_device *csdev, struct coresight_path *path)
@@ -922,6 +931,7 @@ static int etm4_enable_sysfs(struct coresight_device *csdev, struct coresight_pa
 	 * ensures that register writes occur when cpu is powered.
 	 */
 	arg.drvdata = drvdata;
+	arg.path = path;
 	ret = smp_call_function_single(drvdata->cpu,
 				       etm4_enable_sysfs_smp_call, &arg, 1);
 	if (!ret)
@@ -1063,6 +1073,7 @@ static void etm4_disable_sysfs_smp_call(void *info)
 
 	etm4_disable_hw(drvdata);
 
+	coresight_set_percpu_local_path(NULL);
 	coresight_set_mode(drvdata->csdev, CS_MODE_DISABLED);
 }
 
@@ -1092,6 +1103,7 @@ static int etm4_disable_perf(struct coresight_device *csdev,
 	/* TRCVICTLR::SSSTATUS, bit[9] */
 	filters->ssstatus = (control & BIT(9));
 
+	coresight_set_percpu_local_path(NULL);
 	coresight_set_mode(drvdata->csdev, CS_MODE_DISABLED);
 
 	/*
diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
index 1ea882dffd703b2873e41b4ce0c2564d2ce9bbad..ff8a720339deb854ac3b4eb916f49e844f442d34 100644
--- a/drivers/hwtracing/coresight/coresight-priv.h
+++ b/drivers/hwtracing/coresight/coresight-priv.h
@@ -251,5 +251,6 @@ struct coresight_device *coresight_get_percpu_sink(int cpu);
 void coresight_disable_source(struct coresight_device *csdev, void *data);
 void coresight_pause_source(struct coresight_device *csdev);
 int coresight_resume_source(struct coresight_device *csdev);
+void coresight_set_percpu_local_path(struct coresight_path *path);
 
 #endif

-- 
2.34.1

[PATCH v10 06/20] coresight: etm3x: Set per-CPU path on local CPU

[Leo Yan]

Set the path pointer on the local CPU during ETMv3 enable and disable
operations.

Tested-by: James Clark <james.clark@linaro.org>
Reviewed-by: Yeoreum Yun <yeoreum.yun@arm.com>
Reviewed-by: James Clark <james.clark@linaro.org>
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-etm3x-core.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c
index aeeb284abdbe4b6a0960da45baa1138e203f3e3c..46ea66b5cf1985bd7129688f175f6f92372d04ad 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c
@@ -441,6 +441,7 @@ static int etm_enable_hw(struct etm_drvdata *drvdata)
 
 struct etm_enable_arg {
 	struct etm_drvdata *drvdata;
+	struct coresight_path *path;
 	int rc;
 };
 
@@ -462,8 +463,12 @@ static void etm_enable_sysfs_smp_call(void *info)
 	arg->rc = etm_enable_hw(arg->drvdata);
 
 	/* The tracer didn't start */
-	if (arg->rc)
+	if (arg->rc) {
 		coresight_set_mode(csdev, CS_MODE_DISABLED);
+		return;
+	}
+
+	coresight_set_percpu_local_path(arg->path);
 }
 
 void etm_release_trace_id(struct etm_drvdata *drvdata)
@@ -492,10 +497,13 @@ static int etm_enable_perf(struct coresight_device *csdev,
 	ret = etm_enable_hw(drvdata);
 
 	/* Failed to start tracer; roll back to DISABLED mode */
-	if (ret)
+	if (ret) {
 		coresight_set_mode(csdev, CS_MODE_DISABLED);
+		return ret;
+	}
 
-	return ret;
+	coresight_set_percpu_local_path(path);
+	return 0;
 }
 
 static int etm_enable_sysfs(struct coresight_device *csdev, struct coresight_path *path)
@@ -514,6 +522,7 @@ static int etm_enable_sysfs(struct coresight_device *csdev, struct coresight_pat
 	 */
 	if (cpu_online(drvdata->cpu)) {
 		arg.drvdata = drvdata;
+		arg.path = path;
 		ret = smp_call_function_single(drvdata->cpu,
 					       etm_enable_sysfs_smp_call, &arg, 1);
 		if (!ret)
@@ -583,6 +592,7 @@ static void etm_disable_sysfs_smp_call(void *info)
 
 	etm_disable_hw(drvdata);
 
+	coresight_set_percpu_local_path(NULL);
 	coresight_set_mode(drvdata->csdev, CS_MODE_DISABLED);
 }
 
@@ -607,6 +617,7 @@ static void etm_disable_perf(struct coresight_device *csdev)
 
 	CS_LOCK(drvdata->csa.base);
 
+	coresight_set_percpu_local_path(NULL);
 	coresight_set_mode(drvdata->csdev, CS_MODE_DISABLED);
 
 	/*

-- 
2.34.1

[PATCH v10 07/20] coresight: Register CPU PM notifier in core layer

[Leo Yan]

The current implementation only saves and restores the context for ETM
sources while ignoring the context of links.  However, if funnels or
replicators on a linked path resides in a CPU or cluster power domain,
the hardware context for the link will be lost after resuming from low
power states.

To support context management for links during CPU low power modes, a
better way is to implement CPU PM callbacks in the Arm CoreSight core
layer.  As the core layer has sufficient information for linked paths,
from tracers to links, which can be used for power management.

As a first step, this patch registers CPU PM notifier in the core layer.
If a source device provides callbacks for saving and restoring context,
these callbacks will be invoked in CPU suspend and resume.

Reviewed-by: James Clark <james.clark@linaro.org>
Tested-by: James Clark <james.clark@linaro.org>
Reviewed-by: Yeoreum Yun <yeoreum.yun@arm.com>
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-core.c | 101 +++++++++++++++++++++++++++
 include/linux/coresight.h                    |   2 +
 2 files changed, 103 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 74c9f0dd43784dd735885249c1e50fc86f610582..967eac5027a1f02cde29fb3a7be36304c90320b2 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -6,6 +6,7 @@
 #include <linux/acpi.h>
 #include <linux/bitfield.h>
 #include <linux/build_bug.h>
+#include <linux/cpu_pm.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/types.h>
@@ -431,6 +432,11 @@ void coresight_set_percpu_local_path(struct coresight_path *path)
 }
 EXPORT_SYMBOL_GPL(coresight_set_percpu_local_path);
 
+static struct coresight_path *coresight_get_percpu_local_path(void)
+{
+	return this_cpu_read(percpu_path);
+}
+
 /*
  * 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
@@ -1647,6 +1653,94 @@ static void coresight_release_device_list(void)
 	}
 }
 
+/* Return: 1 if PM is required, 0 if skip */
+static int coresight_pm_check(struct coresight_path *path)
+{
+	struct coresight_device *source;
+	bool source_has_cb;
+
+	if (!path)
+		return 0;
+
+	source = coresight_get_source(path);
+	if (!source)
+		return 0;
+
+	/* Don't save and restore if the source is inactive */
+	if (coresight_get_mode(source) == CS_MODE_DISABLED)
+		return 0;
+
+	/* pm_save_disable() and pm_restore_enable() must be paired */
+	source_has_cb = coresight_ops(source)->pm_save_disable &&
+			coresight_ops(source)->pm_restore_enable;
+	if (source_has_cb)
+		return 1;
+
+	return 0;
+}
+
+static int coresight_pm_device_save(struct coresight_device *csdev)
+{
+	return coresight_ops(csdev)->pm_save_disable(csdev);
+}
+
+static void coresight_pm_device_restore(struct coresight_device *csdev)
+{
+	coresight_ops(csdev)->pm_restore_enable(csdev);
+}
+
+static int coresight_pm_save(struct coresight_path *path)
+{
+	struct coresight_device *source = coresight_get_source(path);
+
+	return coresight_pm_device_save(source);
+}
+
+static void coresight_pm_restore(struct coresight_path *path)
+{
+	struct coresight_device *source = coresight_get_source(path);
+
+	coresight_pm_device_restore(source);
+}
+
+static int coresight_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd,
+				   void *v)
+{
+	struct coresight_path *path = coresight_get_percpu_local_path();
+
+	if (!coresight_pm_check(path))
+		return NOTIFY_DONE;
+
+	switch (cmd) {
+	case CPU_PM_ENTER:
+		if (coresight_pm_save(path))
+			return NOTIFY_BAD;
+		break;
+	case CPU_PM_EXIT:
+	case CPU_PM_ENTER_FAILED:
+		coresight_pm_restore(path);
+		break;
+	default:
+		return NOTIFY_DONE;
+	}
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block coresight_cpu_pm_nb = {
+	.notifier_call = coresight_cpu_pm_notify,
+};
+
+static int __init coresight_pm_setup(void)
+{
+	return cpu_pm_register_notifier(&coresight_cpu_pm_nb);
+}
+
+static void coresight_pm_cleanup(void)
+{
+	cpu_pm_unregister_notifier(&coresight_cpu_pm_nb);
+}
+
 const struct bus_type coresight_bustype = {
 	.name	= "coresight",
 };
@@ -1701,9 +1795,15 @@ static int __init coresight_init(void)
 
 	/* initialise the coresight syscfg API */
 	ret = cscfg_init();
+	if (ret)
+		goto exit_notifier;
+
+	ret = coresight_pm_setup();
 	if (!ret)
 		return 0;
 
+	cscfg_exit();
+exit_notifier:
 	atomic_notifier_chain_unregister(&panic_notifier_list,
 					     &coresight_notifier);
 exit_perf:
@@ -1715,6 +1815,7 @@ static int __init coresight_init(void)
 
 static void __exit coresight_exit(void)
 {
+	coresight_pm_cleanup();
 	cscfg_exit();
 	atomic_notifier_chain_unregister(&panic_notifier_list,
 					     &coresight_notifier);
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index e9c20ceb9016fa3db256b8c1147c1fd2027b7b0d..5f9d7ea9f5941ab01eb6a084ca558a9417c7727f 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -438,6 +438,8 @@ struct coresight_ops_panic {
 struct coresight_ops {
 	int (*trace_id)(struct coresight_device *csdev, enum cs_mode mode,
 			struct coresight_device *sink);
+	int (*pm_save_disable)(struct coresight_device *csdev);
+	void (*pm_restore_enable)(struct coresight_device *csdev);
 	const struct coresight_ops_sink *sink_ops;
 	const struct coresight_ops_link *link_ops;
 	const struct coresight_ops_source *source_ops;

-- 
2.34.1

[PATCH v10 08/20] coresight: etm4x: Hook CPU PM callbacks

[Leo Yan]

Add a helper etm4_pm_save_needed() to detect if need save context for
self-hosted PM mode and hook pm_save_disable() and pm_restore_enable()
callbacks through etm4_cs_pm_ops structure.  With this, ETMv4 PM
save/restore is integrated into the core layer's CPU PM.

Organize etm4_cs_ops and etm4_cs_pm_ops together for more readable.

The CPU PM notifier in the ETMv4 driver is no longer needed, remove it
along with its registration and unregistration code.

Reviewed-by: James Clark <james.clark@linaro.org>
Tested-by: James Clark <james.clark@linaro.org>
Reviewed-by: Yeoreum Yun <yeoreum.yun@arm.com>
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 75 +++++++---------------
 1 file changed, 24 insertions(+), 51 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 7b91fab9895d7b2a65ebb1161b117d9d3f5fca1b..3fe9ebf31fe19738c8cfa3cfe6d3464ab96726ce 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1203,11 +1203,6 @@ static const struct coresight_ops_source etm4_source_ops = {
 	.pause_perf	= etm4_pause_perf,
 };
 
-static const struct coresight_ops etm4_cs_ops = {
-	.trace_id	= coresight_etm_get_trace_id,
-	.source_ops	= &etm4_source_ops,
-};
-
 static bool cpu_supports_sysreg_trace(void)
 {
 	u64 dfr0 = read_sysreg_s(SYS_ID_AA64DFR0_EL1);
@@ -1861,6 +1856,11 @@ static int etm4_dying_cpu(unsigned int cpu)
 	return 0;
 }
 
+static inline bool etm4_pm_save_needed(void)
+{
+	return pm_save_enable == PARAM_PM_SAVE_SELF_HOSTED;
+}
+
 static int __etm4_cpu_save(struct etmv4_drvdata *drvdata)
 {
 	int i, ret = 0;
@@ -2003,11 +2003,12 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata)
 	return ret;
 }
 
-static int etm4_cpu_save(struct etmv4_drvdata *drvdata)
+static int etm4_cpu_save(struct coresight_device *csdev)
 {
+	struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
 	int ret = 0;
 
-	if (pm_save_enable != PARAM_PM_SAVE_SELF_HOSTED)
+	if (!etm4_pm_save_needed())
 		return 0;
 
 	/*
@@ -2120,47 +2121,27 @@ static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata)
 	etm4_cs_lock(drvdata, csa);
 }
 
-static void etm4_cpu_restore(struct etmv4_drvdata *drvdata)
+static void etm4_cpu_restore(struct coresight_device *csdev)
 {
-	if (pm_save_enable != PARAM_PM_SAVE_SELF_HOSTED)
+	struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+
+	if (!etm4_pm_save_needed())
 		return;
 
 	if (coresight_get_mode(drvdata->csdev))
 		__etm4_cpu_restore(drvdata);
 }
 
-static int etm4_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd,
-			      void *v)
-{
-	struct etmv4_drvdata *drvdata;
-	unsigned int cpu = smp_processor_id();
-
-	if (!etmdrvdata[cpu])
-		return NOTIFY_OK;
-
-	drvdata = etmdrvdata[cpu];
-
-	if (WARN_ON_ONCE(drvdata->cpu != cpu))
-		return NOTIFY_BAD;
-
-	switch (cmd) {
-	case CPU_PM_ENTER:
-		if (etm4_cpu_save(drvdata))
-			return NOTIFY_BAD;
-		break;
-	case CPU_PM_EXIT:
-	case CPU_PM_ENTER_FAILED:
-		etm4_cpu_restore(drvdata);
-		break;
-	default:
-		return NOTIFY_DONE;
-	}
-
-	return NOTIFY_OK;
-}
+static const struct coresight_ops etm4_cs_ops = {
+	.trace_id	= coresight_etm_get_trace_id,
+	.source_ops	= &etm4_source_ops,
+};
 
-static struct notifier_block etm4_cpu_pm_nb = {
-	.notifier_call = etm4_cpu_pm_notify,
+static const struct coresight_ops etm4_cs_pm_ops = {
+	.trace_id		= coresight_etm_get_trace_id,
+	.source_ops		= &etm4_source_ops,
+	.pm_save_disable	= etm4_cpu_save,
+	.pm_restore_enable	= etm4_cpu_restore,
 };
 
 /* Setup PM. Deals with error conditions and counts */
@@ -2168,16 +2149,12 @@ static int __init etm4_pm_setup(void)
 {
 	int ret;
 
-	ret = cpu_pm_register_notifier(&etm4_cpu_pm_nb);
-	if (ret)
-		return ret;
-
 	ret = cpuhp_setup_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING,
 					"arm/coresight4:starting",
 					etm4_starting_cpu, etm4_dying_cpu);
 
 	if (ret)
-		goto unregister_notifier;
+		return ret;
 
 	ret = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN,
 					"arm/coresight4:online",
@@ -2191,15 +2168,11 @@ static int __init etm4_pm_setup(void)
 
 	/* failed dyn state - remove others */
 	cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING);
-
-unregister_notifier:
-	cpu_pm_unregister_notifier(&etm4_cpu_pm_nb);
 	return ret;
 }
 
 static void etm4_pm_clear(void)
 {
-	cpu_pm_unregister_notifier(&etm4_cpu_pm_nb);
 	cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING);
 	if (hp_online) {
 		cpuhp_remove_state_nocalls(hp_online);
@@ -2251,7 +2224,7 @@ static int etm4_add_coresight_dev(struct etm4_init_arg *init_arg)
 
 	desc.type = CORESIGHT_DEV_TYPE_SOURCE;
 	desc.subtype.source_subtype = CORESIGHT_DEV_SUBTYPE_SOURCE_PROC;
-	desc.ops = &etm4_cs_ops;
+	desc.ops = etm4_pm_save_needed() ? &etm4_cs_pm_ops : &etm4_cs_ops;
 	desc.pdata = pdata;
 	desc.dev = dev;
 	desc.groups = coresight_etmv4_groups;
@@ -2306,7 +2279,7 @@ static int etm4_probe(struct device *dev)
 		pm_save_enable = coresight_loses_context_with_cpu(dev) ?
 			       PARAM_PM_SAVE_SELF_HOSTED : PARAM_PM_SAVE_NEVER;
 
-	if (pm_save_enable != PARAM_PM_SAVE_NEVER) {
+	if (etm4_pm_save_needed()) {
 		drvdata->save_state = devm_kmalloc(dev,
 				sizeof(struct etmv4_save_state), GFP_KERNEL);
 		if (!drvdata->save_state)

-- 
2.34.1

[PATCH v10 09/20] coresight: etm4x: Remove redundant checks in PM save and restore

[Leo Yan]

ETMv4 driver save/restore callbacks still re-check conditions that are
already validated by the core layer before the callbacks are invoked.

Remove the duplicated checks and fold the two-level functions into
direct callback implementations.  The obsolete WARN_ON() checks are
removed as well.

Reviewed-by: Mike Leach <mike.leach@linaro.org>
Tested-by: James Clark <james.clark@linaro.org>
Reviewed-by: Yeoreum Yun <yeoreum.yun@arm.com>
Reviewed-by: James Clark <james.clark@linaro.org>
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 41 +++-------------------
 1 file changed, 4 insertions(+), 37 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 3fe9ebf31fe19738c8cfa3cfe6d3464ab96726ce..5ccbf792fe71bec7d5b74c24288c5313afdb7cd0 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1861,17 +1861,14 @@ static inline bool etm4_pm_save_needed(void)
 	return pm_save_enable == PARAM_PM_SAVE_SELF_HOSTED;
 }
 
-static int __etm4_cpu_save(struct etmv4_drvdata *drvdata)
+static int etm4_cpu_save(struct coresight_device *csdev)
 {
 	int i, ret = 0;
+	struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
 	struct etmv4_save_state *state;
-	struct coresight_device *csdev = drvdata->csdev;
 	struct csdev_access *csa;
 	struct device *etm_dev;
 
-	if (WARN_ON(!csdev))
-		return -ENODEV;
-
 	etm_dev = &csdev->dev;
 	csa = &csdev->access;
 
@@ -2003,32 +2000,13 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata)
 	return ret;
 }
 
-static int etm4_cpu_save(struct coresight_device *csdev)
-{
-	struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
-	int ret = 0;
-
-	if (!etm4_pm_save_needed())
-		return 0;
-
-	/*
-	 * Save and restore the ETM Trace registers only if
-	 * the ETM is active.
-	 */
-	if (coresight_get_mode(drvdata->csdev))
-		ret = __etm4_cpu_save(drvdata);
-	return ret;
-}
-
-static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata)
+static void etm4_cpu_restore(struct coresight_device *csdev)
 {
 	int i;
+	struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
 	struct etmv4_save_state *state = drvdata->save_state;
 	struct csdev_access *csa = &drvdata->csdev->access;
 
-	if (WARN_ON(!drvdata->csdev))
-		return;
-
 	etm4_cs_unlock(drvdata, csa);
 	etm4x_relaxed_write32(csa, state->trcclaimset, TRCCLAIMSET);
 
@@ -2121,17 +2099,6 @@ static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata)
 	etm4_cs_lock(drvdata, csa);
 }
 
-static void etm4_cpu_restore(struct coresight_device *csdev)
-{
-	struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
-
-	if (!etm4_pm_save_needed())
-		return;
-
-	if (coresight_get_mode(drvdata->csdev))
-		__etm4_cpu_restore(drvdata);
-}
-
 static const struct coresight_ops etm4_cs_ops = {
 	.trace_id	= coresight_etm_get_trace_id,
 	.source_ops	= &etm4_source_ops,

-- 
2.34.1

[PATCH v10 10/20] coresight: syscfg: Use IRQ-safe spinlock to protect active variables

[Leo Yan]

cscfg_config_sysfs_get_active_cfg() will be used from idle flows, while
sleeping locks are not allowed.  Introduce sysfs_store_lock to replace
the mutex to protect sysfs_active_config and sysfs_active_preset
accesses, with IRQ-safe locking to avoid lockdep complaint.

Refactor cscfg_config_sysfs_activate() to handle errors first and use
finer-grained locking only for sysfs_active_config.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-syscfg.c | 35 +++++++++++++-------------
 drivers/hwtracing/coresight/coresight-syscfg.h |  2 ++
 2 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-syscfg.c b/drivers/hwtracing/coresight/coresight-syscfg.c
index d7f5037953d6ba7fb7f83a8012a1abc5ffd0a147..2ccd7659b472e4993a16be408d0647b5993c5ec7 100644
--- a/drivers/hwtracing/coresight/coresight-syscfg.c
+++ b/drivers/hwtracing/coresight/coresight-syscfg.c
@@ -953,39 +953,40 @@ int cscfg_config_sysfs_activate(struct cscfg_config_desc *config_desc, bool acti
 	unsigned long cfg_hash;
 	int err = 0;
 
-	mutex_lock(&cscfg_mutex);
+	guard(mutex)(&cscfg_mutex);
 
 	cfg_hash = (unsigned long)config_desc->event_ea->var;
 
 	if (activate) {
 		/* cannot be a current active value to activate this */
-		if (cscfg_mgr->sysfs_active_config) {
-			err = -EBUSY;
-			goto exit_unlock;
-		}
+		if (cscfg_mgr->sysfs_active_config)
+			return -EBUSY;
+
 		err = _cscfg_activate_config(cfg_hash);
-		if (!err)
+		if (err)
+			return err;
+
+		scoped_guard(raw_spinlock_irqsave, &cscfg_mgr->sysfs_store_lock)
 			cscfg_mgr->sysfs_active_config = cfg_hash;
 	} else {
+		if (cscfg_mgr->sysfs_active_config != cfg_hash)
+			return -EINVAL;
+
 		/* disable if matching current value */
-		if (cscfg_mgr->sysfs_active_config == cfg_hash) {
-			_cscfg_deactivate_config(cfg_hash);
+		_cscfg_deactivate_config(cfg_hash);
+
+		scoped_guard(raw_spinlock_irqsave, &cscfg_mgr->sysfs_store_lock)
 			cscfg_mgr->sysfs_active_config = 0;
-		} else
-			err = -EINVAL;
 	}
 
-exit_unlock:
-	mutex_unlock(&cscfg_mutex);
-	return err;
+	return 0;
 }
 
 /* set the sysfs preset value */
 void cscfg_config_sysfs_set_preset(int preset)
 {
-	mutex_lock(&cscfg_mutex);
+	guard(raw_spinlock_irqsave)(&cscfg_mgr->sysfs_store_lock);
 	cscfg_mgr->sysfs_active_preset = preset;
-	mutex_unlock(&cscfg_mutex);
 }
 
 /*
@@ -994,10 +995,9 @@ void cscfg_config_sysfs_set_preset(int preset)
  */
 void cscfg_config_sysfs_get_active_cfg(unsigned long *cfg_hash, int *preset)
 {
-	mutex_lock(&cscfg_mutex);
+	guard(raw_spinlock_irqsave)(&cscfg_mgr->sysfs_store_lock);
 	*preset = cscfg_mgr->sysfs_active_preset;
 	*cfg_hash = cscfg_mgr->sysfs_active_config;
-	mutex_unlock(&cscfg_mutex);
 }
 EXPORT_SYMBOL_GPL(cscfg_config_sysfs_get_active_cfg);
 
@@ -1201,6 +1201,7 @@ static int cscfg_create_device(void)
 	INIT_LIST_HEAD(&cscfg_mgr->load_order_list);
 	atomic_set(&cscfg_mgr->sys_active_cnt, 0);
 	cscfg_mgr->load_state = CSCFG_NONE;
+	raw_spin_lock_init(&cscfg_mgr->sysfs_store_lock);
 
 	/* setup the device */
 	dev = cscfg_device();
diff --git a/drivers/hwtracing/coresight/coresight-syscfg.h b/drivers/hwtracing/coresight/coresight-syscfg.h
index 66e2db890d8203853a0c3c907b48aa66dd8014e6..658e93c3705f1cb3ba3523d0bc27ac704697dd70 100644
--- a/drivers/hwtracing/coresight/coresight-syscfg.h
+++ b/drivers/hwtracing/coresight/coresight-syscfg.h
@@ -42,6 +42,7 @@ enum cscfg_load_ops {
  * @sysfs_active_config:Active config hash used if CoreSight controlled from sysfs.
  * @sysfs_active_preset:Active preset index used if CoreSight controlled from sysfs.
  * @load_state:		A multi-stage load/unload operation is in progress.
+ * @sysfs_store_lock:	Exclusive access sysfs stored variables.
  */
 struct cscfg_manager {
 	struct device dev;
@@ -54,6 +55,7 @@ struct cscfg_manager {
 	u32 sysfs_active_config;
 	int sysfs_active_preset;
 	enum cscfg_load_ops load_state;
+	raw_spinlock_t sysfs_store_lock;
 };
 
 /* get reference to dev in cscfg_manager */

-- 
2.34.1

[PATCH v10 11/20] coresight: Move source helper disabling to coresight_disable_path()

[Leo Yan]

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.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-core.c | 32 ++++++++++++----------------
 1 file changed, 14 insertions(+), 18 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 967eac5027a1f02cde29fb3a7be36304c90320b2..348eb16a23ad7606ede9f26ae6820ea102ddb8cd 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -388,19 +388,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);
 
@@ -451,7 +444,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;
 
@@ -471,12 +464,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;
@@ -523,12 +510,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;
 
@@ -582,6 +573,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;
+
+	nd = list_next_entry(nd, link);
 	coresight_disable_path_from(path, nd);
 	goto out;
 }

-- 
2.34.1

[PATCH v10 12/20] coresight: Control path with range

[Leo Yan]

Add parameters @from and @to for path enabling and disabling, allowing
finer-grained control of a path.  The range is [@from..@to], where both
@from and @to are inclusive, introduce coresight_path_nodes_in_order()
to validate the given range if is ordered.

The helpers coresight_path_{first|last}_node() are provided for
conveniently fetching the first and last nodes on the path.

A minor refactoring removes the local variable "source" from
coresight_enable_path_from_to(), deferring reading the source until it
is needed.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-core.c | 95 ++++++++++++++++++++++------
 1 file changed, 77 insertions(+), 18 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 348eb16a23ad7606ede9f26ae6820ea102ddb8cd..752fe3b4b90fce34c4d245ff994a8afdb8ece463 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -61,6 +61,24 @@ static LIST_HEAD(coresight_dev_idx_list);
 
 static const struct cti_assoc_op *cti_assoc_ops;
 
+static struct coresight_node *
+coresight_path_first_node(struct coresight_path *path)
+{
+	if (list_empty(&path->path_list))
+		return NULL;
+
+	return list_first_entry(&path->path_list, struct coresight_node, link);
+}
+
+static struct coresight_node *
+coresight_path_last_node(struct coresight_path *path)
+{
+	if (list_empty(&path->path_list))
+		return NULL;
+
+	return list_last_entry(&path->path_list, struct coresight_node, link);
+}
+
 void coresight_set_cti_ops(const struct cti_assoc_op *cti_op)
 {
 	cti_assoc_ops = cti_op;
@@ -431,19 +449,41 @@ static struct coresight_path *coresight_get_percpu_local_path(void)
 }
 
 /*
- * 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.
+ * Callers must fetch nodes from the path and pass @from and @to to the path
+ * enable/disable functions. Walk the path from @from to locate @to. If @to
+ * is found, it indicates @from and @to are in order. Otherwise, they are out
+ * of order.
  */
-static void coresight_disable_path_from(struct coresight_path *path,
-					struct coresight_node *nd)
+static bool coresight_path_nodes_in_order(struct coresight_path *path,
+					  struct coresight_node *from,
+					  struct coresight_node *to)
+{
+	struct coresight_node *nd;
+
+	if (WARN_ON_ONCE(!from || !to))
+		return false;
+
+	nd = from;
+	list_for_each_entry_from(nd, &path->path_list, link) {
+		if (nd == to)
+			return true;
+	}
+
+	return false;
+}
+
+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;
 
+	nd = from;
 	list_for_each_entry_from(nd, &path->path_list, link) {
 		csdev = nd->csdev;
 		type = csdev->type;
@@ -477,12 +517,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);
 
@@ -506,19 +552,20 @@ static int coresight_enable_helpers(struct coresight_device *csdev,
 	return 0;
 }
 
-int coresight_enable_path(struct coresight_path *path, enum cs_mode mode)
+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;
-	struct coresight_node *nd, *last;
+	struct coresight_node *nd;
 	struct coresight_device *csdev, *parent, *child;
-	struct coresight_device *source;
 
-	source = coresight_get_source(path);
-
-	last = list_last_entry(&path->path_list, struct coresight_node, link);
+	if (!coresight_path_nodes_in_order(path, from, to))
+		return -EINVAL;
 
-	nd = last;
+	nd = to;
 	list_for_each_entry_from_reverse(nd, &path->path_list, link) {
 		csdev = nd->csdev;
 		type = csdev->type;
@@ -558,7 +605,8 @@ int coresight_enable_path(struct coresight_path *path, enum cs_mode mode)
 		case CORESIGHT_DEV_TYPE_LINK:
 			parent = list_prev_entry(nd, link)->csdev;
 			child = list_next_entry(nd, link)->csdev;
-			ret = coresight_enable_link(csdev, parent, child, source);
+			ret = coresight_enable_link(csdev, parent, child,
+						    coresight_get_source(path));
 			if (ret)
 				goto err_disable_helpers;
 			break;
@@ -566,6 +614,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:
@@ -574,14 +626,21 @@ int coresight_enable_path(struct coresight_path *path, enum cs_mode mode)
 	coresight_disable_helpers(csdev, path);
 err_disable_path:
 	/* No device is actually enabled */
-	if (nd == last)
+	if (nd == to)
 		goto out;
 
 	nd = list_next_entry(nd, link);
-	coresight_disable_path_from(path, nd);
+	coresight_disable_path_from_to(path, nd, to);
 	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;

-- 
2.34.1

[PATCH v10 13/20] coresight: Use helpers to fetch first and last nodes

[Leo Yan]

Replace open code with coresight_path_first_node() and
coresight_path_last_node() for fetching the nodes.

Check that the node is not NULL before accessing csdev field.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-core.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 752fe3b4b90fce34c4d245ff994a8afdb8ece463..3d2876d37c5bc371dd5a88640e080f6e52af5294 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -106,11 +106,16 @@ EXPORT_SYMBOL_GPL(coresight_get_percpu_sink);
 static struct coresight_device *coresight_get_source(struct coresight_path *path)
 {
 	struct coresight_device *csdev;
+	struct coresight_node *nd;
 
 	if (!path)
 		return NULL;
 
-	csdev = list_first_entry(&path->path_list, struct coresight_node, link)->csdev;
+	nd = coresight_path_first_node(path);
+	if (!nd)
+		return NULL;
+
+	csdev = nd->csdev;
 	if (!coresight_is_device_source(csdev))
 		return NULL;
 
@@ -644,11 +649,16 @@ int coresight_enable_path(struct coresight_path *path, enum cs_mode mode)
 struct coresight_device *coresight_get_sink(struct coresight_path *path)
 {
 	struct coresight_device *csdev;
+	struct coresight_node *nd;
 
 	if (!path)
 		return NULL;
 
-	csdev = list_last_entry(&path->path_list, struct coresight_node, link)->csdev;
+	nd = coresight_path_last_node(path);
+	if (!nd)
+		return NULL;
+
+	csdev = nd->csdev;
 	if (csdev->type != CORESIGHT_DEV_TYPE_SINK &&
 	    csdev->type != CORESIGHT_DEV_TYPE_LINKSINK)
 		return NULL;

-- 
2.34.1

[PATCH v10 14/20] coresight: Introduce coresight_enable_source() helper

[Leo Yan]

Introduce the coresight_enable_source() helper for enabling source
device.

Add validation to ensure the device is a source before proceeding with
further operations.

Reviewed-by: James Clark <james.clark@linaro.org>
Tested-by: James Clark <james.clark@linaro.org>
Reviewed-by: Yeoreum Yun <yeoreum.yun@arm.com>
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-core.c     | 18 ++++++++++++++++--
 drivers/hwtracing/coresight/coresight-etm-perf.c |  2 +-
 drivers/hwtracing/coresight/coresight-priv.h     |  3 +++
 drivers/hwtracing/coresight/coresight-sysfs.c    |  2 +-
 4 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 3d2876d37c5bc371dd5a88640e080f6e52af5294..02d26dd0171ebf4e884bb3e0028b9a21588f061a 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -411,11 +411,25 @@ static void coresight_disable_helpers(struct coresight_device *csdev,
 }
 
 /*
- * coresight_disable_source() only disables the source, but do nothing for
- * the associated helpers, which are controlled as part of the path.
+ * coresight_enable_source() and coresight_disable_source() only enable and
+ * disable the source, but do nothing for the associated helpers, which are
+ * controlled as part of the path.
  */
+int coresight_enable_source(struct coresight_device *csdev,
+			    struct perf_event *event, enum cs_mode mode,
+			    struct coresight_path *path)
+{
+	if (!coresight_is_device_source(csdev))
+		return -EINVAL;
+
+	return source_ops(csdev)->enable(csdev, event, mode, path);
+}
+
 void coresight_disable_source(struct coresight_device *csdev, void *data)
 {
+	if (!coresight_is_device_source(csdev))
+		return;
+
 	source_ops(csdev)->disable(csdev, data);
 }
 EXPORT_SYMBOL_GPL(coresight_disable_source);
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
index cae6738e9906c35d291e4b0f3feae37306b95c06..b23c8c4b87972ed530743ef93bea69544c2c5ebf 100644
--- a/drivers/hwtracing/coresight/coresight-etm-perf.c
+++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
@@ -533,7 +533,7 @@ static void etm_event_start(struct perf_event *event, int flags)
 		goto fail_end_stop;
 
 	/* Finally enable the tracer */
-	if (source_ops(csdev)->enable(csdev, event, CS_MODE_PERF, path))
+	if (coresight_enable_source(csdev, event, CS_MODE_PERF, path))
 		goto fail_disable_path;
 
 	/*
diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
index ff8a720339deb854ac3b4eb916f49e844f442d34..43ce810ad5deab339f0a336266b6018ddaf8c4fd 100644
--- a/drivers/hwtracing/coresight/coresight-priv.h
+++ b/drivers/hwtracing/coresight/coresight-priv.h
@@ -248,6 +248,9 @@ void coresight_add_helper(struct coresight_device *csdev,
 
 void coresight_set_percpu_sink(int cpu, struct coresight_device *csdev);
 struct coresight_device *coresight_get_percpu_sink(int cpu);
+int coresight_enable_source(struct coresight_device *csdev,
+			    struct perf_event *event, enum cs_mode mode,
+			    struct coresight_path *path);
 void coresight_disable_source(struct coresight_device *csdev, void *data);
 void coresight_pause_source(struct coresight_device *csdev);
 int coresight_resume_source(struct coresight_device *csdev);
diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/hwtracing/coresight/coresight-sysfs.c
index f398dbfbcb8de0c5a873837c19b3fdcf97b64abe..9449de66ba3941614928924086100866f3c88a54 100644
--- a/drivers/hwtracing/coresight/coresight-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-sysfs.c
@@ -66,7 +66,7 @@ static int coresight_enable_source_sysfs(struct coresight_device *csdev,
 	 */
 	lockdep_assert_held(&coresight_mutex);
 	if (coresight_get_mode(csdev) != CS_MODE_SYSFS) {
-		ret = source_ops(csdev)->enable(csdev, NULL, mode, path);
+		ret = coresight_enable_source(csdev, NULL, mode, path);
 		if (ret)
 			return ret;
 	}

-- 
2.34.1

[PATCH v10 15/20] coresight: Control path during CPU idle

[Leo Yan]

Extend the CPU PM flow to control the path: disable from source up to
the node before the sink, then re-enable the same range on restore.
To avoid latency, control it up to the node before the sink.

Track per-CPU PM restore failures using percpu_pm_failed.  Once a CPU
hits a restore failure, set the percpu_pm_failed and return NOTIFY_BAD
on subsequent notifications to avoid repeating half-completed
transitions.

Setting percpu_pm_failed permanently blocks CPU PM on that CPU.  Such
failures are typically seen during development; disabling PM operations
simplifies the implementation, and a warning highlights the issue.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-core.c | 47 +++++++++++++++++++++++++---
 1 file changed, 43 insertions(+), 4 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 02d26dd0171ebf4e884bb3e0028b9a21588f061a..c1e8debc76aba7eb5ecf7efe2a3b9b8b3e11b10c 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -37,6 +37,7 @@ DEFINE_MUTEX(coresight_mutex);
 static DEFINE_PER_CPU(struct coresight_device *, csdev_sink);
 
 static DEFINE_PER_CPU(struct coresight_path *, percpu_path);
+static DEFINE_PER_CPU(bool, percpu_pm_failed);
 
 /**
  * struct coresight_node - elements of a path, from source to sink
@@ -1732,7 +1733,7 @@ static void coresight_release_device_list(void)
 	}
 }
 
-/* Return: 1 if PM is required, 0 if skip */
+/* Return: 1 if PM is required, 0 if skip, <0 on error */
 static int coresight_pm_check(struct coresight_path *path)
 {
 	struct coresight_device *source;
@@ -1749,6 +1750,9 @@ static int coresight_pm_check(struct coresight_path *path)
 	if (coresight_get_mode(source) == CS_MODE_DISABLED)
 		return 0;
 
+	if (this_cpu_read(percpu_pm_failed))
+		return -EIO;
+
 	/* pm_save_disable() and pm_restore_enable() must be paired */
 	source_has_cb = coresight_ops(source)->pm_save_disable &&
 			coresight_ops(source)->pm_restore_enable;
@@ -1771,24 +1775,59 @@ static void coresight_pm_device_restore(struct coresight_device *csdev)
 static int coresight_pm_save(struct coresight_path *path)
 {
 	struct coresight_device *source = coresight_get_source(path);
+	struct coresight_node *from, *to;
+	int ret;
+
+	ret = coresight_pm_device_save(source);
+	if (ret)
+		return ret;
+
+	from = coresight_path_first_node(path);
+	/* Up to the node before sink to avoid latency */
+	to = list_prev_entry(coresight_path_last_node(path), link);
+	coresight_disable_path_from_to(path, from, to);
 
-	return coresight_pm_device_save(source);
+	return 0;
 }
 
 static void coresight_pm_restore(struct coresight_path *path)
 {
 	struct coresight_device *source = coresight_get_source(path);
+	struct coresight_node *from, *to;
+	int ret;
+
+	from = coresight_path_first_node(path);
+	/* Up to the node before sink to avoid latency */
+	to = list_prev_entry(coresight_path_last_node(path), link);
+	ret = coresight_enable_path_from_to(path, coresight_get_mode(source),
+					    from, to);
+	if (ret)
+		goto path_failed;
 
 	coresight_pm_device_restore(source);
+	return;
+
+path_failed:
+	pr_err("Failed in coresight PM restore on CPU%d: %d\n",
+	       smp_processor_id(), ret);
+
+	/*
+	 * Once PM fails on a CPU, set percpu_pm_failed and leave it set until
+	 * reboot. This prevents repeated partial transitions during idle
+	 * entry and exit.
+	 */
+	this_cpu_write(percpu_pm_failed, true);
 }
 
 static int coresight_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd,
 				   void *v)
 {
 	struct coresight_path *path = coresight_get_percpu_local_path();
+	int ret;
 
-	if (!coresight_pm_check(path))
-		return NOTIFY_DONE;
+	ret = coresight_pm_check(path);
+	if (ret <= 0)
+		return ret ? NOTIFY_BAD : NOTIFY_DONE;
 
 	switch (cmd) {
 	case CPU_PM_ENTER:

-- 
2.34.1

[PATCH v10 16/20] coresight: Add PM callbacks for sink device

[Leo Yan]

Unlike system level sinks, per-CPU sinks may lose power during CPU idle
states.  Currently, this applies specifically to TRBE.  This commit
invokes save and restore callbacks for the sink in the CPU PM notifier.

If the sink provides PM callbacks but the source does not, this is
unsafe because the sink cannot be disabled safely unless the source
can also be controlled, so veto low power entry to avoid lockups.

Tested-by: James Clark <james.clark@linaro.org>
Reviewed-by: Yeoreum Yun <yeoreum.yun@arm.com>
Reviewed-by: James Clark <james.clark@linaro.org>
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-core.c | 46 ++++++++++++++++++++++++++--
 1 file changed, 43 insertions(+), 3 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index c1e8debc76aba7eb5ecf7efe2a3b9b8b3e11b10c..a918bf6398a932de30fe9b4947020cc4c1cfb2f7 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -1736,14 +1736,15 @@ static void coresight_release_device_list(void)
 /* Return: 1 if PM is required, 0 if skip, <0 on error */
 static int coresight_pm_check(struct coresight_path *path)
 {
-	struct coresight_device *source;
-	bool source_has_cb;
+	struct coresight_device *source, *sink;
+	bool source_has_cb, sink_has_cb;
 
 	if (!path)
 		return 0;
 
 	source = coresight_get_source(path);
-	if (!source)
+	sink = coresight_get_sink(path);
+	if (!source || !sink)
 		return 0;
 
 	/* Don't save and restore if the source is inactive */
@@ -1759,16 +1760,36 @@ static int coresight_pm_check(struct coresight_path *path)
 	if (source_has_cb)
 		return 1;
 
+	sink_has_cb = coresight_ops(sink)->pm_save_disable &&
+		      coresight_ops(sink)->pm_restore_enable;
+	/*
+	 * It is not permitted that the source has no callbacks while the sink
+	 * does, as the sink cannot be disabled without disabling the source,
+	 * which may lead to lockups. Alternatively, the ETM driver should
+	 * enable self-hosted PM mode at probe (see etm4_probe()).
+	 */
+	if (sink_has_cb) {
+		pr_warn_once("coresight PM failed: source has no PM callbacks; "
+			     "cannot safely control sink\n");
+		return -EINVAL;
+	}
+
 	return 0;
 }
 
 static int coresight_pm_device_save(struct coresight_device *csdev)
 {
+	if (!csdev || !coresight_ops(csdev)->pm_save_disable)
+		return 0;
+
 	return coresight_ops(csdev)->pm_save_disable(csdev);
 }
 
 static void coresight_pm_device_restore(struct coresight_device *csdev)
 {
+	if (!csdev || !coresight_ops(csdev)->pm_restore_enable)
+		return;
+
 	coresight_ops(csdev)->pm_restore_enable(csdev);
 }
 
@@ -1787,15 +1808,32 @@ static int coresight_pm_save(struct coresight_path *path)
 	to = list_prev_entry(coresight_path_last_node(path), link);
 	coresight_disable_path_from_to(path, from, to);
 
+	ret = coresight_pm_device_save(coresight_get_sink(path));
+	if (ret)
+		goto sink_failed;
+
 	return 0;
+
+sink_failed:
+	if (!coresight_enable_path_from_to(path, coresight_get_mode(source),
+					   from, to))
+		coresight_pm_device_restore(source);
+
+	pr_err("Failed in coresight PM save on CPU%d: %d\n",
+	       smp_processor_id(), ret);
+	this_cpu_write(percpu_pm_failed, true);
+	return ret;
 }
 
 static void coresight_pm_restore(struct coresight_path *path)
 {
 	struct coresight_device *source = coresight_get_source(path);
+	struct coresight_device *sink = coresight_get_sink(path);
 	struct coresight_node *from, *to;
 	int ret;
 
+	coresight_pm_device_restore(sink);
+
 	from = coresight_path_first_node(path);
 	/* Up to the node before sink to avoid latency */
 	to = list_prev_entry(coresight_path_last_node(path), link);
@@ -1808,6 +1846,8 @@ static void coresight_pm_restore(struct coresight_path *path)
 	return;
 
 path_failed:
+	coresight_pm_device_save(sink);
+
 	pr_err("Failed in coresight PM restore on CPU%d: %d\n",
 	       smp_processor_id(), ret);
 

-- 
2.34.1

Re: [PATCH v10 16/20] coresight: Add PM callbacks for sink device

[James Clark]

On 05/04/2026 4:02 pm, Leo Yan wrote:
> Unlike system level sinks, per-CPU sinks may lose power during CPU idle
> states.  Currently, this applies specifically to TRBE.  This commit
> invokes save and restore callbacks for the sink in the CPU PM notifier.
> 
> If the sink provides PM callbacks but the source does not, this is
> unsafe because the sink cannot be disabled safely unless the source
> can also be controlled, so veto low power entry to avoid lockups.
> 
> Tested-by: James Clark <james.clark@linaro.org>
> Reviewed-by: Yeoreum Yun <yeoreum.yun@arm.com>
> Reviewed-by: James Clark <james.clark@linaro.org>
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>   drivers/hwtracing/coresight/coresight-core.c | 46 ++++++++++++++++++++++++++--
>   1 file changed, 43 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index c1e8debc76aba7eb5ecf7efe2a3b9b8b3e11b10c..a918bf6398a932de30fe9b4947020cc4c1cfb2f7 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -1736,14 +1736,15 @@ static void coresight_release_device_list(void)
>   /* Return: 1 if PM is required, 0 if skip, <0 on error */
>   static int coresight_pm_check(struct coresight_path *path)
>   {
> -	struct coresight_device *source;
> -	bool source_has_cb;
> +	struct coresight_device *source, *sink;
> +	bool source_has_cb, sink_has_cb;
>   
>   	if (!path)
>   		return 0;
>   
>   	source = coresight_get_source(path);
> -	if (!source)
> +	sink = coresight_get_sink(path);
> +	if (!source || !sink)
>   		return 0;
>   
>   	/* Don't save and restore if the source is inactive */
> @@ -1759,16 +1760,36 @@ static int coresight_pm_check(struct coresight_path *path)
>   	if (source_has_cb)
>   		return 1;
>   
> +	sink_has_cb = coresight_ops(sink)->pm_save_disable &&
> +		      coresight_ops(sink)->pm_restore_enable;
> +	/*
> +	 * It is not permitted that the source has no callbacks while the sink
> +	 * does, as the sink cannot be disabled without disabling the source,
> +	 * which may lead to lockups. Alternatively, the ETM driver should
> +	 * enable self-hosted PM mode at probe (see etm4_probe()).
> +	 */
> +	if (sink_has_cb) {
> +		pr_warn_once("coresight PM failed: source has no PM callbacks; "
> +			     "cannot safely control sink\n");

This prints out on my Orion board on a fresh boot because of how 
pm_save_enable is setup there. Do we really need the configuration of 
pm_save_enable for ETE/TRBE if we know that it always needs saving?

It also stops warning if I rmmod and modprobe the module after booting. 
Seems like pm_save_enable is different depending on how the module is 
loaded which doesn't seem right.

> +		return -EINVAL;
> +	}
> +
>   	return 0;
>   }
>   
>   static int coresight_pm_device_save(struct coresight_device *csdev)
>   {
> +	if (!csdev || !coresight_ops(csdev)->pm_save_disable)
> +		return 0;
> +
>   	return coresight_ops(csdev)->pm_save_disable(csdev);
>   }
>   
>   static void coresight_pm_device_restore(struct coresight_device *csdev)
>   {
> +	if (!csdev || !coresight_ops(csdev)->pm_restore_enable)
> +		return;
> +
>   	coresight_ops(csdev)->pm_restore_enable(csdev);
>   }
>   
> @@ -1787,15 +1808,32 @@ static int coresight_pm_save(struct coresight_path *path)
>   	to = list_prev_entry(coresight_path_last_node(path), link);
>   	coresight_disable_path_from_to(path, from, to);
>   
> +	ret = coresight_pm_device_save(coresight_get_sink(path));
> +	if (ret)
> +		goto sink_failed;
> +

The comment directly above this says "Up to the node before sink to 
avoid latency". But then this line goes and saves the sink anyway. So 
I'm not sure what's meant by the comment?

>   	return 0;
> +
> +sink_failed:
> +	if (!coresight_enable_path_from_to(path, coresight_get_mode(source),
> +					   from, to))
> +		coresight_pm_device_restore(source);
> +
> +	pr_err("Failed in coresight PM save on CPU%d: %d\n",
> +	       smp_processor_id(), ret);
> +	this_cpu_write(percpu_pm_failed, true);

Why does only a failing sink set percpu_pm_failed when failing to save 
the source exits early. Sashiko has a similar comment that this could 
result in restoring uninitialised source save data later, but a comment 
in this function about why the flow is like this would be helpful.

We have coresight_disable_path_from_to() which always succeeds and 
doesn't return an error. TRBE is the only sink with a pm_save_disable()
callback, but it always succeeds anyway.

Would it not be much simpler to require that sink save/restore callbacks 
always succeed and don't return anything? Seems like this 
percpu_pm_failed stuff is extra complexity for a scenario that doesn't 
exist? The only thing that can fail is saving the source but it doesn't 
goto sink_failed when that happens.

Ideally etm4_cpu_save() wouldn't have a return value either. It would be 
good if we could find away to skip or ignore the timeouts in there 
somehow because that's the only reason it can fail.

> +	return ret;
>   }
>   
>   static void coresight_pm_restore(struct coresight_path *path)
>   {
>   	struct coresight_device *source = coresight_get_source(path);
> +	struct coresight_device *sink = coresight_get_sink(path);
>   	struct coresight_node *from, *to;
>   	int ret;
>   
> +	coresight_pm_device_restore(sink);
> +
>   	from = coresight_path_first_node(path);
>   	/* Up to the node before sink to avoid latency */
>   	to = list_prev_entry(coresight_path_last_node(path), link);
> @@ -1808,6 +1846,8 @@ static void coresight_pm_restore(struct coresight_path *path)
>   	return;
>   
>   path_failed:
> +	coresight_pm_device_save(sink);
> +
>   	pr_err("Failed in coresight PM restore on CPU%d: %d\n",
>   	       smp_processor_id(), ret);
>   
> 

Re: [PATCH v10 16/20] coresight: Add PM callbacks for sink device

[James Clark]

On 09/04/2026 11:52 am, James Clark wrote:
> 
> 
> On 05/04/2026 4:02 pm, Leo Yan wrote:
>> Unlike system level sinks, per-CPU sinks may lose power during CPU idle
>> states.  Currently, this applies specifically to TRBE.  This commit
>> invokes save and restore callbacks for the sink in the CPU PM notifier.
>>
>> If the sink provides PM callbacks but the source does not, this is
>> unsafe because the sink cannot be disabled safely unless the source
>> can also be controlled, so veto low power entry to avoid lockups.
>>
>> Tested-by: James Clark <james.clark@linaro.org>
>> Reviewed-by: Yeoreum Yun <yeoreum.yun@arm.com>
>> Reviewed-by: James Clark <james.clark@linaro.org>
>> Signed-off-by: Leo Yan <leo.yan@arm.com>
>> ---
>>   drivers/hwtracing/coresight/coresight-core.c | 46 ++++++++++++++++++ 
>> ++++++++--
>>   1 file changed, 43 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/ 
>> hwtracing/coresight/coresight-core.c
>> index 
>> c1e8debc76aba7eb5ecf7efe2a3b9b8b3e11b10c..a918bf6398a932de30fe9b4947020cc4c1cfb2f7 100644
>> --- a/drivers/hwtracing/coresight/coresight-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>> @@ -1736,14 +1736,15 @@ static void coresight_release_device_list(void)
>>   /* Return: 1 if PM is required, 0 if skip, <0 on error */
>>   static int coresight_pm_check(struct coresight_path *path)
>>   {
>> -    struct coresight_device *source;
>> -    bool source_has_cb;
>> +    struct coresight_device *source, *sink;
>> +    bool source_has_cb, sink_has_cb;
>>       if (!path)
>>           return 0;
>>       source = coresight_get_source(path);
>> -    if (!source)
>> +    sink = coresight_get_sink(path);
>> +    if (!source || !sink)
>>           return 0;
>>       /* Don't save and restore if the source is inactive */
>> @@ -1759,16 +1760,36 @@ static int coresight_pm_check(struct 
>> coresight_path *path)
>>       if (source_has_cb)
>>           return 1;
>> +    sink_has_cb = coresight_ops(sink)->pm_save_disable &&
>> +              coresight_ops(sink)->pm_restore_enable;
>> +    /*
>> +     * It is not permitted that the source has no callbacks while the 
>> sink
>> +     * does, as the sink cannot be disabled without disabling the 
>> source,
>> +     * which may lead to lockups. Alternatively, the ETM driver should
>> +     * enable self-hosted PM mode at probe (see etm4_probe()).
>> +     */
>> +    if (sink_has_cb) {
>> +        pr_warn_once("coresight PM failed: source has no PM callbacks; "
>> +                 "cannot safely control sink\n");
> 
> This prints out on my Orion board on a fresh boot because of how 
> pm_save_enable is setup there. Do we really need the configuration of 
> pm_save_enable for ETE/TRBE if we know that it always needs saving?
> 
> It also stops warning if I rmmod and modprobe the module after booting. 
> Seems like pm_save_enable is different depending on how the module is 
> loaded which doesn't seem right.
> 
>> +        return -EINVAL;
>> +    }
>> +
>>       return 0;
>>   }
>>   static int coresight_pm_device_save(struct coresight_device *csdev)
>>   {
>> +    if (!csdev || !coresight_ops(csdev)->pm_save_disable)
>> +        return 0;
>> +
>>       return coresight_ops(csdev)->pm_save_disable(csdev);
>>   }
>>   static void coresight_pm_device_restore(struct coresight_device *csdev)
>>   {
>> +    if (!csdev || !coresight_ops(csdev)->pm_restore_enable)
>> +        return;
>> +
>>       coresight_ops(csdev)->pm_restore_enable(csdev);
>>   }
>> @@ -1787,15 +1808,32 @@ static int coresight_pm_save(struct 
>> coresight_path *path)
>>       to = list_prev_entry(coresight_path_last_node(path), link);
>>       coresight_disable_path_from_to(path, from, to);
>> +    ret = coresight_pm_device_save(coresight_get_sink(path));
>> +    if (ret)
>> +        goto sink_failed;
>> +
> 
> The comment directly above this says "Up to the node before sink to 
> avoid latency". But then this line goes and saves the sink anyway. So 
> I'm not sure what's meant by the comment?
> 
>>       return 0;
>> +
>> +sink_failed:
>> +    if (!coresight_enable_path_from_to(path, coresight_get_mode(source),
>> +                       from, to))
>> +        coresight_pm_device_restore(source);
>> +
>> +    pr_err("Failed in coresight PM save on CPU%d: %d\n",
>> +           smp_processor_id(), ret);
>> +    this_cpu_write(percpu_pm_failed, true);
> 
> Why does only a failing sink set percpu_pm_failed when failing to save 
> the source exits early. Sashiko has a similar comment that this could 
> result in restoring uninitialised source save data later, but a comment 
> in this function about why the flow is like this would be helpful.
> 
> We have coresight_disable_path_from_to() which always succeeds and 
> doesn't return an error. TRBE is the only sink with a pm_save_disable()
> callback, but it always succeeds anyway.
> 
> Would it not be much simpler to require that sink save/restore callbacks 
> always succeed and don't return anything? Seems like this 
> percpu_pm_failed stuff is extra complexity for a scenario that doesn't 
> exist? The only thing that can fail is saving the source but it doesn't 
> goto sink_failed when that happens.
> 
> Ideally etm4_cpu_save() wouldn't have a return value either. It would be 
> good if we could find away to skip or ignore the timeouts in there 
> somehow because that's the only reason it can fail.
> 

etm4_disable_trace_unit() and etm4_enable_hw() also have timeouts but 
the timeouts only print warnings, they don't cause early exits or return 
error values.

Can we do the same for etm4_cpu_save()? It would be better to read and 
restore potentially 'unstable' register values than to early exit and 
restore known uninitialised values as it is here.

>> +    return ret;
>>   }
>>   static void coresight_pm_restore(struct coresight_path *path)
>>   {
>>       struct coresight_device *source = coresight_get_source(path);
>> +    struct coresight_device *sink = coresight_get_sink(path);
>>       struct coresight_node *from, *to;
>>       int ret;
>> +    coresight_pm_device_restore(sink);
>> +
>>       from = coresight_path_first_node(path);
>>       /* Up to the node before sink to avoid latency */
>>       to = list_prev_entry(coresight_path_last_node(path), link);
>> @@ -1808,6 +1846,8 @@ static void coresight_pm_restore(struct 
>> coresight_path *path)
>>       return;
>>   path_failed:
>> +    coresight_pm_device_save(sink);
>> +
>>       pr_err("Failed in coresight PM restore on CPU%d: %d\n",
>>              smp_processor_id(), ret);
>>
> 

Re: [PATCH v10 16/20] coresight: Add PM callbacks for sink device

[Suzuki K Poulose]

On 09/04/2026 11:52, James Clark wrote:
> 
> 
> On 05/04/2026 4:02 pm, Leo Yan wrote:
>> Unlike system level sinks, per-CPU sinks may lose power during CPU idle
>> states.  Currently, this applies specifically to TRBE.  This commit
>> invokes save and restore callbacks for the sink in the CPU PM notifier.
>>
>> If the sink provides PM callbacks but the source does not, this is
>> unsafe because the sink cannot be disabled safely unless the source
>> can also be controlled, so veto low power entry to avoid lockups.
>>
>> Tested-by: James Clark <james.clark@linaro.org>
>> Reviewed-by: Yeoreum Yun <yeoreum.yun@arm.com>
>> Reviewed-by: James Clark <james.clark@linaro.org>
>> Signed-off-by: Leo Yan <leo.yan@arm.com>
>> ---
>>   drivers/hwtracing/coresight/coresight-core.c | 46 ++++++++++++++++++ 
>> ++++++++--
>>   1 file changed, 43 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/ 
>> hwtracing/coresight/coresight-core.c
>> index 
>> c1e8debc76aba7eb5ecf7efe2a3b9b8b3e11b10c..a918bf6398a932de30fe9b4947020cc4c1cfb2f7 100644
>> --- a/drivers/hwtracing/coresight/coresight-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>> @@ -1736,14 +1736,15 @@ static void coresight_release_device_list(void)
>>   /* Return: 1 if PM is required, 0 if skip, <0 on error */
>>   static int coresight_pm_check(struct coresight_path *path)
>>   {
>> -    struct coresight_device *source;
>> -    bool source_has_cb;
>> +    struct coresight_device *source, *sink;
>> +    bool source_has_cb, sink_has_cb;
>>       if (!path)
>>           return 0;
>>       source = coresight_get_source(path);
>> -    if (!source)
>> +    sink = coresight_get_sink(path);
>> +    if (!source || !sink)
>>           return 0;
>>       /* Don't save and restore if the source is inactive */
>> @@ -1759,16 +1760,36 @@ static int coresight_pm_check(struct 
>> coresight_path *path)
>>       if (source_has_cb)
>>           return 1;
>> +    sink_has_cb = coresight_ops(sink)->pm_save_disable &&
>> +              coresight_ops(sink)->pm_restore_enable;
>> +    /*
>> +     * It is not permitted that the source has no callbacks while the 
>> sink
>> +     * does, as the sink cannot be disabled without disabling the 
>> source,
>> +     * which may lead to lockups. Alternatively, the ETM driver should
>> +     * enable self-hosted PM mode at probe (see etm4_probe()).
>> +     */
>> +    if (sink_has_cb) {
>> +        pr_warn_once("coresight PM failed: source has no PM callbacks; "
>> +                 "cannot safely control sink\n");
> 
> This prints out on my Orion board on a fresh boot because of how 
> pm_save_enable is setup there. Do we really need the configuration of 
> pm_save_enable for ETE/TRBE if we know that it always needs saving?
> 
> It also stops warning if I rmmod and modprobe the module after booting. 
> Seems like pm_save_enable is different depending on how the module is 
> loaded which doesn't seem right.

Thats because the warning is pr_warn_*once*()

Suzuki


> 
>> +        return -EINVAL;
>> +    }
>> +
>>       return 0;
>>   }
>>   static int coresight_pm_device_save(struct coresight_device *csdev)
>>   {
>> +    if (!csdev || !coresight_ops(csdev)->pm_save_disable)
>> +        return 0;
>> +
>>       return coresight_ops(csdev)->pm_save_disable(csdev);
>>   }
>>   static void coresight_pm_device_restore(struct coresight_device *csdev)
>>   {
>> +    if (!csdev || !coresight_ops(csdev)->pm_restore_enable)
>> +        return;
>> +
>>       coresight_ops(csdev)->pm_restore_enable(csdev);
>>   }
>> @@ -1787,15 +1808,32 @@ static int coresight_pm_save(struct 
>> coresight_path *path)
>>       to = list_prev_entry(coresight_path_last_node(path), link);
>>       coresight_disable_path_from_to(path, from, to);
>> +    ret = coresight_pm_device_save(coresight_get_sink(path));
>> +    if (ret)
>> +        goto sink_failed;
>> +
> 
> The comment directly above this says "Up to the node before sink to 
> avoid latency". But then this line goes and saves the sink anyway. So 
> I'm not sure what's meant by the comment?
> 
>>       return 0;
>> +
>> +sink_failed:
>> +    if (!coresight_enable_path_from_to(path, coresight_get_mode(source),
>> +                       from, to))
>> +        coresight_pm_device_restore(source);
>> +
>> +    pr_err("Failed in coresight PM save on CPU%d: %d\n",
>> +           smp_processor_id(), ret);
>> +    this_cpu_write(percpu_pm_failed, true);
> 
> Why does only a failing sink set percpu_pm_failed when failing to save 
> the source exits early. Sashiko has a similar comment that this could 
> result in restoring uninitialised source save data later, but a comment 
> in this function about why the flow is like this would be helpful.
> 
> We have coresight_disable_path_from_to() which always succeeds and 
> doesn't return an error. TRBE is the only sink with a pm_save_disable()
> callback, but it always succeeds anyway.
> 
> Would it not be much simpler to require that sink save/restore callbacks 
> always succeed and don't return anything? Seems like this 
> percpu_pm_failed stuff is extra complexity for a scenario that doesn't 
> exist? The only thing that can fail is saving the source but it doesn't 
> goto sink_failed when that happens.
> 
> Ideally etm4_cpu_save() wouldn't have a return value either. It would be 
> good if we could find away to skip or ignore the timeouts in there 
> somehow because that's the only reason it can fail.
> 
>> +    return ret;
>>   }
>>   static void coresight_pm_restore(struct coresight_path *path)
>>   {
>>       struct coresight_device *source = coresight_get_source(path);
>> +    struct coresight_device *sink = coresight_get_sink(path);
>>       struct coresight_node *from, *to;
>>       int ret;
>> +    coresight_pm_device_restore(sink);
>> +
>>       from = coresight_path_first_node(path);
>>       /* Up to the node before sink to avoid latency */
>>       to = list_prev_entry(coresight_path_last_node(path), link);
>> @@ -1808,6 +1846,8 @@ static void coresight_pm_restore(struct 
>> coresight_path *path)
>>       return;
>>   path_failed:
>> +    coresight_pm_device_save(sink);
>> +
>>       pr_err("Failed in coresight PM restore on CPU%d: %d\n",
>>              smp_processor_id(), ret);
>>
> 

Re: [PATCH v10 16/20] coresight: Add PM callbacks for sink device

[James Clark]

On 09/04/2026 2:14 pm, Suzuki K Poulose wrote:
> On 09/04/2026 11:52, James Clark wrote:
>>
>>
>> On 05/04/2026 4:02 pm, Leo Yan wrote:
>>> Unlike system level sinks, per-CPU sinks may lose power during CPU idle
>>> states.  Currently, this applies specifically to TRBE.  This commit
>>> invokes save and restore callbacks for the sink in the CPU PM notifier.
>>>
>>> If the sink provides PM callbacks but the source does not, this is
>>> unsafe because the sink cannot be disabled safely unless the source
>>> can also be controlled, so veto low power entry to avoid lockups.
>>>
>>> Tested-by: James Clark <james.clark@linaro.org>
>>> Reviewed-by: Yeoreum Yun <yeoreum.yun@arm.com>
>>> Reviewed-by: James Clark <james.clark@linaro.org>
>>> Signed-off-by: Leo Yan <leo.yan@arm.com>
>>> ---
>>>   drivers/hwtracing/coresight/coresight-core.c | 46 +++++++++++++++++ 
>>> + ++++++++--
>>>   1 file changed, 43 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/ 
>>> hwtracing/coresight/coresight-core.c
>>> index 
>>> c1e8debc76aba7eb5ecf7efe2a3b9b8b3e11b10c..a918bf6398a932de30fe9b4947020cc4c1cfb2f7 100644
>>> --- a/drivers/hwtracing/coresight/coresight-core.c
>>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>>> @@ -1736,14 +1736,15 @@ static void coresight_release_device_list(void)
>>>   /* Return: 1 if PM is required, 0 if skip, <0 on error */
>>>   static int coresight_pm_check(struct coresight_path *path)
>>>   {
>>> -    struct coresight_device *source;
>>> -    bool source_has_cb;
>>> +    struct coresight_device *source, *sink;
>>> +    bool source_has_cb, sink_has_cb;
>>>       if (!path)
>>>           return 0;
>>>       source = coresight_get_source(path);
>>> -    if (!source)
>>> +    sink = coresight_get_sink(path);
>>> +    if (!source || !sink)
>>>           return 0;
>>>       /* Don't save and restore if the source is inactive */
>>> @@ -1759,16 +1760,36 @@ static int coresight_pm_check(struct 
>>> coresight_path *path)
>>>       if (source_has_cb)
>>>           return 1;
>>> +    sink_has_cb = coresight_ops(sink)->pm_save_disable &&
>>> +              coresight_ops(sink)->pm_restore_enable;
>>> +    /*
>>> +     * It is not permitted that the source has no callbacks while 
>>> the sink
>>> +     * does, as the sink cannot be disabled without disabling the 
>>> source,
>>> +     * which may lead to lockups. Alternatively, the ETM driver should
>>> +     * enable self-hosted PM mode at probe (see etm4_probe()).
>>> +     */
>>> +    if (sink_has_cb) {
>>> +        pr_warn_once("coresight PM failed: source has no PM 
>>> callbacks; "
>>> +                 "cannot safely control sink\n");
>>
>> This prints out on my Orion board on a fresh boot because of how 
>> pm_save_enable is setup there. Do we really need the configuration of 
>> pm_save_enable for ETE/TRBE if we know that it always needs saving?
>>
>> It also stops warning if I rmmod and modprobe the module after 
>> booting. Seems like pm_save_enable is different depending on how the 
>> module is loaded which doesn't seem right.
> 
> Thats because the warning is pr_warn_*once*()
> 
> Suzuki
> 
> 

I don't think so, I tested it with a printf instead of a warn once and 
also tested modprobeing straight after a reboot.

>>
>>> +        return -EINVAL;
>>> +    }
>>> +
>>>       return 0;
>>>   }
>>>   static int coresight_pm_device_save(struct coresight_device *csdev)
>>>   {
>>> +    if (!csdev || !coresight_ops(csdev)->pm_save_disable)
>>> +        return 0;
>>> +
>>>       return coresight_ops(csdev)->pm_save_disable(csdev);
>>>   }
>>>   static void coresight_pm_device_restore(struct coresight_device 
>>> *csdev)
>>>   {
>>> +    if (!csdev || !coresight_ops(csdev)->pm_restore_enable)
>>> +        return;
>>> +
>>>       coresight_ops(csdev)->pm_restore_enable(csdev);
>>>   }
>>> @@ -1787,15 +1808,32 @@ static int coresight_pm_save(struct 
>>> coresight_path *path)
>>>       to = list_prev_entry(coresight_path_last_node(path), link);
>>>       coresight_disable_path_from_to(path, from, to);
>>> +    ret = coresight_pm_device_save(coresight_get_sink(path));
>>> +    if (ret)
>>> +        goto sink_failed;
>>> +
>>
>> The comment directly above this says "Up to the node before sink to 
>> avoid latency". But then this line goes and saves the sink anyway. So 
>> I'm not sure what's meant by the comment?
>>
>>>       return 0;
>>> +
>>> +sink_failed:
>>> +    if (!coresight_enable_path_from_to(path, 
>>> coresight_get_mode(source),
>>> +                       from, to))
>>> +        coresight_pm_device_restore(source);
>>> +
>>> +    pr_err("Failed in coresight PM save on CPU%d: %d\n",
>>> +           smp_processor_id(), ret);
>>> +    this_cpu_write(percpu_pm_failed, true);
>>
>> Why does only a failing sink set percpu_pm_failed when failing to save 
>> the source exits early. Sashiko has a similar comment that this could 
>> result in restoring uninitialised source save data later, but a 
>> comment in this function about why the flow is like this would be 
>> helpful.
>>
>> We have coresight_disable_path_from_to() which always succeeds and 
>> doesn't return an error. TRBE is the only sink with a pm_save_disable()
>> callback, but it always succeeds anyway.
>>
>> Would it not be much simpler to require that sink save/restore 
>> callbacks always succeed and don't return anything? Seems like this 
>> percpu_pm_failed stuff is extra complexity for a scenario that doesn't 
>> exist? The only thing that can fail is saving the source but it 
>> doesn't goto sink_failed when that happens.
>>
>> Ideally etm4_cpu_save() wouldn't have a return value either. It would 
>> be good if we could find away to skip or ignore the timeouts in there 
>> somehow because that's the only reason it can fail.
>>
>>> +    return ret;
>>>   }
>>>   static void coresight_pm_restore(struct coresight_path *path)
>>>   {
>>>       struct coresight_device *source = coresight_get_source(path);
>>> +    struct coresight_device *sink = coresight_get_sink(path);
>>>       struct coresight_node *from, *to;
>>>       int ret;
>>> +    coresight_pm_device_restore(sink);
>>> +
>>>       from = coresight_path_first_node(path);
>>>       /* Up to the node before sink to avoid latency */
>>>       to = list_prev_entry(coresight_path_last_node(path), link);
>>> @@ -1808,6 +1846,8 @@ static void coresight_pm_restore(struct 
>>> coresight_path *path)
>>>       return;
>>>   path_failed:
>>> +    coresight_pm_device_save(sink);
>>> +
>>>       pr_err("Failed in coresight PM restore on CPU%d: %d\n",
>>>              smp_processor_id(), ret);
>>>
>>
> 

Re: [PATCH v10 16/20] coresight: Add PM callbacks for sink device

[James Clark]

On 09/04/2026 3:30 pm, James Clark wrote:
> 
> 
> On 09/04/2026 2:14 pm, Suzuki K Poulose wrote:
>> On 09/04/2026 11:52, James Clark wrote:
>>>
>>>
>>> On 05/04/2026 4:02 pm, Leo Yan wrote:
>>>> Unlike system level sinks, per-CPU sinks may lose power during CPU idle
>>>> states.  Currently, this applies specifically to TRBE.  This commit
>>>> invokes save and restore callbacks for the sink in the CPU PM notifier.
>>>>
>>>> If the sink provides PM callbacks but the source does not, this is
>>>> unsafe because the sink cannot be disabled safely unless the source
>>>> can also be controlled, so veto low power entry to avoid lockups.
>>>>
>>>> Tested-by: James Clark <james.clark@linaro.org>
>>>> Reviewed-by: Yeoreum Yun <yeoreum.yun@arm.com>
>>>> Reviewed-by: James Clark <james.clark@linaro.org>
>>>> Signed-off-by: Leo Yan <leo.yan@arm.com>
>>>> ---
>>>>   drivers/hwtracing/coresight/coresight-core.c | 46 ++++++++++++++++ 
>>>> + + ++++++++--
>>>>   1 file changed, 43 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/ 
>>>> hwtracing/coresight/coresight-core.c
>>>> index 
>>>> c1e8debc76aba7eb5ecf7efe2a3b9b8b3e11b10c..a918bf6398a932de30fe9b4947020cc4c1cfb2f7 100644
>>>> --- a/drivers/hwtracing/coresight/coresight-core.c
>>>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>>>> @@ -1736,14 +1736,15 @@ static void coresight_release_device_list(void)
>>>>   /* Return: 1 if PM is required, 0 if skip, <0 on error */
>>>>   static int coresight_pm_check(struct coresight_path *path)
>>>>   {
>>>> -    struct coresight_device *source;
>>>> -    bool source_has_cb;
>>>> +    struct coresight_device *source, *sink;
>>>> +    bool source_has_cb, sink_has_cb;
>>>>       if (!path)
>>>>           return 0;
>>>>       source = coresight_get_source(path);
>>>> -    if (!source)
>>>> +    sink = coresight_get_sink(path);
>>>> +    if (!source || !sink)
>>>>           return 0;
>>>>       /* Don't save and restore if the source is inactive */
>>>> @@ -1759,16 +1760,36 @@ static int coresight_pm_check(struct 
>>>> coresight_path *path)
>>>>       if (source_has_cb)
>>>>           return 1;
>>>> +    sink_has_cb = coresight_ops(sink)->pm_save_disable &&
>>>> +              coresight_ops(sink)->pm_restore_enable;
>>>> +    /*
>>>> +     * It is not permitted that the source has no callbacks while 
>>>> the sink
>>>> +     * does, as the sink cannot be disabled without disabling the 
>>>> source,
>>>> +     * which may lead to lockups. Alternatively, the ETM driver should
>>>> +     * enable self-hosted PM mode at probe (see etm4_probe()).
>>>> +     */
>>>> +    if (sink_has_cb) {
>>>> +        pr_warn_once("coresight PM failed: source has no PM 
>>>> callbacks; "
>>>> +                 "cannot safely control sink\n");
>>>
>>> This prints out on my Orion board on a fresh boot because of how 
>>> pm_save_enable is setup there. Do we really need the configuration of 
>>> pm_save_enable for ETE/TRBE if we know that it always needs saving?
>>>
>>> It also stops warning if I rmmod and modprobe the module after 
>>> booting. Seems like pm_save_enable is different depending on how the 
>>> module is loaded which doesn't seem right.
>>
>> Thats because the warning is pr_warn_*once*()
>>
>> Suzuki
>>
>>
> 
> I don't think so, I tested it with a printf instead of a warn once and 
> also tested modprobeing straight after a reboot.
> 

I also printed the pointers to the callbacks and they really do change.

>>>
>>>> +        return -EINVAL;
>>>> +    }
>>>> +
>>>>       return 0;
>>>>   }
>>>>   static int coresight_pm_device_save(struct coresight_device *csdev)
>>>>   {
>>>> +    if (!csdev || !coresight_ops(csdev)->pm_save_disable)
>>>> +        return 0;
>>>> +
>>>>       return coresight_ops(csdev)->pm_save_disable(csdev);
>>>>   }
>>>>   static void coresight_pm_device_restore(struct coresight_device 
>>>> *csdev)
>>>>   {
>>>> +    if (!csdev || !coresight_ops(csdev)->pm_restore_enable)
>>>> +        return;
>>>> +
>>>>       coresight_ops(csdev)->pm_restore_enable(csdev);
>>>>   }
>>>> @@ -1787,15 +1808,32 @@ static int coresight_pm_save(struct 
>>>> coresight_path *path)
>>>>       to = list_prev_entry(coresight_path_last_node(path), link);
>>>>       coresight_disable_path_from_to(path, from, to);
>>>> +    ret = coresight_pm_device_save(coresight_get_sink(path));
>>>> +    if (ret)
>>>> +        goto sink_failed;
>>>> +
>>>
>>> The comment directly above this says "Up to the node before sink to 
>>> avoid latency". But then this line goes and saves the sink anyway. So 
>>> I'm not sure what's meant by the comment?
>>>
>>>>       return 0;
>>>> +
>>>> +sink_failed:
>>>> +    if (!coresight_enable_path_from_to(path, 
>>>> coresight_get_mode(source),
>>>> +                       from, to))
>>>> +        coresight_pm_device_restore(source);
>>>> +
>>>> +    pr_err("Failed in coresight PM save on CPU%d: %d\n",
>>>> +           smp_processor_id(), ret);
>>>> +    this_cpu_write(percpu_pm_failed, true);
>>>
>>> Why does only a failing sink set percpu_pm_failed when failing to 
>>> save the source exits early. Sashiko has a similar comment that this 
>>> could result in restoring uninitialised source save data later, but a 
>>> comment in this function about why the flow is like this would be 
>>> helpful.
>>>
>>> We have coresight_disable_path_from_to() which always succeeds and 
>>> doesn't return an error. TRBE is the only sink with a pm_save_disable()
>>> callback, but it always succeeds anyway.
>>>
>>> Would it not be much simpler to require that sink save/restore 
>>> callbacks always succeed and don't return anything? Seems like this 
>>> percpu_pm_failed stuff is extra complexity for a scenario that 
>>> doesn't exist? The only thing that can fail is saving the source but 
>>> it doesn't goto sink_failed when that happens.
>>>
>>> Ideally etm4_cpu_save() wouldn't have a return value either. It would 
>>> be good if we could find away to skip or ignore the timeouts in there 
>>> somehow because that's the only reason it can fail.
>>>
>>>> +    return ret;
>>>>   }
>>>>   static void coresight_pm_restore(struct coresight_path *path)
>>>>   {
>>>>       struct coresight_device *source = coresight_get_source(path);
>>>> +    struct coresight_device *sink = coresight_get_sink(path);
>>>>       struct coresight_node *from, *to;
>>>>       int ret;
>>>> +    coresight_pm_device_restore(sink);
>>>> +
>>>>       from = coresight_path_first_node(path);
>>>>       /* Up to the node before sink to avoid latency */
>>>>       to = list_prev_entry(coresight_path_last_node(path), link);
>>>> @@ -1808,6 +1846,8 @@ static void coresight_pm_restore(struct 
>>>> coresight_path *path)
>>>>       return;
>>>>   path_failed:
>>>> +    coresight_pm_device_save(sink);
>>>> +
>>>>       pr_err("Failed in coresight PM restore on CPU%d: %d\n",
>>>>              smp_processor_id(), ret);
>>>>
>>>
>>
> 

Re: [PATCH v10 16/20] coresight: Add PM callbacks for sink device

[Leo Yan]

On Thu, Apr 09, 2026 at 03:30:56PM +0100, James Clark wrote:

[...]

> > > > @@ -1759,16 +1760,36 @@ static int coresight_pm_check(struct
> > > > coresight_path *path)
> > > >       if (source_has_cb)
> > > >           return 1;
> > > > +    sink_has_cb = coresight_ops(sink)->pm_save_disable &&
> > > > +              coresight_ops(sink)->pm_restore_enable;
> > > > +    /*
> > > > +     * It is not permitted that the source has no callbacks
> > > > while the sink
> > > > +     * does, as the sink cannot be disabled without disabling
> > > > the source,
> > > > +     * which may lead to lockups. Alternatively, the ETM driver should
> > > > +     * enable self-hosted PM mode at probe (see etm4_probe()).
> > > > +     */
> > > > +    if (sink_has_cb) {
> > > > +        pr_warn_once("coresight PM failed: source has no PM
> > > > callbacks; "
> > > > +                 "cannot safely control sink\n");
> > > 
> > > This prints out on my Orion board on a fresh boot because of how
> > > pm_save_enable is setup there. Do we really need the configuration
> > > of pm_save_enable for ETE/TRBE if we know that it always needs
> > > saving?

Yeah, I can remove this check and always bind CPU PM ops for ETE.

> > > It also stops warning if I rmmod and modprobe the module after
> > > booting. Seems like pm_save_enable is different depending on how the
> > > module is loaded which doesn't seem right.
> > 
> > Thats because the warning is pr_warn_*once*()
> 
> I don't think so, I tested it with a printf instead of a warn once and also
> tested modprobeing straight after a reboot.

I am a bit surprised that Orion6 hits the CPU idle flow, as I observed
that idle states are not enabled on my board:

  # ls /sys/devices/system/cpu/cpu*/cpuidle
  ls: cannot access '/sys/devices/system/cpu/cpu*/cpuidle': No such file or directory

If you hit only once CPU idle notifier, it is good to add a
dump_stack() in coresight_cpu_pm_notify and print the "cmd" argument,
so we can know the calling coming from where.  I am a bit suspect it
might be a glitch in CPUIdle layer.

Thanks,
Leo

Re: [PATCH v10 16/20] coresight: Add PM callbacks for sink device

[Leo Yan]

On Thu, Apr 09, 2026 at 11:52:00AM +0100, James Clark wrote:

[...]

> > @@ -1787,15 +1808,32 @@ static int coresight_pm_save(struct coresight_path *path)
> >   	to = list_prev_entry(coresight_path_last_node(path), link);
> >   	coresight_disable_path_from_to(path, from, to);
> > +	ret = coresight_pm_device_save(coresight_get_sink(path));
> > +	if (ret)
> > +		goto sink_failed;
> > +
> 
> The comment directly above this says "Up to the node before sink to avoid
> latency". But then this line goes and saves the sink anyway. So I'm not sure
> what's meant by the comment?

I will refine the comment to: "Control the path up to the node before
the _system_ sink to avoid latency caused by memory copying; afterwards,
saves context if the sink provides a callback".


> >   	return 0;
> > +
> > +sink_failed:
> > +	if (!coresight_enable_path_from_to(path, coresight_get_mode(source),
> > +					   from, to))
> > +		coresight_pm_device_restore(source);
> > +
> > +	pr_err("Failed in coresight PM save on CPU%d: %d\n",
> > +	       smp_processor_id(), ret);
> > +	this_cpu_write(percpu_pm_failed, true);
> 
> Why does only a failing sink set percpu_pm_failed when failing to save the
> source exits early.

My purpose is to use "percpu_pm_failed == true" to prevent further
issues if we already know the link has failed.  To be clear, this is not
a recovery mechanism; it simply means "if link enabling or disabling
fails in CPU PM, do not continue to avoid potential hardware lockups.”

The source save failure is a bit special, it fails at the beginning of
the CoreSight CPU PM flow, and the returned error prevents the CPU idle
flow from proceeding.  As a result, there is nothing further to do.

> Sashiko has a similar comment that this could result in
> restoring uninitialised source save data later, but a comment in this
> function about why the flow is like this would be helpful.

The comment of "Restoring uninitialised source save data later" does not
make sense.

When the save fails, the notifier only rolls back the callbacks that ran
before the failing callback by invoking CPU_PM_ENTER_FAILED.  And there
is no CPU_PM_EXIT, as the idle state is never entered.

> We have coresight_disable_path_from_to() which always succeeds and doesn't
> return an error. TRBE is the only sink with a pm_save_disable()
> callback, but it always succeeds anyway.
> 
> Would it not be much simpler to require that sink save/restore callbacks
> always succeed and don't return anything? Seems like this percpu_pm_failed
> stuff is extra complexity for a scenario that doesn't exist? The only thing
> that can fail is saving the source but it doesn't goto sink_failed when that
> happens.

As you suggested, I can simplify the code with assuming sink ops
always success, but we might expect complaints from Sashiko.

> Ideally etm4_cpu_save() wouldn't have a return value either. It would be
> good if we could find away to skip or ignore the timeouts in there somehow
> because that's the only reason it can fail.

Seems to me, ETMv4 has a timeout log and returns error is not bad, as
it can be helpful to locate issue in a cheap way (sometimes, hang
issue caused by lockup might be a time-consumed debugging).

Thanks,
Leo

[PATCH v10 17/20] coresight: trbe: Save and restore state across CPU low power state

[Leo Yan]

From: Yabin Cui <yabinc@google.com>

TRBE context can be lost when a CPU enters low power states.  If a trace
source is restored while TRBE is not, tracing may run without an active
sink, which can lead to hangs on some devices (e.g., Pixel 9).

The save and restore flows are described in the section K5.5 "Context
switching" of Arm ARM (ARM DDI 0487 L.a). This commit adds save and
restore callbacks with following the software usages defined in the
architecture manual.

During the restore flow, since TRBLIMITR_EL1.E resets to 0 on a warm
reset, the trace buffer unit is disabled when idle resume, it is safe to
restore base/pointer/status registers first and program TRBLIMITR_EL1
last.

Signed-off-by: Yabin Cui <yabinc@google.com>
Tested-by: James Clark <james.clark@linaro.org>
Reviewed-by: Yeoreum Yun <yeoreum.yun@arm.com>
Reviewed-by: James Clark <james.clark@linaro.org>
Co-developed-by: Leo Yan <leo.yan@arm.com>
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-trbe.c | 59 +++++++++++++++++++++++++++-
 1 file changed, 58 insertions(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
index 14e35b9660d76e47619cc6026b94929b3bb3e02b..c7cbca45f2debd4047b93283ea9fe5dd9e1f2ebf 100644
--- a/drivers/hwtracing/coresight/coresight-trbe.c
+++ b/drivers/hwtracing/coresight/coresight-trbe.c
@@ -116,6 +116,20 @@ static int trbe_errata_cpucaps[] = {
  */
 #define TRBE_WORKAROUND_OVERWRITE_FILL_MODE_SKIP_BYTES	256
 
+/*
+ * struct trbe_save_state: Register values representing TRBE state
+ * @trblimitr		- Trace Buffer Limit Address Register value
+ * @trbbaser		- Trace Buffer Base Register value
+ * @trbptr		- Trace Buffer Write Pointer Register value
+ * @trbsr		- Trace Buffer Status Register value
+ */
+struct trbe_save_state {
+	u64 trblimitr;
+	u64 trbbaser;
+	u64 trbptr;
+	u64 trbsr;
+};
+
 /*
  * struct trbe_cpudata: TRBE instance specific data
  * @trbe_flag		- TRBE dirty/access flag support
@@ -134,6 +148,7 @@ struct trbe_cpudata {
 	enum cs_mode mode;
 	struct trbe_buf *buf;
 	struct trbe_drvdata *drvdata;
+	struct trbe_save_state save_state;
 	DECLARE_BITMAP(errata, TRBE_ERRATA_MAX);
 };
 
@@ -1189,6 +1204,46 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)
 	return IRQ_HANDLED;
 }
 
+static int arm_trbe_save(struct coresight_device *csdev)
+{
+	struct trbe_cpudata *cpudata = dev_get_drvdata(&csdev->dev);
+	struct trbe_save_state *state = &cpudata->save_state;
+
+	state->trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
+
+	/* Disable the unit, ensure the writes to memory are complete */
+	if (state->trblimitr & TRBLIMITR_EL1_E)
+		trbe_drain_and_disable_local(cpudata);
+
+	state->trbbaser = read_sysreg_s(SYS_TRBBASER_EL1);
+	state->trbptr = read_sysreg_s(SYS_TRBPTR_EL1);
+	state->trbsr = read_sysreg_s(SYS_TRBSR_EL1);
+	return 0;
+}
+
+static void arm_trbe_restore(struct coresight_device *csdev)
+{
+	struct trbe_cpudata *cpudata = dev_get_drvdata(&csdev->dev);
+	struct trbe_save_state *state = &cpudata->save_state;
+
+	write_sysreg_s(state->trbbaser, SYS_TRBBASER_EL1);
+	write_sysreg_s(state->trbptr, SYS_TRBPTR_EL1);
+	write_sysreg_s(state->trbsr, SYS_TRBSR_EL1);
+
+	if (!(state->trblimitr & TRBLIMITR_EL1_E)) {
+		write_sysreg_s(state->trblimitr, SYS_TRBLIMITR_EL1);
+	} else {
+		/*
+		 * The section K5.5 Context switching, Arm ARM (ARM DDI 0487
+		 * L.a), S_PKLXF requires a Context synchronization event to
+		 * guarantee the Trace Buffer Unit will observe the new values
+		 * of the system registers.
+		 */
+		isb();
+		set_trbe_enabled(cpudata, state->trblimitr);
+	}
+}
+
 static const struct coresight_ops_sink arm_trbe_sink_ops = {
 	.enable		= arm_trbe_enable,
 	.disable	= arm_trbe_disable,
@@ -1198,7 +1253,9 @@ static const struct coresight_ops_sink arm_trbe_sink_ops = {
 };
 
 static const struct coresight_ops arm_trbe_cs_ops = {
-	.sink_ops	= &arm_trbe_sink_ops,
+	.pm_save_disable	= arm_trbe_save,
+	.pm_restore_enable	= arm_trbe_restore,
+	.sink_ops		= &arm_trbe_sink_ops,
 };
 
 static ssize_t align_show(struct device *dev, struct device_attribute *attr, char *buf)

-- 
2.34.1

Re: [PATCH v10 17/20] coresight: trbe: Save and restore state across CPU low power state

[James Clark]

On 05/04/2026 4:02 pm, Leo Yan wrote:
> From: Yabin Cui <yabinc@google.com>
> 
> TRBE context can be lost when a CPU enters low power states.  If a trace
> source is restored while TRBE is not, tracing may run without an active
> sink, which can lead to hangs on some devices (e.g., Pixel 9).

Can't this still happen if saving the source times out on "wait for 
TRCSTATR.IDLE to go up"?

That would make coresight_pm_save() exit early, not saving the active 
TRBE state. Then when coresight_pm_restore() is called it restores a 
stale inactive sink, then enables the source again which is the state 
that can hang.

> 
> The save and restore flows are described in the section K5.5 "Context
> switching" of Arm ARM (ARM DDI 0487 L.a). This commit adds save and
> restore callbacks with following the software usages defined in the
> architecture manual.
> 
> During the restore flow, since TRBLIMITR_EL1.E resets to 0 on a warm
> reset, the trace buffer unit is disabled when idle resume, it is safe to
> restore base/pointer/status registers first and program TRBLIMITR_EL1
> last.
> 
> Signed-off-by: Yabin Cui <yabinc@google.com>
> Tested-by: James Clark <james.clark@linaro.org>
> Reviewed-by: Yeoreum Yun <yeoreum.yun@arm.com>
> Reviewed-by: James Clark <james.clark@linaro.org>
> Co-developed-by: Leo Yan <leo.yan@arm.com>
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>   drivers/hwtracing/coresight/coresight-trbe.c | 59 +++++++++++++++++++++++++++-
>   1 file changed, 58 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
> index 14e35b9660d76e47619cc6026b94929b3bb3e02b..c7cbca45f2debd4047b93283ea9fe5dd9e1f2ebf 100644
> --- a/drivers/hwtracing/coresight/coresight-trbe.c
> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
> @@ -116,6 +116,20 @@ static int trbe_errata_cpucaps[] = {
>    */
>   #define TRBE_WORKAROUND_OVERWRITE_FILL_MODE_SKIP_BYTES	256
>   
> +/*
> + * struct trbe_save_state: Register values representing TRBE state
> + * @trblimitr		- Trace Buffer Limit Address Register value
> + * @trbbaser		- Trace Buffer Base Register value
> + * @trbptr		- Trace Buffer Write Pointer Register value
> + * @trbsr		- Trace Buffer Status Register value
> + */
> +struct trbe_save_state {
> +	u64 trblimitr;
> +	u64 trbbaser;
> +	u64 trbptr;
> +	u64 trbsr;
> +};
> +
>   /*
>    * struct trbe_cpudata: TRBE instance specific data
>    * @trbe_flag		- TRBE dirty/access flag support
> @@ -134,6 +148,7 @@ struct trbe_cpudata {
>   	enum cs_mode mode;
>   	struct trbe_buf *buf;
>   	struct trbe_drvdata *drvdata;
> +	struct trbe_save_state save_state;
>   	DECLARE_BITMAP(errata, TRBE_ERRATA_MAX);
>   };
>   
> @@ -1189,6 +1204,46 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)
>   	return IRQ_HANDLED;
>   }
>   
> +static int arm_trbe_save(struct coresight_device *csdev)
> +{
> +	struct trbe_cpudata *cpudata = dev_get_drvdata(&csdev->dev);
> +	struct trbe_save_state *state = &cpudata->save_state;
> +
> +	state->trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
> +
> +	/* Disable the unit, ensure the writes to memory are complete */
> +	if (state->trblimitr & TRBLIMITR_EL1_E)
> +		trbe_drain_and_disable_local(cpudata);
> +
> +	state->trbbaser = read_sysreg_s(SYS_TRBBASER_EL1);
> +	state->trbptr = read_sysreg_s(SYS_TRBPTR_EL1);
> +	state->trbsr = read_sysreg_s(SYS_TRBSR_EL1);
> +	return 0;
> +}
> +
> +static void arm_trbe_restore(struct coresight_device *csdev)
> +{
> +	struct trbe_cpudata *cpudata = dev_get_drvdata(&csdev->dev);
> +	struct trbe_save_state *state = &cpudata->save_state;
> +
> +	write_sysreg_s(state->trbbaser, SYS_TRBBASER_EL1);
> +	write_sysreg_s(state->trbptr, SYS_TRBPTR_EL1);
> +	write_sysreg_s(state->trbsr, SYS_TRBSR_EL1);
> +
> +	if (!(state->trblimitr & TRBLIMITR_EL1_E)) {
> +		write_sysreg_s(state->trblimitr, SYS_TRBLIMITR_EL1);
> +	} else {
> +		/*
> +		 * The section K5.5 Context switching, Arm ARM (ARM DDI 0487
> +		 * L.a), S_PKLXF requires a Context synchronization event to
> +		 * guarantee the Trace Buffer Unit will observe the new values
> +		 * of the system registers.
> +		 */
> +		isb();
> +		set_trbe_enabled(cpudata, state->trblimitr);
> +	}
> +}
> +
>   static const struct coresight_ops_sink arm_trbe_sink_ops = {
>   	.enable		= arm_trbe_enable,
>   	.disable	= arm_trbe_disable,
> @@ -1198,7 +1253,9 @@ static const struct coresight_ops_sink arm_trbe_sink_ops = {
>   };
>   
>   static const struct coresight_ops arm_trbe_cs_ops = {
> -	.sink_ops	= &arm_trbe_sink_ops,
> +	.pm_save_disable	= arm_trbe_save,
> +	.pm_restore_enable	= arm_trbe_restore,
> +	.sink_ops		= &arm_trbe_sink_ops,
>   };
>   
>   static ssize_t align_show(struct device *dev, struct device_attribute *attr, char *buf)
> 

Re: [PATCH v10 17/20] coresight: trbe: Save and restore state across CPU low power state

[Leo Yan]

On Thu, Apr 09, 2026 at 11:52:06AM +0100, James Clark wrote:
> On 05/04/2026 4:02 pm, Leo Yan wrote:
> > From: Yabin Cui <yabinc@google.com>
> > 
> > TRBE context can be lost when a CPU enters low power states.  If a trace
> > source is restored while TRBE is not, tracing may run without an active
> > sink, which can lead to hangs on some devices (e.g., Pixel 9).
> 
> Can't this still happen if saving the source times out on "wait for
> TRCSTATR.IDLE to go up"?
> 
> That would make coresight_pm_save() exit early, not saving the active TRBE
> state. Then when coresight_pm_restore() is called it restores a stale
> inactive sink, then enables the source again which is the state that can
> hang.

Hmm... I don't expect this to happen anyway.

If CPU PM enter fails and returns an error, it will bail out early of
the idle flow and the CPU will not proceed entering low power states;
therefore, the restore flow does not exist.

Thanks,
Leo

[PATCH v10 18/20] coresight: sysfs: Increment refcount only for system tracers

[Leo Yan]

Except for system tracers (e.g. STM), other sources treat multiple
enables as equivalent to a single enable. The device mode already
tracks the binary state, so it is redundant to operate refcount.

Refactor to maintain the refcount only for system sources. This
simplifies future CPU PM handling without refcount logic.

Tested-by: James Clark <james.clark@linaro.org>
Reviewed-by: Yeoreum Yun <yeoreum.yun@arm.com>
Reviewed-by: James Clark <james.clark@linaro.org>
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-sysfs.c | 34 ++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/hwtracing/coresight/coresight-sysfs.c
index 9449de66ba3941614928924086100866f3c88a54..75dd8c9f78174b7e604fbad6e8735656a2d643a0 100644
--- a/drivers/hwtracing/coresight/coresight-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-sysfs.c
@@ -53,6 +53,28 @@ ssize_t coresight_simple_show32(struct device *_dev,
 }
 EXPORT_SYMBOL_GPL(coresight_simple_show32);
 
+static void coresight_source_get_refcnt(struct coresight_device *csdev)
+{
+	/*
+	 * There could be multiple applications driving the software
+	 * source. So keep the refcount for each such user when the
+	 * source is already enabled.
+	 *
+	 * No need to increment the reference counter for other source
+	 * types, as multiple enables are the same as a single enable.
+	 */
+	if (csdev->subtype.source_subtype ==
+			CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE)
+		csdev->refcnt++;
+}
+
+static void coresight_source_put_refcnt(struct coresight_device *csdev)
+{
+	if (csdev->subtype.source_subtype ==
+			CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE)
+		csdev->refcnt--;
+}
+
 static int coresight_enable_source_sysfs(struct coresight_device *csdev,
 					 enum cs_mode mode,
 					 struct coresight_path *path)
@@ -71,7 +93,7 @@ static int coresight_enable_source_sysfs(struct coresight_device *csdev,
 			return ret;
 	}
 
-	csdev->refcnt++;
+	coresight_source_get_refcnt(csdev);
 
 	return 0;
 }
@@ -93,7 +115,7 @@ static bool coresight_disable_source_sysfs(struct coresight_device *csdev,
 	if (coresight_get_mode(csdev) != CS_MODE_SYSFS)
 		return false;
 
-	csdev->refcnt--;
+	coresight_source_put_refcnt(csdev);
 	if (csdev->refcnt == 0) {
 		coresight_disable_source(csdev, data);
 		return true;
@@ -188,13 +210,7 @@ int coresight_enable_sysfs(struct coresight_device *csdev)
 	 * doesn't hold coresight_mutex.
 	 */
 	if (coresight_get_mode(csdev) == CS_MODE_SYSFS) {
-		/*
-		 * There could be multiple applications driving the software
-		 * source. So keep the refcount for each such user when the
-		 * source is already enabled.
-		 */
-		if (subtype == CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE)
-			csdev->refcnt++;
+		coresight_source_get_refcnt(csdev);
 		goto out;
 	}
 

-- 
2.34.1

[PATCH v10 19/20] coresight: Move CPU hotplug callbacks to core layer

[Leo Yan]

This commit moves CPU hotplug callbacks from ETMv4 driver to core layer.
The motivation is the core layer can control all components on an
activated path rather but not only managing tracer in ETMv4 driver.

The perf event layer will disable CoreSight PMU event 'cs_etm' when
hotplug off a CPU.  That means a perf mode will be always converted to
disabled mode in CPU hotplug.  Arm CoreSight CPU hotplug callbacks only
need to handle the Sysfs mode and ignore the perf mode.

Change CPUHP_AP_ARM_CORESIGHT_STARTING to CPUHP_AP_ARM_CORESIGHT_ONLINE
so that the CPU hotplug callback runs in the online state and thread
context, allowing coresight_disable_sysfs() to be called directly to
disable the path.

Tested-by: James Clark <james.clark@linaro.org>
Reviewed-by: Yeoreum Yun <yeoreum.yun@arm.com>
Reviewed-by: James Clark <james.clark@linaro.org>
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-core.c       | 42 +++++++++++++++++++++-
 drivers/hwtracing/coresight/coresight-etm3x-core.c | 40 ---------------------
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 37 -------------------
 include/linux/cpuhotplug.h                         |  2 +-
 4 files changed, 42 insertions(+), 79 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index a918bf6398a932de30fe9b4947020cc4c1cfb2f7..af0ec6d1df7baea2a7beb9ee0a28bb969eb4ccf2 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -1889,13 +1889,53 @@ static struct notifier_block coresight_cpu_pm_nb = {
 	.notifier_call = coresight_cpu_pm_notify,
 };
 
+static int coresight_dying_cpu(unsigned int cpu)
+{
+	struct coresight_device *source;
+	struct coresight_path *path;
+
+	/* Disable interrupt to avoid race with perf session */
+	scoped_guard(irqsave) {
+		path = coresight_get_percpu_local_path();
+		if (!path)
+			return 0;
+
+		source = coresight_get_source(path);
+		if (!source)
+			return 0;
+
+		/*
+		 * The perf event layer will disable PMU events in the CPU
+		 * hotplug. Here only handles SYSFS case.
+		 */
+		if (coresight_get_mode(source) != CS_MODE_SYSFS)
+			return 0;
+	}
+
+	coresight_disable_sysfs(source);
+	return 0;
+}
+
 static int __init coresight_pm_setup(void)
 {
-	return cpu_pm_register_notifier(&coresight_cpu_pm_nb);
+	int ret;
+
+	ret = cpu_pm_register_notifier(&coresight_cpu_pm_nb);
+	if (ret)
+		return ret;
+
+	ret = cpuhp_setup_state_nocalls(CPUHP_AP_ARM_CORESIGHT_ONLINE,
+					"arm/coresight-core:starting",
+					NULL, coresight_dying_cpu);
+	if (ret)
+		cpu_pm_unregister_notifier(&coresight_cpu_pm_nb);
+
+	return ret;
 }
 
 static void coresight_pm_cleanup(void)
 {
+	cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_ONLINE);
 	cpu_pm_unregister_notifier(&coresight_cpu_pm_nb);
 }
 
diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c
index 46ea66b5cf1985bd7129688f175f6f92372d04ad..5c6b131465c52fd8ab9161bf36fc78ba9a7dd99b 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c
@@ -699,35 +699,6 @@ static int etm_online_cpu(unsigned int cpu)
 	return 0;
 }
 
-static int etm_starting_cpu(unsigned int cpu)
-{
-	if (!etmdrvdata[cpu])
-		return 0;
-
-	spin_lock(&etmdrvdata[cpu]->spinlock);
-	if (!etmdrvdata[cpu]->os_unlock) {
-		etm_os_unlock(etmdrvdata[cpu]);
-		etmdrvdata[cpu]->os_unlock = true;
-	}
-
-	if (coresight_get_mode(etmdrvdata[cpu]->csdev))
-		etm_enable_hw(etmdrvdata[cpu]);
-	spin_unlock(&etmdrvdata[cpu]->spinlock);
-	return 0;
-}
-
-static int etm_dying_cpu(unsigned int cpu)
-{
-	if (!etmdrvdata[cpu])
-		return 0;
-
-	spin_lock(&etmdrvdata[cpu]->spinlock);
-	if (coresight_get_mode(etmdrvdata[cpu]->csdev))
-		etm_disable_hw(etmdrvdata[cpu]);
-	spin_unlock(&etmdrvdata[cpu]->spinlock);
-	return 0;
-}
-
 static bool etm_arch_supported(u8 arch)
 {
 	switch (arch) {
@@ -795,13 +766,6 @@ static int __init etm_hp_setup(void)
 {
 	int ret;
 
-	ret = cpuhp_setup_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING,
-					"arm/coresight:starting",
-					etm_starting_cpu, etm_dying_cpu);
-
-	if (ret)
-		return ret;
-
 	ret = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN,
 					"arm/coresight:online",
 					etm_online_cpu, NULL);
@@ -812,15 +776,11 @@ static int __init etm_hp_setup(void)
 		return 0;
 	}
 
-	/* failed dyn state - remove others */
-	cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING);
-
 	return ret;
 }
 
 static void etm_hp_clear(void)
 {
-	cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING);
 	if (hp_online) {
 		cpuhp_remove_state_nocalls(hp_online);
 		hp_online = 0;
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 5ccbf792fe71bec7d5b74c24288c5313afdb7cd0..879950ec0f527c0151f00266f6bf5d569ad460d8 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1829,33 +1829,6 @@ static int etm4_online_cpu(unsigned int cpu)
 	return 0;
 }
 
-static int etm4_starting_cpu(unsigned int cpu)
-{
-	if (!etmdrvdata[cpu])
-		return 0;
-
-	raw_spin_lock(&etmdrvdata[cpu]->spinlock);
-	if (!etmdrvdata[cpu]->os_unlock)
-		etm4_os_unlock(etmdrvdata[cpu]);
-
-	if (coresight_get_mode(etmdrvdata[cpu]->csdev))
-		etm4_enable_hw(etmdrvdata[cpu]);
-	raw_spin_unlock(&etmdrvdata[cpu]->spinlock);
-	return 0;
-}
-
-static int etm4_dying_cpu(unsigned int cpu)
-{
-	if (!etmdrvdata[cpu])
-		return 0;
-
-	raw_spin_lock(&etmdrvdata[cpu]->spinlock);
-	if (coresight_get_mode(etmdrvdata[cpu]->csdev))
-		etm4_disable_hw(etmdrvdata[cpu]);
-	raw_spin_unlock(&etmdrvdata[cpu]->spinlock);
-	return 0;
-}
-
 static inline bool etm4_pm_save_needed(void)
 {
 	return pm_save_enable == PARAM_PM_SAVE_SELF_HOSTED;
@@ -2116,13 +2089,6 @@ static int __init etm4_pm_setup(void)
 {
 	int ret;
 
-	ret = cpuhp_setup_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING,
-					"arm/coresight4:starting",
-					etm4_starting_cpu, etm4_dying_cpu);
-
-	if (ret)
-		return ret;
-
 	ret = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN,
 					"arm/coresight4:online",
 					etm4_online_cpu, NULL);
@@ -2133,14 +2099,11 @@ static int __init etm4_pm_setup(void)
 		return 0;
 	}
 
-	/* failed dyn state - remove others */
-	cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING);
 	return ret;
 }
 
 static void etm4_pm_clear(void)
 {
-	cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING);
 	if (hp_online) {
 		cpuhp_remove_state_nocalls(hp_online);
 		hp_online = 0;
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index 62cd7b35a29c94f214fec3b55f9462eceffad694..b9196ef6d83471400bb8bc5a8c4fd486c0acd4cc 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -181,7 +181,6 @@ enum cpuhp_state {
 	CPUHP_AP_DUMMY_TIMER_STARTING,
 	CPUHP_AP_ARM_XEN_STARTING,
 	CPUHP_AP_ARM_XEN_RUNSTATE_STARTING,
-	CPUHP_AP_ARM_CORESIGHT_STARTING,
 	CPUHP_AP_ARM_CORESIGHT_CTI_STARTING,
 	CPUHP_AP_ARM64_ISNDEP_STARTING,
 	CPUHP_AP_SMPCFD_DYING,
@@ -201,6 +200,7 @@ enum cpuhp_state {
 	CPUHP_AP_IRQ_AFFINITY_ONLINE,
 	CPUHP_AP_BLK_MQ_ONLINE,
 	CPUHP_AP_ARM_MVEBU_SYNC_CLOCKS,
+	CPUHP_AP_ARM_CORESIGHT_ONLINE,
 	CPUHP_AP_X86_INTEL_EPB_ONLINE,
 	CPUHP_AP_PERF_ONLINE,
 	CPUHP_AP_PERF_X86_ONLINE,

-- 
2.34.1

[PATCH v10 20/20] coresight: sysfs: Validate CPU online status for per-CPU sources

[Leo Yan]

The current SysFS flow first enables the links and sink, then rolls back
to disable them if the source fails to enable. This failure can occur if
the associated CPU is offline, which causes the SMP call to fail.

Validate whether the associated CPU is online for a per-CPU tracer.  If
the CPU is offline, return -ENODEV and bail out.

Tested-by: James Clark <james.clark@linaro.org>
Reviewed-by: Yeoreum Yun <yeoreum.yun@arm.com>
Reviewed-by: James Clark <james.clark@linaro.org>
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-sysfs.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/hwtracing/coresight/coresight-sysfs.c
index 75dd8c9f78174b7e604fbad6e8735656a2d643a0..b79c0abfeb77ed6f8fa299f083e0b6c656e0e6c1 100644
--- a/drivers/hwtracing/coresight/coresight-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-sysfs.c
@@ -184,6 +184,9 @@ static int coresight_validate_source_sysfs(struct coresight_device *csdev,
 		return -EINVAL;
 	}
 
+	if (coresight_is_percpu_source(csdev) && !cpu_online(csdev->cpu))
+		return -ENODEV;
+
 	return 0;
 }
 

-- 
2.34.1