[PATCH 0/8] coresight: cti: Miscellaneous fixes and CPU PM cleanup

[PATCH 0/8] coresight: cti: Miscellaneous fixes and CPU PM cleanup

[Leo Yan]

The CPU power management issue in the CTI driver was first observed in
series [1]; this series resolves that issue.  It fixes bugs and removes
CPU PM operations from the CoreSight CTI driver, the goal is to use the
CoreSight core layer as the central place for CPU power management.
Removing CPU PM from CTI driver can avoid conflicts with the core layer.

Based on review of the Arm ARM, ASICCTL is the only CTI register that
could potentially reside in the CPU power domain.  However, this is
considered highly unlikely for the following reasons:

 - Standard Arm CTIs place the ASICCTL register in the debug power
   domain;
 - ASICCTL is implemented only when CTIDEVID.EXTMUXNUM is non-zero,
   which is rare for CPU CTIs.

As a result, it is safe to remove the CPU PM code as done in this
series.  In addition, avoiding support local CPU access (via SMP calls)
to ASICCTL significantly reduces driver complexity.

If a future hardware implements ASICCTL in the CPU power domain, we can
consider adding a property to describe that characteristic.  That said,
from a software point of view, keeping all CTI registers in the same
power domain is preferable, as it makes the driver implementation much
simpler.

This series can be divided into:

  Patches 01 ~ 02: Fix spinlock with irqsave and register read with CS
                   lock.
  Patches 03 ~ 08: Access ASICCTL condintioanlly, remove CPU PM code,
                   and refactor register access in sysfs knob.

This series is based on coresight-next branch and has been validated on
Juno r1 and r2 platforms, pass normal sysfs and perf test, as well as
CPU PM stress testing.

[1] https://lore.kernel.org/all/20250915-arm_coresight_power_management_fix-v3-0-ea49e91124ec@arm.com/

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
Leo Yan (8):
      coresight: cti: Make spinlock usage consistent
      coresight: cti: Fix register reads
      coresight: cti: Access ASICCTL only when implemented
      coresight: cti: Remove CPU power management code
      coresight: cti: Rename cti_active() to cti_is_active()
      coresight: cti: Remove hw_powered flag
      coresight: cti: Remove hw_enabled flag
      coresight: cti: Refactor cti_reg32_{show|store}()

 drivers/hwtracing/coresight/coresight-cti-core.c  | 278 ++++------------------
 drivers/hwtracing/coresight/coresight-cti-sysfs.c | 168 ++++++-------
 drivers/hwtracing/coresight/coresight-cti.h       |  13 +-
 3 files changed, 134 insertions(+), 325 deletions(-)
---
base-commit: eebe8dbd8630f51cf70b1f68a440cd3d7f7a914d
change-id: 20251223-arm_coresight_cti_refactor_v1-76e1bda8b716

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

[PATCH 1/8] coresight: cti: Make spinlock usage consistent

[Leo Yan]

The spinlock is acquired sometimes with IRQs disabled and sometimes
without.  This leads to inconsistent semantics: the lock can be either
HARDIRQ-safe or HARDIRQ-unsafe, which may trigger lockdep complaints.

Make spinlock usage consistent by acquiring it with disabling IRQs.  It
is possible for sysfs knobs to acquire the spinlock for accessing a
CTI device, while at the same time a perf session sends an IPI to
enable the same CTI device.  In this case, the spinlock must be
IRQ-safe, which is why all lock acquisitions are changed to disable
IRQs.

Use guard() and scoped_guard() for spinlock to tidy up the code.

Fixes: 984f37efa385 ("coresight: cti: Write regsiters directly in cti_enable_hw()")
Tested-by: James Clark <james.clark@linaro.org>
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-cti-core.c  |  74 ++++-------
 drivers/hwtracing/coresight/coresight-cti-sysfs.c | 145 ++++++++++++----------
 2 files changed, 103 insertions(+), 116 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c
index bfbc365bb2ef2744efab11c056b8450472957005..b75c4ec2c8d5d58da8761334643dba34001b48c7 100644
--- a/drivers/hwtracing/coresight/coresight-cti-core.c
+++ b/drivers/hwtracing/coresight/coresight-cti-core.c
@@ -90,10 +90,9 @@ void cti_write_all_hw_regs(struct cti_drvdata *drvdata)
 static int cti_enable_hw(struct cti_drvdata *drvdata)
 {
 	struct cti_config *config = &drvdata->config;
-	unsigned long flags;
-	int rc = 0;
+	int rc;
 
-	raw_spin_lock_irqsave(&drvdata->spinlock, flags);
+	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
 
 	/* no need to do anything if enabled or unpowered*/
 	if (config->hw_enabled || !config->hw_powered)
@@ -102,22 +101,15 @@ static int cti_enable_hw(struct cti_drvdata *drvdata)
 	/* claim the device */
 	rc = coresight_claim_device(drvdata->csdev);
 	if (rc)
-		goto cti_err_not_enabled;
+		return rc;
 
 	cti_write_all_hw_regs(drvdata);
 
 	config->hw_enabled = true;
-	drvdata->config.enable_req_count++;
-	raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
-	return rc;
 
 cti_state_unchanged:
 	drvdata->config.enable_req_count++;
-
-	/* cannot enable due to error */
-cti_err_not_enabled:
-	raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
-	return rc;
+	return 0;
 }
 
 /* re-enable CTI on CPU when using CPU hotplug */
@@ -125,25 +117,21 @@ static void cti_cpuhp_enable_hw(struct cti_drvdata *drvdata)
 {
 	struct cti_config *config = &drvdata->config;
 
-	raw_spin_lock(&drvdata->spinlock);
+	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
+
 	config->hw_powered = true;
 
 	/* no need to do anything if no enable request */
 	if (!drvdata->config.enable_req_count)
-		goto cti_hp_not_enabled;
+		return;
 
 	/* try to claim the device */
 	if (coresight_claim_device(drvdata->csdev))
-		goto cti_hp_not_enabled;
+		return;
 
 	cti_write_all_hw_regs(drvdata);
 	config->hw_enabled = true;
-	raw_spin_unlock(&drvdata->spinlock);
 	return;
-
-	/* did not re-enable due to no claim / no request */
-cti_hp_not_enabled:
-	raw_spin_unlock(&drvdata->spinlock);
 }
 
 /* disable hardware */
@@ -151,23 +139,20 @@ static int cti_disable_hw(struct cti_drvdata *drvdata)
 {
 	struct cti_config *config = &drvdata->config;
 	struct coresight_device *csdev = drvdata->csdev;
-	int ret = 0;
 
-	raw_spin_lock(&drvdata->spinlock);
+	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
 
 	/* don't allow negative refcounts, return an error */
-	if (!drvdata->config.enable_req_count) {
-		ret = -EINVAL;
-		goto cti_not_disabled;
-	}
+	if (!drvdata->config.enable_req_count)
+		return -EINVAL;
 
 	/* check refcount - disable on 0 */
 	if (--drvdata->config.enable_req_count > 0)
-		goto cti_not_disabled;
+		return 0;
 
 	/* no need to do anything if disabled or cpu unpowered */
 	if (!config->hw_enabled || !config->hw_powered)
-		goto cti_not_disabled;
+		return 0;
 
 	CS_UNLOCK(drvdata->base);
 
@@ -177,13 +162,7 @@ static int cti_disable_hw(struct cti_drvdata *drvdata)
 
 	coresight_disclaim_device_unlocked(csdev);
 	CS_LOCK(drvdata->base);
-	raw_spin_unlock(&drvdata->spinlock);
-	return ret;
-
-	/* not disabled this call */
-cti_not_disabled:
-	raw_spin_unlock(&drvdata->spinlock);
-	return ret;
+	return 0;
 }
 
 void cti_write_single_reg(struct cti_drvdata *drvdata, int offset, u32 value)
@@ -198,11 +177,11 @@ void cti_write_intack(struct device *dev, u32 ackval)
 	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
 	struct cti_config *config = &drvdata->config;
 
-	raw_spin_lock(&drvdata->spinlock);
+	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
+
 	/* write if enabled */
 	if (cti_active(config))
 		cti_write_single_reg(drvdata, CTIINTACK, ackval);
-	raw_spin_unlock(&drvdata->spinlock);
 }
 
 /*
@@ -369,7 +348,7 @@ int cti_channel_trig_op(struct device *dev, enum cti_chan_op op,
 	reg_offset = (direction == CTI_TRIG_IN ? CTIINEN(trigger_idx) :
 		      CTIOUTEN(trigger_idx));
 
-	raw_spin_lock(&drvdata->spinlock);
+	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
 
 	/* read - modify write - the trigger / channel enable value */
 	reg_value = direction == CTI_TRIG_IN ? config->ctiinen[trigger_idx] :
@@ -388,7 +367,7 @@ int cti_channel_trig_op(struct device *dev, enum cti_chan_op op,
 	/* write through if enabled */
 	if (cti_active(config))
 		cti_write_single_reg(drvdata, reg_offset, reg_value);
-	raw_spin_unlock(&drvdata->spinlock);
+
 	return 0;
 }
 
@@ -406,7 +385,8 @@ int cti_channel_gate_op(struct device *dev, enum cti_chan_gate_op op,
 
 	chan_bitmask = BIT(channel_idx);
 
-	raw_spin_lock(&drvdata->spinlock);
+	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
+
 	reg_value = config->ctigate;
 	switch (op) {
 	case CTI_GATE_CHAN_ENABLE:
@@ -426,7 +406,7 @@ int cti_channel_gate_op(struct device *dev, enum cti_chan_gate_op op,
 		if (cti_active(config))
 			cti_write_single_reg(drvdata, CTIGATE, reg_value);
 	}
-	raw_spin_unlock(&drvdata->spinlock);
+
 	return err;
 }
 
@@ -445,7 +425,8 @@ int cti_channel_setop(struct device *dev, enum cti_chan_set_op op,
 
 	chan_bitmask = BIT(channel_idx);
 
-	raw_spin_lock(&drvdata->spinlock);
+	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
+
 	reg_value = config->ctiappset;
 	switch (op) {
 	case CTI_CHAN_SET:
@@ -473,7 +454,6 @@ int cti_channel_setop(struct device *dev, enum cti_chan_set_op op,
 
 	if ((err == 0) && cti_active(config))
 		cti_write_single_reg(drvdata, reg_offset, reg_value);
-	raw_spin_unlock(&drvdata->spinlock);
 
 	return err;
 }
@@ -676,7 +656,7 @@ static int cti_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd,
 	if (WARN_ON_ONCE(drvdata->ctidev.cpu != cpu))
 		return NOTIFY_BAD;
 
-	raw_spin_lock(&drvdata->spinlock);
+	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
 
 	switch (cmd) {
 	case CPU_PM_ENTER:
@@ -716,7 +696,6 @@ static int cti_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd,
 	}
 
 cti_notify_exit:
-	raw_spin_unlock(&drvdata->spinlock);
 	return notify_res;
 }
 
@@ -743,11 +722,12 @@ static int cti_dying_cpu(unsigned int cpu)
 	if (!drvdata)
 		return 0;
 
-	raw_spin_lock(&drvdata->spinlock);
+	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
+
 	drvdata->config.hw_powered = false;
 	if (drvdata->config.hw_enabled)
 		coresight_disclaim_device(drvdata->csdev);
-	raw_spin_unlock(&drvdata->spinlock);
+
 	return 0;
 }
 
diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
index 572b80ee96fbf18ec8cf9abc30d109a676dfbc5d..455d08bcccd49a3f1eac8abd8246806ef73a9ab6 100644
--- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
@@ -84,11 +84,11 @@ static ssize_t enable_show(struct device *dev,
 	bool enabled, powered;
 	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
 
-	raw_spin_lock(&drvdata->spinlock);
-	enable_req = drvdata->config.enable_req_count;
-	powered = drvdata->config.hw_powered;
-	enabled = drvdata->config.hw_enabled;
-	raw_spin_unlock(&drvdata->spinlock);
+	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock) {
+		enable_req = drvdata->config.enable_req_count;
+		powered = drvdata->config.hw_powered;
+		enabled = drvdata->config.hw_enabled;
+	}
 
 	if (powered)
 		return sprintf(buf, "%d\n", enabled);
@@ -134,9 +134,8 @@ static ssize_t powered_show(struct device *dev,
 	bool powered;
 	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
 
-	raw_spin_lock(&drvdata->spinlock);
-	powered = drvdata->config.hw_powered;
-	raw_spin_unlock(&drvdata->spinlock);
+	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock)
+		powered = drvdata->config.hw_powered;
 
 	return sprintf(buf, "%d\n", powered);
 }
@@ -181,10 +180,12 @@ static ssize_t coresight_cti_reg_show(struct device *dev,
 	u32 val = 0;
 
 	pm_runtime_get_sync(dev->parent);
-	raw_spin_lock(&drvdata->spinlock);
-	if (drvdata->config.hw_powered)
-		val = readl_relaxed(drvdata->base + cti_attr->off);
-	raw_spin_unlock(&drvdata->spinlock);
+
+	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock) {
+		if (drvdata->config.hw_powered)
+			val = readl_relaxed(drvdata->base + cti_attr->off);
+	}
+
 	pm_runtime_put_sync(dev->parent);
 	return sysfs_emit(buf, "0x%x\n", val);
 }
@@ -202,10 +203,12 @@ static __maybe_unused ssize_t coresight_cti_reg_store(struct device *dev,
 		return -EINVAL;
 
 	pm_runtime_get_sync(dev->parent);
-	raw_spin_lock(&drvdata->spinlock);
-	if (drvdata->config.hw_powered)
-		cti_write_single_reg(drvdata, cti_attr->off, val);
-	raw_spin_unlock(&drvdata->spinlock);
+
+	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock) {
+		if (drvdata->config.hw_powered)
+			cti_write_single_reg(drvdata, cti_attr->off, val);
+	}
+
 	pm_runtime_put_sync(dev->parent);
 	return size;
 }
@@ -264,17 +267,18 @@ static ssize_t cti_reg32_show(struct device *dev, char *buf,
 	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
 	struct cti_config *config = &drvdata->config;
 
-	raw_spin_lock(&drvdata->spinlock);
-	if ((reg_offset >= 0) && cti_active(config)) {
-		CS_UNLOCK(drvdata->base);
-		val = readl_relaxed(drvdata->base + reg_offset);
-		if (pcached_val)
-			*pcached_val = val;
-		CS_LOCK(drvdata->base);
-	} else if (pcached_val) {
-		val = *pcached_val;
+	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock) {
+		if ((reg_offset >= 0) && cti_active(config)) {
+			CS_UNLOCK(drvdata->base);
+			val = readl_relaxed(drvdata->base + reg_offset);
+			if (pcached_val)
+				*pcached_val = val;
+			CS_LOCK(drvdata->base);
+		} else if (pcached_val) {
+			val = *pcached_val;
+		}
 	}
-	raw_spin_unlock(&drvdata->spinlock);
+
 	return sprintf(buf, "%#x\n", val);
 }
 
@@ -293,15 +297,16 @@ static ssize_t cti_reg32_store(struct device *dev, const char *buf,
 	if (kstrtoul(buf, 0, &val))
 		return -EINVAL;
 
-	raw_spin_lock(&drvdata->spinlock);
-	/* local store */
-	if (pcached_val)
-		*pcached_val = (u32)val;
+	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock) {
+		/* local store */
+		if (pcached_val)
+			*pcached_val = (u32)val;
+
+		/* write through if offset and enabled */
+		if ((reg_offset >= 0) && cti_active(config))
+			cti_write_single_reg(drvdata, reg_offset, val);
+	}
 
-	/* write through if offset and enabled */
-	if ((reg_offset >= 0) && cti_active(config))
-		cti_write_single_reg(drvdata, reg_offset, val);
-	raw_spin_unlock(&drvdata->spinlock);
 	return size;
 }
 
@@ -349,9 +354,9 @@ static ssize_t inout_sel_store(struct device *dev,
 	if (val > (CTIINOUTEN_MAX - 1))
 		return -EINVAL;
 
-	raw_spin_lock(&drvdata->spinlock);
+	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
+
 	drvdata->config.ctiinout_sel = val;
-	raw_spin_unlock(&drvdata->spinlock);
 	return size;
 }
 static DEVICE_ATTR_RW(inout_sel);
