Re: [RFC PATCH v4 09/15] coresight: cti: Add connection information to sysfs

Fixed as suggested.

Thanks

Mike

On Thu, 3 Oct 2019 at 22:22, Mathieu Poirier mathieu.poirier@linaro.org wrote:

On Mon, Aug 19, 2019 at 10:13:55PM +0100, Mike Leach wrote:

...

Dynamically adds sysfs attributes for all connections defined in the CTI.

Each connection has a triggers<N> sub-directory with name, in_signals, in_types, out_signals and out_types as read-only parameters in the directory. in_ or out_ parameters may be omitted if there are no in or out signals for the connection.

Additionally each device has a nr_cons and trigout_filtered parameter in the connections sub-directory. . This allows clients to explore the connection and trigger signal details without needing to refer to device tree or specification of the device.

Standardised type information is provided for certain common functions - e.g. snk_full for a trigger from a sink indicating full. Otherwise type defaults to genio.

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

.../hwtracing/coresight/coresight-cti-sysfs.c | 343 +++++++++++++++++- drivers/hwtracing/coresight/coresight-cti.c | 13 +- drivers/hwtracing/coresight/coresight-cti.h | 6 + 3 files changed, 360 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c b/drivers/hwtracing/coresight/coresight-cti-sysfs.c index dbbfd1de58b3..a7ae479ee2b9 100644 --- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c @@ -829,7 +829,342 @@ static struct attribute *coresight_cti_channel_attrs[] = { NULL, };

-/* sysfs groups */ +/* Create the connections trigger groups and attrs dynamically */

+/*

• • Each connection has dynamic group + name, trigin/out sigs/types attrs,
• • • each device has static nr_conns + filter attr
• */

+static ssize_t trigout_filtered_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 size = 0, nr_trig_max = cfg->nr_trig_max;
  

• unsigned long mask = cfg->trig_out_filter;
  

• if (mask)
  

•         size = bitmap_print_to_pagebuf(true, buf, &mask, nr_trig_max);
  

• else
  

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

• return size;
  

+} +static DEVICE_ATTR_RO(trigout_filtered);

+static ssize_t nr_cons_show(struct device *dev,

•                     struct device_attribute *attr,
  

•                     char *buf)
  

+{

• struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
  

• return scnprintf(buf, PAGE_SIZE, "%d\n", drvdata->ctidev.nr_trig_con);
  

+} +static DEVICE_ATTR_RO(nr_cons);

+static struct attribute *coresight_cti_cons_attrs[] = {

• &dev_attr_trigout_filtered.attr,
  

• &dev_attr_nr_cons.attr,
  

After reviewing the CS sysfs topology set [1] I question the choice of lumping trigout_filtered under "connections" - there is very little semantic linking them together. Why not moving it under "channels" where it would show next to "trig_filter_enable"?

[1]. https://lists.linaro.org/pipermail/coresight/2019-August/003199.html

...

• NULL,
  

+};

+static ssize_t con_name_show(struct device *dev,

•                      struct device_attribute *attr,
  

•                      char *buf)
  

+{

• struct dev_ext_attribute *ext_attr = (struct dev_ext_attribute *)attr;
  

• struct cti_trig_con *con = (struct cti_trig_con *)ext_attr->var;
  

• return scnprintf(buf, PAGE_SIZE, "%s\n", con->con_dev_name);
  

+}

+static ssize_t trigin_sig_show(struct device *dev,

•                        struct device_attribute *attr,
  

•                        char *buf)
  

+{

• struct dev_ext_attribute *ext_attr = (struct dev_ext_attribute *)attr;
  

• struct cti_trig_con *con = (struct cti_trig_con *)ext_attr->var;
  

• struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
  

• struct cti_config *cfg = &drvdata->config;
  

• unsigned long mask = con->con_in->used_mask;
  

• return bitmap_print_to_pagebuf(true, buf, &mask, cfg->nr_trig_max);
  

+}

+static ssize_t trigout_sig_show(struct device *dev,

•                         struct device_attribute *attr,
  

•                         char *buf)
  

+{

• struct dev_ext_attribute *ext_attr = (struct dev_ext_attribute *)attr;
  

• struct cti_trig_con *con = (struct cti_trig_con *)ext_attr->var;
  

• struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
  

• struct cti_config *cfg = &drvdata->config;
  

• unsigned long mask = con->con_out->used_mask;
  

• return bitmap_print_to_pagebuf(true, buf, &mask, cfg->nr_trig_max);
  

+}

+/* convert a sig type id to a name */ +static const char * +cti_sig_type_name(struct cti_trig_con *con, int used_count, bool in) +{

• static const char * const sig_type_names[] = {
  

•         "genio", "intreq", "intack", "haltreq", "restartreq",
  

•         "pe_edbgreq", "pe_dbgrestart", "pe_ctiirq", "pe_pmuirq",
  

•         "pe_dbgtrigger", "etm_extout",  "etm_extin", "snk_full",
  

•         "snk_acqcomp", "snk_flushcomp", "snk_flushin", "snk_trigin",
  

•         "stm_asyncout", "stm_tout_spte", "stm_tout_sw", "stm_tout_hete",
  

•         "stm_hwevent", "ela_tstart", "ela_tstop", "ela_dbgreq",
  

• };
  

• int idx = 0;
  

• struct cti_trig_grp *grp = in ? con->con_in : con->con_out;
  

• if (grp->sig_types) {
  

•         if (used_count < grp->nr_sigs)
  

•                 idx = grp->sig_types[used_count];
  

• }
  

• return sig_type_names[idx];
  

+}

+static ssize_t trigin_type_show(struct device *dev,

•                         struct device_attribute *attr,
  

•                         char *buf)
  

+{

• struct dev_ext_attribute *ext_attr = (struct dev_ext_attribute *)attr;
  

• struct cti_trig_con *con = (struct cti_trig_con *)ext_attr->var;
  

• int sig_idx, used = 0, b_sz = PAGE_SIZE;
  

• const char *name;
  

• for (sig_idx = 0; sig_idx < con->con_in->nr_sigs; sig_idx++) {
  

•         name = cti_sig_type_name(con, sig_idx, true);
  

•         used += scnprintf(buf+used, b_sz-used, "%s ", name);
  

• }
  

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

• return used;
  

+}

+static ssize_t trigout_type_show(struct device *dev,

•                          struct device_attribute *attr,
  

•                          char *buf)
  

+{

• struct dev_ext_attribute *ext_attr = (struct dev_ext_attribute *)attr;
  

• struct cti_trig_con *con = (struct cti_trig_con *)ext_attr->var;
  

• int sig_idx, used = 0, b_sz = PAGE_SIZE;
  

• const char *name;
  

• for (sig_idx = 0; sig_idx < con->con_out->nr_sigs; sig_idx++) {
  

•         name = cti_sig_type_name(con, sig_idx, false);
  

•         used += scnprintf(buf+used, b_sz-used, "%s ", name);
  

• }
  

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

• return used;
  

+}

+typedef ssize_t (*p_show_fn)(struct device *dev, struct device_attribute *attr,

•                      char *buf);
  

+enum cti_conn_attr_type {

• CTI_CON_ATTR_NAME,
  

• CTI_CON_ATTR_TRIGIN_SIG,
  

• CTI_CON_ATTR_TRIGOUT_SIG,
  

• CTI_CON_ATTR_TRIGIN_TYPES,
  

• CTI_CON_ATTR_TRIGOUT_TYPES
  

+};

+static p_show_fn show_fns[] = {

• con_name_show,
  

• trigin_sig_show,
  

• trigout_sig_show,
  

• trigin_type_show,
  

• trigout_type_show,
  

+};

+static const char * const base_names[] = {

• "name",
  

• "in_signals",
  

• "out_signals",
  

• "in_types",
  

• "out_types",
  

+};

+static int cti_create_con_sysfs_attr(struct cti_trig_con *con,

•                              enum cti_conn_attr_type attr_type,
  

•                              int attr_idx)
  

+{

• struct dev_ext_attribute *dev_ext_attr = 0;
  

• char *name = 0;
  

• dev_ext_attr = kzalloc(sizeof(struct dev_ext_attribute), GFP_KERNEL);
  

• if (dev_ext_attr) {
  

•         name = kzalloc(sizeof(char) * 16, GFP_KERNEL);
  

•         if (name) {
  

•                 /* fill out the underlying attribute struct */
  

•                 sprintf(name, "%s", base_names[attr_type]);
  

•                 dev_ext_attr->attr.attr.name = name;
  

•                 dev_ext_attr->attr.attr.mode = 0444;
  

•                 /* now the device_attribute struct */
  

•                 dev_ext_attr->attr.show = show_fns[attr_type];
  

•         } else {
  

•                 kfree(dev_ext_attr);
  

•                 return -ENOMEM;
  

•         }
  

• } else {
  

•         return -ENOMEM;
  

• }
  

• dev_ext_attr->var = con;
  

• con->con_attrs[attr_idx] = dev_ext_attr;
  

• return 0;
  

+}

+/* number of static groups (5) declared below + null terminator */ +#define NR_STATIC_GROUPS 5

