On 19/11/2019 23:19, Mike Leach wrote:
Adds in sysfs programming support for the CTI function register sets.
Allows direct manipulation of channel / trigger association registers.
Reviewed-by: Mathieu Poirier <mathieu.poirier@xxxxxxxxxx>
Signed-off-by: Mike Leach <mike.leach@xxxxxxxxxx>
---
.../hwtracing/coresight/coresight-cti-sysfs.c | 362 ++++++++++++++++++
drivers/hwtracing/coresight/coresight-cti.c | 19 +
drivers/hwtracing/coresight/coresight-cti.h | 5 +
3 files changed, 386 insertions(+)
diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
index 507f8eb487fe..02d3ee0c1278 100644
--- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
@@ -109,6 +109,362 @@ static struct attribute *coresight_cti_mgmt_attrs[] = {
NULL,
};
+/* CTI low level programming registers */
+
+/*
+ * Show a simple 32 bit value if enabled and powered.
+ * If inaccessible & pcached_val not NULL then show cached value.
+ */
Also I am not sure if it makes sense to mention that the value is cached.
+static ssize_t cti_reg32_show(struct device *dev, char *buf,
+ u32 *pcached_val, int reg_offset)
+{
+ u32 val = 0;
+ char *state = "";
+ struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
+ struct cti_config *config = &drvdata->config;
+
+ spin_lock(&drvdata->spinlock);
+ if ((reg_offset >= 0) && CTI_PWR_ENA(config)) {
minor nit: Personally I don't like the naming here. This could simply
be: cti_accessible(config) , may be defined as a static inline function
instead of a macro:
static inline bool cti_accessible(struct cti_drvdata *drvdata)
{
struct cti_config *cfg = &drvdata->config;
return cfg->hw_powered && cfg->hw_enabled;
}
+ 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;
+ state = " (cached)";
+ }
+ spin_unlock(&drvdata->spinlock);
+ return scnprintf(buf, PAGE_SIZE, "%#x\n", val);
+ return scnprintf(buf, PAGE_SIZE, "%#x%s\n", val, state);
+}
+
+/*
+ * Store a simple 32 bit value.
+ * If pcached_val not NULL, then copy to here too,
+ * if reg_offset >= 0 then write through if enabled.
+ */
+static ssize_t cti_reg32_store(struct device *dev, const char *buf,
+ size_t size, u32 *pcached_val, int reg_offset)
+static ssize_t appclear_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ unsigned long val;
+ struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
+ struct cti_config *config = &drvdata->config;
+
+ if (kstrtoul(buf, 0, &val))
+ return -EINVAL;
+
+ spin_lock(&drvdata->spinlock);
+
+ /* a 1'b1 in appclr clears down the same bit in appset*/
nit: a 0b1 ?
+ config->ctiappset &= ~val;
+
+ /* write through if enabled */
+ if (CTI_PWR_ENA(config))
+ cti_write_single_reg(drvdata, CTIAPPCLEAR, val);
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_WO(appclear);
+
Otherwise looks good to me.
Suzuki