@@ -364,10 +369,11 @@ static ssize_t inen_show(struct device *dev,
 	int index;
 	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
 
-	raw_spin_lock(&drvdata->spinlock);
-	index = drvdata->config.ctiinout_sel;
-	val = drvdata->config.ctiinen[index];
-	raw_spin_unlock(&drvdata->spinlock);
+	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock) {
+		index = drvdata->config.ctiinout_sel;
+		val = drvdata->config.ctiinen[index];
+	}
+
 	return sprintf(buf, "%#lx\n", val);
 }
 
@@ -383,14 +389,15 @@ static ssize_t inen_store(struct device *dev,
 	if (kstrtoul(buf, 0, &val))
 		return -EINVAL;
 
-	raw_spin_lock(&drvdata->spinlock);
+	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
+
 	index = config->ctiinout_sel;
 	config->ctiinen[index] = val;
 
 	/* write through if enabled */
 	if (cti_active(config))
 		cti_write_single_reg(drvdata, CTIINEN(index), val);
-	raw_spin_unlock(&drvdata->spinlock);
+
 	return size;
 }
 static DEVICE_ATTR_RW(inen);
@@ -403,10 +410,11 @@ static ssize_t outen_show(struct device *dev,
 	int index;
 	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
 
-	raw_spin_lock(&drvdata->spinlock);
-	index = drvdata->config.ctiinout_sel;
-	val = drvdata->config.ctiouten[index];
-	raw_spin_unlock(&drvdata->spinlock);
+	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock) {
+		index = drvdata->config.ctiinout_sel;
+		val = drvdata->config.ctiouten[index];
+	}
+
 	return sprintf(buf, "%#lx\n", val);
 }
 
@@ -422,14 +430,15 @@ static ssize_t outen_store(struct device *dev,
 	if (kstrtoul(buf, 0, &val))
 		return -EINVAL;
 
-	raw_spin_lock(&drvdata->spinlock);
+	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
+
 	index = config->ctiinout_sel;
 	config->ctiouten[index] = val;
 
 	/* write through if enabled */
 	if (cti_active(config))
 		cti_write_single_reg(drvdata, CTIOUTEN(index), val);
-	raw_spin_unlock(&drvdata->spinlock);
+
 	return size;
 }
 static DEVICE_ATTR_RW(outen);
@@ -463,7 +472,7 @@ static ssize_t appclear_store(struct device *dev,
 	if (kstrtoul(buf, 0, &val))
 		return -EINVAL;
 
-	raw_spin_lock(&drvdata->spinlock);
+	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
 
 	/* a 1'b1 in appclr clears down the same bit in appset*/
 	config->ctiappset &= ~val;
@@ -471,7 +480,7 @@ static ssize_t appclear_store(struct device *dev,
 	/* write through if enabled */
 	if (cti_active(config))
 		cti_write_single_reg(drvdata, CTIAPPCLEAR, val);
-	raw_spin_unlock(&drvdata->spinlock);
+
 	return size;
 }
 static DEVICE_ATTR_WO(appclear);
@@ -487,12 +496,12 @@ static ssize_t apppulse_store(struct device *dev,
 	if (kstrtoul(buf, 0, &val))
 		return -EINVAL;
 
-	raw_spin_lock(&drvdata->spinlock);
+	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
 
 	/* write through if enabled */
 	if (cti_active(config))
 		cti_write_single_reg(drvdata, CTIAPPPULSE, val);
-	raw_spin_unlock(&drvdata->spinlock);
+
 	return size;
 }
 static DEVICE_ATTR_WO(apppulse);
@@ -681,9 +690,9 @@ static ssize_t trig_filter_enable_show(struct device *dev,
 	u32 val;
 	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
 
-	raw_spin_lock(&drvdata->spinlock);
-	val = drvdata->config.trig_filter_enable;
-	raw_spin_unlock(&drvdata->spinlock);
+	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock)
+		val = drvdata->config.trig_filter_enable;
+
 	return sprintf(buf, "%d\n", val);
 }
 
@@ -697,9 +706,9 @@ static ssize_t trig_filter_enable_store(struct device *dev,
 	if (kstrtoul(buf, 0, &val))
 		return -EINVAL;
 
-	raw_spin_lock(&drvdata->spinlock);
+	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
+
 	drvdata->config.trig_filter_enable = !!val;
-	raw_spin_unlock(&drvdata->spinlock);
 	return size;
 }
 static DEVICE_ATTR_RW(trig_filter_enable);
