On Tue, 11 Jun 2019 at 22:43, Mathieu Poirier mathieu.poirier@linaro.org wrote:
On Wed, Jun 05, 2019 at 08:03:00PM +0100, Mike Leach wrote:
Adds in sysfs programming support for the CTI function register sets. Allows direct manipulation of channel / trigger association registers.
Signed-off-by: Mike Leach mike.leach@linaro.org
.../hwtracing/coresight/coresight-cti-sysfs.c | 402 ++++++++++++++++++ drivers/hwtracing/coresight/coresight-cti.c | 19 + drivers/hwtracing/coresight/coresight-cti.h | 6 + 3 files changed, 427 insertions(+)
diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c b/drivers/hwtracing/coresight/coresight-cti-sysfs.c index ad7c45390313..caecf011f6e0 100644 --- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c @@ -115,6 +115,402 @@ static struct attribute *coresight_cti_mgmt_attrs[] = { NULL, };
+/* CTI low level programming registers */
+/*
- Show a simple 32 bit value. If pval is NULL then live read,
- otherwise read from supplied pointer only.
- */
+static ssize_t cti_reg32_show(struct device *dev, char *buf,
u32 *pval, int reg_offset)+{
u32 val = 0;struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);struct cti_config *config = &drvdata->config;spin_lock(&drvdata->spinlock);if (pval) {val = *pval;} else if ((reg_offset >= 0) && CTI_PWR_ENA(config)) {CS_UNLOCK(drvdata->base);val = readl_relaxed(drvdata->base + reg_offset);CS_LOCK(drvdata->base);}spin_unlock(&drvdata->spinlock);return scnprintf(buf, PAGE_SIZE, "%#x\n", val);+}
+/*
- Store a simple 32 bit value.
- If pval 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 *pval,int reg_offset)reg_offset could have been on the second line.
agreed.
+{
unsigned long val;struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);struct cti_config *config = &drvdata->config;if (kstrtoul(buf, 16, &val))return -EINVAL;spin_lock(&drvdata->spinlock);/* local store */if (pval)*pval = (u32)val;/* write through of offset and enabled */if ((reg_offset >= 0) && CTI_PWR_ENA(config))cti_write_single_reg(drvdata, reg_offset, val);spin_unlock(&drvdata->spinlock);return size;+}
+static ssize_t inout_sel_show(struct device *dev,
struct device_attribute *attr,char *buf)+{
u32 val;struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);val = (u32)drvdata->config.ctiinout_sel;return scnprintf(buf, PAGE_SIZE, "%d\n", val);+}
+static ssize_t inout_sel_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);if (kstrtoul(buf, 10, &val))return -EINVAL;if (val > (CTIINOUTEN_MAX-1))return -EINVAL;spin_lock(&drvdata->spinlock);drvdata->config.ctiinout_sel = val;spin_unlock(&drvdata->spinlock);return size;+} +static DEVICE_ATTR_RW(inout_sel);
+static ssize_t inen_show(struct device *dev,
struct device_attribute *attr,char *buf)+{
unsigned long val;int index;struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);spin_lock(&drvdata->spinlock);index = drvdata->config.ctiinout_sel;val = drvdata->config.ctiinen[index];spin_unlock(&drvdata->spinlock);return scnprintf(buf, PAGE_SIZE, "INEN%d %#lx\n", index, val);+}
+static ssize_t inen_store(struct device *dev,
struct device_attribute *attr,const char *buf, size_t size)+{
unsigned long val;int index;struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);struct cti_config *config = &drvdata->config;if (kstrtoul(buf, 16, &val))return -EINVAL;spin_lock(&drvdata->spinlock);index = config->ctiinout_sel;config->ctiinen[index] = val;/* write through if enabled */if (CTI_PWR_ENA(config))cti_write_single_reg(drvdata, CTIINEN(index), val);spin_unlock(&drvdata->spinlock);return size;+} +static DEVICE_ATTR_RW(inen);
+static ssize_t outen_show(struct device *dev,
struct device_attribute *attr,char *buf)+{
unsigned long val;int index;struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);spin_lock(&drvdata->spinlock);index = drvdata->config.ctiinout_sel;val = drvdata->config.ctiouten[index];spin_unlock(&drvdata->spinlock);return scnprintf(buf, PAGE_SIZE, "OUTEN%d %#lx\n", index, val);+}
+static ssize_t outen_store(struct device *dev,
struct device_attribute *attr,const char *buf, size_t size)+{
unsigned long val;int index;struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);struct cti_config *config = &drvdata->config;if (kstrtoul(buf, 16, &val))return -EINVAL;spin_lock(&drvdata->spinlock);index = config->ctiinout_sel;config->ctiouten[index] = val;/* write through if enabled */if (CTI_PWR_ENA(config))cti_write_single_reg(drvdata, CTIOUTEN(index), val);spin_unlock(&drvdata->spinlock);return size;+} +static DEVICE_ATTR_RW(outen);
+static ssize_t gate_show(struct device *dev,
struct device_attribute *attr,char *buf)+{
struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);return cti_reg32_show(dev, buf, &drvdata->config.ctigate, -1);Why the -1 here? Wouldn't you want to see the real HW value if the device is enabled? The only logic I can see here is that if the HW is enabled then the value of CTIGATE *must* be the same as ->config.ctigate. The same comment/question for those below.
Logic of the cti_reg32_show() function means that if we provide a pointer to the cached value, then we use that irrespective of the -1 / offset provided. This in theory is more efficient than always reading back a value (and these static registers will _always_ be the same as the cached value).
Saying that, perhaps there is merit in changing the logic of the function to only use the cached value if we can't get at the real hw value - given that the cost of the read is low.
This has now been re-written as part of the "macro" changes suggested by suzuki
+}
+static ssize_t gate_store(struct device *dev,
struct device_attribute *attr,const char *buf, size_t size)+{
struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);return cti_reg32_store(dev, buf, size,&drvdata->config.ctigate, CTIGATE);+} +static DEVICE_ATTR_RW(gate);
+static ssize_t asicctl_show(struct device *dev,
struct device_attribute *attr,char *buf)+{
struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);return cti_reg32_show(dev, buf, &drvdata->config.asicctl, -1);+}
+static ssize_t asicctl_store(struct device *dev,
struct device_attribute *attr,const char *buf, size_t size)+{
struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);return cti_reg32_store(dev, buf, size,&drvdata->config.asicctl, ASICCTL);+} +static DEVICE_ATTR_RW(asicctl);
+static ssize_t intack_store(struct device *dev,
struct device_attribute *attr,const char *buf, size_t size)+{
unsigned long val;if (kstrtoul(buf, 16, &val))return -EINVAL;cti_write_intack(dev, val);return size;+} +static DEVICE_ATTR_WO(intack);
+static ssize_t appset_show(struct device *dev,
struct device_attribute *attr,char *buf)+{
struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);return cti_reg32_show(dev, buf, &drvdata->config.ctiappset, -1);+}
+static ssize_t appset_store(struct device *dev,
struct device_attribute *attr,const char *buf, size_t size)+{
struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);return cti_reg32_store(dev, buf, size,&drvdata->config.ctiappset, CTIAPPSET);+} +static DEVICE_ATTR_RW(appset);
+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, 16, &val))return -EINVAL;spin_lock(&drvdata->spinlock);/* a 1'b1 in appclr clears down the same bit in appset*/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);
+static ssize_t apppulse_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, 16, &val))return -EINVAL;spin_lock(&drvdata->spinlock);/** a 1'b1 in apppulse sets then clears the bit,* effectively clears down the same bit in appset*/config->ctiappset &= ~val;/* write through if enabled */if (CTI_PWR_ENA(config))cti_write_single_reg(drvdata, CTIAPPPULSE, val);spin_unlock(&drvdata->spinlock);return size;+} +static DEVICE_ATTR_WO(apppulse);
+/*
- #define CTI_DEBUG_INTEGRATION_CTRL to enable the access to the integration
- control registers. Normally only used to investigate connection data.
- */
+// #define CTI_DEBUG_INTEGRATION_CTRL
C++ style comment, please remove.
Done.
+#ifdef CTI_DEBUG_INTEGRATION_CTRL
+static ssize_t itchout_show(struct device *dev,
struct device_attribute *attr,char *buf)+{
return cti_reg32_show(dev, buf, NULL, ITCHOUT);+}
+static ssize_t itchout_store(struct device *dev,
struct device_attribute *attr,const char *buf, size_t size)+{
return cti_reg32_store(dev, buf, size, NULL, ITCHOUT);+} +static DEVICE_ATTR_RW(itchout);
+static ssize_t ittrigout_show(struct device *dev,
struct device_attribute *attr,char *buf)+{
return cti_reg32_show(dev, buf, NULL, ITTRIGOUT);+}
+static ssize_t ittrigout_store(struct device *dev,
struct device_attribute *attr,const char *buf, size_t size)+{
return cti_reg32_store(dev, buf, size, NULL, ITTRIGOUT);+} +static DEVICE_ATTR_RW(ittrigout);
+static ssize_t itchinack_store(struct device *dev,
struct device_attribute *attr,const char *buf, size_t size)+{
return cti_reg32_store(dev, buf, size, NULL, ITCHINACK);+} +static DEVICE_ATTR_WO(itchinack);
+static ssize_t ittriginack_store(struct device *dev,
struct device_attribute *attr,const char *buf, size_t size)+{
return cti_reg32_store(dev, buf, size, NULL, ITTRIGINACK);+} +static DEVICE_ATTR_WO(ittriginack);
+static ssize_t itctrl_show(struct device *dev,
struct device_attribute *attr,char *buf)+{
return cti_reg32_show(dev, buf, NULL, CORESIGHT_ITCTRL);+}
+static ssize_t itctrl_store(struct device *dev,
struct device_attribute *attr,const char *buf, size_t size)+{
return cti_reg32_store(dev, buf, size, NULL, CORESIGHT_ITCTRL);+} +static DEVICE_ATTR_RW(itctrl);
+coresight_cti_reg(ittrigin, ITTRIGIN); +coresight_cti_reg(itchin, ITCHIN); +coresight_cti_reg(itchoutack, ITCHOUTACK); +coresight_cti_reg(ittrigoutack, ITTRIGOUTACK);
+#endif /* CTI_DEBUG_INTEGRATION_CTRL */
+coresight_cti_reg(triginstatus, CTITRIGINSTATUS); +coresight_cti_reg(trigoutstatus, CTITRIGOUTSTATUS); +coresight_cti_reg(chinstatus, CTICHINSTATUS); +coresight_cti_reg(choutstatus, CTICHOUTSTATUS);
+static struct attribute *coresight_cti_regs_attrs[] = {
&dev_attr_inout_sel.attr,&dev_attr_inen.attr,&dev_attr_outen.attr,&dev_attr_gate.attr,&dev_attr_asicctl.attr,&dev_attr_intack.attr,&dev_attr_appset.attr,&dev_attr_appclear.attr,&dev_attr_apppulse.attr,&dev_attr_triginstatus.attr,&dev_attr_trigoutstatus.attr,&dev_attr_chinstatus.attr,&dev_attr_choutstatus.attr,+#ifdef CTI_DEBUG_INTEGRATION_CTRL
&dev_attr_itctrl.attr,&dev_attr_ittrigin.attr,&dev_attr_itchin.attr,&dev_attr_ittrigout.attr,&dev_attr_itchout.attr,&dev_attr_itchoutack.attr,&dev_attr_ittrigoutack.attr,&dev_attr_ittriginack.attr,&dev_attr_itchinack.attr,+#endif
NULL,+};
+/* sysfs groups */ static const struct attribute_group coresight_cti_group = { .attrs = coresight_cti_attrs, }; @@ -124,8 +520,14 @@ static const struct attribute_group coresight_cti_mgmt_group = { .name = "mgmt", };
+static const struct attribute_group coresight_cti_regs_group = {
.attrs = coresight_cti_regs_attrs,.name = "regs",+};
const struct attribute_group *coresight_cti_groups[] = { &coresight_cti_group, &coresight_cti_mgmt_group,
&coresight_cti_regs_group, NULL,}; diff --git a/drivers/hwtracing/coresight/coresight-cti.c b/drivers/hwtracing/coresight/coresight-cti.c index 4b3a5a91c4ff..cd804b29df07 100644 --- a/drivers/hwtracing/coresight/coresight-cti.c +++ b/drivers/hwtracing/coresight/coresight-cti.c @@ -158,6 +158,25 @@ static int cti_disable_hw(struct cti_drvdata *drvdata, bool update_refcount) return 0; }
+void cti_write_single_reg(struct cti_drvdata *drvdata, int offset, u32 value) +{
CS_UNLOCK(drvdata->base);writel_relaxed(value, drvdata->base+offset);CS_LOCK(drvdata->base);+}
+void cti_write_intack(struct device *dev, u32 ackval) +{
struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);struct cti_config *config = &drvdata->config;spin_lock(&drvdata->spinlock);/* write if enabled */if (CTI_PWR_ENA(config))cti_write_single_reg(drvdata, CTIINTACK, ackval);spin_unlock(&drvdata->spinlock);+}
static void cti_set_default_config(struct device *dev, struct cti_drvdata *drvdata) { diff --git a/drivers/hwtracing/coresight/coresight-cti.h b/drivers/hwtracing/coresight/coresight-cti.h index bc4c9bdf745e..215b4a4ac45d 100644 --- a/drivers/hwtracing/coresight/coresight-cti.h +++ b/drivers/hwtracing/coresight/coresight-cti.h @@ -205,7 +205,13 @@ extern const struct attribute_group *coresight_cti_groups[]; int cti_add_default_connection(struct cti_drvdata *drvdata); int cti_enable(struct coresight_device *csdev, void *__unused); int cti_disable(struct coresight_device *csdev, void *__unused); +void cti_write_intack(struct device *dev, u32 ackval); +void cti_write_single_reg(struct cti_drvdata *drvdata, int offset, u32 value);
struct coresight_platform_data * coresight_cti_get_platform_data(struct device *dev);
+/* cti powered and enabled */ +#define CTI_PWR_ENA(p_cfg) (p_cfg->hw_enabled && p_cfg->hw_powered)
#endif /* _CORESIGHT_CORESIGHT_CTI_H */
With the above,
Reviewed-by: Mathieu Poirier mathieu.poirier@linaro.org
Ok, thanks
Mike
-- 2.20.1
CoreSight mailing list CoreSight@lists.linaro.org https://lists.linaro.org/mailman/listinfo/coresight
-- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK