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...
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,
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)
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);
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;
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[] = {
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; }
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 */
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 */
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); }
-
Leo Yan