+static struct attribute_group * +cti_create_con_sysfs_group(struct cti_device *ctidev, int con_idx) +{

• struct attribute_group *group = NULL;
  

• group = kzalloc(sizeof(struct attribute_group), GFP_KERNEL);
  

• if (!group)
  

•         return NULL;
  

• group->name = kasprintf(GFP_KERNEL, "triggers%d", con_idx);
  

• if (!group->name) {
  

•         kfree(group);
  

•         return NULL;
  

• }
  

• ctidev->con_groups[con_idx + NR_STATIC_GROUPS] = group;
  

• return group;
  

+}

+/* dynamic attr - 5 per connection + null terminator */ +#define CTI_SYSFS_CONS_DYN_ATTR 5

+/* create a triggers connection group and the attributes for that group */ +static int cti_create_con_attr_set(int con_idx, struct cti_device *ctidev,

•                            struct cti_trig_con *con)
  

+{

• struct attribute_group *attr_group = NULL;
  

• int attr_idx = 0;
  

• int err = -ENOMEM;
  

• attr_group = cti_create_con_sysfs_group(ctidev, con_idx);
  

• if (!attr_group)
  

•         return -ENOMEM;
  

• con->con_attrs = kcalloc(CTI_SYSFS_CONS_DYN_ATTR + 1,
  

•                          sizeof(struct dev_ext_attribute *),
  

•                          GFP_KERNEL);
  

• if (!con->con_attrs)
  

•         return -ENOMEM;
  

• err = cti_create_con_sysfs_attr(con, CTI_CON_ATTR_NAME, attr_idx++);
  

• if (err)
  

•         return err;
  

• if (con->con_in->nr_sigs > 0) {
  

•         err = cti_create_con_sysfs_attr(con, CTI_CON_ATTR_TRIGIN_SIG,
  

•                                         attr_idx++);
  

•         if (err)
  

•                 return err;
  

•         err = cti_create_con_sysfs_attr(con, CTI_CON_ATTR_TRIGIN_TYPES,
  

•                                         attr_idx++);
  

•         if (err)
  

•                 return err;
  

• }
  

• if (con->con_in->nr_sigs > 0) {
  

•         err = cti_create_con_sysfs_attr(con, CTI_CON_ATTR_TRIGOUT_SIG,
  

•                                         attr_idx++);
  

•         if (err)
  

•                 return err;
  

•         err = cti_create_con_sysfs_attr(con, CTI_CON_ATTR_TRIGOUT_TYPES,
  

•                                         attr_idx++);
  

•         if (err)
  

•                 return err;
  

• }
  

• attr_group->attrs = (struct attribute **)con->con_attrs;
  

• return 0;
  

+}

+/* create the array of group pointers for the CTI sysfs groups */ +int cti_create_cons_groups(struct cti_device *ctidev) +{

• int i, nr_groups;
  

• /* nr groups - dynamic + static + NULL terminator */
  

• nr_groups = ctidev->nr_trig_con + NR_STATIC_GROUPS + 1;
  

• ctidev->con_groups = kcalloc(nr_groups,
  

•                              sizeof(struct attribute_group *),
  

•                              GFP_KERNEL);
  

• if (!ctidev->con_groups)
  

•         return -ENOMEM;
  

• /* populate first locations with the static set of groups */
  

• for (i = 0; i < NR_STATIC_GROUPS; i++)
  

•         ctidev->con_groups[i] = coresight_cti_groups[i];
  

• return 0;
  

+}

+int cti_create_cons_sysfs(struct cti_drvdata *drvdata) +{

• struct cti_device *ctidev = &drvdata->ctidev;
  

• int err, con_idx = 0;
  

• struct cti_trig_con *tc = NULL;
  

• err = cti_create_cons_groups(ctidev);
  

• if (err)
  

•         return err;
  

• /* add dynamic set for each connection */
  

• list_for_each_entry(tc, &ctidev->trig_cons, node) {
  

•         err = cti_create_con_attr_set(con_idx++, ctidev, tc);
  

•         if (err)
  

•                 goto cons_sysfs_err;
  

• }
  

• return 0;
  

+cons_sysfs_err:

• cti_destroy_cons_sysfs(ctidev);
  

• return err;
  

+}

+void cti_destroy_cons_attr_set(int con_idx, struct cti_device *ctidev,

•                        struct cti_trig_con *con)
  

