Re: [RFC PATCH v2 02/13] drivers: coresight: cti: Adds sysfs functionality to CTI driver.

On Wed, May 01, 2019 at 09:49:34AM +0100, Mike Leach wrote:

Adds in required sysfs access for:-

• CoreSight management registers.
• CTI device specific registers.
• Channel Cross triggering API.

I would really appreciate if you could split the patch along the line of the cathegories listed above. Otherwise it is really arduous to review.

This provides the basic programming interface for the device.

Signed-off-by: Mike Leach mike.leach@linaro.org

.../hwtracing/coresight/coresight-cti-sysfs.c | 831 +++++++++++++++++- drivers/hwtracing/coresight/coresight-cti.c | 188 +++- drivers/hwtracing/coresight/coresight-cti.h | 42 + drivers/hwtracing/coresight/coresight-priv.h | 2 + 4 files changed, 1053 insertions(+), 10 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c b/drivers/hwtracing/coresight/coresight-cti-sysfs.c index 9b6749621dcb..ff8dad02a779 100644 --- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c @@ -34,8 +34,8 @@ static ssize_t enable_show(struct device *dev, struct device_attribute *attr, char *buf) {

• int enable_req;
• bool enabled, powered, cpuid;

• int enable_req, cpuid;
• bool enabled, powered; struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent); ssize_t size = 0;

@@ -46,12 +46,18 @@ static ssize_t enable_show(struct device *dev, cpuid = drvdata->ctidev.cpu; spin_unlock(&drvdata->spinlock);

• if (powered) {

• size = scnprintf(buf, PAGE_SIZE, "cti %s; cpu%d powered;\n",
  

• 	enabled ? "enabled" : "disabled", cpuid);
  

• } else if (cpuid >= 0) {

• size = scnprintf(buf, PAGE_SIZE, "cti %s; cpu%d unpowered;\n",
  

• 	enable_req ? "enable req" : "disable req", cpuid);
  

• if (cpuid >= 0) {

• if (powered) {
  

• 	size = scnprintf(buf, PAGE_SIZE,
  

• 			 "cti %s; cpu%d powered;\n",
  

• 			 enabled ? "enabled" : "disabled",
  

• 			 cpuid);
  

• } else {
  

• 	size = scnprintf(buf, PAGE_SIZE,
  

• 		"cti %s; cpu%d unpowered;\n",
  

• 		enable_req ? "enable req" : "disable req",
  

• 		cpuid);
  

• }
  

This seems to be a re-arrangement of what was done in the previous patch - please move it there.

I am also not sure about keeping track of the device's power status but I will wait until I've seen the whole set to comment on that part.

} else { size = scnprintf(buf, PAGE_SIZE, "cti %s; no assoc cpu;\n", enabled ? "enabled" : "disabled"); @@ -81,17 +87,826 @@ static ssize_t enable_store(struct device *dev, } static DEVICE_ATTR_RW(enable); +static ssize_t ctmid_show(struct device *dev,

• 	      struct device_attribute *attr,
  

• 	      char *buf)
  

Indentation problems - the 's' and 'c' on the second and third line need to be aligned with the 's' of "struct" on the first line.

+{

• struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
• return scnprintf(buf, PAGE_SIZE, "0x%x\n", drvdata->ctidev.ctm_id);

Please drop the '0x' in "0x%x".

+} +static DEVICE_ATTR_RO(ctmid);

/* attribute and group sysfs tables. */ static struct attribute *coresight_cti_attrs[] = { &dev_attr_enable.attr,

• &dev_attr_ctmid.attr, NULL,

}; +/* register based attributes */ +#define coresight_cti_reg(name, offset) \

• coresight_simple_reg32(struct cti_drvdata, name, offset)

+/*

• • 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)
  

+{

• unsigned long val = 0;
• struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
• struct cti_config *config = &drvdata->config;
• spin_lock(&drvdata->spinlock);
• if (pval) {

• val = (unsigned long)*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, "%#lx\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)
  

+{

• 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;

+}

+/* coresight management registers */ +coresight_cti_reg(devaff0, CTIDEVAFF0); +coresight_cti_reg(devaff1, CTIDEVAFF1); +coresight_cti_reg(authstatus, CORESIGHT_AUTHSTATUS); +coresight_cti_reg(devarch, CORESIGHT_DEVARCH); +coresight_cti_reg(devid, CORESIGHT_DEVID); +coresight_cti_reg(devtype, CORESIGHT_DEVTYPE); +coresight_cti_reg(pidr0, CORESIGHT_PERIPHIDR0); +coresight_cti_reg(pidr1, CORESIGHT_PERIPHIDR1); +coresight_cti_reg(pidr2, CORESIGHT_PERIPHIDR2); +coresight_cti_reg(pidr3, CORESIGHT_PERIPHIDR3); +coresight_cti_reg(pidr4, CORESIGHT_PERIPHIDR4);

+static struct attribute *coresight_cti_mgmt_attrs[] = {

• &dev_attr_devaff0.attr,
• &dev_attr_devaff1.attr,
• &dev_attr_authstatus.attr,
• &dev_attr_devarch.attr,
• &dev_attr_devid.attr,
• &dev_attr_devtype.attr,
• &dev_attr_pidr0.attr,
• &dev_attr_pidr1.attr,
• &dev_attr_pidr2.attr,
• &dev_attr_pidr3.attr,
• &dev_attr_pidr4.attr,
• NULL,

+};

+/* CTI low level programming registers */ +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, "%#x\n", val);

+}

Function inen_show() and outen_show() are both rendering the value of ctiinout_sel as an integer but inout_sel_show() and inout_sel_store() are manipulating the same variable as an hexadecimal. I think the latter need to be integers as well.

+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, 16, &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);
  

I'm confused here - can you write to the device when it is enabled? If that is the case it is weird that in cti_write_all_hw_regs() the CTI needs to be disabled before the configuration registers are written.

• 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)
  

Indentation

+{

• 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)
  

Indentation

+{

• 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);

+}

+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)
  

Indentation

+{

• 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)
  

Indentation

+{

• 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)
  

Indentation

+{

• 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)
  

Indentation - there are too many of those to point out. Please address.

+{

• 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, mask;
• 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*/
• mask = ~val;
• config->ctiappset &= mask;
• /* 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, mask;
• 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
  

• */
  

• mask = ~val;
• config->ctiappset &= mask;
• /* 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);

+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);

Was it useful to you to make the integration registers available from sysfs? If so keep them, otherwiser I suggest removing as none of the other drivers have them. I'm especially weary that people will want to play with them, rendering the devices unusable and generating unwarranted phone calls.

A good compromise could be to enclose the test register with a in-file defined CTI_DEBUG. That way they are not visible unless someone really intends to enable them by recompiling their kernel.

+coresight_cti_reg(triginstatus, CTITRIGINSTATUS); +coresight_cti_reg(trigoutstatus, CTITRIGOUTSTATUS); +coresight_cti_reg(chinstatus, CTICHINSTATUS); +coresight_cti_reg(choutstatus, CTICHOUTSTATUS); +coresight_cti_reg(ittrigin, ITTRIGIN); +coresight_cti_reg(itchin, ITCHIN); +coresight_cti_reg(itchoutack, ITCHOUTACK); +coresight_cti_reg(ittrigoutack, ITTRIGOUTACK);

+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,
• &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,
• NULL,

+};

+/* CTI channel x-trigger programming */

There is more comments at the bottom of the patch. I will review the x-trigger programming section tomorrow.

+static int +cti_trig_op_parse(struct device *dev, enum cti_chan_op op,

•   enum cti_trig_dir dir, const char *buf, size_t size)
  

+{

• u32 chan_idx;
• u32 trig_idx;
• int items, err = size;
• /* extract chan idx and trigger idx */
• items = sscanf(buf, "%d %d", &chan_idx, &trig_idx);
• if (items) {

• err = cti_channel_trig_op(dev, op, dir, chan_idx, trig_idx);
  

• if (!err)
  

• 	err = size;
  

• } else

• err = -EINVAL;
  

• return err;

+}

+static ssize_t trigin_attach_store(struct device *dev,

• 	  struct device_attribute *attr,
  

• 	  const char *buf, size_t size)
  

+{

• return cti_trig_op_parse(dev, CTI_CHAN_ATTACH, CTI_TRIG_IN,

• 		 buf, size);
  

+} +static DEVICE_ATTR_WO(trigin_attach);

+static ssize_t trigin_detach_store(struct device *dev,

• 	  struct device_attribute *attr,
  

• 	  const char *buf, size_t size)
  

+{

• return cti_trig_op_parse(dev, CTI_CHAN_DETACH, CTI_TRIG_IN,

• 		 buf, size);
  

+} +static DEVICE_ATTR_WO(trigin_detach);

+static ssize_t trigout_attach_store(struct device *dev,

• 	  struct device_attribute *attr,
  

• 	  const char *buf, size_t size)
  

+{

• return cti_trig_op_parse(dev, CTI_CHAN_ATTACH, CTI_TRIG_OUT,

• 		 buf, size);
  

+} +static DEVICE_ATTR_WO(trigout_attach);

+static ssize_t trigout_detach_store(struct device *dev,

• 	  struct device_attribute *attr,
  

• 	  const char *buf, size_t size)
  

+{

• return cti_trig_op_parse(dev, CTI_CHAN_DETACH, CTI_TRIG_OUT,

• 		 buf, size);
  

+} +static DEVICE_ATTR_WO(trigout_detach);

+static ssize_t gate_enable_store(struct device *dev,

• 	 struct device_attribute *attr,
  

• 	 const char *buf, size_t size)
  

+{

• int err = 0, channel = 0;
• if ((size >= 3) && (strncmp(buf, "all", 3) == 0)) {

• err = cti_channel_gate_op(dev, CTI_GATE_CHAN_ENABLE_ALL, 0);
  

• } else {

• if (kstrtoint(buf, 0, &channel))
  

• 	return -EINVAL;
  

• err = cti_channel_gate_op(dev, CTI_GATE_CHAN_ENABLE, channel);
  

• }
• return err ? err : size;

+} +static DEVICE_ATTR_WO(gate_enable);

+static ssize_t gate_disable_store(struct device *dev,

• 	 struct device_attribute *attr,
  

• 	 const char *buf, size_t size)
  

+{

• int err = 0, channel = 0;
• if ((size >= 3) && (strncmp(buf, "all", 3) == 0)) {

• err = cti_channel_gate_op(dev, CTI_GATE_CHAN_DISABLE_ALL, 0);
  

• } else {

• if (kstrtoint(buf, 0, &channel))
  

• 	return -EINVAL;
  

• err = cti_channel_gate_op(dev, CTI_GATE_CHAN_DISABLE, channel);
  

• }
• return err ? err : size;

+} +static DEVICE_ATTR_WO(gate_disable);

+static int +chan_op_parse(struct device *dev, enum cti_chan_set_op op, const char *buf) +{

• int err = 0, channel = 0;
• if (kstrtoint(buf, 0, &channel))

• return -EINVAL;
  

• err = cti_channel_setop(dev, op, channel);
• return err;

+}

+static ssize_t chan_set_store(struct device *dev,

• 	 struct device_attribute *attr,
  

• 	 const char *buf, size_t size)
  

+{

• int err = chan_op_parse(dev, CTI_CHAN_SET, buf);
• return err ? err : size;

+} +static DEVICE_ATTR_WO(chan_set);

+static ssize_t chan_clear_store(struct device *dev,

• 	 struct device_attribute *attr,
  

• 	 const char *buf, size_t size)
  

+{

• int err = chan_op_parse(dev, CTI_CHAN_CLR, buf);
• return err ? err : size;

+} +static DEVICE_ATTR_WO(chan_clear);

+static ssize_t chan_pulse_store(struct device *dev,

• 	 struct device_attribute *attr,
  

• 	 const char *buf, size_t size)
  

+{

• int err = chan_op_parse(dev, CTI_CHAN_PULSE, buf);
• return err ? err : size;

+} +static DEVICE_ATTR_WO(chan_pulse);

+static ssize_t trig_filter_enable_show(struct device *dev,

• 	 struct device_attribute *attr,
  

• 	 char *buf)
  

+{

• unsigned long val;
• struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
• spin_lock(&drvdata->spinlock);
• val = drvdata->config.trig_filter_enable;
• spin_unlock(&drvdata->spinlock);
• return scnprintf(buf, PAGE_SIZE, "%ld (%s)\n", val,

• 	 val ? "enabled" : "disabled");
  

+}

+static ssize_t trig_filter_enable_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, 16, &val))

