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

On 13/01/2020 21:31, 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 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 Reviewed-by: Mathieu Poirier mathieu.poirier@linaro.org

---

.../hwtracing/coresight/coresight-cti-sysfs.c | 333 +++++++++++++++++- drivers/hwtracing/coresight/coresight-cti.c | 10 +- drivers/hwtracing/coresight/coresight-cti.h | 8 + 3 files changed, 348 insertions(+), 3 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c b/drivers/hwtracing/coresight/coresight-cti-sysfs.c index 8af1986ed69f..db3395c46f51 100644 --- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c @@ -8,6 +8,67 @@ #include "coresight-cti.h"

+/*

• • Declare the number of static declared attribute groups
• • Value includes groups + NULL value at end of table.
• */

+#define CORESIGHT_CTI_STATIC_GROUPS_MAX 5

minor nit: see below.

+static struct attribute_group * +cti_create_con_sysfs_group(struct device *dev, struct cti_device *ctidev,

• 	   int con_idx, struct cti_trig_con *tc)
  

+{

• struct attribute_group *group = NULL;
• int grp_idx;
• group = devm_kzalloc(dev, sizeof(struct attribute_group), GFP_KERNEL);
• if (!group)

• return NULL;
  

• group->name = devm_kasprintf(dev, GFP_KERNEL, "triggers%d", con_idx);
• if (!group->name)

• return NULL;
  

• grp_idx = con_idx + CORESIGHT_CTI_STATIC_GROUPS_MAX - 1;
• ctidev->con_groups[grp_idx] = group;
• tc->attr_group = group;
• return group;

+}

• if (tc->con_out->nr_sigs > 0) {

• err = cti_create_con_sysfs_attr(dev, tc,
  

• 				CTI_CON_ATTR_TRIGOUT_SIG,
  

• 				attr_idx++);
  

• if (err)
  

• 	return err;
  

• err = cti_create_con_sysfs_attr(dev, tc,
  

• 				CTI_CON_ATTR_TRIGOUT_TYPES,
  

• 				attr_idx++);
  

• if (err)
  

• 	return err;
  

• }
• attr_group->attrs = tc->con_attrs;
• return 0;

+}

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

• int nr_groups;
• /* nr groups - dynamic + static + NULL terminator */

nit: nr_groups = ?

• nr_groups = ctidev->nr_trig_con + CORESIGHT_CTI_STATIC_GROUPS_MAX;
• ctidev->con_groups = devm_kcalloc(dev, nr_groups,

• 			  sizeof(struct attribute_group *),
  

• 			  GFP_KERNEL);
  

• if (!ctidev->con_groups)

• return -ENOMEM;
  

• return 0;

+}

+/* attribute and group sysfs tables. */ static const struct attribute_group coresight_cti_group = { .attrs = coresight_cti_attrs, }; @@ -837,7 +1165,8 @@ static const struct attribute_group coresight_cti_channels_group = { .name = "channels", }; -const struct attribute_group *coresight_cti_groups[] = { +const struct attribute_group * +coresight_cti_groups[CORESIGHT_CTI_STATIC_GROUPS_MAX] = { &coresight_cti_group, &coresight_cti_mgmt_group, &coresight_cti_regs_group,

minor nit: You may as well do :

corsight_cti_groups[] = { ... .... }

#define CORESIGHT_CTI_STATIC_GROUPS_MAX \ ARRAY_SIZE(coresight_cti_groups) + 1

To avoid hard coding the number 5. Of course that would involve a forward declaration, so can ignore it for the time being.

With the comment fixed, above:

Reviewed-by: Suzuki K Poulose suzuki.poulose@arm.com