@@ -728,7 +737,7 @@ static ssize_t chan_xtrigs_reset_store(struct device *dev,
 	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
 	struct cti_config *config = &drvdata->config;
 
-	raw_spin_lock(&drvdata->spinlock);
+	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
 
 	/* clear the CTI trigger / channel programming registers */
 	for (i = 0; i < config->nr_trig_max; i++) {
@@ -747,7 +756,6 @@ static ssize_t chan_xtrigs_reset_store(struct device *dev,
 	if (cti_active(config))
 		cti_write_all_hw_regs(drvdata);
 
-	raw_spin_unlock(&drvdata->spinlock);
 	return size;
 }
 static DEVICE_ATTR_WO(chan_xtrigs_reset);
@@ -768,9 +776,9 @@ static ssize_t chan_xtrigs_sel_store(struct device *dev,
 	if (val > (drvdata->config.nr_ctm_channels - 1))
 		return -EINVAL;
 
-	raw_spin_lock(&drvdata->spinlock);
+	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
+
 	drvdata->config.xtrig_rchan_sel = val;
-	raw_spin_unlock(&drvdata->spinlock);
 	return size;
 }
 
@@ -781,9 +789,8 @@ static ssize_t chan_xtrigs_sel_show(struct device *dev,
 	unsigned long val;
 	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
 
-	raw_spin_lock(&drvdata->spinlock);
-	val = drvdata->config.xtrig_rchan_sel;
-	raw_spin_unlock(&drvdata->spinlock);
+	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock)
+		val = drvdata->config.xtrig_rchan_sel;
 
 	return sprintf(buf, "%ld\n", val);
 }
@@ -838,12 +845,12 @@ static ssize_t print_chan_list(struct device *dev,
 	unsigned long inuse_bits = 0, chan_mask;
 
 	/* scan regs to get bitmap of channels in use. */
-	raw_spin_lock(&drvdata->spinlock);
-	for (i = 0; i < config->nr_trig_max; i++) {
-		inuse_bits |= config->ctiinen[i];
-		inuse_bits |= config->ctiouten[i];
+	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock) {
+		for (i = 0; i < config->nr_trig_max; i++) {
+			inuse_bits |= config->ctiinen[i];
+			inuse_bits |= config->ctiouten[i];
+		}
 	}
-	raw_spin_unlock(&drvdata->spinlock);
 
 	/* inverse bits if printing free channels */
 	if (!inuse)

-- 
2.34.1

RE: [PATCH 1/8] coresight: cti: Make spinlock usage consistent

[Mike Leach]

Hi Leo

> -----Original Message-----
> From: Leo Yan <leo.yan@arm.com>
> Sent: Monday, February 9, 2026 6:01 PM
> To: Suzuki Poulose <Suzuki.Poulose@arm.com>; Mike Leach
> <Mike.Leach@arm.com>; James Clark <james.clark@linaro.org>; Alexander
> Shishkin <alexander.shishkin@linux.intel.com>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>; Mathieu Poirier
> <mathieu.poirier@linaro.org>; Tingwei Zhang <quic_tingwei@quicinc.com>;
> Yingchao Deng <yingchao.deng@oss.qualcomm.com>; Jie Gan
> <jie.gan@oss.qualcomm.com>
> Cc: coresight@lists.linaro.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; Leo Yan <Leo.Yan@arm.com>
> Subject: [PATCH 1/8] coresight: cti: Make spinlock usage consistent
> 
> The spinlock is acquired sometimes with IRQs disabled and sometimes
> without.  This leads to inconsistent semantics: the lock can be either
> HARDIRQ-safe or HARDIRQ-unsafe, which may trigger lockdep complaints.
> 
> Make spinlock usage consistent by acquiring it with disabling IRQs.  It
> is possible for sysfs knobs to acquire the spinlock for accessing a
> CTI device, while at the same time a perf session sends an IPI to
> enable the same CTI device.  In this case, the spinlock must be
> IRQ-safe, which is why all lock acquisitions are changed to disable
> IRQs.
> 
> Use guard() and scoped_guard() for spinlock to tidy up the code.
> 
> Fixes: 984f37efa385 ("coresight: cti: Write regsiters directly in
> cti_enable_hw()")
> Tested-by: James Clark <james.clark@linaro.org>
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-cti-core.c  |  74 ++++-------
>  drivers/hwtracing/coresight/coresight-cti-sysfs.c | 145 ++++++++++++-------
> ---
>  2 files changed, 103 insertions(+), 116 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c
> b/drivers/hwtracing/coresight/coresight-cti-core.c
> index
> bfbc365bb2ef2744efab11c056b8450472957005..b75c4ec2c8d5d58da8761
> 334643dba34001b48c7 100644
> --- a/drivers/hwtracing/coresight/coresight-cti-core.c
> +++ b/drivers/hwtracing/coresight/coresight-cti-core.c
> @@ -90,10 +90,9 @@ void cti_write_all_hw_regs(struct cti_drvdata
> *drvdata)
>  static int cti_enable_hw(struct cti_drvdata *drvdata)
>  {
>  	struct cti_config *config = &drvdata->config;
> -	unsigned long flags;
> -	int rc = 0;
> +	int rc;
> 
> -	raw_spin_lock_irqsave(&drvdata->spinlock, flags);
> +	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
> 
>  	/* no need to do anything if enabled or unpowered*/
>  	if (config->hw_enabled || !config->hw_powered)
> @@ -102,22 +101,15 @@ static int cti_enable_hw(struct cti_drvdata
> *drvdata)
>  	/* claim the device */
>  	rc = coresight_claim_device(drvdata->csdev);
>  	if (rc)
> -		goto cti_err_not_enabled;
> +		return rc;
> 
>  	cti_write_all_hw_regs(drvdata);
> 
>  	config->hw_enabled = true;
> -	drvdata->config.enable_req_count++;
> -	raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
> -	return rc;
> 
>  cti_state_unchanged:
>  	drvdata->config.enable_req_count++;
> -
> -	/* cannot enable due to error */
> -cti_err_not_enabled:
> -	raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
> -	return rc;
> +	return 0;
>  }
> 
>  /* re-enable CTI on CPU when using CPU hotplug */
> @@ -125,25 +117,21 @@ static void cti_cpuhp_enable_hw(struct
> cti_drvdata *drvdata)
>  {
>  	struct cti_config *config = &drvdata->config;
> 
> -	raw_spin_lock(&drvdata->spinlock);
> +	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
> +
>  	config->hw_powered = true;
> 
>  	/* no need to do anything if no enable request */
>  	if (!drvdata->config.enable_req_count)
> -		goto cti_hp_not_enabled;
> +		return;
> 
>  	/* try to claim the device */
>  	if (coresight_claim_device(drvdata->csdev))
> -		goto cti_hp_not_enabled;
> +		return;
> 
>  	cti_write_all_hw_regs(drvdata);
>  	config->hw_enabled = true;
> -	raw_spin_unlock(&drvdata->spinlock);
>  	return;
> -
> -	/* did not re-enable due to no claim / no request */
> -cti_hp_not_enabled:
> -	raw_spin_unlock(&drvdata->spinlock);
>  }
> 
>  /* disable hardware */
> @@ -151,23 +139,20 @@ static int cti_disable_hw(struct cti_drvdata
> *drvdata)
>  {
>  	struct cti_config *config = &drvdata->config;
>  	struct coresight_device *csdev = drvdata->csdev;
> -	int ret = 0;
> 
> -	raw_spin_lock(&drvdata->spinlock);
> +	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
> 
>  	/* don't allow negative refcounts, return an error */
> -	if (!drvdata->config.enable_req_count) {
> -		ret = -EINVAL;
> -		goto cti_not_disabled;
> -	}
> +	if (!drvdata->config.enable_req_count)
> +		return -EINVAL;
> 
>  	/* check refcount - disable on 0 */
>  	if (--drvdata->config.enable_req_count > 0)
> -		goto cti_not_disabled;
> +		return 0;
> 
>  	/* no need to do anything if disabled or cpu unpowered */
>  	if (!config->hw_enabled || !config->hw_powered)
> -		goto cti_not_disabled;
> +		return 0;
> 
>  	CS_UNLOCK(drvdata->base);
> 
> @@ -177,13 +162,7 @@ static int cti_disable_hw(struct cti_drvdata
> *drvdata)
> 
>  	coresight_disclaim_device_unlocked(csdev);
>  	CS_LOCK(drvdata->base);
> -	raw_spin_unlock(&drvdata->spinlock);
> -	return ret;
> -
> -	/* not disabled this call */
> -cti_not_disabled:
> -	raw_spin_unlock(&drvdata->spinlock);
> -	return ret;
> +	return 0;
>  }
> 
>  void cti_write_single_reg(struct cti_drvdata *drvdata, int offset, u32 value)
> @@ -198,11 +177,11 @@ void cti_write_intack(struct device *dev, u32
> ackval)
>  	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
>  	struct cti_config *config = &drvdata->config;
> 
> -	raw_spin_lock(&drvdata->spinlock);
> +	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
> +
>  	/* write if enabled */
>  	if (cti_active(config))
>  		cti_write_single_reg(drvdata, CTIINTACK, ackval);
> -	raw_spin_unlock(&drvdata->spinlock);
>  }
> 
>  /*
> @@ -369,7 +348,7 @@ int cti_channel_trig_op(struct device *dev, enum
> cti_chan_op op,
>  	reg_offset = (direction == CTI_TRIG_IN ? CTIINEN(trigger_idx) :
>  		      CTIOUTEN(trigger_idx));
> 
> -	raw_spin_lock(&drvdata->spinlock);
> +	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
> 
>  	/* read - modify write - the trigger / channel enable value */
>  	reg_value = direction == CTI_TRIG_IN ? config->ctiinen[trigger_idx] :
> @@ -388,7 +367,7 @@ int cti_channel_trig_op(struct device *dev, enum
> cti_chan_op op,
>  	/* write through if enabled */
>  	if (cti_active(config))
>  		cti_write_single_reg(drvdata, reg_offset, reg_value);
> -	raw_spin_unlock(&drvdata->spinlock);
> +
>  	return 0;
>  }
> 
> @@ -406,7 +385,8 @@ int cti_channel_gate_op(struct device *dev, enum
> cti_chan_gate_op op,
> 
>  	chan_bitmask = BIT(channel_idx);
> 
> -	raw_spin_lock(&drvdata->spinlock);
> +	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
> +
>  	reg_value = config->ctigate;
>  	switch (op) {
>  	case CTI_GATE_CHAN_ENABLE:
> @@ -426,7 +406,7 @@ int cti_channel_gate_op(struct device *dev, enum
> cti_chan_gate_op op,
>  		if (cti_active(config))
>  			cti_write_single_reg(drvdata, CTIGATE, reg_value);
>  	}
> -	raw_spin_unlock(&drvdata->spinlock);
> +
>  	return err;
>  }
> 
> @@ -445,7 +425,8 @@ int cti_channel_setop(struct device *dev, enum
> cti_chan_set_op op,
> 
>  	chan_bitmask = BIT(channel_idx);
> 
> -	raw_spin_lock(&drvdata->spinlock);
> +	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
> +
>  	reg_value = config->ctiappset;
>  	switch (op) {
>  	case CTI_CHAN_SET:
> @@ -473,7 +454,6 @@ int cti_channel_setop(struct device *dev, enum
> cti_chan_set_op op,
> 
>  	if ((err == 0) && cti_active(config))
>  		cti_write_single_reg(drvdata, reg_offset, reg_value);
> -	raw_spin_unlock(&drvdata->spinlock);
> 
>  	return err;
>  }
> @@ -676,7 +656,7 @@ static int cti_cpu_pm_notify(struct notifier_block
> *nb, unsigned long cmd,
>  	if (WARN_ON_ONCE(drvdata->ctidev.cpu != cpu))
>  		return NOTIFY_BAD;
> 
> -	raw_spin_lock(&drvdata->spinlock);
> +	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
> 
>  	switch (cmd) {
>  	case CPU_PM_ENTER:
> @@ -716,7 +696,6 @@ static int cti_cpu_pm_notify(struct notifier_block
> *nb, unsigned long cmd,
>  	}
> 
>  cti_notify_exit:
> -	raw_spin_unlock(&drvdata->spinlock);
>  	return notify_res;
>  }
> 
> @@ -743,11 +722,12 @@ static int cti_dying_cpu(unsigned int cpu)
>  	if (!drvdata)
>  		return 0;
> 
> -	raw_spin_lock(&drvdata->spinlock);
> +	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
> +
>  	drvdata->config.hw_powered = false;
>  	if (drvdata->config.hw_enabled)
>  		coresight_disclaim_device(drvdata->csdev);
> -	raw_spin_unlock(&drvdata->spinlock);
> +
>  	return 0;
>  }
> 
> diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> index
> 572b80ee96fbf18ec8cf9abc30d109a676dfbc5d..455d08bcccd49a3f1eac8ab
> d8246806ef73a9ab6 100644
> --- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> @@ -84,11 +84,11 @@ static ssize_t enable_show(struct device *dev,
>  	bool enabled, powered;
>  	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
> 
> -	raw_spin_lock(&drvdata->spinlock);
> -	enable_req = drvdata->config.enable_req_count;
> -	powered = drvdata->config.hw_powered;
> -	enabled = drvdata->config.hw_enabled;
> -	raw_spin_unlock(&drvdata->spinlock);
> +	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock) {
> +		enable_req = drvdata->config.enable_req_count;
> +		powered = drvdata->config.hw_powered;
> +		enabled = drvdata->config.hw_enabled;
> +	}
> 
>  	if (powered)
>  		return sprintf(buf, "%d\n", enabled);
> @@ -134,9 +134,8 @@ static ssize_t powered_show(struct device *dev,
>  	bool powered;
>  	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
> 
> -	raw_spin_lock(&drvdata->spinlock);
> -	powered = drvdata->config.hw_powered;
> -	raw_spin_unlock(&drvdata->spinlock);
> +	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock)
> +		powered = drvdata->config.hw_powered;
> 
>  	return sprintf(buf, "%d\n", powered);
>  }
> @@ -181,10 +180,12 @@ static ssize_t coresight_cti_reg_show(struct device
> *dev,
>  	u32 val = 0;
> 
>  	pm_runtime_get_sync(dev->parent);
> -	raw_spin_lock(&drvdata->spinlock);
> -	if (drvdata->config.hw_powered)
> -		val = readl_relaxed(drvdata->base + cti_attr->off);
> -	raw_spin_unlock(&drvdata->spinlock);
> +
> +	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock) {
> +		if (drvdata->config.hw_powered)
> +			val = readl_relaxed(drvdata->base + cti_attr->off);
> +	}
> +
>  	pm_runtime_put_sync(dev->parent);
>  	return sysfs_emit(buf, "0x%x\n", val);
>  }
> @@ -202,10 +203,12 @@ static __maybe_unused ssize_t
> coresight_cti_reg_store(struct device *dev,
>  		return -EINVAL;
> 
>  	pm_runtime_get_sync(dev->parent);
> -	raw_spin_lock(&drvdata->spinlock);
> -	if (drvdata->config.hw_powered)
> -		cti_write_single_reg(drvdata, cti_attr->off, val);
> -	raw_spin_unlock(&drvdata->spinlock);
> +
> +	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock) {
> +		if (drvdata->config.hw_powered)
> +			cti_write_single_reg(drvdata, cti_attr->off, val);
> +	}
> +
>  	pm_runtime_put_sync(dev->parent);
>  	return size;
>  }
> @@ -264,17 +267,18 @@ static ssize_t cti_reg32_show(struct device *dev,
> char *buf,
>  	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
>  	struct cti_config *config = &drvdata->config;
> 
> -	raw_spin_lock(&drvdata->spinlock);
> -	if ((reg_offset >= 0) && cti_active(config)) {
> -		CS_UNLOCK(drvdata->base);
> -		val = readl_relaxed(drvdata->base + reg_offset);
> -		if (pcached_val)
> -			*pcached_val = val;
> -		CS_LOCK(drvdata->base);
> -	} else if (pcached_val) {
> -		val = *pcached_val;
> +	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock) {
> +		if ((reg_offset >= 0) && cti_active(config)) {
> +			CS_UNLOCK(drvdata->base);
> +			val = readl_relaxed(drvdata->base + reg_offset);
> +			if (pcached_val)
> +				*pcached_val = val;
> +			CS_LOCK(drvdata->base);
> +		} else if (pcached_val) {
> +			val = *pcached_val;
> +		}
>  	}
> -	raw_spin_unlock(&drvdata->spinlock);
> +
>  	return sprintf(buf, "%#x\n", val);
>  }
> 
> @@ -293,15 +297,16 @@ static ssize_t cti_reg32_store(struct device *dev,
> const char *buf,
>  	if (kstrtoul(buf, 0, &val))
>  		return -EINVAL;
> 
> -	raw_spin_lock(&drvdata->spinlock);
> -	/* local store */
> -	if (pcached_val)
> -		*pcached_val = (u32)val;
> +	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock) {
> +		/* local store */
> +		if (pcached_val)
> +			*pcached_val = (u32)val;
> +
> +		/* write through if offset and enabled */
> +		if ((reg_offset >= 0) && cti_active(config))
> +			cti_write_single_reg(drvdata, reg_offset, val);
> +	}
> 
> -	/* write through if offset and enabled */
> -	if ((reg_offset >= 0) && cti_active(config))
> -		cti_write_single_reg(drvdata, reg_offset, val);
> -	raw_spin_unlock(&drvdata->spinlock);
>  	return size;
>  }
> 
> @@ -349,9 +354,9 @@ static ssize_t inout_sel_store(struct device *dev,
>  	if (val > (CTIINOUTEN_MAX - 1))
>  		return -EINVAL;
> 
> -	raw_spin_lock(&drvdata->spinlock);
> +	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
> +
>  	drvdata->config.ctiinout_sel = val;
> -	raw_spin_unlock(&drvdata->spinlock);
>  	return size;
>  }
>  static DEVICE_ATTR_RW(inout_sel);
> @@ -364,10 +369,11 @@ static ssize_t inen_show(struct device *dev,
>  	int index;
>  	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
> 
> -	raw_spin_lock(&drvdata->spinlock);
> -	index = drvdata->config.ctiinout_sel;
> -	val = drvdata->config.ctiinen[index];
> -	raw_spin_unlock(&drvdata->spinlock);
> +	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock) {
> +		index = drvdata->config.ctiinout_sel;
> +		val = drvdata->config.ctiinen[index];
> +	}
> +
>  	return sprintf(buf, "%#lx\n", val);
>  }
> 
> @@ -383,14 +389,15 @@ static ssize_t inen_store(struct device *dev,
>  	if (kstrtoul(buf, 0, &val))
>  		return -EINVAL;
> 
> -	raw_spin_lock(&drvdata->spinlock);
> +	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
> +
>  	index = config->ctiinout_sel;
>  	config->ctiinen[index] = val;
> 
>  	/* write through if enabled */
>  	if (cti_active(config))
>  		cti_write_single_reg(drvdata, CTIINEN(index), val);
> -	raw_spin_unlock(&drvdata->spinlock);
> +
>  	return size;
>  }
>  static DEVICE_ATTR_RW(inen);
> @@ -403,10 +410,11 @@ static ssize_t outen_show(struct device *dev,
>  	int index;
>  	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
> 
> -	raw_spin_lock(&drvdata->spinlock);
> -	index = drvdata->config.ctiinout_sel;
> -	val = drvdata->config.ctiouten[index];
> -	raw_spin_unlock(&drvdata->spinlock);
> +	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock) {
> +		index = drvdata->config.ctiinout_sel;
> +		val = drvdata->config.ctiouten[index];
> +	}
> +
>  	return sprintf(buf, "%#lx\n", val);
>  }
> 
> @@ -422,14 +430,15 @@ static ssize_t outen_store(struct device *dev,
>  	if (kstrtoul(buf, 0, &val))
>  		return -EINVAL;
> 
> -	raw_spin_lock(&drvdata->spinlock);
> +	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
> +
>  	index = config->ctiinout_sel;
>  	config->ctiouten[index] = val;
> 
>  	/* write through if enabled */
>  	if (cti_active(config))
>  		cti_write_single_reg(drvdata, CTIOUTEN(index), val);
> -	raw_spin_unlock(&drvdata->spinlock);
> +
>  	return size;
>  }
>  static DEVICE_ATTR_RW(outen);
> @@ -463,7 +472,7 @@ static ssize_t appclear_store(struct device *dev,
>  	if (kstrtoul(buf, 0, &val))
>  		return -EINVAL;
> 
> -	raw_spin_lock(&drvdata->spinlock);
> +	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
> 
>  	/* a 1'b1 in appclr clears down the same bit in appset*/
>  	config->ctiappset &= ~val;
> @@ -471,7 +480,7 @@ static ssize_t appclear_store(struct device *dev,
>  	/* write through if enabled */
>  	if (cti_active(config))
>  		cti_write_single_reg(drvdata, CTIAPPCLEAR, val);
> -	raw_spin_unlock(&drvdata->spinlock);
> +
>  	return size;
>  }
>  static DEVICE_ATTR_WO(appclear);
> @@ -487,12 +496,12 @@ static ssize_t apppulse_store(struct device *dev,
>  	if (kstrtoul(buf, 0, &val))
>  		return -EINVAL;
> 
> -	raw_spin_lock(&drvdata->spinlock);
> +	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
> 
>  	/* write through if enabled */
>  	if (cti_active(config))
>  		cti_write_single_reg(drvdata, CTIAPPPULSE, val);
> -	raw_spin_unlock(&drvdata->spinlock);
> +
>  	return size;
>  }
>  static DEVICE_ATTR_WO(apppulse);
> @@ -681,9 +690,9 @@ static ssize_t trig_filter_enable_show(struct device
> *dev,
>  	u32 val;
>  	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
> 
> -	raw_spin_lock(&drvdata->spinlock);
> -	val = drvdata->config.trig_filter_enable;
> -	raw_spin_unlock(&drvdata->spinlock);
> +	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock)
> +		val = drvdata->config.trig_filter_enable;
> +
>  	return sprintf(buf, "%d\n", val);
>  }
> 
> @@ -697,9 +706,9 @@ static ssize_t trig_filter_enable_store(struct device
> *dev,
>  	if (kstrtoul(buf, 0, &val))
>  		return -EINVAL;
> 
> -	raw_spin_lock(&drvdata->spinlock);
> +	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
> +
>  	drvdata->config.trig_filter_enable = !!val;
> -	raw_spin_unlock(&drvdata->spinlock);
>  	return size;
>  }
>  static DEVICE_ATTR_RW(trig_filter_enable);
> @@ -728,7 +737,7 @@ static ssize_t chan_xtrigs_reset_store(struct device
> *dev,
>  	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
>  	struct cti_config *config = &drvdata->config;
> 
> -	raw_spin_lock(&drvdata->spinlock);
> +	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
> 
>  	/* clear the CTI trigger / channel programming registers */
>  	for (i = 0; i < config->nr_trig_max; i++) {
> @@ -747,7 +756,6 @@ static ssize_t chan_xtrigs_reset_store(struct device
> *dev,
>  	if (cti_active(config))
>  		cti_write_all_hw_regs(drvdata);
> 
> -	raw_spin_unlock(&drvdata->spinlock);
>  	return size;
>  }
>  static DEVICE_ATTR_WO(chan_xtrigs_reset);
> @@ -768,9 +776,9 @@ static ssize_t chan_xtrigs_sel_store(struct device
> *dev,
>  	if (val > (drvdata->config.nr_ctm_channels - 1))
>  		return -EINVAL;
> 
> -	raw_spin_lock(&drvdata->spinlock);
> +	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
> +
>  	drvdata->config.xtrig_rchan_sel = val;
> -	raw_spin_unlock(&drvdata->spinlock);
>  	return size;
>  }
> 
> @@ -781,9 +789,8 @@ static ssize_t chan_xtrigs_sel_show(struct device
> *dev,
>  	unsigned long val;
>  	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
> 
> -	raw_spin_lock(&drvdata->spinlock);
> -	val = drvdata->config.xtrig_rchan_sel;
> -	raw_spin_unlock(&drvdata->spinlock);
> +	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock)
> +		val = drvdata->config.xtrig_rchan_sel;
> 
>  	return sprintf(buf, "%ld\n", val);
>  }
> @@ -838,12 +845,12 @@ static ssize_t print_chan_list(struct device *dev,
>  	unsigned long inuse_bits = 0, chan_mask;
> 
>  	/* scan regs to get bitmap of channels in use. */
> -	raw_spin_lock(&drvdata->spinlock);
> -	for (i = 0; i < config->nr_trig_max; i++) {
> -		inuse_bits |= config->ctiinen[i];
> -		inuse_bits |= config->ctiouten[i];
> +	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock) {
> +		for (i = 0; i < config->nr_trig_max; i++) {
> +			inuse_bits |= config->ctiinen[i];
> +			inuse_bits |= config->ctiouten[i];
> +		}
>  	}
> -	raw_spin_unlock(&drvdata->spinlock);
> 
>  	/* inverse bits if printing free channels */
>  	if (!inuse)
> 
> --
> 2.34.1

Reviewed-by: Mike Leach <mike.leach@arm.com>

[PATCH 2/8] coresight: cti: Fix register reads

[Leo Yan]

Introduce cti_read_single_reg() as an interface for reading registers
with unlocking the CS lock.  Consolidate register read in sysfs
interfaces using this new helper.

Fixes: b5213376c240 ("coresight: cti: Add sysfs access to program function registers")
Fixes: 1a556ca6cc24 ("coresight: cti: Add sysfs coresight mgmt register access")
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-cti-core.c  | 11 +++++++++++
 drivers/hwtracing/coresight/coresight-cti-sysfs.c |  6 ++----
 drivers/hwtracing/coresight/coresight-cti.h       |  1 +
 3 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c
index b75c4ec2c8d5d58da8761334643dba34001b48c7..2d8b1cbe5bf5c9ce0383dc90335f0085b22a7f61 100644
--- a/drivers/hwtracing/coresight/coresight-cti-core.c
+++ b/drivers/hwtracing/coresight/coresight-cti-core.c
@@ -165,6 +165,17 @@ static int cti_disable_hw(struct cti_drvdata *drvdata)
 	return 0;
 }
 
+u32 cti_read_single_reg(struct cti_drvdata *drvdata, int offset)
+{
+	int val;
+
+	CS_UNLOCK(drvdata->base);
+	val = readl_relaxed(drvdata->base + offset);
+	CS_LOCK(drvdata->base);
+
+	return val;
+}
+
 void cti_write_single_reg(struct cti_drvdata *drvdata, int offset, u32 value)
 {
 	CS_UNLOCK(drvdata->base);
diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
index 455d08bcccd49a3f1eac8abd8246806ef73a9ab6..9a997b2f090472761e9734fffc534663df8b06c6 100644
--- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
@@ -183,7 +183,7 @@ static ssize_t coresight_cti_reg_show(struct device *dev,
 
 	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock) {
 		if (drvdata->config.hw_powered)
-			val = readl_relaxed(drvdata->base + cti_attr->off);
+			val = cti_read_single_reg(drvdata, cti_attr->off);
 	}
 
 	pm_runtime_put_sync(dev->parent);
@@ -269,11 +269,9 @@ static ssize_t cti_reg32_show(struct device *dev, char *buf,
 
 	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock) {
 		if ((reg_offset >= 0) && cti_active(config)) {
-			CS_UNLOCK(drvdata->base);
-			val = readl_relaxed(drvdata->base + reg_offset);
+			val = cti_read_single_reg(drvdata, reg_offset);
 			if (pcached_val)
 				*pcached_val = val;
-			CS_LOCK(drvdata->base);
 		} else if (pcached_val) {
 			val = *pcached_val;
 		}
diff --git a/drivers/hwtracing/coresight/coresight-cti.h b/drivers/hwtracing/coresight/coresight-cti.h
index 4f89091ee93f5fb046d93b97a4085051fca6b39d..64f7324f098e8b5f90c6554e3872e3bf01988717 100644
--- a/drivers/hwtracing/coresight/coresight-cti.h
+++ b/drivers/hwtracing/coresight/coresight-cti.h
@@ -222,6 +222,7 @@ int cti_disable(struct coresight_device *csdev, struct coresight_path *path);
 void cti_write_all_hw_regs(struct cti_drvdata *drvdata);
 void cti_write_intack(struct device *dev, u32 ackval);
 void cti_write_single_reg(struct cti_drvdata *drvdata, int offset, u32 value);
+u32 cti_read_single_reg(struct cti_drvdata *drvdata, int offset);
 int cti_channel_trig_op(struct device *dev, enum cti_chan_op op,
 			enum cti_trig_dir direction, u32 channel_idx,
 			u32 trigger_idx);

-- 
2.34.1

RE: [PATCH 2/8] coresight: cti: Fix register reads

[Mike Leach]

> -----Original Message-----
> From: Leo Yan <leo.yan@arm.com>
> Sent: Monday, February 9, 2026 6:01 PM
> To: Suzuki Poulose <Suzuki.Poulose@arm.com>; Mike Leach
> <Mike.Leach@arm.com>; James Clark <james.clark@linaro.org>; Alexander
> Shishkin <alexander.shishkin@linux.intel.com>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>; Mathieu Poirier
> <mathieu.poirier@linaro.org>; Tingwei Zhang <quic_tingwei@quicinc.com>;
> Yingchao Deng <yingchao.deng@oss.qualcomm.com>; Jie Gan
> <jie.gan@oss.qualcomm.com>
> Cc: coresight@lists.linaro.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; Leo Yan <Leo.Yan@arm.com>
> Subject: [PATCH 2/8] coresight: cti: Fix register reads
> 
> Introduce cti_read_single_reg() as an interface for reading registers
> with unlocking the CS lock.  Consolidate register read in sysfs
> interfaces using this new helper.
> 
> Fixes: b5213376c240 ("coresight: cti: Add sysfs access to program function
> registers")
> Fixes: 1a556ca6cc24 ("coresight: cti: Add sysfs coresight mgmt register
> access")
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-cti-core.c  | 11 +++++++++++
>  drivers/hwtracing/coresight/coresight-cti-sysfs.c |  6 ++----
>  drivers/hwtracing/coresight/coresight-cti.h       |  1 +
>  3 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c
> b/drivers/hwtracing/coresight/coresight-cti-core.c
> index
> b75c4ec2c8d5d58da8761334643dba34001b48c7..2d8b1cbe5bf5c9ce0383
> dc90335f0085b22a7f61 100644
> --- a/drivers/hwtracing/coresight/coresight-cti-core.c
> +++ b/drivers/hwtracing/coresight/coresight-cti-core.c
> @@ -165,6 +165,17 @@ static int cti_disable_hw(struct cti_drvdata
> *drvdata)
>  	return 0;
>  }
> 
> +u32 cti_read_single_reg(struct cti_drvdata *drvdata, int offset)
> +{
> +	int val;
> +
> +	CS_UNLOCK(drvdata->base);
> +	val = readl_relaxed(drvdata->base + offset);
> +	CS_LOCK(drvdata->base);
> +
> +	return val;
> +}
> +
>  void cti_write_single_reg(struct cti_drvdata *drvdata, int offset, u32 value)
>  {
>  	CS_UNLOCK(drvdata->base);
> diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> index
> 455d08bcccd49a3f1eac8abd8246806ef73a9ab6..9a997b2f090472761e973
> 4fffc534663df8b06c6 100644
> --- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> @@ -183,7 +183,7 @@ static ssize_t coresight_cti_reg_show(struct device
> *dev,
> 
>  	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock) {
>  		if (drvdata->config.hw_powered)
> -			val = readl_relaxed(drvdata->base + cti_attr->off);
> +			val = cti_read_single_reg(drvdata, cti_attr->off);
>  	}
> 
>  	pm_runtime_put_sync(dev->parent);
> @@ -269,11 +269,9 @@ static ssize_t cti_reg32_show(struct device *dev,
> char *buf,
> 
>  	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock) {
>  		if ((reg_offset >= 0) && cti_active(config)) {
> -			CS_UNLOCK(drvdata->base);
> -			val = readl_relaxed(drvdata->base + reg_offset);
> +			val = cti_read_single_reg(drvdata, reg_offset);
>  			if (pcached_val)
>  				*pcached_val = val;
> -			CS_LOCK(drvdata->base);
>  		} else if (pcached_val) {
>  			val = *pcached_val;
>  		}
> diff --git a/drivers/hwtracing/coresight/coresight-cti.h
> b/drivers/hwtracing/coresight/coresight-cti.h
> index
> 4f89091ee93f5fb046d93b97a4085051fca6b39d..64f7324f098e8b5f90c65
> 54e3872e3bf01988717 100644
> --- a/drivers/hwtracing/coresight/coresight-cti.h
> +++ b/drivers/hwtracing/coresight/coresight-cti.h
> @@ -222,6 +222,7 @@ int cti_disable(struct coresight_device *csdev, struct
> coresight_path *path);
>  void cti_write_all_hw_regs(struct cti_drvdata *drvdata);
>  void cti_write_intack(struct device *dev, u32 ackval);
>  void cti_write_single_reg(struct cti_drvdata *drvdata, int offset, u32 value);
> +u32 cti_read_single_reg(struct cti_drvdata *drvdata, int offset);
>  int cti_channel_trig_op(struct device *dev, enum cti_chan_op op,
>  			enum cti_trig_dir direction, u32 channel_idx,
>  			u32 trigger_idx);
> 
> --
> 2.34.1

Reviewed-by: Mike Leach <mike.leach@arm.com>

[PATCH 3/8] coresight: cti: Access ASICCTL only when implemented

[Leo Yan]

According to the Arm ARM (DDI 0487 L.b), ASICCTL is implemented only
when CTIDEVID.EXTMUXNUM is non-zero.

Based on CTIDEVID.EXTMUXNUM, add a flag 'asicctl_impl' to indicate
whether the register is implemented, and access ASICCTL conditionally
based on the flag.

Allow the sysfs node to be visible only when the register is
implemented.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-cti-core.c  |  5 ++++-
 drivers/hwtracing/coresight/coresight-cti-sysfs.c | 13 +++++++++++++
 drivers/hwtracing/coresight/coresight-cti.h       |  2 ++
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c
index 2d8b1cbe5bf5c9ce0383dc90335f0085b22a7f61..1950e9b757ae4879a2671ddaf5675c54aa7956d5 100644
--- a/drivers/hwtracing/coresight/coresight-cti-core.c
+++ b/drivers/hwtracing/coresight/coresight-cti-core.c
@@ -77,7 +77,8 @@ void cti_write_all_hw_regs(struct cti_drvdata *drvdata)
 
 	/* other regs */
 	writel_relaxed(config->ctigate, drvdata->base + CTIGATE);
-	writel_relaxed(config->asicctl, drvdata->base + ASICCTL);
+	if (config->asicctl_impl)
+		writel_relaxed(config->asicctl, drvdata->base + ASICCTL);
 	writel_relaxed(config->ctiappset, drvdata->base + CTIAPPSET);
 
 	/* re-enable CTI */
@@ -230,6 +231,8 @@ static void cti_set_default_config(struct device *dev,
 	config->trig_filter_enable = true;
 	config->ctigate = GENMASK(config->nr_ctm_channels - 1, 0);
 	config->enable_req_count = 0;
+
+	config->asicctl_impl = !!FIELD_GET(GENMASK(4, 0), devid);
 }
 
 /*
diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
index 9a997b2f090472761e9734fffc534663df8b06c6..c15a580f6e90f57b1376e0b883a27700966feb1a 100644
--- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
@@ -537,6 +537,18 @@ static struct attribute *coresight_cti_regs_attrs[] = {
 	NULL,
 };
 
+static umode_t coresight_cti_regs_is_visible(struct kobject *kobj,
+					     struct attribute *attr, int idx)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+	if (attr == &dev_attr_asicctl.attr && !drvdata->config.asicctl_impl)
+		return 0;
+
+	return attr->mode;
+}
+
 /* CTI channel x-trigger programming */
 static int
 cti_trig_op_parse(struct device *dev, enum cti_chan_op op,
@@ -1174,6 +1186,7 @@ static const struct attribute_group coresight_cti_mgmt_group = {
 
 static const struct attribute_group coresight_cti_regs_group = {
 	.attrs = coresight_cti_regs_attrs,
+	.is_visible = coresight_cti_regs_is_visible,
 	.name = "regs",
 };
 
diff --git a/drivers/hwtracing/coresight/coresight-cti.h b/drivers/hwtracing/coresight/coresight-cti.h
index 64f7324f098e8b5f90c6554e3872e3bf01988717..7a3e7f806dcb093e504f1aacb3b29564bea28f6c 100644
--- a/drivers/hwtracing/coresight/coresight-cti.h
+++ b/drivers/hwtracing/coresight/coresight-cti.h
@@ -119,6 +119,7 @@ struct cti_device {
  * @nr_trig_max: Max number of trigger signals implemented on device.
  *		 (max of trig_in or trig_out) - from ID register.
  * @nr_ctm_channels: number of available CTM channels - from ID register.
+ * @asicctl_impl: true if asicctl is implemented.
  * @enable_req_count: CTI is enabled alongside >=1 associated devices.
  * @hw_enabled: true if hw is currently enabled.
  * @hw_powered: true if associated cpu powered on, or no cpu.
@@ -140,6 +141,7 @@ struct cti_config {
 	/* hardware description */
 	int nr_ctm_channels;
 	int nr_trig_max;
+	bool asicctl_impl;
 
 	/* cti enable control */
 	int enable_req_count;

-- 
2.34.1

RE: [PATCH 3/8] coresight: cti: Access ASICCTL only when implemented

[Mike Leach]

> -----Original Message-----
> From: Leo Yan <leo.yan@arm.com>
> Sent: Monday, February 9, 2026 6:01 PM
> To: Suzuki Poulose <Suzuki.Poulose@arm.com>; Mike Leach
> <Mike.Leach@arm.com>; James Clark <james.clark@linaro.org>; Alexander
> Shishkin <alexander.shishkin@linux.intel.com>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>; Mathieu Poirier
> <mathieu.poirier@linaro.org>; Tingwei Zhang <quic_tingwei@quicinc.com>;
> Yingchao Deng <yingchao.deng@oss.qualcomm.com>; Jie Gan
> <jie.gan@oss.qualcomm.com>
> Cc: coresight@lists.linaro.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; Leo Yan <Leo.Yan@arm.com>
> Subject: [PATCH 3/8] coresight: cti: Access ASICCTL only when implemented
> 
> According to the Arm ARM (DDI 0487 L.b), ASICCTL is implemented only
> when CTIDEVID.EXTMUXNUM is non-zero.
> 
> Based on CTIDEVID.EXTMUXNUM, add a flag 'asicctl_impl' to indicate
> whether the register is implemented, and access ASICCTL conditionally
> based on the flag.
> 
> Allow the sysfs node to be visible only when the register is
> implemented.
> 
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-cti-core.c  |  5 ++++-
>  drivers/hwtracing/coresight/coresight-cti-sysfs.c | 13 +++++++++++++
>  drivers/hwtracing/coresight/coresight-cti.h       |  2 ++
>  3 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c
> b/drivers/hwtracing/coresight/coresight-cti-core.c
> index
> 2d8b1cbe5bf5c9ce0383dc90335f0085b22a7f61..1950e9b757ae4879a267
> 1ddaf5675c54aa7956d5 100644
> --- a/drivers/hwtracing/coresight/coresight-cti-core.c
> +++ b/drivers/hwtracing/coresight/coresight-cti-core.c
> @@ -77,7 +77,8 @@ void cti_write_all_hw_regs(struct cti_drvdata *drvdata)
> 
>  	/* other regs */
>  	writel_relaxed(config->ctigate, drvdata->base + CTIGATE);
> -	writel_relaxed(config->asicctl, drvdata->base + ASICCTL);
> +	if (config->asicctl_impl)
> +		writel_relaxed(config->asicctl, drvdata->base + ASICCTL);
>  	writel_relaxed(config->ctiappset, drvdata->base + CTIAPPSET);
> 
>  	/* re-enable CTI */
> @@ -230,6 +231,8 @@ static void cti_set_default_config(struct device *dev,
>  	config->trig_filter_enable = true;
>  	config->ctigate = GENMASK(config->nr_ctm_channels - 1, 0);
>  	config->enable_req_count = 0;
> +
> +	config->asicctl_impl = !!FIELD_GET(GENMASK(4, 0), devid);
>  }
> 
>  /*
> diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> index
> 9a997b2f090472761e9734fffc534663df8b06c6..c15a580f6e90f57b1376e0
> b883a27700966feb1a 100644
> --- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> @@ -537,6 +537,18 @@ static struct attribute *coresight_cti_regs_attrs[] = {
>  	NULL,
>  };
> 
> +static umode_t coresight_cti_regs_is_visible(struct kobject *kobj,
> +					     struct attribute *attr, int idx)
> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +
> +	if (attr == &dev_attr_asicctl.attr && !drvdata->config.asicctl_impl)
> +		return 0;
> +
> +	return attr->mode;
> +}
> +
>  /* CTI channel x-trigger programming */
>  static int
>  cti_trig_op_parse(struct device *dev, enum cti_chan_op op,
> @@ -1174,6 +1186,7 @@ static const struct attribute_group
> coresight_cti_mgmt_group = {
> 
>  static const struct attribute_group coresight_cti_regs_group = {
>  	.attrs = coresight_cti_regs_attrs,
> +	.is_visible = coresight_cti_regs_is_visible,
>  	.name = "regs",
>  };
> 
> diff --git a/drivers/hwtracing/coresight/coresight-cti.h
> b/drivers/hwtracing/coresight/coresight-cti.h
> index
> 64f7324f098e8b5f90c6554e3872e3bf01988717..7a3e7f806dcb093e504f1
> aacb3b29564bea28f6c 100644
> --- a/drivers/hwtracing/coresight/coresight-cti.h
> +++ b/drivers/hwtracing/coresight/coresight-cti.h
> @@ -119,6 +119,7 @@ struct cti_device {
>   * @nr_trig_max: Max number of trigger signals implemented on device.
>   *		 (max of trig_in or trig_out) - from ID register.
>   * @nr_ctm_channels: number of available CTM channels - from ID register.
> + * @asicctl_impl: true if asicctl is implemented.
>   * @enable_req_count: CTI is enabled alongside >=1 associated devices.
>   * @hw_enabled: true if hw is currently enabled.
>   * @hw_powered: true if associated cpu powered on, or no cpu.
> @@ -140,6 +141,7 @@ struct cti_config {
>  	/* hardware description */
>  	int nr_ctm_channels;
>  	int nr_trig_max;
> +	bool asicctl_impl;
> 
>  	/* cti enable control */
>  	int enable_req_count;
> 
> --
> 2.34.1

Reviewed-by: Mike Leach <mike.leach@arm.com>

[PATCH 4/8] coresight: cti: Remove CPU power management code

[Leo Yan]

According Arm ARM, the CTI ASICCTL register:

  "It is IMPLEMENTATION DEFINED whether ASICCTL is implemented in the
   Core power domain or in the Debug power domain."

This is the only CTI register that may reside in the core power domain.
However, it has been confirmed that Arm designed CTIs place ASICCTL in
the debug power domain.  Furthermore, ASICCTL is implemented only when
CTIDEVID.EXTMUXNUM is non-zero, which is a rare case for CPU CTIs.

For these reasons, it is safe to conclude that all CTI registers are not
located in the CPU power domain. Therefore, the CTI driver does not need
CPU power management.

This commit removes the CPU power management from CTI driver.

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

diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c
index 1950e9b757ae4879a2671ddaf5675c54aa7956d5..3becef607e5ec5225cb6fd616da804903651fdf1 100644
--- a/drivers/hwtracing/coresight/coresight-cti-core.c
+++ b/drivers/hwtracing/coresight/coresight-cti-core.c
@@ -42,12 +42,6 @@ static DEFINE_MUTEX(ect_mutex);
 #define csdev_to_cti_drvdata(csdev)	\
 	dev_get_drvdata(csdev->dev.parent)
 
-/* power management handling */
-static int nr_cti_cpu;
-
-/* quick lookup list for CPU bound CTIs when power handling */
-static struct cti_drvdata *cti_cpu_drvdata[NR_CPUS];
-
 /*
  * CTI naming. CTI bound to cores will have the name cti_cpu<N> where
  * N is the CPU ID. System CTIs will have the name cti_sys<I> where I
@@ -113,28 +107,6 @@ static int cti_enable_hw(struct cti_drvdata *drvdata)
 	return 0;
 }
 
-/* re-enable CTI on CPU when using CPU hotplug */
-static void cti_cpuhp_enable_hw(struct cti_drvdata *drvdata)
-{
-	struct cti_config *config = &drvdata->config;
-
-	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
-
-	config->hw_powered = true;
-
-	/* no need to do anything if no enable request */
-	if (!drvdata->config.enable_req_count)
-		return;
-
-	/* try to claim the device */
-	if (coresight_claim_device(drvdata->csdev))
-		return;
-
-	cti_write_all_hw_regs(drvdata);
-	config->hw_enabled = true;
-	return;
-}
-
 /* disable hardware */
 static int cti_disable_hw(struct cti_drvdata *drvdata)
 {
@@ -652,146 +624,6 @@ static void cti_remove_conn_xrefs(struct cti_drvdata *drvdata)
 	}
 }
 
-/** cti PM callbacks **/
-static int cti_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd,
-			     void *v)
-{
-	struct cti_drvdata *drvdata;
-	struct coresight_device *csdev;
-	unsigned int cpu = smp_processor_id();
-	int notify_res = NOTIFY_OK;
-
-	if (!cti_cpu_drvdata[cpu])
-		return NOTIFY_OK;
-
-	drvdata = cti_cpu_drvdata[cpu];
-	csdev = drvdata->csdev;
-
-	if (WARN_ON_ONCE(drvdata->ctidev.cpu != cpu))
-		return NOTIFY_BAD;
-
-	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
-
-	switch (cmd) {
-	case CPU_PM_ENTER:
-		/* CTI regs all static - we have a copy & nothing to save */
-		drvdata->config.hw_powered = false;
-		if (drvdata->config.hw_enabled)
-			coresight_disclaim_device(csdev);
-		break;
-
-	case CPU_PM_ENTER_FAILED:
-		drvdata->config.hw_powered = true;
-		if (drvdata->config.hw_enabled) {
-			if (coresight_claim_device(csdev))
-				drvdata->config.hw_enabled = false;
-		}
-		break;
-
-	case CPU_PM_EXIT:
-		/* write hardware registers to re-enable. */
-		drvdata->config.hw_powered = true;
-		drvdata->config.hw_enabled = false;
-
-		/* check enable reference count to enable HW */
-		if (drvdata->config.enable_req_count) {
-			/* check we can claim the device as we re-power */
-			if (coresight_claim_device(csdev))
-				goto cti_notify_exit;
-
-			drvdata->config.hw_enabled = true;
-			cti_write_all_hw_regs(drvdata);
-		}
-		break;
-
-	default:
-		notify_res = NOTIFY_DONE;
-		break;
-	}
-
-cti_notify_exit:
-	return notify_res;
-}
-
-static struct notifier_block cti_cpu_pm_nb = {
-	.notifier_call = cti_cpu_pm_notify,
-};
-
-/* CPU HP handlers */
-static int cti_starting_cpu(unsigned int cpu)
-{
-	struct cti_drvdata *drvdata = cti_cpu_drvdata[cpu];
-
-	if (!drvdata)
-		return 0;
-
-	cti_cpuhp_enable_hw(drvdata);
-	return 0;
-}
-
-static int cti_dying_cpu(unsigned int cpu)
-{
-	struct cti_drvdata *drvdata = cti_cpu_drvdata[cpu];
-
-	if (!drvdata)
-		return 0;
-
-	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
-
-	drvdata->config.hw_powered = false;
-	if (drvdata->config.hw_enabled)
-		coresight_disclaim_device(drvdata->csdev);
-
-	return 0;
-}
-
-static int cti_pm_setup(struct cti_drvdata *drvdata)
-{
-	int ret;
-
-	if (drvdata->ctidev.cpu == -1)
-		return 0;
-
-	if (nr_cti_cpu)
-		goto done;
-
-	cpus_read_lock();
-	ret = cpuhp_setup_state_nocalls_cpuslocked(
-			CPUHP_AP_ARM_CORESIGHT_CTI_STARTING,
-			"arm/coresight_cti:starting",
-			cti_starting_cpu, cti_dying_cpu);
-	if (ret) {
-		cpus_read_unlock();
-		return ret;
-	}
-
-	ret = cpu_pm_register_notifier(&cti_cpu_pm_nb);
-	cpus_read_unlock();
-	if (ret) {
-		cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_CTI_STARTING);
-		return ret;
-	}
-
-done:
-	nr_cti_cpu++;
-	cti_cpu_drvdata[drvdata->ctidev.cpu] = drvdata;
-
-	return 0;
-}
-
-/* release PM registrations */
-static void cti_pm_release(struct cti_drvdata *drvdata)
-{
-	if (drvdata->ctidev.cpu == -1)
-		return;
-
-	cti_cpu_drvdata[drvdata->ctidev.cpu] = NULL;
-	if (--nr_cti_cpu == 0) {
-		cpu_pm_unregister_notifier(&cti_cpu_pm_nb);
-		cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_CTI_STARTING);
-	}
-}
-
 /** cti ect operations **/
 int cti_enable(struct coresight_device *csdev, enum cs_mode mode,
 	       struct coresight_path *path)
@@ -827,7 +659,6 @@ static void cti_device_release(struct device *dev)
 	struct cti_drvdata *ect_item, *ect_tmp;
 
 	mutex_lock(&ect_mutex);
-	cti_pm_release(drvdata);
 
 	/* remove from the list */
 	list_for_each_entry_safe(ect_item, ect_tmp, &ect_net, node) {
@@ -906,17 +737,12 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id)
 	if (!cti_desc.name)
 		return -ENOMEM;
 
-	/* setup CPU power management handling for CPU bound CTI devices. */
-	ret = cti_pm_setup(drvdata);
-	if (ret)
-		return ret;
-
 	/* create dynamic attributes for connections */
 	ret = cti_create_cons_sysfs(dev, drvdata);
 	if (ret) {
 		dev_err(dev, "%s: create dynamic sysfs entries failed\n",
 			cti_desc.name);
-		goto pm_release;
+		return ret;
 	}
 
 	/* set up coresight component description */
@@ -929,10 +755,8 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id)
 
 	coresight_clear_self_claim_tag(&cti_desc.access);
 	drvdata->csdev = coresight_register(&cti_desc);
-	if (IS_ERR(drvdata->csdev)) {
-		ret = PTR_ERR(drvdata->csdev);
-		goto pm_release;
-	}
+	if (IS_ERR(drvdata->csdev))
+		return PTR_ERR(drvdata->csdev);
 
 	/* add to list of CTI devices */
 	mutex_lock(&ect_mutex);
@@ -949,10 +773,6 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id)
 	pm_runtime_put(&adev->dev);
 	dev_info(&drvdata->csdev->dev, "CTI initialized\n");
 	return 0;
-
-pm_release:
-	cti_pm_release(drvdata);
-	return ret;
 }
 
 static struct amba_cs_uci_id uci_id_cti[] = {

-- 
2.34.1

RE: [PATCH 4/8] coresight: cti: Remove CPU power management code

[Mike Leach]

> -----Original Message-----
> From: Leo Yan <leo.yan@arm.com>
> Sent: Monday, February 9, 2026 6:01 PM
> To: Suzuki Poulose <Suzuki.Poulose@arm.com>; Mike Leach
> <Mike.Leach@arm.com>; James Clark <james.clark@linaro.org>; Alexander
> Shishkin <alexander.shishkin@linux.intel.com>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>; Mathieu Poirier
> <mathieu.poirier@linaro.org>; Tingwei Zhang <quic_tingwei@quicinc.com>;
> Yingchao Deng <yingchao.deng@oss.qualcomm.com>; Jie Gan
> <jie.gan@oss.qualcomm.com>
> Cc: coresight@lists.linaro.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; Leo Yan <Leo.Yan@arm.com>
> Subject: [PATCH 4/8] coresight: cti: Remove CPU power management code
> 
> According Arm ARM, the CTI ASICCTL register:
> 
>   "It is IMPLEMENTATION DEFINED whether ASICCTL is implemented in the
>    Core power domain or in the Debug power domain."
> 
> This is the only CTI register that may reside in the core power domain.
> However, it has been confirmed that Arm designed CTIs place ASICCTL in
> the debug power domain.  Furthermore, ASICCTL is implemented only when
> CTIDEVID.EXTMUXNUM is non-zero, which is a rare case for CPU CTIs.
> 
> For these reasons, it is safe to conclude that all CTI registers are not
> located in the CPU power domain. Therefore, the CTI driver does not need
> CPU power management.
> 
> This commit removes the CPU power management from CTI driver.
> 
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-cti-core.c | 186 +----------------------
>  1 file changed, 3 insertions(+), 183 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c
> b/drivers/hwtracing/coresight/coresight-cti-core.c
> index
> 1950e9b757ae4879a2671ddaf5675c54aa7956d5..3becef607e5ec5225cb6f
> d616da804903651fdf1 100644
> --- a/drivers/hwtracing/coresight/coresight-cti-core.c
> +++ b/drivers/hwtracing/coresight/coresight-cti-core.c
> @@ -42,12 +42,6 @@ static DEFINE_MUTEX(ect_mutex);
>  #define csdev_to_cti_drvdata(csdev)	\
>  	dev_get_drvdata(csdev->dev.parent)
> 
> -/* power management handling */
> -static int nr_cti_cpu;
> -
> -/* quick lookup list for CPU bound CTIs when power handling */
> -static struct cti_drvdata *cti_cpu_drvdata[NR_CPUS];
> -
>  /*
>   * CTI naming. CTI bound to cores will have the name cti_cpu<N> where
>   * N is the CPU ID. System CTIs will have the name cti_sys<I> where I
> @@ -113,28 +107,6 @@ static int cti_enable_hw(struct cti_drvdata *drvdata)
>  	return 0;
>  }
> 
> -/* re-enable CTI on CPU when using CPU hotplug */
> -static void cti_cpuhp_enable_hw(struct cti_drvdata *drvdata)
> -{
> -	struct cti_config *config = &drvdata->config;
> -
> -	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
> -
> -	config->hw_powered = true;
> -
> -	/* no need to do anything if no enable request */
> -	if (!drvdata->config.enable_req_count)
> -		return;
> -
> -	/* try to claim the device */
> -	if (coresight_claim_device(drvdata->csdev))
> -		return;
> -
> -	cti_write_all_hw_regs(drvdata);
> -	config->hw_enabled = true;
> -	return;
> -}
> -
>  /* disable hardware */
>  static int cti_disable_hw(struct cti_drvdata *drvdata)
>  {
> @@ -652,146 +624,6 @@ static void cti_remove_conn_xrefs(struct
> cti_drvdata *drvdata)
>  	}
>  }
> 
> -/** cti PM callbacks **/
> -static int cti_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd,
> -			     void *v)
> -{
> -	struct cti_drvdata *drvdata;
> -	struct coresight_device *csdev;
> -	unsigned int cpu = smp_processor_id();
> -	int notify_res = NOTIFY_OK;
> -
> -	if (!cti_cpu_drvdata[cpu])
> -		return NOTIFY_OK;
> -
> -	drvdata = cti_cpu_drvdata[cpu];
> -	csdev = drvdata->csdev;
> -
> -	if (WARN_ON_ONCE(drvdata->ctidev.cpu != cpu))
> -		return NOTIFY_BAD;
> -
> -	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
> -
> -	switch (cmd) {
> -	case CPU_PM_ENTER:
> -		/* CTI regs all static - we have a copy & nothing to save */
> -		drvdata->config.hw_powered = false;
> -		if (drvdata->config.hw_enabled)
> -			coresight_disclaim_device(csdev);
> -		break;
> -
> -	case CPU_PM_ENTER_FAILED:
> -		drvdata->config.hw_powered = true;
> -		if (drvdata->config.hw_enabled) {
> -			if (coresight_claim_device(csdev))
> -				drvdata->config.hw_enabled = false;
> -		}
> -		break;
> -
> -	case CPU_PM_EXIT:
> -		/* write hardware registers to re-enable. */
> -		drvdata->config.hw_powered = true;
> -		drvdata->config.hw_enabled = false;
> -
> -		/* check enable reference count to enable HW */
> -		if (drvdata->config.enable_req_count) {
> -			/* check we can claim the device as we re-power */
> -			if (coresight_claim_device(csdev))
> -				goto cti_notify_exit;
> -
> -			drvdata->config.hw_enabled = true;
> -			cti_write_all_hw_regs(drvdata);
> -		}
> -		break;
> -
> -	default:
> -		notify_res = NOTIFY_DONE;
> -		break;
> -	}
> -
> -cti_notify_exit:
> -	return notify_res;
> -}
> -
> -static struct notifier_block cti_cpu_pm_nb = {
> -	.notifier_call = cti_cpu_pm_notify,
> -};
> -
> -/* CPU HP handlers */
> -static int cti_starting_cpu(unsigned int cpu)
> -{
> -	struct cti_drvdata *drvdata = cti_cpu_drvdata[cpu];
> -
> -	if (!drvdata)
> -		return 0;
> -
> -	cti_cpuhp_enable_hw(drvdata);
> -	return 0;
> -}
> -
> -static int cti_dying_cpu(unsigned int cpu)
> -{
> -	struct cti_drvdata *drvdata = cti_cpu_drvdata[cpu];
> -
> -	if (!drvdata)
> -		return 0;
> -
> -	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
> -
> -	drvdata->config.hw_powered = false;
> -	if (drvdata->config.hw_enabled)
> -		coresight_disclaim_device(drvdata->csdev);
> -
> -	return 0;
> -}
> -
> -static int cti_pm_setup(struct cti_drvdata *drvdata)
> -{
> -	int ret;
> -
> -	if (drvdata->ctidev.cpu == -1)
> -		return 0;
> -
> -	if (nr_cti_cpu)
> -		goto done;
> -
> -	cpus_read_lock();
> -	ret = cpuhp_setup_state_nocalls_cpuslocked(
> -			CPUHP_AP_ARM_CORESIGHT_CTI_STARTING,
> -			"arm/coresight_cti:starting",
> -			cti_starting_cpu, cti_dying_cpu);
> -	if (ret) {
> -		cpus_read_unlock();
> -		return ret;
> -	}
> -
> -	ret = cpu_pm_register_notifier(&cti_cpu_pm_nb);
> -	cpus_read_unlock();
> -	if (ret) {
> -
> 	cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_CTI_STA
> RTING);
> -		return ret;
> -	}
> -
> -done:
> -	nr_cti_cpu++;
> -	cti_cpu_drvdata[drvdata->ctidev.cpu] = drvdata;
> -
> -	return 0;
> -}
> -
> -/* release PM registrations */
> -static void cti_pm_release(struct cti_drvdata *drvdata)
> -{
> -	if (drvdata->ctidev.cpu == -1)
> -		return;
> -
> -	cti_cpu_drvdata[drvdata->ctidev.cpu] = NULL;
> -	if (--nr_cti_cpu == 0) {
> -		cpu_pm_unregister_notifier(&cti_cpu_pm_nb);
> -
> 	cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_CTI_STA
> RTING);
> -	}
> -}
> -
>  /** cti ect operations **/
>  int cti_enable(struct coresight_device *csdev, enum cs_mode mode,
>  	       struct coresight_path *path)
> @@ -827,7 +659,6 @@ static void cti_device_release(struct device *dev)
>  	struct cti_drvdata *ect_item, *ect_tmp;
> 
>  	mutex_lock(&ect_mutex);
> -	cti_pm_release(drvdata);
> 
>  	/* remove from the list */
>  	list_for_each_entry_safe(ect_item, ect_tmp, &ect_net, node) {
> @@ -906,17 +737,12 @@ static int cti_probe(struct amba_device *adev,
> const struct amba_id *id)
>  	if (!cti_desc.name)
>  		return -ENOMEM;
> 
> -	/* setup CPU power management handling for CPU bound CTI
> devices. */
> -	ret = cti_pm_setup(drvdata);
> -	if (ret)
> -		return ret;
> -
>  	/* create dynamic attributes for connections */
>  	ret = cti_create_cons_sysfs(dev, drvdata);
>  	if (ret) {
>  		dev_err(dev, "%s: create dynamic sysfs entries failed\n",
>  			cti_desc.name);
> -		goto pm_release;
> +		return ret;
>  	}
> 
>  	/* set up coresight component description */
> @@ -929,10 +755,8 @@ static int cti_probe(struct amba_device *adev, const
> struct amba_id *id)
> 
>  	coresight_clear_self_claim_tag(&cti_desc.access);
>  	drvdata->csdev = coresight_register(&cti_desc);
> -	if (IS_ERR(drvdata->csdev)) {
> -		ret = PTR_ERR(drvdata->csdev);
> -		goto pm_release;
> -	}
> +	if (IS_ERR(drvdata->csdev))
> +		return PTR_ERR(drvdata->csdev);
> 
>  	/* add to list of CTI devices */
>  	mutex_lock(&ect_mutex);
> @@ -949,10 +773,6 @@ static int cti_probe(struct amba_device *adev, const
> struct amba_id *id)
>  	pm_runtime_put(&adev->dev);
>  	dev_info(&drvdata->csdev->dev, "CTI initialized\n");
>  	return 0;
> -
> -pm_release:
> -	cti_pm_release(drvdata);
> -	return ret;
>  }
> 
>  static struct amba_cs_uci_id uci_id_cti[] = {
> 
> --
> 2.34.1
Reviewed-by: Mike Leach <mike.leach@arm.com>

[PATCH 5/8] coresight: cti: Rename cti_active() to cti_is_active()

[Leo Yan]

Rename cti_active() to cti_is_active() to clarify that it checks whether
the CTI device is active.

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

diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c
index 3becef607e5ec5225cb6fd616da804903651fdf1..48db87e2fd0f1a669f2d20a0014108da215e26fd 100644
--- a/drivers/hwtracing/coresight/coresight-cti-core.c
+++ b/drivers/hwtracing/coresight/coresight-cti-core.c
@@ -164,7 +164,7 @@ void cti_write_intack(struct device *dev, u32 ackval)
 	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
 
 	/* write if enabled */
-	if (cti_active(config))
+	if (cti_is_active(config))
 		cti_write_single_reg(drvdata, CTIINTACK, ackval);
 }
 
@@ -351,7 +351,7 @@ int cti_channel_trig_op(struct device *dev, enum cti_chan_op op,
 		config->ctiouten[trigger_idx] = reg_value;
 
 	/* write through if enabled */
-	if (cti_active(config))
+	if (cti_is_active(config))
 		cti_write_single_reg(drvdata, reg_offset, reg_value);
 
 	return 0;
@@ -389,7 +389,7 @@ int cti_channel_gate_op(struct device *dev, enum cti_chan_gate_op op,
 	}
 	if (err == 0) {
 		config->ctigate = reg_value;
-		if (cti_active(config))
+		if (cti_is_active(config))
 			cti_write_single_reg(drvdata, CTIGATE, reg_value);
 	}
 
@@ -438,7 +438,7 @@ int cti_channel_setop(struct device *dev, enum cti_chan_set_op op,
 		break;
 	}
 
-	if ((err == 0) && cti_active(config))
+	if ((err == 0) && cti_is_active(config))
 		cti_write_single_reg(drvdata, reg_offset, reg_value);
 
 	return err;
diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
index c15a580f6e90f57b1376e0b883a27700966feb1a..a22cc9a2bee24eb6115e7adb61880cc86d03e12e 100644
--- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
@@ -268,7 +268,7 @@ static ssize_t cti_reg32_show(struct device *dev, char *buf,
 	struct cti_config *config = &drvdata->config;
 
 	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock) {
-		if ((reg_offset >= 0) && cti_active(config)) {
+		if ((reg_offset >= 0) && cti_is_active(config)) {
 			val = cti_read_single_reg(drvdata, reg_offset);
 			if (pcached_val)
 				*pcached_val = val;
@@ -301,7 +301,7 @@ static ssize_t cti_reg32_store(struct device *dev, const char *buf,
 			*pcached_val = (u32)val;
 
 		/* write through if offset and enabled */
-		if ((reg_offset >= 0) && cti_active(config))
+		if ((reg_offset >= 0) && cti_is_active(config))
 			cti_write_single_reg(drvdata, reg_offset, val);
 	}
 
@@ -393,7 +393,7 @@ static ssize_t inen_store(struct device *dev,
 	config->ctiinen[index] = val;
 
 	/* write through if enabled */
-	if (cti_active(config))
+	if (cti_is_active(config))
 		cti_write_single_reg(drvdata, CTIINEN(index), val);
 
 	return size;
@@ -434,7 +434,7 @@ static ssize_t outen_store(struct device *dev,
 	config->ctiouten[index] = val;
 
 	/* write through if enabled */
-	if (cti_active(config))
+	if (cti_is_active(config))
 		cti_write_single_reg(drvdata, CTIOUTEN(index), val);
 
 	return size;
@@ -476,7 +476,7 @@ static ssize_t appclear_store(struct device *dev,
 	config->ctiappset &= ~val;
 
 	/* write through if enabled */
-	if (cti_active(config))
+	if (cti_is_active(config))
 		cti_write_single_reg(drvdata, CTIAPPCLEAR, val);
 
 	return size;
@@ -497,7 +497,7 @@ static ssize_t apppulse_store(struct device *dev,
 	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
 
 	/* write through if enabled */
-	if (cti_active(config))
+	if (cti_is_active(config))
 		cti_write_single_reg(drvdata, CTIAPPPULSE, val);
 
 	return size;
@@ -763,7 +763,7 @@ static ssize_t chan_xtrigs_reset_store(struct device *dev,
 	config->xtrig_rchan_sel = 0;
 
 	/* if enabled then write through */
-	if (cti_active(config))
+	if (cti_is_active(config))
 		cti_write_all_hw_regs(drvdata);
 
 	return size;
diff --git a/drivers/hwtracing/coresight/coresight-cti.h b/drivers/hwtracing/coresight/coresight-cti.h
index 7a3e7f806dcb093e504f1aacb3b29564bea28f6c..400c1545b22bfe631144d24faceb354ff2d49166 100644
--- a/drivers/hwtracing/coresight/coresight-cti.h
+++ b/drivers/hwtracing/coresight/coresight-cti.h
@@ -238,7 +238,7 @@ coresight_cti_get_platform_data(struct device *dev);
 const char *cti_plat_get_node_name(struct fwnode_handle *fwnode);
 
 /* cti powered and enabled */
-static inline bool cti_active(struct cti_config *cfg)
+static inline bool cti_is_active(struct cti_config *cfg)
 {
 	return cfg->hw_powered && cfg->hw_enabled;
 }

-- 
2.34.1

RE: [PATCH 5/8] coresight: cti: Rename cti_active() to cti_is_active()

[Mike Leach]

> -----Original Message-----
> From: Leo Yan <leo.yan@arm.com>
> Sent: Monday, February 9, 2026 6:01 PM
> To: Suzuki Poulose <Suzuki.Poulose@arm.com>; Mike Leach
> <Mike.Leach@arm.com>; James Clark <james.clark@linaro.org>; Alexander
> Shishkin <alexander.shishkin@linux.intel.com>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>; Mathieu Poirier
> <mathieu.poirier@linaro.org>; Tingwei Zhang <quic_tingwei@quicinc.com>;
> Yingchao Deng <yingchao.deng@oss.qualcomm.com>; Jie Gan
> <jie.gan@oss.qualcomm.com>
> Cc: coresight@lists.linaro.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; Leo Yan <Leo.Yan@arm.com>
> Subject: [PATCH 5/8] coresight: cti: Rename cti_active() to cti_is_active()
> 
> Rename cti_active() to cti_is_active() to clarify that it checks whether
> the CTI device is active.
> 
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-cti-core.c  |  8 ++++----
>  drivers/hwtracing/coresight/coresight-cti-sysfs.c | 14 +++++++-------
>  drivers/hwtracing/coresight/coresight-cti.h       |  2 +-
>  3 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c
> b/drivers/hwtracing/coresight/coresight-cti-core.c
> index
> 3becef607e5ec5225cb6fd616da804903651fdf1..48db87e2fd0f1a669f2d20
> a0014108da215e26fd 100644
> --- a/drivers/hwtracing/coresight/coresight-cti-core.c
> +++ b/drivers/hwtracing/coresight/coresight-cti-core.c
> @@ -164,7 +164,7 @@ void cti_write_intack(struct device *dev, u32 ackval)
>  	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
> 
>  	/* write if enabled */
> -	if (cti_active(config))
> +	if (cti_is_active(config))
>  		cti_write_single_reg(drvdata, CTIINTACK, ackval);
>  }
> 
> @@ -351,7 +351,7 @@ int cti_channel_trig_op(struct device *dev, enum
> cti_chan_op op,
>  		config->ctiouten[trigger_idx] = reg_value;
> 
>  	/* write through if enabled */
> -	if (cti_active(config))
> +	if (cti_is_active(config))
>  		cti_write_single_reg(drvdata, reg_offset, reg_value);
> 
>  	return 0;
> @@ -389,7 +389,7 @@ int cti_channel_gate_op(struct device *dev, enum
> cti_chan_gate_op op,
>  	}
>  	if (err == 0) {
>  		config->ctigate = reg_value;
> -		if (cti_active(config))
> +		if (cti_is_active(config))
>  			cti_write_single_reg(drvdata, CTIGATE, reg_value);
>  	}
> 
> @@ -438,7 +438,7 @@ int cti_channel_setop(struct device *dev, enum
> cti_chan_set_op op,
>  		break;
>  	}
> 
> -	if ((err == 0) && cti_active(config))
> +	if ((err == 0) && cti_is_active(config))
>  		cti_write_single_reg(drvdata, reg_offset, reg_value);
> 
>  	return err;
> diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> index
> c15a580f6e90f57b1376e0b883a27700966feb1a..a22cc9a2bee24eb6115e7
> adb61880cc86d03e12e 100644
> --- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> @@ -268,7 +268,7 @@ static ssize_t cti_reg32_show(struct device *dev, char
> *buf,
>  	struct cti_config *config = &drvdata->config;
> 
>  	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock) {
> -		if ((reg_offset >= 0) && cti_active(config)) {
> +		if ((reg_offset >= 0) && cti_is_active(config)) {
>  			val = cti_read_single_reg(drvdata, reg_offset);
>  			if (pcached_val)
>  				*pcached_val = val;
> @@ -301,7 +301,7 @@ static ssize_t cti_reg32_store(struct device *dev,
> const char *buf,
>  			*pcached_val = (u32)val;
> 
>  		/* write through if offset and enabled */
> -		if ((reg_offset >= 0) && cti_active(config))
> +		if ((reg_offset >= 0) && cti_is_active(config))
>  			cti_write_single_reg(drvdata, reg_offset, val);
>  	}
> 
> @@ -393,7 +393,7 @@ static ssize_t inen_store(struct device *dev,
>  	config->ctiinen[index] = val;
> 
>  	/* write through if enabled */
> -	if (cti_active(config))
> +	if (cti_is_active(config))
>  		cti_write_single_reg(drvdata, CTIINEN(index), val);
> 
>  	return size;
> @@ -434,7 +434,7 @@ static ssize_t outen_store(struct device *dev,
>  	config->ctiouten[index] = val;
> 
>  	/* write through if enabled */
> -	if (cti_active(config))
> +	if (cti_is_active(config))
>  		cti_write_single_reg(drvdata, CTIOUTEN(index), val);
> 
>  	return size;
> @@ -476,7 +476,7 @@ static ssize_t appclear_store(struct device *dev,
>  	config->ctiappset &= ~val;
> 
>  	/* write through if enabled */
> -	if (cti_active(config))
> +	if (cti_is_active(config))
>  		cti_write_single_reg(drvdata, CTIAPPCLEAR, val);
> 
>  	return size;
> @@ -497,7 +497,7 @@ static ssize_t apppulse_store(struct device *dev,
>  	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
> 
>  	/* write through if enabled */
> -	if (cti_active(config))
> +	if (cti_is_active(config))
>  		cti_write_single_reg(drvdata, CTIAPPPULSE, val);
> 
>  	return size;
> @@ -763,7 +763,7 @@ static ssize_t chan_xtrigs_reset_store(struct device
> *dev,
>  	config->xtrig_rchan_sel = 0;
> 
>  	/* if enabled then write through */
> -	if (cti_active(config))
> +	if (cti_is_active(config))
>  		cti_write_all_hw_regs(drvdata);
> 
>  	return size;
> diff --git a/drivers/hwtracing/coresight/coresight-cti.h
> b/drivers/hwtracing/coresight/coresight-cti.h
> index
> 7a3e7f806dcb093e504f1aacb3b29564bea28f6c..400c1545b22bfe631144d
> 24faceb354ff2d49166 100644
> --- a/drivers/hwtracing/coresight/coresight-cti.h
> +++ b/drivers/hwtracing/coresight/coresight-cti.h
> @@ -238,7 +238,7 @@ coresight_cti_get_platform_data(struct device *dev);
>  const char *cti_plat_get_node_name(struct fwnode_handle *fwnode);
> 
>  /* cti powered and enabled */
> -static inline bool cti_active(struct cti_config *cfg)
> +static inline bool cti_is_active(struct cti_config *cfg)
>  {
>  	return cfg->hw_powered && cfg->hw_enabled;
>  }
> 
> --
> 2.34.1

Reviewed-by: Mike Leach <mike.leach@arm.com>

[PATCH 6/8] coresight: cti: Remove hw_powered flag

[Leo Yan]

Since the CPU PM code has been removed from the CTI driver and the device
is enabled via runtime PM, pm_runtime_active() can be used to check
whether the device is powered.

As a result, the hw_powered flag is redundant, remove it.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-cti-core.c  | 11 ++++-----
 drivers/hwtracing/coresight/coresight-cti-sysfs.c | 29 ++++++-----------------
 drivers/hwtracing/coresight/coresight-cti.h       |  6 ++---
 3 files changed, 13 insertions(+), 33 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c
index 48db87e2fd0f1a669f2d20a0014108da215e26fd..3b3b851a018851a8656eba64e638c61f3d04b99f 100644
--- a/drivers/hwtracing/coresight/coresight-cti-core.c
+++ b/drivers/hwtracing/coresight/coresight-cti-core.c
@@ -89,8 +89,8 @@ static int cti_enable_hw(struct cti_drvdata *drvdata)
 
 	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
 
-	/* no need to do anything if enabled or unpowered*/
-	if (config->hw_enabled || !config->hw_powered)
+	/* no need to do anything if enabled */
+	if (config->hw_enabled)
 		goto cti_state_unchanged;
 
 	/* claim the device */
@@ -123,8 +123,8 @@ static int cti_disable_hw(struct cti_drvdata *drvdata)
 	if (--drvdata->config.enable_req_count > 0)
 		return 0;
 
-	/* no need to do anything if disabled or cpu unpowered */
-	if (!config->hw_enabled || !config->hw_powered)
+	/* no need to do anything if disabled */
+	if (!config->hw_enabled)
 		return 0;
 
 	CS_UNLOCK(drvdata->base);
@@ -725,9 +725,6 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id)
 		return  PTR_ERR(pdata);
 	}
 
-	/* default to powered - could change on PM notifications */
-	drvdata->config.hw_powered = true;
-
 	/* set up device name - will depend if cpu bound or otherwise */
 	if (drvdata->ctidev.cpu >= 0)
 		cti_desc.name = devm_kasprintf(dev, GFP_KERNEL, "cti_cpu%d",
diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
index a22cc9a2bee24eb6115e7adb61880cc86d03e12e..9ab586a5c9a4fd2a64c542aaaaa625e2299edd62 100644
--- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
@@ -81,19 +81,12 @@ static ssize_t enable_show(struct device *dev,
 			   char *buf)
 {
 	int enable_req;
-	bool enabled, powered;
 	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
 
-	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock) {
+	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock)
 		enable_req = drvdata->config.enable_req_count;
-		powered = drvdata->config.hw_powered;
-		enabled = drvdata->config.hw_enabled;
-	}
 
-	if (powered)
-		return sprintf(buf, "%d\n", enabled);
-	else
-		return sprintf(buf, "%d\n", !!enable_req);
+	return sprintf(buf, "%d\n", !!enable_req);
 }
 
 static ssize_t enable_store(struct device *dev,
@@ -131,11 +124,7 @@ static ssize_t powered_show(struct device *dev,
 			    struct device_attribute *attr,
 			    char *buf)
 {
-	bool powered;
-	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
-
-	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock)
-		powered = drvdata->config.hw_powered;
+	bool powered = pm_runtime_active(dev->parent);
 
 	return sprintf(buf, "%d\n", powered);
 }
@@ -181,10 +170,8 @@ static ssize_t coresight_cti_reg_show(struct device *dev,
 
 	pm_runtime_get_sync(dev->parent);
 
-	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock) {
-		if (drvdata->config.hw_powered)
-			val = cti_read_single_reg(drvdata, cti_attr->off);
-	}
+	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock)
+		val = cti_read_single_reg(drvdata, cti_attr->off);
 
 	pm_runtime_put_sync(dev->parent);
 	return sysfs_emit(buf, "0x%x\n", val);
@@ -204,10 +191,8 @@ static __maybe_unused ssize_t coresight_cti_reg_store(struct device *dev,
 
 	pm_runtime_get_sync(dev->parent);
 
-	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock) {
-		if (drvdata->config.hw_powered)
-			cti_write_single_reg(drvdata, cti_attr->off, val);
-	}
+	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock)
+		cti_write_single_reg(drvdata, cti_attr->off, val);
 
 	pm_runtime_put_sync(dev->parent);
 	return size;
diff --git a/drivers/hwtracing/coresight/coresight-cti.h b/drivers/hwtracing/coresight/coresight-cti.h
index 400c1545b22bfe631144d24faceb354ff2d49166..c858847a5b80036fb48180ff7fbbfe684028cb89 100644
--- a/drivers/hwtracing/coresight/coresight-cti.h
+++ b/drivers/hwtracing/coresight/coresight-cti.h
@@ -122,7 +122,6 @@ struct cti_device {
  * @asicctl_impl: true if asicctl is implemented.
  * @enable_req_count: CTI is enabled alongside >=1 associated devices.
  * @hw_enabled: true if hw is currently enabled.
- * @hw_powered: true if associated cpu powered on, or no cpu.
  * @trig_in_use: bitfield of in triggers registered as in use.
  * @trig_out_use: bitfield of out triggers registered as in use.
  * @trig_out_filter: bitfield of out triggers that are blocked if filter
@@ -146,7 +145,6 @@ struct cti_config {
 	/* cti enable control */
 	int enable_req_count;
 	bool hw_enabled;
-	bool hw_powered;
 
 	/* registered triggers and filtering */
 	u32 trig_in_use;
@@ -237,10 +235,10 @@ struct coresight_platform_data *
 coresight_cti_get_platform_data(struct device *dev);
 const char *cti_plat_get_node_name(struct fwnode_handle *fwnode);
 
-/* cti powered and enabled */
+/* Check if a cti device is enabled */
 static inline bool cti_is_active(struct cti_config *cfg)
 {
-	return cfg->hw_powered && cfg->hw_enabled;
+	return cfg->hw_enabled;
 }
 
 #endif  /* _CORESIGHT_CORESIGHT_CTI_H */

-- 
2.34.1

RE: [PATCH 6/8] coresight: cti: Remove hw_powered flag

[Mike Leach]

> -----Original Message-----
> From: Leo Yan <leo.yan@arm.com>
> Sent: Monday, February 9, 2026 6:01 PM
> To: Suzuki Poulose <Suzuki.Poulose@arm.com>; Mike Leach
> <Mike.Leach@arm.com>; James Clark <james.clark@linaro.org>; Alexander
> Shishkin <alexander.shishkin@linux.intel.com>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>; Mathieu Poirier
> <mathieu.poirier@linaro.org>; Tingwei Zhang <quic_tingwei@quicinc.com>;
> Yingchao Deng <yingchao.deng@oss.qualcomm.com>; Jie Gan
> <jie.gan@oss.qualcomm.com>
> Cc: coresight@lists.linaro.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; Leo Yan <Leo.Yan@arm.com>
> Subject: [PATCH 6/8] coresight: cti: Remove hw_powered flag
> 
> Since the CPU PM code has been removed from the CTI driver and the device
> is enabled via runtime PM, pm_runtime_active() can be used to check
> whether the device is powered.
> 
> As a result, the hw_powered flag is redundant, remove it.
> 
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-cti-core.c  | 11 ++++-----
>  drivers/hwtracing/coresight/coresight-cti-sysfs.c | 29 ++++++-----------------
>  drivers/hwtracing/coresight/coresight-cti.h       |  6 ++---
>  3 files changed, 13 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c
> b/drivers/hwtracing/coresight/coresight-cti-core.c
> index
> 48db87e2fd0f1a669f2d20a0014108da215e26fd..3b3b851a018851a8656e
> ba64e638c61f3d04b99f 100644
> --- a/drivers/hwtracing/coresight/coresight-cti-core.c
> +++ b/drivers/hwtracing/coresight/coresight-cti-core.c
> @@ -89,8 +89,8 @@ static int cti_enable_hw(struct cti_drvdata *drvdata)
> 
>  	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
> 
> -	/* no need to do anything if enabled or unpowered*/
> -	if (config->hw_enabled || !config->hw_powered)
> +	/* no need to do anything if enabled */
> +	if (config->hw_enabled)
>  		goto cti_state_unchanged;
> 
>  	/* claim the device */
> @@ -123,8 +123,8 @@ static int cti_disable_hw(struct cti_drvdata *drvdata)
>  	if (--drvdata->config.enable_req_count > 0)
>  		return 0;
> 
> -	/* no need to do anything if disabled or cpu unpowered */
> -	if (!config->hw_enabled || !config->hw_powered)
> +	/* no need to do anything if disabled */
> +	if (!config->hw_enabled)
>  		return 0;
> 
>  	CS_UNLOCK(drvdata->base);
> @@ -725,9 +725,6 @@ static int cti_probe(struct amba_device *adev, const
> struct amba_id *id)
>  		return  PTR_ERR(pdata);
>  	}
> 
> -	/* default to powered - could change on PM notifications */
> -	drvdata->config.hw_powered = true;
> -
>  	/* set up device name - will depend if cpu bound or otherwise */
>  	if (drvdata->ctidev.cpu >= 0)
>  		cti_desc.name = devm_kasprintf(dev, GFP_KERNEL,
> "cti_cpu%d",
> diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> index
> a22cc9a2bee24eb6115e7adb61880cc86d03e12e..9ab586a5c9a4fd2a64c54
> 2aaaaa625e2299edd62 100644
> --- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> @@ -81,19 +81,12 @@ static ssize_t enable_show(struct device *dev,
>  			   char *buf)
>  {
>  	int enable_req;
> -	bool enabled, powered;
>  	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
> 
> -	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock) {
> +	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock)
>  		enable_req = drvdata->config.enable_req_count;
> -		powered = drvdata->config.hw_powered;
> -		enabled = drvdata->config.hw_enabled;
> -	}
> 
> -	if (powered)
> -		return sprintf(buf, "%d\n", enabled);
> -	else
> -		return sprintf(buf, "%d\n", !!enable_req);
> +	return sprintf(buf, "%d\n", !!enable_req);
>  }
> 
>  static ssize_t enable_store(struct device *dev,
> @@ -131,11 +124,7 @@ static ssize_t powered_show(struct device *dev,
>  			    struct device_attribute *attr,
>  			    char *buf)
>  {
> -	bool powered;
> -	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
> -
> -	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock)
> -		powered = drvdata->config.hw_powered;
> +	bool powered = pm_runtime_active(dev->parent);
> 
>  	return sprintf(buf, "%d\n", powered);
>  }
> @@ -181,10 +170,8 @@ static ssize_t coresight_cti_reg_show(struct device
> *dev,
> 
>  	pm_runtime_get_sync(dev->parent);
> 
> -	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock) {
> -		if (drvdata->config.hw_powered)
> -			val = cti_read_single_reg(drvdata, cti_attr->off);
> -	}
> +	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock)
> +		val = cti_read_single_reg(drvdata, cti_attr->off);
> 
>  	pm_runtime_put_sync(dev->parent);
>  	return sysfs_emit(buf, "0x%x\n", val);
> @@ -204,10 +191,8 @@ static __maybe_unused ssize_t
> coresight_cti_reg_store(struct device *dev,
> 
>  	pm_runtime_get_sync(dev->parent);
> 
> -	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock) {
> -		if (drvdata->config.hw_powered)
> -			cti_write_single_reg(drvdata, cti_attr->off, val);
> -	}
> +	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock)
> +		cti_write_single_reg(drvdata, cti_attr->off, val);
> 
>  	pm_runtime_put_sync(dev->parent);
>  	return size;
> diff --git a/drivers/hwtracing/coresight/coresight-cti.h
> b/drivers/hwtracing/coresight/coresight-cti.h
> index
> 400c1545b22bfe631144d24faceb354ff2d49166..c858847a5b80036fb4818
> 0ff7fbbfe684028cb89 100644
> --- a/drivers/hwtracing/coresight/coresight-cti.h
> +++ b/drivers/hwtracing/coresight/coresight-cti.h
> @@ -122,7 +122,6 @@ struct cti_device {
>   * @asicctl_impl: true if asicctl is implemented.
>   * @enable_req_count: CTI is enabled alongside >=1 associated devices.
>   * @hw_enabled: true if hw is currently enabled.
> - * @hw_powered: true if associated cpu powered on, or no cpu.
>   * @trig_in_use: bitfield of in triggers registered as in use.
>   * @trig_out_use: bitfield of out triggers registered as in use.
>   * @trig_out_filter: bitfield of out triggers that are blocked if filter
> @@ -146,7 +145,6 @@ struct cti_config {
>  	/* cti enable control */
>  	int enable_req_count;
>  	bool hw_enabled;
> -	bool hw_powered;
> 
>  	/* registered triggers and filtering */
>  	u32 trig_in_use;
> @@ -237,10 +235,10 @@ struct coresight_platform_data *
>  coresight_cti_get_platform_data(struct device *dev);
>  const char *cti_plat_get_node_name(struct fwnode_handle *fwnode);
> 
> -/* cti powered and enabled */
> +/* Check if a cti device is enabled */
>  static inline bool cti_is_active(struct cti_config *cfg)
>  {
> -	return cfg->hw_powered && cfg->hw_enabled;
> +	return cfg->hw_enabled;
>  }
> 
>  #endif  /* _CORESIGHT_CORESIGHT_CTI_H */
> 
> --
> 2.34.1

Reviewed-by: Mike Leach <mike.leach@arm.com>

[PATCH 7/8] coresight: cti: Remove hw_enabled flag

[Leo Yan]

The enable_req_count field already tracks whether the CTI device is
enabled.  A non-zero value indicates that the device is active, the
hw_enabled flag is redundant if so.

Remove hw_enabled and update cti_is_active() to check enable_req_count.
Replace open-coded enable_req_count checks with cti_is_active().

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-cti-core.c  | 11 ++---------
 drivers/hwtracing/coresight/coresight-cti-sysfs.c |  2 +-
 drivers/hwtracing/coresight/coresight-cti.h       |  4 +---
 3 files changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c
index 3b3b851a018851a8656eba64e638c61f3d04b99f..a7acee27d0aa2c842733f62b7ff88a4c296b51cc 100644
--- a/drivers/hwtracing/coresight/coresight-cti-core.c
+++ b/drivers/hwtracing/coresight/coresight-cti-core.c
@@ -90,7 +90,7 @@ static int cti_enable_hw(struct cti_drvdata *drvdata)
 	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
 
 	/* no need to do anything if enabled */
-	if (config->hw_enabled)
+	if (cti_is_active(config))
 		goto cti_state_unchanged;
 
 	/* claim the device */
@@ -100,8 +100,6 @@ static int cti_enable_hw(struct cti_drvdata *drvdata)
 
 	cti_write_all_hw_regs(drvdata);
 
-	config->hw_enabled = true;
-
 cti_state_unchanged:
 	drvdata->config.enable_req_count++;
 	return 0;
@@ -116,22 +114,17 @@ static int cti_disable_hw(struct cti_drvdata *drvdata)
 	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
 
 	/* don't allow negative refcounts, return an error */
-	if (!drvdata->config.enable_req_count)
+	if (!cti_is_active(config))
 		return -EINVAL;
 
 	/* check refcount - disable on 0 */
 	if (--drvdata->config.enable_req_count > 0)
 		return 0;
 
-	/* no need to do anything if disabled */
-	if (!config->hw_enabled)
-		return 0;
-
 	CS_UNLOCK(drvdata->base);
 
 	/* disable CTI */
 	writel_relaxed(0, drvdata->base + CTICONTROL);
-	config->hw_enabled = false;
 
 	coresight_disclaim_device_unlocked(csdev);
 	CS_LOCK(drvdata->base);
diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
index 9ab586a5c9a4fd2a64c542aaaaa625e2299edd62..9ef44956ebdc7781717d773fa014165989df2048 100644
--- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
@@ -84,7 +84,7 @@ static ssize_t enable_show(struct device *dev,
 	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
 
 	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock)
-		enable_req = drvdata->config.enable_req_count;
+		enable_req = cti_is_active(&drvdata->config);
 
 	return sprintf(buf, "%d\n", !!enable_req);
 }
diff --git a/drivers/hwtracing/coresight/coresight-cti.h b/drivers/hwtracing/coresight/coresight-cti.h
index c858847a5b80036fb48180ff7fbbfe684028cb89..fbb48eb5b0b6a571235d7fecb2c13fc294d8ba50 100644
--- a/drivers/hwtracing/coresight/coresight-cti.h
+++ b/drivers/hwtracing/coresight/coresight-cti.h
@@ -121,7 +121,6 @@ struct cti_device {
  * @nr_ctm_channels: number of available CTM channels - from ID register.
  * @asicctl_impl: true if asicctl is implemented.
  * @enable_req_count: CTI is enabled alongside >=1 associated devices.
- * @hw_enabled: true if hw is currently enabled.
  * @trig_in_use: bitfield of in triggers registered as in use.
  * @trig_out_use: bitfield of out triggers registered as in use.
  * @trig_out_filter: bitfield of out triggers that are blocked if filter
@@ -144,7 +143,6 @@ struct cti_config {
 
 	/* cti enable control */
 	int enable_req_count;
-	bool hw_enabled;
 
 	/* registered triggers and filtering */
 	u32 trig_in_use;
@@ -238,7 +236,7 @@ const char *cti_plat_get_node_name(struct fwnode_handle *fwnode);
 /* Check if a cti device is enabled */
 static inline bool cti_is_active(struct cti_config *cfg)
 {
-	return cfg->hw_enabled;
+	return !!cfg->enable_req_count;
 }
 
 #endif  /* _CORESIGHT_CORESIGHT_CTI_H */

-- 
2.34.1

RE: [PATCH 7/8] coresight: cti: Remove hw_enabled flag

[Mike Leach]

Hi Leo,


> -----Original Message-----
> From: Leo Yan <leo.yan@arm.com>
> Sent: Monday, February 9, 2026 6:01 PM
> To: Suzuki Poulose <Suzuki.Poulose@arm.com>; Mike Leach
> <Mike.Leach@arm.com>; James Clark <james.clark@linaro.org>; Alexander
> Shishkin <alexander.shishkin@linux.intel.com>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>; Mathieu Poirier
> <mathieu.poirier@linaro.org>; Tingwei Zhang <quic_tingwei@quicinc.com>;
> Yingchao Deng <yingchao.deng@oss.qualcomm.com>; Jie Gan
> <jie.gan@oss.qualcomm.com>
> Cc: coresight@lists.linaro.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; Leo Yan <Leo.Yan@arm.com>
> Subject: [PATCH 7/8] coresight: cti: Remove hw_enabled flag
> 
> The enable_req_count field already tracks whether the CTI device is
> enabled.  A non-zero value indicates that the device is active, the
> hw_enabled flag is redundant if so.
> 

Minor nit - removal of this flag actually started in the previous patch. If respinning might be worth combining this and the previous one.

> Remove hw_enabled and update cti_is_active() to check enable_req_count.
> Replace open-coded enable_req_count checks with cti_is_active().
> 
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-cti-core.c  | 11 ++---------
>  drivers/hwtracing/coresight/coresight-cti-sysfs.c |  2 +-
>  drivers/hwtracing/coresight/coresight-cti.h       |  4 +---
>  3 files changed, 4 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c
> b/drivers/hwtracing/coresight/coresight-cti-core.c
> index
> 3b3b851a018851a8656eba64e638c61f3d04b99f..a7acee27d0aa2c842733f
> 62b7ff88a4c296b51cc 100644
> --- a/drivers/hwtracing/coresight/coresight-cti-core.c
> +++ b/drivers/hwtracing/coresight/coresight-cti-core.c
> @@ -90,7 +90,7 @@ static int cti_enable_hw(struct cti_drvdata *drvdata)
>  	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
> 
>  	/* no need to do anything if enabled */
> -	if (config->hw_enabled)
> +	if (cti_is_active(config))
>  		goto cti_state_unchanged;
> 
>  	/* claim the device */
> @@ -100,8 +100,6 @@ static int cti_enable_hw(struct cti_drvdata *drvdata)
> 
>  	cti_write_all_hw_regs(drvdata);
> 
> -	config->hw_enabled = true;
> -
>  cti_state_unchanged:
>  	drvdata->config.enable_req_count++;
>  	return 0;
> @@ -116,22 +114,17 @@ static int cti_disable_hw(struct cti_drvdata
> *drvdata)
>  	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
> 
>  	/* don't allow negative refcounts, return an error */
> -	if (!drvdata->config.enable_req_count)
> +	if (!cti_is_active(config))
>  		return -EINVAL;
> 
>  	/* check refcount - disable on 0 */
>  	if (--drvdata->config.enable_req_count > 0)
>  		return 0;
> 
> -	/* no need to do anything if disabled */
> -	if (!config->hw_enabled)
> -		return 0;
> -
>  	CS_UNLOCK(drvdata->base);
> 
>  	/* disable CTI */
>  	writel_relaxed(0, drvdata->base + CTICONTROL);
> -	config->hw_enabled = false;
> 
>  	coresight_disclaim_device_unlocked(csdev);
>  	CS_LOCK(drvdata->base);
> diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> index
> 9ab586a5c9a4fd2a64c542aaaaa625e2299edd62..9ef44956ebdc7781717d7
> 73fa014165989df2048 100644
> --- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> @@ -84,7 +84,7 @@ static ssize_t enable_show(struct device *dev,
>  	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
> 
>  	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock)
> -		enable_req = drvdata->config.enable_req_count;
> +		enable_req = cti_is_active(&drvdata->config);
> 
>  	return sprintf(buf, "%d\n", !!enable_req);
>  }
> diff --git a/drivers/hwtracing/coresight/coresight-cti.h
> b/drivers/hwtracing/coresight/coresight-cti.h
> index
> c858847a5b80036fb48180ff7fbbfe684028cb89..fbb48eb5b0b6a571235d7f
> ecb2c13fc294d8ba50 100644
> --- a/drivers/hwtracing/coresight/coresight-cti.h
> +++ b/drivers/hwtracing/coresight/coresight-cti.h
> @@ -121,7 +121,6 @@ struct cti_device {
>   * @nr_ctm_channels: number of available CTM channels - from ID register.
>   * @asicctl_impl: true if asicctl is implemented.
>   * @enable_req_count: CTI is enabled alongside >=1 associated devices.
> - * @hw_enabled: true if hw is currently enabled.
>   * @trig_in_use: bitfield of in triggers registered as in use.
>   * @trig_out_use: bitfield of out triggers registered as in use.
>   * @trig_out_filter: bitfield of out triggers that are blocked if filter
> @@ -144,7 +143,6 @@ struct cti_config {
> 
>  	/* cti enable control */
>  	int enable_req_count;
> -	bool hw_enabled;
> 
>  	/* registered triggers and filtering */
>  	u32 trig_in_use;
> @@ -238,7 +236,7 @@ const char *cti_plat_get_node_name(struct
> fwnode_handle *fwnode);
>  /* Check if a cti device is enabled */
>  static inline bool cti_is_active(struct cti_config *cfg)
>  {
> -	return cfg->hw_enabled;
> +	return !!cfg->enable_req_count;
>  }
> 
>  #endif  /* _CORESIGHT_CORESIGHT_CTI_H */
> 
> --
> 2.34.1

Reviewed-by: Mike Leach <mike.leach@arm.com>

Re: [PATCH 7/8] coresight: cti: Remove hw_enabled flag

[Leo Yan]

Hi Mike,

On Fri, Feb 20, 2026 at 02:31:11PM +0000, Mike Leach wrote:

[...]

> > The enable_req_count field already tracks whether the CTI device is
> > enabled.  A non-zero value indicates that the device is active, the
> > hw_enabled flag is redundant if so.
> > 
> 
> Minor nit - removal of this flag actually started in the previous patch.
> If respinning might be worth combining this and the previous one.

In the new version, I will keep separate patches for removing the
hw_enabled and hw_powered flags, as they are two different things.
This would be easier to review and friendly for bisection.

I will take your suggestion in patch 08.

Thanks,
Leo

[PATCH 8/8] coresight: cti: Refactor cti_reg32_{show|store}()

[Leo Yan]

Return an error for any negative offset.  Since the cached value is used
to store user config, it is not updated when reading back the register
in cti_reg32_show().

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-cti-sysfs.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
index 9ef44956ebdc7781717d773fa014165989df2048..baac2a5dd467032fafbc6523d8885de59cb2665b 100644
--- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
@@ -252,14 +252,14 @@ static ssize_t cti_reg32_show(struct device *dev, char *buf,
 	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
 	struct cti_config *config = &drvdata->config;
 
+	if (reg_offset < 0)
+		return -EINVAL;
+
 	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock) {
-		if ((reg_offset >= 0) && cti_is_active(config)) {
+		if (cti_is_active(config))
 			val = cti_read_single_reg(drvdata, reg_offset);
-			if (pcached_val)
-				*pcached_val = val;
-		} else if (pcached_val) {
+		else if (pcached_val)
 			val = *pcached_val;
-		}
 	}
 
 	return sprintf(buf, "%#x\n", val);
@@ -280,13 +280,16 @@ static ssize_t cti_reg32_store(struct device *dev, const char *buf,
 	if (kstrtoul(buf, 0, &val))
 		return -EINVAL;
 
+	if (reg_offset < 0)
+		return -EINVAL;
+
 	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock) {
 		/* local store */
 		if (pcached_val)
 			*pcached_val = (u32)val;
 
 		/* write through if offset and enabled */
-		if ((reg_offset >= 0) && cti_is_active(config))
+		if (cti_is_active(config))
 			cti_write_single_reg(drvdata, reg_offset, val);
 	}
 

-- 
2.34.1

RE: [PATCH 8/8] coresight: cti: Refactor cti_reg32_{show|store}()

[Mike Leach]

Hi Leo,

> -----Original Message-----
> From: Leo Yan <leo.yan@arm.com>
> Sent: Monday, February 9, 2026 6:01 PM
> To: Suzuki Poulose <Suzuki.Poulose@arm.com>; Mike Leach
> <Mike.Leach@arm.com>; James Clark <james.clark@linaro.org>; Alexander
> Shishkin <alexander.shishkin@linux.intel.com>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>; Mathieu Poirier
> <mathieu.poirier@linaro.org>; Tingwei Zhang <quic_tingwei@quicinc.com>;
> Yingchao Deng <yingchao.deng@oss.qualcomm.com>; Jie Gan
> <jie.gan@oss.qualcomm.com>
> Cc: coresight@lists.linaro.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; Leo Yan <Leo.Yan@arm.com>
> Subject: [PATCH 8/8] coresight: cti: Refactor cti_reg32_{show|store}()
> 
> Return an error for any negative offset.  Since the cached value is used
> to store user config, it is not updated when reading back the register
> in cti_reg32_show().
> 
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-cti-sysfs.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> index
> 9ef44956ebdc7781717d773fa014165989df2048..baac2a5dd467032fafbc6
> 523d8885de59cb2665b 100644
> --- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> @@ -252,14 +252,14 @@ static ssize_t cti_reg32_show(struct device *dev,
> char *buf,
>  	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
>  	struct cti_config *config = &drvdata->config;
> 
> +	if (reg_offset < 0)
> +		return -EINVAL;
> +
>  	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock) {
> -		if ((reg_offset >= 0) && cti_is_active(config)) {
> +		if (cti_is_active(config))
>  			val = cti_read_single_reg(drvdata, reg_offset);
> -			if (pcached_val)
> -				*pcached_val = val;

I think we still need this. If a register with an none-zero default value / status that can change over time is read when active, then read when inactive, but never written, then the inactive value will not reflect the last read value.
Relatively minor issue but does represent a potential change in functionality for the driver - even if I cannot see specific issues for current ARM CTIs. This is a R/W cache so should be updated on both R and W.

> -		} else if (pcached_val) {
> +		else if (pcached_val)
>  			val = *pcached_val;
> -		}
>  	}
> 
>  	return sprintf(buf, "%#x\n", val);
> @@ -280,13 +280,16 @@ static ssize_t cti_reg32_store(struct device *dev,
> const char *buf,
>  	if (kstrtoul(buf, 0, &val))
>  		return -EINVAL;
> 
> +	if (reg_offset < 0)
> +		return -EINVAL;
> +
>  	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock) {
>  		/* local store */
>  		if (pcached_val)
>  			*pcached_val = (u32)val;
> 
>  		/* write through if offset and enabled */
> -		if ((reg_offset >= 0) && cti_is_active(config))
> +		if (cti_is_active(config))
>  			cti_write_single_reg(drvdata, reg_offset, val);
>  	}
> 
> 
> --
> 2.34.1

Regards

Mike

Re: [PATCH 8/8] coresight: cti: Refactor cti_reg32_{show|store}()

[Leo Yan]

Hi Mike,

On Fri, Feb 20, 2026 at 02:44:03PM +0000, Mike Leach wrote:

[...]

> > @@ -252,14 +252,14 @@ static ssize_t cti_reg32_show(struct device *dev,
> > char *buf,
> >  	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
> >  	struct cti_config *config = &drvdata->config;
> > 
> > +	if (reg_offset < 0)
> > +		return -EINVAL;
> > +
> >  	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock) {
> > -		if ((reg_offset >= 0) && cti_is_active(config)) {
> > +		if (cti_is_active(config))
> >  			val = cti_read_single_reg(drvdata, reg_offset);
> > -			if (pcached_val)
> > -				*pcached_val = val;
> 
> I think we still need this. If a register with an none-zero default
> value / status that can change over time is read when active, then
> read when inactive, but never written, then the inactive value will
> not reflect the last read value.

Let us consider below operations using your suggested approach:

  echo 0xab > /sys/bus/coresight/devices/cti_cpu0/regs/gate
  cat /sys/bus/coresight/devices/cti_cpu0/regs/gate

  echo 1 > /sys/bus/coresight/devices/cti_cpu0/enable
  cat /sys/bus/coresight/devices/cti_cpu0/regs/gate

  echo 0 > /sys/bus/coresight/devices/cti_cpu0/enable
  cat /sys/bus/coresight/devices/cti_cpu0/regs/gate

If a user reads the "gate" knob three times in the above flow, it ends
up having three different semantics.

 - The first "cat ../gate" outputs the user specified value (0xab).
 - The second "cat" outputs the real-time register value after the
   module has been enabled.
 - The third "cat" outputs the last read value before disabling.

This could be confusing for users (even for ourseleves), as it needs to
understand the meaning of the value under specific conditions and based
on prior operations.

To simplify the semantics in this patch, the "cat" returns the realtime
register value when the module is enabled; otherwise, it returns the
user configured value.  Since "*pcached_val" always stores the user
configuration, it remains consistent configurations across multiple
enable/disable cycles.

> Relatively minor issue but does represent a potential change in
> functionality for the driver - even if I cannot see specific issues
> for current ARM CTIs. This is a R/W cache so should be updated on
> both R and W.

"R/W cache" mixes the cached user configured value with the hardware
register dump and may become inconsistent across multiple
enable/disable cycles.

It seems to me that this is a bit overdesigned and complex. thoughts?

Thanks a lot for review!

Leo

Re: [PATCH 8/8] coresight: cti: Refactor cti_reg32_{show|store}()

[Mike Leach]

Hi Leo,

On 2/20/26 17:35, Leo Yan wrote:
> Hi Mike,
> 
> On Fri, Feb 20, 2026 at 02:44:03PM +0000, Mike Leach wrote:
> 
> [...]
> 
>>> @@ -252,14 +252,14 @@ static ssize_t cti_reg32_show(struct device *dev,
>>> char *buf,
>>>   	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>>   	struct cti_config *config = &drvdata->config;
>>>
>>> +	if (reg_offset < 0)
>>> +		return -EINVAL;
>>> +
>>>   	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock) {
>>> -		if ((reg_offset >= 0) && cti_is_active(config)) {
>>> +		if (cti_is_active(config))
>>>   			val = cti_read_single_reg(drvdata, reg_offset);
>>> -			if (pcached_val)
>>> -				*pcached_val = val;
>>
>> I think we still need this. If a register with an none-zero default
>> value / status that can change over time is read when active, then
>> read when inactive, but never written, then the inactive value will
>> not reflect the last read value.
> 
> Let us consider below operations using your suggested approach:
> 
>    echo 0xab > /sys/bus/coresight/devices/cti_cpu0/regs/gate
>    cat /sys/bus/coresight/devices/cti_cpu0/regs/gate
> 
>    echo 1 > /sys/bus/coresight/devices/cti_cpu0/enable
>    cat /sys/bus/coresight/devices/cti_cpu0/regs/gate
> 
>    echo 0 > /sys/bus/coresight/devices/cti_cpu0/enable
>    cat /sys/bus/coresight/devices/cti_cpu0/regs/gate
> 
> If a user reads the "gate" knob three times in the above flow, it ends
> up having three different semantics.
> 
>   - The first "cat ../gate" outputs the user specified value (0xab).
>   - The second "cat" outputs the real-time register value after the
>     module has been enabled.

which for a non volatile register will be 0xab as enable writes all the 
regs from the config to the device - or will have been written through 
the cache to the device by the echo command if the device was powered 
and available .

>   - The third "cat" outputs the last read value before disabling.
> 

Still 0xab unless this is a register that can change do to operation in 
which case it will be whatever the last noted value is.

All of the above is entirely expected and required. We should always 
show the user the latest value of the register. If the CTI is powered 
and available then this will be that directly read from the register.

The ETM is not different in this respect, except that the ETM reads all 
registers back on disable - and registers are architecturally forbidden 
to be written while the device is enabled - unlike the CTI where 
registers can be written after the device is enabled, and for some 
registers it makes sense only to write them when enabled.

With the CTI we do not readback all the regs at disable. They are demand 
read for sysfs, which is more efficient.


> This could be confusing for users (even for ourseleves), as it needs to
> understand the meaning of the value under specific conditions and based
> on prior operations.
> 

That is pretty much the definition of reading a hardware register on any 
device. You need to know if it can change during operation or not, if it 
is volatile under the conditions you are reading it.

> To simplify the semantics in this patch, the "cat" returns the realtime
> register value when the module is enabled; otherwise, it returns the
> user configured value.  Since "*pcached_val" always stores the user
> configuration, it remains consistent configurations across multiple
> enable/disable cycles.

The semantics of the cache are to use it if the registers are 
unavailable if not powered.


> 
>> Relatively minor issue but does represent a potential change in
>> functionality for the driver - even if I cannot see specific issues
>> for current ARM CTIs. This is a R/W cache so should be updated on
>> both R and W.
> 
> "R/W cache" mixes the cached user configured value with the hardware
> register dump and may become inconsistent across multiple
> enable/disable cycles.
> 
> It seems to me that this is a bit overdesigned and complex. thoughts?
> 

RW caching has been around for a long time - not sure 2 lines of code 
could be described as complex!

Regards

Mike

> Thanks a lot for review!
> 
> Leo