• return -EINVAL;
  

• spin_lock(&drvdata->spinlock);
• drvdata->config.trig_filter_enable = val ? 1 : 0;
• spin_unlock(&drvdata->spinlock);
• return size;

+} +static DEVICE_ATTR_RW(trig_filter_enable);

+/* clear all xtrigger / channel programming */ +static ssize_t reset_xtrigs_store(struct device *dev,

• 	       struct device_attribute *attr,
  

• 	       const char *buf, size_t size)
  

+{

• int i;
• struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
• struct cti_config *config = &drvdata->config;
• spin_lock(&drvdata->spinlock);
• /* clear the CTI trigger / channel programming registers */
• for (i = 0; i < config->nr_trig_max; i++) {

• config->ctiinen[i] = 0;
  

• config->ctiouten[i] = 0;
  

• }
• /* clear the other regs */
• config->ctigate = (0x1 << config->nr_ctm_channels) - 1;
• config->asicctl = 0;
• config->ctiappset = 0;
• config->ctiinout_sel = 0;
• /* if enabled then write through */
• if (CTI_PWR_ENA(config))

• cti_write_all_hw_regs(drvdata);
  

• spin_unlock(&drvdata->spinlock);
• return size;

+} +static DEVICE_ATTR_WO(reset_xtrigs);

+static ssize_t show_chan_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.xtrig_rchan_sel;
• return scnprintf(buf, PAGE_SIZE, "%d\n", val);

+}

+/* select a channel for the show_chan_xtrig function */ +static ssize_t show_chan_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, 16, &val))

• return -EINVAL;
  

• if (val > (drvdata->config.nr_ctm_channels-1))

• return -EINVAL;
  

• spin_lock(&drvdata->spinlock);
• drvdata->config.xtrig_rchan_sel = val;
• spin_unlock(&drvdata->spinlock);
• return size;

+} +static DEVICE_ATTR_RW(show_chan_sel);

+/* for the selected channel - show the signals attached. */ +static ssize_t show_chan_xtrigs_show(struct device *dev,

• 	 struct device_attribute *attr,
  

• 	 char *buf)
  

+{

• struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
• struct cti_config *cfg = &drvdata->config;
• int used = 0, reg_idx;
• int buf_sz = PAGE_SIZE;
• u32 chan_mask = 0x1 << cfg->xtrig_rchan_sel;
• used += scnprintf(buf, buf_sz, "IN: ");
• for (reg_idx = 0;

•     reg_idx < drvdata->config.nr_trig_max;
  

•     reg_idx++) {
  

• if (chan_mask & cfg->ctiinen[reg_idx]) {
  

• 	used += scnprintf(buf+used, buf_sz-used, "%d ",
  

• 			 reg_idx);
  

• }
  

• }
• used += scnprintf(buf+used, buf_sz-used, " OUT: ");
• for (reg_idx = 0;

•     reg_idx < drvdata->config.nr_trig_max;
  

•     reg_idx++) {
  

• if (chan_mask & cfg->ctiouten[reg_idx]) {
  

• 	used += scnprintf(buf+used, buf_sz-used, "%d ",
  

• 			 reg_idx);
  

• }
  

• }
• used += scnprintf(buf+used, buf_sz-used, "\n");
• return used;

+} +static DEVICE_ATTR_RO(show_chan_xtrigs);

+static ssize_t print_chan_list(struct device *dev,

• 	       char *buf, bool inuse)
  

+{

• struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
• struct cti_config *config = &drvdata->config;
• int used = 0, i;
• u32 inuse_bits = 0, chan_mask, chan_bit_mask;
• /* scan regs to get bitmap of channels in use. */
• spin_lock(&drvdata->spinlock);
• for (i = 0; i < config->nr_trig_max; i++) {

• inuse_bits |= config->ctiinen[i];
  

• inuse_bits |= config->ctiouten[i];
  

• }
• spin_unlock(&drvdata->spinlock);
• /* inverse bits if printing free channels */
• if (!inuse)

• inuse_bits = ~inuse_bits;
  

• /* list of channels, or 'none' */
• chan_mask = ((u32)(0x1 << config->nr_ctm_channels)) - 1;
• if (inuse_bits & chan_mask) {

• for (i = 0; i < config->nr_ctm_channels; i++) {
  

• 	chan_bit_mask = 0x1 << i;
  

• 	if (chan_bit_mask & inuse_bits) {
  

• 		used += scnprintf(buf+used, PAGE_SIZE-used,
  

• 				  "%d ", i);
  

• 	}
  

• }
  

• used += scnprintf(buf+used, PAGE_SIZE-used, "\n");
  

• } else {

• used = scnprintf(buf, PAGE_SIZE, "none\n");
  

• }
• return used;

+}

+static ssize_t list_inuse_show(struct device *dev,

• 	 struct device_attribute *attr,
  

• 	 char *buf)
  

+{

• return print_chan_list(dev, buf, true);

+} +static DEVICE_ATTR_RO(list_inuse);

+static ssize_t list_free_show(struct device *dev,

• 	 struct device_attribute *attr,
  

• 	 char *buf)
  

+{

• return print_chan_list(dev, buf, false);

+} +static DEVICE_ATTR_RO(list_free);

+static ssize_t list_gate_ena_show(struct device *dev,

• 	 struct device_attribute *attr,
  

• 	 char *buf)
  

+{

• struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
• struct cti_config *cfg = &drvdata->config;
• int b_sz = PAGE_SIZE;
• u32 chan_mask;
• int chan, used = 0;
• if (cfg->ctigate == 0) {

• used += scnprintf(buf, b_sz, "none\n");
  

• } else {

• for (chan = 0; chan < cfg->nr_ctm_channels; chan++) {
  

• 	chan_mask = 0x1 << chan;
  

• 	if (chan_mask & cfg->ctigate) {
  

• 		used += scnprintf(buf+used, b_sz-used, "%d ",
  

• 				  chan);
  

• 	}
  

• }
  

• used += scnprintf(buf+used, b_sz-used, "\n");
  

• }
• return used;

+} +static DEVICE_ATTR_RO(list_gate_ena);

+static struct attribute *coresight_cti_channel_attrs[] = {

• &dev_attr_trigin_attach.attr,
• &dev_attr_trigin_detach.attr,
• &dev_attr_trigout_attach.attr,
• &dev_attr_trigout_detach.attr,
• &dev_attr_gate_enable.attr,
• &dev_attr_gate_disable.attr,
• &dev_attr_chan_set.attr,
• &dev_attr_chan_clear.attr,
• &dev_attr_chan_pulse.attr,
• &dev_attr_trig_filter_enable.attr,
• &dev_attr_reset_xtrigs.attr,
• &dev_attr_show_chan_sel.attr,
• &dev_attr_show_chan_xtrigs.attr,
• &dev_attr_list_inuse.attr,
• &dev_attr_list_free.attr,
• &dev_attr_list_gate_ena.attr,
• NULL,

+};