+{

• int i;
  

• if (con->con_attrs) {
  

•         for (i = 0; i <= CTI_SYSFS_CONS_DYN_ATTR; i++) {
  

•                 if (con->con_attrs[i]) {
  

•                         kfree(con->con_attrs[i]->attr.attr.name);
  

•                         kfree(con->con_attrs[i]);
  

•                 }
  

•         }
  

•         kfree(con->con_attrs);
  

• }
  

• kfree(ctidev->con_groups[con_idx + NR_STATIC_GROUPS]);
  

+}

+void cti_destroy_cons_sysfs(struct cti_device *ctidev) +{

• struct cti_trig_con *tc;
  

• int con_idx = 0;
  

• list_for_each_entry(tc, &ctidev->trig_cons, node) {
  

•         cti_destroy_cons_attr_set(con_idx++, ctidev, tc);
  

• }
  

• kfree(ctidev->con_groups);
  

+}

+/* attribute and group sysfs tables. */ static const struct attribute_group coresight_cti_group = { .attrs = coresight_cti_attrs, }; @@ -849,10 +1184,16 @@ static const struct attribute_group coresight_cti_channels_group = { .name = "channels", };

+static struct attribute_group coresight_cti_conns_group = {

• .attrs = coresight_cti_cons_attrs,
  

• .name = "connections",
  

+};

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

• &coresight_cti_conns_group,
  NULL,
  

}; diff --git a/drivers/hwtracing/coresight/coresight-cti.c b/drivers/hwtracing/coresight/coresight-cti.c index 927f58af92a2..dce02f08ece1 100644 --- a/drivers/hwtracing/coresight/coresight-cti.c +++ b/drivers/hwtracing/coresight/coresight-cti.c @@ -571,6 +571,9 @@ static void cti_device_release(struct device *dev)

  mutex_lock(&ect_mutex);

• /* clear the dynamic sysfs associate with connections */
  

• cti_destroy_cons_sysfs(&drvdata->ctidev);
  

• /* remove from the list */
  list_for_each_entry_safe(ect_item, ect_tmp, &ect_net, node) {
          if (ect_item == drvdata) {
  

@@ -647,12 +650,20 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id) goto err_out; }

• /* create dynamic attributes for connections */
  

• ret = cti_create_cons_sysfs(drvdata);
  

• if (ret) {
  

•         pr_err("%s: create dynamic sysfs entries failed\n",
  

•                cti_desc.name);
  

•         goto err_out;
  

• }
  

• /* set up coresight component description */
  cti_desc.pdata = pdata;
  cti_desc.type = CORESIGHT_DEV_TYPE_ECT;
  cti_desc.subtype.ect_subtype = CORESIGHT_DEV_SUBTYPE_ECT_CTI;
  cti_desc.ops = &cti_ops;
  

• cti_desc.groups = coresight_cti_groups;
  

• cti_desc.groups = drvdata->ctidev.con_groups;
  cti_desc.dev = dev;
  drvdata->csdev = coresight_register(&cti_desc);
  if (IS_ERR(drvdata->csdev)) {
  

diff --git a/drivers/hwtracing/coresight/coresight-cti.h b/drivers/hwtracing/coresight/coresight-cti.h index 90622f3be12b..95ed026a2a61 100644 --- a/drivers/hwtracing/coresight/coresight-cti.h +++ b/drivers/hwtracing/coresight/coresight-cti.h @@ -74,6 +74,7 @@ struct cti_trig_grp {

• @con_dev: coresight device connected to the CTI, NULL if not CS device
• @con_dev_name: name of connected device (CS or CPU)
• @node: entry node in list of connections.

• • @con_attrs: Dynamic sysfs attributes specific to this connection.
  */

struct cti_trig_con { struct cti_trig_grp *con_in; @@ -81,6 +82,7 @@ struct cti_trig_con { struct coresight_device *con_dev; char *con_dev_name; struct list_head node;

• struct dev_ext_attribute **con_attrs;
  

};

/** @@ -91,12 +93,14 @@ struct cti_trig_con {

•      assumed there is a single CTM per SoC, ID 0).
  

• @trig_cons: list of connections to this device.
• @cpu: CPU ID if associated with CPU, -1 otherwise.

• • @con_groups: Dynamic sysfs groups for trigger connections.
  */

struct cti_device { int nr_trig_con; u32 ctm_id; struct list_head trig_cons; int cpu;

• const struct attribute_group **con_groups;
  

};

/** @@ -222,6 +226,8 @@ 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); +int cti_create_cons_sysfs(struct cti_drvdata *drvdata); +void cti_destroy_cons_sysfs(struct cti_device *ctidev); struct coresight_platform_data * coresight_cti_get_platform_data(struct device *dev); const char *coresight_cti_get_node_name(struct device *dev); -- 2.17.1

-- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK