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