+/* attribute and group sysfs tables. */ static const struct attribute_group coresight_cti_group = { .attrs = coresight_cti_attrs, }; +static const struct attribute_group coresight_cti_mgmt_group = {

• .attrs = coresight_cti_mgmt_attrs,
• .name = "mgmt",

+};

+static const struct attribute_group coresight_cti_regs_group = {

• .attrs = coresight_cti_regs_attrs,
• .name = "regs",

+};

+static const struct attribute_group coresight_cti_channels_group = {

• .attrs = coresight_cti_channel_attrs,
• .name = "channels",

+};

const struct attribute_group *coresight_cti_groups[] = { &coresight_cti_group,

• &coresight_cti_mgmt_group,
• &coresight_cti_regs_group,
• &coresight_cti_channels_group, NULL,

}; diff --git a/drivers/hwtracing/coresight/coresight-cti.c b/drivers/hwtracing/coresight/coresight-cti.c index 0a0fb6d48237..06b1f84795a4 100644 --- a/drivers/hwtracing/coresight/coresight-cti.c +++ b/drivers/hwtracing/coresight/coresight-cti.c @@ -69,7 +69,7 @@ static int cti_cpu_count; dev_get_drvdata(csdev->dev.parent) /* write set of regs to hardware - call with spinlock claimed */ -static void cti_write_all_hw_regs(struct cti_drvdata *drvdata) +void cti_write_all_hw_regs(struct cti_drvdata *drvdata) { struct cti_config *config = &drvdata->config; int i; @@ -173,6 +173,13 @@ static int cti_disable_hw(void *info) 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);

+}

static void cti_set_default_config(struct cti_config *config) { /* Most regs default to 0 as zalloc'ed except...*/ @@ -285,6 +292,178 @@ int cti_add_default_connection(struct cti_drvdata *drvdata) return ret; } +/** cti channel api **/ +/* attach/detach channel from trigger - write through if enabled. */ +int cti_channel_trig_op(struct device *dev,

• 	enum cti_chan_op op,
  

• 	enum cti_trig_dir direction,
  

• 	u32 channel_idx,
  

• 	u32 trigger_idx)
  

+{

• struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
• struct cti_config *config = &drvdata->config;
• u32 trig_bitmask;
• u32 chan_bitmask;
• u32 reg_value;
• int reg_offset;
• dev_info(dev, "chan_trig_op: op %d, dir %d, chan %d, trig %d\n",

•  op, direction, channel_idx, trigger_idx);
  

• /* ensure indexes in range */
• if ((channel_idx >= config->nr_ctm_channels) ||

•   (trigger_idx >= config->nr_trig_max))
  

• return -EINVAL;
  

• trig_bitmask = 0x1 << trigger_idx;
• /* ensure registered triggers and not out filtered */
• if (direction == CTI_TRIG_IN) {

• if (!(trig_bitmask & config->trig_in_use))
  

• 	return -EINVAL;
  

• } else {

• if (!(trig_bitmask & config->trig_out_use))
  

• 	return -EINVAL;
  

• if ((config->trig_filter_enable) &&
  

•     (config->trig_out_filter & trig_bitmask))
  

• 	return -EINVAL;
  

• }
• /* update the local register values */
• chan_bitmask = 0x1 << channel_idx;
• reg_offset = (direction == CTI_TRIG_IN ? CTIINEN(trigger_idx) :

•       CTIOUTEN(trigger_idx));
  

• spin_lock(&drvdata->spinlock);
• /* read - modify write - the trigger / channel enable value */
• reg_value = (direction == CTI_TRIG_IN ? config->ctiinen[trigger_idx] :

•      config->ctiouten[trigger_idx]);
  

• reg_value = (op == CTI_CHAN_ATTACH) ? reg_value | chan_bitmask :

• reg_value & (~chan_bitmask);
  

• /* write local copy */
• if (direction == CTI_TRIG_IN)

• config->ctiinen[trigger_idx] = reg_value;
  

• else

• config->ctiouten[trigger_idx] = reg_value;
  

• /* write through if enabled */
• if (CTI_PWR_ENA(config))

• cti_write_single_reg(drvdata, reg_offset, reg_value);
  

• spin_unlock(&drvdata->spinlock);
• return 0;

+}

+int cti_channel_gate_op(struct device *dev,

• 	enum cti_chan_gate_op op,
  

• 	u32 channel_idx)
  

+{

• struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
• struct cti_config *config = &drvdata->config;
• u32 chan_bitmask;
• u32 reg_value;
• int err = 0;
• if (channel_idx >= config->nr_ctm_channels)

• return -EINVAL;
  

• chan_bitmask = 0x1 << channel_idx;
• spin_lock(&drvdata->spinlock);
• reg_value = config->ctigate;
• switch (op) {
• case CTI_GATE_CHAN_ENABLE:

• reg_value |= chan_bitmask;
  

• break;
  

• case CTI_GATE_CHAN_DISABLE:

• reg_value &= ~chan_bitmask;
  

• break;
  

• case CTI_GATE_CHAN_ENABLE_ALL:

• reg_value = (0x1 << config->nr_ctm_channels) - 1;
  

• break;
  

• case CTI_GATE_CHAN_DISABLE_ALL:

• reg_value = 0x0;
  

• break;
  

• default:

• err = -EINVAL;
  

• break;
  

• }
• if (err == 0) {

• config->ctigate = reg_value;
  

• if (CTI_PWR_ENA(config))
  

• 	cti_write_single_reg(drvdata, CTIGATE, reg_value);
  

• }
• spin_unlock(&drvdata->spinlock);
• return err;

+}

+int cti_channel_setop(struct device *dev,

•      enum cti_chan_set_op op,
  

•      u32 channel_idx)
  

+{

• struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
• struct cti_config *config = &drvdata->config;
• u32 chan_bitmask;
• u32 reg_value;
• u32 reg_offset;
• int err = 0;
• if (channel_idx >= config->nr_ctm_channels)

• return -EINVAL;
  

• chan_bitmask = 0x1 << channel_idx;
• spin_lock(&drvdata->spinlock);
• reg_value = config->ctiappset;
• switch (op) {
• case CTI_CHAN_SET:

• config->ctiappset |= chan_bitmask;
  

• reg_value  = config->ctiappset;
  

• reg_offset = CTIAPPSET;
  

• break;
  

• case CTI_CHAN_CLR:

• config->ctiappset &= ~chan_bitmask;
  

• reg_value = chan_bitmask;
  

• reg_offset = CTIAPPCLEAR;
  

• break;
  

• case CTI_CHAN_PULSE:

• config->ctiappset &= ~chan_bitmask;
  

• reg_value = chan_bitmask;
  

• reg_offset = CTIAPPPULSE;
  

• break;
  

• default:

• err = -EINVAL;
  

• break;
  

• }
• if ((err == 0) && CTI_PWR_ENA(config))

• cti_write_single_reg(drvdata, reg_offset, reg_value);
  

• spin_unlock(&drvdata->spinlock);
• return err;

+}

+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);

+}

/** cti ect operations **/ int cti_enable(struct coresight_device *csdev, void *__unused) { @@ -432,7 +611,12 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id) /* all done - dec pm refcount */ pm_runtime_put(&adev->dev);

• dev_info(dev, "%s: initialized\n", pdata->name);

• if (drvdata->ctidev.cpu >= 0) {

• dev_info(dev, "%s: cti-CPU%d initialized\n", pdata->name,
  

• 	 drvdata->ctidev.cpu);
  

• } else {

• dev_info(dev, "%s: cti-system initialized\n", pdata->name);
  

• }

Please move this to the previous patch.

cti_count++; return 0; diff --git a/drivers/hwtracing/coresight/coresight-cti.h b/drivers/hwtracing/coresight/coresight-cti.h index c1af6cf9c2fe..a5bf59155f62 100644 --- a/drivers/hwtracing/coresight/coresight-cti.h +++ b/drivers/hwtracing/coresight/coresight-cti.h @@ -153,6 +153,7 @@ struct cti_device {

• @trig_out_filter: bitfield of out triggers that are blocked if filter
• enabled. Typically this would be dbgreq / restart on a core CTI.
• @trig_filter_enable: 1 if filtering enabled.

• • @xtrig_rchan_sel: channel ID when reading set xtrig info
  • cti software programmable regs:
  • @ctiappset: CTI Software application channel set.

@@ -174,6 +175,7 @@ struct cti_config { u32 trig_out_use; u32 trig_out_filter; int trig_filter_enable;

• u8 xtrig_rchan_sel;

This should go in the previous patch.

/* cti cross trig programmable regs */ u32 ctiappset; u8 ctiinout_sel; @@ -202,11 +204,51 @@ struct cti_drvdata { struct cti_config config; }; +/*

• • Channel operation types.
• */

+enum cti_chan_op {

• CTI_CHAN_ATTACH,
• CTI_CHAN_DETACH,

+};

+enum cti_trig_dir {

• CTI_TRIG_IN,
• CTI_TRIG_OUT,

+};

+enum cti_chan_gate_op {

• CTI_GATE_CHAN_ENABLE,
• CTI_GATE_CHAN_DISABLE,
• CTI_GATE_CHAN_ENABLE_ALL,
• CTI_GATE_CHAN_DISABLE_ALL,

+};

+enum cti_chan_set_op {

• CTI_CHAN_SET,
• CTI_CHAN_CLR,
• CTI_CHAN_PULSE,

+};

/* private cti driver fns & vars */ extern const struct attribute_group *coresight_cti_groups[];

Extra newline

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_all_hw_regs(struct cti_drvdata *drvdata); +void cti_write_single_reg(struct cti_drvdata *drvdata, int offset, u32 value); +int cti_channel_trig_op(struct device *dev, enum cti_chan_op op,

• 	enum cti_trig_dir direction, u32 channel_idx,
  

• 	u32 trigger_idx);
  

+int cti_channel_gate_op(struct device *dev, enum cti_chan_gate_op op,

• 	u32 channel_idx);
  

+int cti_channel_setop(struct device *dev, enum cti_chan_set_op op,

•       u32 channel_idx);
  

+void cti_write_intack(struct device *dev, u32 ackval);

+/* cti powered and enabled */ +#define CTI_PWR_ENA(p_cfg) (p_cfg->hw_enabled && p_cfg->hw_powered) #ifdef CONFIG_OF extern int of_cti_get_hw_data(struct device *dev, diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h index 8e9e5b6ad866..9b497e2f8725 100644 --- a/drivers/hwtracing/coresight/coresight-priv.h +++ b/drivers/hwtracing/coresight/coresight-priv.h @@ -23,10 +23,12 @@ #define CORESIGHT_LAR 0xfb0 #define CORESIGHT_LSR 0xfb4 #define CORESIGHT_AUTHSTATUS 0xfb8 +#define CORESIGHT_DEVARCH 0xfbc #define CORESIGHT_DEVID 0xfc8 #define CORESIGHT_DEVTYPE 0xfcc

Extra newline

/*

• Coresight device CLAIM protocol.
• See PSCI - ARM DEN 0022D, Section: 6.8.1 Debug and Trace save and restore.

-- 2.20.1